diff --git a/changelog.d/fixes/10544-a2a-tasks-timing-safe.md b/changelog.d/fixes/10544-a2a-tasks-timing-safe.md new file mode 100644 index 0000000000..f68ac49e3d --- /dev/null +++ b/changelog.d/fixes/10544-a2a-tasks-timing-safe.md @@ -0,0 +1 @@ +- **fix(a2a):** use a constant-time bearer compare in `/api/a2a/tasks` via `crypto.timingSafeEqual`, matching the `tokensMatch` helper already used in `src/app/a2a/route.ts` and removing the last non-constant secret comparison in the repo ([#10544](https://github.com/diegosouzapw/OmniRoute/pull/10544)) diff --git a/src/app/api/a2a/tasks/route.ts b/src/app/api/a2a/tasks/route.ts index 02e264c85a..ddd1ad60f1 100644 --- a/src/app/api/a2a/tasks/route.ts +++ b/src/app/api/a2a/tasks/route.ts @@ -1,3 +1,4 @@ +import { timingSafeEqual } from "node:crypto"; import { NextResponse } from "next/server"; import { z } from "zod"; @@ -68,12 +69,32 @@ const delegationSchema = z.object({ .optional(), }); -/** Mesma semântica de auth do JSON-RPC A2A (src/app/a2a/route.ts): Bearer vs OMNIROUTE_API_KEY; aberto se não configurada. */ -function authenticateA2A(request: Request): boolean { +/** + * Constant-time comparison of the presented bearer token against the configured + * key. A plain `===` short-circuits on the first differing byte, leaking the + * length of the shared prefix through response timing; `timingSafeEqual` does + * not. It requires equal-length buffers, so mismatched lengths are rejected up + * front (the length itself is not secret). + * + * Exported as a test seam only — not part of the route contract. + */ +export function tokensMatch(provided: string, expected: string): boolean { + const a = Buffer.from(provided); + const b = Buffer.from(expected); + if (a.length !== b.length) return false; + return timingSafeEqual(a, b); +} + +/** + * Mesma semântica de auth do JSON-RPC A2A (src/app/a2a/route.ts): Bearer vs OMNIROUTE_API_KEY; aberto se não configurada. + * + * Exported as a test seam only — not part of the route contract. + */ +export function authenticateA2A(request: Request): boolean { const configuredKey = process.env.OMNIROUTE_API_KEY; if (!configuredKey) return true; const token = (request.headers.get("authorization") || "").replace(/^Bearer\s+/i, ""); - return token === configuredKey; + return tokensMatch(token, configuredKey); } /** diff --git a/tests/unit/a2a-tasks-auth.test.ts b/tests/unit/a2a-tasks-auth.test.ts new file mode 100644 index 0000000000..5569905d66 --- /dev/null +++ b/tests/unit/a2a-tasks-auth.test.ts @@ -0,0 +1,91 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const TASKS_ROUTE = path.resolve(__dirname, "../../src/app/api/a2a/tasks/route.ts"); +const A2A_ROUTE = path.resolve(__dirname, "../../src/app/a2a/route.ts"); + +const source = fs.readFileSync(TASKS_ROUTE, "utf-8"); + +const { tokensMatch, authenticateA2A } = await import("../../src/app/api/a2a/tasks/route.ts"); + +function hasImport(src: string, name: string, from: string): boolean { + const pattern = new RegExp( + `import\\s+\\{[^}]*\\b${name}\\b[^}]*\\}\\s+from\\s+["']${from}["']` + ); + return pattern.test(src); +} + +test("tasks route uses the same constant-time contract as src/app/a2a/route.ts", () => { + const a2aSource = fs.readFileSync(A2A_ROUTE, "utf-8"); + assert.ok( + hasImport(a2aSource, "timingSafeEqual", "node:crypto"), + "reference route imports timingSafeEqual" + ); + + assert.ok( + hasImport(source, "timingSafeEqual", "node:crypto"), + "tasks route imports timingSafeEqual" + ); + assert.ok( + /\btokensMatch\s*\(\s*token\s*,\s*configuredKey\s*\)/.test(source), + "tasks route authenticates with tokensMatch(token, configuredKey)" + ); + assert.ok( + !/return\s+token\s*===\s*configuredKey\s*;/.test(source), + "tasks route no longer uses a plain === bearer compare" + ); +}); + +test("tokensMatch behaves like the helper in src/app/a2a/route.ts", () => { + assert.equal(tokensMatch("omniroute-a2a-test-key", "omniroute-a2a-test-key"), true); + assert.equal( + tokensMatch("x".repeat("omniroute-a2a-test-key".length), "omniroute-a2a-test-key"), + false, + "same-length different token is rejected" + ); + assert.equal(tokensMatch("", "omniroute-a2a-test-key"), false, "empty token is rejected"); + assert.equal( + tokensMatch("short", "omniroute-a2a-test-key"), + false, + "different-length token is rejected without throwing" + ); +}); + +test("authenticateA2A preserves the documented semantics", () => { + const API_KEY = "omniroute-a2a-test-key"; + + function makeRequest(token?: string): Request { + return { + headers: { + get(name: string) { + if (name.toLowerCase() !== "authorization") return null; + return token === undefined ? null : `Bearer ${token}`; + }, + }, + } as unknown as Request; + } + + delete process.env.OMNIROUTE_API_KEY; + assert.equal( + authenticateA2A(makeRequest()), + true, + "when OMNIROUTE_API_KEY is not set the route is open" + ); + + process.env.OMNIROUTE_API_KEY = API_KEY; + assert.equal(authenticateA2A(makeRequest(API_KEY)), true, "a valid bearer token passes auth"); + assert.equal( + authenticateA2A(makeRequest("x".repeat(API_KEY.length))), + false, + "a same-length but different token is rejected" + ); + assert.equal(authenticateA2A(makeRequest("")), false, "an empty bearer token is rejected"); + + delete process.env.OMNIROUTE_API_KEY; +});