diff --git a/src/adapters/http/client.ts b/src/adapters/http/client.ts index 1f21c45..5f5640e 100644 --- a/src/adapters/http/client.ts +++ b/src/adapters/http/client.ts @@ -18,11 +18,24 @@ import { durationBucket, statusGroup, } from "../../contracts/diagnostics.ts"; -import type { AuthSessionPort } from "../../application/ports/auth-session-port.ts"; +import type { + AuthSessionPort, + CredentialOperationContext, +} from "../../application/ports/auth-session-port.ts"; import { isValidIdempotencyKey } from "../../contracts/mutation-intent.ts"; /** Sentinel for a credential wait ended by the attempt lifetime. */ const ATTEMPT_ABORTED = Symbol("ATTEMPT_ABORTED"); + +/** + * LEG-02. The pre-V2 operations have no installed auth profile, so this is the + * legacy allowance the shared validator applies to them. It requires nothing, + * which preserves their existing behaviour exactly. + */ +const LEGACY_ALLOWED_CREDENTIAL_HEADERS = Object.freeze([ + "authorization", + "x-csrf-token", +] as const); import type { ClockPort } from "../../application/ports/clock-port.ts"; import type { DiagnosticsPort } from "../../application/ports/diagnostics-port.ts"; import type { TelemetryPort } from "../../application/ports/telemetry-port.ts"; @@ -30,6 +43,7 @@ import type { ApiOperation } from "../../contracts/api-operations.ts"; import type { ApiFailure } from "../../contracts/errors.ts"; import type { OperationRequestInput } from "./request-builder.ts"; import { readBoundedJson } from "./bounded-json.ts"; +import { admitCredentialHeaders } from "./http-contract-bridge.ts"; import type { MappingResult } from "../../contracts/boundary-mapper.ts"; import { createRestProviderProfile, @@ -328,14 +342,29 @@ export function createHttpClient( authSession.onUnauthenticated(); return finalize(outcome, "failed"); } - let recovered: Awaited>; + let recovered: SessionRecoveryOutcome; + // LEG-01. The recovery collaborator receives the request lifetime, and + // the transport races the same signal so a non-cooperative owner cannot + // hold the request open. + const recoveryLifetime = new AbortController(); try { recovered = await withinLogicalDeadline( - recoverSession(authSession, operation, outcome.error), + recoverSession( + authSession, + operation, + outcome.error, + Object.freeze({ + signal: recoveryLifetime.signal, + deadlineAtMonotonicMs: deadlineAt, + }), + ), deadlineAt, input.signal, ); } catch (error) { + // The request is over. Whatever the recovery answers next is observed + // by its own owner, never adopted here. + recoveryLifetime.abort(); return finalize( { ok: false, @@ -356,7 +385,14 @@ export function createHttpClient( error instanceof LogicalDeadlineError ? "failed" : "aborted", ); } - if (!recovered.ok) return finalize(recovered, "failed"); + if (!recovered.ok) { + // The result is adopted here, so the notification happens here. + if (recovered.notifyUnauthenticated) authSession.onUnauthenticated(); + return finalize( + { ok: false, error: recovered.error }, + "failed", + ); + } if (operation.idempotency === "none") { return finalize( { @@ -641,15 +677,30 @@ export function createHttpClient( }; } const patch = raced; - for (const [name, value] of Object.entries(patch.headers)) { - const normalized = name.toLowerCase(); - const allowedHeaders = + // LEG-02. The same admission validator V3 uses. Checking only the + // allowed set let a bearer profile dispatch with no Authorization at + // all, which is precisely the anonymous downgrade the required set + // exists to prevent. + const admission = admitCredentialHeaders(patch.headers, { + allowedCredentialHeaders: security?.auth.allowedCredentialHeaders ?? - (["authorization", "x-csrf-token"] as const); - if (!allowedHeaders.includes(normalized as never)) { - throw new TypeError("Credential patch contains a forbidden header"); - } - headers.set(normalized, value); + LEGACY_ALLOWED_CREDENTIAL_HEADERS, + requiredCredentialHeaders: + security?.auth.requiredCredentialHeaders ?? [], + }); + if (!admission.ok) { + return { + ok: false, + error: failure( + "AUTH_INTEGRATION_FAILURE", + operation.operationId, + attempt, + { code: "AUTH_ATTACH_FAILED" }, + ), + }; + } + for (const [name, value] of Object.entries(admission.headers)) { + headers.set(name, value); } } catch { // An ordinary owner rejection stays an integration failure. @@ -939,20 +990,36 @@ async function parseResponse( }; } +/** + * LEG-01. Recovery returns data only. + * + * The sign-out notification is a user-visible side effect, so it belongs to + * whoever adopts this result — not to the raw recovery call. A recovery that + * loses the race against the deadline or a caller abort still settles, and + * signing the user out then would attribute a request nobody is waiting on to + * an expired session. + */ +type SessionRecoveryOutcome = + | Readonly<{ ok: true }> + | Readonly<{ + ok: false; + error: HttpFailure; + notifyUnauthenticated: boolean; + }>; + async function recoverSession( authSession: HttpAuthSession, operation: ApiOperation, originalFailure: HttpFailure, -): Promise< - Readonly<{ ok: true }> | Readonly<{ ok: false; error: HttpFailure }> -> { + context: CredentialOperationContext, +): Promise { try { - const result = await authSession.recover(); + const result = await authSession.recover(context); if (result === "restored") return { ok: true }; if (result === "no-session") { - authSession.onUnauthenticated(); return { ok: false, + notifyUnauthenticated: true, error: failure("AUTH_REQUIRED", operation.operationId, originalFailure.attemptCount - 1, { code: "AUTH_REQUIRED", httpStatus: 401, @@ -965,6 +1032,7 @@ async function recoverSession( return { ok: false, + notifyUnauthenticated: false, error: failure( "AUTH_INTEGRATION_FAILURE", operation.operationId, diff --git a/src/adapters/http/http-execution-v3.ts b/src/adapters/http/http-execution-v3.ts index 355fc31..4007216 100644 --- a/src/adapters/http/http-execution-v3.ts +++ b/src/adapters/http/http-execution-v3.ts @@ -6,7 +6,9 @@ import { import type { CacheScopeSnapshot } from "../../contracts/server-state-scope.ts"; import { defineMutationIntent, - MUTATION_INTENT_BOUNDS, + // OPT-NET-02. One shared key authority, so the intent factory and this + // admission site cannot drift apart. + isValidIdempotencyKey, type MutationIntent, } from "../../contracts/mutation-intent.ts"; import { @@ -342,7 +344,7 @@ function validateMutationIntent( } const key = validated.idempotencyKey; - if (requiresKey && !validIdempotencyKey(key)) { + if (requiresKey && !isValidIdempotencyKey(key)) { return Object.freeze({ ok: false, violation: "MISSING_IDEMPOTENCY_KEY", @@ -357,31 +359,6 @@ function validateMutationIntent( return Object.freeze({ ok: true, intent: validated }); } -const UTF8 = new TextEncoder(); - -function validIdempotencyKey(value: unknown): value is string { - return ( - typeof value === "string" && - value.trim().length > 0 && - UTF8.encode(value).byteLength <= - MUTATION_INTENT_BOUNDS.idempotencyKeyMaxBytes && - !hasControlCharacter(value) - ); -} - -function hasControlCharacter(value: string): boolean { - for (const character of value) { - const codePoint = character.codePointAt(0) ?? 0; - if ( - codePoint <= 0x1f || - (codePoint >= 0x7f && codePoint <= 0x9f) - ) { - return true; - } - } - return false; -} - export function createContractHttpExecutor( dependencies: ContractHttpExecutorDependencies, ): ContractHttpExecutor { diff --git a/src/adapters/query-cache/cursor-pagination-runtime.ts b/src/adapters/query-cache/cursor-pagination-runtime.ts index 7fc89e8..838cac6 100644 --- a/src/adapters/query-cache/cursor-pagination-runtime.ts +++ b/src/adapters/query-cache/cursor-pagination-runtime.ts @@ -12,6 +12,11 @@ const ABORTED = Symbol("PAGINATION_ABORTED"); * Resolves as soon as the operation settles or the signal aborts, whichever * comes first. A late operation result is observed and discarded, never thrown * as an unhandled rejection. + * + * OPT-NET-01. A loader rejection is *not* an abort. The presence of a signal + * says nothing about why the loader failed, so a rejection is re-thrown exactly + * as it would be with no signal at all; only a signal that has actually + * aborted classifies the outcome as cancellation. */ async function raceAbort( operation: Promise, @@ -20,7 +25,7 @@ async function raceAbort( operation.catch(() => {}); if (!signal) return await operation; if (signal.aborted) return ABORTED; - return await new Promise((resolve) => { + return await new Promise((resolve, reject) => { const onAbort = () => resolve(ABORTED); signal.addEventListener("abort", onAbort, { once: true }); operation.then( @@ -28,9 +33,13 @@ async function raceAbort( signal.removeEventListener("abort", onAbort); resolve(value); }, - () => { + (reason: unknown) => { signal.removeEventListener("abort", onAbort); - resolve(ABORTED); + if (signal.aborted) { + resolve(ABORTED); + return; + } + reject(reason); }, ); }); diff --git a/src/application/ports/auth-session-port.ts b/src/application/ports/auth-session-port.ts index 2741095..3a4d57f 100644 --- a/src/application/ports/auth-session-port.ts +++ b/src/application/ports/auth-session-port.ts @@ -9,7 +9,16 @@ export type SessionGateway = Readonly<{ subscribe(listener: () => void): () => void; beginSignIn(returnTo?: string): Promise; signOut(): Promise; - recover(): Promise<"restored" | "no-session">; + /** + * LEG-01. Recovery is part of a request's lifetime, so it receives the same + * context a credential attach does. The context is optional for one release + * to keep existing owners working; the transport races the signal either way, + * and a recovery that answers after the request already ended is observed but + * never turned into a user-visible sign-out. + */ + recover( + context?: CredentialOperationContext, + ): Promise<"restored" | "no-session">; }>; export type CredentialRequestBinding = Readonly<{ diff --git a/src/contracts/mutation-intent.ts b/src/contracts/mutation-intent.ts index a03e7ee..3d11e1e 100644 --- a/src/contracts/mutation-intent.ts +++ b/src/contracts/mutation-intent.ts @@ -70,11 +70,11 @@ export function defineMutationIntent(intent: MutationIntent): MutationIntent { intent.canonicalInputIdentity, MUTATION_INTENT_BOUNDS.canonicalInputIdentityMaxBytes, ) || + // OPT-NET-02. Intent definition and executor admission share one key + // authority; a second, looser rule here is how a control character reaches + // an `Idempotency-Key` header. (intent.idempotencyKey !== undefined && - !validBoundedString( - intent.idempotencyKey, - MUTATION_INTENT_BOUNDS.idempotencyKeyMaxBytes, - )) || + !isValidIdempotencyKey(intent.idempotencyKey)) || !Number.isFinite(intent.createdAtMonotonicMs) || intent.createdAtMonotonicMs < 0 ) { diff --git a/tests/integration/legacy-http-credential-authority.test.ts b/tests/integration/legacy-http-credential-authority.test.ts new file mode 100644 index 0000000..0ee7200 --- /dev/null +++ b/tests/integration/legacy-http-credential-authority.test.ts @@ -0,0 +1,240 @@ +import { HttpResponse, http } from "msw"; +import { setupServer } from "msw/node"; +import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; + +import { createHttpClient } from "../../src/adapters/http/client.ts"; +import { defineRestOperation } from "../../src/contracts/api-operations.ts"; +import { createRestProviderProfile } from "../../src/contracts/rest-profiles.ts"; +import { TEST_HTTP_CONTRACT } from "../helpers/http-contract-fixture.ts"; + +/** + * LEG-01 / LEG-02. The V2 client is not the default path any more, but a + * rollback re-activates it, so its credential and recovery authority must match + * V3 rather than diverge quietly. + */ + +const BEARER_LIST = defineRestOperation({ + method: "GET", + path: "/api/entities", + operationId: "LIST_ENTITIES", + auth: "external-session", + timeoutMs: null, + idempotency: "safe", + retry: "runtime", + requestSource: "search", + requestSchema: "EntityListQuery", + responseSchema: "EntityListPayload", + owner: "test-fixture", + contractVersion: 2, + protocol: "REST", + semantics: "QUERY", + replayPolicy: "SAFE", + idempotencyKeyPolicy: "NONE", + mapperId: "EntityListMapper", + successStatuses: [200], + responseMediaTypes: ["application/json"], + maxResponseBytes: 32_768, + providerId: "PRIMARY_API", + authProfileId: "REFERENCE_EXTERNAL_BEARER", + csrfProfileId: "NO_CSRF_BEARER", + pathSchema: "NoRequest", + pathParameterNames: [], + maxEncodedSearchBytes: 1_024, +}); + +let observedAuthorization: string | null = null; +let requestCount = 0; +let nextStatus = 200; + +const server = setupServer( + http.get("https://api.test/api/entities", ({ request }) => { + requestCount += 1; + observedAuthorization = request.headers.get("authorization"); + if (nextStatus === 401) { + return HttpResponse.json( + { + success: false, + error: { code: "UNAUTHENTICATED" }, + meta: { requestId: "request-1", traceId: "trace-1" }, + }, + { status: 401 }, + ); + } + return HttpResponse.json({ + success: true, + data: [{ id: "resource-1", name: "Example" }], + meta: { requestId: "request-2", traceId: "trace-1" }, + }); + }), +); + +beforeAll(() => server.listen({ onUnhandledRequest: "error" })); +afterEach(() => { + observedAuthorization = null; + requestCount = 0; + nextStatus = 200; + server.resetHandlers(); +}); +afterAll(() => server.close()); + +const clock = { now: () => 0, sleep: async () => {} }; + +type HttpDependencies = Parameters[0]; + +function bearerClient(options: Partial) { + return createHttpClient({ + ...TEST_HTTP_CONTRACT, + getOperation: (operationId: string) => { + if (operationId !== "LIST_ENTITIES") { + throw new Error(`Unknown test operation: ${operationId}`); + } + return BEARER_LIST; + }, + baseUrl: "https://api.test", + providerProfile: createRestProviderProfile("PRIMARY_API", "https://api.test", [ + "omit", + ]), + // The V2 fixture declares `NoRequest` for its path codec, which the shared + // schema registry does not carry. + validatePath: () => ({ success: true as const, data: {} }), + clock, + ...options, + } as HttpDependencies); +} + +function sessionStub( + overrides: Partial<{ + getState: () => "authenticated" | "unauthenticated" | "recovery-pending" | "integration-failed"; + credentialPatch: (...args: never[]) => Promise<{ headers: Record }>; + recover: (...args: never[]) => Promise<"restored" | "no-session">; + onUnauthenticated: () => void; + }> = {}, +) { + return { + getState: () => "authenticated" as const, + subscribe: () => () => {}, + beginSignIn: async () => {}, + signOut: async () => {}, + credentialPatch: async () => ({ headers: { authorization: "Bearer t" } }), + recover: async () => "restored" as const, + onUnauthenticated: () => {}, + ...overrides, + }; +} + +describe("LEG-02 the bearer profile's required header is enforced", () => { + it("refuses to dispatch when a READY patch omits authorization", async () => { + const client = bearerClient({ + authSession: sessionStub({ + credentialPatch: async () => ({ headers: {} }), + }) as never, + }); + + await expect(client.execute("LIST_ENTITIES")).resolves.toMatchObject({ + ok: false, + error: { kind: "AUTH_INTEGRATION_FAILURE" }, + }); + expect(requestCount).toBe(0); + }); + + it("rejects a malformed or duplicated authorization value", async () => { + const hostilePatches: readonly Record[] = [ + { authorization: "" }, + { authorization: "Bearer bad\nvalue" }, + { Authorization: "Bearer a", authorization: "Bearer b" }, + ]; + for (const headers of hostilePatches) { + const client = bearerClient({ + authSession: sessionStub({ + credentialPatch: async () => ({ headers }), + }) as never, + }); + await expect(client.execute("LIST_ENTITIES")).resolves.toMatchObject({ + ok: false, + error: { kind: "AUTH_INTEGRATION_FAILURE" }, + }); + } + expect(requestCount).toBe(0); + }); + + it("dispatches with the admitted authorization header", async () => { + const client = bearerClient({ + authSession: sessionStub() as never, + }); + + await expect(client.execute("LIST_ENTITIES")).resolves.toMatchObject({ + ok: true, + }); + expect(observedAuthorization).toBe("Bearer t"); + }); +}); + +describe("LEG-01 recovery notification follows the adopted result", () => { + it("does not sign the user out when recovery answers after the deadline", async () => { + nextStatus = 401; + const onUnauthenticated = vi.fn(); + let elapsed = 0; + const client = bearerClient({ + clock: { + now: () => elapsed, + sleep: async () => {}, + }, + timeoutMs: 20, + authSession: sessionStub({ + onUnauthenticated, + recover: async () => { + // The request's own deadline expires while recovery is still out. + elapsed = 10_000; + await new Promise((resolve) => setTimeout(resolve, 30)); + return "no-session" as const; + }, + }) as never, + }); + + const outcome = await client.execute("LIST_ENTITIES"); + expect(outcome.ok).toBe(false); + await new Promise((resolve) => setTimeout(resolve, 60)); + expect(onUnauthenticated).toHaveBeenCalledTimes(0); + }); + + it("signs the user out exactly once for an adopted no-session result", async () => { + nextStatus = 401; + const onUnauthenticated = vi.fn(); + const client = bearerClient({ + authSession: sessionStub({ + onUnauthenticated, + recover: async () => "no-session" as const, + }) as never, + }); + + await expect(client.execute("LIST_ENTITIES")).resolves.toMatchObject({ + ok: false, + error: { kind: "AUTH_REQUIRED" }, + }); + expect(onUnauthenticated).toHaveBeenCalledTimes(1); + }); + + it("stays bounded when recovery never settles", async () => { + nextStatus = 401; + const onUnauthenticated = vi.fn(); + let elapsed = 0; + const client = bearerClient({ + clock: { + now: () => { + elapsed += 5; + return elapsed; + }, + sleep: async () => {}, + }, + timeoutMs: 20, + authSession: sessionStub({ + onUnauthenticated, + recover: () => new Promise<"restored">(() => {}), + }) as never, + }); + + const outcome = await client.execute("LIST_ENTITIES"); + expect(outcome.ok).toBe(false); + expect(onUnauthenticated).toHaveBeenCalledTimes(0); + }); +}); diff --git a/tests/unit/legacy-and-optional-network-remediation.test.ts b/tests/unit/legacy-and-optional-network-remediation.test.ts new file mode 100644 index 0000000..a66b870 --- /dev/null +++ b/tests/unit/legacy-and-optional-network-remediation.test.ts @@ -0,0 +1,149 @@ +import { describe, expect, it, vi } from "vitest"; + +import { createCursorPaginationRuntime } from "../../src/adapters/query-cache/cursor-pagination-runtime.ts"; +import { + defineMutationIntent, + isValidIdempotencyKey, +} from "../../src/contracts/mutation-intent.ts"; +import type { + CursorPage, + CursorPaginationProfile, +} from "../../src/contracts/cursor-pagination.ts"; +import type { Result } from "../../src/application/result.ts"; + +const PROFILE: CursorPaginationProfile = Object.freeze({ + profileId: "TEST_PAGINATION_V1", + maxPages: 3, + maxTotalItems: 30, + maxEstimatedBytes: 32_768, + maxCursorBytes: 512, + allowSparsePage: false, +}); + +function page( + items: readonly number[], + nextCursor: string | null, +): CursorPage { + return Object.freeze({ + items: Object.freeze([...items]), + nextCursor, + hasMore: nextCursor !== null, + snapshotToken: "snapshot-1", + }); +} + +/** + * OPT-NET-01. A loader rejection is evidence about the data source. Reporting + * it as `PAGINATION_ABORTED` because a signal merely *exists* erases a real + * network or contract failure and files it under a user decision nobody made. + */ +describe("OPT-NET-01 cursor pagination abort classification", () => { + it("preserves a loader rejection while the signal is still live", async () => { + const controller = new AbortController(); + const runtime = createCursorPaginationRuntime({ + definitionId: "TEST_PAGINATION", + profile: PROFILE, + loadPage: async () => { + throw new TypeError("upstream exploded"); + }, + }); + + await expect( + runtime.loadAll({ signal: controller.signal }), + ).rejects.toThrow("upstream exploded"); + }); + + it("keeps the same rejection when no signal is supplied", async () => { + const runtime = createCursorPaginationRuntime({ + definitionId: "TEST_PAGINATION", + profile: PROFILE, + loadPage: async () => { + throw new TypeError("upstream exploded"); + }, + }); + + await expect(runtime.loadAll({})).rejects.toThrow("upstream exploded"); + }); + + it("classifies a rejection during a real abort as PAGINATION_ABORTED", async () => { + const controller = new AbortController(); + const runtime = createCursorPaginationRuntime({ + definitionId: "TEST_PAGINATION", + profile: PROFILE, + loadPage: async () => { + controller.abort(); + throw new TypeError("cancelled upstream"); + }, + }); + + const result = await runtime.loadAll({ signal: controller.signal }); + expect(result.ok).toBe(false); + expect(result.ok ? null : result.error.code).toBe("PAGINATION_ABORTED"); + }); + + it("does not admit a page that resolves after the abort", async () => { + const controller = new AbortController(); + const loadPage = vi.fn( + async (): Promise>> => { + controller.abort(); + await new Promise((resolve) => setTimeout(resolve, 5)); + return { ok: true, value: page([1, 2], null) }; + }, + ); + const runtime = createCursorPaginationRuntime({ + definitionId: "TEST_PAGINATION", + profile: PROFILE, + loadPage, + }); + + const result = await runtime.loadAll({ signal: controller.signal }); + expect(result.ok).toBe(false); + expect(result.ok ? null : result.error.code).toBe("PAGINATION_ABORTED"); + }); +}); + +/** + * OPT-NET-02. One idempotency-key authority. Two validators drift, and the + * looser one becomes the way a control character reaches a request header. + */ +describe("OPT-NET-02 shared idempotency key validation", () => { + const rejected = [ + "", + " ", + "key\nwith-newline", + "key\u0000null", + "key\u007fdelete", + "key\u009fc1", + "a".repeat(257), + ]; + + it("rejects the same values at intent definition and at admission", () => { + for (const value of rejected) { + expect(isValidIdempotencyKey(value)).toBe(false); + expect(() => + defineMutationIntent({ + intentId: "intent-1", + operationId: "OP", + canonicalInputIdentity: "identity", + idempotencyKey: value, + createdAtMonotonicMs: 1, + }), + ).toThrow(TypeError); + } + }); + + it("accepts the bounded printable and Unicode values both sides allow", () => { + for (const value of ["key-1", "a".repeat(256), "키-값", "ключ"]) { + expect(isValidIdempotencyKey(value)).toBe(true); + expect( + defineMutationIntent({ + intentId: "intent-1", + operationId: "OP", + canonicalInputIdentity: "identity", + idempotencyKey: value, + createdAtMonotonicMs: 1, + }).idempotencyKey, + ).toBe(value); + } + }); +});