Harden browser sync against untrusted servers and recovery kits

apps/app/src/lib/browser-sync.test.ts

@@ -238,6 +238,16 @@ describe('browser sync controller custody', () => {
 			sameOrigin('https://other.example/v1/device-challenges'),
 		).rejects.toThrow('same-origin');
 	});
+
+	test('same-origin fetch forces redirect: error', async () => {
+		let captured: RequestInit | undefined;
+		const sameOrigin = createSameOriginFetch(ORIGIN, async (_input, init) => {
+			captured = init;
+			return json({ ok: true });
+		});
+		await sameOrigin('/v1/device-challenges');
+		expect(captured?.redirect).toBe('error');
+	});
 });
 
 describe('browser sync reconciliation', () => {
@@ -2485,6 +2495,9 @@ describe('browser native recovery kit import', () => {
 			expect(bound.key.construction).toBe('web');
 			expect(bound.key.deviceId).toBe(deviceId!);
 			expect(bound.key.wrappedKey.length).toBeGreaterThan(0);
+			// A native kit carries no verified paths, so recovered objects must
+			// never be treated as owners of local files.
+			expect(bound.unmapped).toBe(true);
 		}
 	});
 
@@ -2505,6 +2518,44 @@ describe('browser native recovery kit import', () => {
 		}
 	});
 
