From 03dca48032fba71541f363025a7e3de378a63480 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 23 Jun 2026 22:16:44 -0700 Subject: [PATCH 01/14] feat: authorize and enable writes to data connections Writes were short-circuited with a 405 and the registry only authorized reads. Remove the short-circuit so PUT/POST/DELETE flow through the gateway (where the backend-auth middleware signs them), and add a caller-side authorization layer: - `authz`: a wasm-free, natively-tested module. `is_write_action` classifies the operation; `authorize_write` permits a write only when the connection is not read-only, the connection can sign (an S3 web-identity role), and the caller holds the product's `write` permission. Anonymous callers are rejected in the registry. - registry: `get_bucket` threads whether the op is a write into `resolve_product`, which runs the gate after the subject-scoped product and connection fetches. `DataConnection` gains `read_only` (fail-closed). `authorize_key` is overridden so batch-delete keys are permitted (product-level authz already ran in `get_bucket`). - cache: `get_or_fetch_permissions` fetches the caller's product permissions (60s TTL). - CORS advertises PUT/POST/DELETE. Tracks the multistore data-edit-operations branch, which adds batch delete and the `BucketRegistry::authorize_key` hook (forwarded by `MappedRegistry`). Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 16 +++++--------- Cargo.toml | 18 ++++++++++----- src/authz.rs | 57 +++++++++++++++++++++++++++++++++++++++++++++++ src/cache.rs | 33 +++++++++++++++++++++++++++ src/lib.rs | 48 ++++++++++------------------------------ src/registry.rs | 58 ++++++++++++++++++++++++++++++++++++++++++++++-- tests/authz.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 235 insertions(+), 54 deletions(-) create mode 100644 src/authz.rs create mode 100644 tests/authz.rs diff --git a/Cargo.lock b/Cargo.lock index 074b0ae..ac7ecff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -909,8 +909,7 @@ dependencies = [ [[package]] name = "multistore" version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4ae47947aa57e1cfefcbe1eba035abd0033154c56e95812e9a34cf4c43aad06" +source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" dependencies = [ "async-trait", "base64", @@ -921,6 +920,7 @@ dependencies = [ "hmac", "http", "matchit 0.8.6", + "md-5", "object_store", "percent-encoding", "quick-xml 0.37.5", @@ -936,8 +936,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers" version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "518e3a40526f673ed6b0e032c9cec7f45c917a7a50fdf7fca375f27b16e0f253" +source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" dependencies = [ "async-trait", "bytes", @@ -961,8 +960,7 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "04b3ee44b4a1f68ce7b97f288cf1f105f6e5d2f073c2bb06617b9380c29235d4" +source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" dependencies = [ "base64", "chrono", @@ -981,8 +979,7 @@ dependencies = [ [[package]] name = "multistore-path-mapping" version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ffc11cdbe2bb11e105db4ceb63cdcf0ed89155df0f2da6248bcd532666d3db27" +source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" dependencies = [ "multistore", "percent-encoding", @@ -992,8 +989,7 @@ dependencies = [ [[package]] name = "multistore-sts" version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97639b653d38ac5a5bdc01fe400275df028a481188634bee21991a211e19b9e6" +source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" dependencies = [ "aes-gcm", "base64", diff --git a/Cargo.toml b/Cargo.toml index 27cdb1b..a0371a9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -22,12 +22,18 @@ path = "tests/pagination.rs" name = "backend_auth" path = "tests/backend_auth.rs" +[[test]] +name = "authz" +path = "tests/authz.rs" + [dependencies] -# Multistore -multistore = { version = "0.5.1", features = ["azure"] } -multistore-oidc-provider = { version = "0.5.1" } -multistore-path-mapping = { version = "0.5.1" } -multistore-sts = { version = "0.5.1" } +# Multistore — tracks the data-edit-operations feature branch (off 0.5.1): +# batch delete, CopyObject rejection, wider write headers, and the +# `BucketRegistry::authorize_key` per-key authorization hook. +multistore = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan", features = ["azure"] } +multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan" } +multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan" } +multistore-sts = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan" } # Serialization serde = { version = "1", features = ["derive"] } @@ -45,7 +51,7 @@ tracing = "0.1" # Pulled in transitively via object_store -> rand. getrandom doesn't support # wasm32-unknown-unknown unless the `wasm_js` backend is enabled, so opt in here. getrandom = { version = "0.4", features = ["wasm_js"] } -multistore-cf-workers = { version = "0.5.1", features = ["azure"] } +multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan", features = ["azure"] } # 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. We only use `Client::new()`, `.send()` and `.text()`, none diff --git a/src/authz.rs b/src/authz.rs new file mode 100644 index 0000000..42b727f --- /dev/null +++ b/src/authz.rs @@ -0,0 +1,57 @@ +//! Caller-side write authorization. +//! +//! Backend *signing* (how the proxy authenticates outbound) lives in +//! [`backend_auth`](crate::backend_auth); this module is the orthogonal +//! concern: whether a given *caller* may perform a *write* on a product. The +//! decision is a pure function of facts the registry resolves from the Source +//! API, kept free of wasm-only deps so it can be unit-tested natively (see +//! `tests/authz.rs`), despite the crate's `[lib] test = false`. + +use multistore::error::ProxyError; +use multistore::types::Action; + +/// Whether an S3 action mutates the backend. Reads (GET/HEAD/LIST) are served +/// without a write check; everything else is a write and must be authorized. +/// +/// A denylist over the closed [`Action`] set, so it is fail-safe by direction: +/// any action that is not explicitly a read is treated as a write. A future +/// read-only action added upstream would be (harmlessly) gated as a write until +/// added here, never the reverse. +pub(crate) fn is_write_action(action: Action) -> bool { + !matches!( + action, + Action::GetObject | Action::HeadObject | Action::ListBucket + ) +} + +/// Decide whether a write may proceed against a resolved product/connection. +/// +/// Called only for write operations, and only once the caller is known to be +/// authenticated (anonymous writes are rejected by the registry before this, +/// where the caller's identity/subject is in hand) and the product + connection +/// have already been resolved against the subject-scoped Source API (so the +/// caller is cleared to *see* the resource). All remaining conditions must hold +/// or the write is denied with `AccessDenied`: +/// +/// 1. the data connection is not read-only; +/// 2. the connection can sign writes (`signable`) — an unsigned/public or +/// unsupported connection has no credentials to write with, so a write would +/// only fail opaquely at the backend; +/// 3. the caller holds the `write` permission the Source API reports for the +/// product. +pub(crate) fn authorize_write( + read_only: bool, + signable: bool, + permissions: &[String], +) -> Result<(), ProxyError> { + if read_only { + return Err(ProxyError::AccessDenied); + } + if !signable { + return Err(ProxyError::AccessDenied); + } + if !permissions.iter().any(|p| p == "write") { + return Err(ProxyError::AccessDenied); + } + Ok(()) +} diff --git a/src/cache.rs b/src/cache.rs index 2fa3cb6..d7cb53b 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -33,6 +33,10 @@ const DATA_CONNECTION_CACHE_SECS: u32 = 300; // 5 minutes /// Product list for an account (`/api/v1/products/{account}`). const PRODUCT_LIST_CACHE_SECS: u32 = 60; // 1 minute +/// Caller's permissions on a product (`.../permissions`). Short: it gates writes, +/// so a revoked grant should stop taking effect quickly. +const PERMISSIONS_CACHE_SECS: u32 = 60; // 1 minute + // ── Public cache functions ───────────────────────────────────────── /// Fetch a single product's metadata, cached for `PRODUCT_CACHE_SECS`. @@ -62,6 +66,35 @@ pub async fn get_or_fetch_product( .await } +/// Fetch the authenticated caller's permissions on a product, cached for +/// `PERMISSIONS_CACHE_SECS`. Returns the permission strings the API grants for +/// the caller (e.g. `["read", "write"]`); writes are gated on `"write"`. +pub async fn get_or_fetch_permissions( + api_base_url: &str, + account: &str, + product: &str, + api_auth: &crate::ApiAuth, + request_id: &str, + subject: &str, +) -> Result, ProxyError> { + let api_url = format!( + "{}/api/v1/products/{}/{}/permissions", + api_base_url, + utf8_percent_encode(account, PATH_SEGMENT), + utf8_percent_encode(product, PATH_SEGMENT), + ); + let cache_key = cache_key_with_subject(&api_url, Some(subject)); + cached_fetch( + &cache_key, + &api_url, + PERMISSIONS_CACHE_SECS, + api_auth, + request_id, + Some(subject), + ) + .await +} + /// Fetch a single data connection by id, cached for `DATA_CONNECTION_CACHE_SECS`. /// /// Resolving by id (rather than scanning a cached full list) lets the diff --git a/src/lib.rs b/src/lib.rs index 85317b1..30591df 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,6 @@ mod analytics; mod auth; +mod authz; mod backend_auth; mod cache; mod config; @@ -115,36 +116,15 @@ async fn fetch(req: web_sys::Request, env: Env, ctx: Context) -> Result Result Result web_sys::Response { // ── CORS ──────────────────────────────────────────────────────────── -fn add_cors(resp: web_sys::Response, allow_post: bool) -> web_sys::Response { +fn add_cors(resp: web_sys::Response) -> web_sys::Response { let h = resp.headers(); - // The proxy is read-only; POST is advertised only on the STS endpoint. - let methods = if allow_post { - "GET, HEAD, POST, OPTIONS" - } else { - "GET, HEAD, OPTIONS" - }; for (name, value) in [ ("access-control-allow-origin", "*"), - ("access-control-allow-methods", methods), + ( + "access-control-allow-methods", + "GET, HEAD, PUT, POST, DELETE, OPTIONS", + ), ("access-control-allow-headers", "*"), ("access-control-expose-headers", "*"), ] { diff --git a/src/registry.rs b/src/registry.rs index aee6647..9ca4063 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -3,10 +3,11 @@ use multistore::api::response::BucketEntry; use multistore::error::ProxyError; use multistore::registry::{BucketRegistry, ResolvedBucket}; -use multistore::types::{BucketConfig, ResolvedIdentity, S3Operation}; +use multistore::types::{Action, BucketConfig, ResolvedIdentity, S3Operation}; use serde::Deserialize; use std::collections::HashMap; +use crate::authz::{authorize_write, is_write_action}; use crate::backend_auth::{apply_backend_auth, BackendAuth}; /// Registry that resolves Source Cooperative products to multistore `BucketConfig`s @@ -50,7 +51,7 @@ impl BucketRegistry for SourceCoopRegistry { &self, name: &str, identity: &ResolvedIdentity, - _operation: &S3Operation, + operation: &S3Operation, ) -> Result { // Bucket names arrive pre-mapped as "account:product". let (account, product) = name @@ -69,6 +70,7 @@ impl BucketRegistry for SourceCoopRegistry { &self.api_auth, &self.request_id, subject, + is_write_action(operation.action()), ) .await?; @@ -85,6 +87,22 @@ impl BucketRegistry for SourceCoopRegistry { ) -> Result, ProxyError> { unimplemented!("Bucket listing is not supported") } + + async fn authorize_key( + &self, + _name: &str, + _identity: &ResolvedIdentity, + _action: Action, + _key: &str, + ) -> bool { + // Per-key authorization for batch delete. Source Cooperative authorizes + // writes at the product level in `get_bucket` (the caller holds product + // write permission, the connection is writable), so every key in a batch + // delete that reached this point is permitted. The multistore default + // would deny every key, since callers' STS sessions carry no per-bucket + // scopes. + true + } } /// Resolve a product to a `BucketConfig` by querying the Source Cooperative API. @@ -95,6 +113,7 @@ async fn resolve_product( api_auth: &crate::ApiAuth, request_id: &str, subject: Option<&str>, + is_write: bool, ) -> Result { let span = tracing::info_span!( "resolve_product", @@ -139,6 +158,30 @@ async fn resolve_product( ) .await?; + // Authorize writes. The subject-scoped fetches above already cleared the + // caller to *see* this product; a write additionally requires an + // authenticated caller who holds the product's `write` permission, a + // connection that is not read-only, and a connection the proxy can sign as. + if is_write { + // Anonymous callers can never write (and there is no subject to query + // permissions with). + let subject = subject.ok_or(ProxyError::AccessDenied)?; + let permissions = crate::cache::get_or_fetch_permissions( + api_base_url, + account, + product, + api_auth, + request_id, + subject, + ) + .await?; + let signable = matches!( + connection.authentication, + BackendAuth::S3WebIdentityRole { .. } + ); + authorize_write(connection.read_only, signable, &permissions)?; + } + // 4. Build BucketConfig let backend_type = match connection.details.provider.as_str() { "s3" => "s3", @@ -281,9 +324,20 @@ pub struct SourceProductMirror { pub prefix: String, } +/// Fail-closed default for [`DataConnection::read_only`]: an absent flag is +/// treated as read-only so a malformed API response cannot open writes. +fn read_only_fail_closed() -> bool { + true +} + #[derive(Debug, Clone, Deserialize)] pub struct DataConnection { pub data_connection_id: String, + /// Whether the connection forbids writes. The Source API always reports this; + /// if it is ever absent we fail closed (treat as read-only) so a malformed + /// response can't open writes — reads are unaffected. + #[serde(default = "read_only_fail_closed")] + pub read_only: bool, pub details: DataConnectionDetails, /// How the proxy authenticates to this connection's backend. A sibling of /// `details`, matching the Source API's `DataConnection` shape. Absent → diff --git a/tests/authz.rs b/tests/authz.rs new file mode 100644 index 0000000..5ce5f7b --- /dev/null +++ b/tests/authz.rs @@ -0,0 +1,59 @@ +//! Native unit tests for the wasm-free `authz` module, included via `#[path]` +//! (the lib itself is `cdylib` with `test = false`). Mirrors the pattern in +//! `tests/backend_auth.rs`. + +#[path = "../src/authz.rs"] +mod authz; + +use authz::{authorize_write, is_write_action}; +use multistore::types::Action; + +fn perms(p: &[&str]) -> Vec { + p.iter().map(|s| s.to_string()).collect() +} + +// ── is_write_action ──────────────────────────────────────────────── + +#[test] +fn reads_are_not_writes() { + assert!(!is_write_action(Action::GetObject)); + assert!(!is_write_action(Action::HeadObject)); + assert!(!is_write_action(Action::ListBucket)); +} + +#[test] +fn mutations_are_writes() { + for action in [ + Action::PutObject, + Action::DeleteObject, + Action::CreateMultipartUpload, + Action::UploadPart, + Action::CompleteMultipartUpload, + Action::AbortMultipartUpload, + ] { + assert!(is_write_action(action), "{action:?} should be a write"); + } +} + +// ── authorize_write ──────────────────────────────────────────────── + +#[test] +fn allowed_when_writable_signable_and_permitted() { + assert!(authorize_write(false, true, &perms(&["read", "write"])).is_ok()); +} + +#[test] +fn denied_on_read_only_connection() { + assert!(authorize_write(true, true, &perms(&["read", "write"])).is_err()); +} + +#[test] +fn denied_when_connection_not_signable() { + assert!(authorize_write(false, false, &perms(&["read", "write"])).is_err()); +} + +#[test] +fn denied_without_write_permission() { + assert!(authorize_write(false, true, &perms(&["read"])).is_err()); + assert!(authorize_write(false, true, &perms(&[])).is_err()); +} From 0e30e91571e0158fffac009ce40a80019c08dca5 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 23 Jun 2026 22:23:54 -0700 Subject: [PATCH 02/14] refactor: skip /permissions fetch when the connection can't accept writes Check the connection-level write preconditions (not read-only, signable) before the per-caller permissions API call, so a write to a read-only or unsignable connection is denied without the round-trip. Splits the gate into two pure predicates (connection_accepts_writes, permits_write). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/authz.rs | 48 +++++++++++++++--------------------------------- src/registry.rs | 20 ++++++++++++++------ tests/authz.rs | 26 +++++++++++++++----------- 3 files changed, 44 insertions(+), 50 deletions(-) diff --git a/src/authz.rs b/src/authz.rs index 42b727f..ea17741 100644 --- a/src/authz.rs +++ b/src/authz.rs @@ -3,11 +3,10 @@ //! Backend *signing* (how the proxy authenticates outbound) lives in //! [`backend_auth`](crate::backend_auth); this module is the orthogonal //! concern: whether a given *caller* may perform a *write* on a product. The -//! decision is a pure function of facts the registry resolves from the Source -//! API, kept free of wasm-only deps so it can be unit-tested natively (see +//! decisions are pure functions of facts the registry resolves from the Source +//! API, kept free of wasm-only deps so they can be unit-tested natively (see //! `tests/authz.rs`), despite the crate's `[lib] test = false`. -use multistore::error::ProxyError; use multistore::types::Action; /// Whether an S3 action mutates the backend. Reads (GET/HEAD/LIST) are served @@ -24,34 +23,17 @@ pub(crate) fn is_write_action(action: Action) -> bool { ) } -/// Decide whether a write may proceed against a resolved product/connection. -/// -/// Called only for write operations, and only once the caller is known to be -/// authenticated (anonymous writes are rejected by the registry before this, -/// where the caller's identity/subject is in hand) and the product + connection -/// have already been resolved against the subject-scoped Source API (so the -/// caller is cleared to *see* the resource). All remaining conditions must hold -/// or the write is denied with `AccessDenied`: -/// -/// 1. the data connection is not read-only; -/// 2. the connection can sign writes (`signable`) — an unsigned/public or -/// unsupported connection has no credentials to write with, so a write would -/// only fail opaquely at the backend; -/// 3. the caller holds the `write` permission the Source API reports for the -/// product. -pub(crate) fn authorize_write( - read_only: bool, - signable: bool, - permissions: &[String], -) -> Result<(), ProxyError> { - if read_only { - return Err(ProxyError::AccessDenied); - } - if !signable { - return Err(ProxyError::AccessDenied); - } - if !permissions.iter().any(|p| p == "write") { - return Err(ProxyError::AccessDenied); - } - Ok(()) +/// Connection-level preconditions for a write, evaluable with no caller lookup: +/// the connection must not be read-only and must be able to sign requests (an +/// S3 web-identity role — an unsigned/public or unsupported connection has no +/// credentials to write with). Checked *before* the per-caller permission fetch, +/// so a write the connection can never accept is denied without an API call. +pub(crate) fn connection_accepts_writes(read_only: bool, signable: bool) -> bool { + !read_only && signable +} + +/// Whether the caller's product permissions (as reported by the Source API's +/// `/permissions` endpoint) include write access. +pub(crate) fn permits_write(permissions: &[String]) -> bool { + permissions.iter().any(|p| p == "write") } diff --git a/src/registry.rs b/src/registry.rs index 9ca4063..c1da32a 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -7,7 +7,7 @@ use multistore::types::{Action, BucketConfig, ResolvedIdentity, S3Operation}; use serde::Deserialize; use std::collections::HashMap; -use crate::authz::{authorize_write, is_write_action}; +use crate::authz::{connection_accepts_writes, is_write_action, permits_write}; use crate::backend_auth::{apply_backend_auth, BackendAuth}; /// Registry that resolves Source Cooperative products to multistore `BucketConfig`s @@ -166,6 +166,16 @@ async fn resolve_product( // Anonymous callers can never write (and there is no subject to query // permissions with). let subject = subject.ok_or(ProxyError::AccessDenied)?; + // Connection-level denials need no caller lookup — check them first so a + // write the connection can't accept skips the permissions API call. + let signable = matches!( + connection.authentication, + BackendAuth::S3WebIdentityRole { .. } + ); + if !connection_accepts_writes(connection.read_only, signable) { + return Err(ProxyError::AccessDenied); + } + // The caller must hold the product's `write` permission. let permissions = crate::cache::get_or_fetch_permissions( api_base_url, account, @@ -175,11 +185,9 @@ async fn resolve_product( subject, ) .await?; - let signable = matches!( - connection.authentication, - BackendAuth::S3WebIdentityRole { .. } - ); - authorize_write(connection.read_only, signable, &permissions)?; + if !permits_write(&permissions) { + return Err(ProxyError::AccessDenied); + } } // 4. Build BucketConfig diff --git a/tests/authz.rs b/tests/authz.rs index 5ce5f7b..725a429 100644 --- a/tests/authz.rs +++ b/tests/authz.rs @@ -5,7 +5,7 @@ #[path = "../src/authz.rs"] mod authz; -use authz::{authorize_write, is_write_action}; +use authz::{connection_accepts_writes, is_write_action, permits_write}; use multistore::types::Action; fn perms(p: &[&str]) -> Vec { @@ -35,25 +35,29 @@ fn mutations_are_writes() { } } -// ── authorize_write ──────────────────────────────────────────────── +// ── connection_accepts_writes ────────────────────────────────────── #[test] -fn allowed_when_writable_signable_and_permitted() { - assert!(authorize_write(false, true, &perms(&["read", "write"])).is_ok()); +fn connection_accepts_writes_when_writable_and_signable() { + assert!(connection_accepts_writes(false, true)); } #[test] -fn denied_on_read_only_connection() { - assert!(authorize_write(true, true, &perms(&["read", "write"])).is_err()); +fn connection_rejects_read_only() { + assert!(!connection_accepts_writes(true, true)); } #[test] -fn denied_when_connection_not_signable() { - assert!(authorize_write(false, false, &perms(&["read", "write"])).is_err()); +fn connection_rejects_unsignable() { + assert!(!connection_accepts_writes(false, false)); } +// ── permits_write ────────────────────────────────────────────────── + #[test] -fn denied_without_write_permission() { - assert!(authorize_write(false, true, &perms(&["read"])).is_err()); - assert!(authorize_write(false, true, &perms(&[])).is_err()); +fn permits_write_only_when_write_present() { + assert!(permits_write(&perms(&["read", "write"]))); + assert!(permits_write(&perms(&["write"]))); + assert!(!permits_write(&perms(&["read"]))); + assert!(!permits_write(&perms(&[]))); } From a83b5fece488fa543bb222d7a04e8cbaaa9f6ade Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 23 Jun 2026 22:28:01 -0700 Subject: [PATCH 03/14] refactor: inline trivial write-gate checks Drop the connection_accepts_writes / permits_write wrappers (one-line boolean / .any checks) and inline them in resolve_product. Make DataConnection.read_only a required field instead of a fail-closed default helper. authz keeps only is_write_action (the one classification with branching logic worth a native test). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/authz.rs | 27 ++++----------------------- src/registry.rs | 21 +++++++-------------- tests/authz.rs | 35 +---------------------------------- 3 files changed, 12 insertions(+), 71 deletions(-) diff --git a/src/authz.rs b/src/authz.rs index ea17741..bd3d770 100644 --- a/src/authz.rs +++ b/src/authz.rs @@ -1,11 +1,7 @@ -//! Caller-side write authorization. -//! -//! Backend *signing* (how the proxy authenticates outbound) lives in -//! [`backend_auth`](crate::backend_auth); this module is the orthogonal -//! concern: whether a given *caller* may perform a *write* on a product. The -//! decisions are pure functions of facts the registry resolves from the Source -//! API, kept free of wasm-only deps so they can be unit-tested natively (see -//! `tests/authz.rs`), despite the crate's `[lib] test = false`. +//! Write-action classification. Kept wasm-free so it can be unit-tested +//! natively (see `tests/authz.rs`), despite the crate's `[lib] test = false`. +//! The rest of the write gate (read-only / signable / permission checks) is +//! trivial enough to live inline in the registry. use multistore::types::Action; @@ -22,18 +18,3 @@ pub(crate) fn is_write_action(action: Action) -> bool { Action::GetObject | Action::HeadObject | Action::ListBucket ) } - -/// Connection-level preconditions for a write, evaluable with no caller lookup: -/// the connection must not be read-only and must be able to sign requests (an -/// S3 web-identity role — an unsigned/public or unsupported connection has no -/// credentials to write with). Checked *before* the per-caller permission fetch, -/// so a write the connection can never accept is denied without an API call. -pub(crate) fn connection_accepts_writes(read_only: bool, signable: bool) -> bool { - !read_only && signable -} - -/// Whether the caller's product permissions (as reported by the Source API's -/// `/permissions` endpoint) include write access. -pub(crate) fn permits_write(permissions: &[String]) -> bool { - permissions.iter().any(|p| p == "write") -} diff --git a/src/registry.rs b/src/registry.rs index c1da32a..4b12250 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -7,7 +7,7 @@ use multistore::types::{Action, BucketConfig, ResolvedIdentity, S3Operation}; use serde::Deserialize; use std::collections::HashMap; -use crate::authz::{connection_accepts_writes, is_write_action, permits_write}; +use crate::authz::is_write_action; use crate::backend_auth::{apply_backend_auth, BackendAuth}; /// Registry that resolves Source Cooperative products to multistore `BucketConfig`s @@ -167,12 +167,13 @@ async fn resolve_product( // permissions with). let subject = subject.ok_or(ProxyError::AccessDenied)?; // Connection-level denials need no caller lookup — check them first so a - // write the connection can't accept skips the permissions API call. + // write the connection can't accept skips the permissions API call. A + // connection can sign writes only via an S3 web-identity role. let signable = matches!( connection.authentication, BackendAuth::S3WebIdentityRole { .. } ); - if !connection_accepts_writes(connection.read_only, signable) { + if connection.read_only || !signable { return Err(ProxyError::AccessDenied); } // The caller must hold the product's `write` permission. @@ -185,7 +186,7 @@ async fn resolve_product( subject, ) .await?; - if !permits_write(&permissions) { + if !permissions.iter().any(|p| p == "write") { return Err(ProxyError::AccessDenied); } } @@ -332,19 +333,11 @@ pub struct SourceProductMirror { pub prefix: String, } -/// Fail-closed default for [`DataConnection::read_only`]: an absent flag is -/// treated as read-only so a malformed API response cannot open writes. -fn read_only_fail_closed() -> bool { - true -} - #[derive(Debug, Clone, Deserialize)] pub struct DataConnection { pub data_connection_id: String, - /// Whether the connection forbids writes. The Source API always reports this; - /// if it is ever absent we fail closed (treat as read-only) so a malformed - /// response can't open writes — reads are unaffected. - #[serde(default = "read_only_fail_closed")] + /// Whether the connection forbids writes. Required (no serde default): an + /// absent flag fails the fetch rather than defaulting to writable. pub read_only: bool, pub details: DataConnectionDetails, /// How the proxy authenticates to this connection's backend. A sibling of diff --git a/tests/authz.rs b/tests/authz.rs index 725a429..4289428 100644 --- a/tests/authz.rs +++ b/tests/authz.rs @@ -5,15 +5,9 @@ #[path = "../src/authz.rs"] mod authz; -use authz::{connection_accepts_writes, is_write_action, permits_write}; +use authz::is_write_action; use multistore::types::Action; -fn perms(p: &[&str]) -> Vec { - p.iter().map(|s| s.to_string()).collect() -} - -// ── is_write_action ──────────────────────────────────────────────── - #[test] fn reads_are_not_writes() { assert!(!is_write_action(Action::GetObject)); @@ -34,30 +28,3 @@ fn mutations_are_writes() { assert!(is_write_action(action), "{action:?} should be a write"); } } - -// ── connection_accepts_writes ────────────────────────────────────── - -#[test] -fn connection_accepts_writes_when_writable_and_signable() { - assert!(connection_accepts_writes(false, true)); -} - -#[test] -fn connection_rejects_read_only() { - assert!(!connection_accepts_writes(true, true)); -} - -#[test] -fn connection_rejects_unsignable() { - assert!(!connection_accepts_writes(false, false)); -} - -// ── permits_write ────────────────────────────────────────────────── - -#[test] -fn permits_write_only_when_write_present() { - assert!(permits_write(&perms(&["read", "write"]))); - assert!(permits_write(&perms(&["write"]))); - assert!(!permits_write(&perms(&["read"]))); - assert!(!permits_write(&perms(&[]))); -} From 62ef26d540abf4eca949942be23afbc476319334 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 23 Jun 2026 22:33:11 -0700 Subject: [PATCH 04/14] test: replace stale 405 write test with anonymous-write-denied The proxy no longer blanket-405s writes; they route through the gateway and are authorized there. Assert the new contract: an unauthenticated write to a real product is denied with 403 at the gate (before any backend call), instead of the obsolete 405. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_integration.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/test_integration.py b/tests/test_integration.py index 71c6858..e1395f8 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -22,9 +22,12 @@ def test_index(): assert "Source Cooperative Data Proxy" in resp.text -def test_write_rejected(): - resp = requests.put(f"{PROXY_URL}/test/test/file.txt") - assert resp.status_code == 405 +def test_anonymous_write_denied(): + # Writes route through the gateway and are authorized there. An + # unauthenticated write to a real product is denied at the gate (403), + # before any backend call. + resp = requests.put(f"{PROXY_URL}/{ACCOUNT}/{PRODUCT}/{OBJECT_KEY}") + assert resp.status_code == 403 def test_options_cors(): From ff5d081f8e80e3aa68ea50cf26f0b04e06504fdb Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Tue, 23 Jun 2026 22:42:39 -0700 Subject: [PATCH 05/14] harden: gate authorize_key on a write action Defense-in-depth from auto-review: return is_write_action(action) instead of a blanket true. No behavior change for batch delete (DeleteObject is a write), but it never blanket-allows a read should multistore ever call authorize_key on a non-write path. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/registry.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registry.rs b/src/registry.rs index 4b12250..6359e03 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -92,7 +92,7 @@ impl BucketRegistry for SourceCoopRegistry { &self, _name: &str, _identity: &ResolvedIdentity, - _action: Action, + action: Action, _key: &str, ) -> bool { // Per-key authorization for batch delete. Source Cooperative authorizes @@ -100,8 +100,9 @@ impl BucketRegistry for SourceCoopRegistry { // write permission, the connection is writable), so every key in a batch // delete that reached this point is permitted. The multistore default // would deny every key, since callers' STS sessions carry no per-bucket - // scopes. - true + // scopes. Gating on a write action is defense-in-depth: this is only + // reached for write batch ops, and never blanket-allows a read. + is_write_action(action) } } From 566de37ba5baaa48cc1019e57b613cfc169ec445 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 24 Jun 2026 22:06:56 -0700 Subject: [PATCH 06/14] chore: build against released multistore 0.6.0 multistore 0.6.0 ships the data-edit-operations features (developmentseed/ multistore#88: batch delete, CopyObject rejection, wider write headers, authorize_key), so repoint the multistore* git deps from the moving worktree-data-edit-ops-plan branch to the immutable v0.6.0 tag. 0.6.0 also renames RoleConfig.required_audience -> required_audiences (Vec); StsCredentialRegistry::new now maps its Option through into_iter() (None -> [] no restriction, Some(x) -> [x]), preserving behavior. cf-workers 0.6.0 isn't on crates.io yet, so all crates stay git-pinned to the tag (single-sourced); switch to crates.io version deps once it publishes. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 20 ++++++++++---------- Cargo.toml | 18 ++++++++++-------- src/sts.rs | 3 ++- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ac7ecff..41714fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -908,8 +908,8 @@ dependencies = [ [[package]] name = "multistore" -version = "0.5.1" -source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" +version = "0.6.0" +source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" dependencies = [ "async-trait", "base64", @@ -935,8 +935,8 @@ dependencies = [ [[package]] name = "multistore-cf-workers" -version = "0.5.1" -source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" +version = "0.6.0" +source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" dependencies = [ "async-trait", "bytes", @@ -959,8 +959,8 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" -version = "0.5.1" -source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" +version = "0.6.0" +source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" dependencies = [ "base64", "chrono", @@ -978,8 +978,8 @@ dependencies = [ [[package]] name = "multistore-path-mapping" -version = "0.5.1" -source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" +version = "0.6.0" +source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" dependencies = [ "multistore", "percent-encoding", @@ -988,8 +988,8 @@ dependencies = [ [[package]] name = "multistore-sts" -version = "0.5.1" -source = "git+https://github.com/developmentseed/multistore?branch=worktree-data-edit-ops-plan#fe567baa41c37906abd96326847374610bed3434" +version = "0.6.0" +source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" dependencies = [ "aes-gcm", "base64", diff --git a/Cargo.toml b/Cargo.toml index a0371a9..a3bb0d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,13 +27,15 @@ name = "authz" path = "tests/authz.rs" [dependencies] -# Multistore — tracks the data-edit-operations feature branch (off 0.5.1): -# batch delete, CopyObject rejection, wider write headers, and the -# `BucketRegistry::authorize_key` per-key authorization hook. -multistore = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan", features = ["azure"] } -multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan" } -multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan" } -multistore-sts = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan" } +# multistore-cf-workers 0.6.0 is not on crates.io, so all multistore crates are +# pinned to the v0.6.0 git tag to keep `multistore` single-sourced: a git +# cf-workers alongside registry crates would pull two copies of `multistore` +# and fail to compile. Switch to plain `version = "0.6.0"` crates.io deps once +# multistore-cf-workers 0.6.0 publishes. +multistore = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0", features = ["azure"] } +multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0" } +multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0" } +multistore-sts = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0" } # Serialization serde = { version = "1", features = ["derive"] } @@ -51,7 +53,7 @@ tracing = "0.1" # Pulled in transitively via object_store -> rand. getrandom doesn't support # wasm32-unknown-unknown unless the `wasm_js` backend is enabled, so opt in here. getrandom = { version = "0.4", features = ["wasm_js"] } -multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", branch = "worktree-data-edit-ops-plan", features = ["azure"] } +multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0", features = ["azure"] } # 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. We only use `Client::new()`, `.send()` and `.text()`, none diff --git a/src/sts.rs b/src/sts.rs index c36dd17..2667b90 100644 --- a/src/sts.rs +++ b/src/sts.rs @@ -30,7 +30,8 @@ impl StsCredentialRegistry { role_id: "_default".to_string(), name: "Default".to_string(), trusted_oidc_issuers: vec![oidc_issuer], - required_audience, + // None -> [] (no audience restriction), Some(x) -> [x] (require aud == x). + required_audiences: required_audience.into_iter().collect(), subject_conditions: vec![], allowed_scopes: vec![], // unlimited max_session_duration_secs: 3600, From c75bdcb9c957610df28b0a95983d78de4cde9c93 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 24 Jun 2026 22:50:39 -0700 Subject: [PATCH 07/14] docs: clarify read_only TTL trade-off and authorize_key get_bucket dependency Addresses auto-review notes: document why DATA_CONNECTION_CACHE_SECS is longer than the permission TTL despite gating writes (read-path cost; the urgent revocation case rides the 60s permission TTL), and that authorize_key correctness depends on a prior successful get_bucket for the same name. Co-Authored-By: Claude Opus 4.8 --- src/cache.rs | 11 +++++++++-- src/registry.rs | 19 ++++++++++++------- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/cache.rs b/src/cache.rs index d7cb53b..45aafca 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -25,9 +25,16 @@ const PATH_SEGMENT: &AsciiSet = &NON_ALPHANUMERIC /// Product metadata (`/api/v1/products/{account}/{product}`). const PRODUCT_CACHE_SECS: u32 = 300; // 5 minutes -/// A single data connection (`/api/v1/data-connections/{id}`). Short to match -/// product metadata: a per-id response is subject-authorized, so the TTL is an +/// A single data connection (`/api/v1/data-connections/{id}`). Matches product +/// metadata: a per-id response is subject-authorized, so the TTL is an /// authorization-revocation lag, not just a freshness knob. +/// +/// Longer than `PERMISSIONS_CACHE_SECS` on purpose, even though `read_only` +/// gates writes from here. This is fetched on *every* request (read and write), +/// so a short TTL taxes the read path; flipping a connection read-only freezes +/// *all* writers and is a deliberate admin act where ~5 min lag is acceptable. +/// Revoking a single (e.g. compromised) account's write grant is the urgent +/// case, and that rides the 60s permission TTL, not this one. const DATA_CONNECTION_CACHE_SECS: u32 = 300; // 5 minutes /// Product list for an account (`/api/v1/products/{account}`). diff --git a/src/registry.rs b/src/registry.rs index 6359e03..43793e4 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -95,13 +95,18 @@ impl BucketRegistry for SourceCoopRegistry { action: Action, _key: &str, ) -> bool { - // Per-key authorization for batch delete. Source Cooperative authorizes - // writes at the product level in `get_bucket` (the caller holds product - // write permission, the connection is writable), so every key in a batch - // delete that reached this point is permitted. The multistore default - // would deny every key, since callers' STS sessions carry no per-bucket - // scopes. Gating on a write action is defense-in-depth: this is only - // reached for write batch ops, and never blanket-allows a read. + // Per-key authorization for batch delete. Correctness depends on + // `get_bucket` having already authorized this caller for `name`: Source + // Cooperative authorizes writes at the product level there (caller holds + // product write permission, connection is writable), so every key in a + // batch delete that reached this point is permitted. The multistore + // default would deny every key, since callers' STS sessions carry no + // per-bucket scopes. If a future multistore ever invoked `authorize_key` + // without a prior successful `get_bucket` for the same `name`, this gate + // alone would be insufficient — it only confirms the op is a write, not + // that the caller is entitled. Gating on a write action is thus + // defense-in-depth: only reached for write batch ops, never blanket- + // allows a read. is_write_action(action) } } From 86d287bdf40e0a74f902f39ad9d3c5dd638ec228 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 24 Jun 2026 23:12:18 -0700 Subject: [PATCH 08/14] chore: use crates.io multistore 0.6.0 instead of git tag All five multistore crates (including multistore-cf-workers) publish 0.6.0 on crates.io, so the git-tag pin and its single-sourcing rationale were unnecessary. Plain version deps resolve a single multistore 0.6.0 from the registry; no git dependency remains. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 15 ++++++++++----- Cargo.toml | 15 +++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 41714fa..e73a426 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -909,7 +909,8 @@ dependencies = [ [[package]] name = "multistore" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30472373455b9115171db0141411fb9bb5948a49bdc9aed8af1d3242dc0603f3" dependencies = [ "async-trait", "base64", @@ -936,7 +937,8 @@ dependencies = [ [[package]] name = "multistore-cf-workers" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0645977b70ffe6c97718c891302f4bd47b543ee0dac3e893d23a480cf83afe29" dependencies = [ "async-trait", "bytes", @@ -960,7 +962,8 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4acc9c1f7a9e3f0ce2f80ac9d9e313fb0baf702a99a88a5ad73b938e6ac6560a" dependencies = [ "base64", "chrono", @@ -979,7 +982,8 @@ dependencies = [ [[package]] name = "multistore-path-mapping" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "46817871d9c3ed42a914833341af96c30ac8fe900122ba2e630a407530a4e164" dependencies = [ "multistore", "percent-encoding", @@ -989,7 +993,8 @@ dependencies = [ [[package]] name = "multistore-sts" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?tag=v0.6.0#b0f5b7fed72d216d3474ebffacc6de1c074e616f" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ea1072210be4d1ec89a802e59bb836119263064b587eaf90af46390552ec2d58" dependencies = [ "aes-gcm", "base64", diff --git a/Cargo.toml b/Cargo.toml index a3bb0d7..9d148e2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,15 +27,10 @@ name = "authz" path = "tests/authz.rs" [dependencies] -# multistore-cf-workers 0.6.0 is not on crates.io, so all multistore crates are -# pinned to the v0.6.0 git tag to keep `multistore` single-sourced: a git -# cf-workers alongside registry crates would pull two copies of `multistore` -# and fail to compile. Switch to plain `version = "0.6.0"` crates.io deps once -# multistore-cf-workers 0.6.0 publishes. -multistore = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0", features = ["azure"] } -multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0" } -multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0" } -multistore-sts = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0" } +multistore = { version = "0.6.0", features = ["azure"] } +multistore-oidc-provider = "0.6.0" +multistore-path-mapping = "0.6.0" +multistore-sts = "0.6.0" # Serialization serde = { version = "1", features = ["derive"] } @@ -53,7 +48,7 @@ tracing = "0.1" # Pulled in transitively via object_store -> rand. getrandom doesn't support # wasm32-unknown-unknown unless the `wasm_js` backend is enabled, so opt in here. getrandom = { version = "0.4", features = ["wasm_js"] } -multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", tag = "v0.6.0", features = ["azure"] } +multistore-cf-workers = { version = "0.6.0", features = ["azure"] } # 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. We only use `Client::new()`, `.send()` and `.text()`, none From a25596c9605bb95a995feacb3d9316c16cf485a7 Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 24 Jun 2026 23:22:46 -0700 Subject: [PATCH 09/14] harden: match the write permission case-insensitively The source.coop API emits a lowercase "write" today, but comparing with eq_ignore_ascii_case avoids silently denying a legitimately-authorized writer if the casing ever drifts. Strictly fail-safe: only a real write grant matches either way. Co-Authored-By: Claude Opus 4.8 --- src/registry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registry.rs b/src/registry.rs index 43793e4..635d86a 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -192,7 +192,7 @@ async fn resolve_product( subject, ) .await?; - if !permissions.iter().any(|p| p == "write") { + if !permissions.iter().any(|p| p.eq_ignore_ascii_case("write")) { return Err(ProxyError::AccessDenied); } } From 85fb097d27d8dd633f8207aa35da8558e720f0fb Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Wed, 24 Jun 2026 23:36:16 -0700 Subject: [PATCH 10/14] fix: set AUTH_ISSUER/AUTH_AUDIENCE for preview deploys wrangler.preview.toml had no [vars] block, so AUTH_AUDIENCE was unset and /.sts fail-closed with 501 ("STS token exchange is not configured"). Point preview at the staging Ory issuer and accept the staging frontend + CLI client_ids so the token exchange works on preview branches. Co-Authored-By: Claude Opus 4.8 --- wrangler.preview.toml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 7c74542..84a250c 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -2,6 +2,13 @@ compatibility_date = "2026-03-17" main = "build/worker/shim.mjs" workers_dev = true +[vars] +# Preview is staging-equivalent (shares staging analytics + log stream), so +# trust the staging Ory issuer and accept the staging frontend + CLI client_ids. +# AUTH_AUDIENCE must be set or /.sts fail-closes with 501. +AUTH_ISSUER = "https://auth.staging.source.coop" +AUTH_AUDIENCE = "1123dfa8-469f-44fe-b9f4-9b76f06fd325,a79c9537-be78-454a-9ea1-b96a1be811cc" # staging frontend + source-coop-cli client_ids + [build] command = "cargo install -q worker-build@0.7.5 && worker-build --release" From b9c0881ece7c7ddc67e0bd38cd5a857cfd4be21e Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 25 Jun 2026 12:08:49 -0700 Subject: [PATCH 11/14] ci(preview): sign as canonical staging OIDC identity Preview proxies call the staging API and AWS IAM, which trust only issuer https://data.staging.source.coop + kid data-proxy-1 and fetch the JWKS from that host. Signing the on-behalf-of API token / AWS federation assertion as the per-PR workers.dev hostname (kid source-proxy-1) fails JWKS verification (ERR_JWKS_NO_MATCHING_KEY), so authenticated calls fall back to anonymous and 403 on restricted products. Sign as the canonical staging identity instead. Requires the `preview` GitHub environment's OIDC_PROVIDER_KEY to be the staging signing key. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/preview.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index acd9a3a..d584a77 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -17,11 +17,19 @@ jobs: with: worker_name: source-data-proxy-pr-${{ github.event.pull_request.number }} wrangler_config: wrangler.preview.toml + # Sign the proxy's on-behalf-of API token (and AWS federation assertion) + # as the canonical staging proxy, not this ephemeral preview hostname: + # staging.source.coop's API and AWS IAM trust only issuer + # https://data.staging.source.coop + kid data-proxy-1 and fetch its JWKS + # from that host (never the pr-*.workers.dev origin). Signing as anything + # else fails JWKS verification (ERR_JWKS_NO_MATCHING_KEY), so the call + # falls back to anonymous → 403 on restricted products. Requires the + # `preview` environment's OIDC_PROVIDER_KEY to be the staging signing key. var_overrides: >- LOG_LEVEL:DEBUG SOURCE_API_URL:https://staging.source.coop - OIDC_PROVIDER_ISSUER:https://source-data-proxy-pr-${{ github.event.pull_request.number }}.source-coop.workers.dev - OIDC_PROVIDER_KID:source-proxy-1 + OIDC_PROVIDER_ISSUER:https://data.staging.source.coop + OIDC_PROVIDER_KID:data-proxy-1 AUTH_ISSUER:https://auth.staging.source.coop service_overrides: >- PUBLIC_LOG_STREAM:public-log-stream-pr-${{ github.event.pull_request.number }} From 402f3d9a0157e94eb7ddca68087a7197a5889aba Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 25 Jun 2026 12:13:58 -0700 Subject: [PATCH 12/14] ci(preview): move static preview vars into wrangler.preview.toml Every preview var is now static (the OIDC issuer no longer templates the per-PR hostname), so consolidate LOG_LEVEL, SOURCE_API_URL and OIDC_PROVIDER_* into wrangler.preview.toml [vars] beside AUTH_*, and drop the now-empty var_overrides. AUTH_ISSUER was duplicated across both; de-duped. preview.yml keeps only the per-PR dynamic bindings (worker_name, PUBLIC_LOG_STREAM). Behavior-preserving: the same vars reach the worker. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/preview.yml | 16 ++-------------- wrangler.preview.toml | 13 +++++++++++++ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/.github/workflows/preview.yml b/.github/workflows/preview.yml index d584a77..c52a8b7 100644 --- a/.github/workflows/preview.yml +++ b/.github/workflows/preview.yml @@ -17,20 +17,8 @@ jobs: with: worker_name: source-data-proxy-pr-${{ github.event.pull_request.number }} wrangler_config: wrangler.preview.toml - # Sign the proxy's on-behalf-of API token (and AWS federation assertion) - # as the canonical staging proxy, not this ephemeral preview hostname: - # staging.source.coop's API and AWS IAM trust only issuer - # https://data.staging.source.coop + kid data-proxy-1 and fetch its JWKS - # from that host (never the pr-*.workers.dev origin). Signing as anything - # else fails JWKS verification (ERR_JWKS_NO_MATCHING_KEY), so the call - # falls back to anonymous → 403 on restricted products. Requires the - # `preview` environment's OIDC_PROVIDER_KEY to be the staging signing key. - var_overrides: >- - LOG_LEVEL:DEBUG - SOURCE_API_URL:https://staging.source.coop - OIDC_PROVIDER_ISSUER:https://data.staging.source.coop - OIDC_PROVIDER_KID:data-proxy-1 - AUTH_ISSUER:https://auth.staging.source.coop + # All static preview vars live in wrangler.preview.toml [vars]; only the + # per-PR dynamic bindings are templated here. service_overrides: >- PUBLIC_LOG_STREAM:public-log-stream-pr-${{ github.event.pull_request.number }} environment: preview diff --git a/wrangler.preview.toml b/wrangler.preview.toml index 84a250c..0f13699 100644 --- a/wrangler.preview.toml +++ b/wrangler.preview.toml @@ -3,6 +3,19 @@ main = "build/worker/shim.mjs" workers_dev = true [vars] +LOG_LEVEL = "DEBUG" +SOURCE_API_URL = "https://staging.source.coop" + +# Sign the proxy's on-behalf-of API token (and AWS federation assertion) as the +# canonical staging proxy, not this ephemeral preview hostname: staging's API and +# AWS IAM trust only issuer https://data.staging.source.coop + kid data-proxy-1 +# and fetch its JWKS from that host (never the pr-*.workers.dev origin). Signing +# as anything else fails JWKS verification (ERR_JWKS_NO_MATCHING_KEY), so the call +# falls back to anonymous → 403 on restricted products. Requires the `preview` +# environment's OIDC_PROVIDER_KEY to be the staging signing key. +OIDC_PROVIDER_ISSUER = "https://data.staging.source.coop" +OIDC_PROVIDER_KID = "data-proxy-1" + # Preview is staging-equivalent (shares staging analytics + log stream), so # trust the staging Ory issuer and accept the staging frontend + CLI client_ids. # AUTH_AUDIENCE must be set or /.sts fail-closes with 501. From 68b9187a0a8a615db2014d3d8f63f8826a8b5cea Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 25 Jun 2026 15:33:47 -0700 Subject: [PATCH 13/14] build(deps): point multistore at the aws-chunked write-decode fix branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Patch multistore* to developmentseed/multistore#92 (fix/decode-aws-chunked-writes) so preview/staging pick up the aws-chunked body decode. Without it, the modern aws-cli's default trailer-checksum uploads are stored as raw chunk framing — corrupted writes. Revert this [patch.crates-io] block and bump the deps to a crates.io release once the fix ships. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 15 +++++---------- Cargo.toml | 12 ++++++++++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e73a426..04268b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -909,8 +909,7 @@ dependencies = [ [[package]] name = "multistore" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "30472373455b9115171db0141411fb9bb5948a49bdc9aed8af1d3242dc0603f3" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" dependencies = [ "async-trait", "base64", @@ -937,8 +936,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0645977b70ffe6c97718c891302f4bd47b543ee0dac3e893d23a480cf83afe29" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" dependencies = [ "async-trait", "bytes", @@ -962,8 +960,7 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4acc9c1f7a9e3f0ce2f80ac9d9e313fb0baf702a99a88a5ad73b938e6ac6560a" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" dependencies = [ "base64", "chrono", @@ -982,8 +979,7 @@ dependencies = [ [[package]] name = "multistore-path-mapping" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46817871d9c3ed42a914833341af96c30ac8fe900122ba2e630a407530a4e164" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" dependencies = [ "multistore", "percent-encoding", @@ -993,8 +989,7 @@ dependencies = [ [[package]] name = "multistore-sts" version = "0.6.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ea1072210be4d1ec89a802e59bb836119263064b587eaf90af46390552ec2d58" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" dependencies = [ "aes-gcm", "base64", diff --git a/Cargo.toml b/Cargo.toml index 321ba90..df604ef 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -68,3 +68,15 @@ web-sys = { version = "0.3", features = [ ] } worker = { version = "=0.7.4", features = ["http"] } worker-macros = { version = "=0.7.4", features = ["http"] } + +# TEMPORARY: build against the aws-chunked write-decode fix branch of multistore +# until it ships in a release. Without this, the modern aws-cli's default +# trailer-checksum uploads are stored as raw chunk framing (corrupted writes). +# Remove this block and bump the multistore* deps above once +# https://github.com/developmentseed/multistore/pull/92 is released. +[patch.crates-io] +multistore = { git = "https://github.com/developmentseed/multistore", branch = "fix/decode-aws-chunked-writes" } +multistore-cf-workers = { git = "https://github.com/developmentseed/multistore", branch = "fix/decode-aws-chunked-writes" } +multistore-oidc-provider = { git = "https://github.com/developmentseed/multistore", branch = "fix/decode-aws-chunked-writes" } +multistore-path-mapping = { git = "https://github.com/developmentseed/multistore", branch = "fix/decode-aws-chunked-writes" } +multistore-sts = { git = "https://github.com/developmentseed/multistore", branch = "fix/decode-aws-chunked-writes" } From 2f164827696c8331f089249ba3e82d0900c4dd4a Mon Sep 17 00:00:00 2001 From: Anthony Lukach Date: Thu, 25 Jun 2026 15:59:52 -0700 Subject: [PATCH 14/14] build(deps): bump multistore patch to the stream-resign aws-chunked fix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-resolve the multistore git patch to the rewritten developmentseed/multistore#92, which now stream-re-signs aws-chunked uploads (zero-copy) instead of buffering+decoding — relevant on the Workers memory ceiling. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 04268b6..a07d44d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -909,7 +909,7 @@ dependencies = [ [[package]] name = "multistore" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#71b1513a7eacb866328a72eb85818603e859424c" dependencies = [ "async-trait", "base64", @@ -936,7 +936,7 @@ dependencies = [ [[package]] name = "multistore-cf-workers" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#71b1513a7eacb866328a72eb85818603e859424c" dependencies = [ "async-trait", "bytes", @@ -960,7 +960,7 @@ dependencies = [ [[package]] name = "multistore-oidc-provider" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#71b1513a7eacb866328a72eb85818603e859424c" dependencies = [ "base64", "chrono", @@ -979,7 +979,7 @@ dependencies = [ [[package]] name = "multistore-path-mapping" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#71b1513a7eacb866328a72eb85818603e859424c" dependencies = [ "multistore", "percent-encoding", @@ -989,7 +989,7 @@ dependencies = [ [[package]] name = "multistore-sts" version = "0.6.0" -source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#26abb8938be34e0a79209966a751085dde2fe7d5" +source = "git+https://github.com/developmentseed/multistore?branch=fix%2Fdecode-aws-chunked-writes#71b1513a7eacb866328a72eb85818603e859424c" dependencies = [ "aes-gcm", "base64",