From df183496829e5d15aefbad933e827da7cca1f6db Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Sat, 15 Aug 2026 01:25:12 +0900 Subject: [PATCH] fix: validate the snapshot that installs, not the object that was shown Three trust boundaries checked a caller's object and then read it again to use it. Between those two reads an accessor or a Proxy can answer differently, so the value that passed validation and the value that was installed were not the same value. A credential owner's answer was read field by field outside the auth boundary: a throwing `kind` getter escaped into the transport catch and an auth outage reached operators as `NETWORK_FAILURE`. Contract composition validated a contribution and then copied it, so a policy that answered 10,000 to the ceiling check and 999,999 to the copy installed the second value. The cursor runtime validated its profile once and re-read it on every page, so raising `maxPages` after construction widened a cap that had already been checked. `src/contracts/exact-snapshot.ts` is the one descriptor-based decoder they now share: every property is read exactly once, an accessor, a symbol, an inherited or non-enumerable field and a throwing trap all resolve to a typed failure, and validation runs on the owned copy. Separately, the `responseBody: NONE` probe awaited a bare `read()`. The deadline produced a bounded public result while the raw reader kept its lease, so the body stayed locked and the outer compensator could not cancel it. The probe now takes the operation lifetime and owns the cancel and the lock release itself. Co-Authored-By: Claude Opus 5 --- src/adapters/http/bounded-body-reader.ts | 16 +- src/adapters/http/http-execution-v3.ts | 91 +++++-- .../query-cache/cursor-pagination-runtime.ts | 58 ++++- src/contracts/exact-snapshot.ts | 174 +++++++++++++ src/contracts/external-contract-runtime.ts | 134 +++++++--- src/contracts/read-only-registry.ts | 9 + .../http-execution-contract.test.ts | 1 - .../http-execution-v3-live-authority.test.ts | 237 ++++++++++++++++++ .../http-execution-v3-observability.test.ts | 5 - .../contract-registry-immutability.test.ts | 165 ++++++++++++ tests/unit/cursor-pagination-runtime.test.ts | 118 +++++++++ tests/unit/http-execution-v3.test.ts | 14 -- 12 files changed, 943 insertions(+), 79 deletions(-) create mode 100644 src/contracts/exact-snapshot.ts diff --git a/src/adapters/http/bounded-body-reader.ts b/src/adapters/http/bounded-body-reader.ts index 98450d1..a11873e 100644 --- a/src/adapters/http/bounded-body-reader.ts +++ b/src/adapters/http/bounded-body-reader.ts @@ -116,6 +116,7 @@ export async function readBoundedBytes( */ export async function probeForbiddenBody( response: Response, + signal?: AbortSignal, ): Promise { const declared = declaredContentLength(response); if (declared !== null && declared > 0) { @@ -128,7 +129,20 @@ export async function probeForbiddenBody( const reader = response.body.getReader(); try { - const next = await reader.read(); + // NS-03. The probe owns the reader it opened, so the operation's lifetime + // has to reach it. Awaiting a bare `read()` left a non-cooperative stream + // locked after the deadline had already closed the public result, and the + // outer compensator could not cancel a body this reader still held. + const next = await readOrAbandon(reader, signal); + if (next === READ_ABANDONED) { + // Never awaited: cancelling a stream whose source ignores its signal can + // itself hang, and the caller already owns the terminal result. + void reader.cancel().catch(() => {}); + return Object.freeze({ + ok: false as const, + code: "RESPONSE_STREAM_FAILURE" as const, + }); + } if (next.done || !next.value || next.value.byteLength === 0) { return Object.freeze({ ok: true as const, present: false }); } diff --git a/src/adapters/http/http-execution-v3.ts b/src/adapters/http/http-execution-v3.ts index 4007216..9b72b71 100644 --- a/src/adapters/http/http-execution-v3.ts +++ b/src/adapters/http/http-execution-v3.ts @@ -29,6 +29,10 @@ import { INSTALLED_REST_AUTH_PROFILES, type InstalledRestAuthProfiles, } from "../../contracts/rest-profiles.ts"; +import { + snapshotExactObject, + snapshotOwnDataRecord, +} from "../../contracts/exact-snapshot.ts"; import { certaintyForAbandonedAttempt, classifyProblemEffect, @@ -604,25 +608,12 @@ export function createContractHttpExecutor( "TIMEOUT", ); } - const patch = patchResult; - if (patch?.kind === "SCOPE_FENCED") { - return finish(scopeFenced(preDispatchEffect(isCommand)), "NOT_STARTED"); - } - if (patch?.kind === "UNAUTHENTICATED") { - // A missing credential never downgrades into an anonymous request. - return finish(unauthenticated("NOT_STARTED", isCommand), "NOT_STARTED"); - } - if (patch?.kind === "UNAVAILABLE") { - return finish( - authIntegrationFailure("CREDENTIAL_OWNER_UNAVAILABLE", isCommand), - "AUTH_INTEGRATION_FAILURE", - ); - } - if ( - patch?.kind !== "READY" || - patch.headers === null || - typeof patch.headers !== "object" - ) { + // NS-01. The whole answer is decoded once, inside the auth boundary, before + // any field is used. Reading `kind` and `headers` off the raw object left + // the decode outside that boundary: a throwing getter escaped into the + // transport catch and an auth outage was classified as a network failure. + const patch = decodeCredentialPatch(patchResult); + if (patch === null) { // An off-contract answer is a collaborator breach, never a session // verdict the caller may act on. return finish( @@ -630,6 +621,19 @@ export function createContractHttpExecutor( "AUTH_INTEGRATION_FAILURE", ); } + if (patch.kind === "SCOPE_FENCED") { + return finish(scopeFenced(preDispatchEffect(isCommand)), "NOT_STARTED"); + } + if (patch.kind === "UNAUTHENTICATED") { + // A missing credential never downgrades into an anonymous request. + return finish(unauthenticated("NOT_STARTED", isCommand), "NOT_STARTED"); + } + if (patch.kind === "UNAVAILABLE") { + return finish( + authIntegrationFailure("CREDENTIAL_OWNER_UNAVAILABLE", isCommand), + "AUTH_INTEGRATION_FAILURE", + ); + } // The idempotency key is contract-owned, so a credential owner supplying it // stays the more specific request-contract violation. @@ -926,6 +930,53 @@ export function createContractHttpExecutor( return Object.freeze({ execute }); } +const CREDENTIAL_HEADER_VALUE_CEILING = 8_192; + +/** + * NS-01. Decodes a credential owner's answer into an owned, frozen value. Every + * field is read exactly once through its own data descriptor, so an accessor, a + * Proxy that answers differently on a second read, an inherited or smuggled + * field, or a trap that throws all resolve to `null` — a collaborator breach — + * rather than escaping as an exception or being installed unvalidated. + */ +function decodeCredentialPatch( + source: unknown, +): CredentialPatchOutcome | null { + const outer = snapshotExactObject(source, { + allowed: ["kind", "headers"], + required: ["kind"], + }); + if (outer === null) return null; + const kind = outer["kind"]; + if ( + kind === "UNAUTHENTICATED" || + kind === "UNAVAILABLE" || + kind === "SCOPE_FENCED" + ) { + return Object.hasOwn(outer, "headers") + ? null + : Object.freeze({ kind } as const); + } + if (kind !== "READY") return null; + + // The key set stays open here so the profile's own admission — and the more + // specific reserved-header violation — can still report the precise reason. + const headers = snapshotOwnDataRecord(outer["headers"]); + if (headers === null) return null; + for (const value of Object.values(headers)) { + if ( + typeof value !== "string" || + value.length > CREDENTIAL_HEADER_VALUE_CEILING + ) { + return null; + } + } + return Object.freeze({ + kind: "READY" as const, + headers, + }) as CredentialPatchOutcome; +} + type AdmissionOutcome = Readonly<{ result: HttpExecutionOutcome; certainty: string; @@ -1013,7 +1064,7 @@ async function admitResponse( // Success status: body policy first. if (contract.responseBody === "NONE") { - const probe = await probeForbiddenBody(response); + const probe = await probeForbiddenBody(response, signal); if (!probe.ok) { return settled( transportFailure( diff --git a/src/adapters/query-cache/cursor-pagination-runtime.ts b/src/adapters/query-cache/cursor-pagination-runtime.ts index 838cac6..3ab66a6 100644 --- a/src/adapters/query-cache/cursor-pagination-runtime.ts +++ b/src/adapters/query-cache/cursor-pagination-runtime.ts @@ -5,6 +5,7 @@ import type { CursorPaginationRuntime, } from "../../contracts/cursor-pagination.ts"; import { createFailure } from "../../contracts/errors.ts"; +import { snapshotExactObject } from "../../contracts/exact-snapshot.ts"; const ABORTED = Symbol("PAGINATION_ABORTED"); @@ -53,7 +54,17 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ context: Readonly<{ signal?: AbortSignal }>, ): Promise>>; }>): CursorPaginationRuntime { - validateProfile(dependencies.profile); + // NS-07. The caps are captured once, here. Validating the caller's profile and + // then reading it again on every page let a `maxPages` of 1 become 3 after + // construction, so the request count, item total and byte ceiling that were + // checked were not the ones the loop enforced. The collaborators are captured + // for the same reason. + const profile = snapshotProfile(dependencies.profile); + const definitionId = dependencies.definitionId; + const loadPage = dependencies.loadPage; + if (typeof definitionId !== "string" || typeof loadPage !== "function") { + throw new TypeError("Invalid cursor pagination dependencies."); + } return Object.freeze({ async loadAll(context) { const items: Value[] = []; @@ -62,7 +73,7 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ let snapshot: string | null | undefined; for ( let pageIndex = 0; - pageIndex < dependencies.profile.maxPages; + pageIndex < profile.maxPages; pageIndex += 1 ) { if (context.signal?.aborted) { @@ -74,7 +85,7 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ // accumulated into a successful result. const raced: Result> | typeof ABORTED = await raceAbort>>( - dependencies.loadPage(cursor, context), + loadPage(cursor, context), context.signal, ); if (raced === ABORTED || context.signal?.aborted) { @@ -83,7 +94,7 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ const result: Result> = raced; if (!result.ok) return result; const page: CursorPage = result.value; - if (!isValidPage(page, dependencies.profile)) { + if (!isValidPage(page, profile)) { return failure( "PAGINATION_CONTRACT_VIOLATION", "PAGINATION_PAGE_INVALID", @@ -99,8 +110,8 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ } items.push(...page.items); if ( - items.length > dependencies.profile.maxTotalItems || - estimatedBytes(items) > dependencies.profile.maxEstimatedBytes + items.length > profile.maxTotalItems || + estimatedBytes(items) > profile.maxEstimatedBytes ) { return failure( "RESULT_LIMIT_EXCEEDED", @@ -134,13 +145,46 @@ export function createCursorPaginationRuntime(dependencies: Readonly<{ ) { return { ok: false as const, - error: createFailure(kind, dependencies.definitionId, 0, { code }), + error: createFailure(kind, definitionId, 0, { code }), }; } } +/** + * NS-07. Copies the profile into an owned frozen record, reading every field + * exactly once, and validates that copy. An accessor, an inherited or extra + * field, a symbol key or a Proxy trap fails closed rather than becoming a cap + * that can change after it was checked. + */ +function snapshotProfile(source: unknown): CursorPaginationProfile { + const profile = snapshotExactObject(source, { + allowed: [ + "profileId", + "maxPages", + "maxTotalItems", + "maxEstimatedBytes", + "maxCursorBytes", + "allowSparsePage", + ], + required: [ + "profileId", + "maxPages", + "maxTotalItems", + "maxEstimatedBytes", + "maxCursorBytes", + "allowSparsePage", + ], + }) as CursorPaginationProfile | null; + if (profile === null || typeof profile.allowSparsePage !== "boolean") { + throw new TypeError("Invalid cursor pagination profile."); + } + validateProfile(profile); + return profile; +} + function validateProfile(profile: CursorPaginationProfile): void { if ( + typeof profile.profileId !== "string" || !profile.profileId || !Number.isSafeInteger(profile.maxPages) || profile.maxPages < 1 || diff --git a/src/contracts/exact-snapshot.ts b/src/contracts/exact-snapshot.ts new file mode 100644 index 0000000..1707b1f --- /dev/null +++ b/src/contracts/exact-snapshot.ts @@ -0,0 +1,174 @@ +/** + * Descriptor-based exact decoding for values that cross a trust boundary. + * + * Several adapters independently wrote "check the shape, then read it again to + * copy it". That order is the bug: between the check and the copy an accessor + * or a Proxy can answer differently, so the value that was validated and the + * value that was installed are two different things. Every helper here reads a + * property exactly once, through its own data descriptor, and hands back an + * owned plain object. Validation then runs on the snapshot, never on the source. + * + * The helpers are total: a hostile `getPrototypeOf`, `ownKeys` or + * `getOwnPropertyDescriptor` trap yields `null`, never a thrown exception, so a + * caller can keep its own typed failure vocabulary. + */ + +const DEFAULT_PROTOTYPES: readonly (object | null)[] = Object.freeze([ + Object.prototype, + null, +]); + +export type ExactObjectPolicy = Readonly<{ + /** Every own key the value may carry. Anything else rejects the snapshot. */ + allowed: readonly string[]; + /** Keys that must be present as own data properties. */ + required?: readonly string[]; + /** + * Prototypes the value may have. Defaults to a plain object or a null + * prototype, which is what a decoded wire payload or a literal produces. + */ + prototypes?: readonly (object | null)[]; +}>; + +/** + * Reads `source[key]` exactly once through its own data descriptor. An accessor, + * an inherited property or a missing key all answer `undefined`, and a trap that + * throws answers `undefined` rather than escaping. + */ +export function ownDataValue(source: unknown, key: string): unknown { + if (source === null || typeof source !== "object") return undefined; + try { + const descriptor = Object.getOwnPropertyDescriptor(source, key); + if (!descriptor || !("value" in descriptor)) return undefined; + return descriptor.value; + } catch { + return undefined; + } +} + +/** True when `key` is present as an own data property. */ +export function hasOwnDataKey(source: unknown, key: string): boolean { + if (source === null || typeof source !== "object") return false; + try { + const descriptor = Object.getOwnPropertyDescriptor(source, key); + return Boolean(descriptor) && "value" in (descriptor as PropertyDescriptor); + } catch { + return false; + } +} + +/** + * Copies `source` into a frozen plain object, reading every property exactly + * once. Returns `null` when the value is not an object, carries a symbol or an + * unexpected own key, exposes an accessor, has an unapproved prototype, misses a + * required key, or makes any reflection operation throw. + */ +export function snapshotExactObject( + source: unknown, + policy: ExactObjectPolicy, +): Readonly> | null { + if (source === null || typeof source !== "object") return null; + try { + const prototypes = policy.prototypes ?? DEFAULT_PROTOTYPES; + if (!prototypes.includes(Reflect.getPrototypeOf(source))) return null; + if (Object.getOwnPropertySymbols(source).length > 0) return null; + + const allowed = new Set(policy.allowed); + const names = Object.getOwnPropertyNames(source); + const snapshot: Record = {}; + for (const name of names) { + if (!allowed.has(name)) return null; + const descriptor = Object.getOwnPropertyDescriptor(source, name); + // A non-enumerable own property is as much a smuggled field as an + // inherited one, and an accessor is a second read waiting to happen. + if ( + !descriptor || + !("value" in descriptor) || + descriptor.enumerable !== true + ) { + return null; + } + Object.defineProperty(snapshot, name, { + value: descriptor.value, + enumerable: true, + writable: false, + configurable: false, + }); + } + for (const name of policy.required ?? []) { + if (!Object.hasOwn(snapshot, name)) return null; + } + return Object.freeze(snapshot); + } catch { + return null; + } +} + +/** + * Copies an open-keyed record — a header bag, a query map — into a frozen owned + * object, reading every property exactly once. The key set is not constrained + * here; admission against an allow-list stays with the policy that owns it, so + * the more specific rejection can still be reported. Returns `null` for a + * non-object, a symbol key, an accessor, a non-enumerable own key, an + * unapproved prototype, more than `maximumKeys` entries, or a throwing trap. + */ +export function snapshotOwnDataRecord( + source: unknown, + maximumKeys = 64, +): Readonly> | null { + if (source === null || typeof source !== "object") return null; + try { + if (!DEFAULT_PROTOTYPES.includes(Reflect.getPrototypeOf(source))) { + return null; + } + if (Object.getOwnPropertySymbols(source).length > 0) return null; + const names = Object.getOwnPropertyNames(source); + if (names.length > maximumKeys) return null; + const snapshot: Record = {}; + for (const name of names) { + const descriptor = Object.getOwnPropertyDescriptor(source, name); + if ( + !descriptor || + !("value" in descriptor) || + descriptor.enumerable !== true + ) { + return null; + } + Object.defineProperty(snapshot, name, { + value: descriptor.value, + enumerable: true, + writable: false, + configurable: false, + }); + } + return Object.freeze(snapshot); + } catch { + return null; + } +} + +/** + * Copies a genuine array into a frozen owned array, reading each element exactly + * once. Returns `null` for a non-array, a hostile length or a trap that throws. + */ +export function snapshotExactArray( + source: unknown, + maximumLength = 4_096, +): readonly unknown[] | null { + try { + if (!Array.isArray(source)) return null; + const length = source.length; + if (!Number.isSafeInteger(length) || length < 0 || length > maximumLength) { + return null; + } + const items: unknown[] = []; + for (let index = 0; index < length; index += 1) { + const descriptor = Object.getOwnPropertyDescriptor(source, String(index)); + if (!descriptor || !("value" in descriptor)) return null; + items.push(descriptor.value); + } + return Object.freeze(items); + } catch { + return null; + } +} diff --git a/src/contracts/external-contract-runtime.ts b/src/contracts/external-contract-runtime.ts index df30b1d..7ed430c 100644 --- a/src/contracts/external-contract-runtime.ts +++ b/src/contracts/external-contract-runtime.ts @@ -548,15 +548,21 @@ function snapshotHttpContract( label: string, ): InstalledHttpContract { const reject = (detail: string): never => fail(`${label}: ${detail}`); + // NS-02. The outer row is snapshotted first, so every nested read below comes + // from an owned object rather than from the caller's, which could answer + // differently on a second read. + const row = exactOwnDataSnapshot< + InstalledHttpContract + >(installed, ["contract", "frontend"], ["contract", "frontend"], reject); const frontend = exactOwnDataSnapshot( - installed.frontend, + row.frontend, EXECUTION_POLICY_KEYS, EXECUTION_POLICY_KEYS, reject, ); const source = exactOwnDataSnapshot< InstalledHttpContract["contract"] - >(installed.contract, HTTP_CONTRACT_KEYS, HTTP_CONTRACT_KEYS, reject); + >(row.contract, HTTP_CONTRACT_KEYS, HTTP_CONTRACT_KEYS, reject); const contract = Object.freeze({ ...source, acceptedStatuses: Object.freeze([...source.acceptedStatuses]), @@ -605,11 +611,22 @@ export function composeContractContributions( >(); const packagesById = new Map(); const contributionIds = new Set(); + const installedContributions: InstalledContractContribution[] = []; - for (const contribution of contributions) { - if (!contribution || typeof contribution !== "object") { + for (const raw of contributions) { + if (!raw || typeof raw !== "object") { fail("contribution: object required"); } + // NS-02. Snapshot first, then validate the snapshot, then install exactly + // what was validated. Validating the caller's object and reading it again + // to copy it let a stateful answer pass the ceiling check and still install + // a different deadline, retry budget or auth profile. + const contribution = exactOwnDataSnapshot( + raw, + ["contributionId", "featureId", "source", "http", "events"], + ["contributionId", "featureId", "source", "http", "events"], + (detail) => fail(`contribution: ${detail}`), + ); const contributionId = contribution.contributionId; if ( typeof contributionId !== "string" || @@ -625,59 +642,114 @@ export function composeContractContributions( if (typeof featureId !== "string" || !FEATURE_ID.test(featureId)) { fail(`featureId: ${String(featureId)}`); } - const source = contribution.source; - if (!source || typeof source !== "object" || !("kind" in source)) { + const rawSource = contribution.source; + if (!rawSource || typeof rawSource !== "object" || !("kind" in rawSource)) { fail(`${featureId}: source`); } if (!Array.isArray(contribution.http) || !Array.isArray(contribution.events)) { fail(`${featureId}: contribution arrays`); } - if (source.kind === "EXTERNAL_PACKAGE") { - assertPackageIdentity(source.package, featureId); - const existing = packagesById.get(source.package.packageId); + const rejectSource = (detail: string): never => + fail(`${featureId}: source ${detail}`); + let source: ContractContributionSource; + if (rawSource.kind === "EXTERNAL_PACKAGE") { + const outer = exactOwnDataSnapshot< + Readonly<{ kind: "EXTERNAL_PACKAGE"; package: unknown }> + >(rawSource, ["kind", "package"], ["kind", "package"], rejectSource); + const identity = exactOwnDataSnapshot( + outer.package, + [ + "packageId", + "version", + "digest", + "runtimeProtocolVersion", + "sourceRevision", + ], + [ + "packageId", + "version", + "digest", + "runtimeProtocolVersion", + "sourceRevision", + ], + rejectSource, + ); + assertPackageIdentity(identity, featureId); + source = Object.freeze({ + kind: "EXTERNAL_PACKAGE" as const, + package: identity, + }); + const existing = packagesById.get(identity.packageId); if ( existing && - (existing.version !== source.package.version || - existing.digest !== source.package.digest || - existing.sourceRevision !== source.package.sourceRevision) + (existing.version !== identity.version || + existing.digest !== identity.digest || + existing.sourceRevision !== identity.sourceRevision) ) { fail( - `${featureId}: package ${source.package.packageId} has conflicting identities`, + `${featureId}: package ${identity.packageId} has conflicting identities`, ); } - packagesById.set(source.package.packageId, source.package); - } else if (source.kind === "TEMPLATE_FIXTURE") { - if (source.fixtureId !== "REFERENCE_FEATURE_V1" || source.revision !== 1) { + packagesById.set(identity.packageId, identity); + } else if (rawSource.kind === "TEMPLATE_FIXTURE") { + const fixture = exactOwnDataSnapshot< + Readonly<{ + kind: "TEMPLATE_FIXTURE"; + fixtureId: "REFERENCE_FEATURE_V1"; + revision: 1; + }> + >( + rawSource, + ["kind", "fixtureId", "revision"], + ["kind", "fixtureId", "revision"], + rejectSource, + ); + if ( + fixture.fixtureId !== "REFERENCE_FEATURE_V1" || + fixture.revision !== 1 + ) { fail(`${featureId}: template fixture identity`); } if (contribution.events.length !== 0) { fail(`${featureId}: template fixture must not contribute events`); } + source = fixture; } else { fail(`${featureId}: unknown contribution source kind`); } - + const installedHttp: InstalledHttpContract[] = []; for (const installed of contribution.http) { - assertHttpContract(installed, featureId); - const operationId = installed.contract.operationId; + const snapshot = snapshotHttpContract(installed, featureId); + assertHttpContract(snapshot, featureId); + const operationId = snapshot.contract.operationId; const previous = httpByOperationId.get(operationId); if (previous) fail(`duplicate operation: ${operationId}`); - httpByOperationId.set( - operationId, - snapshotHttpContract(installed, `${featureId}/${operationId}`), - ); + httpByOperationId.set(operationId, snapshot); + installedHttp.push(snapshot); } + const installedEvents: InstalledEventContract[] = []; for (const event of contribution.events) { - assertEventContract(event, featureId); - if (eventByType.has(event.eventType)) { - fail(`duplicate event type: ${event.eventType}`); + const snapshot = snapshotEventContract(event, featureId); + assertEventContract(snapshot, featureId); + if (eventByType.has(snapshot.eventType)) { + fail(`duplicate event type: ${snapshot.eventType}`); } - eventByType.set( - event.eventType, - snapshotEventContract(event, `${featureId}/${event.eventType}`), - ); + eventByType.set(snapshot.eventType, snapshot); + installedEvents.push(snapshot); } + + // Everything published downstream is the validated snapshot, so no consumer + // can be handed the caller's still-live object. + installedContributions.push( + Object.freeze({ + contributionId, + featureId, + source, + http: Object.freeze(installedHttp), + events: Object.freeze(installedEvents), + }), + ); } const externalPackages = [...packagesById.values()].map((identity) => @@ -685,7 +757,7 @@ export function composeContractContributions( ); return Object.freeze({ - contributions: Object.freeze([...contributions]), + contributions: Object.freeze(installedContributions), httpByOperationId: createReadOnlyRegistry(httpByOperationId), eventByType: createReadOnlyRegistry(eventByType), externalPackages: Object.freeze(externalPackages), diff --git a/src/contracts/read-only-registry.ts b/src/contracts/read-only-registry.ts index 82c3d98..bc5598b 100644 --- a/src/contracts/read-only-registry.ts +++ b/src/contracts/read-only-registry.ts @@ -63,6 +63,12 @@ export function exactOwnDataSnapshot( if (Object.getOwnPropertySymbols(source).length > 0) { onViolation("symbol-keyed field"); } + // NS-02. A custom prototype carries fields a name sweep never sees, and it + // stays live: replacing one after composition changes what the row answers. + const prototype = Reflect.getPrototypeOf(source); + if (prototype !== Object.prototype && prototype !== null) { + onViolation("unexpected prototype"); + } const allowed = new Set(allowedKeys); const snapshot: Record = {}; for (const key of Object.getOwnPropertyNames(source)) { @@ -71,6 +77,9 @@ export function exactOwnDataSnapshot( if (!descriptor || !("value" in descriptor)) { onViolation(`accessor field ${key}`); } + if (descriptor.enumerable !== true) { + onViolation(`non-enumerable field ${key}`); + } snapshot[key] = descriptor.value; } for (const key of requiredKeys) { diff --git a/tests/integration/http-execution-contract.test.ts b/tests/integration/http-execution-contract.test.ts index ac15afa..e2683b1 100644 --- a/tests/integration/http-execution-contract.test.ts +++ b/tests/integration/http-execution-contract.test.ts @@ -71,7 +71,6 @@ describe("HTTP operation execution contract", () => { const attachCredentials = vi.fn(() => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, })); const observedKeys: Array = []; const fetcher = vi.fn( diff --git a/tests/integration/http-execution-v3-live-authority.test.ts b/tests/integration/http-execution-v3-live-authority.test.ts index 8744d90..3471322 100644 --- a/tests/integration/http-execution-v3-live-authority.test.ts +++ b/tests/integration/http-execution-v3-live-authority.test.ts @@ -99,6 +99,188 @@ describe("LIVE-01 credential integration failures are not user session failures" }); } + /** + * NS-01. Reading `patch.kind` and `patch.headers` off the raw answer put the + * credential decode outside the auth boundary: a throwing getter escaped into + * the transport catch and the outage was reported as `NETWORK_FAILURE`, so + * the operator saw a network incident instead of an auth integration one. + */ + const hostileOwners = [ + { + label: "exposes a throwing kind getter", + attach: () => + Object.defineProperty({}, "kind", { + enumerable: true, + get() { + throw new TypeError("hostile kind getter"); + }, + }) as never, + }, + { + label: "exposes a throwing headers getter", + attach: () => + Object.defineProperty({ kind: "READY" }, "headers", { + enumerable: true, + get() { + throw new TypeError("hostile headers getter"); + }, + }) as never, + }, + { + label: "throws from an ownKeys trap", + attach: () => + new Proxy( + { kind: "READY", headers: { authorization: "Bearer ok" } }, + { + ownKeys() { + throw new TypeError("hostile ownKeys trap"); + }, + }, + ) as never, + }, + { + label: "throws from a getOwnPropertyDescriptor trap", + attach: () => + new Proxy( + { kind: "READY", headers: { authorization: "Bearer ok" } }, + { + getOwnPropertyDescriptor() { + throw new TypeError("hostile descriptor trap"); + }, + }, + ) as never, + }, + { + label: "carries the outcome only on its prototype", + attach: () => + Object.create({ + kind: "READY", + headers: { authorization: "Bearer ok" }, + }) as never, + }, + { + label: "carries an extra own field", + attach: () => + Object.freeze({ + kind: "READY", + headers: Object.freeze({ authorization: "Bearer ok" }), + injected: true, + }) as never, + }, + { + label: "carries a symbol field", + attach: () => + Object.freeze({ + kind: "READY", + headers: Object.freeze({ authorization: "Bearer ok" }), + [Symbol.for("injected")]: true, + }) as never, + }, + { + label: "hides the outcome behind a non-enumerable own field", + attach: () => + Object.defineProperties( + { kind: "READY" }, + { + headers: { + enumerable: false, + value: { authorization: "Bearer ok" }, + }, + }, + ) as never, + }, + ]; + + for (const owner of hostileOwners) { + it(`closes as AUTH_INTEGRATION_FAILURE when the owner ${owner.label}`, async () => { + const fetcher = vi.fn(async () => jsonResponse([])); + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + authProfiles: TEST_PROFILES, + attachCredentials: owner.attach, + fetcher: fetcher as unknown as typeof fetch, + }); + + const outcome = await executor.execute( + bearerOperation(), + { limit: 1 }, + { routeId: ROUTE_ID, scope: scopeSnapshot() }, + ); + + expect(outcome.kind).toBe("AUTH_INTEGRATION_FAILURE"); + expect(outcome.effect).toBe("NOT_APPLICABLE"); + expect(fetcher).toHaveBeenCalledTimes(0); + }); + } + + it("reads each field exactly once so a stateful answer cannot swap it", async () => { + const fetcher = vi.fn(async () => jsonResponse([])); + const kindReads: string[] = []; + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + authProfiles: TEST_PROFILES, + attachCredentials: () => + new Proxy( + { kind: "READY", headers: { authorization: "Bearer first" } }, + { + getOwnPropertyDescriptor(target, key) { + if (key === "kind") { + kindReads.push(key); + return { + configurable: true, + enumerable: true, + // A second read would answer with a different verdict. + value: kindReads.length > 1 ? "UNAUTHENTICATED" : "READY", + }; + } + return Reflect.getOwnPropertyDescriptor(target, key); + }, + }, + ) as never, + fetcher: fetcher as unknown as typeof fetch, + }); + + const outcome = await executor.execute( + bearerOperation(), + { limit: 1 }, + { routeId: ROUTE_ID, scope: scopeSnapshot() }, + ); + + expect(outcome.kind).toBe("SUCCESS"); + expect(kindReads).toHaveLength(1); + }); + + it("sends an owned header snapshot rather than the owner's live object", async () => { + const headers: Record = { authorization: "Bearer first" }; + let sentHeaders: Record | undefined; + const fetcher = vi.fn(async (_url: unknown, init?: RequestInit) => { + sentHeaders = init?.headers as Record; + return jsonResponse([]); + }); + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + authProfiles: TEST_PROFILES, + // The owner keeps a live reference to the object it handed over. + attachCredentials: () => ({ kind: "READY", headers }) as never, + fetcher: fetcher as unknown as typeof fetch, + }); + + const outcome = await executor.execute( + bearerOperation(), + { limit: 1 }, + { routeId: ROUTE_ID, scope: scopeSnapshot() }, + ); + + expect(outcome.kind).toBe("SUCCESS"); + headers.authorization = "Bearer swapped"; + expect(sentHeaders?.["Authorization"] ?? sentHeaders?.["authorization"]).toBe( + "Bearer first", + ); + }); + it("still reports a real absent session as UNAUTHENTICATED", async () => { const fetcher = vi.fn(async () => jsonResponse([])); const executor = createContractHttpExecutor({ @@ -153,6 +335,61 @@ describe("LIVE-04 the total deadline owns every physical wait", () => { ).toBe("TIMEOUT"); }); + /** + * NS-03. The `NONE` probe used to await `reader.read()` with no signal, so a + * deadline produced a bounded public result while the raw reader kept its + * lease on the body: the connection and the buffer stayed held after the + * operation had already ended. + */ + it("cancels and releases the NONE probe reader when the deadline owns the execution", async () => { + let pulls = 0; + let cancels = 0; + const neverEndingBody = new ReadableStream({ + pull() { + pulls += 1; + return new Promise(() => {}); + }, + cancel() { + cancels += 1; + }, + }); + const response = new Response(neverEndingBody, { status: 200 }); + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + authProfiles: TEST_PROFILES, + attachCredentials: () => + Object.freeze({ + kind: "READY" as const, + headers: { authorization: "Bearer t" }, + }), + fetcher: (async () => response) as unknown as typeof fetch, + }); + const noBodyOperation = { + ...bearerOperation(20), + contract: { + ...bearerOperation(20).contract, + responseBody: "NONE" as const, + }, + }; + + const outcome = await executor.execute( + noBodyOperation as never, + { limit: 1 }, + { routeId: ROUTE_ID, scope: scopeSnapshot() }, + ); + + expect(outcome.kind).toBe("TRANSPORT_FAILURE"); + expect( + outcome.kind === "TRANSPORT_FAILURE" ? outcome.failure.kind : null, + ).toBe("TIMEOUT"); + expect(pulls).toBe(1); + await vi.waitFor(() => { + expect(cancels).toBe(1); + }); + expect(response.body?.locked).toBe(false); + }); + it("does not wait for a non-cooperative body reader past the deadline", async () => { const neverEndingBody = new ReadableStream({ pull() { diff --git a/tests/integration/http-execution-v3-observability.test.ts b/tests/integration/http-execution-v3-observability.test.ts index 168597c..f2ccd29 100644 --- a/tests/integration/http-execution-v3-observability.test.ts +++ b/tests/integration/http-execution-v3-observability.test.ts @@ -106,7 +106,6 @@ describe("V3 HTTP observability projection", () => { attachCredentials: () => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, }), fetcher: testCase.fetcher, observe: sinks.projector, @@ -152,7 +151,6 @@ describe("V3 HTTP observability projection", () => { attachCredentials: () => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, }), fetcher: (async () => { throw new TypeError("network down"); @@ -195,7 +193,6 @@ describe("V3 HTTP observability projection", () => { attachCredentials: () => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, }), fetcher: (async () => Response.json([{ id: "a", name: "A" }])) as unknown as typeof fetch, @@ -221,7 +218,6 @@ describe("V3 HTTP observability projection", () => { attachCredentials: () => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, }), fetcher: (async () => Response.json([{ id: "a", name: "A" }])) as unknown as typeof fetch, @@ -288,7 +284,6 @@ describe("V3 HTTP observability projection", () => { attachCredentials: () => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, }), fetcher: (async () => Response.json({ id: "created", name: "Created" }, { diff --git a/tests/unit/contract-registry-immutability.test.ts b/tests/unit/contract-registry-immutability.test.ts index f3a2396..f62746c 100644 --- a/tests/unit/contract-registry-immutability.test.ts +++ b/tests/unit/contract-registry-immutability.test.ts @@ -169,4 +169,169 @@ describe("LIVE-03 composed contract registry", () => { ).toThrow(); } }); + + /** + * NS-02. Composition validated the caller's object and then read it again to + * copy it. Between those two reads a stateful answer could swap a deadline or + * a retry budget, so the value that passed the ceiling check and the value the + * executor ran with were two different things. + */ + describe("validate the snapshot, never the source", () => { + /** Answers a safe value to a plain read and a hostile one to a copy. */ + const statefulFrontend = () => + new Proxy( + { ...TEST_CONTRACT_CONTRIBUTION.http[0]!.frontend }, + { + get(target, key, receiver) { + if (key === "totalDeadlineMs") return 10_000; + return Reflect.get(target, key, receiver); + }, + getOwnPropertyDescriptor(target, key) { + if (key === "totalDeadlineMs") { + return { + configurable: true, + enumerable: true, + value: 999_999, + }; + } + return Reflect.getOwnPropertyDescriptor(target, key); + }, + }, + ); + + const contributionWith = (row: unknown) => [ + { ...TEST_CONTRACT_CONTRIBUTION, http: [row] }, + ]; + + it("validates the deadline it will install, not the one it was shown", () => { + // The copied value exceeds the hard ceiling, so composition must stop + // rather than install a deadline no check ever saw. + expect(() => + composeContractContributions( + contributionWith({ + ...TEST_CONTRACT_CONTRIBUTION.http[0]!, + frontend: statefulFrontend(), + }) as never, + ), + ).toThrow(); + }); + + it("keys the registry by the operation id it copied, not the one it was shown", () => { + const base = { ...TEST_CONTRACT_CONTRIBUTION.http[0]!.contract }; + const contract = new Proxy(base, { + get(target, key, receiver) { + if (key === "operationId") return base.operationId; + return Reflect.get(target, key, receiver); + }, + getOwnPropertyDescriptor(target, key) { + if (key === "operationId") { + return { + configurable: true, + enumerable: true, + value: "SwappedOperation", + }; + } + return Reflect.getOwnPropertyDescriptor(target, key); + }, + }); + const composed = composeContractContributions( + contributionWith({ + ...TEST_CONTRACT_CONTRIBUTION.http[0]!, + contract, + }) as never, + ); + expect([...composed.httpByOperationId.keys()]).toEqual([ + "SwappedOperation", + ]); + expect( + composed.httpByOperationId.get("SwappedOperation")!.contract + .operationId, + ).toBe("SwappedOperation"); + }); + + const hostileRows = [ + { + label: "an installed row with an extra own field", + row: () => ({ + ...TEST_CONTRACT_CONTRIBUTION.http[0]!, + injected: true, + }), + }, + { + label: "an installed row with a symbol field", + row: () => ({ + ...TEST_CONTRACT_CONTRIBUTION.http[0]!, + [Symbol.for("injected")]: true, + }), + }, + { + label: "an installed row with a custom prototype", + row: () => + Object.assign(Object.create({ injected: true }), { + ...TEST_CONTRACT_CONTRIBUTION.http[0]!, + }), + }, + { + label: "an installed row hiding a non-enumerable own field", + row: () => + Object.defineProperty( + { ...TEST_CONTRACT_CONTRIBUTION.http[0]! }, + "injected", + { enumerable: false, value: true }, + ), + }, + { + label: "an installed row behind a throwing ownKeys trap", + row: () => + new Proxy( + { ...TEST_CONTRACT_CONTRIBUTION.http[0]! }, + { + ownKeys() { + throw new TypeError("hostile ownKeys trap"); + }, + }, + ), + }, + ]; + + for (const { label, row } of hostileRows) { + it(`refuses to compose ${label}`, () => { + expect(() => + composeContractContributions(contributionWith(row()) as never), + ).toThrow(); + }); + } + + it("refuses a contribution whose own shape is not exact", () => { + for (const contribution of [ + { ...TEST_CONTRACT_CONTRIBUTION, injected: true }, + Object.assign(Object.create({ injected: true }), { + ...TEST_CONTRACT_CONTRIBUTION, + }), + { + ...TEST_CONTRACT_CONTRIBUTION, + [Symbol.for("injected")]: true, + }, + ]) { + expect(() => + composeContractContributions([contribution] as never), + ).toThrow(); + } + }); + + it("refuses an event contract whose own shape is not exact", () => { + const events = TEST_CONTRACT_CONTRIBUTION.events; + if (events.length === 0) return; + for (const event of [ + { ...events[0]!, injected: true }, + Object.assign(Object.create({ injected: true }), { ...events[0]! }), + ]) { + expect(() => + composeContractContributions([ + { ...TEST_CONTRACT_CONTRIBUTION, events: [event] }, + ] as never), + ).toThrow(); + } + }); + }); }); diff --git a/tests/unit/cursor-pagination-runtime.test.ts b/tests/unit/cursor-pagination-runtime.test.ts index 17c8ad7..9b119c5 100644 --- a/tests/unit/cursor-pagination-runtime.test.ts +++ b/tests/unit/cursor-pagination-runtime.test.ts @@ -158,3 +158,121 @@ describe("bounded cursor pagination runtime", () => { }); }); }); + +/** + * NS-07. The profile was validated once and then re-read on every page, so + * raising `maxPages` after construction widened a cap that had already been + * checked — the runtime issued more requests and returned more items than the + * validated profile allowed. + */ +describe("NS-07 the caps are the ones that were validated", () => { + it("keeps the page cap captured at construction", async () => { + const mutable: { + profileId: string; + maxPages: number; + maxTotalItems: number; + maxEstimatedBytes: number; + maxCursorBytes: number; + allowSparsePage: boolean; + } = { ...profile, maxPages: 1 }; + const loadPage = vi.fn(async () => ({ + ok: true as const, + value: { + items: [1], + nextCursor: `cursor-${loadPage.mock.calls.length}`, + hasMore: true, + snapshotToken: "snapshot-a", + }, + })); + const runtime = createCursorPaginationRuntime({ + definitionId: "LIST_ALL", + profile: mutable, + loadPage, + }); + + mutable.maxPages = 3; + mutable.maxTotalItems = 99; + + await expect(runtime.loadAll({})).resolves.toMatchObject({ + ok: false, + error: { code: "PAGINATION_PAGE_LIMIT" }, + }); + expect(loadPage).toHaveBeenCalledTimes(1); + }); + + it("keeps the loader captured at construction", async () => { + const original = vi.fn(async () => ({ + ok: true as const, + value: { + items: [1], + nextCursor: null, + hasMore: false, + snapshotToken: null, + }, + })); + const replacement = vi.fn(); + const dependencies = { + definitionId: "LIST_ALL", + profile, + loadPage: original, + }; + const runtime = createCursorPaginationRuntime(dependencies); + dependencies.loadPage = replacement as never; + + await expect(runtime.loadAll({})).resolves.toMatchObject({ ok: true }); + expect(original).toHaveBeenCalledTimes(1); + expect(replacement).not.toHaveBeenCalled(); + }); + + const hostileProfiles: readonly (readonly [string, () => unknown])[] = [ + [ + "an accessor cap", + () => + Object.defineProperty({ ...profile }, "maxPages", { + enumerable: true, + get: () => 3, + }), + ], + [ + "an inherited cap", + () => Object.create({ ...profile }) as unknown, + ], + ["an extra own field", () => ({ ...profile, injected: true })], + [ + "a symbol field", + () => ({ ...profile, [Symbol.for("injected")]: true }), + ], + [ + "a non-enumerable own field", + () => + Object.defineProperty({ ...profile }, "injected", { + enumerable: false, + value: true, + }), + ], + [ + "a throwing ownKeys trap", + () => + new Proxy( + { ...profile }, + { + ownKeys() { + throw new TypeError("hostile ownKeys trap"); + }, + }, + ), + ], + ]; + + for (const [label, build] of hostileProfiles) { + it(`refuses to build a runtime from ${label}`, () => { + expect(() => + createCursorPaginationRuntime({ + definitionId: "LIST_ALL", + profile: build() as never, + loadPage: vi.fn(), + }), + ).toThrow(TypeError); + }); + } +}); diff --git a/tests/unit/http-execution-v3.test.ts b/tests/unit/http-execution-v3.test.ts index 1b90e52..8b5dd5c 100644 --- a/tests/unit/http-execution-v3.test.ts +++ b/tests/unit/http-execution-v3.test.ts @@ -74,7 +74,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: async () => Response.json( @@ -106,7 +105,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { const attachCredentials = vi.fn(() => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, })); const fetcher = vi.fn(); const executor = createContractHttpExecutor({ @@ -145,7 +143,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { const attachCredentials = vi.fn(() => ({ kind: "READY" as const, headers: {}, - credentials: "omit" as const, })); const fetcher = vi.fn(); const executor = createContractHttpExecutor({ @@ -191,7 +188,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { const attachCredentials = vi.fn(() => ({ kind: "READY" as const, headers: { [headerName]: "credential-owned-key" }, - credentials: "omit" as const, })); const fetcher = vi.fn(); const executor = createContractHttpExecutor({ @@ -236,7 +232,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher, sleep: async () => {}, @@ -277,7 +272,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher, }); @@ -302,7 +296,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: vi.fn(async (input) => { urls.push(String(input)); @@ -351,7 +344,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher, }); @@ -371,7 +363,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: vi.fn(), }); @@ -409,7 +400,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: vi.fn(async () => Response.json({ malformed: true }, { status: 201 })), }); @@ -445,7 +435,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: vi.fn(async () => { current = false; @@ -582,7 +571,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher, monotonicNow: () => 0, @@ -636,7 +624,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: vi.fn(async () => Response.json( @@ -678,7 +665,6 @@ describe("descriptor-driven HTTP execution lifetime", () => { attachCredentials: () => ({ kind: "READY", headers: {}, - credentials: "omit", }), fetcher: vi.fn(async () => new Response(body, { status: 200 })), });