fix: reject invalid keyed mutation intents
This commit is contained in:
@@ -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(
|
||||
|
||||
@@ -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<string | null> = [];
|
||||
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({
|
||||
|
||||
@@ -22,15 +22,19 @@ const createInstalled: InstalledHttpContract<unknown, unknown, unknown> =
|
||||
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<void> {
|
||||
}
|
||||
|
||||
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<string | null> = [];
|
||||
let attempt = 0;
|
||||
|
||||
Reference in New Issue
Block a user