fix(s3): adapt CopyObject to multistore's copy-source path mapping

.github/workflows/staging.yml

@@ -47,14 +47,47 @@ jobs:
     if: vars.FEDERATION_TEST_ACCOUNT != '' && vars.FEDERATION_TEST_PRODUCT != '' && vars.FEDERATION_TEST_KEY != ''
     runs-on: ubuntu-latest
     timeout-minutes: 10
+    permissions:
+      contents: read
+      # Mints the caller identity for the copy-source authz test; see below.
+      id-token: write
     steps:
       - uses: actions/checkout@v4
       - uses: astral-sh/setup-uv@v5
+      - name: Mint caller identity token (GitHub OIDC)
+        # A GitHub Actions OIDC token, minted per run — short-lived by design
+        # and never stored, unlike a token parked in a repo secret.
+        #
+        # Dormant until Source registers GitHub as a valid IdP, so that products
+        # can accept writes from GitHub Actions. Two things must land first:
+        # the deployment's AUTH_ISSUER must accept GitHub's issuer (today it is
+        # a single Ory URL — src/config.rs reads AUTH_ISSUER as one String,
+        # unlike the comma-separated AUTH_AUDIENCE, so this needs a code change
+        # too), and the audience Source expects must be set as
+        # FEDERATION_TEST_AUDIENCE. Until then no token is minted and the
+        # copy-source authz test skips; the rest of the suite is unaffected.
+        if: vars.FEDERATION_TEST_AUDIENCE != ''
+        run: |
+          set -euo pipefail
+          token=$(curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \
+            "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=${{ vars.FEDERATION_TEST_AUDIENCE }}" \
+            | jq -r '.value')
+          if [ -z "$token" ] || [ "$token" = "null" ]; then
+            echo "::error::FEDERATION_TEST_AUDIENCE is set but minting returned no token"
+            exit 1
+          fi
+          echo "::add-mask::$token"
+          echo "CI_WRITE_ID_TOKEN=$token" >> "$GITHUB_ENV"
       - name: Run federation smoke tests
         env:
           PROXY_URL: ${{ needs.deploy.outputs.deploy_url }}
           FEDERATION_TEST_ACCOUNT: ${{ vars.FEDERATION_TEST_ACCOUNT }}
           FEDERATION_TEST_PRODUCT: ${{ vars.FEDERATION_TEST_PRODUCT }}
           FEDERATION_TEST_KEY: ${{ vars.FEDERATION_TEST_KEY }}
           FEDERATION_RESTRICTED_PRODUCT: ${{ vars.FEDERATION_RESTRICTED_PRODUCT }}
-        run: uvx --with requests pytest tests/test_federation.py -v
+          FEDERATION_WRITE_PRODUCT: ${{ vars.FEDERATION_WRITE_PRODUCT }}
+          # Set by the mint step above, and only when it runs.
+          CI_WRITE_ID_TOKEN: ${{ env.CI_WRITE_ID_TOKEN }}
+        # boto3: the copy-source authz test signs SigV4 through the AWS SDK
+        # rather than hand-rolling requests.
+        run: uvx --with requests --with boto3 pytest tests/test_federation.py -v

Cargo.toml

@@ -36,10 +36,10 @@ path = "tests/fixtures.rs"
 
 [dependencies]
 # Multistore
-multistore = { version = "0.7.0", features = ["azure", "gcp"] }
-multistore-oidc-provider = "0.7.0"
-multistore-path-mapping = "0.7.0"
-multistore-sts = "0.7.0"
+multistore = { version = "0.7.1", features = ["azure", "gcp"] }
+multistore-oidc-provider = "0.7.1"
+multistore-path-mapping = "0.7.1"
+multistore-sts = "0.7.1"
 
 # Serialization
 serde = { version = "1", features = ["derive"] }
@@ -67,7 +67,7 @@ getrandom = { version = "0.4", features = ["wasm_js"] }
 # ring is transitive (via object_store); this direct entry only turns the
 # feature on for the wasm target.
 ring = { version = "0.17", features = ["wasm32_unknown_unknown_js"] }
-multistore-cf-workers = { version = "0.7.0", features = ["azure", "gcp"] }
+multistore-cf-workers = { version = "0.7.1", features = ["azure", "gcp"] }
 # On wasm32-unknown-unknown reqwest selects its browser `fetch` backend by
 # target arch (not a cargo feature), so the default TLS/backend features are
 # unnecessary here. The `form` feature gates `.form()`, which the STS
@@ -86,3 +86,4 @@ web-sys = { version = "0.3", features = [
 ] }
 worker = { version = "=0.7.5", features = ["http"] }
 worker-macros = { version = "=0.7.5", features = ["http"] }
+

src/lib.rs

