From 93d66d69e683ffc10efd14f7da6b8b58898a0741 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Mon, 14 Sep 2026 16:19:44 +0200 Subject: [PATCH] feat: keep a stored provider credential across an unrelated settings save The vault settings record round-tripped every field but the provider bearer. The wasm boundary is write-only for that credential, so a read reports only that one is stored. A save publishes the record whole, which made a blank credential field mean no bearer and turned every unrelated settings change into a credential loss. The bearer is now three-state at every layer. ByoBearer::Keep is a save intent alone: it names no bytes, validate_byo_config refuses it on the encode path and on every request path, and Engine::save_vault_settings resolves it from the provider config the session placement already holds. A kept bearer is bound to the provider it was stored for. The save must name the same endpoint and the same kind, or it is refused. The keep intent crosses an untrusted realm boundary, so without that binding a host could carry a credential it cannot read on to an endpoint it chose and have the next placement present the member's bearer there. The wasm constructor refuses a keep intent that also carries bytes, and zeroizes those bytes before it refuses. The web settings form sends the keep intent when the credential field is untouched and the provider is unchanged, which replaces the refusal that asked the member to retype the credential for every save. --- .../settings/VaultSettingsForm.test.tsx | 26 +++- .../components/settings/VaultSettingsForm.tsx | 23 ++- apps/web/src/settings/vaultSettings.test.ts | 26 +++- apps/web/src/settings/vaultSettings.ts | 15 +- crates/engine/src/content/mod.rs | 2 +- crates/engine/src/content/provider.rs | 87 +++++++++-- crates/engine/src/facade.rs | 26 +++- crates/engine/src/lib.rs | 14 +- crates/engine/src/settings.rs | 138 ++++++++++++++++- crates/engine/src/sync/drain.rs | 7 +- crates/engine/src/sync/upload_mark.rs | 4 +- crates/engine/tests/vault_settings.rs | 139 +++++++++++++++++- crates/engine/tests/write_plane.rs | 6 +- crates/wasm/src/lib.rs | 26 +++- crates/wasm/tests/boundary.rs | 22 +++ .../client/src/broadcastTransport.test.ts | 5 +- packages/client/src/facade.ts | 4 +- packages/client/src/index.ts | 1 + packages/client/src/settings/quota.test.ts | 62 ++++++-- packages/client/src/settings/quota.ts | 43 ++++-- .../client/src/worker/commandCodec.test.ts | 26 +++- packages/client/src/worker/commandCodec.ts | 38 +++-- packages/client/src/worker/engineWasm.ts | 3 +- packages/client/src/worker/protocol.ts | 16 +- 24 files changed, 637 insertions(+), 122 deletions(-) diff --git a/apps/web/src/components/settings/VaultSettingsForm.test.tsx b/apps/web/src/components/settings/VaultSettingsForm.test.tsx index 4de9587cc4..3036f6b1df 100644 --- a/apps/web/src/components/settings/VaultSettingsForm.test.tsx +++ b/apps/web/src/components/settings/VaultSettingsForm.test.tsx @@ -1,4 +1,5 @@ import { act, fireEvent, render, screen } from '@testing-library/react'; +import { KEEP_STORED_BEARER } from '@cipherbox/client'; import type { VaultSettingsSummaryDescriptor } from '@cipherbox/client'; import { describe, expect, it } from 'vitest'; import { VaultSettingsForm } from './VaultSettingsForm'; @@ -123,8 +124,8 @@ describe('the vault settings form', () => { // The fake takes the descriptor in-process rather than transferring it, so // the buffer is still this realm's to scrub — as a refused dispatch leaves it. - const carried = taking.saves[0].byo?.accessToken; - expect(new Uint8Array(carried!)).toEqual(new Uint8Array('opaque'.length)); + const carried = taking.saves[0].byo?.accessToken as ArrayBuffer; + expect(new Uint8Array(carried)).toEqual(new Uint8Array('opaque'.length)); }); it('scrubs the bearer the engine refused rather than leaving it in memory', async () => { @@ -134,8 +135,8 @@ describe('the vault settings form', () => { type('provider access token', 'opaque'); await save(); - const carried = taking.saves[0].byo?.accessToken; - expect(new Uint8Array(carried!)).toEqual(new Uint8Array('opaque'.length)); + const carried = taking.saves[0].byo?.accessToken as ArrayBuffer; + expect(new Uint8Array(carried)).toEqual(new Uint8Array('opaque'.length)); }); it('sends nothing until the member takes on replacing the whole record', () => { @@ -225,14 +226,27 @@ describe('the vault settings form', () => { }); describe('a save over a credential the form cannot show', () => { - it('refuses to blank a stored credential as a side effect of an unrelated edit', async () => { + it('keeps a stored credential through an unrelated edit', async () => { const taking = renderForm(engineTaking(), WITH_CREDENTIAL); type('keep newest versions', '5'); await save(); + expect(taking.saves).toHaveLength(1); + expect(taking.saves[0].byo?.accessToken).toBe(KEEP_STORED_BEARER); + expect(taking.saves[0].keepLatestVersions).toBe(5); + }); + + // The stored bearer belongs to the provider it was stored for. A form that + // kept it on to another endpoint would hand the credential to that endpoint. + it('refuses to keep a stored credential on to a repointed provider', async () => { + const taking = renderForm(engineTaking(), WITH_CREDENTIAL); + + type('your ipfs provider', 'https://elsewhere.example'); + await save(); + expect(taking.saves).toEqual([]); - expect(screen.getByTestId('settings-error').textContent).toMatch(/credential/i); + expect(screen.getByTestId('settings-error').textContent).toMatch(/different provider/i); }); it('clears the stored credential where the member asks for exactly that', async () => { diff --git a/apps/web/src/components/settings/VaultSettingsForm.tsx b/apps/web/src/components/settings/VaultSettingsForm.tsx index dcd35bfda3..af66e3ab9a 100644 --- a/apps/web/src/components/settings/VaultSettingsForm.tsx +++ b/apps/web/src/components/settings/VaultSettingsForm.tsx @@ -1,5 +1,10 @@ import { useEffect, useState } from 'react'; -import { originNotice, prefillFromSummary, settingsSaveVerdict } from '@cipherbox/client'; +import { + KEEP_STORED_BEARER, + originNotice, + prefillFromSummary, + settingsSaveVerdict, +} from '@cipherbox/client'; import type { ByoKind, PinMode, VaultSettingsSummaryDescriptor } from '@cipherbox/client'; import { useCommandRunner } from '../../hooks/useCommandRunner'; import { @@ -39,7 +44,7 @@ const BYO_KINDS: { value: ByoKind; label: string }[] = [ * credential is not: it is the one field the wasm boundary keeps write-only, so * a stored bearer never crosses into JS (`crates/wasm/src/lib.rs`). A save * replaces the whole record with what is on the form, so `settingsSaveVerdict` - * refuses the two shapes that destroy a choice the member did not edit. + * decides how the one field the form cannot show is spelled. */ export function VaultSettingsForm({ summary, onSaved }: VaultSettingsFormProps) { const [fields, setFields] = useState(DEFAULT_VAULT_SETTINGS_FORM); @@ -85,7 +90,10 @@ export function VaultSettingsForm({ summary, onSaved }: VaultSettingsFormProps) origin, credentialStored, byoEndpoint: fields.byoEndpoint, + byoKind: fields.byoKind, byoAccessToken: fields.byoAccessToken, + storedEndpoint: summary.byoEndpoint, + storedKind: summary.byoKind, clearCredential, loadAcknowledged, }); @@ -93,7 +101,7 @@ export function VaultSettingsForm({ summary, onSaved }: VaultSettingsFormProps) setProblem(verdict.problem); return; } - const draft = buildVaultSettings(fields); + const draft = buildVaultSettings(fields, verdict.keepStoredCredential); setProblem(draft.ok ? null : draft.problem); if (!draft.ok) return; void run('saveVaultSettings', (facade) => facade.saveVaultSettings(draft.settings)).then( @@ -101,7 +109,9 @@ export function VaultSettingsForm({ summary, onSaved }: VaultSettingsFormProps) // The form is the bearer's terminal owner: a send transfers the buffer // out and detaches it, so a still-readable one never left this realm. const bearer = draft.settings.byo?.accessToken; - if (bearer && bearer.byteLength > 0) new Uint8Array(bearer).fill(0); + if (bearer != null && bearer !== KEEP_STORED_BEARER && bearer.byteLength > 0) { + new Uint8Array(bearer).fill(0); + } setSaved(accepted); // The bearer is spent by the send that carried it; a retry types it // again rather than re-sending a buffer this realm no longer owns. @@ -228,7 +238,7 @@ export function VaultSettingsForm({ summary, onSaved }: VaultSettingsFormProps) : '// the engine never reads a provider credential back out,'}
{credentialStored - ? '// so keeping the provider means typing it again — or clearing it outright.' + ? '// so leave this blank to keep it. it is kept only for the provider above.' : '// so this field is the only place one can be set.'}

)} @@ -270,8 +280,7 @@ export function VaultSettingsForm({ summary, onSaved }: VaultSettingsFormProps) onChange={(event) => setAcknowledged(event.target.checked)} /> - i understand saving replaces every stored setting with exactly what is on this form, - including the provider credential this form cannot show me + i understand saving replaces every stored setting with exactly what is on this form diff --git a/apps/web/src/settings/vaultSettings.test.ts b/apps/web/src/settings/vaultSettings.test.ts index d2030f3993..51515e61ac 100644 --- a/apps/web/src/settings/vaultSettings.test.ts +++ b/apps/web/src/settings/vaultSettings.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; +import { KEEP_STORED_BEARER } from '@cipherbox/client'; import { buildVaultSettings, DEFAULT_VAULT_SETTINGS_FORM, @@ -42,9 +43,28 @@ describe('the vault settings a save publishes', () => { buildVaultSettings(form({ byoEndpoint: 'https://kubo.example', byoAccessToken: 'opaque' })) ); - const carried = built.byo?.accessToken; - expect(carried?.byteLength).toBe('opaque'.length); - expect(new TextDecoder().decode(new Uint8Array(carried!))).toBe('opaque'); + const carried = built.byo?.accessToken as ArrayBuffer; + expect(carried.byteLength).toBe('opaque'.length); + expect(new TextDecoder().decode(new Uint8Array(carried))).toBe('opaque'); + }); + + it('spells a kept credential as the keep intent, never as bytes', () => { + const built = settings(buildVaultSettings(form({ byoEndpoint: 'https://kubo.example' }), true)); + + expect(built.byo?.accessToken).toBe(KEEP_STORED_BEARER); + }); + + // The keep intent must not smuggle a typed bearer past the engine's + // same-provider binding: the form sends one or the other, never both. + it('drops a typed bearer when the save keeps the stored one', () => { + const built = settings( + buildVaultSettings( + form({ byoEndpoint: 'https://kubo.example', byoAccessToken: 'opaque' }), + true + ) + ); + + expect(built.byo?.accessToken).toBe(KEEP_STORED_BEARER); }); it('mints a fresh bearer buffer per build, because the send detaches it', () => { diff --git a/apps/web/src/settings/vaultSettings.ts b/apps/web/src/settings/vaultSettings.ts index e61a4050ee..f4b3143c0d 100644 --- a/apps/web/src/settings/vaultSettings.ts +++ b/apps/web/src/settings/vaultSettings.ts @@ -8,6 +8,7 @@ * what an empty one means. */ +import { KEEP_STORED_BEARER } from '@cipherbox/client'; import type { ByoKind, PinMode, VaultSettingsDescriptor } from '@cipherbox/client'; /** @@ -49,8 +50,14 @@ export type VaultSettingsDraft = * Builds the descriptor for one save. Called per dispatch, never cached: the * bearer rides a transferable buffer that `saveVaultSettings` detaches, so a * descriptor sent twice would carry a spent credential the second time. + * + * `keepStoredCredential` is `settingsSaveVerdict`'s answer: it is the only way + * a form that can never read a stored bearer publishes without destroying it. */ -export function buildVaultSettings(form: VaultSettingsFields): VaultSettingsDraft { +export function buildVaultSettings( + form: VaultSettingsFields, + keepStoredCredential = false +): VaultSettingsDraft { const keep = form.keepLatestVersions.trim(); if (keep !== '' && !isCount(keep)) { return { @@ -73,7 +80,11 @@ export function buildVaultSettings(form: VaultSettingsFields): VaultSettingsDraf byo: endpoint === '' ? null - : { endpoint, kind: form.byoKind, accessToken: bearer(form.byoAccessToken) }, + : { + endpoint, + kind: form.byoKind, + accessToken: keepStoredCredential ? KEEP_STORED_BEARER : bearer(form.byoAccessToken), + }, keepLatestVersions: keep === '' ? null : Number(keep), binRetentionDays: Number(binDays), }, diff --git a/crates/engine/src/content/mod.rs b/crates/engine/src/content/mod.rs index ae7675a303..af31e353c6 100644 --- a/crates/engine/src/content/mod.rs +++ b/crates/engine/src/content/mod.rs @@ -28,7 +28,7 @@ pub use dag::{ pub use profile::ContentProfile; pub(crate) use provider::place_block; pub use provider::{ - ByoIpfsConfig, ByoKind, PinMode, ProviderError, test_connection, validate_byo_config, + ByoBearer, ByoIpfsConfig, ByoKind, PinMode, ProviderError, test_connection, validate_byo_config, }; pub use read::{ ContentPlane, Gateway, GatewayConfig, GatewayOnly, GatewaySource, LocalBlocks, ReadError, diff --git a/crates/engine/src/content/provider.rs b/crates/engine/src/content/provider.rs index ae975d7d1c..cb47adb8cb 100644 --- a/crates/engine/src/content/provider.rs +++ b/crates/engine/src/content/provider.rs @@ -66,6 +66,43 @@ pub enum ByoKind { Pinata, } +/// What a provider config says about its bearer credential. Three-state so a +/// host that can never read a stored bearer back can still say "leave it +/// alone"; two states make every unrelated settings save destroy it. +/// +/// [`Self::Keep`] is a save intent and names no bytes, so [`validate_byo_config`] +/// refuses it on the encode path and on every request path. +#[derive(Clone, PartialEq, Eq)] +pub enum ByoBearer { + /// The provider needs no credential, or the member cleared the stored one. + None, + /// This bearer. Zeroized on drop, never logged. + Set(Zeroizing), + /// Keep the bearer the session already holds. + Keep, +} + +impl ByoBearer { + /// The bearer this config names, or `None` for a config that names none. + #[must_use] + pub fn token(&self) -> Option<&Zeroizing> { + match self { + Self::Set(token) => Some(token), + Self::None | Self::Keep => None, + } + } +} + +impl fmt::Debug for ByoBearer { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match self { + Self::None => "None", + Self::Set(_) => "Set()", + Self::Keep => "Keep", + }) + } +} + /// A member's bring-your-own IPFS provider config. Stays sealed in vault /// settings (blueprint/engine.md); this is the plaintext the seal wraps. The /// access token is a credential: held in a zeroizing buffer and redacted from @@ -77,8 +114,8 @@ pub struct ByoIpfsConfig { /// The provider kind, selecting the reachability probe. pub kind: ByoKind, /// Bearer credential, when the provider requires one (PSA/Pinata always; - /// Kubo when fronted by an auth proxy). Zeroized on drop, never logged. - pub access_token: Option>, + /// Kubo when fronted by an auth proxy). + pub access_token: ByoBearer, } impl fmt::Debug for ByoIpfsConfig { @@ -86,10 +123,7 @@ impl fmt::Debug for ByoIpfsConfig { f.debug_struct("ByoIpfsConfig") .field("endpoint", &self.endpoint) .field("kind", &self.kind) - .field( - "access_token", - &self.access_token.as_ref().map(|_| ""), - ) + .field("access_token", &self.access_token) .finish() } } @@ -276,7 +310,7 @@ fn base(config: &ByoIpfsConfig) -> &str { /// body. The configured access token is the only credential a BYO endpoint gets. fn headers(config: &ByoIpfsConfig, content_type: Option) -> Vec<(String, String)> { let mut headers = Vec::new(); - if let Some(token) = &config.access_token { + if let Some(token) = config.access_token.token() { headers.push(( AUTHORIZATION.to_owned(), format!("Bearer {}", token.as_str()), @@ -308,6 +342,18 @@ pub enum ProviderError { BlockedAddress, /// The access token carries bytes a header value may not. InvalidCredential, + /// The config still carries [`ByoBearer::Keep`]. Only a save resolves that + /// intent, so anything else holding it would send or seal a provider with + /// no credential at all (AGENTS.md rule 8). + UnresolvedCredential, + /// A save asked to keep the stored bearer and this session holds none, so + /// keeping would publish a credential-free provider rather than the one the + /// member has. + NoStoredCredential, + /// A save asked to keep the stored bearer while naming a different endpoint + /// or kind. A bearer is kept for the provider it was stored for, never + /// carried on to another one. + RepointedCredential, /// The provider could not be reached (transport-level failure). Unreachable, /// The provider answered, but with nothing that says what it did. @@ -335,6 +381,9 @@ impl ProviderError { ProviderError::InsecureTransport => "byo-endpoint-insecure", ProviderError::BlockedAddress => "byo-endpoint-blocked", ProviderError::InvalidCredential => "byo-credential-invalid", + ProviderError::UnresolvedCredential => "byo-credential-unresolved", + ProviderError::NoStoredCredential => "byo-credential-not-stored", + ProviderError::RepointedCredential => "byo-credential-repointed", ProviderError::Unreachable => "byo-unreachable", ProviderError::NoVerdict => "byo-no-verdict", ProviderError::Rejected { .. } => "byo-rejected", @@ -354,6 +403,9 @@ impl ProviderError { | ProviderError::InsecureTransport | ProviderError::BlockedAddress | ProviderError::InvalidCredential + | ProviderError::UnresolvedCredential + | ProviderError::NoStoredCredential + | ProviderError::RepointedCredential ) } } @@ -390,8 +442,11 @@ pub fn validate_byo_config(config: &ByoIpfsConfig) -> Result<(), ProviderError> match &config.access_token { // `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(()), + ByoBearer::Set(token) => { + check_bearer(token.as_str()).map_err(|_| ProviderError::InvalidCredential) + } + ByoBearer::None => Ok(()), + ByoBearer::Keep => Err(ProviderError::UnresolvedCredential), } } @@ -533,11 +588,17 @@ mod tests { use crate::testkit::block_on; use crate::testkit::fakes::ScriptedHttp; + fn bearer(token: Option<&str>) -> ByoBearer { + token.map_or(ByoBearer::None, |t| { + ByoBearer::Set(Zeroizing::new(t.to_owned())) + }) + } + fn config(kind: ByoKind, token: Option<&str>) -> ByoIpfsConfig { ByoIpfsConfig { endpoint: "https://ipfs.member.test/".into(), kind, - access_token: token.map(|t| Zeroizing::new(t.to_owned())), + access_token: bearer(token), } } @@ -679,7 +740,7 @@ mod tests { let cfg = ByoIpfsConfig { endpoint: bad.into(), kind: ByoKind::Psa, - access_token: None, + access_token: ByoBearer::None, }; assert_eq!( block_on(test_connection(&cfg, &http, &DeadlinePolicy::default())).unwrap_err(), @@ -710,7 +771,7 @@ mod tests { let cfg = ByoIpfsConfig { endpoint: endpoint.to_owned(), kind: ByoKind::Kubo, - access_token: None, + access_token: ByoBearer::None, }; block_on(test_connection(&cfg, &http, &DeadlinePolicy::default())).unwrap(); assert_eq!(http.requests()[0].url, format!("{endpoint}/api/v0/id")); @@ -1002,7 +1063,7 @@ mod tests { let bad = ByoIpfsConfig { endpoint: "http://169.254.169.254".into(), kind: ByoKind::Kubo, - access_token: None, + access_token: ByoBearer::None, }; assert_eq!( block_on(place_block( diff --git a/crates/engine/src/facade.rs b/crates/engine/src/facade.rs index 1dc420d399..2104668df9 100644 --- a/crates/engine/src/facade.rs +++ b/crates/engine/src/facade.rs @@ -48,9 +48,9 @@ use crate::content::budget::{Refusal, ReservationId}; use crate::content::limits::folder_listing_budget; use crate::content::read::authority_of; use crate::content::{ - ContentKey, ContentProfile, ContentWriter, Gateway, GatewayConfig, OpenError, PinMode, Refused, - RootManifest, SealError, SessionBearer, StagingLedger, open_content_range, open_content_root, - pre_flight_quota_check, read_pinned_range, sealed_total_bytes, + ByoIpfsConfig, ContentKey, ContentProfile, ContentWriter, Gateway, GatewayConfig, OpenError, + PinMode, Refused, RootManifest, SealError, SessionBearer, StagingLedger, open_content_range, + open_content_root, pre_flight_quota_check, read_pinned_range, sealed_total_bytes, }; use crate::deadlines::DeadlinePolicy; use crate::devices::{self, ApprovalDecision, MalformedDeviceField, PendingApprovalView}; @@ -114,10 +114,10 @@ use crate::seams::{ }; use crate::session::SessionIdentity; use crate::settings::{ - DEFAULT_BIN_RETENTION_DAYS, PlacementRefusal, PlacementSource, SessionPlacement, + DEFAULT_BIN_RETENTION_DAYS, Placement, PlacementRefusal, PlacementSource, SessionPlacement, SettingsOrigin, SettingsPublishError, VaultSettings, VaultSettingsSummary, decide_placement, load_settings, load_settings_at, placement_of, publish_settings, redecide_placement, - summarize_settings, + resolve_kept_bearer, summarize_settings, }; use crate::storage_policy::StoragePolicy; use crate::sync::boot::{ColdStartError, ColdStartOutcome, ColdStartParams, cold_start}; @@ -8678,12 +8678,24 @@ where { ) } + /// The provider config this session holds, which is the one its placement + /// authorises: a session placing no bytes on the member's own provider + /// keeps no credential for it (security rule 7). + fn held_provider(&self) -> Option { + match &self.placement.borrow().as_ref()?.decision { + Ok(Placement::External(config) | Placement::Dual(config)) => Some(config.clone()), + Ok(Placement::Hosted) | Err(_) => None, + } + } + /// Seal and publish the vault settings record, then adopt what it /// published: the renewal enrolment [`publish_settings`] states the need /// for, and the placement this session writes under. async fn save_vault_settings(&self, settings: &VaultSettings) -> Result<(), EngineError> { let session = self.session.as_ref().ok_or(EngineError::NotStarted)?; let api = self.api.as_ref().ok_or(EngineError::NotStarted)?; + let settings = &resolve_kept_bearer(settings, self.held_provider().as_ref()) + .map_err(|e| EngineError::from_settings_publish(SettingsPublishError::Byo(e)))?; let held = publish_settings( &self.record_transport, api, @@ -11475,7 +11487,7 @@ mod tests { use core::num::NonZeroU64; use crate::api::{ChallengeSigner, new_user_login_response}; - use crate::content::{ByoIpfsConfig, ByoKind, RetentionPolicy}; + use crate::content::{ByoBearer, ByoIpfsConfig, ByoKind, RetentionPolicy}; use crate::net::retire::ReclaimStallReason; use crate::seams::{CredentialStore, EndpointId, HttpMethod, HttpResponse, UnixMillis}; use crate::settings::{cached_settings_block, settings_name}; @@ -12576,7 +12588,7 @@ mod tests { byo: Some(ByoIpfsConfig { endpoint: "https://node.example".to_owned(), kind: ByoKind::Kubo, - access_token: Some(Zeroizing::new(BEARER.to_owned())), + access_token: ByoBearer::Set(Zeroizing::new(BEARER.to_owned())), }), retention: RetentionPolicy::KeepLatest(NonZeroU64::new(3).expect("nonzero")), bin_retention_days: DEFAULT_BIN_RETENTION_DAYS, diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 7db1757c38..0eebcd36b4 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -57,13 +57,13 @@ pub use bin_index::{ publish_bin_index, }; pub use content::{ - ByoIpfsConfig, ByoKind, ContentDag, ContentKey, ContentPlane, ContentProfile, ContentVersion, - ContentWriter, DAG_ROOT_CODEC, DagError, ExpandError, Expansion, FinishedContent, Gateway, - GatewayConfig, GatewaySource, PinMode, ProviderError, PrunePlan, QuotaExceeded, - ROOT_FORMAT_VERSION, ReadError, RetentionPolicy, RetireTarget, RootManifest, SealError, - SealedChunk, SealedContent, SessionBearer, assemble, decode_root, expand_retire_targets, - frame_and_seal, leaf_range_for_byte_range, plan_prune, pre_flight_quota_check, read_block, - seal_one_chunk, test_connection, validate_byo_config, + ByoBearer, ByoIpfsConfig, ByoKind, ContentDag, ContentKey, ContentPlane, ContentProfile, + ContentVersion, ContentWriter, DAG_ROOT_CODEC, DagError, ExpandError, Expansion, + FinishedContent, Gateway, GatewayConfig, GatewaySource, PinMode, ProviderError, PrunePlan, + QuotaExceeded, ROOT_FORMAT_VERSION, ReadError, RetentionPolicy, RetireTarget, RootManifest, + SealError, SealedChunk, SealedContent, SessionBearer, assemble, decode_root, + expand_retire_targets, frame_and_seal, leaf_range_for_byte_range, plan_prune, + pre_flight_quota_check, read_block, seal_one_chunk, test_connection, validate_byo_config, }; pub use deadlines::DeadlinePolicy; pub use devices::{ diff --git a/crates/engine/src/settings.rs b/crates/engine/src/settings.rs index 7d789930c1..ccadf44acc 100644 --- a/crates/engine/src/settings.rs +++ b/crates/engine/src/settings.rs @@ -34,7 +34,9 @@ use zeroize::Zeroizing; use crate::api::ApiClient; use crate::content::validate_byo_config; -use crate::content::{ByoIpfsConfig, ByoKind, Gateway, PinMode, ProviderError, RetentionPolicy}; +use crate::content::{ + ByoBearer, ByoIpfsConfig, ByoKind, Gateway, PinMode, ProviderError, RetentionPolicy, +}; use crate::entropy::{Entropy, EntropyError, fresh_ephemeral}; use crate::gate::floor; use crate::gate::floor::RevisionMintError; @@ -104,7 +106,7 @@ impl VaultSettings { byo_credential_stored: self .byo .as_ref() - .is_some_and(|byo| byo.access_token.is_some()), + .is_some_and(|byo| byo.access_token.token().is_some()), retention: self.retention, bin_retention_days: self.bin_retention_days, origin, @@ -112,6 +114,36 @@ impl VaultSettings { } } +/// Settle a save's bearer intent against `held`, the provider config this +/// session already holds, so [`ByoBearer::Keep`] publishes the stored bearer +/// rather than the blank the host can never fill. +/// +/// The keep is bound to the same endpoint and kind, and that binding is the +/// whole security of the intent: without it a host could carry a credential it +/// cannot read on to an endpoint it chose, and the next placement would present +/// the member's bearer there (security rule 3). Keeping with nothing to keep is +/// refused rather than publishing a credential-free provider. +pub fn resolve_kept_bearer( + settings: &VaultSettings, + held: Option<&ByoIpfsConfig>, +) -> Result { + let mut settings = settings.clone(); + if let Some(byo) = settings.byo.as_mut() + && byo.access_token == ByoBearer::Keep + { + let held = held.ok_or(ProviderError::NoStoredCredential)?; + if held.endpoint != byo.endpoint || held.kind != byo.kind { + return Err(ProviderError::RepointedCredential); + } + let kept = held + .access_token + .token() + .ok_or(ProviderError::NoStoredCredential)?; + byo.access_token = ByoBearer::Set(kept.clone()); + } + Ok(settings) +} + /// The member's settings as a host may see them: everything but the provider /// credential, which the wasm boundary exists to keep uncrossable. #[derive(Debug, Clone, PartialEq, Eq)] @@ -943,7 +975,7 @@ fn encode_settings_body( "accessToken", config .access_token - .as_ref() + .token() .map_or(Value::Null, |token| Value::Text(token.to_string())), ); byo.insert("endpoint", Value::Text(config.endpoint.clone())); @@ -1057,8 +1089,8 @@ fn read_byo(value: Option<&mut Value>) -> Result, BodyErro } .into()); } - Some(Value::Null) => None, - Some(Value::Text(token)) => Some(Zeroizing::new(token)), + Some(Value::Null) => ByoBearer::None, + Some(Value::Text(token)) => ByoBearer::Set(Zeroizing::new(token)), Some(other) => return Err(other.as_text().unwrap_err().into()), }; Ok(Some(ByoIpfsConfig { @@ -1112,11 +1144,17 @@ mod tests { RetentionPolicy::KeepLatest(NonZeroU64::new(n).expect("nonzero")) } + fn bearer(token: Option<&str>) -> ByoBearer { + token.map_or(ByoBearer::None, |t| { + ByoBearer::Set(Zeroizing::new(t.to_owned())) + }) + } + fn byo(endpoint: &str, kind: ByoKind, token: Option<&str>) -> ByoIpfsConfig { ByoIpfsConfig { endpoint: endpoint.to_owned(), kind, - access_token: token.map(|t| Zeroizing::new(t.to_owned())), + access_token: bearer(token), } } @@ -1131,6 +1169,94 @@ mod tests { body.settings } + fn keeping(endpoint: &str, kind: ByoKind) -> VaultSettings { + VaultSettings { + pin_mode: PinMode::Dual, + byo: Some(ByoIpfsConfig { + endpoint: endpoint.to_owned(), + kind, + access_token: ByoBearer::Keep, + }), + ..VaultSettings::default() + } + } + + /// The decode path has no spelling for a keep intent, so the encode path + /// must refuse one — release-active, never an assertion (AGENTS.md rule 8). + #[test] + fn the_seal_path_refuses_an_unresolved_keep_intent() { + assert_eq!( + validate(&keeping("https://node.example", ByoKind::Kubo)), + Err(ProviderError::UnresolvedCredential), + ); + } + + #[test] + fn a_kept_bearer_takes_the_one_the_session_holds() { + let held = byo("https://node.example", ByoKind::Kubo, Some("tok")); + let resolved = + resolve_kept_bearer(&keeping("https://node.example", ByoKind::Kubo), Some(&held)) + .expect("the keep resolves"); + + assert_eq!( + resolved.byo.expect("a provider").access_token, + held.access_token + ); + } + + #[test] + fn a_kept_bearer_does_not_travel_to_another_provider() { + let held = byo("https://node.example", ByoKind::Kubo, Some("tok")); + + assert_eq!( + resolve_kept_bearer( + &keeping("https://elsewhere.example", ByoKind::Kubo), + Some(&held) + ), + Err(ProviderError::RepointedCredential), + ); + assert_eq!( + resolve_kept_bearer( + &keeping("https://node.example", ByoKind::Pinata), + Some(&held) + ), + Err(ProviderError::RepointedCredential), + ); + } + + #[test] + fn keeping_a_bearer_the_session_does_not_hold_is_refused() { + let tokenless = byo("https://node.example", ByoKind::Kubo, None); + + assert_eq!( + resolve_kept_bearer(&keeping("https://node.example", ByoKind::Kubo), None), + Err(ProviderError::NoStoredCredential), + ); + assert_eq!( + resolve_kept_bearer( + &keeping("https://node.example", ByoKind::Kubo), + Some(&tokenless) + ), + Err(ProviderError::NoStoredCredential), + ); + } + + /// A save that names its own bearer is untouched by the resolve, whatever + /// the session holds. + #[test] + fn a_supplied_bearer_is_left_alone() { + let held = byo("https://node.example", ByoKind::Kubo, Some("stored")); + let supplied = VaultSettings { + byo: Some(byo("https://node.example", ByoKind::Kubo, Some("typed"))), + ..VaultSettings::default() + }; + + assert_eq!( + resolve_kept_bearer(&supplied, Some(&held)).expect("nothing to resolve"), + supplied, + ); + } + #[test] fn every_tenant_of_the_record_round_trips() { for pin_mode in [PinMode::Hosted, PinMode::External, PinMode::Dual] { diff --git a/crates/engine/src/sync/drain.rs b/crates/engine/src/sync/drain.rs index 30e129c514..4c8ad00e54 100644 --- a/crates/engine/src/sync/drain.rs +++ b/crates/engine/src/sync/drain.rs @@ -6233,7 +6233,10 @@ fn provider_failure(error: &ProviderError) -> &'static str { ProviderError::InvalidEndpoint | ProviderError::InsecureTransport | ProviderError::BlockedAddress - | ProviderError::InvalidCredential => "your own IPFS provider settings were refused", + | ProviderError::InvalidCredential + | ProviderError::UnresolvedCredential + | ProviderError::NoStoredCredential + | ProviderError::RepointedCredential => "your own IPFS provider settings were refused", ProviderError::MalformedBlockAddress => { "the block's address is not one any provider can be told to store" } @@ -6966,7 +6969,7 @@ mod tests { crate::content::ByoIpfsConfig { endpoint: endpoint.to_owned(), kind: crate::content::ByoKind::Kubo, - access_token: None, + access_token: crate::content::ByoBearer::None, } } diff --git a/crates/engine/src/sync/upload_mark.rs b/crates/engine/src/sync/upload_mark.rs index 53b62ee01b..7223422747 100644 --- a/crates/engine/src/sync/upload_mark.rs +++ b/crates/engine/src/sync/upload_mark.rs @@ -100,7 +100,7 @@ pub(crate) fn resume_from(stored: &[u8], here: &Destinations, leaves: usize) -> #[cfg(test)] mod tests { use super::*; - use crate::content::ByoIpfsConfig; + use crate::content::{ByoBearer, ByoIpfsConfig}; use crate::settings::Placement; const ROOT: &[u8] = b"root-cid"; @@ -109,7 +109,7 @@ mod tests { ByoIpfsConfig { endpoint: endpoint.to_owned(), kind: crate::content::ByoKind::Kubo, - access_token: None, + access_token: ByoBearer::None, } } diff --git a/crates/engine/tests/vault_settings.rs b/crates/engine/tests/vault_settings.rs index 1dc9872442..3f95691abb 100644 --- a/crates/engine/tests/vault_settings.rs +++ b/crates/engine/tests/vault_settings.rs @@ -19,7 +19,7 @@ use cipherbox_core::seal::{open_settings_record, seal_settings_record}; use zeroize::Zeroizing; use cipherbox_engine::api::ApiClient; -use cipherbox_engine::content::{ByoIpfsConfig, ByoKind, DAG_ROOT_CODEC, PinMode}; +use cipherbox_engine::content::{ByoBearer, ByoIpfsConfig, ByoKind, DAG_ROOT_CODEC, PinMode}; use cipherbox_engine::net::RE_PUT_INTERVAL; use cipherbox_engine::seams::{ BoxedTask, EndpointId, FloorStore, RecordTransport, Scheduler, SnapshotCache, UnixMillis, @@ -61,7 +61,7 @@ fn configured() -> VaultSettings { byo: Some(ByoIpfsConfig { endpoint: "https://kubo.example".to_owned(), kind: ByoKind::Kubo, - access_token: Some(Zeroizing::new("s3cret".to_owned())), + access_token: ByoBearer::Set(Zeroizing::new("s3cret".to_owned())), }), retention: RetentionPolicy::KeepLatest(NonZeroU64::new(3).expect("nonzero")), bin_retention_days: DEFAULT_BIN_RETENTION_DAYS, @@ -223,7 +223,7 @@ fn a_second_publish_from_the_same_device_supersedes_the_first() { byo: Some(ByoIpfsConfig { endpoint: "https://kubo.example".to_owned(), kind: ByoKind::Kubo, - access_token: Some(Zeroizing::new("rotated".to_owned())), + access_token: ByoBearer::Set(Zeroizing::new("rotated".to_owned())), }), ..configured() }; @@ -839,7 +839,7 @@ fn external_only() -> VaultSettings { byo: Some(ByoIpfsConfig { endpoint: "https://kubo.example".to_owned(), kind: ByoKind::Kubo, - access_token: None, + access_token: ByoBearer::None, }), retention: RetentionPolicy::KeepAll, bin_retention_days: DEFAULT_BIN_RETENTION_DAYS, @@ -1208,7 +1208,7 @@ fn settings_the_reader_would_refuse_are_never_published() { byo: Some(ByoIpfsConfig { endpoint: endpoint.to_owned(), kind: ByoKind::Kubo, - access_token: None, + access_token: ByoBearer::None, }), ..VaultSettings::default() }; @@ -1325,7 +1325,7 @@ fn hand_encoded_settings(endpoint: &str) -> VaultSettings { byo: Some(ByoIpfsConfig { endpoint: endpoint.to_owned(), kind: ByoKind::Kubo, - access_token: None, + access_token: ByoBearer::None, }), retention: RetentionPolicy::KeepAll, bin_retention_days: DEFAULT_BIN_RETENTION_DAYS, @@ -2010,6 +2010,133 @@ fn saving_vault_settings_through_the_facade_publishes_the_record() { } } +/// The keep intent's whole point: a member who changes an unrelated field does +/// not retype the one field no read can show them, and the record still carries +/// a provider that authenticates. +#[test] +fn a_save_that_keeps_the_bearer_republishes_the_stored_one() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + let device = world.device(b"me"); + let (mut engine, _events, _tasks) = boot(&world, &device, &blocks); + + block_on(engine.command(Command::SaveVaultSettings { + settings: configured(), + })) + .expect("the first save publishes the bearer"); + + let mut kept = configured(); + kept.retention = RetentionPolicy::KeepLatest(NonZeroU64::new(9).expect("nonzero")); + kept.byo.as_mut().expect("a provider").access_token = ByoBearer::Keep; + block_on(engine.command(Command::SaveVaultSettings { settings: kept })) + .expect("the keeping save publishes"); + + let mut expected = configured(); + expected.retention = RetentionPolicy::KeepLatest(NonZeroU64::new(9).expect("nonzero")); + let reader = world.device(b"second-device"); + assert_eq!( + load(&world, &reader, &blocks, &SECRET), + SettingsLoad::Resolved(expected), + "the published record kept the provider bearer and took the new retention", + ); +} + +/// Fail-closed: keeping nothing publishes a credential-free provider, which +/// breaks the authentication the save asked to preserve. +#[test] +fn a_save_that_keeps_a_bearer_this_session_does_not_hold_is_refused() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + let device = world.device(b"me"); + let (mut engine, _events, _tasks) = boot(&world, &device, &blocks); + + let mut kept = configured(); + kept.byo.as_mut().expect("a provider").access_token = ByoBearer::Keep; + + assert_eq!( + block_on(engine.command(Command::SaveVaultSettings { settings: kept })), + Err(EngineError::MalformedInput { + check: "byo-credential-not-stored", + }), + ); + let name = settings_name(&SECRET); + for endpoint in world.record_store.endpoints() { + assert!( + world + .record_store + .record_at(&endpoint, name.as_str()) + .is_none(), + "nothing was published", + ); + } +} + +/// A bearer is kept for the provider it was stored for. Carrying it on to an +/// endpoint the caller chose would present the member's credential to that +/// endpoint, which is the exfiltration the write-only boundary denies. +#[test] +fn a_save_that_keeps_a_bearer_on_to_another_provider_is_refused() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + let device = world.device(b"me"); + let (mut engine, _events, _tasks) = boot(&world, &device, &blocks); + + block_on(engine.command(Command::SaveVaultSettings { + settings: configured(), + })) + .expect("the first save publishes the bearer"); + + let mut repointed = configured(); + let byo = repointed.byo.as_mut().expect("a provider"); + byo.endpoint = "https://elsewhere.example".to_owned(); + byo.access_token = ByoBearer::Keep; + + assert_eq!( + block_on(engine.command(Command::SaveVaultSettings { + settings: repointed, + })), + Err(EngineError::MalformedInput { + check: "byo-credential-repointed", + }), + ); + + let reader = world.device(b"second-device"); + assert_eq!( + load(&world, &reader, &blocks, &SECRET), + SettingsLoad::Resolved(configured()), + "the published record still names the provider the first save set", + ); +} + +/// The same binding on the other axis: a kind change is a different provider +/// API, so the stored bearer does not travel to it either. +#[test] +fn a_save_that_keeps_a_bearer_on_to_another_provider_kind_is_refused() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + let device = world.device(b"me"); + let (mut engine, _events, _tasks) = boot(&world, &device, &blocks); + + block_on(engine.command(Command::SaveVaultSettings { + settings: configured(), + })) + .expect("the first save publishes the bearer"); + + let mut repointed = configured(); + let byo = repointed.byo.as_mut().expect("a provider"); + byo.kind = ByoKind::Pinata; + byo.access_token = ByoBearer::Keep; + + assert_eq!( + block_on(engine.command(Command::SaveVaultSettings { + settings: repointed, + })), + Err(EngineError::MalformedInput { + check: "byo-credential-repointed", + }), + ); +} + #[test] fn a_settings_save_before_start_is_not_started() { let world = FakeWorld::new(); diff --git a/crates/engine/tests/write_plane.rs b/crates/engine/tests/write_plane.rs index ded122b6e4..b2182ffe3b 100644 --- a/crates/engine/tests/write_plane.rs +++ b/crates/engine/tests/write_plane.rs @@ -31,8 +31,8 @@ use zeroize::Zeroizing; use cipherbox_engine::api::RetireEntry; use cipherbox_engine::content::chunk::SEALED_LEAF_OVERHEAD; use cipherbox_engine::content::{ - ByoIpfsConfig, ByoKind, DAG_ROOT_CODEC, PinMode, RetentionPolicy, SealedChunk, SessionBearer, - assemble, decode_root, + ByoBearer, ByoIpfsConfig, ByoKind, DAG_ROOT_CODEC, PinMode, RetentionPolicy, SealedChunk, + SessionBearer, assemble, decode_root, }; use cipherbox_engine::facade::{BinOrigin, PendingClass, SnapshotView}; use cipherbox_engine::net::OrphanHeads; @@ -10874,7 +10874,7 @@ fn member_node(kind: ByoKind) -> ByoIpfsConfig { ByoIpfsConfig { endpoint: MEMBER_NODE.to_owned(), kind, - access_token: Some(Zeroizing::new("member-token".to_owned())), + access_token: ByoBearer::Set(Zeroizing::new("member-token".to_owned())), } } diff --git a/crates/wasm/src/lib.rs b/crates/wasm/src/lib.rs index 1a3f4dea8c..5a3e598ee3 100644 --- a/crates/wasm/src/lib.rs +++ b/crates/wasm/src/lib.rs @@ -28,7 +28,9 @@ #![forbid(unsafe_code)] #![warn(missing_docs)] -use cipherbox_engine::content::{ByoIpfsConfig as EngineByo, ByoKind as EngineByoKind}; +use cipherbox_engine::content::{ + ByoBearer as EngineByoBearer, ByoIpfsConfig as EngineByo, ByoKind as EngineByoKind, +}; use cipherbox_engine::facade; use cipherbox_engine::seams::{UnixMillis, check_bearer}; use cipherbox_engine::settings::{DEFAULT_BIN_RETENTION_DAYS, MAX_BIN_RETENTION_DAYS}; @@ -260,9 +262,9 @@ fn decode_bearer(bytes: Vec) -> Result, JsError> { #[wasm_bindgen] impl ByoIpfsConfig { - /// Builds a provider config. `accessToken` is `undefined` for a provider - /// that needs none; when present it arrives as bytes and lands in a - /// zeroizing buffer. + /// Builds a provider config. The credential is three-state: + /// `keepAccessToken` keeps whatever the session already holds, + /// `accessToken` bytes set a new one, and neither clears it. /// /// Bytes rather than a `String` so the host holds the credential in /// something it can scrub: a JS string cannot be overwritten. @@ -276,8 +278,22 @@ impl ByoIpfsConfig { endpoint: String, kind: ByoKind, access_token: Option>, + keep_access_token: bool, ) -> Result { - let access_token = access_token.map(decode_bearer).transpose()?; + let access_token = match (access_token, keep_access_token) { + // "keep this one" and "keep the stored one" are two different + // credentials. Which one the member meant is not recoverable here, + // so neither is published. + (Some(mut bytes), true) => { + bytes.zeroize(); + return Err(JsError::new( + "accessToken and keepAccessToken are contradictory", + )); + } + (Some(bytes), false) => EngineByoBearer::Set(decode_bearer(bytes)?), + (None, true) => EngineByoBearer::Keep, + (None, false) => EngineByoBearer::None, + }; Ok(Self { inner: EngineByo { endpoint, diff --git a/crates/wasm/tests/boundary.rs b/crates/wasm/tests/boundary.rs index c883eaceef..da4d750fe4 100644 --- a/crates/wasm/tests/boundary.rs +++ b/crates/wasm/tests/boundary.rs @@ -594,6 +594,7 @@ fn a_vault_settings_command_carries_the_stable_builder_name() { "https://kubo.example".to_owned(), ByoKind::Kubo, Some(b"s3cret".to_vec()), + false, ) .expect("UTF-8 token bytes build"), ), @@ -625,12 +626,33 @@ fn a_bearer_the_engine_would_refuse_never_builds_a_config() { "https://kubo.example".to_owned(), ByoKind::Kubo, Some(refused), + false, ) .is_err() ); } } +/// "keep the stored bearer" and "use this bearer" are two different +/// credentials, and which one the caller meant is not recoverable. Neither is +/// published. +#[wasm_bindgen_test] +fn a_keep_intent_carrying_its_own_bearer_builds_no_config() { + assert!( + ByoIpfsConfig::new( + "https://kubo.example".to_owned(), + ByoKind::Kubo, + Some(b"s3cret".to_vec()), + true, + ) + .is_err() + ); + assert!( + ByoIpfsConfig::new("https://kubo.example".to_owned(), ByoKind::Kubo, None, true).is_ok(), + "a keep intent on its own builds", + ); +} + /// The `deadLetterReason` ordinals the TypeScript side decodes against /// (`packages/client/src/testkit.ts`, and the raw numbers its unit tests feed). /// A variant inserted mid-enum renumbers every one after it, and both sides go diff --git a/packages/client/src/broadcastTransport.test.ts b/packages/client/src/broadcastTransport.test.ts index 977874a4bf..0429757456 100644 --- a/packages/client/src/broadcastTransport.test.ts +++ b/packages/client/src/broadcastTransport.test.ts @@ -1433,10 +1433,11 @@ function settingsSaveOf(accessToken: ArrayBuffer): CommandDescriptor { /** The bearer bytes a relayed settings command arrived with. */ function bearerOf(command: CommandDescriptor | undefined): Uint8Array { - if (command?.kind !== 'saveVaultSettings' || !command.settings.byo?.accessToken) { + const bearer = command?.kind === 'saveVaultSettings' ? command.settings.byo?.accessToken : null; + if (!(bearer instanceof ArrayBuffer)) { throw new Error('the relayed command carried no bearer'); } - return new Uint8Array(command.settings.byo.accessToken); + return new Uint8Array(bearer); } describe('leader relay write handles', () => { diff --git a/packages/client/src/facade.ts b/packages/client/src/facade.ts index a1cf505234..c2c900e37e 100644 --- a/packages/client/src/facade.ts +++ b/packages/client/src/facade.ts @@ -13,7 +13,7 @@ import { type AccountStoreNaming, eraseAccountStores } from './accountStores.js'; import { isBuffer, wipeBytes } from './buffers.js'; import type { EngineEventListener, EngineTransport } from './transport.js'; -import { MAX_FRAGMENT_CHARS } from './worker/protocol.js'; +import { KEEP_STORED_BEARER, MAX_FRAGMENT_CHARS } from './worker/protocol.js'; import type { ApprovalDecision, AuthMethodDescriptor, @@ -400,7 +400,7 @@ export class EngineFacade { */ saveVaultSettings(settings: VaultSettingsDescriptor): Promise { const token = settings.byo?.accessToken; - if (token != null && !isBuffer(token)) { + if (token != null && token !== KEEP_STORED_BEARER && !isBuffer(token)) { wipeBytes(token); return Promise.reject(new Error('accessToken must be a transferable buffer')); } diff --git a/packages/client/src/index.ts b/packages/client/src/index.ts index 2e2467faa9..b7ded00267 100644 --- a/packages/client/src/index.ts +++ b/packages/client/src/index.ts @@ -53,6 +53,7 @@ export type { MediaReader } from './media/broker.js'; export { fromHex, toHex } from './seams/bytes.js'; // The wire descriptors the UI exchanges with the engine over the transport. +export { KEEP_STORED_BEARER } from './worker/protocol.js'; export type { CommandDescriptor, CommandOutcomeDescriptor, diff --git a/packages/client/src/settings/quota.test.ts b/packages/client/src/settings/quota.test.ts index c7ddd7860a..55e279831a 100644 --- a/packages/client/src/settings/quota.test.ts +++ b/packages/client/src/settings/quota.test.ts @@ -185,38 +185,70 @@ describe('settingsSaveVerdict', () => { origin: 'resolved', credentialStored: false, byoEndpoint: '', + byoKind: 'kubo', byoAccessToken: '', + storedEndpoint: null, + storedKind: null, clearCredential: false, loadAcknowledged: false, ...overrides, }); const stored = (overrides: Partial = {}): SettingsSaveIntent => - intent({ credentialStored: true, byoEndpoint: 'https://kubo.example', ...overrides }); + intent({ + credentialStored: true, + byoEndpoint: 'https://kubo.example', + storedEndpoint: 'https://kubo.example', + storedKind: 'kubo', + ...overrides, + }); it('takes a save off a record this session read', () => { - expect(settingsSaveVerdict(intent())).toEqual({ ok: true }); + expect(settingsSaveVerdict(intent())).toEqual({ ok: true, keepStoredCredential: false }); }); - // The regression the prefill introduced: every other field round-trips, so a - // blank credential reads as "unchanged" while a save would publish it as gone. - it('refuses a blank credential over one the vault still holds', () => { - const verdict = settingsSaveVerdict(stored()); - - expect(verdict.ok).toBe(false); - expect(verdict.ok ? '' : verdict.problem).toMatch(/credential/); + // The point of the keep intent: every other field round-trips, so an + // untouched credential field must leave the stored bearer alone rather than + // publish it as gone. + it('keeps a stored credential the member did not touch', () => { + expect(settingsSaveVerdict(stored())).toEqual({ ok: true, keepStoredCredential: true }); }); it('takes the save once the member asks outright for the credential to go', () => { - expect(settingsSaveVerdict(stored({ clearCredential: true }))).toEqual({ ok: true }); + expect(settingsSaveVerdict(stored({ clearCredential: true }))).toEqual({ + ok: true, + keepStoredCredential: false, + }); }); it('takes the save once a new credential is typed', () => { - expect(settingsSaveVerdict(stored({ byoAccessToken: 'a fresh one' }))).toEqual({ ok: true }); + expect(settingsSaveVerdict(stored({ byoAccessToken: 'a fresh one' }))).toEqual({ + ok: true, + keepStoredCredential: false, + }); }); it('lets a blank credential go with the provider it belonged to', () => { - expect(settingsSaveVerdict(stored({ byoEndpoint: ' ' }))).toEqual({ ok: true }); + expect(settingsSaveVerdict(stored({ byoEndpoint: ' ' }))).toEqual({ + ok: true, + keepStoredCredential: false, + }); + }); + + // A kept bearer belongs to the provider it was stored for. Carrying it on to + // another endpoint would hand the member's credential to that endpoint. + it('refuses to keep a credential on to a repointed endpoint', () => { + const verdict = settingsSaveVerdict(stored({ byoEndpoint: 'https://other.example' })); + + expect(verdict.ok).toBe(false); + expect(verdict.ok ? '' : verdict.problem).toMatch(/different provider/); + }); + + it('refuses to keep a credential on to a different provider kind', () => { + const verdict = settingsSaveVerdict(stored({ byoKind: 'pinata' })); + + expect(verdict.ok).toBe(false); + expect(verdict.ok ? '' : verdict.problem).toMatch(/different provider/); }); it('refuses to publish defaults over a record nothing read', () => { @@ -229,11 +261,15 @@ describe('settingsSaveVerdict', () => { it('publishes them once the member takes that on', () => { expect(settingsSaveVerdict(intent({ origin: 'defaults', loadAcknowledged: true }))).toEqual({ ok: true, + keepStoredCredential: false, }); }); it('asks nothing extra of a stale read: it is still the member’s choice', () => { - expect(settingsSaveVerdict(intent({ origin: 'stale' }))).toEqual({ ok: true }); + expect(settingsSaveVerdict(intent({ origin: 'stale' }))).toEqual({ + ok: true, + keepStoredCredential: false, + }); }); }); diff --git a/packages/client/src/settings/quota.ts b/packages/client/src/settings/quota.ts index 0103322fc0..c7e6ad8d84 100644 --- a/packages/client/src/settings/quota.ts +++ b/packages/client/src/settings/quota.ts @@ -104,7 +104,16 @@ export function originNotice(origin: SettingsOrigin): OriginNotice { return ORIGIN_NOTICES[origin]; } -export type SettingsSaveVerdict = { ok: true } | { ok: false; problem: string }; +export type SettingsSaveVerdict = + | { + ok: true; + /** + * Whether the save must ask the engine to keep the bearer it already + * holds rather than publish one off the form. + */ + keepStoredCredential: boolean; + } + | { ok: false; problem: string }; /** The settings form as it stands, against the summary it was prefilled from. */ export interface SettingsSaveIntent { @@ -112,7 +121,11 @@ export interface SettingsSaveIntent { /** Whether the vault holds a provider bearer, which no read can show. */ credentialStored: boolean; byoEndpoint: string; + byoKind: ByoKind; byoAccessToken: string; + /** The provider the summary reported, which is the one the bearer is stored for. */ + storedEndpoint: string | null; + storedKind: ByoKind | null; /** The member asked outright for the stored credential to go. */ clearCredential: boolean; /** The member took on publishing over a record this session never read. */ @@ -120,10 +133,14 @@ export interface SettingsSaveIntent { } /** - * Whether the form may be published as it stands. A save replaces the whole - * record, so both refusals here are destructive edits the member did not ask - * for: publishing defaults over an unread record, and blanking a bearer the - * form cannot show back. + * Whether the form may be published as it stands, and how it spells the + * provider bearer. A save replaces the whole record, so a blank credential + * field over a stored bearer keeps it rather than clearing it — but only while + * the form still names the provider it was stored for. A repointed provider is + * unlikely to authenticate against the old credential, so it asks for one. + * + * The same binding is enforced in the engine, which is where it is load-bearing + * (`resolve_kept_bearer`). Here it is what turns a refusal into a keep. */ export function settingsSaveVerdict(intent: SettingsSaveIntent): SettingsSaveVerdict { if (originNotice(intent.origin).unread && !intent.loadAcknowledged) { @@ -133,19 +150,19 @@ export function settingsSaveVerdict(intent: SettingsSaveIntent): SettingsSaveVer 'no settings record loaded, so saving would publish these defaults over whatever the vault holds. take that on to save anyway.', }; } - if ( - intent.credentialStored && - intent.byoEndpoint.trim() !== '' && - intent.byoAccessToken === '' && - !intent.clearCredential - ) { + const endpoint = intent.byoEndpoint.trim(); + if (!intent.credentialStored || endpoint === '' || intent.byoAccessToken !== '') { + return { ok: true, keepStoredCredential: false }; + } + if (intent.clearCredential) return { ok: true, keepStoredCredential: false }; + if (endpoint !== (intent.storedEndpoint ?? '').trim() || intent.byoKind !== intent.storedKind) { return { ok: false, problem: - 'a provider credential is stored and this field is blank, which would clear it. re-enter it, or clear it outright.', + 'this names a different provider from the one the stored credential belongs to. enter a credential for it, or clear the stored one.', }; } - return { ok: true }; + return { ok: true, keepStoredCredential: true }; } const UNITS = ['B', 'KB', 'MB', 'GB', 'TB', 'PB'] as const; diff --git a/packages/client/src/worker/commandCodec.test.ts b/packages/client/src/worker/commandCodec.test.ts index 7bbb6104f6..8842af5f49 100644 --- a/packages/client/src/worker/commandCodec.test.ts +++ b/packages/client/src/worker/commandCodec.test.ts @@ -13,6 +13,7 @@ import { readSnapshot, readVaultStorage, } from './commandCodec.js'; +import { KEEP_STORED_BEARER } from './protocol.js'; import type { CommandDescriptor } from './protocol.js'; import type { EngineWasm, @@ -453,7 +454,9 @@ describe('buildCommand', () => { }, }); - expect(byo).toEqual([['https://kubo.example', fakeWasmEnums.ByoKind.Pinata, tokenBytes()]]); + expect(byo).toEqual([ + ['https://kubo.example', fakeWasmEnums.ByoKind.Pinata, tokenBytes(), false], + ]); expect(settings).toHaveLength(1); expect(settings[0][0]).toBe(fakeWasmEnums.PinMode.Dual); expect(settings[0][2]).toBe(3); @@ -529,7 +532,26 @@ describe('buildCommand', () => { }, }); - expect(byo).toEqual([['https://kubo.example', fakeWasmEnums.ByoKind.Kubo, undefined]]); + expect(byo).toEqual([['https://kubo.example', fakeWasmEnums.ByoKind.Kubo, undefined, false]]); + }); + + it('spells a keep intent as the keep flag, with no bearer bytes at all', () => { + const { wasm, byo } = spyWasm(); + + buildCommand(wasm, { + kind: 'saveVaultSettings', + settings: { + pinMode: 'external', + byo: { + endpoint: 'https://kubo.example', + kind: 'kubo', + accessToken: KEEP_STORED_BEARER, + }, + keepLatestVersions: null, + }, + }); + + expect(byo).toEqual([['https://kubo.example', fakeWasmEnums.ByoKind.Kubo, undefined, true]]); }); it('scrubs the worker copy of the bearer once the builder holds it', () => { diff --git a/packages/client/src/worker/commandCodec.ts b/packages/client/src/worker/commandCodec.ts index bc9d5a442c..8dbffd36df 100644 --- a/packages/client/src/worker/commandCodec.ts +++ b/packages/client/src/worker/commandCodec.ts @@ -7,7 +7,12 @@ * No interpretation, no crypto — the engine below the facade owns all of that. */ -import { BIN_INDEX_HOLD_CHECKS, MAX_FRAGMENT_CHARS, SETTINGS_HOLD_CHECKS } from './protocol.js'; +import { + BIN_INDEX_HOLD_CHECKS, + KEEP_STORED_BEARER, + MAX_FRAGMENT_CHARS, + SETTINGS_HOLD_CHECKS, +} from './protocol.js'; import type { AuthMethodDescriptor, AuthMethodKind, @@ -205,27 +210,32 @@ function binRetentionDays(value: unknown, field: string): number { return days; } -function byoConfig( - wasm: EngineWasm, - value: unknown, - token: Uint8Array | undefined -): WasmByoIpfsConfig { +function byoConfig(wasm: EngineWasm, value: unknown, bearer: BearerIntent): WasmByoIpfsConfig { const config = record(value, 'settings.byo'); return new wasm.ByoIpfsConfig( text(config.endpoint, 'settings.byo.endpoint'), byoKind(wasm, config.kind), - token + bearer.token, + bearer.keep ); } -/** The bearer a settings descriptor carries, checked but not yet spent. */ -function byoToken(value: unknown): Uint8Array | undefined { +/** The bearer intent a settings descriptor carries, checked but not yet spent. */ +interface BearerIntent { + token: Uint8Array | undefined; + keep: boolean; +} + +function byoBearer(value: unknown): BearerIntent { const byo = record(value, 'settings').byo ?? undefined; - if (byo === undefined) return undefined; + if (byo === undefined) return { token: undefined, keep: false }; const raw = record(byo, 'settings.byo').accessToken ?? undefined; + if (raw === KEEP_STORED_BEARER) return { token: undefined, keep: true }; // A view over the transferred buffer, not a copy: scrubbing it scrubs the // only copy that crossed into this realm. - return raw === undefined ? undefined : new Uint8Array(buffer(raw, 'settings.byo.accessToken')); + const token = + raw === undefined ? undefined : new Uint8Array(buffer(raw, 'settings.byo.accessToken')); + return { token, keep: false }; } /** @@ -238,7 +248,7 @@ function byoToken(value: unknown): Uint8Array | undefined { * builder copies what it keeps. */ function vaultSettings(wasm: EngineWasm, value: unknown): WasmVaultSettings { - const token = byoToken(value); + const bearer = byoBearer(value); try { const settings = record(value, 'settings'); const mode = pinMode(wasm, settings.pinMode); @@ -251,12 +261,12 @@ function vaultSettings(wasm: EngineWasm, value: unknown): WasmVaultSettings { const byo = settings.byo ?? undefined; return new wasm.VaultSettings( mode, - byo === undefined ? undefined : byoConfig(wasm, byo, token), + byo === undefined ? undefined : byoConfig(wasm, byo, bearer), keep, bin ); } finally { - token?.fill(0); + bearer.token?.fill(0); } } diff --git a/packages/client/src/worker/engineWasm.ts b/packages/client/src/worker/engineWasm.ts index f5314bb21e..732e49ad2f 100644 --- a/packages/client/src/worker/engineWasm.ts +++ b/packages/client/src/worker/engineWasm.ts @@ -392,7 +392,8 @@ export interface EngineWasm { ByoIpfsConfig: new ( endpoint: string, kind: number, - accessToken?: Uint8Array + accessToken: Uint8Array | undefined, + keepAccessToken: boolean ) => WasmByoIpfsConfig; VaultSettings: new ( pinMode: number, diff --git a/packages/client/src/worker/protocol.ts b/packages/client/src/worker/protocol.ts index 6b1a9aa720..159ee73df1 100644 --- a/packages/client/src/worker/protocol.ts +++ b/packages/client/src/worker/protocol.ts @@ -331,17 +331,23 @@ export type PinMode = 'hosted' | 'external' | 'dual'; /** The kind of member-supplied IPFS provider (mirrors the facade `ByoKind`). */ export type ByoKind = 'kubo' | 'psa' | 'pinata'; +/** The `accessToken` value that keeps the bearer the engine already holds. */ +export const KEEP_STORED_BEARER = 'keep'; + /** A member's own IPFS provider, as data. */ export interface ByoIpfsConfigDescriptor { endpoint: string; kind: ByoKind; /** - * Bearer credential, `null` for a provider that needs none. A transferable - * buffer rather than a string, which cannot be overwritten: every hop moves - * it ([`commandTransfer`]), so the receiving realm is the only holder left - * and is the terminal owner that scrubs it. + * Bearer credential, three-state: a buffer sets a new one, `'keep'` keeps + * whatever the engine already holds, and `null` stores none. `'keep'` is the + * only way a host that can never read a stored bearer back leaves one alone. + * + * A transferable buffer rather than a string, which cannot be overwritten: + * every hop moves it ([`commandTransfer`]), so the receiving realm is the + * only holder left and is the terminal owner that scrubs it. */ - accessToken: ArrayBuffer | null; + accessToken: ArrayBuffer | typeof KEEP_STORED_BEARER | null; } /** The member's placement, provider and retention choice, as data. */