From 31e52b34f342b433101e64f5ed350f3a48437496 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 15:03:44 -0700 Subject: [PATCH 01/10] chore(deps): test against multistore main (unreleased 0.7.1) Patch the multistore crates to the git `main` branch so CI exercises the CopyObject fixes queued for the 0.7.1 release (developmentseed/multistore#128, #129) ahead of publication. This is a temporary patch: the version requirements in `[dependencies]` already accept 0.7.1, so the `[patch.crates-io]` block is dropped once the release lands. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 15 +++++---------- Cargo.toml | 10 ++++++++++ 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c6cd587..870a083 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1035,8 +1035,7 @@ dependencies = [ [[package]] name = "multistore" version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "10553bb6d41a7caf40663151d2ebb73f0032bed3474031bb69c81ce8a0a46b39" +source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" dependencies = [ "async-trait", "base64", @@ -1064,8 +1063,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers" version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "89cb288f2d08ba9fa1b1970e3323266da7cee51dea786d3be6c73cbb85ebeac6" +source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" dependencies = [ "async-trait", "bytes", @@ -1091,8 +1089,7 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b743bc10e0fdfb5c70f9f302e0bf756714b7e09cfd41d837918425db3c6b36eb" +source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" dependencies = [ "base64", "chrono", @@ -1111,8 +1108,7 @@ dependencies = [ [[package]] name = "multistore-path-mapping" version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17ce2241c2cdd6bbbf239498ec7caa9a4488b28c159f58288261435b4f090975" +source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" dependencies = [ "multistore", "percent-encoding", @@ -1122,8 +1118,7 @@ dependencies = [ [[package]] name = "multistore-sts" version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ee8407b2bc78eeadd420fc6c952cc9c6ac5f51c7e52643417d3ea048859cdad2" +source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" dependencies = [ "aes-gcm", "base64", diff --git a/Cargo.toml b/Cargo.toml index cd16c3c..10306f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -86,3 +86,13 @@ web-sys = { version = "0.3", features = [ ] } worker = { version = "=0.7.5", features = ["http"] } worker-macros = { version = "=0.7.5", features = ["http"] } + +# ponytail: temporary — exercise the unreleased CopyObject fixes queued for +# multistore 0.7.1 (developmentseed/multistore#128, #129). Drop this block once +# 0.7.1 is published; the version requirements above already accept it. +[patch.crates-io] +multistore = { git = "https://github.com/developmentseed/multistore", branch = "main" } +multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", branch = "main" } +multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", branch = "main" } +multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", branch = "main" } +multistore-sts = { git = "https://github.com/developmentseed/multistore", branch = "main" } From fa5206d007e6f765ca0187a25acfc2ba3a676cf6 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 15:12:20 -0700 Subject: [PATCH 02/10] fix(copy): map x-amz-copy-source into the registry namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CopyObject` names its source in a header rather than the URL, in client coordinates (`/account/product/key`). This deployment is path-mapped, so the registry only knows mapped bucket names — an unmapped source resolves to a bucket it has never heard of and every copy fails with 404 NoSuchBucket. multistore 0.7.1 adds `RequestInfo::with_copy_source` for exactly this (developmentseed/multistore#128), but it is opt-in: the host app has to supply the mapped value. Wire it up via `PathMapping::rewrite_copy_source`. The client signed the original header, so it is left untouched and signature verification still uses it as sent; only the out-of-band mapped value differs. Adds two integration tests: an anonymous copy is denied at the authz gate, and a credentialed copy must reach the federation seam rather than dying as NoSuchBucket — which is what distinguishes a mapped source from an unmapped one. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib.rs | 15 +++++++++++++- tests/test_writes.py | 47 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 05eb846..c2576df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -316,6 +316,18 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Result Date: Mon, 27 Jul 2026 15:16:45 -0700 Subject: [PATCH 03/10] TEETH-CHECK: temporarily drop with_copy_source (will revert) --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index c2576df..2c8b971 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -338,7 +338,7 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Date: Mon, 27 Jul 2026 15:18:52 -0700 Subject: [PATCH 04/10] Revert "TEETH-CHECK: temporarily drop with_copy_source (will revert)" This reverts commit a389871c431ee21102cd5f4639174820b66bbb93. --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 2c8b971..c2576df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -338,7 +338,7 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Date: Mon, 27 Jul 2026 15:21:22 -0700 Subject: [PATCH 05/10] test(copy): pin the copy-source mapping where it can actually fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The integration test added alongside the mapping fix claimed to pin it but could not: federation is attempted against the destination before the copy source is resolved, so a copy fails identically whether or not `with_copy_source` is wired up. Verified by deleting the wiring and pushing — CI stayed green. Move the check to a layer that can fail. The header-to-mapped-source logic is extracted from `fetch` into `object_path::mapped_copy_source` (that module is deliberately wasm-free so it can be unit-tested natively, the lib being cdylib with `test = false`), and covered there: the account/product fold, leading-slash normalization, nested keys, `versionId` and percent-encoding preservation, and the two None cases. These fail without the mapping. The integration test keeps its narrower real value — a copy is parsed and dispatched as a copy and fails closed at federation — and its docstring now states what it does not prove. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib.rs | 19 ++++------- src/object_path.rs | 26 +++++++++++++++ tests/object_path.rs | 76 +++++++++++++++++++++++++++++++++++++++++++- tests/test_writes.py | 37 ++++++++++----------- 4 files changed, 124 insertions(+), 34 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c2576df..5ba2f4b 100644 --- a/src/lib.rs +++ b/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,17 +316,10 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Result (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 { + 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 diff --git a/tests/object_path.rs b/tests/object_path.rs index 58f32b6..59ab34f 100644 --- a/tests/object_path.rs +++ b/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() { diff --git a/tests/test_writes.py b/tests/test_writes.py index 7b54987..0122ec3 100644 --- a/tests/test_writes.py +++ b/tests/test_writes.py @@ -355,19 +355,18 @@ def test_anonymous_copy_is_denied(): @needs_token -def test_copy_source_is_path_mapped(): - """multistore#128 wiring pin: `x-amz-copy-source` names its source in - client coordinates (`/account/product/key`), but the registry only knows - mapped bucket names. Unless the proxy passes the *mapped* source alongside - the client-signed header (`RequestInfo::with_copy_source`), the source - resolves to a bucket the registry has never heard of and every copy dies - as 404 NoSuchBucket — long before the backend is consulted. - - So the assertion is on the failure *mode*, not on success: CI's throwaway - signing key means federation can never succeed here (see - test_federated_write_fails_closed), but a copy that reaches the federation - seam at all proves the source resolved. NoSuchBucket/NoSuchKey means the - mapping regressed.""" +def test_copy_reaches_the_federation_seam(): + """A credentialed CopyObject is parsed and dispatched as a copy, and fails + closed at federation like every other write — a bounded, parseable S3 + error rather than a hang or a silent success. + + Scope, measured rather than assumed: this does NOT pin the copy-source + path mapping. Federation is attempted against the destination before the + source is resolved, so a copy fails identically here whether or not + `RequestInfo::with_copy_source` is wired up — verified by deleting the + wiring and watching this test still pass. The mapping's real regression + test is native: `copy_source_is_folded_into_the_internal_bucket_name` in + tests/object_path.rs, which does fail without it.""" import botocore.exceptions client = s3_client(retries={"max_attempts": 1}) @@ -379,13 +378,11 @@ def test_copy_source_is_path_mapped(): ) code = exc.value.response["Error"].get("Code") assert code, "unparseable error response" - assert code not in {"NoSuchBucket", "NotImplemented"}, ( - f"{code}: the copy source was not mapped into the registry's namespace " - "— is RequestInfo::with_copy_source still wired up in src/lib.rs?" - ) - assert code not in {"SignatureDoesNotMatch", "InvalidRequest"}, ( - f"{code}: the client-signed x-amz-copy-source header was mutated; only " - "the out-of-band mapped value may differ from what the client sent" + # NotImplemented would mean the copy was rejected as a cross-store copy; + # SignatureDoesNotMatch would mean the client-signed x-amz-copy-source + # header got mutated instead of the mapped value being passed alongside it. + assert code not in {"NotImplemented", "SignatureDoesNotMatch", "InvalidRequest"}, ( + f"{code}: rejected before the federation seam this test pins" ) From 4e18ca98da8cbf6856eed10961d8651189a3f7bd Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 15:25:35 -0700 Subject: [PATCH 06/10] chore(deps): bump multistore to 0.7.1 0.7.1 is released, so drop the temporary `[patch.crates-io]` block that pointed the multistore crates at git `main` and pin the released versions instead. `Cargo.lock` now resolves them from crates.io with checksums rather than a moving branch ref. This is what unblocks the copy-source mapping in this branch: `RequestInfo::with_copy_source` is new in 0.7.1 and absent from 0.7.0. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.lock | 25 +++++++++++++++---------- Cargo.toml | 19 +++++-------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 870a083..a056ce4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1034,8 +1034,9 @@ dependencies = [ [[package]] name = "multistore" -version = "0.7.0" -source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2ea59297a4aac2cbea49e6e55320f6cc5d974cd167337afc9dfcc1b386bd3f4" dependencies = [ "async-trait", "base64", @@ -1062,8 +1063,9 @@ dependencies = [ [[package]] name = "multistore-cf-workers" -version = "0.7.0" -source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b23afaa6c9ecc498314858a3a2c2600b4880ffa8c4f0f3339cb2a86c8b70e857" dependencies = [ "async-trait", "bytes", @@ -1088,8 +1090,9 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" -version = "0.7.0" -source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "00a3ee043aa16fbbca9b324e001865d9d7e3c1fb55041106cf8d8c52b452f95d" dependencies = [ "base64", "chrono", @@ -1107,8 +1110,9 @@ dependencies = [ [[package]] name = "multistore-path-mapping" -version = "0.7.0" -source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7fd87532e2f48fa4496597012735c05dd4cdfcb3e9517d8c5cfa89e5cd20e4e" dependencies = [ "multistore", "percent-encoding", @@ -1117,8 +1121,9 @@ dependencies = [ [[package]] name = "multistore-sts" -version = "0.7.0" -source = "git+https://github.com/developmentseed/multistore?branch=main#8261ebef3f2a984c68c576f552ede3201acdaa82" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3e89dd8f5e7755359eaf9d9ba94a30ed69cc2a5b067ccc53c61a8a28b10fd044" dependencies = [ "aes-gcm", "base64", diff --git a/Cargo.toml b/Cargo.toml index 10306f3..9f29b5a 100644 --- a/Cargo.toml +++ b/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 @@ -87,12 +87,3 @@ web-sys = { version = "0.3", features = [ worker = { version = "=0.7.5", features = ["http"] } worker-macros = { version = "=0.7.5", features = ["http"] } -# ponytail: temporary — exercise the unreleased CopyObject fixes queued for -# multistore 0.7.1 (developmentseed/multistore#128, #129). Drop this block once -# 0.7.1 is published; the version requirements above already accept it. -[patch.crates-io] -multistore = { git = "https://github.com/developmentseed/multistore", branch = "main" } -multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", branch = "main" } -multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", branch = "main" } -multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", branch = "main" } -multistore-sts = { git = "https://github.com/developmentseed/multistore", branch = "main" } From 1b50a45073e498e75aed6c2fa677da5356dd46e4 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 17:05:31 -0700 Subject: [PATCH 07/10] test(copy): assert copy-source authorization in the federation smoke suite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A copy is authorized in two halves — the destination as a write, the source as a synthetic GetObject. Nothing asserted the source half, so a caller holding write on some product could in principle have used CopyObject to read a product they aren't entitled to. multistore covers this upstream (#128's `copy_source_read_enforces_prefix_via_get_object`); this pins it through this repo's subject-scoped registry wiring. It goes in the deployed-environment suite rather than the CI integration tier because CI could not fail it: 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 a federation error code, so the assertion would pass without proving anything. The staging suite has a deployment where the destination write genuinely succeeds, leaving the source half as the only thing that can deny the copy. Dormant like the rest of that suite: it needs FEDERATION_WRITE_PRODUCT (a product the caller may write) plus a caller identity the *deployed* worker accepts — whose audience is not CI's `source-data-proxy-ci`, so the token is minted only when FEDERATION_TEST_AUDIENCE names the deployment's AUTH_AUDIENCE. Unset, the test skips and the rest of the suite is unaffected. staging.yml also gains boto3, which the SigV4 client needs. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/staging.yml | 28 ++++++++++++++++++- tests/test_federation.py | 52 +++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index cd1e7c0..0e9bbf7 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -47,9 +47,32 @@ jobs: if: vars.FEDERATION_TEST_ACCOUNT != '' && vars.FEDERATION_TEST_PRODUCT != '' && vars.FEDERATION_TEST_KEY != '' runs-on: ubuntu-latest timeout-minutes: 10 + permissions: + contents: read + # Only used to mint the caller identity for the copy-source authz test, + # and only when FEDERATION_TEST_AUDIENCE is set (see below). + id-token: write steps: - uses: actions/checkout@v4 - uses: astral-sh/setup-uv@v5 + - name: Mint caller identity token (GitHub OIDC) + # Dormant like the rest of this job: the copy-source authz test needs a + # caller the *deployed* worker will accept, so the audience must match + # that deployment's AUTH_AUDIENCE — which is not CI's + # `source-data-proxy-ci`. Left unset, no token is minted and that one + # 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 }} @@ -57,4 +80,7 @@ jobs: 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 }} + # 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 diff --git a/tests/test_federation.py b/tests/test_federation.py index ae97c21..903c19e 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -24,6 +24,10 @@ 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, which + also needs that token (see test_writes.py) """ import os @@ -36,6 +40,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 +92,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 " + "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" + ) From 24649cf386c06fc2714d3d28b2a46128e0a3e1cf Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 17:14:27 -0700 Subject: [PATCH 08/10] fix(ci): drop the unusable GitHub-OIDC mint from federation-smoke MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mint step added with the copy-source authz test could never have produced a usable caller. That job talks to the *deployed* worker, whose AUTH_ISSUER is Ory (`https://auth.staging.source.coop`, see wrangler.toml [env.staging.vars]) — a GitHub Actions OIDC token fails issuer verification before its audience is ever examined, so no FEDERATION_TEST_AUDIENCE value could have made it authenticate. The variable was a knob on a door that doesn't open; it is removed rather than documented, along with the `id-token: write` permission it needed. The token now comes from a secret, FEDERATION_CALLER_TOKEN, which must be issued by the deployment's own issuer with an `aud` among its AUTH_AUDIENCE client_ids and a subject holding write on FEDERATION_WRITE_PRODUCT. The test's gate is unchanged, so it still skips cleanly until that is provisioned. Not added to the wrangler files: this is a property of the CI caller, while AUTH_AUDIENCE there is what the worker accepts. Duplicating it would be a second source of truth able to drift from the value it must match. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/staging.yml | 31 ++++++++----------------------- tests/test_federation.py | 17 +++++++++++++---- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 0e9bbf7..29d37d2 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -47,32 +47,9 @@ jobs: if: vars.FEDERATION_TEST_ACCOUNT != '' && vars.FEDERATION_TEST_PRODUCT != '' && vars.FEDERATION_TEST_KEY != '' runs-on: ubuntu-latest timeout-minutes: 10 - permissions: - contents: read - # Only used to mint the caller identity for the copy-source authz test, - # and only when FEDERATION_TEST_AUDIENCE is set (see below). - id-token: write steps: - uses: actions/checkout@v4 - uses: astral-sh/setup-uv@v5 - - name: Mint caller identity token (GitHub OIDC) - # Dormant like the rest of this job: the copy-source authz test needs a - # caller the *deployed* worker will accept, so the audience must match - # that deployment's AUTH_AUDIENCE — which is not CI's - # `source-data-proxy-ci`. Left unset, no token is minted and that one - # 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 }} @@ -81,6 +58,14 @@ jobs: FEDERATION_TEST_KEY: ${{ vars.FEDERATION_TEST_KEY }} FEDERATION_RESTRICTED_PRODUCT: ${{ vars.FEDERATION_RESTRICTED_PRODUCT }} FEDERATION_WRITE_PRODUCT: ${{ vars.FEDERATION_WRITE_PRODUCT }} + # Caller identity for the copy-source authz test. NOT a GitHub OIDC + # token: this job talks to the *deployed* worker, whose AUTH_ISSUER is + # Ory (https://auth.staging.source.coop), so a GitHub-minted token + # fails issuer verification no matter its audience. Must be issued by + # that issuer, with an `aud` among staging's AUTH_AUDIENCE client_ids + # (see wrangler.toml [env.staging.vars]) and a subject holding write + # on FEDERATION_WRITE_PRODUCT. Unset, that one test skips. + CI_WRITE_ID_TOKEN: ${{ secrets.FEDERATION_CALLER_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 diff --git a/tests/test_federation.py b/tests/test_federation.py index 903c19e..fdabbd4 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -26,8 +26,17 @@ 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, which - also needs that token (see test_writes.py) + only by the copy-source authorization test + CI_WRITE_ID_TOKEN that caller's identity token. Unlike in ci.yml, + this is NOT a GitHub Actions OIDC token: these + tests hit a deployed worker whose AUTH_ISSUER is + Ory (e.g. https://auth.staging.source.coop), so a + GitHub-minted token fails issuer verification + whatever its audience. It must come from that + issuer, carry an ``aud`` among the deployment's + AUTH_AUDIENCE client_ids (wrangler.toml + ``[env.staging.vars]``), and belong to a subject + holding write on FEDERATION_WRITE_PRODUCT """ import os @@ -98,8 +107,8 @@ def test_restricted_product_denied_to_anonymous(): 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 " - "CI_WRITE_ID_TOKEN against a deployed proxy)" + "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(): From f1506aa2f0dc7b418a857fa795f3849a836b511e Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 17:19:52 -0700 Subject: [PATCH 09/10] fix(ci): don't wire a caller token that cannot work as a stored secret MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit replaced the unusable GitHub-OIDC mint with `secrets.FEDERATION_CALLER_TOKEN`, which is no better: an Ory ID token is short-lived, so a token parked in a repo secret goes stale within the hour. Setting it would buy a brief window of green followed by failures that read as an authorization regression rather than an expired credential — worse than the test plainly skipping. Supplying a caller needs a step that mints a token per run from a durable credential (a service-account client_id/secret, or a refresh token) against the deployment's own issuer. That is a provisioning decision this PR has no basis to make, so no token is wired and the copy-source authz test skips, with the requirement recorded where someone acting on it will look. The unambiguous parts of the earlier wiring stay: boto3, which the SigV4 client needs, and the FEDERATION_WRITE_PRODUCT passthrough. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/staging.yml | 16 ++++++++-------- tests/test_federation.py | 8 +++++++- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 29d37d2..a22dc83 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -58,14 +58,14 @@ jobs: FEDERATION_TEST_KEY: ${{ vars.FEDERATION_TEST_KEY }} FEDERATION_RESTRICTED_PRODUCT: ${{ vars.FEDERATION_RESTRICTED_PRODUCT }} FEDERATION_WRITE_PRODUCT: ${{ vars.FEDERATION_WRITE_PRODUCT }} - # Caller identity for the copy-source authz test. NOT a GitHub OIDC - # token: this job talks to the *deployed* worker, whose AUTH_ISSUER is - # Ory (https://auth.staging.source.coop), so a GitHub-minted token - # fails issuer verification no matter its audience. Must be issued by - # that issuer, with an `aud` among staging's AUTH_AUDIENCE client_ids - # (see wrangler.toml [env.staging.vars]) and a subject holding write - # on FEDERATION_WRITE_PRODUCT. Unset, that one test skips. - CI_WRITE_ID_TOKEN: ${{ secrets.FEDERATION_CALLER_TOKEN }} + # No CI_WRITE_ID_TOKEN is supplied, so the copy-source authz test + # skips here. Wiring one is a design decision that hasn't been made: + # this job talks to the *deployed* worker, whose AUTH_ISSUER is Ory, + # so the token must come from that issuer — and an Ory ID token is + # short-lived, so it cannot simply be parked in a repo secret. It + # needs a step that mints one per run from a durable credential + # (a service-account client_id/secret or refresh token). See the + # CI_WRITE_ID_TOKEN notes in tests/test_federation.py. # 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 diff --git a/tests/test_federation.py b/tests/test_federation.py index fdabbd4..0748b2a 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -36,7 +36,13 @@ issuer, carry an ``aud`` among the deployment's AUTH_AUDIENCE client_ids (wrangler.toml ``[env.staging.vars]``), and belong to a subject - holding write on FEDERATION_WRITE_PRODUCT + holding write on FEDERATION_WRITE_PRODUCT. + Not yet wired into staging.yml: an Ory ID token is + short-lived, so it cannot be parked in a repo + secret — supplying it needs a step that mints one + per run from a durable credential (a + service-account client_id/secret or a refresh + token). Until that exists this test skips """ import os From efa59b39f69bebdf47da475da7786a306d8672f3 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Mon, 27 Jul 2026 17:24:45 -0700 Subject: [PATCH 10/10] ci: mint the federation caller as a GitHub Actions OIDC token MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Restores the per-run GitHub OIDC mint for the copy-source authz test, ahead of registering GitHub as a valid IdP with Source so products can accept writes from GitHub Actions. This is the right shape for the credential: minted per run and short-lived, with nothing durable stored. The earlier attempt at a repo secret was worse — an ID token parked there goes stale within the hour, and the resulting failures read as an authorization regression rather than an expired credential. Still dormant, now waiting on the IdP registration rather than on a design decision. Two things must land before it can run: the deployment's AUTH_ISSUER must accept GitHub's issuer, which is a code change and not only config — src/config.rs reads AUTH_ISSUER as a single String, unlike the comma-separated AUTH_AUDIENCE — and FEDERATION_TEST_AUDIENCE must name the audience Source expects. Until both, no token is minted and the test skips. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/staging.yml | 38 +++++++++++++++++++++++++++-------- tests/test_federation.py | 32 ++++++++++++++--------------- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index a22dc83..835fbc6 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -47,9 +47,37 @@ 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 }} @@ -58,14 +86,8 @@ jobs: FEDERATION_TEST_KEY: ${{ vars.FEDERATION_TEST_KEY }} FEDERATION_RESTRICTED_PRODUCT: ${{ vars.FEDERATION_RESTRICTED_PRODUCT }} FEDERATION_WRITE_PRODUCT: ${{ vars.FEDERATION_WRITE_PRODUCT }} - # No CI_WRITE_ID_TOKEN is supplied, so the copy-source authz test - # skips here. Wiring one is a design decision that hasn't been made: - # this job talks to the *deployed* worker, whose AUTH_ISSUER is Ory, - # so the token must come from that issuer — and an Ory ID token is - # short-lived, so it cannot simply be parked in a repo secret. It - # needs a step that mints one per run from a durable credential - # (a service-account client_id/secret or refresh token). See the - # CI_WRITE_ID_TOKEN notes in tests/test_federation.py. + # 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 diff --git a/tests/test_federation.py b/tests/test_federation.py index 0748b2a..fff5ad2 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -27,22 +27,22 @@ 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. Unlike in ci.yml, - this is NOT a GitHub Actions OIDC token: these - tests hit a deployed worker whose AUTH_ISSUER is - Ory (e.g. https://auth.staging.source.coop), so a - GitHub-minted token fails issuer verification - whatever its audience. It must come from that - issuer, carry an ``aud`` among the deployment's - AUTH_AUDIENCE client_ids (wrangler.toml - ``[env.staging.vars]``), and belong to a subject - holding write on FEDERATION_WRITE_PRODUCT. - Not yet wired into staging.yml: an Ory ID token is - short-lived, so it cannot be parked in a repo - secret — supplying it needs a step that mints one - per run from a durable credential (a - service-account client_id/secret or a refresh - token). Until that exists this test skips + 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