mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-24 08:02:14 +03:00
fix(security): A2A REST auth + task owner scoping (GHSA-jcm5-6wpp-wjj8)
The REST task routes (/api/a2a/tasks, /api/a2a/tasks/[id], /[id]/cancel) had NO authentication call at all — open regardless of configuration — and the task manager stored tasks in an owner-less Map, so any caller could read or cancel any task by id over either the JSON-RPC or the REST surface. - New shared src/lib/a2a/authenticate.ts (the v54m JSON-RPC posture, lifted so both surfaces cannot drift) + src/app/api/a2a/_auth.ts implementing the full posture matrix: REQUIRE_API_KEY=true demands a valid key (management session also passes via alwaysRequireAuth); requireLogin=true accepts management or a valid key; the keyless local-first default stays open by design. - Tasks bind to an owner (hashed API key) at creation; get/cancel/list are owner-scoped. Another principal's task answers with the same not-found a missing one would (no existence oracle). Ownerless tasks (keyless posture) stay visible to everyone; management/operator view sees all tasks. - Callers discriminate the auth failure with instanceof Response, never instanceof NextResponse — createErrorResponse() returns a plain Response, which silently fell through to the handler (caught by the 401-vs-404 test).
This commit is contained in:
@@ -10,15 +10,13 @@
|
||||
* Auth: Bearer token via Authorization header
|
||||
*/
|
||||
|
||||
import { timingSafeEqual } from "node:crypto";
|
||||
import { NextRequest, NextResponse } from "next/server";
|
||||
import { getTaskManager } from "@/lib/a2a/taskManager";
|
||||
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";
|
||||
import { authenticateA2ARequest, resolveA2AOwner } from "@/lib/a2a/authenticate";
|
||||
|
||||
// ============ A2A v1.0 ↔ v0.3 compatibility layer ============
|
||||
// A2A 1.0 renamed the JSON-RPC methods (message/send → SendMessage,
|
||||
@@ -55,7 +53,7 @@ function buildV1Task(
|
||||
? result.artifacts
|
||||
.map((a) =>
|
||||
a && typeof a === "object" && typeof (a as { content?: unknown }).content === "string"
|
||||
? ((a as { content: string }).content)
|
||||
? (a as { content: string }).content
|
||||
: ""
|
||||
)
|
||||
.filter((s) => s.length > 0)
|
||||
@@ -124,39 +122,13 @@ function toMessageArray(raw: unknown): A2AMessage[] | null {
|
||||
|
||||
// ============ Auth ============
|
||||
|
||||
/**
|
||||
* 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).
|
||||
*/
|
||||
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);
|
||||
}
|
||||
|
||||
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 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;
|
||||
// (GHSA-v54m-6rm3-p565). The shared helper applies the same posture on both
|
||||
// the JSON-RPC and the REST task surfaces (GHSA-jcm5-6wpp-wjj8).
|
||||
return authenticateA2ARequest(req);
|
||||
}
|
||||
|
||||
// ============ JSON-RPC Helpers ============
|
||||
@@ -213,6 +185,9 @@ export async function POST(req: NextRequest) {
|
||||
if (disabledResponse) return disabledResponse;
|
||||
|
||||
const tm = getTaskManager();
|
||||
// GHSA-jcm5-6wpp-wjj8: scope every task read/mutation below to the caller's
|
||||
// owner id (hashed API key; undefined under the keyless local-first posture).
|
||||
const callerOwner = resolveA2AOwner(req);
|
||||
|
||||
// A2A 1.0 method-name compatibility (SendMessage → message/send, etc.)
|
||||
const isV1Method = method in V1_METHOD_ALIASES;
|
||||
@@ -236,7 +211,7 @@ export async function POST(req: NextRequest) {
|
||||
return jsonRpcError(id, -32601, `Unknown skill: ${skill}`);
|
||||
}
|
||||
|
||||
const task = tm.createTask({ skill, messages, metadata: params?.metadata });
|
||||
const task = tm.createTask({ skill, messages, metadata: params?.metadata }, callerOwner);
|
||||
try {
|
||||
tm.updateTask(task.id, "working");
|
||||
const result = await handler(task);
|
||||
@@ -302,7 +277,7 @@ export async function POST(req: NextRequest) {
|
||||
return jsonRpcError(id, -32601, `Unknown skill: ${skill}`);
|
||||
}
|
||||
|
||||
const task = tm.createTask({ skill, messages, metadata: params?.metadata });
|
||||
const task = tm.createTask({ skill, messages, metadata: params?.metadata }, callerOwner);
|
||||
tm.updateTask(task.id, "working");
|
||||
|
||||
const stream = createA2AStream(
|
||||
@@ -323,7 +298,7 @@ export async function POST(req: NextRequest) {
|
||||
const taskId = params?.taskId || params?.id;
|
||||
if (!taskId) return jsonRpcError(id, -32602, "Invalid params: taskId required");
|
||||
|
||||
const task = tm.getTask(taskId);
|
||||
const task = tm.getTask(taskId, callerOwner);
|
||||
if (!task) return jsonRpcError(id, -32601, `Task not found: ${taskId}`);
|
||||
|
||||
return jsonRpcResult(id, { task });
|
||||
@@ -335,7 +310,7 @@ export async function POST(req: NextRequest) {
|
||||
if (!taskId) return jsonRpcError(id, -32602, "Invalid params: taskId required");
|
||||
|
||||
try {
|
||||
const task = tm.cancelTask(taskId);
|
||||
const task = tm.cancelTask(taskId, callerOwner);
|
||||
return jsonRpcResult(id, { task: { id: task.id, state: task.state } });
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
|
||||
51
src/app/api/a2a/_auth.ts
Normal file
51
src/app/api/a2a/_auth.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
/**
|
||||
* Shared authorization for the REST A2A task routes (GHSA-jcm5-6wpp-wjj8).
|
||||
*
|
||||
* Dual audience: the dashboard calls these routes with a management session,
|
||||
* A2A clients with an inference API key. Posture matrix:
|
||||
*
|
||||
* - REQUIRE_API_KEY=true: a valid OmniRoute key is mandatory (the same
|
||||
* posture the /v1 inference plane enforces); a management session also
|
||||
* passes (dashboard), via alwaysRequireAuth so requireLogin=false cannot
|
||||
* bypass it.
|
||||
* - otherwise + requireLogin=true: management session, or a valid key.
|
||||
* - otherwise + requireLogin=false (local-first default): open, by design.
|
||||
*
|
||||
* Callers authenticated by key are owner-scoped — another principal's tasks
|
||||
* answer as if they did not exist. Management/operator view sees all tasks.
|
||||
*/
|
||||
|
||||
import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
|
||||
import { extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import { isRequireApiKeyEnabled } from "@/shared/utils/featureFlags";
|
||||
import { resolveA2AOwner } from "@/lib/a2a/authenticate";
|
||||
|
||||
export interface A2ARestAuth {
|
||||
/** Owner scope for task reads/mutations; undefined = operator view (all tasks). */
|
||||
owner: string | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* NOTE: the failure branch is whatever requireManagementAuth returns — today a
|
||||
* plain `Response` from createErrorResponse(), NOT a NextResponse. Callers must
|
||||
* test with `instanceof Response` (NextResponse extends Response), never
|
||||
* `instanceof NextResponse`, or the 401 silently falls through to the handler.
|
||||
*/
|
||||
export async function authorizeA2ATaskRoute(request: Request): Promise<A2ARestAuth | Response> {
|
||||
const apiKey = extractApiKey(request);
|
||||
|
||||
if (isRequireApiKeyEnabled()) {
|
||||
if (apiKey && (await isValidApiKey(apiKey))) return { owner: resolveA2AOwner(request) };
|
||||
const managementError = await requireManagementAuth(request, {
|
||||
invalidApiKeyStatus: 401,
|
||||
alwaysRequireAuth: true,
|
||||
});
|
||||
if (managementError === null) return { owner: undefined };
|
||||
return managementError;
|
||||
}
|
||||
|
||||
const managementError = await requireManagementAuth(request, { invalidApiKeyStatus: 401 });
|
||||
if (managementError === null) return { owner: undefined };
|
||||
if (apiKey && (await isValidApiKey(apiKey))) return { owner: resolveA2AOwner(request) };
|
||||
return managementError;
|
||||
}
|
||||
@@ -1,14 +1,23 @@
|
||||
import { NextResponse } from "next/server";
|
||||
import { getTaskManager } from "@/lib/a2a/taskManager";
|
||||
import { authorizeA2ATaskRoute } from "@/app/api/a2a/_auth";
|
||||
import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
|
||||
|
||||
export async function POST(_request: Request, { params }: { params: Promise<{ id: string }> }) {
|
||||
export async function POST(request: Request, { params }: { params: Promise<{ id: string }> }) {
|
||||
// GHSA-jcm5-6wpp-wjj8: this route had no auth call at all. The owner check
|
||||
// happens inside cancelTask: another principal's task throws the same
|
||||
// "not found" a missing one would (no existence oracle).
|
||||
const auth = await authorizeA2ATaskRoute(request);
|
||||
if (auth instanceof Response) return auth;
|
||||
try {
|
||||
const { id } = await params;
|
||||
const tm = getTaskManager();
|
||||
const task = tm.cancelTask(id);
|
||||
const task = tm.cancelTask(id, auth.owner);
|
||||
return NextResponse.json({ task: { id: task.id, state: task.state } });
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : "Failed to cancel A2A task";
|
||||
const message = sanitizeErrorMessage(
|
||||
error instanceof Error ? error.message : "Failed to cancel A2A task"
|
||||
);
|
||||
const status = message.includes("not found") ? 404 : 400;
|
||||
return NextResponse.json({ error: message }, { status });
|
||||
}
|
||||
|
||||
@@ -1,17 +1,30 @@
|
||||
import { NextResponse } from "next/server";
|
||||
import { getTaskManager } from "@/lib/a2a/taskManager";
|
||||
import { authorizeA2ATaskRoute } from "@/app/api/a2a/_auth";
|
||||
import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
|
||||
|
||||
export async function GET(_request: Request, { params }: { params: Promise<{ id: string }> }) {
|
||||
export async function GET(request: Request, { params }: { params: Promise<{ id: string }> }) {
|
||||
// GHSA-jcm5-6wpp-wjj8: this route had no auth call at all — open regardless
|
||||
// of configuration. Another principal's task answers 404, same as a missing
|
||||
// one, so an IDOR probe cannot tell the two apart.
|
||||
const auth = await authorizeA2ATaskRoute(request);
|
||||
if (auth instanceof Response) return auth;
|
||||
try {
|
||||
const { id } = await params;
|
||||
const tm = getTaskManager();
|
||||
const task = tm.getTask(id);
|
||||
const task = tm.getTask(id, auth.owner);
|
||||
if (!task) {
|
||||
return NextResponse.json({ error: `Task not found: ${id}` }, { status: 404 });
|
||||
}
|
||||
return NextResponse.json({ task });
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : "Failed to load A2A task";
|
||||
return NextResponse.json({ error: message }, { status: 500 });
|
||||
return NextResponse.json(
|
||||
{
|
||||
error: sanitizeErrorMessage(
|
||||
error instanceof Error ? error.message : "Failed to load A2A task"
|
||||
),
|
||||
},
|
||||
{ status: 500 }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3,6 +3,7 @@ import { NextResponse } from "next/server";
|
||||
import { z } from "zod";
|
||||
|
||||
import { getTaskManager, type TaskState } from "@/lib/a2a/taskManager";
|
||||
import { authorizeA2ATaskRoute } from "@/app/api/a2a/_auth";
|
||||
import { createConductorTask } from "@/lib/conductor/hubProxy";
|
||||
import { getSettings } from "@/lib/db/settings";
|
||||
|
||||
@@ -22,6 +23,11 @@ function parseIntParam(value: string | null, fallback: number): number {
|
||||
}
|
||||
|
||||
export async function GET(request: Request) {
|
||||
// GHSA-jcm5-6wpp-wjj8: the list route had no auth call at all. Management
|
||||
// (or the keyless posture) sees every task; a bare API key must be valid
|
||||
// and is owner-scoped.
|
||||
const auth = await authorizeA2ATaskRoute(request);
|
||||
if (auth instanceof Response) return auth;
|
||||
try {
|
||||
const { searchParams } = new URL(request.url);
|
||||
const stateParam = searchParams.get("state");
|
||||
@@ -36,7 +42,7 @@ export async function GET(request: Request) {
|
||||
|
||||
const tm = getTaskManager();
|
||||
const total = tm.countTasks({ state, skill });
|
||||
const tasks = tm.listTasks({ state, skill, limit, offset });
|
||||
const tasks = tm.listTasks({ state, skill, limit, offset }, auth.owner);
|
||||
|
||||
return NextResponse.json({
|
||||
tasks,
|
||||
@@ -104,7 +110,10 @@ export function authenticateA2A(request: Request): boolean {
|
||||
*/
|
||||
export async function POST(request: Request) {
|
||||
if (!authenticateA2A(request)) {
|
||||
return NextResponse.json({ error: "Unauthorized: missing or invalid API key" }, { status: 401 });
|
||||
return NextResponse.json(
|
||||
{ error: "Unauthorized: missing or invalid API key" },
|
||||
{ status: 401 }
|
||||
);
|
||||
}
|
||||
const settings = await getSettings();
|
||||
if (settings.a2aEnabled !== true) {
|
||||
@@ -122,12 +131,18 @@ export async function POST(request: Request) {
|
||||
}
|
||||
const parsed = delegationSchema.safeParse(raw);
|
||||
if (!parsed.success) {
|
||||
return NextResponse.json({ error: "Invalid A2A task: provide messages[] (and metadata.conductor)" }, { status: 400 });
|
||||
return NextResponse.json(
|
||||
{ error: "Invalid A2A task: provide messages[] (and metadata.conductor)" },
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
const { skill, messages, metadata } = parsed.data;
|
||||
if (skill !== "conductor" && !skill.startsWith("conductor-cli-")) {
|
||||
return NextResponse.json(
|
||||
{ error: "Only Conductor fleet skills are delegable here (conductor / conductor-cli-<profile>)" },
|
||||
{
|
||||
error:
|
||||
"Only Conductor fleet skills are delegable here (conductor / conductor-cli-<profile>)",
|
||||
},
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
@@ -138,7 +153,9 @@ export async function POST(request: Request) {
|
||||
{ status: 400 }
|
||||
);
|
||||
}
|
||||
const prompt = [...messages].reverse().find((m) => m.role === "user")?.content ?? messages[messages.length - 1].content;
|
||||
const prompt =
|
||||
[...messages].reverse().find((m) => m.role === "user")?.content ??
|
||||
messages[messages.length - 1].content;
|
||||
|
||||
const created = await createConductorTask({
|
||||
repoUrl: conductor.repo.url,
|
||||
|
||||
53
src/lib/a2a/authenticate.ts
Normal file
53
src/lib/a2a/authenticate.ts
Normal file
@@ -0,0 +1,53 @@
|
||||
/**
|
||||
* Shared A2A authentication + caller-owner resolution (GHSA-jcm5-6wpp-wjj8).
|
||||
*
|
||||
* The JSON-RPC router (/a2a) grew its own authenticate() for GHSA-v54m, but
|
||||
* the REST task routes under /api/a2a/tasks/ had no auth call at all. Both
|
||||
* surfaces now share this single implementation so they cannot drift again:
|
||||
* same REQUIRE_API_KEY posture as /v1, same keyless local-first default, and
|
||||
* a stable owner id (hashed API key) used to scope task visibility.
|
||||
*/
|
||||
|
||||
import { createHash, timingSafeEqual } from "crypto";
|
||||
import type { NextRequest } from "next/server";
|
||||
import { extractApiKey, isValidApiKey } from "@/sse/services/auth";
|
||||
import { isRequireApiKeyEnabled } from "@/shared/utils/featureFlags";
|
||||
|
||||
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);
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the request may use the A2A surface at all. Mirrors the JSON-RPC
|
||||
* posture: 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).
|
||||
*/
|
||||
export async function authenticateA2ARequest(req: NextRequest | Request): Promise<boolean> {
|
||||
const apiKey = extractApiKey(req as NextRequest);
|
||||
if (isRequireApiKeyEnabled()) {
|
||||
return apiKey ? await isValidApiKey(apiKey) : false;
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
|
||||
/**
|
||||
* Owner id for task scoping (GHSA-jcm5-6wpp-wjj8): a stable hash of the
|
||||
* caller's API key, or `undefined` when the call carries no key (keyless
|
||||
* posture — ownerless tasks stay visible to everyone, by design).
|
||||
*/
|
||||
export function resolveA2AOwner(req: NextRequest | Request): string | undefined {
|
||||
const apiKey = extractApiKey(req as NextRequest);
|
||||
if (!apiKey) return undefined;
|
||||
return createHash("sha256").update(apiKey).digest("hex").slice(0, 32);
|
||||
}
|
||||
@@ -45,6 +45,13 @@ export interface A2ATask {
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
expiresAt: string;
|
||||
/**
|
||||
* GHSA-jcm5-6wpp-wjj8: principal that created the task (hashed API key).
|
||||
* `undefined` = created under the keyless local-first posture — such tasks
|
||||
* stay visible to every caller, matching the pre-owner behavior. Tasks WITH
|
||||
* an owner are only returned/cancelled/listed for the same owner.
|
||||
*/
|
||||
owner?: string;
|
||||
}
|
||||
|
||||
export interface TaskListFilter {
|
||||
@@ -91,7 +98,7 @@ export class A2ATaskManager {
|
||||
}
|
||||
}
|
||||
|
||||
createTask(input: TaskInput): A2ATask {
|
||||
createTask(input: TaskInput, owner?: string): A2ATask {
|
||||
const now = new Date();
|
||||
const task: A2ATask = {
|
||||
id: randomUUID(),
|
||||
@@ -104,19 +111,31 @@ export class A2ATaskManager {
|
||||
createdAt: now.toISOString(),
|
||||
updatedAt: now.toISOString(),
|
||||
expiresAt: new Date(now.getTime() + this.ttlMs).toISOString(),
|
||||
...(owner !== undefined ? { owner } : {}),
|
||||
};
|
||||
this.tasks.set(task.id, task);
|
||||
return task;
|
||||
}
|
||||
|
||||
getTask(taskId: string): A2ATask | undefined {
|
||||
/**
|
||||
* Owner scoping (GHSA-jcm5-6wpp-wjj8): a task carrying an owner is visible
|
||||
* only to that owner. Ownerless tasks (keyless posture, or created before
|
||||
* this field existed) stay visible to everyone — no behavior change there.
|
||||
*/
|
||||
private isVisibleTo(task: A2ATask, owner?: string): boolean {
|
||||
return task.owner === undefined || task.owner === owner;
|
||||
}
|
||||
|
||||
getTask(taskId: string, owner?: string): A2ATask | undefined {
|
||||
const task = this.tasks.get(taskId);
|
||||
if (task && new Date(task.expiresAt) < new Date()) {
|
||||
if (task.state === "submitted" || task.state === "working") {
|
||||
this.updateTask(taskId, "failed", undefined, "Task expired");
|
||||
}
|
||||
}
|
||||
return this.tasks.get(taskId);
|
||||
const current = this.tasks.get(taskId);
|
||||
if (!current || !this.isVisibleTo(current, owner)) return undefined;
|
||||
return current;
|
||||
}
|
||||
|
||||
updateTask(
|
||||
@@ -142,7 +161,15 @@ export class A2ATaskManager {
|
||||
return task;
|
||||
}
|
||||
|
||||
cancelTask(taskId: string): A2ATask {
|
||||
cancelTask(taskId: string, owner?: string): A2ATask {
|
||||
// Owner check BEFORE the mutation (GHSA-jcm5-6wpp-wjj8): a caller must not
|
||||
// cancel another principal's task by id. Uses the same not-found error as
|
||||
// a missing task so an IDOR probe cannot distinguish "exists but not
|
||||
// yours" from "does not exist".
|
||||
const task = this.tasks.get(taskId);
|
||||
if (!task || !this.isVisibleTo(task, owner)) {
|
||||
throw new Error(`Task ${taskId} not found`);
|
||||
}
|
||||
return this.updateTask(taskId, "cancelled", undefined, "Cancelled by client");
|
||||
}
|
||||
|
||||
@@ -153,8 +180,11 @@ export class A2ATaskManager {
|
||||
return tasks.length;
|
||||
}
|
||||
|
||||
listTasks(filter?: TaskListFilter): A2ATask[] {
|
||||
listTasks(filter?: TaskListFilter, owner?: string): A2ATask[] {
|
||||
let tasks = [...this.tasks.values()];
|
||||
// GHSA-jcm5-6wpp-wjj8: when an owner scope is supplied, owned tasks of
|
||||
// other principals are hidden; ownerless tasks remain visible (posture).
|
||||
if (owner !== undefined) tasks = tasks.filter((t) => this.isVisibleTo(t, owner));
|
||||
if (filter?.state) tasks = tasks.filter((t) => t.state === filter.state);
|
||||
if (filter?.skill) tasks = tasks.filter((t) => t.skill === filter.skill);
|
||||
tasks.sort((a, b) => new Date(b.createdAt).getTime() - new Date(a.createdAt).getTime());
|
||||
|
||||
136
tests/unit/a2a-task-owner-idor.test.ts
Normal file
136
tests/unit/a2a-task-owner-idor.test.ts
Normal file
@@ -0,0 +1,136 @@
|
||||
/**
|
||||
* GHSA-jcm5-6wpp-wjj8 — A2A task IDOR + unauthenticated REST task routes.
|
||||
*
|
||||
* Two gaps closed here:
|
||||
* 1. The REST routes /api/a2a/tasks/[id] and /api/a2a/tasks/[id]/cancel had
|
||||
* NO auth call at all — open regardless of configuration. They now share
|
||||
* the JSON-RPC surface's authentication (REQUIRE_API_KEY posture).
|
||||
* 2. Tasks lived in an owner-less Map: any caller could read/cancel any
|
||||
* task by id. Tasks now bind to an owner (hashed API key) at creation and
|
||||
* reads/cancels/lists are owner-scoped. Ownerless tasks (keyless
|
||||
* local-first posture) stay visible to everyone — by design.
|
||||
*
|
||||
* Run with:
|
||||
* node --import tsx/esm --test tests/unit/a2a-task-owner-idor.test.ts
|
||||
*/
|
||||
|
||||
import { describe, it, after } 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-idor-"));
|
||||
process.env.DATA_DIR = TEST_DATA_DIR;
|
||||
process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "a2a-idor-test-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 { A2ATaskManager, getTaskManager } = await import("../../src/lib/a2a/taskManager.ts");
|
||||
const { resolveA2AOwner } = await import("../../src/lib/a2a/authenticate.ts");
|
||||
const restGet = await import("../../src/app/api/a2a/tasks/[id]/route.ts");
|
||||
|
||||
const ORIGINAL_REQUIRE = process.env.REQUIRE_API_KEY;
|
||||
|
||||
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;
|
||||
});
|
||||
|
||||
function makeManager() {
|
||||
const tm = new A2ATaskManager(5);
|
||||
// Prevent the per-instance cleanup interval from keeping the process alive.
|
||||
clearInterval((tm as unknown as { cleanupInterval: NodeJS.Timeout }).cleanupInterval);
|
||||
return tm;
|
||||
}
|
||||
|
||||
describe("A2ATaskManager — owner scoping (GHSA-jcm5)", () => {
|
||||
it("another principal cannot READ an owned task (same undefined as missing)", () => {
|
||||
const tm = makeManager();
|
||||
const task = tm.createTask({ skill: "smart-routing", messages: [] }, "owner-a");
|
||||
assert.equal(tm.getTask(task.id, "owner-a")?.id, task.id, "the owner still reads it");
|
||||
assert.equal(tm.getTask(task.id, "owner-b"), undefined, "another owner gets undefined");
|
||||
});
|
||||
|
||||
it("another principal cannot CANCEL an owned task (not-found error, no existence oracle)", () => {
|
||||
const tm = makeManager();
|
||||
const task = tm.createTask({ skill: "smart-routing", messages: [] }, "owner-a");
|
||||
assert.throws(() => tm.cancelTask(task.id, "owner-b"), /not found/);
|
||||
assert.equal(tm.getTask(task.id, "owner-a")?.state, "submitted", "task untouched");
|
||||
assert.equal(tm.cancelTask(task.id, "owner-a").state, "cancelled", "the owner can cancel");
|
||||
});
|
||||
|
||||
it("owner-scoped listTasks hides other principals' owned tasks", () => {
|
||||
const tm = makeManager();
|
||||
tm.createTask({ skill: "s1", messages: [] }, "owner-a");
|
||||
const mine = tm.createTask({ skill: "s1", messages: [] }, "owner-b");
|
||||
const listed = tm.listTasks(undefined, "owner-b");
|
||||
assert.deepEqual(
|
||||
listed.map((t) => t.id),
|
||||
[mine.id]
|
||||
);
|
||||
// No owner scope (management/dashboard path) still sees everything.
|
||||
assert.equal(tm.listTasks(undefined).length, 2);
|
||||
});
|
||||
|
||||
it("ownerless tasks stay visible to everyone (keyless local-first posture)", () => {
|
||||
const tm = makeManager();
|
||||
const task = tm.createTask({ skill: "smart-routing", messages: [] });
|
||||
assert.equal(tm.getTask(task.id, "anyone")?.id, task.id);
|
||||
assert.equal(tm.getTask(task.id)?.id, task.id);
|
||||
assert.equal(tm.cancelTask(task.id, "anyone").state, "cancelled");
|
||||
});
|
||||
});
|
||||
|
||||
describe("REST /api/a2a/tasks/[id] — authentication (GHSA-jcm5)", () => {
|
||||
it("rejects an unkeyed call when REQUIRE_API_KEY=true (was: no auth at all)", async () => {
|
||||
process.env.REQUIRE_API_KEY = "true";
|
||||
delete process.env.OMNIROUTE_API_KEY;
|
||||
const res = await restGet.GET(new Request("http://localhost/api/a2a/tasks/abc") as never, {
|
||||
params: Promise.resolve({ id: "abc" }),
|
||||
});
|
||||
assert.equal(res.status, 401);
|
||||
});
|
||||
|
||||
it("serves a keyed call under REQUIRE_API_KEY=true", async () => {
|
||||
process.env.REQUIRE_API_KEY = "true";
|
||||
const key = await apiKeysDb.createApiKey("a2a-rest-client", "machine-rest", []);
|
||||
const res = await restGet.GET(
|
||||
new Request("http://localhost/api/a2a/tasks/definitely-missing", {
|
||||
headers: { authorization: `Bearer ${key.key}` },
|
||||
}) as never,
|
||||
{ params: Promise.resolve({ id: "definitely-missing" }) }
|
||||
);
|
||||
// Authenticated — the 404 now comes from the task lookup, not the auth gate.
|
||||
assert.equal(res.status, 404);
|
||||
});
|
||||
|
||||
it("keyed caller gets 404 for another principal's task (route-level IDOR, GHSA-jcm5)", async () => {
|
||||
process.env.REQUIRE_API_KEY = "true";
|
||||
const tm = getTaskManager();
|
||||
// A task owned by a DIFFERENT principal than the caller's key hash.
|
||||
const foreign = tm.createTask({ skill: "smart-routing", messages: [] }, "some-other-owner");
|
||||
const key = await apiKeysDb.createApiKey("a2a-rest-idor", "machine-idor", []);
|
||||
const req = new Request(`http://localhost/api/a2a/tasks/${foreign.id}`, {
|
||||
headers: { authorization: `Bearer ${key.key}` },
|
||||
});
|
||||
const res = await restGet.GET(req as never, { params: Promise.resolve({ id: foreign.id }) });
|
||||
assert.equal(res.status, 404, "another principal's task is invisible");
|
||||
|
||||
// And the same task IS visible to its owner (owner hash derived from the key).
|
||||
const owned = tm.createTask(
|
||||
{ skill: "smart-routing", messages: [] },
|
||||
resolveA2AOwner(req as never)
|
||||
);
|
||||
const res2 = await restGet.GET(
|
||||
new Request(`http://localhost/api/a2a/tasks/${owned.id}`, {
|
||||
headers: { authorization: `Bearer ${key.key}` },
|
||||
}) as never,
|
||||
{ params: Promise.resolve({ id: owned.id }) }
|
||||
);
|
||||
assert.equal(res2.status, 200, "the owner reads its own task");
|
||||
});
|
||||
});
|
||||
@@ -8,7 +8,9 @@ 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");
|
||||
// GHSA-jcm5-6wpp-wjj8: the constant-time token comparison moved out of
|
||||
// src/app/a2a/route.ts into the shared helper both surfaces now use.
|
||||
const A2A_AUTH_HELPER = path.resolve(__dirname, "../../src/lib/a2a/authenticate.ts");
|
||||
|
||||
const source = fs.readFileSync(TASKS_ROUTE, "utf-8");
|
||||
|
||||
@@ -21,11 +23,11 @@ function hasImport(src: string, name: string, from: string): boolean {
|
||||
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");
|
||||
test("tasks route uses the same constant-time contract as the shared A2A auth helper", () => {
|
||||
const a2aSource = fs.readFileSync(A2A_AUTH_HELPER, "utf-8");
|
||||
assert.ok(
|
||||
hasImport(a2aSource, "timingSafeEqual", "node:crypto"),
|
||||
"reference route imports timingSafeEqual"
|
||||
hasImport(a2aSource, "timingSafeEqual", "crypto"),
|
||||
"shared auth helper imports timingSafeEqual"
|
||||
);
|
||||
|
||||
assert.ok(
|
||||
|
||||
Reference in New Issue
Block a user