From a4862f78072d0aa3d1f69bb1af23299dba7701f8 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Thu, 6 Aug 2026 01:21:18 +0200 Subject: [PATCH 1/3] fix: pin the login challenge shape and give the bearer header one home Four adjacent auth/transport defects on the engine's Http seam. engine: `login_identity` signed whatever string `/auth/challenge` answered, with no shape check. The signer hands `sha256(utf8(challenge))` to the secp256k1 identity key via the same `sign_detcbor` primitive that signs det-CBOR structures, so anything answering at `apiBaseUrl` was a signing oracle for an arbitrary UTF-8 preimage. The challenge is now pinned to the whole shape the API issues -- the `cipherbox-login:v2:` domain tag plus exactly 32 bytes of lowercase hex -- not merely the prefix, which would have left the rest of the preimage steerable. engine: a single-flight refresh leader dropped while parked on the network left `refresh_waiters` occupied forever, so every later caller enqueued behind a rotation that would never finish. An RAII guard hands leadership back however the leader leaves, and a waiter woken by a cancelled leader is told it was an availability failure rather than a dead session. engine: the `Authorization` header name and the rule that makes a bearer safe to send now live once, on the seam, as `bearer_header`. All three splice sites use it -- the BYO config token, the access token decoded out of an `/auth/*` body, and a gateway source's bearer -- so the rule no longer depends on whichever caller remembered to run a config gate first. client + desktop-seams: both transports refuse redirects, as the record transport already does. Every target on this seam is directly addressed and gated on the URL the engine supplied, so a hop the engine did not choose can only escape that gate. Closes #1034 Closes #713 Closes #933 Closes #1086 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WegkkQ3uhNREerTW4MMeY2 --- crates/desktop-seams/src/http.rs | 28 +-- crates/engine/src/api/client.rs | 240 +++++++++++++++++++++-- crates/engine/src/content/provider.rs | 84 ++++---- crates/engine/src/content/read.rs | 50 ++++- crates/engine/src/facade.rs | 8 +- crates/engine/src/seams/http.rs | 72 ++++++- crates/engine/src/seams/mod.rs | 5 +- packages/client/src/seams/http.test.ts | 21 ++ packages/client/src/seams/http.ts | 4 + packages/client/test/browser/mockAuth.ts | 4 +- 10 files changed, 425 insertions(+), 91 deletions(-) diff --git a/crates/desktop-seams/src/http.rs b/crates/desktop-seams/src/http.rs index 28343bd60..a874f305f 100644 --- a/crates/desktop-seams/src/http.rs +++ b/crates/desktop-seams/src/http.rs @@ -34,25 +34,15 @@ impl ReqwestHttp { pub fn new() -> SeamResult { let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) - .redirect(reqwest::redirect::Policy::custom(|attempt| { - // The engine decides over which transport a request carrying an - // `Authorization` header may go (blueprint/engine.md "Content - // plane"). A redirect that downgrades to plaintext would carry - // the credential onto the clear network past that decision, so - // it is surfaced as the 3xx it is rather than followed. - let downgrade = attempt.url().scheme() == "http" - && attempt - .previous() - .last() - .is_some_and(|previous| previous.scheme() == "https"); - if downgrade { - attempt.stop() - } else if attempt.previous().len() > 10 { - attempt.error("too many redirects") - } else { - attempt.follow() - } - })) + // The engine picks every target on this seam and gates it on the URL + // it supplied — the API base, a gateway, a BYO endpoint past + // `validate_byo_config`. A hop the engine did not choose can only + // escape that gate, so none is followed: it re-points a directly + // addressed request at a host of the responder's choosing, and a + // downgrade to `http` would carry an `Authorization` header onto the + // clear network. Mirrors `ReqwestRecordTransport`; a 3xx comes back + // as the non-2xx response it is. + .redirect(reqwest::redirect::Policy::none()) .build() .map_err(|err| SeamError::new(format!("http client build: {err}")))?; Ok(Self { client }) diff --git a/crates/engine/src/api/client.rs b/crates/engine/src/api/client.rs index 585c2052c..e0beff463 100644 --- a/crates/engine/src/api/client.rs +++ b/crates/engine/src/api/client.rs @@ -25,7 +25,10 @@ use super::types::{ TestLoginResponse, TokenResponse, UploadResult, }; use crate::content::DAG_ROOT_CODEC; -use crate::seams::{CredentialStore, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse}; +use crate::seams::{ + CredentialStore, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, SeamError, + bearer_header, +}; /// Control-plane deadline: small JSON round trips must not park a UI flow. const CONTROL_TIMEOUT_MS: u64 = 10_000; @@ -34,7 +37,6 @@ const CONTROL_TIMEOUT_MS: u64 = 10_000; const TRANSFER_TIMEOUT_MS: u64 = 120_000; const CONTENT_TYPE: &str = "Content-Type"; -const AUTHORIZATION: &str = "Authorization"; const APPLICATION_JSON: &str = "application/json"; const APPLICATION_OCTET_STREAM: &str = "application/octet-stream"; /// Carries an upload's declared content address. A header, not a query @@ -56,6 +58,35 @@ struct State { refresh_waiters: Option>>>, } +/// Holds single-flight leadership for the duration of one rotation and gives it +/// back however the leader leaves — including a drop mid-`await`. +/// +/// Without this, a leader cancelled while parked on the network leaves +/// `refresh_waiters` occupied with senders nothing will ever fire, and every +/// later caller enqueues behind it forever. +struct RefreshLead<'a> { + state: &'a RefCell, +} + +impl RefreshLead<'_> { + /// Releases leadership and takes the waiters to notify. + fn finish(self) -> Vec>> { + self.state + .borrow_mut() + .refresh_waiters + .take() + .unwrap_or_default() + } +} + +impl Drop for RefreshLead<'_> { + fn drop(&mut self) { + // Dropping the senders wakes every waiter with `Canceled`; leaving the + // slot occupied would park them, and everyone after, indefinitely. + self.state.borrow_mut().refresh_waiters = None; + } +} + /// The engine's single API client. Generic over the two seams it drives so the /// contract suite can point it at a real HTTP stack while unit tests drive it /// with the scripted fake. @@ -116,6 +147,9 @@ impl ApiClient { .await?; let response = ok_or_err(response)?; let body: ChallengeResponse = decode(&response)?; + if !is_identity_challenge(&body.challenge) { + return Err(ApiError::Decode("unusable login challenge".into())); + } body.challenge }; let signature = signer.sign_challenge(&challenge); @@ -236,17 +270,21 @@ impl ApiClient { if let Some(rx) = receiver { return match rx.await { Ok(result) => result, - Err(oneshot::Canceled) => Err(ApiError::Unauthorized), + // The leader was dropped before it could answer: availability, + // not a dead session — the caller must not be told to re-login. + // The leader clears the slot as it goes, so the next call leads + // a fresh rotation instead of parking behind this one. + Err(oneshot::Canceled) => Err(ApiError::Transport(SeamError::new( + "refresh was cancelled before it completed", + ))), }; } + let lead = RefreshLead { state: &self.state }; let result = self.do_refresh().await; - let waiters = self - .state - .borrow_mut() - .refresh_waiters - .take() - .unwrap_or_default(); + // Hand leadership back before notifying: a waiter woken with an error + // may lead its own retry, and must not find the slot still occupied. + let waiters = lead.finish(); for tx in waiters { let _ = tx.send(result.clone()); } @@ -515,12 +553,14 @@ impl ApiClient { for (name, value) in extra_headers { headers.push(((*name).to_owned(), (*value).to_owned())); } - // Scope the borrow so it never crosses the await below. + // Scope the borrow so it never crosses the await below. The access token + // is decoded out of an `/auth/*` body, so it is the API's bytes, not the + // engine's: it meets the seam's header-value rule like any other bearer. if let Some(token) = self.state.borrow().access_token.as_ref() { - headers.push(( - AUTHORIZATION.to_owned(), - format!("Bearer {}", token.as_str()), - )); + headers.push( + bearer_header(token.as_str()) + .map_err(|_| ApiError::Decode("unusable access token".into()))?, + ); } let request = HttpRequest { method, @@ -605,6 +645,33 @@ fn is_success(status: u16) -> bool { (200..300).contains(&status) } +/// The domain tag the API stamps on an identity login challenge +/// (`apps/api/src/auth/services/challenge.service.ts`). Versioned, so a format +/// change bumps it rather than silently widening what this key will sign. +const IDENTITY_CHALLENGE_PREFIX: &str = "cipherbox-login:v2:"; +/// The challenge's random tail: 32 bytes rendered lowercase hex. +const IDENTITY_CHALLENGE_NONCE_LEN: usize = 64; + +/// Whether the server's answer is a challenge this key may sign: the login +/// domain tag followed by exactly the API's random tail. +/// +/// The signer hands `sha256(utf8(challenge))` to the same secp256k1 identity +/// key that signs det-CBOR structures, so an unchecked challenge is a signing +/// oracle for any UTF-8 preimage. The prefix alone would not close it — it +/// pins the first 19 bytes and leaves the rest of the preimage to whatever +/// answers at the API base URL — so the whole shape is pinned instead, leaving +/// a hostile responder no steerable bytes outside `[0-9a-f]`. +fn is_identity_challenge(challenge: &str) -> bool { + challenge + .strip_prefix(IDENTITY_CHALLENGE_PREFIX) + .is_some_and(|nonce| { + nonce.len() == IDENTITY_CHALLENGE_NONCE_LEN + && nonce + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + }) +} + /// EIP-4361 fixes the nonce at 8+ alphanumerics. The check is fail-closed /// rather than cosmetic: the nonce is interpolated verbatim into the text a /// wallet signs, so anything outside that class lets a hostile challenge @@ -651,6 +718,7 @@ mod tests { use super::*; use cipherbox_core::content::{CONTENT_CID_CODEC, compute_cid, encode_content_cid_str}; + use crate::seams::AUTHORIZATION; use crate::testkit::block_on; use crate::testkit::fakes::{InMemoryCredentialStore, ScriptedHttp}; use serde_json::{Value, json}; @@ -702,11 +770,17 @@ mod tests { (http, creds, client) } + /// A challenge shaped exactly as the API issues one: the login domain tag + /// plus 32 random bytes in lowercase hex. + fn challenge() -> String { + IDENTITY_CHALLENGE_PREFIX.to_owned() + &"0123456789abcdef".repeat(4) + } + /// Log in so the client holds an access token and a stored refresh token. fn login(http: &ScriptedHttp, client: &ApiClient) { http.enqueue_response(json_response( 200, - json!({ "challenge": "cipherbox-login:v2:abc", "expiresAt": "2026-01-01T00:00:00Z" }), + json!({ "challenge": challenge(), "expiresAt": "2026-01-01T00:00:00Z" }), )); http.enqueue_response(json_response( 200, @@ -737,19 +811,70 @@ mod tests { ); assert_eq!(requests[1].url, "http://api.test/auth/login"); let login_body = body_json(&requests[1]); - assert_eq!(login_body["challenge"], "cipherbox-login:v2:abc"); - assert_eq!(login_body["signature"], "sig-for-cipherbox-login:v2:abc"); + assert_eq!(login_body["challenge"], challenge()); + assert_eq!(login_body["signature"], format!("sig-for-{}", challenge())); let stored = block_on(creds.load_refresh_token()).unwrap().unwrap(); assert_eq!(stored, "a".repeat(64).as_bytes()); } + /// A challenge the API could not have issued is never signed, and the flow + /// stops before `/auth/login` — the identity key answers no one else's + /// preimage. Each case names the part of the shape it breaks. + #[test] + fn a_challenge_the_api_could_not_have_issued_is_never_signed() { + let hex64 = "0123456789abcdef".repeat(4); + let hostile = [ + "".to_owned(), + // No domain tag: an arbitrary preimage of the responder's choosing. + hex64.clone(), + // Another protocol's tag. + format!("cipherbox-grant:v2:{hex64}"), + // Right tag, but the tail is the responder's text. + "cipherbox-login:v2:sign-over-this-instead".to_owned(), + // Right tag and alphabet, wrong width — short. + format!("{IDENTITY_CHALLENGE_PREFIX}{}", "ab".repeat(8)), + // Right tag and alphabet, wrong width — long. + format!("{IDENTITY_CHALLENGE_PREFIX}{hex64}0"), + // Right width, outside the hex alphabet the API renders. + format!( + "{IDENTITY_CHALLENGE_PREFIX}{}", + "0123456789ABCDEF".repeat(4) + ), + // The tag as a suffix rather than a prefix. + format!("{hex64}{IDENTITY_CHALLENGE_PREFIX}"), + // Leading whitespace before the tag. + format!(" {IDENTITY_CHALLENGE_PREFIX}{hex64}"), + ]; + + for challenge in hostile { + let (http, _creds, client) = fakes(); + http.enqueue_response(json_response( + 200, + json!({ "challenge": challenge, "expiresAt": "2026-01-01T00:00:00Z" }), + )); + assert_eq!( + block_on(client.login_identity(&StubSigner)).unwrap_err(), + ApiError::Decode("unusable login challenge".into()), + "challenge {challenge:?} must be refused" + ); + let requests = http.requests(); + assert_eq!( + requests.len(), + 1, + "only /auth/challenge was sent for {challenge:?}" + ); + assert_eq!(requests[0].url, "http://api.test/auth/challenge"); + assert!(!client.is_authenticated()); + } + } + #[test] fn identity_login_bad_signature_is_unauthorized() { let (http, _creds, client) = fakes(); http.enqueue_response(json_response( 200, - json!({ "challenge": "c", "expiresAt": "2026-01-01T00:00:00Z" }), + json!({ "challenge": challenge(), "expiresAt": "2026-01-01T00:00:00Z" }), )); http.enqueue_response(json_response( 401, @@ -1137,6 +1262,85 @@ mod tests { assert!(client.is_authenticated()); } + /// A leader dropped while parked on the network must hand leadership back, + /// or every later caller enqueues behind a rotation that will never finish. + #[test] + fn a_cancelled_refresh_leader_lets_the_next_caller_lead() { + let http = GatedHttp::default(); + let creds = InMemoryCredentialStore::default(); + block_on(creds.store_refresh_token(b"seed-refresh-token")).unwrap(); + let client = ApiClient::new(http.clone(), creds, "http://api.test"); + let mut cx = Context::from_waker(Waker::noop()); + + { + let mut leader = pin!(client.refresh()); + assert!(leader.as_mut().poll(&mut cx).is_pending()); + assert_eq!(http.request_count(), 1); + } // The leader's future is dropped mid-flight. + + // The next caller leads a fresh rotation rather than parking. + let mut next = pin!(client.refresh()); + assert!(next.as_mut().poll(&mut cx).is_pending()); + assert_eq!(http.request_count(), 2, "the slot was handed back"); + + http.release(json_response( + 200, + json!({ "accessToken": "jwt", "refreshToken": "f".repeat(64) }), + )); + assert!(matches!(next.as_mut().poll(&mut cx), Poll::Ready(Ok(())))); + assert!(client.is_authenticated()); + } + + /// A waiter behind a cancelled leader is woken, and told it was an + /// availability failure — not that its session is dead. + #[test] + fn a_waiter_behind_a_cancelled_leader_is_woken_not_parked() { + let http = GatedHttp::default(); + let creds = InMemoryCredentialStore::default(); + block_on(creds.store_refresh_token(b"seed-refresh-token")).unwrap(); + let client = ApiClient::new(http.clone(), creds, "http://api.test"); + let mut cx = Context::from_waker(Waker::noop()); + + let mut waiter = pin!(client.refresh()); + { + let mut leader = pin!(client.refresh()); + assert!(leader.as_mut().poll(&mut cx).is_pending()); + assert!(waiter.as_mut().poll(&mut cx).is_pending()); + assert_eq!(http.request_count(), 1, "the waiter coalesced"); + } + + match waiter.as_mut().poll(&mut cx) { + Poll::Ready(Err(ApiError::Transport(_))) => {} + other => panic!("a cancelled leader is availability, got {other:?}"), + } + } + + /// The access token is the API's bytes, so it meets the seam's header-value + /// rule like any other bearer: an unusable one fails closed rather than + /// reaching the transport. + #[test] + fn an_access_token_that_cannot_be_a_header_value_is_refused() { + let (http, _creds, client) = fakes(); + http.enqueue_response(json_response( + 200, + json!({ "challenge": challenge(), "expiresAt": "2026-01-01T00:00:00Z" }), + )); + http.enqueue_response(json_response( + 200, + json!({ + "accessToken": "jwt-1\r\nX-Injected: yes", + "refreshToken": "a".repeat(64), + }), + )); + block_on(client.login_identity(&StubSigner)).expect("login"); + + assert_eq!( + block_on(client.quota()).unwrap_err(), + ApiError::Decode("unusable access token".into()) + ); + assert_eq!(http.requests().len(), 2, "no request carried the token"); + } + #[test] fn upload_declares_the_block_address_on_the_wire() { let (http, _creds, client) = fakes(); diff --git a/crates/engine/src/content/provider.rs b/crates/engine/src/content/provider.rs index df9c98027..c6258f08f 100644 --- a/crates/engine/src/content/provider.rs +++ b/crates/engine/src/content/provider.rs @@ -17,7 +17,7 @@ use zeroize::Zeroizing; use crate::content::DAG_ROOT_CODEC; use crate::seams::{ - CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, + CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, bearer_header, }; /// Deadline for a BYO-provider reachability probe: an unresponsive endpoint @@ -35,7 +35,6 @@ const PLACEMENT_TIMEOUT_MS: u64 = 60_000; /// stream is the largest, one short object per block put. const MAX_PROVIDER_RESPONSE_BYTES: usize = 64 * 1024; -const AUTHORIZATION: &str = "Authorization"; const CONTENT_TYPE: &str = "Content-Type"; const APPLICATION_JSON: &str = "application/json"; @@ -118,9 +117,9 @@ pub(crate) async fn place_block( validate_byo_config(config)?; let address = content_address(cid)?; let request = match config.kind { - ByoKind::Kubo => kubo_block_put(config, &address, block), - ByoKind::Psa => pin_by_cid(config, "/pins", "cid", &address.cid), - ByoKind::Pinata => pin_by_cid(config, "/pinning/pinByHash", "hashToPin", &address.cid), + ByoKind::Kubo => kubo_block_put(config, &address, block)?, + ByoKind::Psa => pin_by_cid(config, "/pins", "cid", &address.cid)?, + ByoKind::Pinata => pin_by_cid(config, "/pinning/pinByHash", "hashToPin", &address.cid)?, }; let response = capped(http, request).await?; if !(200..300).contains(&response.status) { @@ -176,7 +175,11 @@ fn content_address(cid: &[u8]) -> Result { /// `block/put` under the block's own codec and the frozen BLAKE3-256 framing, /// pinned in the same call, so the member's node addresses it exactly as the /// engine does. -fn kubo_block_put(config: &ByoIpfsConfig, address: &ContentAddress, block: &[u8]) -> HttpRequest { +fn kubo_block_put( + config: &ByoIpfsConfig, + address: &ContentAddress, + block: &[u8], +) -> Result { // Derived from the block's own address, so the delimiter cannot occur in the // payload it frames: that would take a block carrying the base32 of its own // BLAKE3 digest, which is a preimage. 62 bytes of base32 and `-`, inside RFC @@ -193,7 +196,7 @@ fn kubo_block_put(config: &ByoIpfsConfig, address: &ContentAddress, block: &[u8] body.extend_from_slice(block); body.extend_from_slice(tail.as_bytes()); let codec = address.codec; - HttpRequest { + Ok(HttpRequest { method: HttpMethod::Post, // A DAG root inlines a CID per leaf, so it passes Kubo's 1 MiB // block/put advisory well before the flat-DAG ceiling does. The block is @@ -206,24 +209,29 @@ fn kubo_block_put(config: &ByoIpfsConfig, address: &ContentAddress, block: &[u8] headers: headers( config, Some(format!("multipart/form-data; boundary={boundary}")), - ), + )?, body: Some(body), credentials: HttpCredentials::Omit, timeout_ms: Some(PLACEMENT_TIMEOUT_MS), - } + }) } /// Ask a pin-by-CID service to pin an address it fetches itself. -fn pin_by_cid(config: &ByoIpfsConfig, path: &str, field: &str, cid: &str) -> HttpRequest { - HttpRequest { +fn pin_by_cid( + config: &ByoIpfsConfig, + path: &str, + field: &str, + cid: &str, +) -> Result { + Ok(HttpRequest { method: HttpMethod::Post, url: format!("{}{path}", base(config)), - headers: headers(config, Some(APPLICATION_JSON.to_owned())), + headers: headers(config, Some(APPLICATION_JSON.to_owned()))?, // The CID is base32 alphanumerics, so it needs no JSON escaping. body: Some(format!("{{\"{field}\":\"{cid}\"}}").into_bytes()), credentials: HttpCredentials::Omit, timeout_ms: Some(PLACEMENT_TIMEOUT_MS), - } + }) } /// The address Kubo reports storing the block under, held to the caller's. The @@ -262,18 +270,22 @@ fn base(config: &ByoIpfsConfig) -> &str { /// The bearer the config carries, plus a content type when the request has a /// body. The configured access token is the only credential a BYO endpoint gets. -fn headers(config: &ByoIpfsConfig, content_type: Option) -> Vec<(String, String)> { +/// +/// Fallible at the splice as well as at [`validate_byo_config`]: the rule that +/// makes a bearer safe to send belongs to the request being built, not to +/// whichever caller remembered to run the config gate first. +fn headers( + config: &ByoIpfsConfig, + content_type: Option, +) -> Result, ProviderError> { let mut headers = Vec::new(); if let Some(token) = &config.access_token { - headers.push(( - AUTHORIZATION.to_owned(), - format!("Bearer {}", token.as_str()), - )); + headers.push(bearer_header(token.as_str()).map_err(|_| ProviderError::InvalidCredential)?); } if let Some(content_type) = content_type { headers.push((CONTENT_TYPE.to_owned(), content_type)); } - headers + Ok(headers) } /// Why a provider connection test did not succeed. The first four are policy @@ -326,7 +338,7 @@ pub async fn test_connection( http: &impl Http, ) -> Result<(), ProviderError> { validate_byo_config(config)?; - let response = capped(http, probe_request(config)).await?; + let response = capped(http, probe_request(config)?).await?; if (200..300).contains(&response.status) { Ok(()) } else { @@ -343,14 +355,14 @@ pub async fn test_connection( pub fn validate_byo_config(config: &ByoIpfsConfig) -> Result<(), ProviderError> { validate_endpoint(&config.endpoint)?; match &config.access_token { - // The token is spliced into a header value verbatim, so a control - // character in it would inject a header. A present-but-empty one is an - // `Authorization: Bearer ` no provider accepts; `None` is how a - // credential-less provider is spelled. - Some(token) if token.is_empty() || !token.bytes().all(is_bearer_byte) => { - Err(ProviderError::InvalidCredential) - } - _ => Ok(()), + // Held to the seam's header-value rule: a present-but-empty token is an + // `Authorization: Bearer ` no provider accepts, and a control character + // in one would inject a header. `None` is how a credential-less + // provider is spelled, so it is not a verdict. + Some(token) => bearer_header(token.as_str()) + .map(drop) + .map_err(|_| ProviderError::InvalidCredential), + None => Ok(()), } } @@ -464,29 +476,23 @@ fn is_path_byte(b: u8) -> bool { is_authority_byte(b) || matches!(b, b'/' | b'_' | b'~' | b'%' | b'+' | b'=' | b'&' | b',') } -/// The bytes a bearer credential admits: visible ASCII, the header-value set -/// minus the whitespace no token carries. -fn is_bearer_byte(b: u8) -> bool { - matches!(b, 0x21..=0x7e) -} - /// The per-kind reachability probe. The endpoints are each provider's standard /// identity/auth check: Kubo `POST /api/v0/id`, PSA `GET /pins?limit=1`, Pinata /// `GET /data/testAuthentication`. -fn probe_request(config: &ByoIpfsConfig) -> HttpRequest { +fn probe_request(config: &ByoIpfsConfig) -> Result { let (method, path) = match config.kind { ByoKind::Kubo => (HttpMethod::Post, "/api/v0/id"), ByoKind::Psa => (HttpMethod::Get, "/pins?limit=1"), ByoKind::Pinata => (HttpMethod::Get, "/data/testAuthentication"), }; - HttpRequest { + Ok(HttpRequest { method, url: format!("{}{path}", base(config)), - headers: headers(config, None), + headers: headers(config, None)?, body: None, credentials: HttpCredentials::Omit, timeout_ms: Some(PROBE_TIMEOUT_MS), - } + }) } #[cfg(test)] @@ -494,7 +500,7 @@ mod tests { use super::*; use cipherbox_core::content::compute_cid; - use crate::seams::HttpResponse; + use crate::seams::{AUTHORIZATION, HttpResponse}; use crate::testkit::block_on; use crate::testkit::fakes::ScriptedHttp; diff --git a/crates/engine/src/content/read.rs b/crates/engine/src/content/read.rs index 1c72eb4f0..65dd3a8ae 100644 --- a/crates/engine/src/content/read.rs +++ b/crates/engine/src/content/read.rs @@ -20,9 +20,10 @@ use zeroize::Zeroizing; use super::dag::DAG_ROOT_CODEC; use super::limits::MAX_RESOLVED_RECORD_BYTES; -use crate::seams::{CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest}; +use crate::seams::{ + CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, SeamError, bearer_header, +}; -const AUTHORIZATION: &str = "Authorization"; const ACCEPT: &str = "Accept"; /// Deadline for one leaf-block GET: a seek issues one per leaf against sources /// of unknown quality, so a stalled gateway must fail over. @@ -270,10 +271,11 @@ async fn fetch( let base = source.base_url.trim_end_matches('/'); let mut headers = vec![(ACCEPT.to_owned(), RAW_BLOCK.to_owned())]; if let Some(bearer) = &source.bearer { - headers.push(( - AUTHORIZATION.to_owned(), - format!("Bearer {}", bearer.as_str()), - )); + // A source whose token cannot be a header value is skipped, never + // contacted unauthenticated: rotation drops to the next source. + headers.push(bearer_header(bearer.as_str()).map_err(|_| { + CappedFetchError::Transport(SeamError::new("gateway source bearer is unusable")) + })?); } let request = HttpRequest { method: HttpMethod::Get, @@ -322,7 +324,7 @@ mod tests { use super::*; use crate::content::chunk::{ContentKey, frame_and_seal}; use crate::content::profile::ContentProfile; - use crate::seams::HttpResponse; + use crate::seams::{AUTHORIZATION, HttpResponse}; use crate::testkit::SeededEntropy; use crate::testkit::block_on; use crate::testkit::fakes::ScriptedHttp; @@ -550,6 +552,40 @@ mod tests { ); } + /// A source whose bearer cannot be a header value is skipped, never + /// contacted without it — and rotation still reaches a healthy source. + #[test] + fn a_source_with_an_unusable_bearer_is_skipped_not_contacted_bare() { + let leaf = one_leaf(); + let http = ScriptedHttp::default(); + http.enqueue_response(raw_response(leaf.sealed.clone())); + + let gateway = Gateway { + accelerator: Some(GatewaySource { + base_url: "https://gw.cipherbox.test".into(), + bearer: Some(Zeroizing::new("member\r\nX-Injected: 1".to_owned())), + }), + public_fallbacks: vec![GatewaySource { + base_url: "https://public.gw.test".into(), + bearer: None, + }], + }; + + let out = block_on(read_block( + &gateway, + &http, + &cid_str(), + &leaf.cid, + ContentPlane::Leaf, + )) + .unwrap(); + assert_eq!(out, leaf.sealed); + + let requests = http.requests(); + assert_eq!(requests.len(), 1, "the accelerator was never contacted"); + assert!(requests[0].url.starts_with("https://public.gw.test/ipfs/")); + } + #[test] fn all_sources_unavailable_is_unavailable_not_a_violation() { let leaf = one_leaf(); diff --git a/crates/engine/src/facade.rs b/crates/engine/src/facade.rs index c68e1f95c..9b69fe085 100644 --- a/crates/engine/src/facade.rs +++ b/crates/engine/src/facade.rs @@ -3367,11 +3367,15 @@ mod tests { #[test] fn start_performs_identity_login_and_persists_a_refresh_token() { + /// Shaped as the API issues one; the engine signs nothing else. + const LOGIN_CHALLENGE: &str = + "cipherbox-login:v2:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + let (mut engine, _events, device) = engine_over(ApiBaseUrl::parse("http://api.test").expect("a configured base")); device.http.enqueue_response(json_response( 200, - json!({ "challenge": "cipherbox-login:v2:abc", "expiresAt": "2099-01-01T00:00:00Z" }), + json!({ "challenge": LOGIN_CHALLENGE, "expiresAt": "2099-01-01T00:00:00Z" }), )); device.http.enqueue_response(json_response( 200, @@ -3394,7 +3398,7 @@ mod tests { let identity = engine.session().unwrap().identity(); let expected = hex_lower( &identity - .sign_detcbor(b"cipherbox-login:v2:abc") + .sign_detcbor(LOGIN_CHALLENGE.as_bytes()) .to_compact(), ); assert_eq!(login_body["signature"], expected); diff --git a/crates/engine/src/seams/http.rs b/crates/engine/src/seams/http.rs index 582aa0ecc..4bd86f376 100644 --- a/crates/engine/src/seams/http.rs +++ b/crates/engine/src/seams/http.rs @@ -24,6 +24,31 @@ pub enum CappedFetchError { }, } +/// The `Authorization` header name — one spelling for every splice site. +pub const AUTHORIZATION: &str = "Authorization"; + +/// A bearer credential refused before it became a header value. Carries +/// nothing: the token itself must never reach an error string or a log. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct InvalidBearer; + +/// Builds the `Authorization: Bearer …` pair for `token`, refusing an empty one +/// or one carrying a byte outside visible ASCII (`0x21..=0x7e`). +/// +/// A header value is the host transport's input, and a control character or +/// space in one splits or injects a header — which of the two happens depends +/// on the transport, so the decision does not belong to the seam. The engine's +/// three bearer sources differ in trust class (a member's BYO config token, an +/// access token decoded out of an `/auth/*` body, a gateway source's token) but +/// not in this obligation, so it lives once, here, beside the request type that +/// carries it. +pub fn bearer_header(token: &str) -> Result<(String, String), InvalidBearer> { + if token.is_empty() || !token.bytes().all(|byte| (0x21..=0x7e).contains(&byte)) { + return Err(InvalidBearer); + } + Ok((AUTHORIZATION.to_owned(), format!("Bearer {token}"))) +} + /// Formats headers as their names only. Header values ride this seam /// carrying live credentials (`Authorization` bearer JWTs, refresh /// cookies) and must never reach logs. @@ -154,10 +179,14 @@ impl fmt::Debug for HttpResponse { /// are responses, not errors — a seam `Err` is reserved for transport-level /// failure (unreachable, aborted, deadline elapsed). /// -/// One obligation the transport owns: a request the engine sent over `https` -/// must not be replayed over `http` by following a redirect, or an -/// `Authorization` header would reach the clear network past the engine's -/// transport decision (blueprint/engine.md "Content plane"). +/// One obligation the transport owns: it must not follow a redirect. Every +/// target on this seam — the API, a gateway, a BYO provider — is directly +/// addressed and gated on the URL the engine supplied, so a hop the engine did +/// not choose can only escape that gate: it replays the request past +/// [`crate::content::validate_byo_config`]'s endpoint rules, and a downgrade to +/// `http` would carry an `Authorization` header onto the clear network +/// (blueprint/engine.md "Content plane"). A 3xx is surfaced as the response it +/// is, and the engine treats it as the non-2xx it is. /// /// No conformance kit ships for this seam: it has no seam-local durable /// semantics; its behavior is exercised end-to-end by the live contract @@ -220,6 +249,41 @@ mod tests { assert!(debug.contains("<19 bytes>"), "body renders as a length"); } + #[test] + fn a_usable_bearer_becomes_the_authorization_pair() { + assert_eq!( + bearer_header("eyJhbGciOi.J9-_~+/=").unwrap(), + ( + AUTHORIZATION.to_owned(), + "Bearer eyJhbGciOi.J9-_~+/=".to_owned() + ) + ); + } + + #[test] + fn a_bearer_that_could_reshape_the_request_is_refused() { + for token in [ + "", // an `Authorization: Bearer ` no server accepts + "jwt\r\nX-Injected: 1", // header injection + "jwt\nX-Injected: 1", // bare LF + "jwt\r", // bare CR + "jwt token", // a space splits the credential + "jwt\ttoken", // tab + "jwt\0", // NUL + "jwt\u{7f}", // DEL + "jwt\u{80}", // non-ASCII + "jwt\u{2028}", // line separator + ] { + assert_eq!(bearer_header(token), Err(InvalidBearer), "token {token:?}"); + } + } + + #[test] + fn the_bearer_refusal_carries_no_credential() { + let refusal = bearer_header("super-secret-jwt token").unwrap_err(); + assert!(!format!("{refusal:?}").contains("super-secret-jwt")); + } + #[test] fn response_debug_redacts_header_values_and_body() { let response = HttpResponse { diff --git a/crates/engine/src/seams/mod.rs b/crates/engine/src/seams/mod.rs index a77712959..bd671cb7a 100644 --- a/crates/engine/src/seams/mod.rs +++ b/crates/engine/src/seams/mod.rs @@ -22,7 +22,10 @@ mod staging_store; pub use credential_store::CredentialStore; pub use floor_store::{FloorNamespace, FloorRaise, FloorStore}; -pub use http::{CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse}; +pub use http::{ + AUTHORIZATION, CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, + InvalidBearer, bearer_header, +}; pub use mailbox::{Mailbox, MailboxItem}; pub use record_transport::{EndpointId, RecordTransport}; pub use refresh_hint::{RefreshHint, RefreshHintSource}; diff --git a/packages/client/src/seams/http.test.ts b/packages/client/src/seams/http.test.ts index 158401cdc..d82e12169 100644 --- a/packages/client/src/seams/http.test.ts +++ b/packages/client/src/seams/http.test.ts @@ -95,6 +95,27 @@ describe('FetchHttp credential scoping', () => { }); }); +describe('FetchHttp redirects', () => { + it('refuses them on both paths, as the record transport does', async () => { + const fetches = recordingFetch(); + const http = new FetchHttp(); + + await http.send(GET); + await http.sendCapped(GET, 1000); + + expect(fetches.inits.map((init) => init.redirect)).toEqual(['error', 'error']); + }); + + it('rejects rather than resolving when the browser refuses a hop', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.reject(new TypeError('Failed to fetch'))) + ); + + await expect(new FetchHttp().send(GET)).rejects.toThrow(TypeError); + }); +}); + describe('FetchHttp deadlines', () => { it('carries no abort signal when the request sets no deadline', async () => { const fetches = recordingFetch(); diff --git a/packages/client/src/seams/http.ts b/packages/client/src/seams/http.ts index 7564a2d42..0eaaecc30 100644 --- a/packages/client/src/seams/http.ts +++ b/packages/client/src/seams/http.ts @@ -25,6 +25,10 @@ function requestInit(request: HttpRequestData): RequestInit { headers, // Fail-safe default: authority is opt-in per request, never inferred. credentials: request.credentials ?? 'omit', + // Every target here is directly addressed and gated on the URL the engine + // supplied, so a hop the engine did not choose can only escape that gate. + // Mirrors `FetchRecordTransport`; a redirect rejects rather than resolving. + redirect: 'error', }; const timeoutMs = request.timeoutMs; if (timeoutMs !== undefined && timeoutMs !== null) { diff --git a/packages/client/test/browser/mockAuth.ts b/packages/client/test/browser/mockAuth.ts index 33ccea991..56610bbdf 100644 --- a/packages/client/test/browser/mockAuth.ts +++ b/packages/client/test/browser/mockAuth.ts @@ -42,7 +42,9 @@ function challenge(res: ServerResponse, dto: Fields): void { send(res, 400, { error: 'publicKey must be a string' }); return; } - const value = `cipherbox-login:v2:${publicKey.slice(0, 16)}`; + // Shaped as the API issues one — the domain tag plus 32 bytes of lowercase + // hex — because the engine refuses to sign anything else. + const value = `cipherbox-login:v2:${publicKey.slice(0, 64).padEnd(64, '0')}`; issued.set(publicKey, value); completed.challenges += 1; send(res, 200, { challenge: value, expiresAt: '2099-01-01T00:00:00Z' }); From c26919db3445a24c2b5d676503d16b711e75f25e Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Thu, 6 Aug 2026 01:35:47 +0200 Subject: [PATCH 2/3] fix: split the bearer predicate from its formatter and drop the unreachable splice plumbing Folds the /simplify, /security-review and /crypto-privacy-review passes over this branch's own diff back into the code. crypto-privacy, LOW, a real regression this branch introduced: routing `validate_byo_config` through `bearer_header` made a pure byte scan build and drop a non-zeroized `"Bearer " + token` on every settings encode and decode -- inside the module that declares itself the token's terminal zeroizing owner. `check_bearer` now owns the rule and `bearer_header` calls it, so a caller that only asks the question never materializes a second copy of the credential. simplify: the fallibility threaded through `headers`, `kubo_block_put`, `pin_by_cid` and `probe_request` was unreachable -- both public entries run `validate_byo_config` on the line above, and that gate is now the same predicate. Reverted, with the shared rule kept at the gate and the shared header name kept at the splice, plus a test pinning that the two agree on every token. The genuinely ungated splices, in the API client and the gateway read path, keep their check. simplify: comment and test bloat. The refresh invariant was stated three times, the redirect rationale re-derived at each impl rather than cited from the trait, and the challenge doc argued against a design that is not in the code while hardcoding a byte count that would rot. `finish` and `Drop` both released leadership; `waiters` now only takes. crypto-privacy: the hostile-challenge test proved the flow never *sent*, not that the key never *signed*. It now runs against a signer that panics if invoked, and covers the bare tag, mixed case, a wholly attacker-chosen tail, both bytes-vs-chars boundaries, and version confusion -- plus an accept-side test so a tightening that would break a real login fails here rather than in staging. The API side gains the matching shape assertion, so the cross-language contract is pinned on both ends. altitude: the no-redirect obligation was documented but unchecked on the Rust host. A live 302 route in the desktop mock proves the seam surfaces the hop rather than taking it, and never reaches its target. Also carries the fix into `crates/load`, the fourth bearer splice site and the one `with_client` caller, which was still hand-splicing an unvalidated bearer and still following up to ten hops. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WegkkQ3uhNREerTW4MMeY2 --- .../auth/services/challenge.service.test.ts | 8 ++ crates/desktop-seams/src/http.rs | 9 +- crates/desktop-seams/tests/conformance.rs | 28 ++++ crates/desktop-seams/tests/mock_http/mod.rs | 6 + crates/engine/src/api/client.rs | 123 ++++++++++-------- crates/engine/src/content/provider.rs | 89 ++++++------- crates/engine/src/seams/http.rs | 45 ++++--- crates/engine/src/seams/mod.rs | 7 +- crates/load/src/runner.rs | 14 +- crates/load/src/seams.rs | 4 + packages/client/src/seams/http.test.ts | 9 -- packages/client/src/seams/http.ts | 4 +- 12 files changed, 202 insertions(+), 144 deletions(-) diff --git a/apps/api/src/auth/services/challenge.service.test.ts b/apps/api/src/auth/services/challenge.service.test.ts index 003c10dd5..197c1fa75 100644 --- a/apps/api/src/auth/services/challenge.service.test.ts +++ b/apps/api/src/auth/services/challenge.service.test.ts @@ -21,6 +21,14 @@ describe('ChallengeService', () => { expect(expiresAt.getTime()).toBe(clock.now().getTime() + 300_000); }); + // The engine pins this exact shape before it will sign a challenge, and + // refuses anything else. Narrowing the tail here fails every login, so the + // contract is pinned on both sides rather than only in Rust. + it('issues a tail the engine will sign: 32 bytes of lowercase hex', () => { + const { challenge } = service.issueIdentityChallenge(PUBLIC_KEY); + expect(challenge).toMatch(/^cipherbox-login:v2:[0-9a-f]{64}$/); + }); + it('consumes a live challenge exactly once', () => { const { challenge } = service.issueIdentityChallenge(PUBLIC_KEY); service.consume(challenge, 'identity', PUBLIC_KEY); diff --git a/crates/desktop-seams/src/http.rs b/crates/desktop-seams/src/http.rs index a874f305f..837f7da57 100644 --- a/crates/desktop-seams/src/http.rs +++ b/crates/desktop-seams/src/http.rs @@ -34,14 +34,7 @@ impl ReqwestHttp { pub fn new() -> SeamResult { let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) - // The engine picks every target on this seam and gates it on the URL - // it supplied — the API base, a gateway, a BYO endpoint past - // `validate_byo_config`. A hop the engine did not choose can only - // escape that gate, so none is followed: it re-points a directly - // addressed request at a host of the responder's choosing, and a - // downgrade to `http` would carry an `Authorization` header onto the - // clear network. Mirrors `ReqwestRecordTransport`; a 3xx comes back - // as the non-2xx response it is. + // No redirects; see the `Http` seam contract. .redirect(reqwest::redirect::Policy::none()) .build() .map_err(|err| SeamError::new(format!("http client build: {err}")))?; diff --git a/crates/desktop-seams/tests/conformance.rs b/crates/desktop-seams/tests/conformance.rs index a87339482..2e50cfaf0 100644 --- a/crates/desktop-seams/tests/conformance.rs +++ b/crates/desktop-seams/tests/conformance.rs @@ -337,6 +337,34 @@ async fn reqwest_http_returns_non_2xx_as_a_response_not_an_error() { assert_eq!(response.status, 418); } +/// The seam follows no redirect: the 3xx comes back as the response it is, and +/// the `Authorization` header never reaches the hop's target. A doc comment +/// cannot fail CI, so the policy is asserted against a live server. +#[tokio::test] +async fn reqwest_http_follows_no_redirect_and_does_not_replay_the_bearer() { + let server = MockServer::start(); + let http = ReqwestHttp::new().expect("client builds"); + + let response = http + .send(HttpRequest { + method: HttpMethod::Get, + url: format!("{}/redirect", server.base_url()), + headers: vec![("Authorization".into(), "Bearer member-token".into())], + body: None, + credentials: HttpCredentials::Omit, + timeout_ms: None, + }) + .await + .expect("a 3xx is a response, never a seam Err"); + + assert_eq!(response.status, 302, "the hop is surfaced, not taken"); + + // `/redirect` served no body and `/echo` would have echoed one, so the + // recorded request proves the target was never reached. + let recorded = server.last_request().expect("a request was recorded"); + assert_eq!(recorded.path, "/redirect"); +} + #[tokio::test] async fn reqwest_http_capped_fetch_rejects_a_chunk_larger_than_the_cap() { let server = MockServer::start(); diff --git a/crates/desktop-seams/tests/mock_http/mod.rs b/crates/desktop-seams/tests/mock_http/mod.rs index 33c2e17a3..cc729295a 100644 --- a/crates/desktop-seams/tests/mock_http/mod.rs +++ b/crates/desktop-seams/tests/mock_http/mod.rs @@ -10,6 +10,7 @@ //! header; records the request for assertions. //! - `GET /teapot` — returns 418, to prove a non-2xx status is a response, //! not a seam error. +//! - `GET /redirect` — 302 to `/echo`, to prove the seam follows no hop. //! - `GET /stream/` — `n` bytes with `Transfer-Encoding: chunked` and no //! `Content-Length`, so a capped read has only the streaming drain to gate //! on. @@ -28,6 +29,8 @@ use std::time::Duration; /// One request the server received, captured for test assertions. #[derive(Clone)] pub struct RecordedRequest { + /// Request target, so a test can prove which route was reached. + pub path: String, pub headers: Vec<(String, String)>, pub body: Vec, } @@ -157,6 +160,7 @@ fn handle_conn( body.truncate(content_length); *last.lock().expect("lock") = Some(RecordedRequest { + path: path.clone(), headers, body: body.clone(), }); @@ -186,6 +190,8 @@ fn handle_conn( (200, "OK", vec![("x-echo", "yes")], body) } else if path == "/teapot" { (418, "I'm a teapot", Vec::new(), b"teapot".to_vec()) + } else if path == "/redirect" { + (302, "Found", vec![("location", "/echo")], Vec::new()) } else { (404, "Not Found", Vec::new(), Vec::new()) }; diff --git a/crates/engine/src/api/client.rs b/crates/engine/src/api/client.rs index e0beff463..e54ae4752 100644 --- a/crates/engine/src/api/client.rs +++ b/crates/engine/src/api/client.rs @@ -58,19 +58,16 @@ struct State { refresh_waiters: Option>>>, } -/// Holds single-flight leadership for the duration of one rotation and gives it -/// back however the leader leaves — including a drop mid-`await`. -/// -/// Without this, a leader cancelled while parked on the network leaves -/// `refresh_waiters` occupied with senders nothing will ever fire, and every -/// later caller enqueues behind it forever. +/// Holds single-flight leadership for one rotation and releases it on `Drop`, +/// so a leader cancelled while parked on the network cannot leave the slot +/// occupied by senders nothing will ever fire. struct RefreshLead<'a> { state: &'a RefCell, } impl RefreshLead<'_> { - /// Releases leadership and takes the waiters to notify. - fn finish(self) -> Vec>> { + /// Takes the waiters to notify, leaving the slot for `Drop` to release. + fn waiters(&self) -> Vec>> { self.state .borrow_mut() .refresh_waiters @@ -81,8 +78,6 @@ impl RefreshLead<'_> { impl Drop for RefreshLead<'_> { fn drop(&mut self) { - // Dropping the senders wakes every waiter with `Canceled`; leaving the - // slot occupied would park them, and everyone after, indefinitely. self.state.borrow_mut().refresh_waiters = None; } } @@ -270,10 +265,8 @@ impl ApiClient { if let Some(rx) = receiver { return match rx.await { Ok(result) => result, - // The leader was dropped before it could answer: availability, - // not a dead session — the caller must not be told to re-login. - // The leader clears the slot as it goes, so the next call leads - // a fresh rotation instead of parking behind this one. + // A cancelled leader is availability, not a dead session — the + // caller must not be told to re-login. Err(oneshot::Canceled) => Err(ApiError::Transport(SeamError::new( "refresh was cancelled before it completed", ))), @@ -282,10 +275,7 @@ impl ApiClient { let lead = RefreshLead { state: &self.state }; let result = self.do_refresh().await; - // Hand leadership back before notifying: a waiter woken with an error - // may lead its own retry, and must not find the slot still occupied. - let waiters = lead.finish(); - for tx in waiters { + for tx in lead.waiters() { let _ = tx.send(result.clone()); } result @@ -553,9 +543,7 @@ impl ApiClient { for (name, value) in extra_headers { headers.push(((*name).to_owned(), (*value).to_owned())); } - // Scope the borrow so it never crosses the await below. The access token - // is decoded out of an `/auth/*` body, so it is the API's bytes, not the - // engine's: it meets the seam's header-value rule like any other bearer. + // Scope the borrow so it never crosses the await below. if let Some(token) = self.state.borrow().access_token.as_ref() { headers.push( bearer_header(token.as_str()) @@ -655,12 +643,10 @@ const IDENTITY_CHALLENGE_NONCE_LEN: usize = 64; /// Whether the server's answer is a challenge this key may sign: the login /// domain tag followed by exactly the API's random tail. /// -/// The signer hands `sha256(utf8(challenge))` to the same secp256k1 identity -/// key that signs det-CBOR structures, so an unchecked challenge is a signing -/// oracle for any UTF-8 preimage. The prefix alone would not close it — it -/// pins the first 19 bytes and leaves the rest of the preimage to whatever -/// answers at the API base URL — so the whole shape is pinned instead, leaving -/// a hostile responder no steerable bytes outside `[0-9a-f]`. +/// The signer hands `sha256(utf8(challenge))` to the secp256k1 identity key, +/// so an unchecked challenge makes that key a signing oracle for any UTF-8 +/// preimage. Pinning the whole shape — not just the tag — leaves a hostile +/// responder no steerable byte outside the hex alphabet the API renders. fn is_identity_challenge(challenge: &str) -> bool { challenge .strip_prefix(IDENTITY_CHALLENGE_PREFIX) @@ -668,7 +654,7 @@ fn is_identity_challenge(challenge: &str) -> bool { nonce.len() == IDENTITY_CHALLENGE_NONCE_LEN && nonce .bytes() - .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) + .all(|byte| matches!(byte, b'0'..=b'9' | b'a'..=b'f')) }) } @@ -818,52 +804,81 @@ mod tests { assert_eq!(stored, "a".repeat(64).as_bytes()); } - /// A challenge the API could not have issued is never signed, and the flow - /// stops before `/auth/login` — the identity key answers no one else's - /// preimage. Each case names the part of the shape it breaks. - #[test] - fn a_challenge_the_api_could_not_have_issued_is_never_signed() { + /// Every shape the API could not have issued. Each breaks a different part + /// of the pin, so none is subsumed by another. + fn hostile_challenges() -> Vec { let hex64 = "0123456789abcdef".repeat(4); - let hostile = [ - "".to_owned(), + vec![ + String::new(), + // The tag with no tail at all. + IDENTITY_CHALLENGE_PREFIX.to_owned(), // No domain tag: an arbitrary preimage of the responder's choosing. hex64.clone(), - // Another protocol's tag. + // Another protocol's tag, and an older version of this one. format!("cipherbox-grant:v2:{hex64}"), - // Right tag, but the tail is the responder's text. - "cipherbox-login:v2:sign-over-this-instead".to_owned(), - // Right tag and alphabet, wrong width — short. - format!("{IDENTITY_CHALLENGE_PREFIX}{}", "ab".repeat(8)), - // Right tag and alphabet, wrong width — long. + format!("cipherbox-login:v1:{hex64}"), + // The tag as a suffix, not a prefix — guards a `contains` regression. + format!("{hex64}{IDENTITY_CHALLENGE_PREFIX}"), + // Right tag and alphabet, wrong width — short, then long. + format!("{IDENTITY_CHALLENGE_PREFIX}{}", &hex64[..63]), format!("{IDENTITY_CHALLENGE_PREFIX}{hex64}0"), - // Right width, outside the hex alphabet the API renders. + // Right width, outside the hex alphabet: all-caps, one uppercase + // digit among 64, then a wholly attacker-chosen tail. format!( "{IDENTITY_CHALLENGE_PREFIX}{}", "0123456789ABCDEF".repeat(4) ), - // The tag as a suffix rather than a prefix. - format!("{hex64}{IDENTITY_CHALLENGE_PREFIX}"), - // Leading whitespace before the tag. - format!(" {IDENTITY_CHALLENGE_PREFIX}{hex64}"), - ]; + format!("{IDENTITY_CHALLENGE_PREFIX}{}A", &hex64[..63]), + format!("{IDENTITY_CHALLENGE_PREFIX}{:_<64}", "sign anything"), + // 64 chars but 65 bytes, then 64 bytes with a multi-byte tail: the + // width check counts bytes, and the alphabet catches what it misses. + format!("{IDENTITY_CHALLENGE_PREFIX}{}\u{e9}", &hex64[..63]), + format!("{IDENTITY_CHALLENGE_PREFIX}{}\u{e9}", &hex64[..62]), + // An interior control character; the tail is echoed to /auth/login. + format!("{IDENTITY_CHALLENGE_PREFIX}{}\0", &hex64[..63]), + ] + } + + /// The accept side of the pin: every tail the API's hex renderer can emit + /// is admitted, so a tightening that would break a real login fails here + /// rather than in staging. + #[test] + fn the_shape_the_api_issues_is_accepted_at_the_class_boundaries() { + for tail in ["0".repeat(64), "f".repeat(64), "0123456789abcdef".repeat(4)] { + assert!(is_identity_challenge(&format!( + "{IDENTITY_CHALLENGE_PREFIX}{tail}" + ))); + } + } - for challenge in hostile { + /// The guard is "never signs", not merely "never sends": a refused + /// challenge must not reach the identity key at all. + #[test] + fn a_challenge_the_api_could_not_have_issued_is_never_signed() { + struct PanickingSigner; + + impl ChallengeSigner for PanickingSigner { + fn public_key_hex(&self) -> String { + "02".to_owned() + &"ab".repeat(32) + } + fn sign_challenge(&self, challenge: &str) -> String { + panic!("the identity key signed a refused challenge: {challenge:?}"); + } + } + + for challenge in hostile_challenges() { let (http, _creds, client) = fakes(); http.enqueue_response(json_response( 200, json!({ "challenge": challenge, "expiresAt": "2026-01-01T00:00:00Z" }), )); assert_eq!( - block_on(client.login_identity(&StubSigner)).unwrap_err(), + block_on(client.login_identity(&PanickingSigner)).unwrap_err(), ApiError::Decode("unusable login challenge".into()), "challenge {challenge:?} must be refused" ); let requests = http.requests(); - assert_eq!( - requests.len(), - 1, - "only /auth/challenge was sent for {challenge:?}" - ); + assert_eq!(requests.len(), 1, "only /auth/challenge for {challenge:?}"); assert_eq!(requests[0].url, "http://api.test/auth/challenge"); assert!(!client.is_authenticated()); } diff --git a/crates/engine/src/content/provider.rs b/crates/engine/src/content/provider.rs index c6258f08f..5466b793d 100644 --- a/crates/engine/src/content/provider.rs +++ b/crates/engine/src/content/provider.rs @@ -17,7 +17,8 @@ use zeroize::Zeroizing; use crate::content::DAG_ROOT_CODEC; use crate::seams::{ - CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, bearer_header, + AUTHORIZATION, CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, + check_bearer, }; /// Deadline for a BYO-provider reachability probe: an unresponsive endpoint @@ -117,9 +118,9 @@ pub(crate) async fn place_block( validate_byo_config(config)?; let address = content_address(cid)?; let request = match config.kind { - ByoKind::Kubo => kubo_block_put(config, &address, block)?, - ByoKind::Psa => pin_by_cid(config, "/pins", "cid", &address.cid)?, - ByoKind::Pinata => pin_by_cid(config, "/pinning/pinByHash", "hashToPin", &address.cid)?, + ByoKind::Kubo => kubo_block_put(config, &address, block), + ByoKind::Psa => pin_by_cid(config, "/pins", "cid", &address.cid), + ByoKind::Pinata => pin_by_cid(config, "/pinning/pinByHash", "hashToPin", &address.cid), }; let response = capped(http, request).await?; if !(200..300).contains(&response.status) { @@ -175,11 +176,7 @@ fn content_address(cid: &[u8]) -> Result { /// `block/put` under the block's own codec and the frozen BLAKE3-256 framing, /// pinned in the same call, so the member's node addresses it exactly as the /// engine does. -fn kubo_block_put( - config: &ByoIpfsConfig, - address: &ContentAddress, - block: &[u8], -) -> Result { +fn kubo_block_put(config: &ByoIpfsConfig, address: &ContentAddress, block: &[u8]) -> HttpRequest { // Derived from the block's own address, so the delimiter cannot occur in the // payload it frames: that would take a block carrying the base32 of its own // BLAKE3 digest, which is a preimage. 62 bytes of base32 and `-`, inside RFC @@ -196,7 +193,7 @@ fn kubo_block_put( body.extend_from_slice(block); body.extend_from_slice(tail.as_bytes()); let codec = address.codec; - Ok(HttpRequest { + HttpRequest { method: HttpMethod::Post, // A DAG root inlines a CID per leaf, so it passes Kubo's 1 MiB // block/put advisory well before the flat-DAG ceiling does. The block is @@ -209,29 +206,24 @@ fn kubo_block_put( headers: headers( config, Some(format!("multipart/form-data; boundary={boundary}")), - )?, + ), body: Some(body), credentials: HttpCredentials::Omit, timeout_ms: Some(PLACEMENT_TIMEOUT_MS), - }) + } } /// Ask a pin-by-CID service to pin an address it fetches itself. -fn pin_by_cid( - config: &ByoIpfsConfig, - path: &str, - field: &str, - cid: &str, -) -> Result { - Ok(HttpRequest { +fn pin_by_cid(config: &ByoIpfsConfig, path: &str, field: &str, cid: &str) -> HttpRequest { + HttpRequest { method: HttpMethod::Post, url: format!("{}{path}", base(config)), - headers: headers(config, Some(APPLICATION_JSON.to_owned()))?, + headers: headers(config, Some(APPLICATION_JSON.to_owned())), // The CID is base32 alphanumerics, so it needs no JSON escaping. body: Some(format!("{{\"{field}\":\"{cid}\"}}").into_bytes()), credentials: HttpCredentials::Omit, timeout_ms: Some(PLACEMENT_TIMEOUT_MS), - }) + } } /// The address Kubo reports storing the block under, held to the caller's. The @@ -270,22 +262,18 @@ fn base(config: &ByoIpfsConfig) -> &str { /// The bearer the config carries, plus a content type when the request has a /// body. The configured access token is the only credential a BYO endpoint gets. -/// -/// Fallible at the splice as well as at [`validate_byo_config`]: the rule that -/// makes a bearer safe to send belongs to the request being built, not to -/// whichever caller remembered to run the config gate first. -fn headers( - config: &ByoIpfsConfig, - content_type: Option, -) -> Result, ProviderError> { +fn headers(config: &ByoIpfsConfig, content_type: Option) -> Vec<(String, String)> { let mut headers = Vec::new(); if let Some(token) = &config.access_token { - headers.push(bearer_header(token.as_str()).map_err(|_| ProviderError::InvalidCredential)?); + headers.push(( + AUTHORIZATION.to_owned(), + format!("Bearer {}", token.as_str()), + )); } if let Some(content_type) = content_type { headers.push((CONTENT_TYPE.to_owned(), content_type)); } - Ok(headers) + headers } /// Why a provider connection test did not succeed. The first four are policy @@ -338,7 +326,7 @@ pub async fn test_connection( http: &impl Http, ) -> Result<(), ProviderError> { validate_byo_config(config)?; - let response = capped(http, probe_request(config)?).await?; + let response = capped(http, probe_request(config)).await?; if (200..300).contains(&response.status) { Ok(()) } else { @@ -355,13 +343,9 @@ pub async fn test_connection( pub fn validate_byo_config(config: &ByoIpfsConfig) -> Result<(), ProviderError> { validate_endpoint(&config.endpoint)?; match &config.access_token { - // Held to the seam's header-value rule: a present-but-empty token is an - // `Authorization: Bearer ` no provider accepts, and a control character - // in one would inject a header. `None` is how a credential-less - // provider is spelled, so it is not a verdict. - Some(token) => bearer_header(token.as_str()) - .map(drop) - .map_err(|_| ProviderError::InvalidCredential), + // `None` is how a credential-less provider is spelled, so it is not a + // verdict; a token that is present must be sendable as a header value. + Some(token) => check_bearer(token.as_str()).map_err(|_| ProviderError::InvalidCredential), None => Ok(()), } } @@ -479,20 +463,20 @@ fn is_path_byte(b: u8) -> bool { /// The per-kind reachability probe. The endpoints are each provider's standard /// identity/auth check: Kubo `POST /api/v0/id`, PSA `GET /pins?limit=1`, Pinata /// `GET /data/testAuthentication`. -fn probe_request(config: &ByoIpfsConfig) -> Result { +fn probe_request(config: &ByoIpfsConfig) -> HttpRequest { let (method, path) = match config.kind { ByoKind::Kubo => (HttpMethod::Post, "/api/v0/id"), ByoKind::Psa => (HttpMethod::Get, "/pins?limit=1"), ByoKind::Pinata => (HttpMethod::Get, "/data/testAuthentication"), }; - Ok(HttpRequest { + HttpRequest { method, url: format!("{}{path}", base(config)), - headers: headers(config, None)?, + headers: headers(config, None), body: None, credentials: HttpCredentials::Omit, timeout_ms: Some(PROBE_TIMEOUT_MS), - }) + } } #[cfg(test)] @@ -500,7 +484,7 @@ mod tests { use super::*; use cipherbox_core::content::compute_cid; - use crate::seams::{AUTHORIZATION, HttpResponse}; + use crate::seams::HttpResponse; use crate::testkit::block_on; use crate::testkit::fakes::ScriptedHttp; @@ -687,6 +671,23 @@ mod tests { ); } + /// The config gate and the header the splice builds are the same rule, so + /// no token can pass one and be refused by the other. + #[test] + fn the_config_gate_and_the_header_splice_agree_on_every_token() { + for token in ["", "tok\r\n", "tok tok", "tok\u{80}", "ok-token", "!", "~"] { + assert_eq!( + validate_byo_config(&config(ByoKind::Psa, Some(token))).is_ok(), + crate::seams::bearer_header(token).is_ok(), + "{token:?}" + ); + } + assert!( + validate_byo_config(&config(ByoKind::Psa, None)).is_ok(), + "a credential-less provider is not a verdict" + ); + } + fn leaf(bytes: &[u8]) -> Vec { compute_cid(CONTENT_CID_CODEC, bytes) } diff --git a/crates/engine/src/seams/http.rs b/crates/engine/src/seams/http.rs index 4bd86f376..34c7c30a1 100644 --- a/crates/engine/src/seams/http.rs +++ b/crates/engine/src/seams/http.rs @@ -32,20 +32,24 @@ pub const AUTHORIZATION: &str = "Authorization"; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct InvalidBearer; -/// Builds the `Authorization: Bearer …` pair for `token`, refusing an empty one -/// or one carrying a byte outside visible ASCII (`0x21..=0x7e`). +/// The rule a bearer must meet to be sent: non-empty, and visible ASCII +/// throughout (`0x21..=0x7e`). A control character or space in a header value +/// splits or injects a header at the host transport. /// -/// A header value is the host transport's input, and a control character or -/// space in one splits or injects a header — which of the two happens depends -/// on the transport, so the decision does not belong to the seam. The engine's -/// three bearer sources differ in trust class (a member's BYO config token, an -/// access token decoded out of an `/auth/*` body, a gateway source's token) but -/// not in this obligation, so it lives once, here, beside the request type that -/// carries it. -pub fn bearer_header(token: &str) -> Result<(String, String), InvalidBearer> { - if token.is_empty() || !token.bytes().all(|byte| (0x21..=0x7e).contains(&byte)) { +/// Separate from [`bearer_header`] so a caller that only asks the question — +/// a config gate — never materializes a second, non-zeroized copy of the +/// credential just to throw it away. +pub fn check_bearer(token: &str) -> Result<(), InvalidBearer> { + if token.is_empty() || !token.bytes().all(|byte| matches!(byte, 0x21..=0x7e)) { return Err(InvalidBearer); } + Ok(()) +} + +/// Builds the `Authorization: Bearer …` pair for a token meeting +/// [`check_bearer`]. +pub fn bearer_header(token: &str) -> Result<(String, String), InvalidBearer> { + check_bearer(token)?; Ok((AUTHORIZATION.to_owned(), format!("Bearer {token}"))) } @@ -185,8 +189,9 @@ impl fmt::Debug for HttpResponse { /// not choose can only escape that gate: it replays the request past /// [`crate::content::validate_byo_config`]'s endpoint rules, and a downgrade to /// `http` would carry an `Authorization` header onto the clear network -/// (blueprint/engine.md "Content plane"). A 3xx is surfaced as the response it -/// is, and the engine treats it as the non-2xx it is. +/// (blueprint/engine.md "Content plane"). How a refusal surfaces is the +/// transport's own: desktop returns the 3xx as a response, web rejects it as a +/// transport failure — both fail closed, so no caller may branch on which. /// /// No conformance kit ships for this seam: it has no seam-local durable /// semantics; its behavior is exercised end-to-end by the live contract @@ -258,6 +263,8 @@ mod tests { "Bearer eyJhbGciOi.J9-_~+/=".to_owned() ) ); + assert!(bearer_header("!").is_ok(), "0x21, the low edge"); + assert!(bearer_header("~").is_ok(), "0x7e, the high edge"); } #[test] @@ -265,19 +272,17 @@ mod tests { for token in [ "", // an `Authorization: Bearer ` no server accepts "jwt\r\nX-Injected: 1", // header injection - "jwt\nX-Injected: 1", // bare LF - "jwt\r", // bare CR "jwt token", // a space splits the credential - "jwt\ttoken", // tab - "jwt\0", // NUL - "jwt\u{7f}", // DEL - "jwt\u{80}", // non-ASCII - "jwt\u{2028}", // line separator + "jwt\u{7f}", // DEL, just above the class + "jwt\u{80}", // non-ASCII, and every multi-byte tail with it ] { assert_eq!(bearer_header(token), Err(InvalidBearer), "token {token:?}"); + assert_eq!(check_bearer(token), Err(InvalidBearer), "token {token:?}"); } } + /// A tripwire on the refusal type: it must stay field-less, so no future + /// diagnostic can carry the credential into an error string or a log. #[test] fn the_bearer_refusal_carries_no_credential() { let refusal = bearer_header("super-secret-jwt token").unwrap_err(); diff --git a/crates/engine/src/seams/mod.rs b/crates/engine/src/seams/mod.rs index bd671cb7a..a47503d0b 100644 --- a/crates/engine/src/seams/mod.rs +++ b/crates/engine/src/seams/mod.rs @@ -23,9 +23,12 @@ mod staging_store; pub use credential_store::CredentialStore; pub use floor_store::{FloorNamespace, FloorRaise, FloorStore}; pub use http::{ - AUTHORIZATION, CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, - InvalidBearer, bearer_header, + CappedFetchError, Http, HttpCredentials, HttpMethod, HttpRequest, HttpResponse, InvalidBearer, + bearer_header, check_bearer, }; +// The header name is an engine-internal spelling: a host implements the +// transport, it never builds a bearer. +pub(crate) use http::AUTHORIZATION; pub use mailbox::{Mailbox, MailboxItem}; pub use record_transport::{EndpointId, RecordTransport}; pub use refresh_hint::{RefreshHint, RefreshHintSource}; diff --git a/crates/load/src/runner.rs b/crates/load/src/runner.rs index d2aea3cfa..b913c4037 100644 --- a/crates/load/src/runner.rs +++ b/crates/load/src/runner.rs @@ -10,7 +10,9 @@ use std::time::{Duration, Instant}; use cipherbox_core::content::{CONTENT_CID_CODEC, compute_cid, encode_content_cid_str}; use cipherbox_desktop_seams::ReqwestHttp; use cipherbox_engine::api::{ApiClient, ApiError}; -use cipherbox_engine::seams::{Http, HttpCredentials, HttpMethod, HttpRequest}; +use cipherbox_engine::seams::{ + Http, HttpCredentials, HttpMethod, HttpRequest, SeamError, bearer_header, +}; use crate::metrics::{Collector, Outcome, Sample}; use crate::plan::{MAX_BLOCK_BYTES, RunPlan}; @@ -87,9 +89,13 @@ pub(crate) async fn gateway_get( url: &str, token: Option<&str>, ) -> Result { - let headers = token - .map(|token| vec![("Authorization".to_owned(), format!("Bearer {token}"))]) - .unwrap_or_default(); + let headers = match token { + Some(token) => vec![ + bearer_header(token) + .map_err(|_| ApiError::Transport(SeamError::new("gateway bearer is unusable")))?, + ], + None => Vec::new(), + }; let request = HttpRequest { method: HttpMethod::Get, url: url.to_owned(), diff --git a/crates/load/src/seams.rs b/crates/load/src/seams.rs index ebc3e81c4..b91ffd7fe 100644 --- a/crates/load/src/seams.rs +++ b/crates/load/src/seams.rs @@ -18,6 +18,10 @@ pub(crate) fn build_http() -> Result { let client = reqwest::Client::builder() .connect_timeout(Duration::from_secs(10)) .timeout(Duration::from_secs(60)) + // The seam forbids following a redirect, and a `with_client` caller + // owns that policy — a measured run must move bytes the way the + // shipping transport does. + .redirect(reqwest::redirect::Policy::none()) .build() .map_err(|error| format!("build reqwest client: {error}"))?; Ok(ReqwestHttp::with_client(client)) diff --git a/packages/client/src/seams/http.test.ts b/packages/client/src/seams/http.test.ts index d82e12169..0a0afe559 100644 --- a/packages/client/src/seams/http.test.ts +++ b/packages/client/src/seams/http.test.ts @@ -105,15 +105,6 @@ describe('FetchHttp redirects', () => { expect(fetches.inits.map((init) => init.redirect)).toEqual(['error', 'error']); }); - - it('rejects rather than resolving when the browser refuses a hop', async () => { - vi.stubGlobal( - 'fetch', - vi.fn(() => Promise.reject(new TypeError('Failed to fetch'))) - ); - - await expect(new FetchHttp().send(GET)).rejects.toThrow(TypeError); - }); }); describe('FetchHttp deadlines', () => { diff --git a/packages/client/src/seams/http.ts b/packages/client/src/seams/http.ts index 0eaaecc30..64545af5e 100644 --- a/packages/client/src/seams/http.ts +++ b/packages/client/src/seams/http.ts @@ -25,9 +25,7 @@ function requestInit(request: HttpRequestData): RequestInit { headers, // Fail-safe default: authority is opt-in per request, never inferred. credentials: request.credentials ?? 'omit', - // Every target here is directly addressed and gated on the URL the engine - // supplied, so a hop the engine did not choose can only escape that gate. - // Mirrors `FetchRecordTransport`; a redirect rejects rather than resolving. + // No redirects; see the `Http` seam contract. Rejects rather than resolving. redirect: 'error', }; const timeoutMs = request.timeoutMs; From bdb8309d00021f455b67c73a7f36759e51125400 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Thu, 6 Aug 2026 06:52:12 +0200 Subject: [PATCH 3/3] fix: drop an unusable access token so the next call can self-heal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An access token the seam refuses as a header value stayed in memory, so every later authenticated call repeated the same refusal and the session could not recover. Drop it instead: the next call goes out unauthenticated and its 401 buys one rotation. Only the in-memory access token is dropped — the refresh credential is a separate secret, so a malformed response cannot end the session. Also exercise the Http seam's redirect refusal at runtime rather than asserting the RequestInit field alone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WegkkQ3uhNREerTW4MMeY2 --- crates/engine/src/api/client.rs | 46 +++++++++++++++++++++++--- packages/client/src/seams/http.test.ts | 24 ++++++++++++++ 2 files changed, 65 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/api/client.rs b/crates/engine/src/api/client.rs index e54ae4752..c6dd8a5e9 100644 --- a/crates/engine/src/api/client.rs +++ b/crates/engine/src/api/client.rs @@ -544,11 +544,28 @@ impl ApiClient { headers.push(((*name).to_owned(), (*value).to_owned())); } // Scope the borrow so it never crosses the await below. - if let Some(token) = self.state.borrow().access_token.as_ref() { - headers.push( - bearer_header(token.as_str()) - .map_err(|_| ApiError::Decode("unusable access token".into()))?, - ); + let bearer = { + let mut state = self.state.borrow_mut(); + let built = state + .access_token + .as_ref() + .map(|token| bearer_header(token.as_str())); + match built { + Some(Ok(header)) => Some(header), + // Drop a token that can never be a header value instead of + // refusing every later call while still holding it: the next + // call then goes out unauthenticated and its 401 drives one + // refresh. The refresh credential is a separate secret and + // stays, so a malformed response cannot end the session. + Some(Err(_)) => { + state.access_token = None; + return Err(ApiError::Decode("unusable access token".into())); + } + None => None, + } + }; + if let Some(header) = bearer { + headers.push(header); } let request = HttpRequest { method, @@ -1354,6 +1371,25 @@ mod tests { ApiError::Decode("unusable access token".into()) ); assert_eq!(http.requests().len(), 2, "no request carried the token"); + assert!(!client.is_authenticated(), "the unusable token was dropped"); + + // Self-heal: the next call goes out unauthenticated, and its 401 buys + // one rotation off the still-held refresh credential. + http.enqueue_response(json_response(401, json!({ "message": "no bearer" }))); + http.enqueue_response(json_response( + 200, + json!({ "accessToken": "jwt-2", "refreshToken": "b".repeat(64) }), + )); + http.enqueue_response(json_response( + 200, + json!({ "usedBytes": 1, "limitBytes": 2, "advisory": false }), + )); + + block_on(client.quota()).expect("the session recovered"); + let requests = http.requests(); + assert!(!has_bearer(&requests[2]), "the dropped token was not sent"); + assert_eq!(requests[3].url, "http://api.test/auth/refresh"); + assert!(has_bearer(&requests[4]), "the retry carried the new token"); } #[test] diff --git a/packages/client/src/seams/http.test.ts b/packages/client/src/seams/http.test.ts index 0a0afe559..3a58ee710 100644 --- a/packages/client/src/seams/http.test.ts +++ b/packages/client/src/seams/http.test.ts @@ -95,6 +95,22 @@ describe('FetchHttp credential scoping', () => { }); }); +/** + * A server answering 3xx, modelled as the Fetch spec resolves one: under + * `redirect: 'error'` the redirect is a network error, otherwise it is handed + * back as a response. + */ +function redirectingFetch(): void { + vi.stubGlobal( + 'fetch', + vi.fn((_url: string, init: RequestInit) => + init.redirect === 'error' + ? Promise.reject(new TypeError('Failed to fetch')) + : Promise.resolve(new Response(null, { status: 302, headers: { location: '/elsewhere' } })) + ) + ); +} + describe('FetchHttp redirects', () => { it('refuses them on both paths, as the record transport does', async () => { const fetches = recordingFetch(); @@ -105,6 +121,14 @@ describe('FetchHttp redirects', () => { expect(fetches.inits.map((init) => init.redirect)).toEqual(['error', 'error']); }); + + it('rejects a redirecting server rather than following or returning it', async () => { + redirectingFetch(); + const http = new FetchHttp(); + + await expect(http.send(GET)).rejects.toThrow(TypeError); + await expect(http.sendCapped(GET, 1000)).rejects.toThrow(TypeError); + }); }); describe('FetchHttp deadlines', () => {