From 3e4aab4281bf1cbaad931d4f718556b8562040fe Mon Sep 17 00:00:00 2001 From: iaj6 Date: Sat, 11 Jul 2026 23:24:41 -0400 Subject: [PATCH] fix(auth): gate the four unauthenticated run/policy sub-resource GET routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /api/runs/[id]/agents, /api/runs/[id]/metrics, /api/runs/[id]/policies, and /api/policies/[id]/results shipped with no requireUser and no ownership check while every sibling route enforced both — any request with any (even invalid) credential could read any run's cost metrics, full event timeline, and policy results across all users. - The three run sub-resources now apply the same rules as GET /api/runs/[id]: authenticated, members see only their own runs, pre-auth (userId=null) runs are admin-only, and non-owners get 404 (not 403) so run IDs can't be enumerated. - /api/policies/[id]/results now requires auth and scopes results to the member's own runs via a join against the runs table (getPolicyResultsForPolicy gained an optional ownedByUserId filter); admins keep the cross-user view. - auth-gaps.test.ts extended with 8 cases covering anon/non-owner/owner/ admin and null-owner behavior for all four routes — these were exactly the routes the earlier auth sweep missed because they had no UI callers. Co-Authored-By: Claude Fable 5 --- packages/db/src/policies.ts | 41 +++++- .../src/app/api/__tests__/auth-gaps.test.ts | 133 ++++++++++++++++++ .../app/api/policies/[id]/results/route.ts | 15 +- .../web/src/app/api/runs/[id]/agents/route.ts | 20 ++- .../src/app/api/runs/[id]/metrics/route.ts | 19 ++- .../src/app/api/runs/[id]/policies/route.ts | 19 ++- 6 files changed, 228 insertions(+), 19 deletions(-) diff --git a/packages/db/src/policies.ts b/packages/db/src/policies.ts index f0ecd59..f59195b 100644 --- a/packages/db/src/policies.ts +++ b/packages/db/src/policies.ts @@ -1,9 +1,9 @@ -import { eq, sql } from "drizzle-orm"; +import { and, eq, sql } from "drizzle-orm"; import type { PolicyId, RunId } from "@agentops/core"; import { createPolicyId } from "@agentops/core"; import type { Policy, PolicySeverity } from "@agentops/core"; import type { AgentOpsDb } from "./connection.js"; -import { policies, policyResults } from "./schema.js"; +import { policies, policyResults, runs } from "./schema.js"; interface ListPoliciesFilters { type?: string; @@ -224,6 +224,10 @@ export function deletePolicy( export function getPolicyResultsForPolicy( db: AgentOpsDb, policyId: PolicyId, + // When set, only results whose run belongs to this user are returned. + // Policy results reference runs across every user, so member-facing + // callers must pass their own user id; omitting it is the admin view. + ownedByUserId?: string, ): Array<{ id: string; runId: string; @@ -233,11 +237,34 @@ export function getPolicyResultsForPolicy( details: Record; evaluatedAt: string; }> { - const rows = db - .select() - .from(policyResults) - .where(eq(policyResults.policyId, policyId as string)) - .all() as DbPolicyResult[]; + const resultColumns = { + id: policyResults.id, + runId: policyResults.runId, + policyId: policyResults.policyId, + passed: policyResults.passed, + message: policyResults.message, + details: policyResults.details, + evaluatedAt: policyResults.evaluatedAt, + }; + const rows = ( + ownedByUserId === undefined + ? db + .select(resultColumns) + .from(policyResults) + .where(eq(policyResults.policyId, policyId as string)) + .all() + : db + .select(resultColumns) + .from(policyResults) + .innerJoin(runs, eq(policyResults.runId, runs.id)) + .where( + and( + eq(policyResults.policyId, policyId as string), + eq(runs.userId, ownedByUserId), + ), + ) + .all() + ) as DbPolicyResult[]; return rows.map((row) => ({ id: row.id, diff --git a/packages/web/src/app/api/__tests__/auth-gaps.test.ts b/packages/web/src/app/api/__tests__/auth-gaps.test.ts index c2f4967..ea9ca06 100644 --- a/packages/web/src/app/api/__tests__/auth-gaps.test.ts +++ b/packages/web/src/app/api/__tests__/auth-gaps.test.ts @@ -2,7 +2,9 @@ import { describe, it, expect, beforeEach, vi } from "vitest"; import { insertRun, insertPolicy, + insertPolicyResult, insertSession, + runMetrics, type AgentOpsDb, } from "@agentops/db"; import { @@ -65,6 +67,10 @@ import { GET as activeSessionsRoute } from "@/app/api/sessions/active/route"; import { GET as usageLocalRoute } from "@/app/api/usage/local/route"; import { GET as usageByUserRoute } from "@/app/api/usage/by-user/route"; import { POST as searchMetaRoute } from "@/app/api/runs/search/route"; +import { GET as runAgentsRoute } from "@/app/api/runs/[id]/agents/route"; +import { GET as runMetricsRoute } from "@/app/api/runs/[id]/metrics/route"; +import { GET as runPoliciesRoute } from "@/app/api/runs/[id]/policies/route"; +import { GET as policyResultsRoute } from "@/app/api/policies/[id]/results/route"; let db: AgentOpsDb; let admin: TestUser; @@ -494,3 +500,130 @@ describe("POST /api/runs/search (filter options) scoping", () => { expect(body.repos).toEqual(["owner/repo"]); }); }); + +// ─── Run sub-resource + policy-results routes (July 2026 audit) ──────────────── +// These four GET routes shipped without requireUser or ownership checks and +// leaked cross-tenant metrics, event timelines, and policy results. + +describe("run sub-resource routes ownership", () => { + function seedMetrics(runId: string) { + db.insert(runMetrics) + .values({ + id: `rm_${runId}`, + runId, + tokenUsage: { input: 100, output: 50, total: 150 }, + wallTimeMs: 1000, + costCents: 500, + flakeRate: 0, + recordedAt: "2025-01-01T00:00:00.000Z", + }) + .run(); + } + + const cases = [ + { + name: "GET /api/runs/[id]/metrics", + call: (runId: string, token?: string) => + runMetricsRoute( + token + ? authedRequest(`http://localhost/api/runs/${runId}/metrics`, { method: "GET", token }) + : anonRequest(`http://localhost/api/runs/${runId}/metrics`, { method: "GET" }), + withParams({ id: runId }), + ), + }, + { + name: "GET /api/runs/[id]/agents", + call: (runId: string, token?: string) => + runAgentsRoute( + token + ? authedRequest(`http://localhost/api/runs/${runId}/agents`, { method: "GET", token }) + : anonRequest(`http://localhost/api/runs/${runId}/agents`, { method: "GET" }), + withParams({ id: runId }), + ), + }, + { + name: "GET /api/runs/[id]/policies", + call: (runId: string, token?: string) => + runPoliciesRoute( + token + ? authedRequest(`http://localhost/api/runs/${runId}/policies`, { method: "GET", token }) + : anonRequest(`http://localhost/api/runs/${runId}/policies`, { method: "GET" }), + withParams({ id: runId }), + ), + }, + ]; + + for (const { name, call } of cases) { + it(`${name}: 401 anon, 404 non-owner, 200 owner, 200 admin`, async () => { + const run = makeRun({ userId: owner.user.id }); + seedMetrics(run.id as string); + + expect((await call(run.id as string)).status).toBe(401); + // 404 (not 403) for the non-owner — no existence leak. + expect((await call(run.id as string, other.token)).status).toBe(404); + expect((await call(run.id as string, owner.token)).status).toBe(200); + expect((await call(run.id as string, admin.token)).status).toBe(200); + }); + + it(`${name}: null-owner (pre-auth) runs are admin-only`, async () => { + const run = makeRun({}); + seedMetrics(run.id as string); + expect((await call(run.id as string, owner.token)).status).toBe(404); + expect((await call(run.id as string, admin.token)).status).toBe(200); + }); + } +}); + +describe("GET /api/policies/[id]/results scoping", () => { + function seedResult(id: string, runId: string, policyId = "pol_test") { + insertPolicyResult(db, { + id, + runId, + policyId, + passed: true, + message: "ok", + details: {}, + evaluatedAt: "2025-01-01T00:00:00.000Z", + }); + } + + it("401 without auth", async () => { + seedPolicy(); + const res = await policyResultsRoute( + anonRequest("http://localhost/api/policies/pol_test/results", { method: "GET" }), + withParams({ id: "pol_test" }), + ); + expect(res.status).toBe(401); + }); + + it("members only see results for their own runs; admins see all", async () => { + seedPolicy(); + const ownerRun = makeRun({ userId: owner.user.id }); + const otherRun = makeRun({ userId: other.user.id }); + seedResult("pr_owner", ownerRun.id as string); + seedResult("pr_other", otherRun.id as string); + + const memberRes = await policyResultsRoute( + authedRequest("http://localhost/api/policies/pol_test/results", { + method: "GET", + token: owner.token, + }), + withParams({ id: "pol_test" }), + ); + expect(memberRes.status).toBe(200); + const memberBody = (await jsonOf(memberRes)) as Array<{ runId: string }>; + expect(memberBody.map((r) => r.runId)).toEqual([ownerRun.id as string]); + + const adminRes = await policyResultsRoute( + authedRequest("http://localhost/api/policies/pol_test/results", { + method: "GET", + token: admin.token, + }), + withParams({ id: "pol_test" }), + ); + const adminBody = (await jsonOf(adminRes)) as Array<{ runId: string }>; + expect(adminBody.map((r) => r.runId).sort()).toEqual( + [ownerRun.id as string, otherRun.id as string].sort(), + ); + }); +}); diff --git a/packages/web/src/app/api/policies/[id]/results/route.ts b/packages/web/src/app/api/policies/[id]/results/route.ts index 0e6d6b2..71f6101 100644 --- a/packages/web/src/app/api/policies/[id]/results/route.ts +++ b/packages/web/src/app/api/policies/[id]/results/route.ts @@ -2,16 +2,27 @@ import { NextRequest, NextResponse } from "next/server"; import { getPolicyResultsForPolicy } from "@agentops/db"; import { createPolicyId } from "@agentops/core"; import { db } from "@/lib/db"; +import { requireUser } from "@/lib/auth"; export const dynamic = "force-dynamic"; export async function GET( - _request: NextRequest, + request: NextRequest, { params }: { params: Promise<{ id: string }> }, ) { + const user = await requireUser(request); + if (user instanceof NextResponse) return user; + try { const { id } = await params; - const results = getPolicyResultsForPolicy(db(), createPolicyId(id)); + // Policy results reference runs across every user. Members only see + // results for their own runs; admins see everything (matching the + // view-scoping rules used by the runs/sessions lists). + const results = getPolicyResultsForPolicy( + db(), + createPolicyId(id), + user.role === "admin" ? undefined : user.id, + ); return NextResponse.json(results); } catch (error) { console.error("API error:", error); diff --git a/packages/web/src/app/api/runs/[id]/agents/route.ts b/packages/web/src/app/api/runs/[id]/agents/route.ts index 1e7e473..9a2f5e2 100644 --- a/packages/web/src/app/api/runs/[id]/agents/route.ts +++ b/packages/web/src/app/api/runs/[id]/agents/route.ts @@ -1,17 +1,29 @@ import { NextRequest, NextResponse } from "next/server"; -import { getEventsBySource } from "@agentops/db"; -import { buildAgentTimeline } from "@agentops/core"; +import { getEventsBySource, getRun } from "@agentops/db"; +import { buildAgentTimeline, createRunId } from "@agentops/core"; import { db } from "@/lib/db"; +import { requireUser } from "@/lib/auth"; export const dynamic = "force-dynamic"; export async function GET( - _request: NextRequest, + request: NextRequest, { params }: { params: Promise<{ id: string }> }, ) { + const user = await requireUser(request); + if (user instanceof NextResponse) return user; + try { const { id } = await params; - const events = getEventsBySource(db(), id, 500); + const d = db(); + // Same ownership rules as GET /api/runs/[id]: members only see their + // own runs; pre-auth runs (userId == null) are admin-only. 404 (not + // 403) on non-owner so run IDs can't be enumerated. + const run = getRun(d, createRunId(id)); + if (!run || (user.role !== "admin" && run.userId !== user.id)) { + return NextResponse.json({ error: "Run not found" }, { status: 404 }); + } + const events = getEventsBySource(d, id, 500); const timeline = buildAgentTimeline(events); return NextResponse.json({ timeline }); } catch (error) { diff --git a/packages/web/src/app/api/runs/[id]/metrics/route.ts b/packages/web/src/app/api/runs/[id]/metrics/route.ts index 96dd30a..3938c96 100644 --- a/packages/web/src/app/api/runs/[id]/metrics/route.ts +++ b/packages/web/src/app/api/runs/[id]/metrics/route.ts @@ -1,17 +1,30 @@ import { NextRequest, NextResponse } from "next/server"; -import { getRunMetrics } from "@agentops/db"; +import { getRunMetrics, getRun } from "@agentops/db"; import { createRunId } from "@agentops/core"; import { db } from "@/lib/db"; +import { requireUser } from "@/lib/auth"; export const dynamic = "force-dynamic"; export async function GET( - _request: NextRequest, + request: NextRequest, { params }: { params: Promise<{ id: string }> }, ) { + const user = await requireUser(request); + if (user instanceof NextResponse) return user; + try { const { id } = await params; - const metrics = getRunMetrics(db(), createRunId(id)); + const d = db(); + const runId = createRunId(id); + // Same ownership rules as GET /api/runs/[id]: members only see their + // own runs; pre-auth runs (userId == null) are admin-only. 404 (not + // 403) on non-owner so run IDs can't be enumerated. + const run = getRun(d, runId); + if (!run || (user.role !== "admin" && run.userId !== user.id)) { + return NextResponse.json({ error: "Run not found" }, { status: 404 }); + } + const metrics = getRunMetrics(d, runId); if (!metrics) { return NextResponse.json({ error: "Metrics not found" }, { status: 404 }); } diff --git a/packages/web/src/app/api/runs/[id]/policies/route.ts b/packages/web/src/app/api/runs/[id]/policies/route.ts index 325da6d..ca70380 100644 --- a/packages/web/src/app/api/runs/[id]/policies/route.ts +++ b/packages/web/src/app/api/runs/[id]/policies/route.ts @@ -1,17 +1,30 @@ import { NextRequest, NextResponse } from "next/server"; -import { getPolicyResults } from "@agentops/db"; +import { getPolicyResults, getRun } from "@agentops/db"; import { createRunId } from "@agentops/core"; import { db } from "@/lib/db"; +import { requireUser } from "@/lib/auth"; export const dynamic = "force-dynamic"; export async function GET( - _request: NextRequest, + request: NextRequest, { params }: { params: Promise<{ id: string }> }, ) { + const user = await requireUser(request); + if (user instanceof NextResponse) return user; + try { const { id } = await params; - const results = getPolicyResults(db(), createRunId(id)); + const d = db(); + const runId = createRunId(id); + // Same ownership rules as GET /api/runs/[id]: members only see their + // own runs; pre-auth runs (userId == null) are admin-only. 404 (not + // 403) on non-owner so run IDs can't be enumerated. + const run = getRun(d, runId); + if (!run || (user.role !== "admin" && run.userId !== user.id)) { + return NextResponse.json({ error: "Run not found" }, { status: 404 }); + } + const results = getPolicyResults(d, runId); return NextResponse.json(results); } catch (error) { console.error("API error:", error);