@@ -35,7 +35,7 @@ use multistore_oidc_provider::{HttpExchange, OidcCredentialProvider, OidcProvide
 use multistore_path_mapping::{MappedRegistry, PathMapping};
 use multistore_sts::jwks::JwksCache;
 use multistore_sts::route_handler::StsRouterExt;
-use object_path::{extract_path_segments, is_keyless_write};
+use object_path::{extract_path_segments, is_keyless_write, mapped_copy_source};
 use std::sync::OnceLock;
 use sts::StsCredentialRegistry;
 use worker::{event, Context, Env, Result};
@@ -316,6 +316,11 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result<web_sys:
         .map(|u| u.path().to_string())
         .unwrap_or_else(|_| rewrite.signing_path.clone());
 
+    // See `object_path::mapped_copy_source`: the copy source must be mapped
+    // into the registry's namespace, without mutating the header the client
+    // signed over.
+    let copy_source = mapped_copy_source(&parts.headers, &mapping);
+
     let request_info = RequestInfo::new(
         &parts.method,
         &rewrite.path,
@@ -325,7 +330,8 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result<web_sys:
     )
     .with_signing_path(&signing_path)
     .with_signing_query(rewrite.signing_query.as_deref())
-    .with_form_body(parts.form_body.as_deref());
+    .with_form_body(parts.form_body.as_deref())
+    .with_copy_source(copy_source.as_deref());
 
     let start_ms = js_sys::Date::now();
     let response = gateway

src/object_path.rs

@@ -23,6 +23,32 @@ pub(crate) fn extract_path_segments(path: &str) -> (Option<&str>, Option<&str>,
     (account, product, key)
 }
 
+/// Map an inbound `x-amz-copy-source` header into the registry's namespace.
+///
+/// `CopyObject` names its source in a header rather than the URL, in client
+/// coordinates (`/{account}/{product}/{key}`). The registry only knows mapped
+/// bucket names (`account:product`), so an unmapped source resolves to a bucket
+/// it has never heard of and the copy fails with 404 NoSuchBucket. The client
+/// signed the header, so it must not be mutated — the mapped value produced
+/// here is passed alongside it via `RequestInfo::with_copy_source`, and
+/// signature verification keeps using the header as sent.
+///
+/// Returns `None` when there is no copy-source header (the request isn't a
+/// copy) or when the value can't be mapped — too few segments to name an
+/// object, or not valid UTF-8. `None` leaves multistore reading the header
+/// as-is, which is the correct fallback: a non-copy request has nothing to map,
+/// and an unmappable source should fail on its own merits rather than on a
+/// silently-invented bucket name.
+pub(crate) fn mapped_copy_source(
+    headers: &http::HeaderMap,
+    mapping: &multistore_path_mapping::PathMapping,
+) -> Option<String> {
+    headers
+        .get("x-amz-copy-source")
+        .and_then(|v| v.to_str().ok())
+        .and_then(|v| mapping.rewrite_copy_source(v))
+}
+
 /// Whether `method` writes to a single object but `path` carries no object key.
 ///
 /// `PUT`/`DELETE` (PutObject/DeleteObject) address one object, so they need a

tests/object_path.rs

@@ -6,7 +6,81 @@
 mod object_path;
 
 use http::Method;
-use object_path::{extract_path_segments, is_keyless_write};
+use object_path::{extract_path_segments, is_keyless_write, mapped_copy_source};
+
+/// The deployment's real mapping: `/{account}/{product}/{key}` folds the first
+/// two segments into the internal bucket `account:product`.
+fn mapping() -> multistore_path_mapping::PathMapping {
+    multistore_path_mapping::PathMapping {
+        bucket_segments: 2,
+        bucket_separator: ":".to_string(),
+        display_bucket_segments: 1,
+    }
+}
+
+fn headers(copy_source: Option<&str>) -> http::HeaderMap {
+    let mut h = http::HeaderMap::new();
+    if let Some(v) = copy_source {
+        h.insert("x-amz-copy-source", v.parse().unwrap());
+    }
+    h
+}
+
+/// The whole point: a client-coordinate copy source (`/account/product/key`)
+/// becomes a registry-coordinate one (`account:product/key`). Without this the
+/// source names a bucket the registry has never heard of and the copy 404s.
+#[test]
+fn copy_source_is_folded_into_the_internal_bucket_name() {
+    assert_eq!(
+        mapped_copy_source(&headers(Some("/acct/prod/README.md")), &mapping()),
+        Some("/acct:prod/README.md".to_string())
+    );
+    // Leading slash is optional in `x-amz-copy-source`; both forms map alike.
+    assert_eq!(
+        mapped_copy_source(&headers(Some("acct/prod/README.md")), &mapping()),
+        Some("/acct:prod/README.md".to_string())
+    );
+    // Nested keys keep every segment past the bucket.
+    assert_eq!(
+        mapped_copy_source(&headers(Some("/acct/prod/a/b/c.txt")), &mapping()),
+        Some("/acct:prod/a/b/c.txt".to_string())
+    );
+}
+
+/// `versionId` rides along untouched — multistore#129 authorizes the source
+/// against that version, so losing it here would silently copy the wrong one.
+/// Percent-encoding is likewise preserved rather than decoded.
+#[test]
+fn copy_source_preserves_version_and_encoding() {
+    assert_eq!(
+        mapped_copy_source(
+            &headers(Some("/acct/prod/a%20b.txt?versionId=v42")),
+            &mapping()
+        ),
+        Some("/acct:prod/a%20b.txt?versionId=v42".to_string())
+    );
+}
+
+/// No header means the request isn't a copy: nothing to map, and multistore
+/// must be left reading the (absent) header itself rather than handed a value.
+#[test]
+fn absent_copy_source_maps_to_none() {
+    assert_eq!(mapped_copy_source(&headers(None), &mapping()), None);
+}
+
+/// Too few segments to name an object inside a product. Mapping these would
+/// invent a bucket name; `None` lets them fail on their own merits instead.
+#[test]
+fn unmappable_copy_source_maps_to_none() {
+    assert_eq!(
+        mapped_copy_source(&headers(Some("/acct/prod")), &mapping()),
+        None
+    );
+    assert_eq!(
+        mapped_copy_source(&headers(Some("/acct")), &mapping()),
+        None
+    );
+}
 
 #[test]
 fn extract_splits_account_product_key() {

tests/test_federation.py

@@ -24,6 +24,25 @@
                              same account, used by the authz/confused-deputy test
                              (an anonymous caller must be denied before federation
                              reads its private backend)
+  FEDERATION_WRITE_PRODUCT   product id, in the same account, that the caller
+                             identified by CI_WRITE_ID_TOKEN may *write*. Used
+                             only by the copy-source authorization test
+  CI_WRITE_ID_TOKEN          that caller's identity token — a GitHub Actions
+                             OIDC token, minted per run by staging.yml (short-
+                             lived by design, never stored) once
+                             FEDERATION_TEST_AUDIENCE is set.
+
+                             Dormant until Source registers GitHub as a valid
+                             IdP so products can accept writes from GitHub
+                             Actions. That needs the deployment's AUTH_ISSUER to
+                             accept GitHub's issuer — today it is a single Ory
+                             URL, and ``src/config.rs`` reads AUTH_ISSUER as one
+                             String (unlike the comma-separated AUTH_AUDIENCE),
+                             so it is a code change as well as config — and the
+                             audience Source expects to be set as
+                             FEDERATION_TEST_AUDIENCE. Until then this test
+                             skips. The caller must also hold write on
+                             FEDERATION_WRITE_PRODUCT
 """
 
 import os
@@ -36,6 +55,8 @@
 PRODUCT = os.environ.get("FEDERATION_TEST_PRODUCT")
 KEY = os.environ.get("FEDERATION_TEST_KEY")
 RESTRICTED_PRODUCT = os.environ.get("FEDERATION_RESTRICTED_PRODUCT")
+WRITE_PRODUCT = os.environ.get("FEDERATION_WRITE_PRODUCT")
+ID_TOKEN = os.environ.get("CI_WRITE_ID_TOKEN")
 
 
 @pytest.mark.skipif(
@@ -86,3 +107,49 @@ def test_restricted_product_denied_to_anonymous():
         "anonymous caller was not denied a restricted federated product "
         f"(status {resp.status_code}); federation may have served private data"
     )
+
+
+@pytest.mark.skipif(
+    not (ACCOUNT and RESTRICTED_PRODUCT and WRITE_PRODUCT and ID_TOKEN),
+    reason=(
+        "copy-source authz target not configured (set FEDERATION_TEST_ACCOUNT/"
+        "FEDERATION_RESTRICTED_PRODUCT/FEDERATION_WRITE_PRODUCT and an "
+        "issuer-appropriate CI_WRITE_ID_TOKEN against a deployed proxy)"
+    ),
+)
+def test_copy_source_authorization_is_enforced():
+    """The confused-deputy guard for `CopyObject`: holding write on the
+    destination must not let a caller read a product they aren't entitled to.
+
+    A copy is authorized in two halves — the destination as a write, the source
+    as a synthetic `GetObject`. This names a source the caller cannot read and a
+    destination they can write, so only the source half can deny it. A success
+    would mean CopyObject is a hole around product authorization: read any
+    restricted product by copying it somewhere readable.
+
+    Deliberately here and not in the CI integration tier, where it could not
+    fail: federation is attempted against the destination before the source is
+    resolved, so on CI's throwaway signing key every copy dies at federation
+    first — and `AccessDenied` is itself one of the federation error codes, so
+    the assertion below would pass without proving anything. It needs a
+    deployment where the destination write genuinely succeeds, which is what
+    this suite provides.
+    """
+    import botocore.exceptions
+
+    from test_writes import s3_client  # /.sts exchange + SigV4, see its module docstring
+
+    client = s3_client(retries={"max_attempts": 1})
+    # A success raises nothing and fails here as DID NOT RAISE — the outcome
+    # this test exists to catch.
+    with pytest.raises(botocore.exceptions.ClientError) as exc:
+        client.copy_object(
+            Bucket=ACCOUNT,
+            Key=f"{WRITE_PRODUCT}/copy-authz-probe.txt",
+            CopySource=f"{ACCOUNT}/{RESTRICTED_PRODUCT}/{KEY or 'any-key'}",
+        )
+    status = exc.value.response["ResponseMetadata"]["HTTPStatusCode"]
+    assert status in (401, 403, 404), (
+        f"copy from a restricted source was not denied (status {status}); "
+        "CopyObject may be bypassing source authorization"
+    )