From fa2f699125e3a32dde080c31d196b03177c0ae45 Mon Sep 17 00:00:00 2001 From: DongHyeonka Date: Sun, 2 Aug 2026 01:21:50 +0900 Subject: [PATCH] fix: reject invalid keyed mutation intents --- src/adapters/http/http-execution-v3.ts | 112 +++++++++++++++++- .../http-execution-contract.test.ts | 75 ++++++++++++ tests/unit/http-execution-v3.test.ts | 93 ++++++++++++++- 3 files changed, 272 insertions(+), 8 deletions(-) diff --git a/src/adapters/http/http-execution-v3.ts b/src/adapters/http/http-execution-v3.ts index 7e86eb8..bf563b8 100644 --- a/src/adapters/http/http-execution-v3.ts +++ b/src/adapters/http/http-execution-v3.ts @@ -4,7 +4,11 @@ import { type InstalledHttpContract, } from "../../contracts/external-contract-runtime.ts"; import type { CacheScopeSnapshot } from "../../contracts/server-state-scope.ts"; -import type { MutationIntent } from "../../contracts/mutation-intent.ts"; +import { + defineMutationIntent, + MUTATION_INTENT_BOUNDS, + type MutationIntent, +} from "../../contracts/mutation-intent.ts"; import { decodeJsonBytes, isEffectivelyEmpty, @@ -40,6 +44,8 @@ export type SafeResponseMetadata = Readonly<{ }>; export type HttpContractViolationKind = + | "MISSING_IDEMPOTENCY_KEY" + | "UNEXPECTED_IDEMPOTENCY_KEY" | "UNEXPECTED_STATUS" | "UNEXPECTED_EMPTY_BODY" | "UNEXPECTED_BODY" @@ -174,6 +180,98 @@ const RETRY_BASE_DELAY_MS = 250; const RETRY_MAX_LOCAL_DELAY_MS = 2_000; const RETRY_AFTER_CEILING_MS = 5_000; +type MutationIntentValidation = + | Readonly<{ ok: true; intent?: MutationIntent }> + | Readonly<{ + ok: false; + violation: "MISSING_IDEMPOTENCY_KEY" | "UNEXPECTED_IDEMPOTENCY_KEY"; + }>; + +function validateMutationIntent( + contract: Readonly<{ + operationId: string; + retrySemantics: "SAFE" | "IDEMPOTENT" | "KEYED" | "NEVER"; + commandEffect: unknown | null; + }>, + intent: MutationIntent | undefined, +): MutationIntentValidation { + const isCommand = contract.commandEffect !== null; + const requiresKey = contract.retrySemantics === "KEYED"; + if (!isCommand) { + return intent === undefined + ? Object.freeze({ ok: true }) + : Object.freeze({ + ok: false, + violation: "UNEXPECTED_IDEMPOTENCY_KEY", + }); + } + if (intent === undefined) { + return requiresKey + ? Object.freeze({ ok: false, violation: "MISSING_IDEMPOTENCY_KEY" }) + : Object.freeze({ ok: true }); + } + + let validated: MutationIntent; + try { + validated = defineMutationIntent(intent); + } catch { + return Object.freeze({ + ok: false, + violation: requiresKey + ? "MISSING_IDEMPOTENCY_KEY" + : "UNEXPECTED_IDEMPOTENCY_KEY", + }); + } + if (validated.operationId !== contract.operationId) { + return Object.freeze({ + ok: false, + violation: requiresKey + ? "MISSING_IDEMPOTENCY_KEY" + : "UNEXPECTED_IDEMPOTENCY_KEY", + }); + } + + const key = validated.idempotencyKey; + if (requiresKey && !validIdempotencyKey(key)) { + return Object.freeze({ + ok: false, + violation: "MISSING_IDEMPOTENCY_KEY", + }); + } + if (!requiresKey && key !== undefined) { + return Object.freeze({ + ok: false, + violation: "UNEXPECTED_IDEMPOTENCY_KEY", + }); + } + 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 { @@ -274,6 +372,14 @@ export function createContractHttpExecutor( return finish(scopeFenced(preDispatchEffect(isCommand)), "NOT_STARTED"); } + const intentValidation = validateMutationIntent(contract, context.intent); + if (!intentValidation.ok) { + return finish( + violation(intentValidation.violation, "REQUEST", "NOT_STARTED"), + "NOT_STARTED", + ); + } + const validated = invokeValidator(contract.inputValidator, input); if (validated.outcome === "THROWN") { return finish( @@ -370,8 +476,8 @@ export function createContractHttpExecutor( if (contract.requestBody === "JSON") { headers["Content-Type"] = "application/json"; } - if (isCommand && context.intent?.idempotencyKey) { - headers["Idempotency-Key"] = context.intent.idempotencyKey; + if (intentValidation.intent?.idempotencyKey !== undefined) { + headers["Idempotency-Key"] = intentValidation.intent.idempotencyKey; } const retryCeiling = Math.min( diff --git a/tests/integration/http-execution-contract.test.ts b/tests/integration/http-execution-contract.test.ts index 1efc566..eebedd5 100644 --- a/tests/integration/http-execution-contract.test.ts +++ b/tests/integration/http-execution-contract.test.ts @@ -1,10 +1,12 @@ import { describe, expect, it, vi } from "vitest"; import { createHttpClient } from "../../src/adapters/http/client.ts"; +import { createContractHttpExecutor } from "../../src/adapters/http/http-execution-v3.ts"; import { entityQueryKeys, TEST_HTTP_CONTRACT, } from "../helpers/http-contract-fixture.ts"; +import { TEST_CREATE_HTTP_CONTRACT } from "../helpers/external-contract-fixture.ts"; function successResponse(data: unknown) { return Response.json({ @@ -48,6 +50,79 @@ function testClient(options: HttpDependencies) { } describe("HTTP operation execution contract", () => { + it("permits a keyless command intent but rejects an unexpected key before dispatch", async () => { + const attachCredentials = vi.fn(() => ({ + kind: "READY" as const, + headers: {}, + credentials: "omit" as const, + })); + const observedKeys: Array = []; + const fetcher = vi.fn( + async (_input: RequestInfo | URL, init?: RequestInit) => { + observedKeys.push(new Headers(init?.headers).get("Idempotency-Key")); + return Response.json( + { id: "created", name: "Created" }, + { status: 201 }, + ); + }, + ); + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + attachCredentials, + fetcher, + }); + const nonKeyedCommand = { + ...TEST_CREATE_HTTP_CONTRACT, + contract: { + ...TEST_CREATE_HTTP_CONTRACT.contract, + retrySemantics: "NEVER" as const, + commandRecovery: null, + }, + }; + const scope = Object.freeze({ + generation: 1, + fingerprint: "scope-1", + identities: Object.freeze({}) as never, + signal: new AbortController().signal, + isCurrent: () => true, + }); + const intent = Object.freeze({ + intentId: "intent-1", + operationId: "TEST_CREATE_ENTITY", + canonicalInputIdentity: "opaque-input-identity", + createdAtMonotonicMs: 1, + }); + + await expect( + executor.execute( + nonKeyedCommand, + { name: "created" }, + { scope, intent }, + ), + ).resolves.toMatchObject({ kind: "SUCCESS" }); + expect(observedKeys).toEqual([null]); + + attachCredentials.mockClear(); + fetcher.mockClear(); + await expect( + executor.execute( + nonKeyedCommand, + { name: "created" }, + { scope, intent: { ...intent, idempotencyKey: "unexpected-key" } }, + ), + ).resolves.toMatchObject({ + kind: "CONTRACT_VIOLATION", + violation: { + kind: "UNEXPECTED_IDEMPOTENCY_KEY", + operation: "REQUEST", + }, + effect: "NOT_STARTED", + }); + expect(attachCredentials).not.toHaveBeenCalled(); + expect(fetcher).not.toHaveBeenCalled(); + }); + it("fails auth-required execution before fetch when the session integration is unavailable", async () => { const fetcher = vi.fn(); const client = testClient({ diff --git a/tests/unit/http-execution-v3.test.ts b/tests/unit/http-execution-v3.test.ts index 48228c3..76e9ad9 100644 --- a/tests/unit/http-execution-v3.test.ts +++ b/tests/unit/http-execution-v3.test.ts @@ -22,15 +22,19 @@ const createInstalled: InstalledHttpContract = TEST_CREATE_HTTP_CONTRACT; function mutationIntent( - overrides: Readonly<{ intentId?: string; idempotencyKey?: string }> = {}, + overrides: Readonly<{ + intentId?: string; + operationId?: string; + idempotencyKey?: string | null; + }> = {}, ) { return Object.freeze({ intentId: overrides.intentId ?? "intent-1", - operationId: "TEST_CREATE_ENTITY", + operationId: overrides.operationId ?? "TEST_CREATE_ENTITY", canonicalInputIdentity: "opaque-input-identity", - ...(overrides.idempotencyKey === undefined - ? { idempotencyKey: "key-1" } - : { idempotencyKey: overrides.idempotencyKey }), + ...(overrides.idempotencyKey === null + ? {} + : { idempotencyKey: overrides.idempotencyKey ?? "key-1" }), createdAtMonotonicMs: 1, }); } @@ -61,6 +65,85 @@ async function flushMicrotasks(): Promise { } describe("descriptor-driven HTTP execution lifetime", () => { + it.each([ + ["absent intent", () => undefined], + ["empty key", () => mutationIntent({ idempotencyKey: "" })], + ["control-character key", () => mutationIntent({ idempotencyKey: "key\u0000private" })], + ["over-budget key", () => mutationIntent({ idempotencyKey: "k".repeat(257) })], + [ + "wrong operation", + () => mutationIntent({ operationId: "TEST_OTHER_COMMAND" }), + ], + ])( + "rejects a KEYED command with %s before credentials or fetch", + async (_label, intentFactory) => { + const attachCredentials = vi.fn(() => ({ + kind: "READY" as const, + headers: {}, + credentials: "omit" as const, + })); + const fetcher = vi.fn(); + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + attachCredentials, + fetcher, + }); + const intent = intentFactory(); + + await expect( + executor.execute( + createInstalled, + { name: "created" }, + { scope, ...(intent === undefined ? {} : { intent }) }, + ), + ).resolves.toMatchObject({ + kind: "CONTRACT_VIOLATION", + violation: { + kind: "MISSING_IDEMPOTENCY_KEY", + operation: "REQUEST", + }, + effect: "NOT_STARTED", + }); + expect(attachCredentials).not.toHaveBeenCalled(); + expect(fetcher).not.toHaveBeenCalled(); + }, + ); + + it.each([ + ["an intent without a key", mutationIntent({ idempotencyKey: null })], + ["an intent with a key", mutationIntent()], + ])( + "rejects a query carrying %s before credentials or fetch", + async (_label, intent) => { + const attachCredentials = vi.fn(() => ({ + kind: "READY" as const, + headers: {}, + credentials: "omit" as const, + })); + const fetcher = vi.fn(); + const executor = createContractHttpExecutor({ + baseUrl: "https://api.example/", + maxRetryAttempts: 0, + attachCredentials, + fetcher, + }); + + await expect( + executor.execute(installed, { limit: 20 }, { scope, intent }), + ).resolves.toMatchObject({ + kind: "CONTRACT_VIOLATION", + violation: { + kind: "UNEXPECTED_IDEMPOTENCY_KEY", + operation: "REQUEST", + }, + effect: "NOT_STARTED", + }); + expect(attachCredentials).not.toHaveBeenCalled(); + expect(fetcher).not.toHaveBeenCalled(); + }, + ); + it("reuses one supplied idempotency key across every physical retry", async () => { const observedKeys: Array = []; let attempt = 0;