From 3d629b868b5e2319258699bd1a212fe7ff12cc1d Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 13:01:39 +0200 Subject: [PATCH 1/4] fix(proxy): name the reason a control-plane signature was rejected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every failure path in verifyInternalControlPlaneSignature returned a bare `false`. An unconfigured verification key, a stripped x-token, a missing signature header and a genuinely bad signature were indistinguishable, with no log line at any of them. That cost real time on veryfront-issue-inbox#356. veryfront-api was not minting the request_method / request_path claims this repo began requiring in 0.1.1189 (#3251), so the bypass never fired and protected environments 302'd every control-plane run to the sign-in page. Nothing in the logs distinguished that from a missing key or a config drift. Failures now carry a reason: missing_x_token, verification_key_not_configured, missing_signature_header or signature_rejected. Ordinary non-internal routes stay silent. Also adds a cross-repo contract test. control-plane-signature.test.ts mints its own compliant JWS, so it proves the verifier works on a good token but never that veryfront-api produces one — which is exactly the gap that let #3251 ship. The new test mints the payload as veryfront-api does and pins both sides together. Refs veryfront-issue-inbox#356 --- ...ntrol-plane-signature.api-contract.test.ts | 202 ++++++++++++++++++ src/proxy/control-plane-signature.ts | 61 +++++- src/proxy/handler.ts | 6 +- 3 files changed, 256 insertions(+), 13 deletions(-) create mode 100644 src/proxy/control-plane-signature.api-contract.test.ts diff --git a/src/proxy/control-plane-signature.api-contract.test.ts b/src/proxy/control-plane-signature.api-contract.test.ts new file mode 100644 index 0000000000..2f3e0f8462 --- /dev/null +++ b/src/proxy/control-plane-signature.api-contract.test.ts @@ -0,0 +1,202 @@ +import "#veryfront/schemas/_test-setup.ts"; +import { assertEquals } from "#veryfront/testing/assert"; +import { afterEach, describe, it } from "#veryfront/testing/bdd"; +import { isAuthenticInternalControlPlaneCandidate } from "./control-plane-signature.ts"; + +/** + * Cross-repo contract: control-plane-signature.test.ts mints its own compliant + * JWS, so it never exercises what veryfront-api actually sends. This mints the + * payload exactly as veryfront-api's createControlPlaneRequestSignature does. + */ + +const PUBLIC_KEY_ENV = "CHANNEL_DISPATCH_SIGNING_PUBLIC_KEY"; +const RUN_STREAM_URL = + "http://outlook-agent-hvjoe9.preview.veryfront.org/api/control-plane/runs/r_1/stream"; +const encoder = new TextEncoder(); + +function base64url(data: string): string { + return btoa(data).replace(/\+/g, "-").replace(/\//g, "_").replace(/=+$/, ""); +} + +function base64urlBytes(bytes: Uint8Array): string { + return base64url(String.fromCharCode(...bytes)); +} + +async function sha256Base64url(value: string): Promise { + const digest = await crypto.subtle.digest("SHA-256", encoder.encode(value)); + return base64urlBytes(new Uint8Array(digest)); +} + +function encodePem(label: string, der: ArrayBuffer): string { + const base64 = btoa(String.fromCharCode(...new Uint8Array(der))); + const lines = base64.match(/.{1,64}/g) ?? [base64]; + return `-----BEGIN ${label}-----\n${lines.join("\n")}\n-----END ${label}-----`; +} + +/** Mint a control-plane JWS with exactly the claim set veryfront-api signs. */ +async function mintApiStyleJws( + body: string, + claimOverrides: Record = {}, +): Promise<{ jws: string; publicKeyPem: string }> { + const keyPair = await crypto.subtle.generateKey( + "Ed25519", + true, + ["sign", "verify"], + ) as CryptoKeyPair; + const publicKeyPem = encodePem( + "PUBLIC KEY", + await crypto.subtle.exportKey("spki", keyPair.publicKey), + ); + + const now = Math.floor(Date.now() / 1000); + const payload: Record = { + iss: "veryfront-api", + aud: "outlook-agent-hvjoe9", + sub: "r_1", + surface: "studio", + project_id: "979f3e04-e951-4807-8aa8-98530d9b8ba1", + request_hash: await sha256Base64url(body), + request_method: "POST", + request_path: "/api/control-plane/runs/r_1/stream", + iat: now, + exp: now + 300, + ...claimOverrides, + }; + for (const [key, value] of Object.entries(claimOverrides)) { + if (value === undefined) delete payload[key]; + } + + const encodedHeader = base64url(JSON.stringify({ alg: "EdDSA", typ: "JWT" })); + const encodedPayload = base64url(JSON.stringify(payload)); + const signature = await crypto.subtle.sign( + "Ed25519", + keyPair.privateKey, + encoder.encode(`${encodedHeader}.${encodedPayload}`), + ); + + return { + publicKeyPem, + jws: `${encodedHeader}.${encodedPayload}.${base64urlBytes(new Uint8Array(signature))}`, + }; +} + +async function verifyApiStyleRequest( + claimOverrides: Record = {}, +): Promise { + const body = JSON.stringify({ messages: [{ role: "user", content: "hi" }] }); + const { jws, publicKeyPem } = await mintApiStyleJws(body, claimOverrides); + Deno.env.set(PUBLIC_KEY_ENV, publicKeyPem); + + const req = new Request(RUN_STREAM_URL, { + method: "POST", + headers: { + "x-token": "user-token", + "x-veryfront-control-plane-jws": jws, + }, + body, + }); + + return await isAuthenticInternalControlPlaneCandidate(req, new URL(RUN_STREAM_URL)); +} + +describe("control-plane signature: veryfront-api contract", () => { + afterEach(() => { + Deno.env.delete(PUBLIC_KEY_ENV); + }); + + it("accepts the JWS veryfront-api actually mints", async () => { + assertEquals(await verifyApiStyleRequest(), true); + }); + + // 0.1.1189 (#3251) added these claims; veryfront-api kept minting the old set + // and every run against a protected environment was 302'd to sign-in. + it("rejects a JWS missing request_method", async () => { + assertEquals(await verifyApiStyleRequest({ request_method: undefined }), false); + }); + + it("rejects a JWS missing request_path", async () => { + assertEquals(await verifyApiStyleRequest({ request_path: undefined }), false); + }); + + it("rejects a JWS whose request_method does not match the request", async () => { + assertEquals(await verifyApiStyleRequest({ request_method: "DELETE" }), false); + }); + + it("rejects a JWS whose request_path does not match the request", async () => { + assertEquals( + await verifyApiStyleRequest({ request_path: "/api/control-plane/runs/r_1" }), + false, + ); + }); +}); + +describe("control-plane signature: rejection reasons", () => { + afterEach(() => { + Deno.env.delete(PUBLIC_KEY_ENV); + }); + + async function reasonFor( + build: (jws: string) => { headers: Record; publicKeyPem?: string }, + ): Promise { + const body = JSON.stringify({ messages: [] }); + const { jws, publicKeyPem } = await mintApiStyleJws(body); + const built = build(jws); + Deno.env.delete(PUBLIC_KEY_ENV); + if (built.publicKeyPem ?? publicKeyPem) { + Deno.env.set(PUBLIC_KEY_ENV, built.publicKeyPem ?? publicKeyPem); + } + + const reasons: string[] = []; + const req = new Request(RUN_STREAM_URL, { method: "POST", headers: built.headers, body }); + await isAuthenticInternalControlPlaneCandidate(req, new URL(RUN_STREAM_URL), { + warn: (_msg, extra) => { + if (typeof extra?.reason === "string") reasons.push(extra.reason); + }, + }); + return reasons[0]; + } + + it("reports a missing x-token", async () => { + assertEquals( + await reasonFor((jws) => ({ headers: { "x-veryfront-control-plane-jws": jws } })), + "missing_x_token", + ); + }); + + it("reports an unconfigured verification key", async () => { + assertEquals( + await reasonFor((jws) => ({ + headers: { "x-token": "t", "x-veryfront-control-plane-jws": jws }, + publicKeyPem: "", + })), + "verification_key_not_configured", + ); + }); + + it("reports a missing signature header", async () => { + assertEquals( + await reasonFor(() => ({ headers: { "x-token": "t" } })), + "missing_signature_header", + ); + }); + + it("reports a rejected signature", async () => { + assertEquals( + await reasonFor((jws) => ({ + headers: { "x-token": "t", "x-veryfront-control-plane-jws": `${jws}tampered` }, + })), + "signature_rejected", + ); + }); + + it("stays silent for ordinary non-internal routes", async () => { + const reasons: string[] = []; + const pageUrl = "http://slug.preview.veryfront.org/"; + await isAuthenticInternalControlPlaneCandidate( + new Request(pageUrl, { method: "GET" }), + new URL(pageUrl), + { warn: (_msg, extra) => reasons.push(String(extra?.reason)) }, + ); + assertEquals(reasons.length, 0); + }); +}); diff --git a/src/proxy/control-plane-signature.ts b/src/proxy/control-plane-signature.ts index 57f9be769e..07f44bd5d5 100644 --- a/src/proxy/control-plane-signature.ts +++ b/src/proxy/control-plane-signature.ts @@ -33,6 +33,10 @@ import { DEFAULT_MAX_BODY_SIZE_BYTES } from "#veryfront/utils/constants/index.ts import { isWellFormedString } from "#veryfront/utils/is-well-formed-string.ts"; import { isCanonicalOpaqueProjectIdentifier } from "#veryfront/utils/project-identity.ts"; +export interface InternalControlPlaneSignatureLogger { + warn: (msg: string, extra?: Record) => void; +} + const CONTROL_PLANE_JWS_HEADER = "x-veryfront-control-plane-jws"; const DISPATCH_JWS_HEADER = "x-veryfront-dispatch-jws"; @@ -250,25 +254,33 @@ export async function resolveVerifiedControlPlaneBranchBinding( return parseVerifiedBranchBinding(rawBody); } -async function verifyInternalControlPlaneSignature( +/** Why a signed-internal check did not admit the request. */ +type InternalControlPlaneRejection = + | "not_an_internal_route" + | "missing_x_token" + | "verification_key_not_configured" + | "missing_signature_header" + | "signature_rejected"; + +async function checkInternalControlPlaneSignature( req: Request, url: URL, binding?: InternalControlPlaneProjectBinding, -): Promise { +): Promise { const routeKind = classifyInternalControlPlaneRequest(req.method, url.pathname); - if (routeKind === "public" || routeKind === "reserved") return false; + if (routeKind === "public" || routeKind === "reserved") return "not_an_internal_route"; // The candidate only matters when there is an x-token to use for metadata // lookup or forward after the resolved project binding succeeds. - if (!req.headers.get("x-token")) return false; + if (!req.headers.get("x-token")) return "missing_x_token"; const publicKeyPem = getHostEnv(PUBLIC_KEY_ENV_VAR); - if (!publicKeyPem) return false; + if (!publicKeyPem) return "verification_key_not_configured"; if (routeKind === "dispatch") { const dispatchJws = req.headers.get(DISPATCH_JWS_HEADER); - if (!dispatchJws) return false; - return await verifyDispatchJwsSignature(dispatchJws, { + if (!dispatchJws) return "missing_signature_header"; + const verified = await verifyDispatchJwsSignature(dispatchJws, { publicKeyPem, maxAgeSeconds: MAX_SIGNATURE_AGE_SECONDS, ...(binding @@ -278,11 +290,12 @@ async function verifyInternalControlPlaneSignature( } : {}), }); + return verified ? null : "signature_rejected"; } const controlPlaneJws = req.headers.get(CONTROL_PLANE_JWS_HEADER); - if (!controlPlaneJws) return false; - return await verifyControlPlaneJwsSignature(controlPlaneJws, { + if (!controlPlaneJws) return "missing_signature_header"; + const verified = await verifyControlPlaneJwsSignature(controlPlaneJws, { publicKeyPem, maxAgeSeconds: MAX_SIGNATURE_AGE_SECONDS, requestMethod: req.method, @@ -294,6 +307,30 @@ async function verifyInternalControlPlaneSignature( } : {}), }); + return verified ? null : "signature_rejected"; +} + +async function verifyInternalControlPlaneSignature( + req: Request, + url: URL, + binding?: InternalControlPlaneProjectBinding, + logger?: InternalControlPlaneSignatureLogger, +): Promise { + const rejection = await checkInternalControlPlaneSignature(req, url, binding); + if (rejection === null) return true; + + // Every ordinary page request lands here, so only log the cases where a + // request that meant to be internal was turned away. + if (rejection !== "not_an_internal_route") { + logger?.warn("Internal control-plane signature not accepted", { + reason: rejection, + method: req.method, + pathname: url.pathname, + ...(binding?.audience ? { audience: binding.audience } : {}), + }); + } + + return false; } /** @@ -308,8 +345,9 @@ async function verifyInternalControlPlaneSignature( export async function isAuthenticInternalControlPlaneCandidate( req: Request, url: URL, + logger?: InternalControlPlaneSignatureLogger, ): Promise { - return await verifyInternalControlPlaneSignature(req, url); + return await verifyInternalControlPlaneSignature(req, url, undefined, logger); } /** @@ -323,7 +361,8 @@ export async function isVerifiedInternalControlPlaneRequest( req: Request, url: URL, binding: InternalControlPlaneProjectBinding, + logger?: InternalControlPlaneSignatureLogger, ): Promise { if (!binding.audience) return false; - return await verifyInternalControlPlaneSignature(req, url, binding); + return await verifyInternalControlPlaneSignature(req, url, binding, logger); } diff --git a/src/proxy/handler.ts b/src/proxy/handler.ts index edcd7babbe..51dc77dfa7 100644 --- a/src/proxy/handler.ts +++ b/src/proxy/handler.ts @@ -794,11 +794,13 @@ export function createProxyHandler(options: ProxyHandlerOptions) { req, url, { audience: projectSlug }, + logger, ); } else if (isCustomDomain) { signedInternalControlPlaneCandidate = await isAuthenticInternalControlPlaneCandidate( req, url, + logger, ); } let signedInternalControlPlaneRequest = false; @@ -810,7 +812,7 @@ export function createProxyHandler(options: ProxyHandlerOptions) { await isVerifiedInternalControlPlaneRequest(req, url, { audience: resolvedProjectSlug, expectedProjectId: resolvedProjectId, - }); + }, logger); if (!projectSlug && parsedDomain.isVeryfrontDomain) { return { @@ -1268,7 +1270,7 @@ export function createProxyHandler(options: ProxyHandlerOptions) { const scope = getScope(parsedDomain.environment); const projectSlug = parsedDomain.slug ?? undefined; const signedInternalControlPlaneRequest = projectSlug !== undefined && - await isVerifiedInternalControlPlaneRequest(req, url, { audience: projectSlug }); + await isVerifiedInternalControlPlaneRequest(req, url, { audience: projectSlug }, logger); const { token } = await resolveProxyRequestToken({ req, url, From 3b22e4143e6f15ad6227efd896fdccb5a30e7092 Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 13:18:57 +0200 Subject: [PATCH 2/4] test(proxy): use placeholder identifiers in the contract fixture veryfront-code is public. The fixture carried a real project slug and project UUID; replace them with the placeholders the sibling signature test already uses (protected / proj-1), and switch the test-helper imports to the explicit .ts paths the guidelines require. Addresses review feedback on #3357. --- .../control-plane-signature.api-contract.test.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/proxy/control-plane-signature.api-contract.test.ts b/src/proxy/control-plane-signature.api-contract.test.ts index 2f3e0f8462..bb93b603f3 100644 --- a/src/proxy/control-plane-signature.api-contract.test.ts +++ b/src/proxy/control-plane-signature.api-contract.test.ts @@ -1,6 +1,6 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals } from "#veryfront/testing/assert"; -import { afterEach, describe, it } from "#veryfront/testing/bdd"; +import { assertEquals } from "#veryfront/testing/assert.ts"; +import { afterEach, describe, it } from "#veryfront/testing/bdd.ts"; import { isAuthenticInternalControlPlaneCandidate } from "./control-plane-signature.ts"; /** @@ -10,8 +10,7 @@ import { isAuthenticInternalControlPlaneCandidate } from "./control-plane-signat */ const PUBLIC_KEY_ENV = "CHANNEL_DISPATCH_SIGNING_PUBLIC_KEY"; -const RUN_STREAM_URL = - "http://outlook-agent-hvjoe9.preview.veryfront.org/api/control-plane/runs/r_1/stream"; +const RUN_STREAM_URL = "http://protected.preview.veryfront.com/api/control-plane/runs/r_1/stream"; const encoder = new TextEncoder(); function base64url(data: string): string { @@ -51,10 +50,10 @@ async function mintApiStyleJws( const now = Math.floor(Date.now() / 1000); const payload: Record = { iss: "veryfront-api", - aud: "outlook-agent-hvjoe9", + aud: "protected", sub: "r_1", surface: "studio", - project_id: "979f3e04-e951-4807-8aa8-98530d9b8ba1", + project_id: "proj-1", request_hash: await sha256Base64url(body), request_method: "POST", request_path: "/api/control-plane/runs/r_1/stream", @@ -191,7 +190,7 @@ describe("control-plane signature: rejection reasons", () => { it("stays silent for ordinary non-internal routes", async () => { const reasons: string[] = []; - const pageUrl = "http://slug.preview.veryfront.org/"; + const pageUrl = "http://protected.preview.veryfront.com/"; await isAuthenticInternalControlPlaneCandidate( new Request(pageUrl, { method: "GET" }), new URL(pageUrl), From 10d55fe4f54b8ea56da72240eb4f846b9104c438 Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 13:37:59 +0200 Subject: [PATCH 3/4] fix(proxy): bound rejection logging and pin the full claim contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on #3357. B1 — unauthenticated log amplification. The warn logged url.pathname verbatim. Two admissible route patterns carry an unbounded runId segment, so a client with no headers at all could POST an 8KB path and write one 8KB line per request into log ingest, drowning the signal this feature exists to surface. Now: pathname bounded to 256 code units, and the two 'missing' reasons are logged only when the caller actually presented a signature header. Anonymous traffic is silent; every reason still fires for a real internal caller. B2 — the contract test asserted through the unbound candidate check, which verifies neither aud, project_id nor request_hash. A JWS with a wrong audience and a hash of a different body was accepted. The earlier identifier scrub is the proof: it changed aud and project_id and nothing failed. Now asserts through isVerifiedInternalControlPlaneRequest with a binding, with explicit rejection cases for a drifted aud and project_id, plus body-binding cases through resolveVerifiedControlPlaneBranchBinding that pin request_hash. Also: rename not_an_internal_route -> route_not_admissible, since reserved routes are internal but inadmissible; export the reason union so dashboards do not couple to an unexported type; assert exactly one warn per rejection; drop a redundant undefined-deletion loop; document the logging side effect in the module header. --- ...ntrol-plane-signature.api-contract.test.ts | 126 +++++++++++++++--- src/proxy/control-plane-signature.ts | 41 ++++-- 2 files changed, 143 insertions(+), 24 deletions(-) diff --git a/src/proxy/control-plane-signature.api-contract.test.ts b/src/proxy/control-plane-signature.api-contract.test.ts index bb93b603f3..eab61189cc 100644 --- a/src/proxy/control-plane-signature.api-contract.test.ts +++ b/src/proxy/control-plane-signature.api-contract.test.ts @@ -1,7 +1,11 @@ import "#veryfront/schemas/_test-setup.ts"; -import { assertEquals } from "#veryfront/testing/assert.ts"; +import { assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; import { afterEach, describe, it } from "#veryfront/testing/bdd.ts"; -import { isAuthenticInternalControlPlaneCandidate } from "./control-plane-signature.ts"; +import { + isAuthenticInternalControlPlaneCandidate, + isVerifiedInternalControlPlaneRequest, + resolveVerifiedControlPlaneBranchBinding, +} from "./control-plane-signature.ts"; /** * Cross-repo contract: control-plane-signature.test.ts mints its own compliant @@ -61,10 +65,6 @@ async function mintApiStyleJws( exp: now + 300, ...claimOverrides, }; - for (const [key, value] of Object.entries(claimOverrides)) { - if (value === undefined) delete payload[key]; - } - const encodedHeader = base64url(JSON.stringify({ alg: "EdDSA", typ: "JWT" })); const encodedPayload = base64url(JSON.stringify(payload)); const signature = await crypto.subtle.sign( @@ -79,6 +79,11 @@ async function mintApiStyleJws( }; } +/** + * Verify through the BOUND entry point. The unbound candidate check leaves aud, + * project_id and request_hash unverified, so asserting through it would let + * those three claims drift silently. + */ async function verifyApiStyleRequest( claimOverrides: Record = {}, ): Promise { @@ -95,7 +100,10 @@ async function verifyApiStyleRequest( body, }); - return await isAuthenticInternalControlPlaneCandidate(req, new URL(RUN_STREAM_URL)); + return await isVerifiedInternalControlPlaneRequest(req, new URL(RUN_STREAM_URL), { + audience: "protected", + expectedProjectId: "proj-1", + }); } describe("control-plane signature: veryfront-api contract", () => { @@ -121,6 +129,16 @@ describe("control-plane signature: veryfront-api contract", () => { assertEquals(await verifyApiStyleRequest({ request_method: "DELETE" }), false); }); + // Verified through the bound entry point, so a drifted audience or project id + // is caught. The unbound candidate check leaves both unverified. + it("rejects a JWS whose aud is not the bound project", async () => { + assertEquals(await verifyApiStyleRequest({ aud: "another-project" }), false); + }); + + it("rejects a JWS whose project_id is not the bound project", async () => { + assertEquals(await verifyApiStyleRequest({ project_id: "another-project-id" }), false); + }); + it("rejects a JWS whose request_path does not match the request", async () => { assertEquals( await verifyApiStyleRequest({ request_path: "/api/control-plane/runs/r_1" }), @@ -129,29 +147,78 @@ describe("control-plane signature: veryfront-api contract", () => { }); }); +describe("control-plane signature: body binding", () => { + afterEach(() => { + Deno.env.delete(PUBLIC_KEY_ENV); + }); + + async function resolveBinding(signedBody: string, sentBody: string) { + const { jws, publicKeyPem } = await mintApiStyleJws(signedBody); + Deno.env.set(PUBLIC_KEY_ENV, publicKeyPem); + const req = new Request(RUN_STREAM_URL, { + method: "POST", + headers: { "x-token": "t", "x-veryfront-control-plane-jws": jws }, + body: sentBody, + }); + return await resolveVerifiedControlPlaneBranchBinding(req, new URL(RUN_STREAM_URL), { + audience: "protected", + expectedProjectId: "proj-1", + }); + } + + const RUN_BODY = JSON.stringify({ + run: { project: {} }, + agentSource: { type: "release" }, + }); + + it("accepts a body matching the signed request_hash", async () => { + assertEquals(await resolveBinding(RUN_BODY, RUN_BODY), {}); + }); + + it("rejects a body that does not match the signed request_hash", async () => { + await assertRejects(() => + resolveBinding( + RUN_BODY, + JSON.stringify({ run: { project: {} }, agentSource: { type: "release" }, tampered: true }), + ) + ); + }); +}); + describe("control-plane signature: rejection reasons", () => { afterEach(() => { Deno.env.delete(PUBLIC_KEY_ENV); }); - async function reasonFor( + async function reasonsFor( build: (jws: string) => { headers: Record; publicKeyPem?: string }, - ): Promise { + url = RUN_STREAM_URL, + ): Promise<{ reasons: string[]; pathnames: string[] }> { const body = JSON.stringify({ messages: [] }); const { jws, publicKeyPem } = await mintApiStyleJws(body); const built = build(jws); Deno.env.delete(PUBLIC_KEY_ENV); - if (built.publicKeyPem ?? publicKeyPem) { - Deno.env.set(PUBLIC_KEY_ENV, built.publicKeyPem ?? publicKeyPem); - } + const key = built.publicKeyPem ?? publicKeyPem; + if (key !== "") Deno.env.set(PUBLIC_KEY_ENV, key); const reasons: string[] = []; - const req = new Request(RUN_STREAM_URL, { method: "POST", headers: built.headers, body }); - await isAuthenticInternalControlPlaneCandidate(req, new URL(RUN_STREAM_URL), { + const pathnames: string[] = []; + const req = new Request(url, { method: "POST", headers: built.headers, body }); + await isAuthenticInternalControlPlaneCandidate(req, new URL(url), { warn: (_msg, extra) => { if (typeof extra?.reason === "string") reasons.push(extra.reason); + if (typeof extra?.pathname === "string") pathnames.push(extra.pathname); }, }); + return { reasons, pathnames }; + } + + /** Exactly one warn, carrying the expected reason. */ + async function reasonFor( + build: (jws: string) => { headers: Record; publicKeyPem?: string }, + ): Promise { + const { reasons } = await reasonsFor(build); + assertEquals(reasons.length, 1); return reasons[0]; } @@ -172,13 +239,40 @@ describe("control-plane signature: rejection reasons", () => { ); }); - it("reports a missing signature header", async () => { + it("reports a missing control-plane header when a dispatch signature was presented", async () => { assertEquals( - await reasonFor(() => ({ headers: { "x-token": "t" } })), + await reasonFor((jws) => ({ headers: { "x-token": "t", "x-veryfront-dispatch-jws": jws } })), "missing_signature_header", ); }); + // An unauthenticated client picks the runId segment, so logging one line per + // request would be a remote write into log ingest. + it("stays silent for a caller that presented no signature header", async () => { + const { reasons } = await reasonsFor(() => ({ headers: { "x-token": "t" } })); + assertEquals(reasons, []); + }); + + it("stays silent for an unauthenticated request with a huge path", async () => { + const huge = `http://protected.preview.veryfront.com/api/control-plane/runs/${ + "A".repeat(8000) + }/stream`; + const { reasons } = await reasonsFor(() => ({ headers: {} }), huge); + assertEquals(reasons, []); + }); + + it("bounds the logged pathname", async () => { + const huge = `http://protected.preview.veryfront.com/api/control-plane/runs/${ + "A".repeat(8000) + }/stream`; + const { pathnames } = await reasonsFor( + (jws) => ({ headers: { "x-veryfront-control-plane-jws": jws } }), + huge, + ); + assertEquals(pathnames.length, 1); + assertEquals(pathnames[0].length, 256); + }); + it("reports a rejected signature", async () => { assertEquals( await reasonFor((jws) => ({ diff --git a/src/proxy/control-plane-signature.ts b/src/proxy/control-plane-signature.ts index 07f44bd5d5..42c4f64e14 100644 --- a/src/proxy/control-plane-signature.ts +++ b/src/proxy/control-plane-signature.ts @@ -19,6 +19,10 @@ * not consume the body: authoritative body-hash verification still runs in the * renderer. Signature headers remain available to that downstream verifier. * + * Rejections are logged with a reason so a turned-away internal caller can be + * diagnosed from one line. The reason is never returned to the client, and + * anonymous traffic that presented no signature header is not logged at all. + * * @module proxy/control-plane-signature */ @@ -46,6 +50,7 @@ export const INTERNAL_CONTROL_PLANE_SIGNATURE_HEADERS = [ DISPATCH_JWS_HEADER, ] as const; +const MAX_LOGGED_PATHNAME_CODE_UNITS = 256; const PUBLIC_KEY_ENV_VAR = "CHANNEL_DISPATCH_SIGNING_PUBLIC_KEY"; const MAX_SIGNATURE_AGE_SECONDS = 60; const MAX_BRANCH_NAME_CODE_UNITS = 255; @@ -254,9 +259,14 @@ export async function resolveVerifiedControlPlaneBranchBinding( return parseVerifiedBranchBinding(rawBody); } -/** Why a signed-internal check did not admit the request. */ -type InternalControlPlaneRejection = - | "not_an_internal_route" +/** + * Why a signed-internal check did not admit the request. + * + * "reserved" routes are internal but never admissible, so they share the silent + * `route_not_admissible` reason with ordinary public traffic. + */ +export type InternalControlPlaneRejection = + | "route_not_admissible" | "missing_x_token" | "verification_key_not_configured" | "missing_signature_header" @@ -268,7 +278,7 @@ async function checkInternalControlPlaneSignature( binding?: InternalControlPlaneProjectBinding, ): Promise { const routeKind = classifyInternalControlPlaneRequest(req.method, url.pathname); - if (routeKind === "public" || routeKind === "reserved") return "not_an_internal_route"; + if (routeKind === "public" || routeKind === "reserved") return "route_not_admissible"; // The candidate only matters when there is an x-token to use for metadata // lookup or forward after the resolved project binding succeeds. @@ -319,13 +329,14 @@ async function verifyInternalControlPlaneSignature( const rejection = await checkInternalControlPlaneSignature(req, url, binding); if (rejection === null) return true; - // Every ordinary page request lands here, so only log the cases where a - // request that meant to be internal was turned away. - if (rejection !== "not_an_internal_route") { + if (shouldLogRejection(req, rejection)) { logger?.warn("Internal control-plane signature not accepted", { reason: rejection, method: req.method, - pathname: url.pathname, + // Two admissible route patterns carry an unbounded runId segment, and any + // unauthenticated client can choose it. Logging it whole is a remote + // write into log ingest, so bound it. + pathname: url.pathname.slice(0, MAX_LOGGED_PATHNAME_CODE_UNITS), ...(binding?.audience ? { audience: binding.audience } : {}), }); } @@ -333,6 +344,20 @@ async function verifyInternalControlPlaneSignature( return false; } +/** + * Log only rejections that describe a caller which tried to authenticate. + * + * A request carrying no signature header at all is anonymous internet traffic + * that chose its own path; one line per request is amplification, and it buries + * the rejections that describe a real internal caller. Every reason still logs + * once a signature header is present. + */ +function shouldLogRejection(req: Request, rejection: InternalControlPlaneRejection): boolean { + if (rejection === "route_not_admissible") return false; + if (rejection === "verification_key_not_configured") return true; + return INTERNAL_CONTROL_PLANE_SIGNATURE_HEADERS.some((header) => req.headers.get(header)); +} + /** * Authenticate a signed internal request before a custom domain has resolved * to its project audience. From 33f407e3f84de549d82e71f2639c9e285bb3d648 Mon Sep 17 00:00:00 2001 From: Kentaro Wakayama Date: Tue, 4 Aug 2026 13:53:18 +0200 Subject: [PATCH 4/4] fix(proxy): make the contract test typecheck under noUncheckedIndexedAccess MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deno check failed on the test added in the previous commit: with noUncheckedIndexedAccess, pathnames[0] is string | undefined, so pathnames[0].length is TS2532. The test-typecheck ratchet in the ci (lint) chain gates on this. Optional-chain it — the preceding length assertion already guarantees presence. Also: - shouldLogRejection uses headers.has rather than get() truthiness, so a set-but-empty signature header counts as presented and its rejection is logged rather than silently dropped. - Name sub and surface in the test as deliberately unbound at the proxy: the run id is already pinned through request_path, and surface is only checked for membership of CONTROL_PLANE_SURFACES. --- src/proxy/control-plane-signature.api-contract.test.ts | 7 ++++++- src/proxy/control-plane-signature.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/proxy/control-plane-signature.api-contract.test.ts b/src/proxy/control-plane-signature.api-contract.test.ts index eab61189cc..fa790319b9 100644 --- a/src/proxy/control-plane-signature.api-contract.test.ts +++ b/src/proxy/control-plane-signature.api-contract.test.ts @@ -83,6 +83,11 @@ async function mintApiStyleJws( * Verify through the BOUND entry point. The unbound candidate check leaves aud, * project_id and request_hash unverified, so asserting through it would let * those three claims drift silently. + * + * Two claims stay deliberately unbound here. `sub` is never compared on this + * path — the run id is already pinned through request_path — and `surface` is + * only checked for membership of CONTROL_PLANE_SURFACES, because the proxy has + * no business asserting which surface a caller speaks for. */ async function verifyApiStyleRequest( claimOverrides: Record = {}, @@ -270,7 +275,7 @@ describe("control-plane signature: rejection reasons", () => { huge, ); assertEquals(pathnames.length, 1); - assertEquals(pathnames[0].length, 256); + assertEquals(pathnames[0]?.length, 256); }); it("reports a rejected signature", async () => { diff --git a/src/proxy/control-plane-signature.ts b/src/proxy/control-plane-signature.ts index 42c4f64e14..cda6fd38e1 100644 --- a/src/proxy/control-plane-signature.ts +++ b/src/proxy/control-plane-signature.ts @@ -355,7 +355,7 @@ async function verifyInternalControlPlaneSignature( function shouldLogRejection(req: Request, rejection: InternalControlPlaneRejection): boolean { if (rejection === "route_not_admissible") return false; if (rejection === "verification_key_not_configured") return true; - return INTERNAL_CONTROL_PLANE_SIGNATURE_HEADERS.some((header) => req.headers.get(header)); + return INTERNAL_CONTROL_PLANE_SIGNATURE_HEADERS.some((header) => req.headers.has(header)); } /**