diff --git a/docs/operations/adapter-remediation-ledger.md b/docs/operations/adapter-remediation-ledger.md index e5fbc6d..d009458 100644 --- a/docs/operations/adapter-remediation-ledger.md +++ b/docs/operations/adapter-remediation-ledger.md @@ -89,13 +89,13 @@ Rollout state starts at `NOT_STARTED`; documented-unimplemented items start at | ID | Activation | Red test command | Fix commit/PR | Rollout state | Rollback trigger | Evidence | | --- | --- | --- | --- | --- | --- | --- | | STO-01 | OPFS not composed in template; **Critical** for any product writer | `corepack pnpm exec vitest run tests/unit/opfs-byte-store.test.ts tests/unit/opfs-worker-runtime.test.ts tests/unit/indexeddb-opfs-journal.test.ts` | `fix: preserve OPFS recovery authority during cleanup` | `FIXED_NOT_RELEASED` | OPFS reconcile backlog or journal growth | Red 4 new saga cases → green 25/25 across the three OPFS suites; `check:types` PASS (incl. web-worker); `check:browser-file-storage-boundaries` PASS; `lint` PASS; `test:unit` 1511 passed with only the pre-existing environmental `ci-artifact-contract` failures | -| STO-02 | Browser file runtime not composed | `corepack pnpm exec vitest run tests/unit/browser-file-download.test.ts` | — | `NOT_STARTED` | download navigation blocked by canonical target | — | +| STO-02 | Browser file runtime not composed | `corepack pnpm exec vitest run tests/unit/browser-file-download.test.ts` | `fix: execute canonical browser download targets` | `FIXED_NOT_RELEASED` | download navigation blocked by canonical target | Red 2 failed (raw relative href handed to host) → green 17/17 | | STO-03 | Public cache not composed | `corepack pnpm exec vitest run tests/unit/public-response-cache.test.ts` | — | `NOT_STARTED` | composition rejection of an existing policy | — | | STO-04 | Public cache not composed | `corepack pnpm exec vitest run tests/unit/public-response-cache.test.ts` | — | `NOT_STARTED` | restage loop or bandwidth spike | — | | STO-05 | Public cache not composed | `corepack pnpm exec vitest run tests/unit/public-response-cache.test.ts` | — | `NOT_STARTED` | activation permitted without required capability | — | | STO-06 | IndexedDB maintenance | `corepack pnpm exec vitest run tests/unit/indexeddb-maintenance.test.ts` | — | `NOT_STARTED` | migration checkpoint stall | — | | STO-07 | OPFS worker protocol | `corepack pnpm exec vitest run tests/unit/opfs-worker-runtime.test.ts` | — | `NOT_STARTED` | page/worker `INCOMPATIBLE` spike | — | -| STO-08 | Hypothesis; browser characterization required | `corepack pnpm exec playwright test --config playwright.capabilities.config.ts tests/browser-capabilities/browser-files.spec.ts` | — | `NOT_STARTED` | n/a until characterized | — | +| STO-08 | Hypothesis; browser characterization required | `corepack pnpm exec playwright test --config playwright.capabilities.config.ts tests/browser-capabilities/browser-files.spec.ts` | none (source unchanged) | `UNVERIFIED` | n/a until characterized | chromium 2/2 PASS; webkit could not launch (`libevent-2.1-7t64`, `libavif16` missing — environmental). The existing spec does not exercise `Window.showOpenFilePicker`/`showSaveFilePicker`, which need a user gesture and a native dialog, so the receiver-binding hypothesis is **neither reproduced nor refuted**. No `SystemPickerHost` was introduced: the plan forbids implementing an uncharacterized hypothesis as a defect. | | GAP-01 | Documented unimplemented (VD-15) | promotion evidence, not a red test | — | `PROMOTION_BLOCKED` | n/a | — | | GAP-02 | Documented unimplemented (VD-15) | promotion evidence, not a red test | — | `PROMOTION_BLOCKED` | n/a | — | | GAP-03 | Documented unimplemented (VD-15) | promotion evidence, not a red test | — | `PROMOTION_BLOCKED` | n/a | — | diff --git a/src/adapters/browser-files/download-delivery-adapter.ts b/src/adapters/browser-files/download-delivery-adapter.ts index 7a6b4fd..6b25314 100644 --- a/src/adapters/browser-files/download-delivery-adapter.ts +++ b/src/adapters/browser-files/download-delivery-adapter.ts @@ -443,20 +443,23 @@ function browserManagedHandoff(context: Readonly<{ context.options.observer, ); } - const href = capability.value.href; - if ( - !safeBrowserManagedTarget(href, context.baseOrigin, { + const target = resolveBrowserManagedTarget( + capability.value.href, + context.baseOrigin, + { allowCrossOrigin: context.options.allowCrossOriginBrowserHandoff ?? false, allowQuery: context.options.allowBrowserManagedQuery ?? false, - }) - ) { - return observeResult( - browserDataFailure("POLICY_REJECTED", "DOWNLOAD"), - context.options.observer, - ); + }, + ); + if (!target.ok) { + return observeResult(target, context.options.observer); } - context.options.host.handoff(href, context.suggestedFileName); + // The host receives the parsed canonical URL, never the raw string. + context.options.host.handoff( + target.value.absoluteHref, + context.suggestedFileName, + ); return observeResult( browserDataSuccess( Object.freeze({ @@ -1245,28 +1248,45 @@ function validateBrowserManagedCapability( ); } -function safeBrowserManagedTarget( +type ResolvedBrowserManagedTarget = Readonly<{ absoluteHref: string }>; + +/** + * STO-02. Parse once, canonicalize, then execute the canonical value. + * + * Returning a boolean and handing the raw href to the host let the browser + * re-resolve a relative target against `document.baseURI`, so a hostile + * `` could send the navigation to a different origin than the one this + * policy just approved. + */ +function resolveBrowserManagedTarget( href: string, baseOrigin: string, policy: Readonly<{ allowCrossOrigin: boolean; allowQuery: boolean; }>, -): boolean { +): BrowserDataResult { + let base: URL; + let target: URL; try { - const base = new URL(baseOrigin); - const target = new URL(href, base); - return ( - ["http:", "https:"].includes(target.protocol) && - target.username.length === 0 && - target.password.length === 0 && - (policy.allowCrossOrigin || target.origin === base.origin) && - (policy.allowQuery || target.search.length === 0) && - target.hash.length === 0 - ); + base = new URL(baseOrigin); + target = new URL(href, base); } catch { - return false; + return browserDataFailure("POLICY_REJECTED", "DOWNLOAD"); } + if ( + !["http:", "https:"].includes(target.protocol) || + target.username.length > 0 || + target.password.length > 0 || + (!policy.allowCrossOrigin && target.origin !== base.origin) || + (!policy.allowQuery && target.search.length > 0) || + target.hash.length > 0 + ) { + return browserDataFailure("POLICY_REJECTED", "DOWNLOAD"); + } + return browserDataSuccess( + Object.freeze({ absoluteHref: target.href }), + ); } function safeOpaqueId(value: unknown): value is string { diff --git a/tests/unit/browser-file-download.test.ts b/tests/unit/browser-file-download.test.ts index 95c7912..e56232c 100644 --- a/tests/unit/browser-file-download.test.ts +++ b/tests/unit/browser-file-download.test.ts @@ -248,12 +248,49 @@ describe("browser download delivery", () => { ok: true, value: { kind: "BROWSER_HANDOFF", transferId: "transfer:1" }, }); + // STO-02. Parse once, then execute exactly what was validated. expect(handoff).toHaveBeenCalledWith( - "/downloads/artifact-1", + "https://app.example/downloads/artifact-1", "invoice_exe.pdf", ); }); + it("executes the canonical target instead of a document-base-relative href", async () => { + const handoff = vi.fn(); + const adapter = createDownloadDeliveryAdapter({ + ...HARD_LIMITS, + policies, + host: { handoff }, + // A relative href the host would otherwise resolve against a hostile + // document . + browserManagedCapabilities: capabilityResolver(() => "downloads/a"), + baseOrigin: "https://app.example", + createTransferId: () => "transfer:1", + userActivation: { isActive: true }, + }); + + expect( + await adapter.deliver( + deliveryInput( + { + kind: "BROWSER_MANAGED_RESOURCE", + resourceId: "artifact-1", + capabilityReceipt, + }, + "BROWSER_MANAGED", + ), + ), + ).toMatchObject({ ok: true }); + expect(handoff).toHaveBeenCalledWith( + "https://app.example/downloads/a", + "invoice_exe.pdf", + ); + for (const [href] of handoff.mock.calls) { + expect(String(href).startsWith("https://app.example/")).toBe(true); + expect(String(href)).not.toContain("evil.example"); + } + }); + it("rejects cross-origin or query-bearing browser-managed targets", async () => { const handoff = vi.fn(); const adapter = createDownloadDeliveryAdapter({