mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-21 22:52:19 +03:00
fix(security): honor REQUIRE_API_KEY on the /a2a route
/a2a is outside the authz proxy matcher, so the REQUIRE_API_KEY posture the pipeline enforces for /v1 never ran there — the route accepted every caller whenever OMNIROUTE_API_KEY was unset (the shipped default). authenticate() now applies the same posture directly: a valid OmniRoute key when REQUIRE_API_KEY is on, the legacy explicit A2A key otherwise, and keyless local-first only when nothing is configured (matching /v1). A2A stays off by default. Reported by @rafaelfiguereod-stack via GHSA-v54m-6rm3-p565.
This commit is contained in:
@@ -17,6 +17,8 @@ import { logRoutingDecision } from "@/lib/a2a/routingLogger";
|
||||
import { createA2AStream, SSE_HEADERS } from "@/lib/a2a/streaming";
|
||||
import { A2A_SKILL_HANDLERS, executeA2ATaskWithState } from "@/lib/a2a/taskExecution";
|
||||
import { getSettings } from "@/lib/db/settings";
|
||||
import { isRequireApiKeyEnabled } from "@/shared/utils/featureFlags";
|
||||
import { extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
|
||||
// ============ A2A v1.0 ↔ v0.3 compatibility layer ============
|
||||
// A2A 1.0 renamed the JSON-RPC methods (message/send → SendMessage,
|
||||
@@ -136,14 +138,25 @@ function tokensMatch(provided: string, expected: string): boolean {
|
||||
return timingSafeEqual(a, b);
|
||||
}
|
||||
|
||||
function authenticate(req: NextRequest): boolean {
|
||||
// If no API key is configured, allow all requests
|
||||
const configuredKey = process.env.OMNIROUTE_API_KEY;
|
||||
if (!configuredKey) return true;
|
||||
async function authenticate(req: NextRequest): Promise<boolean> {
|
||||
// /a2a is outside the authz proxy matcher, so the REQUIRE_API_KEY posture the
|
||||
// pipeline enforces for /v1 never ran here — the route accepted every caller
|
||||
// whenever OMNIROUTE_API_KEY was unset, which is the shipped default
|
||||
// (GHSA-v54m-6rm3-p565). Apply the same posture directly: when a client key is
|
||||
// required, demand a valid OmniRoute key; otherwise honor the legacy explicit
|
||||
// A2A key; otherwise stay keyless (the same local-first default as /v1).
|
||||
const apiKey = extractApiKey(req);
|
||||
if (isRequireApiKeyEnabled()) {
|
||||
return apiKey ? await isValidApiKey(apiKey) : false;
|
||||
}
|
||||
|
||||
const authHeader = req.headers.get("authorization") || "";
|
||||
const token = authHeader.replace(/^Bearer\s+/i, "");
|
||||
return tokensMatch(token, configuredKey);
|
||||
const configuredKey = process.env.OMNIROUTE_API_KEY;
|
||||
if (configuredKey) {
|
||||
return apiKey ? tokensMatch(apiKey, configuredKey) : false;
|
||||
}
|
||||
|
||||
// No API key required and none configured — allow (keyless local-first).
|
||||
return true;
|
||||
}
|
||||
|
||||
// ============ JSON-RPC Helpers ============
|
||||
@@ -179,7 +192,7 @@ async function rejectIfA2ADisabled(id: string | number | null) {
|
||||
|
||||
export async function POST(req: NextRequest) {
|
||||
// Auth check
|
||||
if (!authenticate(req)) {
|
||||
if (!(await authenticate(req))) {
|
||||
return jsonRpcError(null, -32600, "Unauthorized: missing or invalid API key");
|
||||
}
|
||||
|
||||
|
||||
69
tests/unit/a2a-route-require-api-key.test.ts
Normal file
69
tests/unit/a2a-route-require-api-key.test.ts
Normal file
@@ -0,0 +1,69 @@
|
||||
/**
|
||||
* GHSA-v54m-6rm3-p565 — /a2a sits outside the authz proxy matcher, so it never
|
||||
* saw the REQUIRE_API_KEY posture and accepted every caller when OMNIROUTE_API_KEY
|
||||
* was unset (the default). authenticate() now honors REQUIRE_API_KEY directly.
|
||||
*/
|
||||
|
||||
import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omni-a2a-require-key-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "a2a-require-key-secret";
|
||||
process.env.OMNIROUTE_DISABLE_REDIS_AUTH_CACHE = "1";
|
||||
|
||||
const core = await import("../../src/lib/db/core.ts");
|
||||
const apiKeysDb = await import("../../src/lib/db/apiKeys.ts");
|
||||
const route = await import("../../src/app/a2a/route.ts");
|
||||
|
||||
const ORIGINAL_REQUIRE = process.env.REQUIRE_API_KEY;
|
||||
const ORIGINAL_A2A_KEY = process.env.OMNIROUTE_API_KEY;
|
||||
|
||||
test.after(() => {
|
||||
core.resetDbInstance();
|
||||
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
|
||||
if (ORIGINAL_REQUIRE === undefined) delete process.env.REQUIRE_API_KEY;
|
||||
else process.env.REQUIRE_API_KEY = ORIGINAL_REQUIRE;
|
||||
if (ORIGINAL_A2A_KEY === undefined) delete process.env.OMNIROUTE_API_KEY;
|
||||
else process.env.OMNIROUTE_API_KEY = ORIGINAL_A2A_KEY;
|
||||
});
|
||||
|
||||
function post(key?: string) {
|
||||
return route.POST(
|
||||
new Request("http://localhost/a2a", {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"content-type": "application/json",
|
||||
...(key ? { authorization: `Bearer ${key}` } : {}),
|
||||
},
|
||||
body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "message/send", params: {} }),
|
||||
}) as never
|
||||
);
|
||||
}
|
||||
|
||||
async function isUnauthorized(res: Response) {
|
||||
const body = (await res.clone().json()) as { error?: { code?: number } };
|
||||
return body.error?.code === -32600;
|
||||
}
|
||||
|
||||
test("REQUIRE_API_KEY=true rejects an unkeyed /a2a call (GHSA-v54m)", async () => {
|
||||
delete process.env.OMNIROUTE_API_KEY;
|
||||
process.env.REQUIRE_API_KEY = "true";
|
||||
assert.equal(await isUnauthorized(await post()), true, "no key must be rejected");
|
||||
|
||||
const key = await apiKeysDb.createApiKey("a2a-client", "machine-a2a", []);
|
||||
assert.equal(
|
||||
await isUnauthorized(await post(key.key)),
|
||||
false,
|
||||
"a valid key must clear the /a2a auth gate"
|
||||
);
|
||||
});
|
||||
|
||||
test("keyless local-first default still allows /a2a (posture preserved)", async () => {
|
||||
delete process.env.REQUIRE_API_KEY;
|
||||
delete process.env.OMNIROUTE_API_KEY;
|
||||
assert.equal(await isUnauthorized(await post()), false, "keyless default must not 401");
|
||||
});
|
||||
Reference in New Issue
Block a user