From 4fe924ee0ff407a1333a3ce9a58fa8b7cbbc05de Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Thu, 13 Aug 2026 23:35:48 +0900 Subject: [PATCH] fix: harden bounded state sidecars N-05: the conditional-validator key was a colon join over components that may themselves contain colons, so two distinct valid bindings could collide and one definition's ETag could be prepared for another. The key is now a validated, byte-bounded fixed tuple encoded with JSON.stringify. N-09: capture localStorage exactly once and compare StorageEvent.storageArea against that object identity, so a pulse from sessionStorage or any other area is rejected instead of matching on key and value alone. The pulse key is registered in the storage registry as CACHE_INVALIDATION_PULSE. N-10: race loadPage against the caller signal and re-check before observing a page, so a non-cooperative loader can neither hold loadAll forever nor have a post-abort completion accumulated into a successful result. Co-Authored-By: Claude Opus 5 (1M context) --- docs/operations/adapter-remediation-ledger.md | 6 +- .../browser-cross-context-host.ts | 41 ++++++++++-- .../browser-cross-context-invalidation.ts | 8 +++ .../conditional-validator-store.ts | 35 +++++++++- .../query-cache/cursor-pagination-runtime.ts | 48 +++++++++++++- src/contracts/storage-keys.ts | 12 ++++ .../unit/conditional-validator-store.test.ts | 25 +++++++ tests/unit/cross-tab-invalidation.test.ts | 66 ++++++++++++++++++- tests/unit/cursor-pagination-runtime.test.ts | 39 +++++++++++ 9 files changed, 261 insertions(+), 19 deletions(-) diff --git a/docs/operations/adapter-remediation-ledger.md b/docs/operations/adapter-remediation-ledger.md index 8cc1bf2..36bff33 100644 --- a/docs/operations/adapter-remediation-ledger.md +++ b/docs/operations/adapter-remediation-ledger.md @@ -76,12 +76,12 @@ Rollout state starts at `NOT_STARTED`; documented-unimplemented items start at | N-02 | Live V3 path | `corepack pnpm exec vitest run tests/integration/http-execution-v3-auth-profile.test.ts` | `fix: enforce installed HTTP auth profiles` | `FIXED_NOT_RELEASED` | authenticated request 4xx spike after profile enforcement | Red suite failed to load (`installRestAuthProfileRegistry` absent) → green 7/7; `check:types` PASS; `check:architecture` PASS; `lint` PASS; unit+integration+features 1560 passed with only the pre-existing environmental `ci-artifact-contract` failures | | N-03 | Live V3 path | `corepack pnpm exec vitest run tests/unit/http-execution-v3.test.ts -t "retry-time fence"` | `fix: preserve command effect certainty across retries` | `FIXED_NOT_RELEASED` | command effect verdict regression | Red reproduced `SCOPE_FENCED` with `NOT_STARTED` after one dispatched attempt → green `MAYBE_APPLIED`; lattice table 9/9; `check:types` PASS; `lint` PASS | | N-04 | Live composition teardown | `corepack pnpm exec vitest run tests/unit/telemetry.test.ts tests/unit/runtime-adapters.test.ts` | `fix: terminate telemetry work on disposal` | `FIXED_NOT_RELEASED` | telemetry delivery loss after teardown change | Red 10 failed (5 lifecycle + 5 capacity) → green 34/34; `check:diagnostics` PASS; `check:types` PASS; `check:architecture` PASS; `lint` PASS | -| N-05 | Rollout blocker (sidecar not composed) | `corepack pnpm exec vitest run tests/unit/conditional-validator-store.test.ts` | — | `NOT_STARTED` | persisted validator key incompatibility | — | +| N-05 | Rollout blocker (sidecar not composed) | `corepack pnpm exec vitest run tests/unit/conditional-validator-store.test.ts` | `fix: harden bounded state sidecars` | `FIXED_NOT_RELEASED` | persisted validator key incompatibility | Red collision case (two valid bindings sharing one delimiter-joined key) → green; key is now a bounded validated tuple encoded with `JSON.stringify` | | N-06 | Legacy V2 rollback seam | `corepack pnpm exec vitest run tests/integration/http-client.test.ts` | — | `NOT_STARTED` | legacy keyed command rejection spike | — | | N-07 | Legacy V2 rollback seam | `corepack pnpm exec vitest run tests/integration/auth-recovery.test.ts` | — | `NOT_STARTED` | legacy credential timeout regression | — | | N-08 | Legacy V2 rollback seam | `corepack pnpm exec vitest run tests/unit/bounded-json-compatibility.test.ts` | — | `NOT_STARTED` | legacy JSON failure-code drift | — | -| N-09 | Live cross-context host | `corepack pnpm exec vitest run tests/unit/cross-tab-invalidation.test.ts` | — | `NOT_STARTED` | cross-tab invalidation drop | — | -| N-10 | Cursor runtime `AVAILABLE_NOT_COMPOSED` | `corepack pnpm exec vitest run tests/unit/cursor-pagination-runtime.test.ts` | — | `NOT_STARTED` | pagination abort semantics change | — | +| N-09 | Live cross-context host | `corepack pnpm exec vitest run tests/unit/cross-tab-invalidation.test.ts` | `fix: harden bounded state sidecars` | `FIXED_NOT_RELEASED` | cross-tab invalidation drop | Red foreign-area pulse accepted → green 13/13; localStorage captured once and `StorageEvent.storageArea` compared by object identity; pulse key registered as `CACHE_INVALIDATION_PULSE`; `check:registries` PASS | +| N-10 | Cursor runtime `AVAILABLE_NOT_COMPOSED` | `corepack pnpm exec vitest run tests/unit/cursor-pagination-runtime.test.ts` | `fix: harden bounded state sidecars` | `FIXED_NOT_RELEASED` | pagination abort semantics change | Red never-settling loader → green `PAGINATION_ABORTED` with the late page ignored | | N-11 | Live composition | `corepack pnpm exec vitest run tests/unit/telemetry.test.ts -t capacity` | `fix: terminate telemetry work on disposal` | `FIXED_NOT_RELEASED` | capacity rejection on valid composition | Red 5/5 capacity cases → green; ceilings documented in VD-07 §7-4 | ### Storage and browser files (`docs/reviews/adapters/03-storage-and-browser-files.md`) diff --git a/src/adapters/cross-context-invalidation/browser-cross-context-host.ts b/src/adapters/cross-context-invalidation/browser-cross-context-host.ts index e1c2ca0..b3d55a2 100644 --- a/src/adapters/cross-context-invalidation/browser-cross-context-host.ts +++ b/src/adapters/cross-context-invalidation/browser-cross-context-host.ts @@ -2,6 +2,7 @@ import { isCacheInvalidationOpaqueIdentifier, type CacheInvalidationTopicDefinition, } from "../../contracts/cache-invalidation.ts"; +import { STORAGE_REGISTRY } from "../../contracts/storage-keys.ts"; import { createBrowserCrossContextInvalidation, type BroadcastChannelFacade, @@ -31,8 +32,9 @@ type NativeBroadcastChannel = Readonly<{ }>; const CHANNEL_NAME = "ca-client-cache-invalidation-v1"; +// N-09. The registry owns the physical-key policy for the pulse. const STORAGE_PULSE_KEY = - "ca-frontend:cache-invalidation:v1:pulse"; + STORAGE_REGISTRY.CACHE_INVALIDATION_PULSE.physicalKey; /** * Captures native capabilities without allowing a SecurityError getter or a @@ -59,6 +61,8 @@ export function createBrowserCrossContextInvalidationFromHost( const sourceEpoch = createOpaqueId("page"); if (!sourceId || !sourceEpoch) return undefined; + const capturedLocalStorage = captureLocalStorage(host); + return createBrowserCrossContextInvalidation({ channelName: CHANNEL_NAME, storagePulseKey: STORAGE_PULSE_KEY, @@ -72,8 +76,13 @@ export function createBrowserCrossContextInvalidationFromHost( return eventId; }, createBroadcastChannel: broadcastFactory(host), - storage: storageFacade(host), - storageEvents: storageEventTarget(host), + // N-09. One capture, one identity: the write facade and the event + // validator must agree about which Storage object they trust. Reading the + // getter twice would let a hostile host return a different object. + storage: capturedLocalStorage + ? storageFacade(capturedLocalStorage) + : undefined, + storageEvents: storageEventTarget(host, capturedLocalStorage), observe: dependencies.observe, }); } @@ -182,11 +191,16 @@ function isNativeBroadcastChannel( ); } -function storageFacade( +function captureLocalStorage( host: Record, -): StoragePulseFacade | undefined { +): object | undefined { const candidate = safeGet(host, "localStorage"); - if (!candidate || typeof candidate !== "object") return undefined; + return candidate && typeof candidate === "object" ? candidate : undefined; +} + +function storageFacade( + candidate: object, +): StoragePulseFacade | undefined { const record = candidate as Record; const setItem = safeGet(record, "setItem"); const removeItem = safeGet(record, "removeItem"); @@ -205,6 +219,7 @@ function storageFacade( function storageEventTarget( host: Record, + expectedLocalStorage: object | undefined, ): StorageEventTargetFacade | undefined { const addEventListener = safeGet(host, "addEventListener"); const removeEventListener = safeGet(host, "removeEventListener"); @@ -222,15 +237,27 @@ function storageEventTarget( addEventListener(_type: "storage", listener: StoragePulseListener) { const bound = (event: unknown) => { if (!event || typeof event !== "object") { - listener({ key: null, newValue: null }); + listener({ + key: null, + newValue: null, + storageArea: "OTHER_OR_UNKNOWN", + }); return; } const record = event as Record; const key = safeGet(record, "key"); const newValue = safeGet(record, "newValue"); + const storageArea = safeGet(record, "storageArea"); listener({ key: typeof key === "string" ? key : null, newValue: typeof newValue === "string" ? newValue : null, + // Compared by object identity against the captured area, never by + // shape or by re-reading `host.localStorage`. + storageArea: + expectedLocalStorage !== undefined && + storageArea === expectedLocalStorage + ? "EXPECTED_LOCAL_STORAGE" + : "OTHER_OR_UNKNOWN", }); }; bindings.set(listener, bound); diff --git a/src/adapters/cross-context-invalidation/browser-cross-context-invalidation.ts b/src/adapters/cross-context-invalidation/browser-cross-context-invalidation.ts index 52407c4..7bea739 100644 --- a/src/adapters/cross-context-invalidation/browser-cross-context-invalidation.ts +++ b/src/adapters/cross-context-invalidation/browser-cross-context-invalidation.ts @@ -98,6 +98,13 @@ export type StoragePulseFacade = Readonly<{ export type StoragePulseEvent = Readonly<{ key: string | null; newValue: string | null; + /** + * N-09. A `storage` event fires for every `Storage` area in the context. + * Matching only key and value cannot prove the write came from the + * localStorage this runtime actually captured, so the host classifies the + * native `storageArea` by object identity and the core admits one value. + */ + storageArea: "EXPECTED_LOCAL_STORAGE" | "OTHER_OR_UNKNOWN"; }>; export type StoragePulseListener = (event: StoragePulseEvent) => void; @@ -179,6 +186,7 @@ export function createBrowserCrossContextInvalidation( const receiveStorage: StoragePulseListener = (event) => { if ( closed || + event.storageArea !== "EXPECTED_LOCAL_STORAGE" || event.key !== dependencies.storagePulseKey || typeof event.newValue !== "string" ) { diff --git a/src/adapters/query-cache/conditional-validator-store.ts b/src/adapters/query-cache/conditional-validator-store.ts index 9d43b66..81e5100 100644 --- a/src/adapters/query-cache/conditional-validator-store.ts +++ b/src/adapters/query-cache/conditional-validator-store.ts @@ -32,6 +32,26 @@ type ValidatorRow = { generation: number; }; +type ConditionalValidatorKeyTuple = readonly [ + scopeFingerprint: string, + definitionId: string, + identityToken: string, + representationVersion: number, +]; + +/** The store is a trust boundary, so key components are validated and bounded. */ +const MAX_KEY_COMPONENT_BYTES = 512; +const KEY_COMPONENT_ENCODER = new TextEncoder(); + +function isBoundedKeyComponent(value: unknown): value is string { + return ( + typeof value === "string" && + value.length > 0 && + KEY_COMPONENT_ENCODER.encode(value).byteLength <= + MAX_KEY_COMPONENT_BYTES + ); +} + export function createConditionalValidatorStore( maxEntries = 1_024, ): ConditionalValidatorStore { @@ -40,22 +60,31 @@ export function createConditionalValidatorStore( } const rows = new Map(); + /** + * N-05. A delimiter join is not injective here: `definitionId`, + * `identityToken` and the scope fingerprint may all contain the delimiter, so + * two distinct valid bindings could encode to the same key and one + * definition's ETag could be sent for another. The key is a validated fixed + * tuple encoded with `JSON.stringify`, which escapes the separators. + */ function key(binding: ConditionalValidatorBinding): string | null { if ( !binding.scope.isCurrent() || - !binding.definitionId || + !isBoundedKeyComponent(binding.scope.fingerprint) || + !isBoundedKeyComponent(binding.definitionId) || !/^[A-Za-z0-9._:-]{16,128}$/.test(binding.identityToken) || !Number.isSafeInteger(binding.representationVersion) || binding.representationVersion < 1 ) { return null; } - return [ + const tuple: ConditionalValidatorKeyTuple = [ binding.scope.fingerprint, binding.definitionId, binding.identityToken, binding.representationVersion, - ].join(":"); + ]; + return JSON.stringify(tuple); } return Object.freeze({ diff --git a/src/adapters/query-cache/cursor-pagination-runtime.ts b/src/adapters/query-cache/cursor-pagination-runtime.ts index 5fd0de6..7fc89e8 100644 --- a/src/adapters/query-cache/cursor-pagination-runtime.ts +++ b/src/adapters/query-cache/cursor-pagination-runtime.ts @@ -6,6 +6,36 @@ import type { } from "../../contracts/cursor-pagination.ts"; import { createFailure } from "../../contracts/errors.ts"; +const ABORTED = Symbol("PAGINATION_ABORTED"); + +/** + * Resolves as soon as the operation settles or the signal aborts, whichever + * comes first. A late operation result is observed and discarded, never thrown + * as an unhandled rejection. + */ +async function raceAbort( + operation: Promise, + signal: AbortSignal | undefined, +): Promise { + operation.catch(() => {}); + if (!signal) return await operation; + if (signal.aborted) return ABORTED; + return await new Promise((resolve) => { + const onAbort = () => resolve(ABORTED); + signal.addEventListener("abort", onAbort, { once: true }); + operation.then( + (value) => { + signal.removeEventListener("abort", onAbort); + resolve(value); + }, + () => { + signal.removeEventListener("abort", onAbort); + resolve(ABORTED); + }, + ); + }); +} + export function createCursorPaginationRuntime(dependencies: Readonly<{ definitionId: string; profile: CursorPaginationProfile; @@ -29,9 +59,21 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ if (context.signal?.aborted) { return failure("REQUEST_ABORTED", "PAGINATION_ABORTED"); } - const result = await dependencies.loadPage(cursor, context); + // N-10. A non-cooperative loader may never settle, or may settle after + // abort. Race the signal so `loadAll` is bounded, and re-check before + // observing the page so a late completion is ignored rather than + // accumulated into a successful result. + const raced: Result> | typeof ABORTED = + await raceAbort>>( + dependencies.loadPage(cursor, context), + context.signal, + ); + if (raced === ABORTED || context.signal?.aborted) { + return failure("REQUEST_ABORTED", "PAGINATION_ABORTED"); + } + const result: Result> = raced; if (!result.ok) return result; - const page = result.value; + const page: CursorPage = result.value; if (!isValidPage(page, dependencies.profile)) { return failure( "PAGINATION_CONTRACT_VIOLATION", @@ -57,7 +99,7 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ ); } if (!page.hasMore) return { ok: true, value: Object.freeze(items) }; - const nextCursor = page.nextCursor; + const nextCursor: string | null = page.nextCursor; if (!nextCursor || cursors.has(nextCursor)) { return failure( "PAGINATION_CONTRACT_VIOLATION", diff --git a/src/contracts/storage-keys.ts b/src/contracts/storage-keys.ts index c92217b..bb726c1 100644 --- a/src/contracts/storage-keys.ts +++ b/src/contracts/storage-keys.ts @@ -69,6 +69,18 @@ export const STORAGE_REGISTRY = Object.freeze({ migration: "discard", quotaFallback: "feature-disable", }), + CACHE_INVALIDATION_PULSE: defineStorageKey({ + logicalName: "CACHE_INVALIDATION_PULSE", + scope: "cache-invalidation", + name: "pulse", + backend: "localStorage", + classification: "opaque-cache", + schemaVersion: 1, + valueCodec: "opaque-string-v1", + ttl: null, + migration: "discard", + quotaFallback: "no-persist", + }), AUTH_TOKEN: defineStorageKey({ logicalName: "AUTH_TOKEN", scope: "auth", diff --git a/tests/unit/conditional-validator-store.test.ts b/tests/unit/conditional-validator-store.test.ts index 75ddd8c..9d4ccc6 100644 --- a/tests/unit/conditional-validator-store.test.ts +++ b/tests/unit/conditional-validator-store.test.ts @@ -44,6 +44,31 @@ describe("conditional validator CAS sidecar", () => { expect(store.acceptNotModified(binding, 7, true)).toBe(false); }); + it("keeps colon-bearing validator tuples injective", () => { + const selectedScope = scope(); + // N-05. Both bindings are individually valid and, under a delimiter join, + // encode to the same key. + const first = { + definitionId: "resource:detail", + identityToken: "identity-token-00000001", + representationVersion: 1, + scope: selectedScope.snapshot, + }; + const second = { + definitionId: "resource", + identityToken: "detail:identity-token-00000001", + representationVersion: 1, + scope: selectedScope.snapshot, + }; + const store = createConditionalValidatorStore(); + + expect(store.install(first, '"etag-a"', 7)).toBe(true); + expect(store.install(second, '"etag-b"', 7)).toBe(true); + + expect(store.prepare(first, 7)).toBe('"etag-a"'); + expect(store.prepare(second, 7)).toBe('"etag-b"'); + }); + it("rejects malformed validators and bounded-capacity overflow", () => { const firstScope = scope(); const store = createConditionalValidatorStore(1); diff --git a/tests/unit/cross-tab-invalidation.test.ts b/tests/unit/cross-tab-invalidation.test.ts index 15d2ad7..49df90b 100644 --- a/tests/unit/cross-tab-invalidation.test.ts +++ b/tests/unit/cross-tab-invalidation.test.ts @@ -112,9 +112,14 @@ class FakeStorageEventTarget implements StorageEventTargetFacade { this.listeners.delete(listener); } - emit(key: string | null, newValue: string | null): void { + emit( + key: string | null, + newValue: string | null, + storageArea: "EXPECTED_LOCAL_STORAGE" | "OTHER_OR_UNKNOWN" = + "EXPECTED_LOCAL_STORAGE", + ): void { for (const listener of [...this.listeners]) { - listener({ key, newValue }); + listener({ key, newValue, storageArea }); } } } @@ -135,6 +140,9 @@ class FakeStorageBus { }); } + /** Simulates the event arriving from a different Storage area. */ + emitAsForeignArea = false; + set( owner: FakeStorageEventTarget, key: string, @@ -142,7 +150,15 @@ class FakeStorageBus { ): void { this.values.set(key, value); for (const target of this.targets) { - if (target !== owner) target.emit(key, value); + if (target !== owner) { + target.emit( + key, + value, + this.emitAsForeignArea + ? "OTHER_OR_UNKNOWN" + : "EXPECTED_LOCAL_STORAGE", + ); + } } } @@ -487,6 +503,50 @@ describe("browser cross-context invalidation transport", () => { second.close(); }); + it("rejects storage pulses from another or unknown storage area", () => { + const storageBus = new FakeStorageBus(); + const publisherStorage = storageBus.createEndpoint(); + const receiverStorage = storageBus.createEndpoint(); + const storageOnly = { + createBroadcastChannel: () => { + throw new DOMException("Denied", "SecurityError"); + }, + }; + const publisher = createBrowserCrossContextInvalidation( + dependencies("tab-a", { + ...storageOnly, + storage: publisherStorage.storage, + storageEvents: publisherStorage.target, + }), + ); + const receiver = createBrowserCrossContextInvalidation( + dependencies("tab-b", { + ...storageOnly, + storage: receiverStorage.storage, + storageEvents: receiverStorage.target, + }), + ); + const received = vi.fn(); + receiver.subscribe(received); + + // N-09. sessionStorage and every other Storage area raise the same event, + // so an identical key and value from a foreign area must be ignored. + storageBus.emitAsForeignArea = true; + expect( + publisher.publish({ topic: TOPIC, topicVersion: 1 }), + ).toMatchObject({ ok: true }); + expect(received).not.toHaveBeenCalled(); + + storageBus.emitAsForeignArea = false; + expect( + publisher.publish({ topic: TOPIC, topicVersion: 1 }), + ).toMatchObject({ ok: true }); + expect(received).toHaveBeenCalledOnce(); + + publisher.close(); + receiver.close(); + }); + it("enters explicit local-only degradation when every transport fails", () => { const storageBus = new FakeStorageBus(); const endpoint = storageBus.createEndpoint(); diff --git a/tests/unit/cursor-pagination-runtime.test.ts b/tests/unit/cursor-pagination-runtime.test.ts index c0a84f7..17c8ad7 100644 --- a/tests/unit/cursor-pagination-runtime.test.ts +++ b/tests/unit/cursor-pagination-runtime.test.ts @@ -12,6 +12,45 @@ const profile = { } as const; describe("bounded cursor pagination runtime", () => { + it("returns PAGINATION_ABORTED when a non-cooperative page resolves after abort", async () => { + const controller = new AbortController(); + let releasePage: ((page: unknown) => void) | undefined; + const loadPage = vi.fn( + () => + new Promise((resolve) => { + releasePage = resolve as (page: unknown) => void; + }), + ); + const runtime = createCursorPaginationRuntime({ + definitionId: "bounded", + profile, + loadPage: loadPage as never, + }); + + const loading = runtime.loadAll({ signal: controller.signal }); + await Promise.resolve(); + controller.abort(); + + const result = await loading; + expect(result).toMatchObject({ + ok: false, + error: { kind: "REQUEST_ABORTED", code: "PAGINATION_ABORTED" }, + }); + + // The late page completion must be ignored, not accumulated. + releasePage?.({ + ok: true, + value: { + items: ["late"], + nextCursor: null, + hasMore: false, + snapshotToken: "snapshot-1", + }, + }); + await Promise.resolve(); + expect(loadPage).toHaveBeenCalledOnce(); + }); + it("loads a stable finite chain without exposing cursors in its value", async () => { const loadPage = vi .fn()