fix: execute canonical browser download targets
Replace the boolean browser-managed target validator with resolveBrowserManagedTarget, which returns the parsed canonical absolute URL, and hand that exact value to the host. Previously the raw href was passed on, so a relative target was re-resolved against document.baseURI and a hostile <base> could send the navigation to an origin the policy never approved. STO-08 stays UNVERIFIED: the capability spec does not exercise the system picker, so the receiver-binding hypothesis is neither reproduced nor refuted and no source change was made for it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
618da9abf5
commit
ba79060a83
@@ -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 |
|
| 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-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-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-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-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-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-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-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-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 | — |
|
| GAP-03 | Documented unimplemented (VD-15) | promotion evidence, not a red test | — | `PROMOTION_BLOCKED` | n/a | — |
|
||||||
|
|||||||
@@ -443,20 +443,23 @@ function browserManagedHandoff(context: Readonly<{
|
|||||||
context.options.observer,
|
context.options.observer,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
const href = capability.value.href;
|
const target = resolveBrowserManagedTarget(
|
||||||
if (
|
capability.value.href,
|
||||||
!safeBrowserManagedTarget(href, context.baseOrigin, {
|
context.baseOrigin,
|
||||||
|
{
|
||||||
allowCrossOrigin:
|
allowCrossOrigin:
|
||||||
context.options.allowCrossOriginBrowserHandoff ?? false,
|
context.options.allowCrossOriginBrowserHandoff ?? false,
|
||||||
allowQuery: context.options.allowBrowserManagedQuery ?? false,
|
allowQuery: context.options.allowBrowserManagedQuery ?? false,
|
||||||
})
|
},
|
||||||
) {
|
);
|
||||||
return observeResult(
|
if (!target.ok) {
|
||||||
browserDataFailure("POLICY_REJECTED", "DOWNLOAD"),
|
return observeResult(target, context.options.observer);
|
||||||
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(
|
return observeResult(
|
||||||
browserDataSuccess(
|
browserDataSuccess(
|
||||||
Object.freeze({
|
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
|
||||||
|
* `<base>` could send the navigation to a different origin than the one this
|
||||||
|
* policy just approved.
|
||||||
|
*/
|
||||||
|
function resolveBrowserManagedTarget(
|
||||||
href: string,
|
href: string,
|
||||||
baseOrigin: string,
|
baseOrigin: string,
|
||||||
policy: Readonly<{
|
policy: Readonly<{
|
||||||
allowCrossOrigin: boolean;
|
allowCrossOrigin: boolean;
|
||||||
allowQuery: boolean;
|
allowQuery: boolean;
|
||||||
}>,
|
}>,
|
||||||
): boolean {
|
): BrowserDataResult<ResolvedBrowserManagedTarget> {
|
||||||
|
let base: URL;
|
||||||
|
let target: URL;
|
||||||
try {
|
try {
|
||||||
const base = new URL(baseOrigin);
|
base = new URL(baseOrigin);
|
||||||
const target = new URL(href, base);
|
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
|
|
||||||
);
|
|
||||||
} catch {
|
} 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 {
|
function safeOpaqueId(value: unknown): value is string {
|
||||||
|
|||||||
@@ -248,12 +248,49 @@ describe("browser download delivery", () => {
|
|||||||
ok: true,
|
ok: true,
|
||||||
value: { kind: "BROWSER_HANDOFF", transferId: "transfer:1" },
|
value: { kind: "BROWSER_HANDOFF", transferId: "transfer:1" },
|
||||||
});
|
});
|
||||||
|
// STO-02. Parse once, then execute exactly what was validated.
|
||||||
expect(handoff).toHaveBeenCalledWith(
|
expect(handoff).toHaveBeenCalledWith(
|
||||||
"/downloads/artifact-1",
|
"https://app.example/downloads/artifact-1",
|
||||||
"invoice_exe.pdf",
|
"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 <base href="https://evil.example/">.
|
||||||
|
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 () => {
|
it("rejects cross-origin or query-bearing browser-managed targets", async () => {
|
||||||
const handoff = vi.fn();
|
const handoff = vi.fn();
|
||||||
const adapter = createDownloadDeliveryAdapter({
|
const adapter = createDownloadDeliveryAdapter({
|
||||||
|
|||||||
Reference in New Issue
Block a user