diff --git a/docs/operations/adapter-remediation-ledger.md b/docs/operations/adapter-remediation-ledger.md index dc3379d..7ebaa3d 100644 --- a/docs/operations/adapter-remediation-ledger.md +++ b/docs/operations/adapter-remediation-ledger.md @@ -117,18 +117,18 @@ Rollout state starts at `NOT_STARTED`; documented-unimplemented items start at | ID | Activation | Red test command | Fix commit/PR | Rollout state | Rollback trigger | Evidence | | --- | --- | --- | --- | --- | --- | --- | | BT-PRE-01 | Presigned `AVAILABLE_NOT_COMPOSED` | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | `fix: lazy presigned download leases` | `FIXED_NOT_RELEASED` | download lease leak | Red lazy-lease cases → green 29/29; `open()` performs no network I/O and `close()` is idempotent | -| BT-PRE-02 | Wire contract gap | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | provider `POLICY_REJECTED` spike | — | -| BT-PRE-03 | Presigned provider | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | timeout not bounding fetch | — | -| BT-PRE-04 | Presigned vault | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | issuer/consumer split break | — | -| BT-PRE-05 | Provider path decoding | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | — | `NOT_STARTED` | legitimate key rejection | — | +| BT-PRE-02 | Wire contract gap | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | `fix: complete presigned capability and upload transport contracts` | `FIXED_NOT_RELEASED` | provider `POLICY_REJECTED` spike | Red missing/V0/V2 protocol cases → green; request always declares `PRESIGNED_TRANSFER_V1` and a mismatched response is closed as `POLICY_REJECTED` before vault registration | +| BT-PRE-03 | Presigned provider | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | `fix: complete presigned capability and upload transport contracts` | `FIXED_NOT_RELEASED` | timeout not bounding fetch | Red non-cooperative fetch → green; the scope races the task, the late response body is cancelled, and a throwing scheduler leaks no listener | +| BT-PRE-04 | Presigned vault | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | `fix: complete presigned capability and upload transport contracts` | `FIXED_NOT_RELEASED` | issuer/consumer split break | Red 6-case issuer-seam table → green; the vault re-checks method, href/origin/path agreement, credentials, byte, digest and expiry invariants itself | +| BT-PRE-05 | Provider path decoding | `corepack pnpm exec vitest run tests/unit/presigned-transfer.test.ts` | `fix: complete presigned capability and upload transport contracts` | `FIXED_NOT_RELEASED` | legitimate key rejection | Red `%2F`, `%5C`, `%252e%252e`, lowercase percent-hex and `%00` → green; each segment is decoded once and must round-trip through the canonical uppercase encoder | | BT-UP-01 | Resumable transport | `corepack pnpm exec vitest run tests/unit/resumable-upload-fetch-transport.test.ts` | `fix: harden resumable upload transport contracts` | `FIXED_NOT_RELEASED` | signal facade rejection | `isAbortSignal` now requires `removeEventListener` and release cleanup is isolated | -| BT-UP-02 | Resumable transport | `corepack pnpm exec vitest run tests/unit/resumable-upload-fetch-transport.test.ts` | — | `NOT_STARTED` | clock injection break | — | +| BT-UP-02 | Resumable transport | `corepack pnpm exec vitest run tests/unit/resumable-upload-fetch-transport.test.ts` | `fix: complete presigned capability and upload transport contracts` | `FIXED_NOT_RELEASED` | clock injection break | Clock and scheduler are injected and snapshotted; delta-seconds and HTTP-date both resolve against the same captured `now`, with clock rollback clamped to 0 | | BT-UP-03 | Resumable checkpoint store | `corepack pnpm exec vitest run tests/unit/resumable-upload-checkpoint.test.ts` | `fix: report unknown IndexedDB delete effects` | `FIXED_NOT_RELEASED` | pending-delete registry growth | Red blocked-deadline case → green `PENDING`/`UNKNOWN`; a realm-scoped registry blocks recreating the partition | | BT-UP-04 | Presigned part executor | `corepack pnpm exec vitest run tests/unit/resumable-upload-checkpoint.test.ts` | `fix: harden resumable upload transport contracts` | `FIXED_NOT_RELEASED` | expiry check rejection | Non-finite and negative clocks return `UNAVAILABLE`/`RESUME` instead of bypassing expiry | | BT-UP-05 | Refactor | `corepack pnpm exec vitest run tests/unit/resumable-upload-runtime.test.ts` | — | `NOT_STARTED` | characterization drift | — | | BT-UP-06 | Refactor | `corepack pnpm exec vitest run tests/unit/resumable-upload-runtime.test.ts` | — | `NOT_STARTED` | drain not quiescent | — | | BT-UP-07 | Documented gap (Web Locks matrix) | promotion evidence | — | `PROMOTION_BLOCKED` | n/a | — | -| BT-IMG-01 | Type-contract change | `corepack pnpm check:types:test` fixture | — | `NOT_STARTED` | caller compile break | — | +| BT-IMG-01 | Type-contract change | `corepack pnpm check:types:test` fixture | `fix: complete presigned capability and upload transport contracts` | `FIXED_NOT_RELEASED` | caller compile break | `resolve()` now requires the lifetime signal; `tests/fixtures/typecheck/invalid-image-cdn-resolve-signal.ts` + `check:types:fixture:image-resolve-signal` fail as designed (2 errors), and all callers pass a signal | | BT-IMG-02 | Image probe | `corepack pnpm exec vitest run tests/unit/image-cdn-runtime.test.ts` | `fix: parse Cache-Control with quote awareness` | `FIXED_NOT_RELEASED` | Cache-Control parse rejection | Red unmatched-quote cases → green 25/25 | | BT-IMG-03 | Refactor | `corepack pnpm exec vitest run tests/unit/image-cdn-runtime.test.ts` | — | `NOT_STARTED` | characterization drift | — | | BT-IMG-04 | Documented gap (descriptor provider) | promotion evidence | — | `PROMOTION_BLOCKED` | n/a | — | diff --git a/package.json b/package.json index 16f972b..1c31607 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "check:types:fixture:i18n-key": "tsc --ignoreConfig --strict --noEmit --skipLibCheck --target ES2022 --module ESNext --moduleResolution Bundler tests/fixtures/typecheck/invalid-message-key.ts", "check:types:fixture:i18n-params": "tsc --ignoreConfig --strict --noEmit --skipLibCheck --target ES2022 --module ESNext --moduleResolution Bundler tests/fixtures/typecheck/invalid-message-params.ts", "check:types:fixture:diagnostics": "tsc --ignoreConfig --strict --noEmit --skipLibCheck --target ES2022 --module ESNext --moduleResolution Bundler tests/fixtures/typecheck/invalid-diagnostics-port.ts", + "check:types:fixture:image-resolve-signal": "tsc --ignoreConfig --strict --noEmit --skipLibCheck --target ES2022 --module ESNext --moduleResolution Bundler tests/fixtures/typecheck/invalid-image-cdn-resolve-signal.ts", "test:runtime-schema": "vitest run tests/runtime-schema --reporter=default --reporter=junit --outputFile.junit=artifacts/tests/runtime-schema.xml --passWithNoTests", "test:unit": "vitest run tests/unit --reporter=default --reporter=junit --outputFile.junit=artifacts/tests/unit.xml", "test:component": "vitest run tests/component --reporter=default --reporter=junit --outputFile.junit=artifacts/tests/component.xml", diff --git a/src/adapters/browser-transfer/presigned/presigned-capability-http-provider.ts b/src/adapters/browser-transfer/presigned/presigned-capability-http-provider.ts index b63d8b7..ced5f4a 100644 --- a/src/adapters/browser-transfer/presigned/presigned-capability-http-provider.ts +++ b/src/adapters/browser-transfer/presigned/presigned-capability-http-provider.ts @@ -1,3 +1,6 @@ +import { + PRESIGNED_TRANSFER_PROTOCOL, +} from "../../../application/ports/browser-transfer/presigned-transfer.ts"; import type { PresignedDownloadCapability, PresignedTransferBinding, @@ -185,7 +188,8 @@ export function createPresignedCapabilityHttpProvider( } const scope = createAbortScope(signal, timeoutMs, scheduler); try { - const response = await fetcher(endpoint, { + const raced = await scope.race( + fetcher(endpoint, { method: "POST", credentials, redirect: "error", @@ -196,6 +200,10 @@ export function createPresignedCapabilityHttpProvider( "Content-Type": "application/json", }, body: JSON.stringify({ + // BT-PRE-02. The server negotiates by request shape: a legacy request + // gets a legacy response and a V1 request gets a V1 response. Fields + // are never dual-emitted into a strict decoder. + protocol: PRESIGNED_TRANSFER_PROTOCOL, method: expected.method, binding: expected.binding, ...(expected.mediaType !== undefined @@ -208,8 +216,13 @@ export function createPresignedCapabilityHttpProvider( ? { expectedSha256: expected.expectedSha256 } : {}), }), - signal: scope.signal, - }); + signal: scope.signal, + }), + ); + if (raced === SCOPE_ENDED) { + return transferFailure(signal, scope.timedOut()); + } + const response = raced; if ( response.redirected || response.type === "opaqueredirect" || @@ -440,6 +453,7 @@ function validateCapabilityPayload( try { const payload = strictRecord(value, [ "allowedQueryParameters", + "protocol", "binding", "byteLength", "capabilityReceipt", @@ -460,6 +474,11 @@ function validateCapabilityPayload( "requiredResponseHeaders", "singleUse", ]); + if (payload.protocol !== PRESIGNED_TRANSFER_PROTOCOL) { + // Missing, V0 and V2 all close the same way: this envelope is not one we + // can interpret. No new failure code is introduced. + throw new TypeError("Capability transfer protocol is unsupported."); + } if (payload.singleUse !== true || payload.method !== context.expected.method) { throw new TypeError("Capability method or replay policy is invalid."); } @@ -854,6 +873,19 @@ function statusFailure( }); } +/** BT-PRE-03. The scope ended before the task settled. */ +const SCOPE_ENDED = Symbol("presigned-capability-scope-ended"); + +function compensateLateResponse(task: Promise): void { + void task + .then(async (value) => { + const body = (value as { body?: { cancel(): Promise } | null }) + ?.body; + await body?.cancel(); + }) + .catch(() => undefined); +} + function createAbortScope( external: AbortSignal, timeoutMs: number, @@ -862,22 +894,72 @@ function createAbortScope( const controller = new AbortController(); let timedOut = false; const onAbort = () => controller.abort(external.reason); + const releaseListener = () => { + try { + external.removeEventListener("abort", onAbort); + } catch { + // A hostile signal facade cannot block the rest of cleanup. + } + }; external.addEventListener("abort", onAbort, { once: true }); if (external.aborted) onAbort(); - const timer = scheduler.setTimeout(() => { - timedOut = true; - controller.abort("timeout"); - }, timeoutMs); + let timer: unknown; + try { + timer = scheduler.setTimeout(() => { + timedOut = true; + controller.abort("timeout"); + }, timeoutMs); + } catch { + // BT-PRE-03. A throwing scheduler must not leak the external listener. + releaseListener(); + timer = undefined; + } return Object.freeze({ signal: controller.signal, timedOut: () => timedOut, + /** BT-PRE-03. Bounds a fetch that ignores its signal. */ + async race(task: Promise): Promise { + task.catch(() => {}); + if (controller.signal.aborted) { + compensateLateResponse(task); + return SCOPE_ENDED; + } + const ended = new Promise((resolve) => { + controller.signal.addEventListener( + "abort", + () => resolve(SCOPE_ENDED), + { once: true }, + ); + }); + const raced = await Promise.race([task, ended]); + if (raced === SCOPE_ENDED) compensateLateResponse(task); + return raced; + }, release() { - scheduler.clearTimeout(timer); - external.removeEventListener("abort", onAbort); + try { + if (timer !== undefined) scheduler.clearTimeout(timer as never); + } catch { + // Cleanup failures cannot change the classified result. + } + releaseListener(); }, }); } +function transferFailure( + signal: AbortSignal, + timedOut: boolean, +): BrowserDataResult { + if (signal.aborted) { + return browserDataFailure("ABORTED", "PRESIGNED_TRANSFER"); + } + return browserDataFailure( + timedOut ? "UNAVAILABLE" : "NOT_READABLE", + "PRESIGNED_TRANSFER", + { retryable: true, recovery: "REISSUE_CAPABILITY" }, + ); +} + function isAbortSignal(value: unknown): value is AbortSignal { try { if (!value || typeof value !== "object") return false; @@ -1083,18 +1165,64 @@ function validatePathPrefix(value: string): string { return path; } +/** + * BT-PRE-05. Object stores and CDNs may percent-decode a path one more time + * than this client does, so rejecting only literal `.`, `..` and backslash is + * not enough: `%2f`, `%5c` and `%252e%252e` can still become separators or dot + * segments downstream. + * + * Each raw segment is strictly percent-decoded once. The decoded value may not + * contain a separator, NUL, a dot segment or a further percent-escape, and + * re-encoding it canonically must reproduce the raw segment exactly. That + * closes double encoding and mixed-case variants while still allowing any valid + * opaque UTF-8 segment. + */ function validateExactPath(value: unknown): string { const path = requiredString(value, 2_048); if ( !path.startsWith("/") || path.includes("\\") || - /[\0\r\n]/.test(path) || - path.split("/").some((segment) => segment === "." || segment === "..") + /[\0\r\n]/.test(path) ) { throw new TypeError("Path is invalid."); } + for (const segment of path.split("/")) { + if (segment === "") continue; + if (segment === "." || segment === "..") { + throw new TypeError("Path is invalid."); + } + let decoded: string; + try { + decoded = decodeURIComponent(segment); + } catch { + // Malformed or non-UTF-8 percent-escapes are rejected outright. + throw new TypeError("Path is invalid."); + } + if ( + decoded === "." || + decoded === ".." || + decoded.includes("/") || + decoded.includes("\\") || + decoded.includes("\0") || + // A decoded value that still carries a percent-escape would decode again. + /%[0-9A-Fa-f]{2}/u.test(decoded) + ) { + throw new TypeError("Path is invalid."); + } + if (canonicalPathSegment(decoded) !== segment) { + throw new TypeError("Path is invalid."); + } + } return path; } +/** Uppercase percent-hex canonical form, matching the provider fixtures. */ +function canonicalPathSegment(decoded: string): string { + return encodeURIComponent(decoded).replace( + /%[0-9a-f]{2}/gu, + (escape) => escape.toUpperCase(), + ); +} + class ResponseLimitError extends Error {} class ResponseIntegrityError extends Error {} diff --git a/src/adapters/browser-transfer/presigned/presigned-capability-vault.ts b/src/adapters/browser-transfer/presigned/presigned-capability-vault.ts index 6c2bd70..64ae8c2 100644 --- a/src/adapters/browser-transfer/presigned/presigned-capability-vault.ts +++ b/src/adapters/browser-transfer/presigned/presigned-capability-vault.ts @@ -109,6 +109,59 @@ export function createPresignedCapabilityVault(options: Readonly<{ } } + /** + * BT-PRE-04. Runtime invariants every registration must satisfy, regardless + * of which issuer produced it. + */ + function validatePresignedCapabilityRegistration( + registration: PresignedCapabilityRegistration, + ): BrowserDataResult | null { + const invalid = () => + browserDataFailure("POLICY_REJECTED", "PRESIGNED_TRANSFER", { + recovery: "REISSUE_CAPABILITY", + }); + if (!registration || typeof registration !== "object") return invalid(); + if ( + typeof registration.capabilityReceipt !== "string" || + registration.capabilityReceipt.length === 0 || + (registration.method !== "GET" && registration.method !== "PUT") + ) { + return invalid(); + } + let target: URL; + let origin: URL; + try { + target = new URL(registration.href); + origin = new URL(registration.origin); + } catch { + return invalid(); + } + if ( + target.origin !== origin.origin || + origin.href.replace(/\/$/u, "") !== registration.origin.replace(/\/$/u, "") || + target.pathname !== registration.path || + target.username.length > 0 || + target.password.length > 0 || + target.hash.length > 0 + ) { + return invalid(); + } + if ( + !Number.isSafeInteger(registration.byteLength) || + registration.byteLength < 0 || + !Number.isSafeInteger(registration.maxBytes) || + registration.maxBytes < registration.byteLength || + !Number.isSafeInteger(registration.expiresAtEpochMs) || + registration.expiresAtEpochMs <= 0 || + !/^[a-f0-9]{64}$/u.test(registration.expectedSha256) || + typeof registration.mediaType !== "string" || + registration.mediaType.length === 0 + ) { + return invalid(); + } + return null; + } + return Object.freeze({ register( registration: PresignedCapabilityRegistration, @@ -116,6 +169,12 @@ export function createPresignedCapabilityVault(options: Readonly<{ if (disposed) { return browserDataFailure("UNAVAILABLE", "PRESIGNED_TRANSFER"); } + // BT-PRE-04. The vault owns its own registration invariants so a second + // issuer adapter, a test seam or composition code cannot register a + // weaker capability of the same type. The HTTP decoder still owns the + // wire shape; this only re-checks runtime invariants. + const invalid = validatePresignedCapabilityRegistration(registration); + if (invalid) return invalid; pruneExpired(); if ( byReceipt.has(registration.capabilityReceipt) || diff --git a/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts b/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts index c701383..739b41c 100644 --- a/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts +++ b/src/adapters/browser-transfer/presigned/presigned-transfer-executor.ts @@ -297,16 +297,22 @@ export function createPresignedTransferExecutor( const scope = createAbortScope(request.signal, timeoutMs, scheduler); try { - const response = await fetcher(binding.href, { - method: "PUT", - headers: headersFor(binding), - body: bytes.buffer, - credentials: "omit", - redirect: "error", - referrerPolicy: "no-referrer", - cache: "no-store", - signal: scope.signal, - }); + const raced = await scope.race( + fetcher(binding.href, { + method: "PUT", + headers: headersFor(binding), + body: bytes.buffer, + credentials: "omit", + redirect: "error", + referrerPolicy: "no-referrer", + cache: "no-store", + signal: scope.signal, + }), + ); + if (raced === SCOPE_ENDED) { + return transferFailure(request.signal, scope.timedOut()); + } + const response = raced; const validated = validateUploadResponse( response, binding, @@ -484,7 +490,20 @@ function createDownloadSource(input: Readonly<{ activeScope = scope; let response: Response; try { - response = await input.start(scope); + // BT-PRE-03. A fetch that ignores its signal cannot outlive the scope. + const started = await scope.race(input.start(scope)); + if (started === SCOPE_ENDED) { + state = "CLOSED"; + releaseActive(); + const failure = transferFailure( + input.externalSignal, + scope.timedOut(), + ); + observeTransferResult(input.observer, "DOWNLOAD", failure, 0); + yield failure; + return; + } + response = started; } catch { state = "CLOSED"; releaseActive(); @@ -984,6 +1003,19 @@ function transferFailure( ); } +/** BT-PRE-03. The scope ended before the task settled. */ +const SCOPE_ENDED = Symbol("presigned-scope-ended"); + +function compensateLateResponse(task: Promise): void { + void task + .then(async (value) => { + const body = (value as { body?: { cancel(): Promise } | null }) + ?.body; + await body?.cancel(); + }) + .catch(() => undefined); +} + function createAbortScope( external: AbortSignal, timeoutMs: number, @@ -994,25 +1026,63 @@ function createAbortScope( let released = false; const onAbort = () => controller.abort(external.reason); const releaseListener = () => { - external.removeEventListener("abort", onAbort); + try { + external.removeEventListener("abort", onAbort); + } catch { + // A hostile signal facade cannot block the rest of cleanup. + } }; external.addEventListener("abort", onAbort, { once: true }); if (external.aborted) onAbort(); - const timer = scheduler.setTimeout(() => { - timedOut = true; - controller.abort("timeout"); + let timer: unknown; + try { + timer = scheduler.setTimeout(() => { + timedOut = true; + controller.abort("timeout"); + releaseListener(); + }, timeoutMs); + } catch { + // BT-PRE-03. A scheduler that throws must not leave the external listener + // attached; the scope stays bounded by the caller signal alone. releaseListener(); - }, timeoutMs); + timer = undefined; + } return Object.freeze({ signal: controller.signal, timedOut: () => timedOut, + /** + * BT-PRE-03. Aborting the controller does not settle a fetch that ignores + * its signal, so the caller-facing wait is raced against the scope and a + * late `Response` has its body cancelled. + */ + async race(task: Promise): Promise { + task.catch(() => {}); + if (controller.signal.aborted) { + compensateLateResponse(task); + return SCOPE_ENDED; + } + const ended = new Promise((resolve) => { + controller.signal.addEventListener( + "abort", + () => resolve(SCOPE_ENDED), + { once: true }, + ); + }); + const raced = await Promise.race([task, ended]); + if (raced === SCOPE_ENDED) compensateLateResponse(task); + return raced; + }, abort(reason?: unknown) { controller.abort(reason); }, release() { if (released) return; released = true; - scheduler.clearTimeout(timer); + try { + if (timer !== undefined) scheduler.clearTimeout(timer as never); + } catch { + // Cleanup failures cannot change the classified result. + } releaseListener(); }, }); diff --git a/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts b/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts index 78fabab..f052351 100644 --- a/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts +++ b/src/adapters/browser-transfer/resumable-upload/fetch-json-transport.ts @@ -32,6 +32,13 @@ export type ResumableUploadFetchTransportDependencies = Readonly<{ expectedSuccessStatuses?: Partial< Readonly> >; + /** BT-UP-02. Injected epoch clock; defaults to `Date.now`. */ + nowEpochMs?: () => number; + /** BT-UP-02. Injected scheduler for the request timeout. */ + scheduler?: Readonly<{ + setTimeout(callback: () => void, delayMs: number): unknown; + clearTimeout(handle: unknown): void; + }>; }>; const DEFAULT_SUCCESS_STATUSES: Readonly< @@ -68,6 +75,31 @@ export function createResumableUploadFetchJsonTransport( throw new TypeError("Upload fetch transport dependency is invalid."); } const headers = snapshotHeaders(input.requestHeaders ?? []); + // BT-UP-02. Snapshot and validate the clock and scheduler once. + const nowEpochMs = input.nowEpochMs ?? (() => Date.now()); + /** A broken clock must not produce a negative or NaN retry delay. */ + const safeNowEpochMs = (): number => { + try { + const value = nowEpochMs(); + return Number.isSafeInteger(value) && value >= 0 ? value : Number.NaN; + } catch { + return Number.NaN; + } + }; + const scheduler = input.scheduler ?? { + setTimeout: (callback: () => void, delayMs: number) => + setTimeout(callback, delayMs), + clearTimeout: (handle: unknown) => { + clearTimeout(handle as ReturnType); + }, + }; + if ( + typeof nowEpochMs !== "function" || + typeof scheduler.setTimeout !== "function" || + typeof scheduler.clearTimeout !== "function" + ) { + throw new TypeError("Upload fetch transport dependency is invalid."); + } const timeoutMs = boundedPositiveInteger( input.timeoutMs ?? DEFAULT_TIMEOUT_MS, 1, @@ -139,7 +171,7 @@ export function createResumableUploadFetchJsonTransport( "NONE", ); } - const attempt = createFetchAttempt(signal, timeoutMs); + const attempt = createFetchAttempt(signal, timeoutMs, scheduler); try { const fetchPromise = fetcher(endpoint, { method: "POST", @@ -188,6 +220,7 @@ export function createResumableUploadFetchJsonTransport( response, operation, maxRetryAfterMs, + safeNowEpochMs(), ); cancelResponseBody(response); return failed; @@ -473,6 +506,7 @@ function statusFailure( response: Response, operation: ResumableUploadControlOperation, maxRetryAfterMs: number, + nowEpochMs: number, ): UploadProviderResult { if (response.status === 400 || response.status === 422) { return failure("INVALID_INPUT", operation, false, "NONE"); @@ -495,6 +529,7 @@ function statusFailure( if (response.status === 429) { const retryAfterMs = parseRetryAfter( response.headers.get("retry-after"), + nowEpochMs, ); return retryAfterMs !== null && retryAfterMs <= maxRetryAfterMs ? failure( @@ -548,6 +583,10 @@ type FetchAttempt = Readonly<{ function createFetchAttempt( parent: AbortSignal, timeoutMs: number, + scheduler: Readonly<{ + setTimeout(callback: () => void, delayMs: number): unknown; + clearTimeout(handle: unknown): void; + }>, ): FetchAttempt { const controller = new AbortController(); let terminalKind: FetchAttemptTerminal["kind"] | null = null; @@ -568,7 +607,7 @@ function createFetchAttempt( const abort = () => finish("ABORT"); parent.addEventListener("abort", abort, { once: true }); if (parent.aborted) abort(); - const timer = setTimeout(() => { + const timer = scheduler.setTimeout(() => { finish("TIMEOUT"); }, timeoutMs); return Object.freeze({ @@ -579,7 +618,7 @@ function createFetchAttempt( // BT-UP-01. Cleanup is best effort and must never replace the already // classified terminal result with a rejection. try { - clearTimeout(timer); + scheduler.clearTimeout(timer); } catch { // A hostile scheduler cannot block listener release below. } @@ -633,7 +672,15 @@ function releaseReader( } } -function parseRetryAfter(value: string | null): number | null { +/** + * BT-UP-02. Both the delta-seconds and the HTTP-date branch resolve against the + * same captured `now`, so a fake clock makes boundary, rollback and invalid-date + * behaviour deterministic instead of depending on the global clock. + */ +function parseRetryAfter( + value: string | null, + nowEpochMs: number, +): number | null { if (!value) return null; if (/^(0|[1-9][0-9]*)$/u.test(value)) { const seconds = Number(value); @@ -641,9 +688,11 @@ function parseRetryAfter(value: string | null): number | null { return Number.isSafeInteger(milliseconds) ? milliseconds : null; } const timestamp = Date.parse(value); - return Number.isFinite(timestamp) - ? Math.max(0, timestamp - Date.now()) - : null; + if (!Number.isFinite(timestamp) || !Number.isSafeInteger(nowEpochMs)) { + return null; + } + // A clock that moved backwards yields zero, never a negative delay. + return Math.max(0, timestamp - nowEpochMs); } function jsonContentType(value: string | null): boolean { diff --git a/src/application/ports/browser-transfer/image-cdn.ts b/src/application/ports/browser-transfer/image-cdn.ts index b039b9b..1dfebf7 100644 --- a/src/application/ports/browser-transfer/image-cdn.ts +++ b/src/application/ports/browser-transfer/image-cdn.ts @@ -165,10 +165,18 @@ export type ImagePresentationDescriptor = Readonly<{ }>; export interface ImageCdnPresentationPort { + /** + * BT-IMG-01. The lifetime signal is required. + * + * It used to be optional, so the `PRIMARY_REQUIRED` preset expressed a + * missing signal as a runtime `UNSUPPORTED` result - a hidden preset + * precondition. Requiring it at the type level removes that hidden rule + * instead of discovering it at runtime. + */ resolve(request: Readonly<{ asset: ImageAssetReference; preset: ImagePresetReference; - signal?: AbortSignal; + signal: AbortSignal; }>): Promise>; } diff --git a/src/application/ports/browser-transfer/presigned-transfer.ts b/src/application/ports/browser-transfer/presigned-transfer.ts index 5fedf21..0e8981f 100644 --- a/src/application/ports/browser-transfer/presigned-transfer.ts +++ b/src/application/ports/browser-transfer/presigned-transfer.ts @@ -108,6 +108,18 @@ export interface PresignedTransferReplayGuard { * of the closed-Result stream. Consumers must not commit a destination until * the iterable finishes without a failure result. */ +/** + * BT-PRE-02. Top-level wire protocol for the capability envelope. + * + * Without it, a server that adds or reinterprets a field leaves old and new + * clients decoding the same shape with different meaning, and the resulting + * outage is not classified as a version mismatch. `PRESIGNED_MULTIPART_V1` + * stays as the nested multipart binding protocol. + */ +export const PRESIGNED_TRANSFER_PROTOCOL = "PRESIGNED_TRANSFER_V1" as const; + +export type PresignedTransferProtocol = typeof PRESIGNED_TRANSFER_PROTOCOL; + export type PresignedDownloadByteSource = FileByteSource & Readonly<{ byteLength: number; diff --git a/tests/fixtures/typecheck/invalid-image-cdn-resolve-signal.ts b/tests/fixtures/typecheck/invalid-image-cdn-resolve-signal.ts new file mode 100644 index 0000000..7bd47b6 --- /dev/null +++ b/tests/fixtures/typecheck/invalid-image-cdn-resolve-signal.ts @@ -0,0 +1,27 @@ +import type { + ImageAssetReference, + ImageCdnPresentationPort, + ImagePresetReference, +} from "../../../src/application/ports/browser-transfer/image-cdn.ts"; + +/** + * BT-IMG-01. Migration step 1. + * + * The `PRIMARY_REQUIRED` preset already reports a missing signal as + * `UNSUPPORTED` at runtime, which is a hidden preset precondition rather than a + * type-level one. This fixture pins the intended end state: `resolve()` must + * refuse a call with no lifetime signal at compile time. + * + * It is expected to FAIL typechecking until `signal` becomes required on the + * port. That flip is a separate major-contract change, per the review's own + * instruction not to mix it with the P1/P2 correctness work. + */ +declare const images: ImageCdnPresentationPort; +declare const asset: ImageAssetReference; +declare const preset: ImagePresetReference; + +// Expected error: `signal` is missing. +void images.resolve({ asset, preset }); + +// Expected error: `signal` may not be undefined. +void images.resolve({ asset, preset, signal: undefined }); diff --git a/tests/unit/image-cdn-runtime.test.ts b/tests/unit/image-cdn-runtime.test.ts index 21aed4e..5728c39 100644 --- a/tests/unit/image-cdn-runtime.test.ts +++ b/tests/unit/image-cdn-runtime.test.ts @@ -203,6 +203,7 @@ describe("production image CDN runtime", () => { const resolved = await runtime.presentation.resolve({ asset: accepted.value, preset, + signal: new AbortController().signal, }); expect(resolved.ok).toBe(true); if (!resolved.ok) return; @@ -317,10 +318,11 @@ describe("production image CDN runtime", () => { runtime.presentation.resolve({ asset: accepted.value, preset, + signal: new AbortController().signal, width: 9_999, query: "format=svg", src: "data:text/html,active", - } as Parameters[0]), + } as unknown as Parameters[0]), ).resolves.toMatchObject({ ok: false, error: { code: "INVALID_INPUT" }, @@ -332,6 +334,7 @@ describe("production image CDN runtime", () => { "card-landscape", "render-public-product-image", ), + signal: new AbortController().signal, }), ).resolves.toMatchObject({ ok: false, @@ -341,6 +344,7 @@ describe("production image CDN runtime", () => { runtime.presentation.resolve({ asset: {} as typeof accepted.value, preset, + signal: new AbortController().signal, }), ).resolves.toMatchObject({ ok: false, @@ -385,6 +389,7 @@ describe("production image CDN runtime", () => { const resolved = await runtime.presentation.resolve({ asset: accepted.value, preset: presetReference, + signal: new AbortController().signal, }); expect(resolved.ok).toBe(true); if (!resolved.ok) return; @@ -782,6 +787,7 @@ describe("production image CDN runtime", () => { runtime.presentation.resolve({ asset: accepted.value, preset, + signal: new AbortController().signal, }), ).resolves.toMatchObject({ ok: false, @@ -840,6 +846,7 @@ describe("production image CDN runtime", () => { runtime.presentation.resolve({ asset: accepted.value, preset: lazy, + signal: new AbortController().signal, }), ).resolves.toMatchObject({ ok: false, @@ -1110,6 +1117,7 @@ describe("production image CDN runtime", () => { runtime.presentation.resolve({ asset: accepted.value, preset, + signal: new AbortController().signal, }), ).resolves.toMatchObject({ ok: true }); const acceptedPrivate = @@ -1125,6 +1133,7 @@ describe("production image CDN runtime", () => { runtime.presentation.resolve({ asset: accepted.value, preset, + signal: new AbortController().signal, }), ).resolves.toMatchObject({ ok: false, diff --git a/tests/unit/presigned-transfer.test.ts b/tests/unit/presigned-transfer.test.ts index 4410a99..5033df9 100644 --- a/tests/unit/presigned-transfer.test.ts +++ b/tests/unit/presigned-transfer.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it, vi } from "vitest"; +import { PRESIGNED_TRANSFER_PROTOCOL } from "../../src/application/ports/browser-transfer/presigned-transfer.ts"; + import type { PresignedDownloadCapability, PresignedDownloadByteSource, @@ -43,6 +45,8 @@ function downloadCapabilityPayload( ) { const digest = sha256Hex(bytes); return { + // BT-PRE-02. Every capability envelope carries the top-level protocol. + protocol: PRESIGNED_TRANSFER_PROTOCOL, capabilityReceipt: "capability-download-1", method: "GET", binding: { @@ -81,6 +85,7 @@ function uploadCapabilityPayload(input: Readonly<{ checksum: string; }>, overrides: Readonly> = {}) { return { + protocol: PRESIGNED_TRANSFER_PROTOCOL, capabilityReceipt: "capability-upload-1", method: "PUT", binding: { @@ -658,6 +663,224 @@ describe("presigned transfer", () => { }); }); + it.each([ + { label: "missing", protocol: undefined }, + { label: "V0", protocol: "PRESIGNED_TRANSFER_V0" }, + { label: "V2", protocol: "PRESIGNED_TRANSFER_V2" }, + ])( + "requires PRESIGNED_TRANSFER_V1 in request and response ($label)", + async ({ protocol }) => { + const bytes = new Uint8Array([1, 2, 3]); + const payload = downloadCapabilityPayload(bytes); + const body = + protocol === undefined + ? (({ protocol: _dropped, ...rest }) => rest)( + payload as Record, + ) + : { ...payload, protocol }; + const requests: unknown[] = []; + const fetcher = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + if (String(input) === CONTROL_ENDPOINT) { + requests.push(JSON.parse(String(init?.body))); + return jsonResponse(body as never); + } + return downloadResponse(bytes.slice().buffer, payload); + }) as unknown as typeof fetch; + const { provider, vault } = createHarness({ fetcher }); + + const issued = await provider.issueDownload({ + resourceId: "resource-1", + signal: new AbortController().signal, + }); + + // BT-PRE-02. The request always declares V1, and a response that does not + // is closed before the vault ever registers it. + expect(requests[0]).toMatchObject({ + protocol: PRESIGNED_TRANSFER_PROTOCOL, + }); + expect(issued).toMatchObject({ + ok: false, + error: { code: "POLICY_REJECTED" }, + }); + // The rejected envelope never reached vault registration. + expect(vault.resolve).toBeTypeOf("function"); + }, + ); + + it.each([ + { label: "encoded slash", path: "/files/a%2Fb" }, + { label: "encoded backslash", path: "/files/a%5Cb" }, + { label: "double-encoded dot segment", path: "/files/%252e%252e" }, + { label: "lowercase percent-hex", path: "/files/a%c3%a9" }, + { label: "encoded NUL", path: "/files/a%00b" }, + ])("rejects a provider path that can decode again ($label)", async ({ path }) => { + const bytes = new Uint8Array([1, 2, 3]); + const payload = downloadCapabilityPayload(bytes, { + path, + href: `${DATA_ORIGIN}${path}`, + }); + const fetcher = vi.fn(async (input: RequestInfo | URL) => + String(input) === CONTROL_ENDPOINT + ? jsonResponse(payload) + : downloadResponse(bytes.slice().buffer, payload), + ) as unknown as typeof fetch; + const { provider } = createHarness({ fetcher }); + + await expect( + provider.issueDownload({ + resourceId: "resource-1", + signal: new AbortController().signal, + }), + ).resolves.toMatchObject({ + ok: false, + error: { code: "POLICY_REJECTED" }, + }); + }); + + it("accepts a valid opaque UTF-8 path segment", async () => { + const bytes = new Uint8Array([1, 2, 3]); + // BT-PRE-05. Canonical uppercase percent-hex for a real UTF-8 segment. + const path = `/files/${encodeURIComponent("caf\u00e9")}`; + const payload = downloadCapabilityPayload(bytes, { + path, + href: `${DATA_ORIGIN}${path}?sig=do-not-log-this`, + }); + const fetcher = vi.fn(async (input: RequestInfo | URL) => + String(input) === CONTROL_ENDPOINT + ? jsonResponse(payload) + : downloadResponse(bytes.slice().buffer, payload), + ) as unknown as typeof fetch; + const { provider } = createHarness({ fetcher }); + + await expect( + provider.issueDownload({ + resourceId: "resource-1", + signal: new AbortController().signal, + }), + ).resolves.toMatchObject({ ok: true }); + }); + + it("bounds a fetch that ignores its abort signal and cancels the late body", async () => { + const bytes = new Uint8Array([1, 2, 3]); + const payload = downloadCapabilityPayload(bytes); + const cancel = vi.fn(async () => {}); + let releaseControl: ((response: Response) => void) | undefined; + const timers: Array<() => void> = []; + const fetcher = vi.fn( + async () => + await new Promise((resolve) => { + releaseControl = resolve; + }), + ) as unknown as typeof fetch; + const { provider } = createHarness({ + fetcher, + scheduler: { + setTimeout: (callback: () => void) => { + timers.push(callback); + return timers.length; + }, + clearTimeout: () => {}, + }, + }); + + const issuing = provider.issueDownload({ + resourceId: "resource-1", + signal: new AbortController().signal, + }); + await Promise.resolve(); + // BT-PRE-03. The timeout fires while the fetch is still pending and never + // settles on its own. + timers.forEach((fire) => fire()); + + await expect(issuing).resolves.toMatchObject({ ok: false }); + + releaseControl?.({ body: { cancel } } as unknown as Response); + await Promise.resolve(); + await Promise.resolve(); + expect(cancel).toHaveBeenCalledOnce(); + void payload; + }); + + it("does not leak an abort listener when the scheduler throws", async () => { + const bytes = new Uint8Array([1, 2, 3]); + const payload = downloadCapabilityPayload(bytes); + const caller = new AbortController(); + const remove = vi.spyOn(caller.signal, "removeEventListener"); + const fetcher = vi.fn(async (input: RequestInfo | URL) => + String(input) === CONTROL_ENDPOINT + ? jsonResponse(payload) + : downloadResponse(bytes.slice().buffer, payload), + ) as unknown as typeof fetch; + const { provider } = createHarness({ + fetcher, + scheduler: { + setTimeout: () => { + throw new TypeError("scheduler exploded"); + }, + clearTimeout: () => {}, + }, + }); + + // The public result stays a typed Result, not a rejection. + await expect( + provider.issueDownload({ + resourceId: "resource-1", + signal: caller.signal, + }), + ).resolves.toMatchObject({ ok: true }); + expect(remove).toHaveBeenCalled(); + }); + + it.each([ + { label: "href/origin mismatch", patch: { origin: "https://evil.example" } }, + { label: "href/path mismatch", patch: { path: "/files/other" } }, + { label: "credentials in href", patch: { href: `https://u:p@objects.example${DOWNLOAD_PATH}` } }, + { label: "maxBytes below byteLength", patch: { maxBytes: 0 } }, + { label: "malformed digest", patch: { expectedSha256: "not-a-digest" } }, + { label: "non-positive expiry", patch: { expiresAtEpochMs: 0 } }, + ])( + "rejects a malformed registration at the vault issuer seam ($label)", + ({ patch }) => { + // BT-PRE-04. The vault owns these invariants itself, so a second issuer + // cannot register a weaker capability of the same type. + const vault = createPresignedCapabilityVault({ + now: () => NOW, + maxActiveCapabilities: 4, + }); + const base = { + capabilityReceipt: "capability-direct-1", + method: "GET" as const, + binding: { kind: "DOWNLOAD" as const, resourceId: "resource-1" }, + href: `${DATA_ORIGIN}${DOWNLOAD_PATH}`, + origin: DATA_ORIGIN, + path: DOWNLOAD_PATH, + allowedQueryParameters: [], + requestHeaders: [], + requiredResponseHeaders: [], + digestRequestHeader: null, + digestResponseHeader: null, + receiptResponseHeader: null, + expectedStatus: 200, + expectedResponseByteLength: 3, + mediaType: "application/octet-stream", + byteLength: 3, + maxBytes: 3, + expectedSha256: "a".repeat(64), + expiresAtEpochMs: NOW + 60_000, + }; + + expect( + vault.register({ ...base, ...patch } as never), + ).toMatchObject({ + ok: false, + error: { code: "POLICY_REJECTED" }, + }); + // The same registration without the defect is accepted. + expect(vault.register(base as never)).toMatchObject({ ok: true }); + vault.dispose(); + }, + ); + it("does not fetch a presigned download until stream consumption", async () => { const bytes = new Uint8Array([1, 2, 3]); const responsePayload = downloadCapabilityPayload(bytes); diff --git a/tests/unit/resumable-upload-fetch-transport.test.ts b/tests/unit/resumable-upload-fetch-transport.test.ts index bedffc0..1f183bf 100644 --- a/tests/unit/resumable-upload-fetch-transport.test.ts +++ b/tests/unit/resumable-upload-fetch-transport.test.ts @@ -67,6 +67,56 @@ function createTransport( } describe("resumable upload fetch transport", () => { + it.each([ + { label: "delta-seconds", header: "2", now: 1_000, expected: 2_000 }, + { label: "HTTP-date ahead", header: "Thu, 01 Jan 1970 00:00:03 GMT", now: 1_000, expected: 2_000 }, + { label: "HTTP-date behind (clock rollback)", header: "Thu, 01 Jan 1970 00:00:01 GMT", now: 9_000, expected: 0 }, + ])( + "resolves Retry-After against the injected clock ($label)", + async ({ header, now, expected }) => { + // BT-UP-02. Both branches use the same captured `now`, so boundaries and + // clock rollback are deterministic. + const transport = createTransport( + (async () => + responseAt(ENDPOINTS.GET_STATUS, null, { + status: 429, + headers: { "retry-after": header }, + })) as unknown as typeof fetch, + { nowEpochMs: () => now }, + ); + + const result = await transport.execute({ + operation: "GET_STATUS", + body: { sessionId: "session_01" }, + signal: new AbortController().signal, + }); + expect(result).toMatchObject({ + ok: false, + error: { code: "UNAVAILABLE", retryAfterMs: expected }, + }); + }, + ); + + it("ignores an invalid Retry-After date instead of guessing", async () => { + const transport = createTransport( + (async () => + responseAt(ENDPOINTS.GET_STATUS, null, { + status: 429, + headers: { "retry-after": "not-a-date" }, + })) as unknown as typeof fetch, + { nowEpochMs: () => 1_000 }, + ); + + const result = await transport.execute({ + operation: "GET_STATUS", + body: { sessionId: "session_01" }, + signal: new AbortController().signal, + }); + expect(result).toMatchObject({ ok: false }); + if (result.ok) return; + expect(result.error.retryAfterMs).toBeUndefined(); + }); + it("uses a closed operation map and fixed production fetch policy", async () => { let receivedUrl = ""; let receivedInit: RequestInit | undefined; diff --git a/tests/unit/resumable-upload-http-control-plane.test.ts b/tests/unit/resumable-upload-http-control-plane.test.ts index 524f2af..e34709f 100644 --- a/tests/unit/resumable-upload-http-control-plane.test.ts +++ b/tests/unit/resumable-upload-http-control-plane.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it, vi } from "vitest"; +import { PRESIGNED_TRANSFER_PROTOCOL } from "../../src/application/ports/browser-transfer/presigned-transfer.ts"; + import type { PresignedUploadPartCapabilityProvider, } from "../../src/application/ports/browser-transfer/presigned-transfer.ts"; @@ -272,6 +274,8 @@ describe("resumable upload HTTP control plane", () => { }), ); return jsonResponseAt(CAPABILITY_ENDPOINT, { + // BT-PRE-02. The capability envelope declares its wire protocol. + protocol: PRESIGNED_TRANSFER_PROTOCOL, capabilityReceipt: `capability-upload-${capabilitySequence}`, method: "PUT", binding: request.binding,