+	test('rejects replacing a binding that points at a different remote workspace', async () => {
+		const { controller, bindingStore } = await nativeController();
+		const boundKey = {
+			deviceId: 'device_existing',
+			wrappedKey: '',
+			signature: '',
+			construction: 'web' as const,
+			recipientPublicKey: '',
+			ephemeralPublicKey: '',
+			salt: '',
+			nonce: '',
+		};
+		await bindingStore.write({
+			version: 1,
+			localWorkspaceId: 'workspace_native',
+			workspaceId: 'workspace_other',
+			revision: '1',
+			objectId: 'object_other',
+			objects: {
+				object_other: {
+					path: 'notes/other.md',
+					epoch: 1,
+					policyRevision: '1',
+					key: boundKey,
+				},
+			},
+			pinnedSigners: { device_existing: '' },
+		});
+		const result = await controller.importNativeRecoveryKit(NATIVE_KIT, {
+			recoveryIdentity: NATIVE_KIT.recovery_identity,
+			localWorkspaceId: 'workspace_native',
+		});
+		expect(result.ok).toBe(false);
+		if (!result.ok) expect(result.code).toBe('custody_failed');
+		// The existing binding must be left untouched.
+		expect((await bindingStore.read())?.workspaceId).toBe('workspace_other');
+	});
+
 	test('accepts a caller-pinned signer matching the kit self-description', async () => {
 		const { controller, bindingStore } = await nativeController();
 		const signer =

apps/app/src/lib/browser-sync.ts

@@ -356,6 +356,10 @@ function parseAccessPolicy(value: unknown): AccessPolicy | null {
 					mode: descriptor.mode,
 				};
 		}
+		// A version-2 policy binds a document descriptor per object; the server
+		// rejects a version-2 object without one. A malformed or partial response
+		// must not be rebuilt and re-signed, so skip provisioning instead.
+		if (version === 2 && document === undefined) return null;
 		objects.push({
 			objectId: object.objectId,
 			epoch: object.epoch as number,
@@ -464,6 +468,12 @@ export interface BrowserSyncObjectBinding {
 	localObjectId?: string;
 	/** Current local path from the managed-object list, when it differs from `path`. */
 	livePath?: string;
+	/**
+	 * True when this object was recovered from a native kit and has no verified
+	 * canonical local path. Unmapped objects can open delivered operations but
+	 * never own a local file or attachment.
+	 */
+	unmapped?: boolean;
 	/** Positive safe-integer key epoch. */
 	epoch: number;
 	/** Canonical decimal access-policy revision the object was bound at. */
@@ -582,6 +592,9 @@ function failure<T = undefined>(
 /**
  * Only same-origin requests are allowed. The hosted app is served by the sync
  * server, so a relative URL and the page origin address the same service.
+ *
+ * Redirects are refused (`redirect: 'error'`): checking only the initial URL
+ * would let a malicious sync server answer with a 30x to an arbitrary origin.
  */
 export function createSameOriginFetch(
 	base: string = globalThis.location?.origin ?? '',
@@ -596,7 +609,7 @@ export function createSameOriginFetch(
 				),
 			);
 		}
-		return fetchImpl(input, init);
+		return fetchImpl(input, { ...init, redirect: 'error' });
 	};
 }
 
@@ -732,7 +745,8 @@ function resolveObjectOwner(
 	if (attachmentId !== null) {
 		const objects = input.objects;
 		if (objects && objects.size > 0) {
-			return objects.get(attachmentId) ?? null;
+			const owner = objects.get(attachmentId) ?? null;
+			return owner && owner.unmapped !== true ? owner : null;
 		}
 		if (attachmentId !== input.objectId) return null;
 		return {
@@ -745,7 +759,8 @@ function resolveObjectOwner(
 	const objects = input.objects;
 	if (objects && objects.size > 0) {
 		const owns = (object: BrowserSyncObjectBinding, path: string): boolean =>
-			object.path === path || object.livePath === path;
+			object.unmapped !== true &&
+			(object.path === path || object.livePath === path);
 		for (const object of objects.values()) {
 			if (owns(object, change.path)) return object;
 		}
@@ -1789,9 +1804,11 @@ function resolveNativeRecoverySigner(
  * Build the binding's per-object metadata from a native recovery object.
  *
  * A native kit does not carry the browser replica's canonical paths, so each
- * recovered object is anchored to its native object id. A native object id is
- * not a canonical file path, so no local file is mis-associated; the binding
- * holds the re-wrapped key until the replica is reconciled.
+ * recovered object is anchored to its native object id. Object ids are
+ * validated as identifiers (never canonical file paths) and the persisted
+ * binding marks every recovered object `unmapped`, so a crafted kit cannot make
+ * a local file or attachment be sealed under a recovered key. The binding holds
+ * the re-wrapped key until the replica is reconciled.
  */
 function nativeRecoveryObjects(recovery: NativeRecoveryObject): {
 	objectId: string;
@@ -1803,7 +1820,7 @@ function nativeRecoveryObjects(recovery: NativeRecoveryObject): {
 		const envelope = raw as NativeRecoveryEnvelope;
 		if (
 			!envelope ||
-			typeof envelope.objectId !== 'string' ||
+			!isIdentifier(envelope.objectId) ||
 			!Number.isSafeInteger(envelope.epoch) ||
 			envelope.epoch < 1
 		)
@@ -2232,6 +2249,24 @@ export class BrowserSyncController {
 				'This browser cannot persist a durable sync binding (OPFS is unavailable).',
 			);
 		}
+		// A recovery kit is untrusted input. Never let it silently repoint an
+		// already-configured workspace at a different remote workspace.
+		let existing: BrowserSyncBindingRecord | null = null;
+		try {
+			existing = await store.read();
+		} catch (error) {
+			this.#error = messageOf(error);
+			return failure('custody_failed', this.#error);
+		}
+		if (
+			existing &&
+			existing.workspaceId !== parsed.recovery.config.workspaceId
+		) {
+			return failure(
+				'custody_failed',
+				'This browser workspace is already bound to a different sync workspace; clear the existing binding before importing a recovery kit.',
+			);
+		}
 		try {
 			const result = await rewrapNativeKeys({
 				recovery: parsed.recovery,
@@ -2443,6 +2478,7 @@ export class BrowserSyncController {
 				...(bound.localObjectId === undefined
 					? {}
 					: { localObjectId: bound.localObjectId }),
+				...(bound.unmapped === true ? { unmapped: true } : {}),
 				epoch: bound.epoch,
 				policyRevision: bound.policyRevision,
 			});

crates/local-core/src/sync/coordinator.rs

@@ -286,14 +286,13 @@ impl WorkspaceSyncCoordinator {
             access = transport.access_state(&engine.manifest().id).await?;
         }
         // Drop approvals for devices the relay no longer lists for this
-        // workspace. A revoked device's stale recipient and account must not
-        // keep it in the authorized-writer set or the sharing gate. Persist the
-        // drop only once the local revision matches the relay head, so a later
-        // chain refresh can still verify policies signed before the revocation.
+        // workspace, for this pass only. The relay roster is unsigned, so it
+        // must never durably delete local trust: a malicious or compromised
+        // relay could otherwise permanently revoke a legitimate device by
+        // omitting it from the roster. The pruned view still keeps a stale
+        // revoked device out of the authorized-writer set and the sharing gate;
+        // a genuine revocation is re-derived on every pass.
         let config = access.effective_config(&persisted, device.device_id());
-        if config != persisted && access.revision() == engine.sync_access_revision()? {
-            engine.sync_save_configuration(&config)?;
-        }
         let mut secrets = engine.sync_restore_secrets(&device, &config.trusted_devices)?;
         // Fetch rotations before capturing new edits. Never select an epoch from an unverified key.
         transport

crates/local-core/src/sync/recovery.rs

@@ -469,7 +469,12 @@ pub struct BrowserRecoveredObject {
     pub object_id: String,
     /// Positive safe-integer key epoch.
     pub epoch: u64,
-    /// Canonical-path anchor recorded at bind time.
+    /// Path anchor recorded at bind time.
+    ///
+    /// This value comes from the kit and is **not** verified against the local
+    /// workspace. Callers must not use it to associate a local file with the
+    /// recovered object; match local objects by `local_object_id` after
+    /// independent verification, or leave the object unmapped.
     pub path: String,
     /// Stable local object ID, when the binding recorded one.
     pub local_object_id: Option<String>,
@@ -714,6 +719,11 @@ fn build_browser_recovery(
     let mut objects = Vec::with_capacity(binding.objects.len());
     for (object_id, object) in &binding.objects {
         identifier(object_id)?;
+        if let Some(local_object_id) = &object.local_object_id {
+            // A local object id is metadata from an untrusted kit; never accept
+            // a path-like value that a caller could mistake for a file path.
+            identifier(local_object_id)?;
+        }
         if object.key.construction != "web"
             || object.key.device_id != bundle.device_id
             || object.epoch == 0

packages/browser-sync/src/access-policy.test.ts

@@ -1,6 +1,7 @@
 import { describe, expect, test } from 'bun:test';
 import fixture from '../../../docs/workspace-format/fixtures/policy-web-v1.json';
 import { accessSigningBytes, type AccessPolicy } from './access-policy';
+import { BrowserSyncError, BrowserSyncErrorCode } from './errors';
 
 type UnsignedPolicy = Omit<AccessPolicy, 'signature'>;
 
@@ -20,4 +21,26 @@ describe('accessSigningBytes', () => {
 		// share canonical bytes with the web-only policy.
 		expect(fixture.mixed.signing_bytes).not.toBe(fixture.signing_bytes);
 	});
+
+	test('rejects a version-2 object without a document descriptor', () => {
+		const policy = {
+			version: 2,
+			workspaceId: 'workspace',
+			revision: '1',
+			previousPolicyDigest: null,
+			deviceId: 'device',
+			members: [{ accountId: 'account', role: 'owner' }],
+			objects: [{ objectId: 'object', epoch: 1, grants: [], envelopes: [] }],
+		} satisfies UnsignedPolicy;
+
+		expect(() => accessSigningBytes(policy)).toThrow(BrowserSyncError);
+		try {
+			accessSigningBytes(policy);
+			throw new Error('expected accessSigningBytes to reject');
+		} catch (error) {
+			expect((error as BrowserSyncError).code).toBe(
+				BrowserSyncErrorCode.InvalidPolicy,
+			);
+		}
+	});
 });

packages/browser-sync/src/access-policy.ts

@@ -95,7 +95,16 @@ export function accessSigningBytes(
 				object.grants.map((grant) => [grant.accountId, grant.role]),
 				object.envelopes.map(envelopeTuple),
 			];
-			if (policy.version === 2 && object.document) {
+			if (policy.version === 2) {
+				// The server signs a document tuple for every version-2 object
+				// and rejects one without a document descriptor. Reject here too
+				// so the two implementations cannot silently diverge.
+				if (!object.document) {
+					throw new BrowserSyncError(
+						BrowserSyncErrorCode.InvalidPolicy,
+						'version-2 access policy object was missing its document descriptor',
+					);
+				}
 				entry.push([object.document.generation, object.document.mode]);
 			}
 			return entry;

packages/browser-sync/src/attachments.ts

@@ -31,7 +31,12 @@ import { syncFileChangeSchema } from '@noura/workspace-schema';
 import { bytesEqual, fixedBytes, isIdentifier, type Bytes } from './crypto';
 import { BrowserSyncError, BrowserSyncErrorCode } from './errors';
 import type { FileChangeBlob, FileChangeV3 } from './file-change';
-import { ensureResponseOk, readJson, type FetchLike } from './http';
+import {
+	ensureResponseOk,
+	readBoundedBytes,
+	readJson,
+	type FetchLike,
+} from './http';
 
 /** BLAKE3 `derive_key` context binding an object key to its blob identity. */
 export const BLOB_KEY_CONTEXT = 'noura.sync.blob.x25519.v1';
@@ -1416,7 +1421,9 @@ export async function downloadBlob(input: DownloadBlobInput): Promise<void> {
 				'attachment range response was malformed',
 			);
 		}
-		const bytes = new Uint8Array(await response.arrayBuffer());
+		// Bound the read at the expected range length so a lying
+		// `Content-Range` cannot force an oversized allocation.
+		const bytes = await readBoundedBytes(response, length);
 		if (bytes.length !== length) {
 			throw new BrowserSyncError(
 				BrowserSyncErrorCode.BlobLengthMismatch,

packages/browser-sync/src/binding.ts

@@ -38,6 +38,14 @@ export interface BrowserSyncBoundObject {
 	epoch: number;
 	/** Canonical access-policy revision the object was bound at. */
 	policyRevision: string;
+	/**
+	 * True when this object came from a recovery import and has no verified
+	 * canonical local path. An unmapped object may decrypt delivered operations
+	 * but must never be treated as the owner of a local file or attachment, so a
+	 * crafted recovery kit cannot cause local bytes to be sealed under an
+	 * attacker-known key.
+	 */
+	unmapped?: boolean;
 	/** Self-wrapped object key; never plaintext. */
 	key: BrowserSyncBoundKey;
 }
@@ -93,6 +101,7 @@ export function isBrowserSyncBindingRecord(
 			typeof bound.path !== 'string' ||
 			(bound.localObjectId !== undefined &&
 				typeof bound.localObjectId !== 'string') ||
+			(bound.unmapped !== undefined && typeof bound.unmapped !== 'boolean') ||
 			!Number.isSafeInteger(bound.epoch) ||
 			(bound.epoch as number) < 1 ||
 			typeof bound.policyRevision !== 'string' ||