From 1ba63f74651ae0cb99dc8cdd52f5a1e0ab34035f Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 03:08:51 +0000 Subject: [PATCH 01/10] feat(auth): split bearer auth into worker and maintainer token tiers (#1111) DISPATCH_AGENT_TOKEN keeps maintainer rights (plus an optional DISPATCH_MAINTAINER_TOKEN alias); the new DISPATCH_WORKER_TOKEN is accepted with an explicit, fail-closed allowlist enforced centrally from a single route->tier table. Worker tokens get a 403 naming the required tier (with an auth_tier_denied audit row) on maintainer routes; issues/claim, issues/unclaim, and pr-fix-queue/mark are split by request body inside the route. OIDC/basic/disabled resolve to maintainer, so deploying changes nothing for existing callers. --- .env.example | 18 ++ AGENTS.md | 4 +- docs/generic-harness-loop.md | 2 + docs/opencode-mcp.md | 2 +- docs/worker-execution-contract.md | 17 ++ src/app/api/agent-runs/route.ts | 9 +- src/app/api/agent-work/checkpoint/route.ts | 7 +- src/app/api/agent-work/finish/route.ts | 7 +- src/app/api/agent-work/route.test.ts | 10 +- src/app/api/agent-work/route.ts | 6 +- src/app/api/agent-work/start/route.ts | 4 +- src/app/api/agent-work/sweep/route.test.ts | 10 +- src/app/api/agent-work/sweep/route.ts | 4 +- .../[agentName]/active-work/route.test.ts | 8 +- .../agents/[agentName]/active-work/route.ts | 4 +- .../api/agents/[agentName]/heartbeat/route.ts | 7 +- .../api/agents/[agentName]/next-task/route.ts | 7 +- src/app/api/agents/[agentName]/queue/route.ts | 7 +- .../agents/[agentName]/tasks/report/route.ts | 7 +- .../agents/[agentName]/work-summary/route.ts | 7 +- src/app/api/audit/route.ts | 7 +- src/app/api/automation/events/route.test.ts | 8 +- src/app/api/automation/events/route.ts | 4 +- .../api/automation/repos/[...repo]/route.ts | 4 +- src/app/api/automation/repos/route.ts | 12 +- src/app/api/automation/repos/tracked/route.ts | 4 +- src/app/api/automation/runs/[runId]/route.ts | 4 +- src/app/api/automation/sync/route.ts | 12 +- .../automation/workflows/[id]/route.test.ts | 8 +- .../api/automation/workflows/[id]/route.ts | 4 +- .../api/automation/workflows/route.test.ts | 8 +- src/app/api/automation/workflows/route.ts | 4 +- src/app/api/ci-failures/sync/route.ts | 4 +- src/app/api/groomer/run/route.ts | 4 +- src/app/api/groomer/runs/[id]/route.test.ts | 6 + src/app/api/groomer/runs/[id]/route.ts | 7 +- src/app/api/groomer/runs/route.test.ts | 6 + src/app/api/groomer/runs/route.ts | 7 +- .../[issueId]/admission-override/route.ts | 6 +- src/app/api/issues/[issueId]/lane/route.ts | 9 +- .../[issueId]/pr-health/refresh/route.ts | 4 +- .../api/issues/actions/agents/route.test.ts | 8 +- src/app/api/issues/actions/agents/route.ts | 4 +- src/app/api/issues/actions/decompose/route.ts | 4 +- src/app/api/issues/actions/route.ts | 4 +- src/app/api/issues/claim/route.test.ts | 47 +++++ src/app/api/issues/claim/route.ts | 27 ++- src/app/api/issues/claimed/route.ts | 7 +- src/app/api/issues/comment/route.ts | 4 +- src/app/api/issues/groom/route.ts | 4 +- src/app/api/issues/label/route.test.ts | 6 + src/app/api/issues/label/route.ts | 4 +- src/app/api/issues/move/route.ts | 4 +- src/app/api/issues/prune-closed/route.ts | 4 +- src/app/api/issues/reconcile/route.ts | 9 +- src/app/api/issues/refresh/route.ts | 4 +- src/app/api/issues/route.ts | 7 +- src/app/api/issues/state/route.ts | 7 +- src/app/api/issues/status/route.ts | 4 +- src/app/api/issues/unassign/route.ts | 4 +- src/app/api/issues/unclaim/route.test.ts | 53 +++++ src/app/api/issues/unclaim/route.ts | 28 ++- src/app/api/issues/unlabel/route.ts | 4 +- src/app/api/issues/untriaged/route.ts | 7 +- src/app/api/issues/webhook/route.ts | 4 +- src/app/api/lanes/route.ts | 7 +- src/app/api/pr-fix-queue/enqueue/route.ts | 4 +- src/app/api/pr-fix-queue/history/route.ts | 7 +- src/app/api/pr-fix-queue/mark/route.test.ts | 78 ++++++- src/app/api/pr-fix-queue/mark/route.ts | 27 ++- src/app/api/pr-fix-queue/queued/route.ts | 7 +- src/app/api/pr-fix-queue/requeue/route.ts | 4 +- src/app/api/pr-followup/sync/route.ts | 4 +- src/app/api/pr-followup/webhook/route.ts | 4 +- src/app/api/repos/route.ts | 12 +- src/app/api/sync/route.ts | 4 +- src/app/api/sync/scheduled/route.ts | 7 +- src/lib/auth.test.ts | 170 +++++++++++++++- src/lib/auth.ts | 192 ++++++++++++++++-- src/lib/dispatch-env.test.ts | 135 ++++++++++++ src/lib/dispatch-env.ts | 95 +++++++-- src/test/route-helpers.ts | 27 ++- 82 files changed, 1106 insertions(+), 215 deletions(-) diff --git a/.env.example b/.env.example index 9f1ebffc..49c1af7b 100644 --- a/.env.example +++ b/.env.example @@ -22,7 +22,25 @@ GITHUB_REPOSITORIES="myorg/myrepo1,myorg/myrepo2" # GITHUB_APP_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY-----\n...\n-----END RSA PRIVATE KEY-----" # Agent API authentication +# +# Bearer tokens come in two tiers. The maintainer tier keeps full rights +# (force claims, releasing other agents' claims, PR-fix queue requeue and +# QUEUED/IGNORED marks, groomer, lanes, admission overrides, automation, +# syncs); the worker tier is an explicit allowlist for autonomous executors +# (next-task, tasks/report, heartbeat, active-work, queue, work-summary, +# agent-work start/checkpoint/finish, non-force claim and own-claim unclaim, +# issue state/status, PR-fix queue reads and FIXED/BLOCKED/STALE marks). +# A worker token calling a maintainer route gets a 403 naming the required +# tier. See "Token Tiers" in docs/worker-execution-contract.md. +# +# DISPATCH_AGENT_TOKEN is the maintainer-tier token. DISPATCH_AGENT_TOKEN="your_agent_token_here" +# Optional alias for the maintainer-tier token (same rights as +# DISPATCH_AGENT_TOKEN). +# DISPATCH_MAINTAINER_TOKEN="your_maintainer_token_here" +# Worker-tier bearer token for autonomous executors (e.g. worker harnesses). +# Unset by default; use a different value from DISPATCH_AGENT_TOKEN. +# DISPATCH_WORKER_TOKEN="your_worker_token_here" # Operator / UI authentication (optional) # diff --git a/AGENTS.md b/AGENTS.md index 2f6d85cc..0129f324 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,9 @@ npm run db:deploy # Deploy migrations (prod) |----------|----------|-------------| | `DATABASE_URL` | Yes | PostgreSQL connection string (canonical) | | `GITHUB_TOKEN` | Yes | GitHub Personal Access Token | -| `DISPATCH_AGENT_TOKEN` | Yes | Bearer token for agent API | +| `DISPATCH_AGENT_TOKEN` | Yes | Bearer token for agent API (maintainer tier — full rights) | +| `DISPATCH_MAINTAINER_TOKEN` | No | Optional alias for the maintainer-tier bearer token (same rights as `DISPATCH_AGENT_TOKEN`) | +| `DISPATCH_WORKER_TOKEN` | No | Worker-tier bearer token for autonomous executors (explicit allowlist: next-task, tasks/report, heartbeat, active-work, queue, work-summary, agent-work start/checkpoint/finish + GET listing, non-force claim / own-claim unclaim, issue state/status, PR-fix queue reads + FIXED/BLOCKED/STALE marks); agent-work operator release/reassign and sweep stay maintainer-only; other routes return 403 | | `GITHUB_REPOSITORIES` | Yes | **One-time** bootstrap seed for tracked repos (comma or newline separated). Read only when `AutomationRepo` is empty. After first seed, manage via `/automation` UI or `POST /api/repos` / `POST /api/automation/repos`. Seeded repos carry `source: "env"`; UI-added repos carry `source: "user"`. | | `DISPATCH_URL` | No | Base URL of your Dispatch instance (used by outbound clients and MCP bridge) | | `DISPATCH_DATABASE_URL` | No | Alternative database URL alias — used if `DATABASE_URL` is not set | diff --git a/docs/generic-harness-loop.md b/docs/generic-harness-loop.md index bc45400a..24e2b979 100644 --- a/docs/generic-harness-loop.md +++ b/docs/generic-harness-loop.md @@ -74,6 +74,8 @@ def worker_heartbeat(agent_name, dispatch_url): # Stop ``` +Autonomous workers may authenticate the loop above with `DISPATCH_WORKER_TOKEN` (worker tier); operator and MCP-bridge agents keep the maintainer token (`DISPATCH_AGENT_TOKEN`). + **Optional preflight sync:** Agents may call `POST /api/sync` before fetching their next task to refresh Dispatch's issue cache. This is a best-effort, out-of-band operation — not required for the worker loop and not something agents depend on before every task. Sync failures should be logged as freshness warnings and must not block task execution. ## Generic Groomer Loop diff --git a/docs/opencode-mcp.md b/docs/opencode-mcp.md index b507c93b..cb2ebed1 100644 --- a/docs/opencode-mcp.md +++ b/docs/opencode-mcp.md @@ -25,7 +25,7 @@ npx prisma generate | `DISPATCH_AGENT_TOKEN` | Yes | Bearer token for agent API authentication | | `DISPATCH_AGENT_NAME` | No | Default agent identity used when MCP tools omit `agentName`. Set this to a stable operator identity such as `jory-opencode` for manual OpenCode usage. **Do not use generic identities like `Dispatch MCP`.** | -The token is **never** printed or logged. Missing variables produce a clear error on startup. +The token is **never** printed or logged. Missing variables produce a clear error on startup. The MCP bridge is an operator/bridge client, so it keeps the maintainer token; autonomous workers can use `DISPATCH_WORKER_TOKEN` (worker tier) for their own loop. ## Running the Server diff --git a/docs/worker-execution-contract.md b/docs/worker-execution-contract.md index c2f37479..117fc2e7 100644 --- a/docs/worker-execution-contract.md +++ b/docs/worker-execution-contract.md @@ -7,6 +7,7 @@ This document defines the generic execution contract for any agent worker consum ## Table of Contents +- [Token Tiers](#token-tiers) - [One Item Per Run](#one-item-per-run) - [PR Fix Queue Precedence](#pr-fix-queue-precedence) - [Duplicate PR Avoidance](#duplicate-pr-avoidance) @@ -18,6 +19,21 @@ This document defines the generic execution contract for any agent worker consum --- +## Token Tiers + +Dispatch bearer tokens have two tiers. A **worker** token (`DISPATCH_WORKER_TOKEN`) may call exactly: + +- `GET /api/agents/{agentName}/next-task`, `POST /api/agents/{agentName}/tasks/report`, `POST /api/agents/{agentName}/heartbeat`, `GET /api/agents/{agentName}/active-work`, `GET /api/agents/{agentName}/queue`, `GET /api/agents/{agentName}/work-summary` +- `POST /api/agent-work/start`, `POST /api/agent-work/checkpoint`, `POST /api/agent-work/finish` +- `POST /api/issues/claim` (without `force`) and `POST /api/issues/unclaim` (own claim only) +- `GET /api/issues/state`, `POST /api/issues/status` +- `GET /api/issues`, `GET /api/pr-fix-queue/queued`, `GET /api/pr-fix-queue/history` +- `POST /api/pr-fix-queue/mark` with `FIXED`, `BLOCKED`, or `STALE` (generation required, as today) + +The **maintainer** token (`DISPATCH_AGENT_TOKEN`, or the `DISPATCH_MAINTAINER_TOKEN` alias) keeps full rights. A worker token calling a maintainer-only route gets an HTTP 403 naming the required tier: force claims, releasing another agent's claim, `QUEUED`/`IGNORED` marks, and `POST /api/pr-fix-queue/requeue` are maintainer-only. + +--- + ## One Item Per Run A worker must handle **exactly one** queue item per execution: @@ -224,6 +240,7 @@ Workers using the canonical `next-task` endpoint automatically receive PR-fix it ## History +- **2026-09-28** — Added Token Tiers section: worker-tier endpoint allowlist and maintainer-only actions (Issue #1111). - **2026-05-16** — Created to document generic worker execution contract and PR completion gates (Issue #65). Consolidates existing normal-worker behavior into a reusable, agent-agnostic specification. - **2026-06-19** — Updated Renovate issue exclusion: Renovate issues are filtered from Dispatch issue surfaces, including Board, Projects, lane summaries, grooming intake, and agent queues. - **2026-05-19** — Added Renovate issue exclusion section: Renovate issues are excluded from agent queues by default (Issue #129). diff --git a/src/app/api/agent-runs/route.ts b/src/app/api/agent-runs/route.ts index 8826440d..67dfe52a 100644 --- a/src/app/api/agent-runs/route.ts +++ b/src/app/api/agent-runs/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { isValidEscalatedOutcome, VALID_ESCALATED_OUTCOMES } from "@/types"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; // Generous per-actor rate limit — agents report runs frequently, so this is @@ -15,8 +15,9 @@ const RATE_LIMIT = { limit: 120, windowMs: 60_000 }; const TOUCHED_ISSUE_URL_PATTERN = /^https?:\/\//; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); const limit = parseInt(searchParams.get("limit") || "50"); @@ -35,7 +36,7 @@ export async function GET(request: Request) { export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`agent-runs:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/agent-work/checkpoint/route.ts b/src/app/api/agent-work/checkpoint/route.ts index e005e60f..ef99cdfe 100644 --- a/src/app/api/agent-work/checkpoint/route.ts +++ b/src/app/api/agent-work/checkpoint/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma, asAgentWorkClient } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { parseCheckpointAgentWorkInput, checkpointAgentWork } from "@/lib/agent-work"; /** @@ -52,8 +52,9 @@ import { parseCheckpointAgentWorkInput, checkpointAgentWork } from "@/lib/agent- * { "error": "Failed to checkpoint agent work" } */ export async function POST(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/agent-work/finish/route.ts b/src/app/api/agent-work/finish/route.ts index 5f3e927b..28193bd5 100644 --- a/src/app/api/agent-work/finish/route.ts +++ b/src/app/api/agent-work/finish/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma, asAgentWorkClient } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { parseFinishAgentWorkInput, finishAgentWork } from "@/lib/agent-work"; /** @@ -47,8 +47,9 @@ import { parseFinishAgentWorkInput, finishAgentWork } from "@/lib/agent-work"; * { "error": "Failed to finish agent work" } */ export async function POST(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/agent-work/route.test.ts b/src/app/api/agent-work/route.test.ts index 6e18895f..61c37377 100644 --- a/src/app/api/agent-work/route.test.ts +++ b/src/app/api/agent-work/route.test.ts @@ -3,6 +3,12 @@ import { makeDispatchEnvMock } from "@/test/route-helpers"; vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); const mockAgentWork = { @@ -108,7 +114,7 @@ function makeGetRequest(url: string) { describe("GET /api/agent-work", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); agentWork.findMany.mockResolvedValue([]); lease.findMany.mockResolvedValue([]); }); @@ -727,7 +733,7 @@ describe("POST /api/agent-work", () => { describe("POST auth", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); }); it("returns 401 when token is invalid", async () => { diff --git a/src/app/api/agent-work/route.ts b/src/app/api/agent-work/route.ts index e89bd649..1f51ec50 100644 --- a/src/app/api/agent-work/route.ts +++ b/src/app/api/agent-work/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { releaseLeaseByAgentAndIssue, releaseAllLeasesByAgent, releaseAgentWorkByAgentAndIssue } from "@/lib/lease"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -52,7 +52,7 @@ function toItem(w: any): AgentWorkItem { export async function GET(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); @@ -129,7 +129,7 @@ export async function GET(request: Request) { export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`agent-work:${auth.actor}`, { limit: 30, windowMs: 10_000 }); diff --git a/src/app/api/agent-work/start/route.ts b/src/app/api/agent-work/start/route.ts index 8bdb8d38..a3fa0d88 100644 --- a/src/app/api/agent-work/start/route.ts +++ b/src/app/api/agent-work/start/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma, asAgentWorkClient } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { parseStartAgentWorkInput, startAgentWork } from "@/lib/agent-work"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -49,7 +49,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 } as const; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`route:agent-work/start:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/agent-work/sweep/route.test.ts b/src/app/api/agent-work/sweep/route.test.ts index 6a03c136..74c03642 100644 --- a/src/app/api/agent-work/sweep/route.test.ts +++ b/src/app/api/agent-work/sweep/route.test.ts @@ -8,7 +8,15 @@ const mocks = vi.hoisted(() => ({ sweepStaleWork: vi.fn(), })); -vi.mock("@/lib/auth", () => ({ authorizeRequest: mocks.authorizeRequest })); +vi.mock("@/lib/auth", () => ({ + authorizeRequest: mocks.authorizeRequest, + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), +})); vi.mock("@/lib/dispatch-env", () => makeDispatchEnvMock()); vi.mock("@/lib/prisma", () => ({ prisma: {} })); vi.mock("@/lib/sync-lock", () => ({ diff --git a/src/app/api/agent-work/sweep/route.ts b/src/app/api/agent-work/sweep/route.ts index a3eaa46a..08e994b6 100644 --- a/src/app/api/agent-work/sweep/route.ts +++ b/src/app/api/agent-work/sweep/route.ts @@ -1,5 +1,5 @@ import { NextResponse } from "next/server"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { acquireLock, releaseLock } from "@/lib/sync-lock"; @@ -12,7 +12,7 @@ const BATCH_SIZE = DEFAULT_STALE_WORK_BATCH_SIZE; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const lock = await acquireLock("stale-work"); diff --git a/src/app/api/agents/[agentName]/active-work/route.test.ts b/src/app/api/agents/[agentName]/active-work/route.test.ts index 8dd21152..a323a42d 100644 --- a/src/app/api/agents/[agentName]/active-work/route.test.ts +++ b/src/app/api/agents/[agentName]/active-work/route.test.ts @@ -10,6 +10,12 @@ const { mocks } = vi.hoisted(() => ({ vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/prisma", () => ({ @@ -47,7 +53,7 @@ function makeActiveWorkRequest(agentName: string) { describe("GET /api/agents/:agentName/active-work", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); // Default: return the same lease for both findFirst calls (resolveActiveWork and leaseId fetch) mocks.leaseFindFirst.mockResolvedValue({ id: "l-1", diff --git a/src/app/api/agents/[agentName]/active-work/route.ts b/src/app/api/agents/[agentName]/active-work/route.ts index 0b48c6e8..762e00cc 100644 --- a/src/app/api/agents/[agentName]/active-work/route.ts +++ b/src/app/api/agents/[agentName]/active-work/route.ts @@ -2,12 +2,12 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { resolveActiveWork } from "@/lib/lease"; import type { ActiveWorkResult } from "@/lib/next-action"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request, { params }: { params: Promise<{ agentName: string }> }) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const { agentName } = await params; diff --git a/src/app/api/agents/[agentName]/heartbeat/route.ts b/src/app/api/agents/[agentName]/heartbeat/route.ts index 1c4e6dd4..1e49670c 100644 --- a/src/app/api/agents/[agentName]/heartbeat/route.ts +++ b/src/app/api/agents/[agentName]/heartbeat/route.ts @@ -14,7 +14,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { runSyncBestEffort, runReconcileBestEffort } from "@/lib/heartbeat"; export type AgentHeartbeatResponse = { @@ -39,8 +39,9 @@ export async function POST( const { agentName } = await params; // Authenticate - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const startedAt = new Date(); diff --git a/src/app/api/agents/[agentName]/next-task/route.ts b/src/app/api/agents/[agentName]/next-task/route.ts index eeb35674..46ad6216 100644 --- a/src/app/api/agents/[agentName]/next-task/route.ts +++ b/src/app/api/agents/[agentName]/next-task/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; import { createIdleTask, @@ -18,8 +18,9 @@ export async function GET( ) { const { agentName } = await params; - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/agents/[agentName]/queue/route.ts b/src/app/api/agents/[agentName]/queue/route.ts index b29680f0..893c6ef8 100644 --- a/src/app/api/agents/[agentName]/queue/route.ts +++ b/src/app/api/agents/[agentName]/queue/route.ts @@ -1,13 +1,14 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { fetchAgentQueueData } from "@/lib/agent-queue-fetch"; export async function GET(request: Request, { params }: { params: Promise<{ agentName: string }> }) { const { agentName } = await params; - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/agents/[agentName]/tasks/report/route.ts b/src/app/api/agents/[agentName]/tasks/report/route.ts index 5bf5fd19..b246ec0b 100644 --- a/src/app/api/agents/[agentName]/tasks/report/route.ts +++ b/src/app/api/agents/[agentName]/tasks/report/route.ts @@ -3,7 +3,7 @@ import { createHash } from "node:crypto"; import { Prisma } from "@prisma/client"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { resolvePrFixFromAgentReport, type ResolvePrFixFromAgentReportResult } from "@/lib/pr-fix-queue"; const VALID_TASK_TYPES = ["implement", "followup-pr", "groom"] as const; @@ -109,8 +109,9 @@ export async function POST( const { agentName } = await params; // Authenticate - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } let body: unknown; diff --git a/src/app/api/agents/[agentName]/work-summary/route.ts b/src/app/api/agents/[agentName]/work-summary/route.ts index 69304476..ba1996e1 100644 --- a/src/app/api/agents/[agentName]/work-summary/route.ts +++ b/src/app/api/agents/[agentName]/work-summary/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { listQueuedPrFixItems } from "@/lib/pr-fix-queue"; import { getConfiguredLanes, getDefaultClaimableLane, resolveLaneId } from "@/lib/lane-config"; @@ -29,8 +29,9 @@ function classifyIssueStatus(labels: string[]): "queued" | "inProgress" { export async function GET(request: Request, { params }: { params: Promise<{ agentName: string }> }) { const { agentName } = await params; - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/audit/route.ts b/src/app/api/audit/route.ts index d71a4c4e..47bb6736 100644 --- a/src/app/api/audit/route.ts +++ b/src/app/api/audit/route.ts @@ -1,11 +1,12 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); const limit = parseInt(searchParams.get("limit") || "50"); diff --git a/src/app/api/automation/events/route.test.ts b/src/app/api/automation/events/route.test.ts index 06b814a3..59775467 100644 --- a/src/app/api/automation/events/route.test.ts +++ b/src/app/api/automation/events/route.test.ts @@ -3,6 +3,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; // Mock auth before importing the route vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); // Mock prisma after auth to avoid import order issues @@ -24,7 +30,7 @@ const mockAuthorizeRequest = vi.mocked(authorizeRequest); describe("GET /api/automation/events", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); }); it("returns 401 when not authenticated", async () => { diff --git a/src/app/api/automation/events/route.ts b/src/app/api/automation/events/route.ts index e02b1a4b..e3bc7d21 100644 --- a/src/app/api/automation/events/route.ts +++ b/src/app/api/automation/events/route.ts @@ -2,12 +2,12 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { jsonSafe } from "@/lib/json"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/automation/repos/[...repo]/route.ts b/src/app/api/automation/repos/[...repo]/route.ts index 82905d4a..d501e103 100644 --- a/src/app/api/automation/repos/[...repo]/route.ts +++ b/src/app/api/automation/repos/[...repo]/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { jsonSafe } from "@/lib/json"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; interface RouteContext { params: Promise<{ repo: string[] }>; @@ -85,7 +85,7 @@ export async function GET(request: Request, context: RouteContext) { export async function DELETE(request: Request, context: RouteContext) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const auditActor = getAuthorizedActor(auth, request); diff --git a/src/app/api/automation/repos/route.ts b/src/app/api/automation/repos/route.ts index c4a4fd06..318ed158 100644 --- a/src/app/api/automation/repos/route.ts +++ b/src/app/api/automation/repos/route.ts @@ -5,11 +5,12 @@ import { prisma } from "@/lib/prisma"; import { jsonSafe } from "@/lib/json"; import { isValidRepoName } from "@/lib/config"; import { auditTrackedRepoCreateFailure, createTrackedRepo } from "@/lib/tracked-repos"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { const repos = await prisma.automationRepo.findMany({ @@ -73,8 +74,9 @@ export async function GET(request: Request) { } export async function POST(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } let body: unknown; diff --git a/src/app/api/automation/repos/tracked/route.ts b/src/app/api/automation/repos/tracked/route.ts index 0d04cb1a..d8b73f46 100644 --- a/src/app/api/automation/repos/tracked/route.ts +++ b/src/app/api/automation/repos/tracked/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { jsonSafe } from "@/lib/json"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; /** * GET /api/automation/repos/tracked @@ -19,7 +19,7 @@ import { authorizeRequest } from "@/lib/auth"; export async function GET(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } try { diff --git a/src/app/api/automation/runs/[runId]/route.ts b/src/app/api/automation/runs/[runId]/route.ts index 746b15e6..14df57d3 100644 --- a/src/app/api/automation/runs/[runId]/route.ts +++ b/src/app/api/automation/runs/[runId]/route.ts @@ -2,12 +2,12 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { rerunWorkflow, triggerWorkflowDispatch } from "@/lib/github"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; export async function POST(request: Request, { params }: { params: Promise<{ runId: string }> }) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const auditActor = getAuthorizedActor(auth, request); diff --git a/src/app/api/automation/sync/route.ts b/src/app/api/automation/sync/route.ts index 196fa28f..55bad168 100644 --- a/src/app/api/automation/sync/route.ts +++ b/src/app/api/automation/sync/route.ts @@ -2,13 +2,14 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { getTrackedRepos } from "@/lib/config"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { acquireLock, releaseLock } from "@/lib/sync-lock"; import { syncAutomationRepo } from "@/lib/automation-sync"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const repos = await prisma.automationRepo.findMany({ orderBy: { fullName: "asc" }, @@ -20,8 +21,9 @@ export async function GET(request: Request) { } export async function POST(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const body = await request.json().catch(() => ({})); diff --git a/src/app/api/automation/workflows/[id]/route.test.ts b/src/app/api/automation/workflows/[id]/route.test.ts index 7abc3f6e..e24a4aef 100644 --- a/src/app/api/automation/workflows/[id]/route.test.ts +++ b/src/app/api/automation/workflows/[id]/route.test.ts @@ -2,6 +2,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/prisma", () => ({ @@ -22,7 +28,7 @@ const mockAuthorizeRequest = vi.mocked(authorizeRequest); describe("GET /api/automation/workflows/[id]", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); }); it("returns 401 when not authenticated", async () => { diff --git a/src/app/api/automation/workflows/[id]/route.ts b/src/app/api/automation/workflows/[id]/route.ts index e9b3c27a..4dd108bb 100644 --- a/src/app/api/automation/workflows/[id]/route.ts +++ b/src/app/api/automation/workflows/[id]/route.ts @@ -2,12 +2,12 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { jsonSafe } from "@/lib/json"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/automation/workflows/route.test.ts b/src/app/api/automation/workflows/route.test.ts index 04e441c2..819d46d9 100644 --- a/src/app/api/automation/workflows/route.test.ts +++ b/src/app/api/automation/workflows/route.test.ts @@ -2,6 +2,12 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/prisma", () => ({ @@ -22,7 +28,7 @@ const mockAuthorizeRequest = vi.mocked(authorizeRequest); describe("GET /api/automation/workflows", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); }); it("returns 401 when not authenticated", async () => { diff --git a/src/app/api/automation/workflows/route.ts b/src/app/api/automation/workflows/route.ts index 24adc529..5cbbc71d 100644 --- a/src/app/api/automation/workflows/route.ts +++ b/src/app/api/automation/workflows/route.ts @@ -2,12 +2,12 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { jsonSafe } from "@/lib/json"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/ci-failures/sync/route.ts b/src/app/api/ci-failures/sync/route.ts index d5899c8b..af830937 100644 --- a/src/app/api/ci-failures/sync/route.ts +++ b/src/app/api/ci-failures/sync/route.ts @@ -1,6 +1,6 @@ import { NextRequest, NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { getTrackedRepos } from "@/lib/config"; import { fetchRepositoryMetadata, @@ -72,7 +72,7 @@ async function filedIssuesFor(repoFullName: string): Promise { export async function POST(request: NextRequest) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`ci-failures-sync:${auth.actor}`, { diff --git a/src/app/api/groomer/run/route.ts b/src/app/api/groomer/run/route.ts index 30dfac7d..b95ad5e6 100644 --- a/src/app/api/groomer/run/route.ts +++ b/src/app/api/groomer/run/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; -import { authorizeGroomerRequest } from "@/lib/auth"; +import { authorizeGroomerRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; import { runHostedGroomer } from "@/lib/groomer/run"; import { getHostedGroomerConfig } from "@/lib/groomer/config"; @@ -13,7 +13,7 @@ const RATE_LIMIT = { limit: 10, windowMs: 60_000 }; export async function POST(request: Request) { const auth = await authorizeGroomerRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`groomer/run:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/groomer/runs/[id]/route.test.ts b/src/app/api/groomer/runs/[id]/route.test.ts index 015e54a1..ec496f73 100644 --- a/src/app/api/groomer/runs/[id]/route.test.ts +++ b/src/app/api/groomer/runs/[id]/route.test.ts @@ -10,6 +10,12 @@ const { mocks } = vi.hoisted(() => ({ vi.mock("@/lib/auth", () => ({ authorizeRequest: mocks.authorizeRequest, + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/groomer/history", () => ({ diff --git a/src/app/api/groomer/runs/[id]/route.ts b/src/app/api/groomer/runs/[id]/route.ts index 5aa617bf..06e84dc8 100644 --- a/src/app/api/groomer/runs/[id]/route.ts +++ b/src/app/api/groomer/runs/[id]/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { getGroomingRunDetail } from "@/lib/groomer/history"; import { jsonSafe } from "@/lib/json"; @@ -9,8 +9,9 @@ export async function GET( request: Request, { params }: { params: Promise<{ id: string }> }, ) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { id } = await params; diff --git a/src/app/api/groomer/runs/route.test.ts b/src/app/api/groomer/runs/route.test.ts index ca42127a..7ce53120 100644 --- a/src/app/api/groomer/runs/route.test.ts +++ b/src/app/api/groomer/runs/route.test.ts @@ -10,6 +10,12 @@ const { mocks } = vi.hoisted(() => ({ vi.mock("@/lib/auth", () => ({ authorizeRequest: mocks.authorizeRequest, + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/groomer/history", () => ({ diff --git a/src/app/api/groomer/runs/route.ts b/src/app/api/groomer/runs/route.ts index 46d4b771..eceabc80 100644 --- a/src/app/api/groomer/runs/route.ts +++ b/src/app/api/groomer/runs/route.ts @@ -1,13 +1,14 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { listGroomingRuns } from "@/lib/groomer/history"; import { jsonSafe } from "@/lib/json"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); const issueNumber = searchParams.get("issueNumber"); diff --git a/src/app/api/issues/[issueId]/admission-override/route.ts b/src/app/api/issues/[issueId]/admission-override/route.ts index 5b069fe3..390dfba0 100644 --- a/src/app/api/issues/[issueId]/admission-override/route.ts +++ b/src/app/api/issues/[issueId]/admission-override/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest, getAuthorizedActor, type AuthorizedRequest } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, type AuthorizedRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; import { enforceRateLimit } from "@/lib/rate-limit"; import { fetchLatestCommit } from "@/lib/github-ci"; @@ -60,7 +60,7 @@ async function readBody(request: Request): Promise | nul */ export async function POST(request: Request, context: { params: Promise<{ issueId: string }> }) { const auth = await authorizeRequest(request); - if (!auth.authorized) return errorResponse("Unauthorized", 401); + if (!auth.authorized) return authErrorResponse(auth); const forbidden = rejectAgentCaller(auth); if (forbidden) return forbidden; @@ -175,7 +175,7 @@ export async function POST(request: Request, context: { params: Promise<{ issueI */ export async function DELETE(request: Request, context: { params: Promise<{ issueId: string }> }) { const auth = await authorizeRequest(request); - if (!auth.authorized) return errorResponse("Unauthorized", 401); + if (!auth.authorized) return authErrorResponse(auth); const forbidden = rejectAgentCaller(auth); if (forbidden) return forbidden; diff --git a/src/app/api/issues/[issueId]/lane/route.ts b/src/app/api/issues/[issueId]/lane/route.ts index 2e0c7d2c..09da59be 100644 --- a/src/app/api/issues/[issueId]/lane/route.ts +++ b/src/app/api/issues/[issueId]/lane/route.ts @@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { parseLaneClassification, classifyLaneByHeuristics, validateLaneRecord } from "@/lib/issue-lane"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; interface LaneRequestBody { @@ -19,7 +19,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: NextRequest, context: { params: Promise<{ issueId: string }> }) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`lane:${auth.actor}`, RATE_LIMIT); @@ -141,8 +141,9 @@ export async function POST(request: NextRequest, context: { params: Promise<{ is * GET /api/issues/[issueId]/lane — Get the current lane classification for an issue. */ export async function GET(_request: NextRequest, context: { params: Promise<{ issueId: string }> }) { - if (!(await authorizeRequest(_request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(_request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/issues/[issueId]/pr-health/refresh/route.ts b/src/app/api/issues/[issueId]/pr-health/refresh/route.ts index 4731bcbc..6618f20d 100644 --- a/src/app/api/issues/[issueId]/pr-health/refresh/route.ts +++ b/src/app/api/issues/[issueId]/pr-health/refresh/route.ts @@ -1,7 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { fetchPullRequests, fetchLinkedPrHealthInput } from "@/lib/github"; import { computeLinkedPrHealth, toPersistedLinkedPrHealth } from "@/lib/linked-pr-health"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -22,7 +22,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: NextRequest, context: { params: Promise<{ issueId: string }> }) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`pr-health-refresh:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/actions/agents/route.test.ts b/src/app/api/issues/actions/agents/route.test.ts index 0191d14a..5bfa671a 100644 --- a/src/app/api/issues/actions/agents/route.test.ts +++ b/src/app/api/issues/actions/agents/route.test.ts @@ -12,6 +12,12 @@ const { mocks } = vi.hoisted(() => ({ vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/config", () => ({ @@ -33,7 +39,7 @@ const mockAuthorizeRequest = vi.mocked(authorizeRequest); describe("GET /api/issues/actions/agents", () => { beforeEach(() => { vi.clearAllMocks(); - mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent" }); + mockAuthorizeRequest.mockResolvedValue({ authorized: true, type: "disabled", actor: "test-agent", tier: "maintainer" }); mocks.parseAgentList.mockReturnValue(["worker", "reviewer"]); mocks.findMany.mockResolvedValue([ { labels: ["status/backlog", "agent/handler", "type/feature"] }, diff --git a/src/app/api/issues/actions/agents/route.ts b/src/app/api/issues/actions/agents/route.ts index 4a648434..63eeb877 100644 --- a/src/app/api/issues/actions/agents/route.ts +++ b/src/app/api/issues/actions/agents/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { parseAgentList } from "@/lib/config"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; /** * GET /api/issues/actions/agents @@ -14,7 +14,7 @@ import { authorizeRequest } from "@/lib/auth"; export async function GET(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } try { diff --git a/src/app/api/issues/actions/decompose/route.ts b/src/app/api/issues/actions/decompose/route.ts index c63320c8..1cb69048 100644 --- a/src/app/api/issues/actions/decompose/route.ts +++ b/src/app/api/issues/actions/decompose/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { resolveActor } from "@/lib/resolve-actor"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -19,7 +19,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 } as const; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`route:issues/actions/decompose:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/actions/route.ts b/src/app/api/issues/actions/route.ts index e12baef5..700575bc 100644 --- a/src/app/api/issues/actions/route.ts +++ b/src/app/api/issues/actions/route.ts @@ -5,7 +5,7 @@ import { updateIssueLabels } from "@/lib/github"; import { analyzeAssignmentConflict, buildNewLabels } from "@/lib/assignment-conflicts"; import { getLiveIssueLabels } from "@/lib/claim-gate"; import { AGENT_PREFIX, OWNER_PREFIX } from "@/types"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; type ActionPayload = { issueId?: string; @@ -19,7 +19,7 @@ type ActionPayload = { export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const auditActor = getAuthorizedActor(auth, request); diff --git a/src/app/api/issues/claim/route.test.ts b/src/app/api/issues/claim/route.test.ts index d5ff8a6a..b2820f06 100644 --- a/src/app/api/issues/claim/route.test.ts +++ b/src/app/api/issues/claim/route.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; import { TEST_AGENT_TOKEN as mockToken, authedRequest } from "@/test/route-helpers"; +const { WORKER_TOKEN } = vi.hoisted(() => ({ WORKER_TOKEN: "worker-token" })); + const { mocks } = vi.hoisted(() => ({ mocks: { findUnique: vi.fn(), updateIssue: vi.fn(), createAuditLog: vi.fn(), @@ -21,6 +23,7 @@ const { mocks } = vi.hoisted(() => ({ })); process.env.DISPATCH_AGENT_TOKEN = mockToken; +process.env.DISPATCH_WORKER_TOKEN = WORKER_TOKEN; vi.mock("@/lib/prisma", () => ({ prisma: { @@ -458,3 +461,47 @@ describe("POST /api/issues/claim — #1037 live label gate", () => { expect((await res.json()).error).toBe("Cannot claim a done issue"); }); }); + +describe("POST /api/issues/claim — worker tier (#1111)", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.findUnique.mockResolvedValue({ id: "issue-1", state: "open", labels: [] as string[] }); + mocks.updateIssue.mockResolvedValue(undefined); + mocks.createAuditLog.mockResolvedValue({ id: "log-1" }); + mocks.addIssueLabel.mockResolvedValue(undefined); + mocks.removeIssueLabel.mockResolvedValue(undefined); + mocks.leaseFindMany.mockResolvedValue([]); + mocks.leaseDeleteMany.mockResolvedValue({ count: 0 }); + mocks.leaseCreate.mockResolvedValue({ id: "l-1", agentName: "worker-agent", issueId: "issue-1", checkpoint: "issue_claimed", branch: null, prUrl: null, expiredAt: new Date(Date.now() + 60000), renewedAt: new Date(), createdAt: new Date() }); + }); + + function workerRequest(overrides = {}) { + const payload = { ...makePayload({ agentName: "worker-agent" }), ...overrides }; + return authedRequest("http://localhost/api/issues/claim", { + method: "POST", + body: payload, + token: WORKER_TOKEN, + headers: { "x-agent-name": "worker-agent" }, + }); + } + + it("returns 403 when a worker force-claims", async () => { + const res = await POST(workerRequest({ force: true })); + expect(res.status).toBe(403); + expect((await res.json()).error).toBe("Force claim requires a maintainer token"); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(mocks.createAuditLog).toHaveBeenCalledWith({ + data: expect.objectContaining({ + action: "claim_issue", + success: false, + errorMessage: "Force claim requires a maintainer token", + }), + }); + }); + + it("allows a worker to claim without force", async () => { + const res = await POST(workerRequest()); + expect(res.status).toBe(200); + expect((await res.json()).success).toBe(true); + }); +}); diff --git a/src/app/api/issues/claim/route.ts b/src/app/api/issues/claim/route.ts index 0bc8660d..cd9fc511 100644 --- a/src/app/api/issues/claim/route.ts +++ b/src/app/api/issues/claim/route.ts @@ -5,7 +5,7 @@ import { addIssueLabel, removeIssueLabel } from "@/lib/github"; import { analyzeAssignmentConflict, buildNewLabels } from "@/lib/assignment-conflicts"; import { getLiveIssueLabels } from "@/lib/claim-gate"; import { AGENT_PREFIX } from "@/types"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { upsertLease, findActiveLeasesForIssue, releaseExpiredLeases } from "@/lib/lease"; import { findAndReleaseStaleAgentWorkForIssue } from "@/lib/agent-work"; import { transitionIssueStatus } from "@/lib/issue-status"; @@ -17,7 +17,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`claim:${auth.actor}`, RATE_LIMIT); @@ -42,6 +42,29 @@ export async function POST(request: Request) { return errorResponse("Missing required fields: issueId, repoFullName, issueNumber, agentName", 400); } + // Worker tokens may only claim normally: force-claiming over another + // agent's lease/assignment requires a maintainer token (#1111). + if (force === true && auth.type === "bearer" && auth.tier === "worker") { + try { + await prisma.auditLog.create({ + data: { + actor: agentName as string, + action: "claim_issue", + repoFullName: repoFullName as string, + issueNumber: issueNumber as number, + issueId: issueId as string, + beforeLabels: [], + afterLabels: [], + success: false, + errorMessage: "Force claim requires a maintainer token", + }, + }); + } catch { + // Audit log failure should not mask the 403 + } + return errorResponse("Force claim requires a maintainer token", 403); + } + // Fetch the issue from the local database to check its state and current labels const issue = await prisma.issue.findUnique({ where: { id: issueId as string }, diff --git a/src/app/api/issues/claimed/route.ts b/src/app/api/issues/claimed/route.ts index 97d688e5..947e53ab 100644 --- a/src/app/api/issues/claimed/route.ts +++ b/src/app/api/issues/claimed/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; /** Statuses this endpoint will list. Kept narrow: it exists to find claimed work, @@ -8,8 +8,9 @@ import { prisma } from "@/lib/prisma"; const ALLOWED_CLAIMED_STATUSES = ["in-progress", "ready"]; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/issues/comment/route.ts b/src/app/api/issues/comment/route.ts index bcfa726f..063cfb4f 100644 --- a/src/app/api/issues/comment/route.ts +++ b/src/app/api/issues/comment/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { addIssueComment } from "@/lib/github-issues"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -21,7 +21,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`comment:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/groom/route.ts b/src/app/api/issues/groom/route.ts index 0833d95c..57f7cf02 100644 --- a/src/app/api/issues/groom/route.ts +++ b/src/app/api/issues/groom/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { getEscalationLane, getDefaultClaimableLane, isClaimableLane } from "@/lib/lane-config"; import { resolveActor } from "@/lib/resolve-actor"; import { transitionIssueStatus } from "@/lib/issue-status"; @@ -15,7 +15,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 } as const; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`route:issues/groom:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/label/route.test.ts b/src/app/api/issues/label/route.test.ts index 5bfc3b98..dca671ac 100644 --- a/src/app/api/issues/label/route.test.ts +++ b/src/app/api/issues/label/route.test.ts @@ -22,6 +22,12 @@ import { resetRateLimits } from "@/lib/rate-limit"; vi.mock("@/lib/auth", () => ({ authorizeRequest: vi.fn(), getAuthorizedActor: vi.fn(), + authErrorResponse: vi.fn((auth: { forbidden?: boolean }) => + new Response(JSON.stringify({ error: auth.forbidden ? "Forbidden" : "Unauthorized" }), { + status: auth.forbidden ? 403 : 401, + headers: { "content-type": "application/json" }, + }), + ), })); vi.mock("@/lib/prisma", () => ({ diff --git a/src/app/api/issues/label/route.ts b/src/app/api/issues/label/route.ts index 33a91c44..af997823 100644 --- a/src/app/api/issues/label/route.ts +++ b/src/app/api/issues/label/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { addIssueLabel, removeIssueLabel } from "@/lib/github-issues"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -25,7 +25,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`label:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/move/route.ts b/src/app/api/issues/move/route.ts index 0c0d4ef2..ce2da6c0 100644 --- a/src/app/api/issues/move/route.ts +++ b/src/app/api/issues/move/route.ts @@ -3,7 +3,7 @@ import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { removeIssueLabel } from "@/lib/github"; import { STATUS_LABELS, isStatusLabel } from "@/types"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; import { transitionIssueStatus } from "@/lib/issue-status"; @@ -14,7 +14,7 @@ const RATE_LIMIT = { limit: 60, windowMs: 60_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`issues/move:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/prune-closed/route.ts b/src/app/api/issues/prune-closed/route.ts index 574be1b5..2c0ee38f 100644 --- a/src/app/api/issues/prune-closed/route.ts +++ b/src/app/api/issues/prune-closed/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; const RATE_LIMIT = { limit: 10, windowMs: 60_000 }; @@ -9,7 +9,7 @@ const RATE_LIMIT = { limit: 10, windowMs: 60_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`prune-closed:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/reconcile/route.ts b/src/app/api/issues/reconcile/route.ts index b2d6f17e..23698b25 100644 --- a/src/app/api/issues/reconcile/route.ts +++ b/src/app/api/issues/reconcile/route.ts @@ -12,7 +12,7 @@ import { } from "@/lib/issue-reconciliation"; import { isBacklogLane } from "@/lib/lane-config"; import { computeLinkedPrHealth, toPersistedLinkedPrHealth, type LinkedPrHealth } from "@/lib/linked-pr-health"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { reconcileStalePrFixItems } from "@/lib/pr-fix-queue"; import { enforceRateLimit } from "@/lib/rate-limit"; import { acquireLock, releaseLock, type AcquiredLock, type LockConflict } from "@/lib/sync-lock"; @@ -34,7 +34,7 @@ const RATE_LIMIT = { limit: 10, windowMs: 60_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`reconcile:${auth.actor}`, RATE_LIMIT); @@ -338,8 +338,9 @@ export async function POST(request: Request) { * GET endpoint to check reconciliation status and last run time. */ export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/issues/refresh/route.ts b/src/app/api/issues/refresh/route.ts index 6dfdcf56..cb156ceb 100644 --- a/src/app/api/issues/refresh/route.ts +++ b/src/app/api/issues/refresh/route.ts @@ -4,7 +4,7 @@ import { prisma } from "@/lib/prisma"; import { fetchIssue as fetchIssueFromGitHub } from "@/lib/github"; import { getSyncRepos } from "@/lib/config"; import { refreshSingleIssue, defaultCurrentLane } from "@/lib/issue-sync"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; const RATE_LIMIT = { limit: 10, windowMs: 60_000 } as const; @@ -12,7 +12,7 @@ const RATE_LIMIT = { limit: 10, windowMs: 60_000 } as const; export async function POST(request: NextRequest) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`route:issues/refresh:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/route.ts b/src/app/api/issues/route.ts index 54835975..1c9b085f 100644 --- a/src/app/api/issues/route.ts +++ b/src/app/api/issues/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; import { appendIssueWhere, @@ -18,8 +18,9 @@ import { withDependencyBlockReasons } from "@/lib/issue-dependency-annotation"; import { withAdmissionAnnotations } from "@/lib/queue-admission"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/issues/state/route.ts b/src/app/api/issues/state/route.ts index ebe4a9ea..c726e927 100644 --- a/src/app/api/issues/state/route.ts +++ b/src/app/api/issues/state/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; /** @@ -18,8 +18,9 @@ import { prisma } from "@/lib/prisma"; * rather than inferring closure. */ export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/issues/status/route.ts b/src/app/api/issues/status/route.ts index ec98d5cc..61d6c8d2 100644 --- a/src/app/api/issues/status/route.ts +++ b/src/app/api/issues/status/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { STATUS_LABELS, StatusLabel, isStatusLabel } from "@/types"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { transitionIssueStatus } from "@/lib/issue-status"; import { getLiveIssueLabels } from "@/lib/claim-gate"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -12,7 +12,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`status:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/unassign/route.ts b/src/app/api/issues/unassign/route.ts index 2b6c160f..bff5b5de 100644 --- a/src/app/api/issues/unassign/route.ts +++ b/src/app/api/issues/unassign/route.ts @@ -3,7 +3,7 @@ import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { updateIssueLabels } from "@/lib/github"; import { buildUnassignedLabels, getAgentLabels, getOwnerLabels } from "@/lib/assignment-conflicts"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; const RATE_LIMIT = { limit: 30, windowMs: 10_000 } as const; @@ -24,7 +24,7 @@ type UnassignPayload = { export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const auditActor = getAuthorizedActor(auth, request); diff --git a/src/app/api/issues/unclaim/route.test.ts b/src/app/api/issues/unclaim/route.test.ts index dbe0a15b..d483be00 100644 --- a/src/app/api/issues/unclaim/route.test.ts +++ b/src/app/api/issues/unclaim/route.test.ts @@ -1,6 +1,8 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; import { TEST_AGENT_TOKEN as mockToken, authedRequest } from "@/test/route-helpers"; +const { WORKER_TOKEN } = vi.hoisted(() => ({ WORKER_TOKEN: "worker-token" })); + const { mocks } = vi.hoisted(() => ({ mocks: { findUnique: vi.fn().mockResolvedValue(null), @@ -16,6 +18,7 @@ const { mocks } = vi.hoisted(() => ({ })); process.env.DISPATCH_AGENT_TOKEN = mockToken; +process.env.DISPATCH_WORKER_TOKEN = WORKER_TOKEN; vi.mock("@/lib/prisma", () => ({ prisma: { @@ -529,4 +532,54 @@ describe("POST /api/issues/unclaim — guards", () => { expect(mocks.addIssueLabel).not.toHaveBeenCalled(); expect(mocks.updateIssueLabels).not.toHaveBeenCalled(); }); +}); + +describe("POST /api/issues/unclaim — worker tier (#1111)", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.findUnique.mockResolvedValue({ + id: "issue-1", + state: "open", + labels: ["agent/test-agent"], + } as never); + mocks.updateIssue.mockResolvedValue(undefined); + mocks.createAuditLog.mockResolvedValue({ id: "log-1" }); + mocks.removeIssueLabel.mockResolvedValue(undefined); + mocks.addIssueLabel.mockResolvedValue(undefined); + mocks.updateIssueLabels.mockResolvedValue(undefined); + mocks.releaseLeaseByAgentAndIssue.mockResolvedValue(undefined); + mocks.releaseAgentWorkByAgentAndIssue.mockResolvedValue(0); + }); + + function workerPost(xAgentName: string) { + return POST( + authedRequest("http://localhost/api/issues/unclaim", { + method: "POST", + body: makePayload(), + token: WORKER_TOKEN, + headers: { "x-agent-name": xAgentName }, + }), + ); + } + + it("allows a worker to release its own claim (x-agent-name matches)", async () => { + const res = await workerPost("test-agent"); + expect(res.status).toBe(200); + expect((await res.json()).success).toBe(true); + expect(mocks.releaseLeaseByAgentAndIssue).toHaveBeenCalledWith("test-agent", "issue-1"); + }); + + it("returns 403 when a worker releases another agent's claim", async () => { + const res = await workerPost("other-agent"); + expect(res.status).toBe(403); + expect((await res.json()).error).toBe("Releasing another agent's claim requires a maintainer token"); + expect(mocks.releaseLeaseByAgentAndIssue).not.toHaveBeenCalled(); + expect(mocks.createAuditLog).toHaveBeenCalledWith({ + data: expect.objectContaining({ + action: "unclaim_issue", + success: false, + errorMessage: "Releasing another agent's claim requires a maintainer token", + }), + }); + }); }); \ No newline at end of file diff --git a/src/app/api/issues/unclaim/route.ts b/src/app/api/issues/unclaim/route.ts index 86f945a1..9476e5c0 100644 --- a/src/app/api/issues/unclaim/route.ts +++ b/src/app/api/issues/unclaim/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; import { getAgentFromLabels, AGENT_PREFIX } from "@/types"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { releaseLeaseByAgentAndIssue, releaseAgentWorkByAgentAndIssue, @@ -15,7 +15,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`unclaim:${auth.actor}`, RATE_LIMIT); @@ -40,6 +40,30 @@ export async function POST(request: Request) { return errorResponse("Missing required fields: issueId, repoFullName, issueNumber, agentName", 400); } + // Worker tokens may only release their own claim: releasing another + // agent's claim requires a maintainer token (#1111). + const callerIdentity = request.headers.get("x-agent-name")?.trim(); + if (auth.type === "bearer" && auth.tier === "worker" && callerIdentity !== agentName) { + try { + await prisma.auditLog.create({ + data: { + actor: agentName as string, + action: "unclaim_issue", + repoFullName: repoFullName as string, + issueNumber: issueNumber as number, + issueId: issueId as string, + beforeLabels: [], + afterLabels: [], + success: false, + errorMessage: "Releasing another agent's claim requires a maintainer token", + }, + }); + } catch { + // Audit log failure should not mask the 403 + } + return errorResponse("Releasing another agent's claim requires a maintainer token", 403); + } + const agentLabel = `${AGENT_PREFIX}${agentName}` as const; const actor = getAuthorizedActor(auth, request, agentName as string); const isAgentSelfUnclaim = auth.type === "bearer" && actor === agentName; diff --git a/src/app/api/issues/unlabel/route.ts b/src/app/api/issues/unlabel/route.ts index ba578916..b249414f 100644 --- a/src/app/api/issues/unlabel/route.ts +++ b/src/app/api/issues/unlabel/route.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma } from "@/lib/prisma"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { removeIssueLabel } from "@/lib/github-issues"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -22,7 +22,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`unlabel:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/issues/untriaged/route.ts b/src/app/api/issues/untriaged/route.ts index a48ee2da..1ac09205 100644 --- a/src/app/api/issues/untriaged/route.ts +++ b/src/app/api/issues/untriaged/route.ts @@ -4,7 +4,7 @@ import { prisma } from "@/lib/prisma"; import { STATUS_LABELS } from "@/types"; import { isRenovateIssue } from "@/lib/agent-queue"; import { applyRenovateIssueExclusion } from "@/lib/issue-filters"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; /** * GET /api/issues/untriaged @@ -32,8 +32,9 @@ interface UntriagedIssue { } export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/issues/webhook/route.ts b/src/app/api/issues/webhook/route.ts index 74cb5f3d..18618aeb 100644 --- a/src/app/api/issues/webhook/route.ts +++ b/src/app/api/issues/webhook/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; import { getSignatureVerificationMode, verifyWebhookSignature } from "@/lib/webhook-signature"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -73,7 +73,7 @@ export async function POST(request: Request) { if (sigMode !== "verify") { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } actor = auth.actor ?? "webhook"; } diff --git a/src/app/api/lanes/route.ts b/src/app/api/lanes/route.ts index 9539d98c..394b0846 100644 --- a/src/app/api/lanes/route.ts +++ b/src/app/api/lanes/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { getConfiguredLanes } from "@/lib/lane-config"; /** @@ -19,8 +19,9 @@ import { getConfiguredLanes } from "@/lib/lane-config"; * is a well-defined question. */ export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/pr-fix-queue/enqueue/route.ts b/src/app/api/pr-fix-queue/enqueue/route.ts index b25227d4..295cd115 100644 --- a/src/app/api/pr-fix-queue/enqueue/route.ts +++ b/src/app/api/pr-fix-queue/enqueue/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { enqueuePrFixItem, parseEnqueuePrFixInput } from "@/lib/pr-fix-queue"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; @@ -10,7 +10,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`pr-fix-enqueue:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/pr-fix-queue/history/route.ts b/src/app/api/pr-fix-queue/history/route.ts index bb174936..eef57342 100644 --- a/src/app/api/pr-fix-queue/history/route.ts +++ b/src/app/api/pr-fix-queue/history/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse, handleApiError } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma } from "@/lib/prisma"; const DEFAULT_LIMIT = 50; @@ -23,8 +23,9 @@ const MAX_LIMIT = 200; * its PR lives on, and that is a different answer from "no history". */ export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } const { searchParams } = new URL(request.url); diff --git a/src/app/api/pr-fix-queue/mark/route.test.ts b/src/app/api/pr-fix-queue/mark/route.test.ts index e7b6fdde..d0d78a39 100644 --- a/src/app/api/pr-fix-queue/mark/route.test.ts +++ b/src/app/api/pr-fix-queue/mark/route.test.ts @@ -1,9 +1,13 @@ import { describe, expect, it, vi, beforeEach } from "vitest"; import { TEST_AGENT_TOKEN as mockToken, makeDispatchEnvMockWithSafeEqual, authedRequest } from "@/test/route-helpers"; +const { WORKER_TOKEN } = vi.hoisted(() => ({ WORKER_TOKEN: "worker-token" })); + process.env.DISPATCH_AGENT_TOKEN = mockToken; -vi.mock("@/lib/dispatch-env", () => makeDispatchEnvMockWithSafeEqual()); +vi.mock("@/lib/dispatch-env", () => + makeDispatchEnvMockWithSafeEqual(mockToken, { [WORKER_TOKEN]: "worker" }), +); const { mocks } = vi.hoisted(() => ({ mocks: { @@ -232,3 +236,75 @@ describe("POST /api/pr-fix-queue/mark", () => { expect(mocks.isPrFixRepoArchived).not.toHaveBeenCalled(); }); }); + +describe("POST /api/pr-fix-queue/mark — worker tier (#1111)", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.prFixQueueClient.mockReturnValue({}); + mocks.auditLogCreate.mockResolvedValue({ id: "log-1" }); + mocks.isPrFixRepoArchived.mockResolvedValue(false); + mocks.markPrFixItem.mockResolvedValue({ mutated: true, item: { id: "fix-1", status: "FIXED" } }); + }); + + function workerPost(body: unknown) { + return POST( + authedRequest("http://localhost/api/pr-fix-queue/mark", { + method: "POST", + body, + token: WORKER_TOKEN, + headers: { "x-agent-name": "worker-agent" }, + }), + ); + } + + it("allows a worker to mark an item FIXED with a generation", async () => { + mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "FIXED", expectedGeneration: 2 }); + mocks.markPrFixItem.mockResolvedValue({ mutated: true, item: { id: "fix-1", status: "FIXED" } }); + + const res = await workerPost({ repo: "org/repo", pr: 42, status: "FIXED", generation: 2 }); + + expect(res.status).toBe(200); + expect(mocks.markPrFixItem).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ status: "FIXED", expectedGeneration: 2 }), + ); + }); + + it("returns 403 when a worker marks an item QUEUED", async () => { + mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "QUEUED", expectedGeneration: 2 }); + + const res = await workerPost({ repo: "org/repo", pr: 42, status: "QUEUED", generation: 2 }); + + expect(res.status).toBe(403); + expect((await res.json()).error).toBe("Marking an item QUEUED or IGNORED requires a maintainer token"); + expect(mocks.markPrFixItem).not.toHaveBeenCalled(); + expect(mocks.auditLogCreate).toHaveBeenCalledWith({ + data: expect.objectContaining({ + action: "pr_fix_mark", + success: false, + errorMessage: "Marking an item QUEUED or IGNORED requires a maintainer token", + }), + }); + }); + + it("returns 403 when a worker marks an item IGNORED", async () => { + mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "IGNORED", expectedGeneration: 2 }); + + const res = await workerPost({ repo: "org/repo", pr: 42, status: "IGNORED", generation: 2 }); + + expect(res.status).toBe(403); + expect((await res.json()).error).toBe("Marking an item QUEUED or IGNORED requires a maintainer token"); + expect(mocks.markPrFixItem).not.toHaveBeenCalled(); + }); + + it("allows a maintainer to mark an item QUEUED", async () => { + mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "QUEUED", expectedGeneration: 2 }); + mocks.isPrFixRepoArchived.mockResolvedValue(false); + mocks.markPrFixItem.mockResolvedValue({ mutated: true, item: { id: "fix-1", status: "QUEUED" } }); + + const res = await postRequest({ repo: "org/repo", pr: 42, status: "QUEUED", generation: 2 }); + + expect(res.status).toBe(200); + expect(mocks.markPrFixItem).toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/pr-fix-queue/mark/route.ts b/src/app/api/pr-fix-queue/mark/route.ts index c3b7ee28..895ab80a 100644 --- a/src/app/api/pr-fix-queue/mark/route.ts +++ b/src/app/api/pr-fix-queue/mark/route.ts @@ -2,7 +2,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { markPrFixItem, parseMarkPrFixInput, isPrFixRepoArchived } from "@/lib/pr-fix-queue"; -import { authorizeRequest, getAuthorizedActor } from "@/lib/auth"; +import { authorizeRequest, getAuthorizedActor, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; @@ -10,7 +10,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 }; export async function POST(request: Request) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`pr-fix-mark:${auth.actor}`, RATE_LIMIT); @@ -29,6 +29,29 @@ export async function POST(request: Request) { const input = parseMarkPrFixInput(body); if ("error" in input) return errorResponse(input.error, 400); + // Worker tokens may only settle attempts (FIXED/BLOCKED/STALE): moving + // an item back to QUEUED or IGNORED changes routing and requires a + // maintainer token (#1111). + if ((input.status === "QUEUED" || input.status === "IGNORED") && auth.type === "bearer" && auth.tier === "worker") { + try { + await prisma.auditLog.create({ + data: { + actor: auditActor, + action: "pr_fix_mark", + repoFullName: input.repo, + issueNumber: null, + success: false, + errorMessage: "Marking an item QUEUED or IGNORED requires a maintainer token", + beforeLabels: [], + afterLabels: [], + }, + }); + } catch { + // Audit log failure should not mask the 403 + } + return errorResponse("Marking an item QUEUED or IGNORED requires a maintainer token", 403); + } + // #1074: bearer (agent/bridge) marks must settle a specific attempt, so // the generation token is required. Operator paths (oidc session, basic, // disabled) keep the optional behavior for compatibility. diff --git a/src/app/api/pr-fix-queue/queued/route.ts b/src/app/api/pr-fix-queue/queued/route.ts index 72cb9fcc..2a32585f 100644 --- a/src/app/api/pr-fix-queue/queued/route.ts +++ b/src/app/api/pr-fix-queue/queued/route.ts @@ -3,11 +3,12 @@ import { errorResponse, handleApiError } from "@/lib/api-errors"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { listQueuedPrFixItems } from "@/lib/pr-fix-queue"; import { isValidPrFixLane, VALID_PR_FIX_LANES } from "@/types"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { diff --git a/src/app/api/pr-fix-queue/requeue/route.ts b/src/app/api/pr-fix-queue/requeue/route.ts index df90c4ca..34869ec2 100644 --- a/src/app/api/pr-fix-queue/requeue/route.ts +++ b/src/app/api/pr-fix-queue/requeue/route.ts @@ -1,7 +1,7 @@ import { NextRequest, NextResponse } from "next/server"; import { prisma } from "@/lib/prisma"; import { requeuePrFixItem, parseRequeuePrFixInput, isPrFixRepoArchived } from "@/lib/pr-fix-queue"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; const RATE_LIMIT = { limit: 30, windowMs: 10_000 } as const; @@ -9,7 +9,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 10_000 } as const; export async function POST(request: NextRequest) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); + return authErrorResponse(auth); } const limited = enforceRateLimit(`route:pr-fix-queue/requeue:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/pr-followup/sync/route.ts b/src/app/api/pr-followup/sync/route.ts index 52f4c2a4..a4761a82 100644 --- a/src/app/api/pr-followup/sync/route.ts +++ b/src/app/api/pr-followup/sync/route.ts @@ -2,7 +2,7 @@ import { NextRequest, NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { reconcileStalePrFixItems, reconcileArchivedRepoPrFixItems } from "@/lib/pr-fix-queue"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { getTrackedRepos } from "@/lib/config"; import { getGitHubToken, fetchPaginated, fetchPullRequests, fetchPullRequestMergeState, fetchFailedJobLogExcerpt, fetchClosedPullRequests, jobIdFromCheckRunUrl, type GithubPR as GithubPRBase } from "@/lib/github"; import { fetchPullRequestCommitMessages } from "@/lib/github"; @@ -85,7 +85,7 @@ interface GithubCheckRun { export async function POST(request: NextRequest) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`pr-followup-sync:${auth.actor}`, { limit: 10, windowMs: 60_000 }); diff --git a/src/app/api/pr-followup/webhook/route.ts b/src/app/api/pr-followup/webhook/route.ts index ba30e6c9..56e472b4 100644 --- a/src/app/api/pr-followup/webhook/route.ts +++ b/src/app/api/pr-followup/webhook/route.ts @@ -1,6 +1,6 @@ import { NextResponse } from "next/server"; import { errorResponse } from "@/lib/api-errors"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { prisma, asPrFixQueueClient } from "@/lib/prisma"; import { processPrFollowupEvents, extractLinkedIssue, PrFollowupEvent } from "@/lib/pr-followup-ingestion"; import { enforceRateLimit } from "@/lib/rate-limit"; @@ -213,7 +213,7 @@ export async function POST(request: Request) { if (sigMode !== "verify") { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } actor = auth.actor ?? "webhook"; } diff --git a/src/app/api/repos/route.ts b/src/app/api/repos/route.ts index d43a7a46..e7d6deba 100644 --- a/src/app/api/repos/route.ts +++ b/src/app/api/repos/route.ts @@ -4,11 +4,12 @@ import { Prisma } from "@prisma/client"; import { prisma } from "@/lib/prisma"; import { isValidRepoName } from "@/lib/config"; import { auditTrackedRepoCreateFailure, createTrackedRepo } from "@/lib/tracked-repos"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; export async function GET(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } try { const repos = await prisma.repository.findMany({ @@ -24,8 +25,9 @@ export async function GET(request: Request) { // Deprecated compatibility endpoint. Use POST /api/automation/repos for // tracked repository management. export async function POST(request: Request) { - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } let body: unknown; diff --git a/src/app/api/sync/route.ts b/src/app/api/sync/route.ts index cef598a0..62ed468e 100644 --- a/src/app/api/sync/route.ts +++ b/src/app/api/sync/route.ts @@ -5,7 +5,7 @@ import { syncStatusLabels } from "@/lib/github"; import { getSyncRepos, parseExcludedLabels } from "@/lib/config"; import { syncIssuesForRepos, makePrismaIssueStore, fetchAllStateIssues } from "@/lib/issue-sync"; import { runGroomingFreshnessPassBestEffort } from "@/lib/groomer/freshness-invalidation"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { enforceRateLimit } from "@/lib/rate-limit"; import { acquireLock, releaseLock } from "@/lib/sync-lock"; @@ -16,7 +16,7 @@ const RATE_LIMIT = { limit: 30, windowMs: 60_000 }; export async function POST(request: NextRequest) { const auth = await authorizeRequest(request); if (!auth.authorized) { - return errorResponse("Unauthorized", 401); + return authErrorResponse(auth); } const limited = enforceRateLimit(`sync:${auth.actor}`, RATE_LIMIT); diff --git a/src/app/api/sync/scheduled/route.ts b/src/app/api/sync/scheduled/route.ts index 367dd465..97c75cd3 100644 --- a/src/app/api/sync/scheduled/route.ts +++ b/src/app/api/sync/scheduled/route.ts @@ -13,7 +13,7 @@ import { ClosedIssueReconcileResponse, } from "@/lib/issue-sync"; import { syncAutomationRepo } from "@/lib/automation-sync"; -import { authorizeRequest } from "@/lib/auth"; +import { authorizeRequest, authErrorResponse } from "@/lib/auth"; import { acquireLock, releaseLock } from "@/lib/sync-lock"; import { runGroomingFreshnessPassBestEffort, type FreshnessPassResult } from "@/lib/groomer/freshness-invalidation"; @@ -23,8 +23,9 @@ import { runGroomingFreshnessPassBestEffort, type FreshnessPassResult } from "@/ export async function POST(request: Request) { // Auth check — require Bearer token matching DISPATCH_AGENT_TOKEN - if (!(await authorizeRequest(request)).authorized) { - return errorResponse("Unauthorized", 401); + const auth = await authorizeRequest(request); + if (!auth.authorized) { + return authErrorResponse(auth); } let body: unknown; diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 7ca0ed54..2124ffe5 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -7,6 +7,8 @@ import { isAuthorizedBasicAuth, authenticateRequest, authorizeRequest, + requiredTierForRoute, + authErrorResponse, resetAuthCaches, validateOidcConfig, } from "./auth"; @@ -14,6 +16,7 @@ import { const { mocks } = vi.hoisted(() => ({ mocks: { auth: vi.fn(), + auditCreate: vi.fn(), }, })); @@ -21,11 +24,24 @@ vi.mock("@/lib/auth-next", () => ({ auth: mocks.auth, })); +// The tier-denial audit row is written through a lazy prisma import; mock it +// so the denial path is testable without a database. +vi.mock("./prisma", () => ({ + prisma: { + auditLog: { + create: mocks.auditCreate, + }, + }, +})); + function clearAll() { delete process.env.DISPATCH_AUTH_MODE; delete process.env.DISPATCH_AUTH_USERNAME; delete process.env.DISPATCH_AUTH_PASSWORD; delete process.env.DISPATCH_AGENT_TOKEN; + delete process.env.DISPATCH_MAINTAINER_TOKEN; + delete process.env.DISPATCH_WORKER_TOKEN; + delete process.env.DISPATCH_GROOMER_TOKEN; } describe("getAuthMode", () => { @@ -250,7 +266,7 @@ describe("authenticateRequest (typed entry point)", () => { it('returns { authorized: true, type: "bearer" } in disabled mode', () => { process.env.DISPATCH_AUTH_MODE = "disabled"; const request = new Request("http://localhost/api/test"); - expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer" }); + expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer", tier: "maintainer" }); }); it('returns { authorized: true, type: "basic", username } for valid Basic Auth', () => { @@ -271,7 +287,7 @@ describe("authenticateRequest (typed entry point)", () => { const request = new Request("http://localhost/api/test", { headers: { Authorization: "Bearer agent-token" }, }); - expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer" }); + expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer", tier: "maintainer" }); }); it("returns { authorized: false } for invalid Basic Auth", () => { @@ -287,7 +303,7 @@ describe("authenticateRequest (typed entry point)", () => { const request = new Request("http://localhost/api/test", { headers: { Authorization: "Bearer agent-token" }, }); - expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer" }); + expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer", tier: "maintainer" }); }); it("returns { authorized: false } for invalid Bearer in legacy mode", () => { @@ -304,7 +320,7 @@ describe("authenticateRequest (typed entry point)", () => { const request = new Request("http://localhost/api/test", { headers: { Authorization: "Bearer agent-token" }, }); - expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer" }); + expect(authenticateRequest(request)).toEqual({ authorized: true, type: "bearer", tier: "maintainer" }); }); it("returns { authorized: false } for invalid Bearer in oidc mode", () => { @@ -334,6 +350,7 @@ describe("authorizeRequest (route helper)", () => { authorized: true, type: "disabled", actor: "operator", + tier: "maintainer", }); }); @@ -349,6 +366,7 @@ describe("authorizeRequest (route helper)", () => { type: "basic", username: "operator", actor: "operator", + tier: "maintainer", }); }); @@ -364,6 +382,7 @@ describe("authorizeRequest (route helper)", () => { authorized: true, type: "bearer", actor: "worker-1", + tier: "maintainer", }); }); @@ -375,6 +394,7 @@ describe("authorizeRequest (route helper)", () => { authorized: true, type: "oidc", actor: "operator@example.com", + tier: "maintainer", }); }); @@ -388,6 +408,7 @@ describe("authorizeRequest (route helper)", () => { authorized: true, type: "bearer", actor: "agent", + tier: "maintainer", }); expect(mocks.auth).not.toHaveBeenCalled(); }); @@ -421,3 +442,144 @@ describe("resetAuthCaches", () => { expect(isAuthorizedBearerToken("token2")).toBe(true); }); }); + +describe("bearer token tiers (#1111)", () => { + const WORKER_TOKEN = "worker-tier-token"; + + beforeEach(() => { + clearAll(); + resetAuthCaches(); + mocks.auth.mockReset(); + mocks.auditCreate.mockReset(); + process.env.DISPATCH_WORKER_TOKEN = WORKER_TOKEN; + }); + afterEach(() => { + clearAll(); + }); + + function workerRequest(pathname: string, method = "GET"): Request { + return new Request(`http://localhost${pathname}`, { + method, + headers: { Authorization: `Bearer ${WORKER_TOKEN}` }, + }); + } + + const workerAllowlistedRoutes: Array<[string, string]> = [ + ["GET", "/api/agents/saffron/next-task"], + ["POST", "/api/agents/saffron/tasks/report"], + ["POST", "/api/agents/saffron/heartbeat"], + ["GET", "/api/agents/saffron/active-work"], + ["GET", "/api/agents/saffron/queue"], + ["GET", "/api/agents/saffron/work-summary"], + ["GET", "/api/agent-work"], + ["POST", "/api/agent-work/start"], + ["POST", "/api/agent-work/checkpoint"], + ["POST", "/api/agent-work/finish"], + ["POST", "/api/issues/claim"], + ["POST", "/api/issues/unclaim"], + ["GET", "/api/issues/state"], + ["POST", "/api/issues/status"], + ["GET", "/api/issues"], + ["GET", "/api/pr-fix-queue/queued"], + ["GET", "/api/pr-fix-queue/history"], + ["POST", "/api/pr-fix-queue/mark"], + ]; + + for (const [method, pathname] of workerAllowlistedRoutes) { + it(`accepts a worker token on ${method} ${pathname}`, async () => { + await expect(authorizeRequest(workerRequest(pathname, method))).resolves.toMatchObject({ + authorized: true, + type: "bearer", + tier: "worker", + }); + }); + } + + const maintainerOnlyRoutes: Array<[string, string]> = [ + ["POST", "/api/agent-work"], + ["POST", "/api/agent-work/sweep"], + ["POST", "/api/pr-fix-queue/requeue"], + ["POST", "/api/sync"], + ["POST", "/api/issues/move"], + ["POST", "/api/issues/groom"], + ["POST", "/api/groomer/run"], + ["DELETE", "/api/automation/repos/foo/bar"], + ["POST", "/api/issues/unassign"], + ]; + + for (const [method, pathname] of maintainerOnlyRoutes) { + it(`forbids a worker token on ${method} ${pathname}`, async () => { + await expect(authorizeRequest(workerRequest(pathname, method))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + }); + } + + it("records an auth_tier_denied audit row on a tier denial", async () => { + await authorizeRequest(workerRequest("/api/sync", "POST")); + expect(mocks.auditCreate).toHaveBeenCalledTimes(1); + const call = mocks.auditCreate.mock.calls[0][0] as { data: Record }; + expect(call.data.action).toBe("auth_tier_denied"); + expect(call.data.success).toBe(false); + }); + + it("writes no audit row when the worker token is accepted", async () => { + await authorizeRequest(workerRequest("/api/issues/claim", "POST")); + expect(mocks.auditCreate).not.toHaveBeenCalled(); + }); + + it("authErrorResponse maps a tier denial to 403 naming the maintainer tier", async () => { + const res = authErrorResponse({ authorized: false, forbidden: true, requiredTier: "maintainer" }); + expect(res.status).toBe(403); + const body = (await res.json()) as { error: string }; + expect(body.error).toContain("maintainer"); + }); + + it("authErrorResponse maps a plain unauthorized result to 401", async () => { + const res = authErrorResponse({ authorized: false }); + expect(res.status).toBe(401); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Unauthorized"); + }); + + it("defaults unknown routes to maintainer for worker tokens", async () => { + await expect(authorizeRequest(workerRequest("/api/something/new", "PATCH"))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + }); + + it("does not match lookalike paths or wrong methods in the worker allowlist", () => { + expect(requiredTierForRoute("/api/agent-work-evil", "POST")).toBe("maintainer"); + expect(requiredTierForRoute("/api/agent-work/sweep", "POST")).toBe("maintainer"); + expect(requiredTierForRoute("/api/issues/claim", "GET")).toBe("maintainer"); + expect(requiredTierForRoute("/api/agents/saffron/next-task", "GET")).toBe("worker"); + expect(requiredTierForRoute("/api/issues/state", "GET")).toBe("worker"); + }); + + it("keeps DISPATCH_AGENT_TOKEN at maintainer rights", async () => { + delete process.env.DISPATCH_WORKER_TOKEN; + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + const request = new Request("http://localhost/api/pr-fix-queue/requeue", { + method: "POST", + headers: { Authorization: "Bearer agent-token" }, + }); + await expect(authorizeRequest(request)).resolves.toMatchObject({ + authorized: true, + type: "bearer", + tier: "maintainer", + }); + }); + + it("resolves non-bearer modes to maintainer even with only a worker token configured", async () => { + process.env.DISPATCH_AUTH_MODE = "disabled"; + const request = new Request("http://localhost/api/sync", { method: "POST" }); + await expect(authorizeRequest(request)).resolves.toMatchObject({ + authorized: true, + tier: "maintainer", + }); + }); +}); diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 0e9d4ac2..bbcabbb7 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -7,14 +7,35 @@ * - "disabled" : No auth enforcement (full open access) * * When DISPATCH_AUTH_MODE is not set, the legacy behavior is preserved: - * Bearer token auth via DISPATCH_AGENT_TOKEN is used for route-level checks. + * Bearer token auth is used for route-level checks. + * + * Bearer tokens carry a tier (see `getBearerTokenTier` in dispatch-env): + * - "maintainer" : DISPATCH_AGENT_TOKEN (and the optional + * DISPATCH_MAINTAINER_TOKEN alias) — full rights on every + * route. OIDC sessions, basic auth, and auth-disabled + * mode all resolve to the maintainer tier as well. + * - "worker" : DISPATCH_WORKER_TOKEN — restricted to the allowlist in + * `WORKER_ALLOWLIST` below; a worker token on any other + * route is rejected with 403 (`forbidden: true`) and a + * best-effort `auth_tier_denied` audit row. + * + * Tiers are enforced centrally from the single route→tier table in + * `requiredTierForRoute`, which defaults every route to "maintainer". * * All mutating routes should use `authorizeRequest(request)` instead of * duplicating auth parsing logic. The middleware protects operator UI routes; * route handlers authorize API access for browsers and agents. */ -import { isAuthorizedBearerToken as _isAuthed, resetCaches as _resetEnvCaches, safeEqual } from "./dispatch-env"; +import { NextResponse } from "next/server"; +import { errorResponse } from "./api-errors"; +import { + getBearerTokenTier, + isAuthorizedBearerToken as _isAuthed, + resetCaches as _resetEnvCaches, + safeEqual, + type TokenTier, +} from "./dispatch-env"; // --------------------------------------------------------------------------- // Auth mode resolution @@ -172,33 +193,88 @@ export function isAuthorizedBasicAuth(username: string, password: string): boole return safeEqual(creds.username, username) && safeEqual(creds.password, password); } +// --------------------------------------------------------------------------- +// Route → tier table (central tier enforcement) +// --------------------------------------------------------------------------- + +/** + * The single source of truth for which routes a worker-tier token + * (DISPATCH_WORKER_TOKEN) may call. Every route not listed here requires the + * maintainer tier, so a new route defaults to maintainer by construction. + * + * `method` is the HTTP method ("*" matches any method). The patterns match + * the request pathname exactly (no query string). + */ +export const WORKER_ALLOWLIST: ReadonlyArray<{ method: string | "*"; pattern: RegExp }> = [ + // Per-agent worker loop routes (name = one path segment) + { method: "GET", pattern: /^\/api\/agents\/[^/]+\/next-task$/ }, + { method: "POST", pattern: /^\/api\/agents\/[^/]+\/tasks\/report$/ }, + { method: "POST", pattern: /^\/api\/agents\/[^/]+\/heartbeat$/ }, + { method: "GET", pattern: /^\/api\/agents\/[^/]+\/active-work$/ }, + { method: "GET", pattern: /^\/api\/agents\/[^/]+\/queue$/ }, + { method: "GET", pattern: /^\/api\/agents\/[^/]+\/work-summary$/ }, + // Agent work lifecycle for the calling worker: read its listing and run its + // own work. The root POST action surface (release/reassign any agent's work) + // and POST /api/agent-work/sweep (stale-work recovery) stay maintainer-only. + { method: "GET", pattern: /^\/api\/agent-work$/ }, + { method: "POST", pattern: /^\/api\/agent-work\/(?:start|checkpoint|finish)$/ }, + // Issue state changes a worker performs on its own claimed work + { method: "POST", pattern: /^\/api\/issues\/claim$/ }, + { method: "POST", pattern: /^\/api\/issues\/unclaim$/ }, + { method: "GET", pattern: /^\/api\/issues\/state$/ }, + { method: "POST", pattern: /^\/api\/issues\/status$/ }, + { method: "GET", pattern: /^\/api\/issues$/ }, + // PR-fix queue reads + status marks + { method: "GET", pattern: /^\/api\/pr-fix-queue\/queued$/ }, + { method: "GET", pattern: /^\/api\/pr-fix-queue\/history$/ }, + { method: "POST", pattern: /^\/api\/pr-fix-queue\/mark$/ }, +]; + +/** + * Resolve the tier required to call a route. Defaults to "maintainer"; only + * returns "worker" when the (pathname, method) pair matches `WORKER_ALLOWLIST`. + * Maintainer-tier callers can use every route. + */ +export function requiredTierForRoute(pathname: string, method: string): TokenTier { + const normalizedMethod = method.toUpperCase(); + for (const entry of WORKER_ALLOWLIST) { + if (entry.method !== "*" && entry.method !== normalizedMethod) continue; + if (entry.pattern.test(pathname)) return "worker"; + } + return "maintainer"; +} + // --------------------------------------------------------------------------- // Unified authorization entry point // --------------------------------------------------------------------------- export type AuthorizedRequest = - | { authorized: true; type: "basic"; username: string; actor: string } - | { authorized: true; type: "bearer"; actor: string } - | { authorized: true; type: "oidc"; actor: string } - | { authorized: true; type: "disabled"; actor: string } - | { authorized: false }; + | { authorized: true; type: "basic"; username: string; actor: string; tier: "maintainer" } + | { authorized: true; type: "bearer"; actor: string; tier: TokenTier } + | { authorized: true; type: "oidc"; actor: string; tier: "maintainer" } + | { authorized: true; type: "disabled"; actor: string; tier: "maintainer" } + | { authorized: false } + | { authorized: false; forbidden: true; requiredTier: TokenTier }; /** * Check header-based auth (Bearer / Basic) and return the parsed auth info. */ export function authenticateRequest(request: Request): | { authorized: true; type: "basic"; username: string } - | { authorized: true; type: "bearer" } + | { authorized: true; type: "bearer"; tier: TokenTier } | { authorized: false } { const authMode = getAuthMode(); // Disabled mode — allow everything as bearer (no-op, just for type safety) - if (authMode === "disabled") return { authorized: true, type: "bearer" }; + if (authMode === "disabled") { + return { authorized: true, type: "bearer", tier: "maintainer" }; + } const parsed = parseAuthorizationHeader(request.headers.get("authorization")); - if (parsed?.type === "bearer" && isAuthorizedBearerToken(parsed.token)) { - return { authorized: true, type: "bearer" }; + if (parsed?.type === "bearer") { + const tier = getBearerTokenTier(parsed.token); + if (tier) return { authorized: true, type: "bearer", tier }; } // OIDC mode — route handlers must call authorizeRequest for session cookies @@ -225,26 +301,71 @@ function resolveSessionActor(user: { email?: string | null; name?: string | null return user?.email?.trim() || user?.name?.trim() || "operator"; } +/** + * Record a best-effort audit row when a worker-tier token is denied on a + * maintainer-tier route. The lazy prisma import keeps this module Edge-safe, + * and the try/catch guarantees a DB failure can never change the auth + * decision — the denial stands either way. + */ +async function recordTierDenialAudit( + request: Request, + pathname: string, + method: string, +): Promise { + try { + const { prisma } = await import("./prisma"); + await prisma.auditLog.create({ + data: { + actor: resolveBearerActor(request), + action: "auth_tier_denied", + repoFullName: "unknown", + success: false, + errorMessage: `worker tier denied ${method} ${pathname}; requires maintainer tier`, + beforeLabels: [], + afterLabels: [], + }, + }); + } catch { + // Best-effort audit only — never let a DB failure affect the auth result. + } +} + /** * Authorize a route handler request and return the authenticated actor. * * Accepts: - * - valid DISPATCH_AGENT_TOKEN Bearer auth in basic, oidc, and legacy modes - * - valid Basic Auth operator credentials in basic mode - * - valid NextAuth/OIDC session cookies in oidc mode + * - valid Bearer auth in basic, oidc, and legacy modes; the tier of the + * token (maintainer vs worker) is resolved and enforced against the + * route's required tier (`requiredTierForRoute`) + * - valid Basic Auth operator credentials in basic mode (maintainer tier) + * - valid NextAuth/OIDC session cookies in oidc mode (maintainer tier) + * + * A worker-tier token on a maintainer-tier route is rejected with + * `{ authorized: false, forbidden: true, requiredTier: "maintainer" }` + * and a best-effort `auth_tier_denied` audit row. */ export async function authorizeRequest(request: Request): Promise { const authMode = getAuthMode(); if (authMode === "disabled") { - return { authorized: true, type: "disabled", actor: "operator" }; + return { authorized: true, type: "disabled", actor: "operator", tier: "maintainer" }; } const headerAuth = authenticateRequest(request); if (headerAuth.authorized) { if (headerAuth.type === "basic") { - return { ...headerAuth, actor: headerAuth.username }; + return { ...headerAuth, actor: headerAuth.username, tier: "maintainer" }; + } + + if (headerAuth.type === "bearer" && headerAuth.tier === "worker") { + const { pathname } = new URL(request.url); + const required = requiredTierForRoute(pathname, request.method); + if (required === "maintainer") { + await recordTierDenialAudit(request, pathname, request.method); + return { authorized: false, forbidden: true, requiredTier: "maintainer" }; + } } + return { ...headerAuth, actor: resolveBearerActor(request) }; } @@ -252,7 +373,12 @@ export async function authorizeRequest(request: Request): Promise, +): NextResponse<{ error: string }> { + if ("forbidden" in auth && auth.forbidden) { + return errorResponse( + "Forbidden: this endpoint requires a maintainer token (DISPATCH_AGENT_TOKEN); worker tokens (DISPATCH_WORKER_TOKEN) are restricted", + 403, + ); + } + return errorResponse("Unauthorized", 401); +} + /** * Authorize a request for the hosted groomer route. * Accepts standard auth (agent token, basic, oidc) OR the dedicated groomer token. @@ -280,13 +425,20 @@ export async function authorizeGroomerRequest(request: Request): Promise { @@ -113,4 +115,137 @@ describe("isAuthorizedBearerToken", () => { const mod = await import("./dispatch-env"); expect(mod.isAuthorizedBearerToken("legacy-token")).toBe(false); }); + + it("returns true for a worker-tier token", async () => { + process.env.DISPATCH_AGENT_TOKEN = "maintainer-token"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + const mod = await import("./dispatch-env"); + expect(mod.isAuthorizedBearerToken("worker-token")).toBe(true); + }); +}); + +describe("getBearerTokenTier", () => { + beforeEach(() => { + clearAll(); + vi.resetModules(); + }); + afterEach(() => { clearAll(); }); + + it('resolves DISPATCH_WORKER_TOKEN to "worker"', async () => { + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("worker-token")).toBe("worker"); + }); + + it('resolves DISPATCH_AGENT_TOKEN to "maintainer"', async () => { + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("agent-token")).toBe("maintainer"); + }); + + it('resolves DISPATCH_MAINTAINER_TOKEN to "maintainer"', async () => { + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-alias"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("maintainer-alias")).toBe("maintainer"); + }); + + it("returns null for an unknown token", async () => { + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("unknown-token")).toBeNull(); + }); + + it("returns null for null/undefined/empty tokens", async () => { + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier(null)).toBeNull(); + expect(mod.getBearerTokenTier(undefined)).toBeNull(); + expect(mod.getBearerTokenTier("")).toBeNull(); + }); + + it("returns null when no tokens are configured", async () => { + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("any-token")).toBeNull(); + }); + + it("maintainer wins when the same value is configured for multiple tiers", async () => { + process.env.DISPATCH_AGENT_TOKEN = "shared-value"; + process.env.DISPATCH_WORKER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("shared-value")).toBe("maintainer"); + }); +}); + +describe("getAcceptedTokenTiers", () => { + beforeEach(() => { + clearAll(); + vi.resetModules(); + }); + afterEach(() => { clearAll(); }); + + it("returns the token→tier table for all configured tokens", async () => { + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-alias"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + const mod = await import("./dispatch-env"); + expect(mod.getAcceptedTokenTiers()).toEqual([ + { token: "agent-token", tier: "maintainer" }, + { token: "maintainer-alias", tier: "maintainer" }, + { token: "worker-token", tier: "worker" }, + ]); + }); + + it("returns an empty array when nothing is configured", async () => { + const mod = await import("./dispatch-env"); + expect(mod.getAcceptedTokenTiers()).toEqual([]); + }); +}); + +describe("getAcceptedAgentTokens (tier-derived)", () => { + beforeEach(() => { + clearAll(); + vi.resetModules(); + }); + afterEach(() => { clearAll(); }); + + it("includes every configured token (all tiers)", async () => { + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-alias"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + const mod = await import("./dispatch-env"); + expect(mod.getAcceptedAgentTokens()).toEqual([ + "agent-token", + "maintainer-alias", + "worker-token", + ]); + }); + + it("de-duplicates a value configured in multiple tiers", async () => { + process.env.DISPATCH_AGENT_TOKEN = "shared-value"; + process.env.DISPATCH_WORKER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); + expect(mod.getAcceptedAgentTokens()).toEqual(["shared-value"]); + }); +}); + +describe("token tier cache reset", () => { + beforeEach(() => { + clearAll(); + vi.resetModules(); + }); + afterEach(() => { clearAll(); }); + + it("picks up new env values after resetCaches", async () => { + process.env.DISPATCH_WORKER_TOKEN = "worker-1"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("worker-1")).toBe("worker"); + + mod.resetCaches(); + process.env.DISPATCH_WORKER_TOKEN = "worker-2"; + delete process.env.DISPATCH_AGENT_TOKEN; + expect(mod.getBearerTokenTier("worker-1")).toBeNull(); + expect(mod.getBearerTokenTier("worker-2")).toBe("worker"); + expect(mod.getAcceptedTokenTiers()).toEqual([{ token: "worker-2", tier: "worker" }]); + }); }); diff --git a/src/lib/dispatch-env.ts b/src/lib/dispatch-env.ts index 02b67eb5..7cbe3761 100644 --- a/src/lib/dispatch-env.ts +++ b/src/lib/dispatch-env.ts @@ -1,9 +1,16 @@ /** * Dispatch environment variable resolution. * - * Supported env vars: DISPATCH_URL, DISPATCH_AGENT_TOKEN, DISPATCH_AGENT_NAME, - * DISPATCH_AUTH_MODE, DISPATCH_AUTH_USERNAME, - * DISPATCH_AUTH_PASSWORD + * Supported env vars: DISPATCH_URL, DISPATCH_AGENT_TOKEN, + * DISPATCH_MAINTAINER_TOKEN, DISPATCH_WORKER_TOKEN, + * DISPATCH_AGENT_NAME, DISPATCH_AUTH_MODE, + * DISPATCH_AUTH_USERNAME, DISPATCH_AUTH_PASSWORD + * + * Bearer tokens carry a tier: + * - "maintainer" : DISPATCH_AGENT_TOKEN and the optional + * DISPATCH_MAINTAINER_TOKEN alias — full rights. + * - "worker" : DISPATCH_WORKER_TOKEN — restricted allowlist of routes + * (see `requiredTierForRoute` in src/lib/auth.ts). * * NOTE: This module is imported by src/middleware.ts, which runs in the Edge * runtime. It must therefore stay free of Node-only APIs (node:crypto, Buffer, @@ -78,36 +85,82 @@ export function getDispatchAgentName(): string | undefined { } // --------------------------------------------------------------------------- -// Accepted tokens (for server-side auth) +// Accepted tokens and tiers (for server-side auth) // --------------------------------------------------------------------------- -let _acceptedTokens: string[] | undefined; +/** + * Bearer token tiers: + * - "maintainer" : full rights (DISPATCH_AGENT_TOKEN, DISPATCH_MAINTAINER_TOKEN) + * - "worker" : restricted allowlist (DISPATCH_WORKER_TOKEN) + */ +export type TokenTier = "worker" | "maintainer"; + +let _tokenTiers: Array<{ token: string; tier: TokenTier }> | undefined; /** - * Return all configured agent tokens that should be accepted for inbound auth. + * Return the canonical token→tier table built from the environment: + * - DISPATCH_AGENT_TOKEN → "maintainer" + * - DISPATCH_MAINTAINER_TOKEN → "maintainer" + * - DISPATCH_WORKER_TOKEN → "worker" + * + * Entries are ordered maintainer-first, and the tier lookup + * (`getBearerTokenTier`) resolves duplicates with maintainer winning. */ -export function getAcceptedAgentTokens(): string[] { - if (_acceptedTokens !== undefined) return _acceptedTokens; +export function getAcceptedTokenTiers(): Array<{ token: string; tier: TokenTier }> { + if (_tokenTiers !== undefined) return _tokenTiers; - const tokens: string[] = []; - const token = process.env.DISPATCH_AGENT_TOKEN; - if (token) tokens.push(token); + const tiers: Array<{ token: string; tier: TokenTier }> = []; + const agentToken = process.env.DISPATCH_AGENT_TOKEN; + if (agentToken) tiers.push({ token: agentToken, tier: "maintainer" }); + + const maintainerToken = process.env.DISPATCH_MAINTAINER_TOKEN; + if (maintainerToken) tiers.push({ token: maintainerToken, tier: "maintainer" }); + + const workerToken = process.env.DISPATCH_WORKER_TOKEN; + if (workerToken) tiers.push({ token: workerToken, tier: "worker" }); - _acceptedTokens = tokens; - return _acceptedTokens; + _tokenTiers = tiers; + return _tokenTiers; } /** - * Check if a bearer token is authorized. Uses timing-safe comparison. + * Return all configured bearer tokens that should be accepted for inbound auth, + * derived from the token→tier table (de-duplicated). */ -export function isAuthorizedBearerToken(token: string | null | undefined): boolean { - if (!token) return false; - const accepted = getAcceptedAgentTokens(); +export function getAcceptedAgentTokens(): string[] { + const seen = new Set(); + const tokens: string[] = []; + for (const { token } of getAcceptedTokenTiers()) { + if (!seen.has(token)) { + seen.add(token); + tokens.push(token); + } + } + return tokens; +} - for (const acceptedToken of accepted) { - if (safeEqual(acceptedToken, token)) return true; +/** + * Resolve the tier of a bearer token using timing-safe comparison against each + * configured token. Returns null when the token matches no configured token. + * If the same value is configured for multiple tiers, "maintainer" wins. + */ +export function getBearerTokenTier(token: string | null | undefined): TokenTier | null { + if (!token) return null; + + let workerMatch = false; + for (const { token: configured, tier } of getAcceptedTokenTiers()) { + if (!safeEqual(configured, token)) continue; + if (tier === "maintainer") return "maintainer"; + workerMatch = true; } - return false; + return workerMatch ? "worker" : null; +} + +/** + * Check if a bearer token is authorized (any tier). Uses timing-safe comparison. + */ +export function isAuthorizedBearerToken(token: string | null | undefined): boolean { + return getBearerTokenTier(token) !== null; } /** @@ -136,5 +189,5 @@ export function resetCaches(): void { _cachedUrl = undefined; _cachedToken = undefined; _cachedAgentName = undefined; - _acceptedTokens = undefined; + _tokenTiers = undefined; } diff --git a/src/test/route-helpers.ts b/src/test/route-helpers.ts index 0e9fe281..90dd51e8 100644 --- a/src/test/route-helpers.ts +++ b/src/test/route-helpers.ts @@ -27,12 +27,24 @@ export const TEST_AGENT_TOKEN = "test-agent-token"; /** * Builds the mock module shape for `@/lib/dispatch-env`, matching the * common pattern of comparing an incoming token against a fixed test token. + * The test token resolves to the "maintainer" tier so existing suites keep + * exercising the full-rights path. Pass `tierMap` to accept extra tokens at + * a specific tier (e.g. a worker token for tier-gate tests). */ -export function makeDispatchEnvMock(token: string = TEST_AGENT_TOKEN) { +export function makeDispatchEnvMock( + token: string = TEST_AGENT_TOKEN, + tierMap: Record = {}, +) { + const accepted = [token, ...Object.keys(tierMap)]; return { - isAuthorizedAgentToken: vi.fn((t: string | null | undefined) => t === token), - isAuthorizedBearerToken: vi.fn((t: string | null | undefined) => t === token), - getAcceptedAgentTokens: vi.fn(() => [token]), + isAuthorizedAgentToken: vi.fn((t: string | null | undefined) => (t !== null && t !== undefined ? accepted.includes(t) : false)), + isAuthorizedBearerToken: vi.fn((t: string | null | undefined) => (t !== null && t !== undefined ? accepted.includes(t) : false)), + getAcceptedAgentTokens: vi.fn(() => accepted), + getBearerTokenTier: vi.fn((t: string | null | undefined) => { + if (t === null || t === undefined) return null; + if (t === token) return tierMap[token] ?? "maintainer"; + return tierMap[t] ?? null; + }), resetCaches: vi.fn(), }; } @@ -41,9 +53,12 @@ export function makeDispatchEnvMock(token: string = TEST_AGENT_TOKEN) { * Same as {@link makeDispatchEnvMock}, plus a `safeEqual` stub for routes * that use constant-time comparisons directly (e.g. webhook signature checks). */ -export function makeDispatchEnvMockWithSafeEqual(token: string = TEST_AGENT_TOKEN) { +export function makeDispatchEnvMockWithSafeEqual( + token: string = TEST_AGENT_TOKEN, + tierMap: Record = {}, +) { return { - ...makeDispatchEnvMock(token), + ...makeDispatchEnvMock(token, tierMap), safeEqual: vi.fn((a: string, b: string) => a === b), }; } From 2ba976e0b072b815baba0f87eb581cf835cb12b5 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 03:16:24 +0000 Subject: [PATCH 02/10] fix(auth): keep prisma out of client bundles by extracting auth-mode module The tier-denial audit's lazy import("./prisma") inside src/lib/auth.ts was statically traced into the browser bundle via the client-reachable auth-controls component (pg module-not-found, breaking the CI build). Extract getAuthMode into a dependency-free src/lib/auth-mode.ts and import it there from auth-controls; auth.ts re-exports it unchanged. --- src/components/auth-controls.test.tsx | 2 +- src/components/auth-controls.tsx | 2 +- src/lib/auth-mode.ts | 41 +++++++++++++++++++++++++++ src/lib/auth.ts | 34 ++++------------------ 4 files changed, 49 insertions(+), 30 deletions(-) create mode 100644 src/lib/auth-mode.ts diff --git a/src/components/auth-controls.test.tsx b/src/components/auth-controls.test.tsx index c607e1f5..aab00d70 100644 --- a/src/components/auth-controls.test.tsx +++ b/src/components/auth-controls.test.tsx @@ -4,7 +4,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { AuthControls } from "./auth-controls"; const getAuthModeMock = vi.fn(); -vi.mock("@/lib/auth", () => ({ +vi.mock("@/lib/auth-mode", () => ({ getAuthMode: () => getAuthModeMock(), })); diff --git a/src/components/auth-controls.tsx b/src/components/auth-controls.tsx index 2d092080..4ceb8589 100644 --- a/src/components/auth-controls.tsx +++ b/src/components/auth-controls.tsx @@ -1,4 +1,4 @@ -import { getAuthMode } from "@/lib/auth"; +import { getAuthMode } from "@/lib/auth-mode"; import { getSession } from "@/lib/session"; import { LogoutButton } from "./logout-button"; diff --git a/src/lib/auth-mode.ts b/src/lib/auth-mode.ts new file mode 100644 index 00000000..5283f5e6 --- /dev/null +++ b/src/lib/auth-mode.ts @@ -0,0 +1,41 @@ +/** + * Authentication mode resolution, isolated in a dependency-free module. + * + * This lives apart from src/lib/auth.ts so client-reachable components can + * read the auth mode without pulling the full auth module (which lazily + * imports Prisma for tier-denial audit rows) into the browser bundle. + */ + +let _cachedAuthMode: "basic" | "oidc" | "disabled" | undefined; + +/** + * Resolve the authentication mode. + * + * - "basic" : Require HTTP Basic Auth for all requests + * - "oidc" : OIDC session-based auth (enforced by NextAuth, not middleware) + * - "disabled" : No auth enforcement (open access) + * - undefined : Legacy mode — no middleware enforcement; routes use Bearer token checks + */ +export function getAuthMode(): "basic" | "oidc" | "disabled" | undefined { + if (_cachedAuthMode !== undefined) return _cachedAuthMode; + + const mode = process.env.DISPATCH_AUTH_MODE; + if (mode === "basic") { + _cachedAuthMode = "basic"; + } else if (mode === "oidc") { + _cachedAuthMode = "oidc"; + } else if (mode === "disabled") { + _cachedAuthMode = "disabled"; + } else { + _cachedAuthMode = undefined; + } + + return _cachedAuthMode; +} + +/** + * Reset the cached auth mode. Intended for test isolation. + */ +export function resetAuthModeCache(): void { + _cachedAuthMode = undefined; +} \ No newline at end of file diff --git a/src/lib/auth.ts b/src/lib/auth.ts index bbcabbb7..44125285 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -29,6 +29,7 @@ import { NextResponse } from "next/server"; import { errorResponse } from "./api-errors"; +import { getAuthMode, resetAuthModeCache } from "./auth-mode"; import { getBearerTokenTier, isAuthorizedBearerToken as _isAuthed, @@ -38,35 +39,12 @@ import { } from "./dispatch-env"; // --------------------------------------------------------------------------- -// Auth mode resolution +// Auth mode resolution (delegates to the client-safe auth-mode module; the +// full auth module lazily imports Prisma for tier-denial audits and must not +// be pulled into client bundles) // --------------------------------------------------------------------------- -let _cachedAuthMode: "basic" | "oidc" | "disabled" | undefined; - -/** - * Resolve the authentication mode. - * - * - "basic" : Require HTTP Basic Auth for all requests - * - "oidc" : OIDC session-based auth (enforced by NextAuth, not middleware) - * - "disabled" : No auth enforcement (open access) - * - undefined : Legacy mode — no middleware enforcement; routes use Bearer token checks - */ -export function getAuthMode(): "basic" | "oidc" | "disabled" | undefined { - if (_cachedAuthMode !== undefined) return _cachedAuthMode; - - const mode = process.env.DISPATCH_AUTH_MODE; - if (mode === "basic") { - _cachedAuthMode = "basic"; - } else if (mode === "oidc") { - _cachedAuthMode = "oidc"; - } else if (mode === "disabled") { - _cachedAuthMode = "disabled"; - } else { - _cachedAuthMode = undefined; - } - - return _cachedAuthMode; -} +export { getAuthMode }; // --------------------------------------------------------------------------- // OIDC config validation (fail-fast startup check) @@ -449,7 +427,7 @@ export async function authorizeGroomerRequest(request: Request): Promise Date: Mon, 28 Sep 2026 03:56:12 +0000 Subject: [PATCH 03/10] fix(auth): address AI review findings on tier enforcement - Throttle auth_tier_denied audit rows to one per actor/method/path per minute so a misconfigured worker cannot write-amplify the audit table (the 403 itself is never throttled) - Document GET /api/agent-work in the worker token-tier allowlist - Test authorizeGroomerRequest preserving the worker-tier forbidden result (403) and the groomer-token path still resolving to maintainer - Regression test: a truthy non-boolean force value cannot override another agent's claim on a worker token --- docs/worker-execution-contract.md | 2 +- src/app/api/issues/claim/route.test.ts | 9 ++++++ src/lib/auth.test.ts | 42 ++++++++++++++++++++++++++ src/lib/auth.ts | 14 +++++++-- 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/docs/worker-execution-contract.md b/docs/worker-execution-contract.md index 117fc2e7..acdc2caa 100644 --- a/docs/worker-execution-contract.md +++ b/docs/worker-execution-contract.md @@ -24,7 +24,7 @@ This document defines the generic execution contract for any agent worker consum Dispatch bearer tokens have two tiers. A **worker** token (`DISPATCH_WORKER_TOKEN`) may call exactly: - `GET /api/agents/{agentName}/next-task`, `POST /api/agents/{agentName}/tasks/report`, `POST /api/agents/{agentName}/heartbeat`, `GET /api/agents/{agentName}/active-work`, `GET /api/agents/{agentName}/queue`, `GET /api/agents/{agentName}/work-summary` -- `POST /api/agent-work/start`, `POST /api/agent-work/checkpoint`, `POST /api/agent-work/finish` +- `GET /api/agent-work`, `POST /api/agent-work/start`, `POST /api/agent-work/checkpoint`, `POST /api/agent-work/finish` - `POST /api/issues/claim` (without `force`) and `POST /api/issues/unclaim` (own claim only) - `GET /api/issues/state`, `POST /api/issues/status` - `GET /api/issues`, `GET /api/pr-fix-queue/queued`, `GET /api/pr-fix-queue/history` diff --git a/src/app/api/issues/claim/route.test.ts b/src/app/api/issues/claim/route.test.ts index b2820f06..bdb968db 100644 --- a/src/app/api/issues/claim/route.test.ts +++ b/src/app/api/issues/claim/route.test.ts @@ -504,4 +504,13 @@ describe("POST /api/issues/claim — worker tier (#1111)", () => { expect(res.status).toBe(200); expect((await res.json()).success).toBe(true); }); + + it("does not let a truthy non-boolean force override another agent", async () => { + mocks.getLiveIssueLabels.mockResolvedValueOnce(["agent/other-agent"]); + const res = await POST(workerRequest({ force: "true" })); + expect(res.status).toBe(409); + expect((await res.json()).error).toContain("Use force=true to override"); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 2124ffe5..5d0ea62a 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -11,7 +11,9 @@ import { authErrorResponse, resetAuthCaches, validateOidcConfig, + authorizeGroomerRequest, } from "./auth"; +import { resetRateLimits } from "./rate-limit"; const { mocks } = vi.hoisted(() => ({ mocks: { @@ -449,6 +451,7 @@ describe("bearer token tiers (#1111)", () => { beforeEach(() => { clearAll(); resetAuthCaches(); + resetRateLimits(); mocks.auth.mockReset(); mocks.auditCreate.mockReset(); process.env.DISPATCH_WORKER_TOKEN = WORKER_TOKEN; @@ -530,6 +533,45 @@ describe("bearer token tiers (#1111)", () => { expect(mocks.auditCreate).not.toHaveBeenCalled(); }); + it("throttles denial audit rows to one per actor/method/path per window", async () => { + await authorizeRequest(workerRequest("/api/sync", "POST")); + await authorizeRequest(workerRequest("/api/sync", "POST")); + await authorizeRequest(workerRequest("/api/sync", "POST")); + expect(mocks.auditCreate).toHaveBeenCalledTimes(1); + + // A different path is a different throttle key — still denied, still audited. + await authorizeRequest(workerRequest("/api/issues/move", "POST")); + expect(mocks.auditCreate).toHaveBeenCalledTimes(2); + }); + + it("authorizeGroomerRequest preserves the worker-tier forbidden result", async () => { + // No dedicated groomer token configured. + await expect(authorizeGroomerRequest(workerRequest("/api/groomer/run", "POST"))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + + // Groomer token configured but the caller presents the worker token. + process.env.DISPATCH_GROOMER_TOKEN = "groomer-token"; + await expect(authorizeGroomerRequest(workerRequest("/api/groomer/run", "POST"))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + + // The dedicated groomer token still authorizes at maintainer tier. + const groomerRequest = new Request("http://localhost/api/groomer/run", { + method: "POST", + headers: { Authorization: "Bearer groomer-token" }, + }); + await expect(authorizeGroomerRequest(groomerRequest)).resolves.toMatchObject({ + authorized: true, + type: "bearer", + tier: "maintainer", + }); + }); + it("authErrorResponse maps a tier denial to 403 naming the maintainer tier", async () => { const res = authErrorResponse({ authorized: false, forbidden: true, requiredTier: "maintainer" }); expect(res.status).toBe(403); diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 44125285..22aeaba8 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -281,15 +281,23 @@ function resolveSessionActor(user: { email?: string | null; name?: string | null /** * Record a best-effort audit row when a worker-tier token is denied on a - * maintainer-tier route. The lazy prisma import keeps this module Edge-safe, - * and the try/catch guarantees a DB failure can never change the auth - * decision — the denial stands either way. + * maintainer-tier route. The lazy prisma import keeps the (otherwise + * client-import-free) module graph lean, and the try/catch guarantees a DB + * failure can never change the auth decision — the denial stands either way. + * + * Rows are throttled to one per (actor, method, pathname) per minute so a + * misconfigured worker hammering maintainer routes cannot write-amplify the + * audit table; the 403 response itself is never throttled. */ async function recordTierDenialAudit( request: Request, pathname: string, method: string, ): Promise { + const { checkRateLimit } = await import("./rate-limit"); + const deniedKey = `auth_tier_denied:${resolveBearerActor(request)}:${method}:${pathname}`; + if (!checkRateLimit(deniedKey, { limit: 1, windowMs: 60_000 }).allowed) return; + try { const { prisma } = await import("./prisma"); await prisma.auditLog.create({ From fed0de23939a72445629282dca818b9a56b97ff5 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 04:29:33 +0000 Subject: [PATCH 04/10] fix(auth): harden tier-denial audit throttling per review - Wrap the whole recordTierDenialAudit body (limiter import, actor resolution, throttle checks) in the try/catch so nothing in the best-effort audit path can turn a 403 into a 500 - Add an overall per-actor ceiling (10/min) on top of the per actor/method/path bucket, so cycling dynamic maintainer paths cannot open a fresh bucket per path - Tests: denial stays forbidden on throttled requests, auditing re-opens after the window, the per-actor cap binds across distinct paths, and the audit actor is the x-agent-name value (never the bearer token) --- src/lib/auth.test.ts | 44 +++++++++++++++++++++++++++++++++++++------- src/lib/auth.ts | 23 ++++++++++++++--------- 2 files changed, 51 insertions(+), 16 deletions(-) diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 5d0ea62a..79ac8105 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -534,14 +534,44 @@ describe("bearer token tiers (#1111)", () => { }); it("throttles denial audit rows to one per actor/method/path per window", async () => { - await authorizeRequest(workerRequest("/api/sync", "POST")); - await authorizeRequest(workerRequest("/api/sync", "POST")); - await authorizeRequest(workerRequest("/api/sync", "POST")); + vi.useFakeTimers(); + try { + const forbidden = { authorized: false, forbidden: true, requiredTier: "maintainer" } as const; + await expect(authorizeRequest(workerRequest("/api/sync", "POST"))).resolves.toEqual(forbidden); + await expect(authorizeRequest(workerRequest("/api/sync", "POST"))).resolves.toEqual(forbidden); + await expect(authorizeRequest(workerRequest("/api/sync", "POST"))).resolves.toEqual(forbidden); + expect(mocks.auditCreate).toHaveBeenCalledTimes(1); + + // A different path is a different per-path key — still denied, still audited. + await expect(authorizeRequest(workerRequest("/api/issues/move", "POST"))).resolves.toEqual(forbidden); + expect(mocks.auditCreate).toHaveBeenCalledTimes(2); + + // Advancing past the window re-opens auditing for the original path. + vi.setSystemTime(Date.now() + 61_000); + await expect(authorizeRequest(workerRequest("/api/sync", "POST"))).resolves.toEqual(forbidden); + expect(mocks.auditCreate).toHaveBeenCalledTimes(3); + } finally { + vi.useRealTimers(); + } + }); + + it("caps per-actor denial audits even across distinct paths", async () => { + for (let i = 0; i < 14; i += 1) { + await authorizeRequest(workerRequest(`/api/issues/${i}/lane`, "POST")); + } + expect(mocks.auditCreate).toHaveBeenCalledTimes(10); + }); + + it("attributes denial audits to x-agent-name, never the bearer token", async () => { + const request = new Request("http://localhost/api/sync", { + method: "POST", + headers: { Authorization: `Bearer ${WORKER_TOKEN}`, "x-agent-name": "courier-1" }, + }); + await authorizeRequest(request); expect(mocks.auditCreate).toHaveBeenCalledTimes(1); - - // A different path is a different throttle key — still denied, still audited. - await authorizeRequest(workerRequest("/api/issues/move", "POST")); - expect(mocks.auditCreate).toHaveBeenCalledTimes(2); + const call = mocks.auditCreate.mock.calls[0][0] as { data: Record }; + expect(call.data.actor).toBe("courier-1"); + expect(JSON.stringify(call.data)).not.toContain(WORKER_TOKEN); }); it("authorizeGroomerRequest preserves the worker-tier forbidden result", async () => { diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 22aeaba8..2020fb7e 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -285,24 +285,28 @@ function resolveSessionActor(user: { email?: string | null; name?: string | null * client-import-free) module graph lean, and the try/catch guarantees a DB * failure can never change the auth decision — the denial stands either way. * - * Rows are throttled to one per (actor, method, pathname) per minute so a - * misconfigured worker hammering maintainer routes cannot write-amplify the - * audit table; the 403 response itself is never throttled. + * Rows are throttled two ways so a misconfigured worker cannot write-amplify + * the audit table: one row per (actor, method, pathname) per minute, with an + * overall per-actor ceiling per minute (dynamic maintainer paths like + * /api/issues/{id}/lane would otherwise each open a fresh bucket). The 403 + * response itself is never throttled — nothing here may change the auth + * decision, so every step sits inside a try/catch. */ async function recordTierDenialAudit( request: Request, pathname: string, method: string, ): Promise { - const { checkRateLimit } = await import("./rate-limit"); - const deniedKey = `auth_tier_denied:${resolveBearerActor(request)}:${method}:${pathname}`; - if (!checkRateLimit(deniedKey, { limit: 1, windowMs: 60_000 }).allowed) return; - try { + const actor = resolveBearerActor(request); + const { checkRateLimit } = await import("./rate-limit"); + if (!checkRateLimit(`auth_tier_denied:${actor}`, { limit: 10, windowMs: 60_000 }).allowed) return; + if (!checkRateLimit(`auth_tier_denied:${actor}:${method}:${pathname}`, { limit: 1, windowMs: 60_000 }).allowed) return; + const { prisma } = await import("./prisma"); await prisma.auditLog.create({ data: { - actor: resolveBearerActor(request), + actor, action: "auth_tier_denied", repoFullName: "unknown", success: false, @@ -312,7 +316,8 @@ async function recordTierDenialAudit( }, }); } catch { - // Best-effort audit only — never let a DB failure affect the auth result. + // Best-effort audit only — never let a limiter or DB failure affect the + // auth result. } } From 433924f8f32b4f574862397a147690b7ed00e061 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 14:53:47 +0000 Subject: [PATCH 05/10] fix(auth): accept tier bearer tokens in middleware; drop unclaim header guard Addresses review feedback on #1122: - middleware: isBearerAuthorized now accepts any configured tier token via isAuthorizedBearerToken, so worker and maintainer-alias bearer tokens reach route handlers in basic mode (tier stays enforced per route); adds basic-mode middleware tests per token type - issues/unclaim: remove the self-reported x-agent-name vs body agentName 403 guard (workers do not send the header; the assignment check remains the boundary); worker can release claims without the header, mismatched agentName yields 400 - docs/AGENTS/.env.example: unclaim documented as assignment-scoped; token->agent-name binding noted as follow-up --- .env.example | 2 +- AGENTS.md | 2 +- docs/worker-execution-contract.md | 4 +- src/app/api/issues/unclaim/route.test.ts | 26 +++----- src/app/api/issues/unclaim/route.ts | 24 -------- src/middleware.test.ts | 75 ++++++++++++++++++++++++ src/middleware.ts | 16 ++--- 7 files changed, 96 insertions(+), 53 deletions(-) diff --git a/.env.example b/.env.example index 49c1af7b..71bcb6b8 100644 --- a/.env.example +++ b/.env.example @@ -28,7 +28,7 @@ GITHUB_REPOSITORIES="myorg/myrepo1,myorg/myrepo2" # QUEUED/IGNORED marks, groomer, lanes, admission overrides, automation, # syncs); the worker tier is an explicit allowlist for autonomous executors # (next-task, tasks/report, heartbeat, active-work, queue, work-summary, -# agent-work start/checkpoint/finish, non-force claim and own-claim unclaim, +# agent-work start/checkpoint/finish, non-force claim and unclaim, # issue state/status, PR-fix queue reads and FIXED/BLOCKED/STALE marks). # A worker token calling a maintainer route gets a 403 naming the required # tier. See "Token Tiers" in docs/worker-execution-contract.md. diff --git a/AGENTS.md b/AGENTS.md index 0129f324..2620f6e7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ npm run db:deploy # Deploy migrations (prod) | `GITHUB_TOKEN` | Yes | GitHub Personal Access Token | | `DISPATCH_AGENT_TOKEN` | Yes | Bearer token for agent API (maintainer tier — full rights) | | `DISPATCH_MAINTAINER_TOKEN` | No | Optional alias for the maintainer-tier bearer token (same rights as `DISPATCH_AGENT_TOKEN`) | -| `DISPATCH_WORKER_TOKEN` | No | Worker-tier bearer token for autonomous executors (explicit allowlist: next-task, tasks/report, heartbeat, active-work, queue, work-summary, agent-work start/checkpoint/finish + GET listing, non-force claim / own-claim unclaim, issue state/status, PR-fix queue reads + FIXED/BLOCKED/STALE marks); agent-work operator release/reassign and sweep stay maintainer-only; other routes return 403 | +| `DISPATCH_WORKER_TOKEN` | No | Worker-tier bearer token for autonomous executors (explicit allowlist: next-task, tasks/report, heartbeat, active-work, queue, work-summary, agent-work start/checkpoint/finish + GET listing, non-force claim / unclaim (assignment-scoped), issue state/status, PR-fix queue reads + FIXED/BLOCKED/STALE marks); agent-work operator release/reassign and sweep stay maintainer-only; other routes return 403 | | `GITHUB_REPOSITORIES` | Yes | **One-time** bootstrap seed for tracked repos (comma or newline separated). Read only when `AutomationRepo` is empty. After first seed, manage via `/automation` UI or `POST /api/repos` / `POST /api/automation/repos`. Seeded repos carry `source: "env"`; UI-added repos carry `source: "user"`. | | `DISPATCH_URL` | No | Base URL of your Dispatch instance (used by outbound clients and MCP bridge) | | `DISPATCH_DATABASE_URL` | No | Alternative database URL alias — used if `DATABASE_URL` is not set | diff --git a/docs/worker-execution-contract.md b/docs/worker-execution-contract.md index acdc2caa..17e04571 100644 --- a/docs/worker-execution-contract.md +++ b/docs/worker-execution-contract.md @@ -25,12 +25,12 @@ Dispatch bearer tokens have two tiers. A **worker** token (`DISPATCH_WORKER_TOKE - `GET /api/agents/{agentName}/next-task`, `POST /api/agents/{agentName}/tasks/report`, `POST /api/agents/{agentName}/heartbeat`, `GET /api/agents/{agentName}/active-work`, `GET /api/agents/{agentName}/queue`, `GET /api/agents/{agentName}/work-summary` - `GET /api/agent-work`, `POST /api/agent-work/start`, `POST /api/agent-work/checkpoint`, `POST /api/agent-work/finish` -- `POST /api/issues/claim` (without `force`) and `POST /api/issues/unclaim` (own claim only) +- `POST /api/issues/claim` (without `force`) and `POST /api/issues/unclaim` (bounded by the assignment check — the issue must be assigned to the `agentName` in the request body) - `GET /api/issues/state`, `POST /api/issues/status` - `GET /api/issues`, `GET /api/pr-fix-queue/queued`, `GET /api/pr-fix-queue/history` - `POST /api/pr-fix-queue/mark` with `FIXED`, `BLOCKED`, or `STALE` (generation required, as today) -The **maintainer** token (`DISPATCH_AGENT_TOKEN`, or the `DISPATCH_MAINTAINER_TOKEN` alias) keeps full rights. A worker token calling a maintainer-only route gets an HTTP 403 naming the required tier: force claims, releasing another agent's claim, `QUEUED`/`IGNORED` marks, and `POST /api/pr-fix-queue/requeue` are maintainer-only. +The **maintainer** token (`DISPATCH_AGENT_TOKEN`, or the `DISPATCH_MAINTAINER_TOKEN` alias) keeps full rights. A worker token calling a maintainer-only route gets an HTTP 403 naming the required tier: force claims, `QUEUED`/`IGNORED` marks, and `POST /api/pr-fix-queue/requeue` are maintainer-only. Unclaim is not tier-gated — the target agent comes from the request body and is only bounded by the assignment check; cryptographic token→agent-name binding is a known follow-up. --- diff --git a/src/app/api/issues/unclaim/route.test.ts b/src/app/api/issues/unclaim/route.test.ts index d483be00..69879e66 100644 --- a/src/app/api/issues/unclaim/route.test.ts +++ b/src/app/api/issues/unclaim/route.test.ts @@ -551,35 +551,27 @@ describe("POST /api/issues/unclaim — worker tier (#1111)", () => { mocks.releaseAgentWorkByAgentAndIssue.mockResolvedValue(0); }); - function workerPost(xAgentName: string) { + function workerPost(payload = makePayload()) { return POST( authedRequest("http://localhost/api/issues/unclaim", { method: "POST", - body: makePayload(), + body: payload, token: WORKER_TOKEN, - headers: { "x-agent-name": xAgentName }, }), ); } - it("allows a worker to release its own claim (x-agent-name matches)", async () => { - const res = await workerPost("test-agent"); + it("allows a worker to release its own claim without an x-agent-name header", async () => { + const res = await workerPost(); expect(res.status).toBe(200); expect((await res.json()).success).toBe(true); expect(mocks.releaseLeaseByAgentAndIssue).toHaveBeenCalledWith("test-agent", "issue-1"); }); - it("returns 403 when a worker releases another agent's claim", async () => { - const res = await workerPost("other-agent"); - expect(res.status).toBe(403); - expect((await res.json()).error).toBe("Releasing another agent's claim requires a maintainer token"); + it("returns 400 when a worker's body agentName is not assigned to the issue", async () => { + const res = await workerPost(makePayload({ agentName: "other-agent" })); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("Issue is not assigned to other-agent"); expect(mocks.releaseLeaseByAgentAndIssue).not.toHaveBeenCalled(); - expect(mocks.createAuditLog).toHaveBeenCalledWith({ - data: expect.objectContaining({ - action: "unclaim_issue", - success: false, - errorMessage: "Releasing another agent's claim requires a maintainer token", - }), - }); }); -}); \ No newline at end of file +}); diff --git a/src/app/api/issues/unclaim/route.ts b/src/app/api/issues/unclaim/route.ts index 9476e5c0..4f654c5a 100644 --- a/src/app/api/issues/unclaim/route.ts +++ b/src/app/api/issues/unclaim/route.ts @@ -40,30 +40,6 @@ export async function POST(request: Request) { return errorResponse("Missing required fields: issueId, repoFullName, issueNumber, agentName", 400); } - // Worker tokens may only release their own claim: releasing another - // agent's claim requires a maintainer token (#1111). - const callerIdentity = request.headers.get("x-agent-name")?.trim(); - if (auth.type === "bearer" && auth.tier === "worker" && callerIdentity !== agentName) { - try { - await prisma.auditLog.create({ - data: { - actor: agentName as string, - action: "unclaim_issue", - repoFullName: repoFullName as string, - issueNumber: issueNumber as number, - issueId: issueId as string, - beforeLabels: [], - afterLabels: [], - success: false, - errorMessage: "Releasing another agent's claim requires a maintainer token", - }, - }); - } catch { - // Audit log failure should not mask the 403 - } - return errorResponse("Releasing another agent's claim requires a maintainer token", 403); - } - const agentLabel = `${AGENT_PREFIX}${agentName}` as const; const actor = getAuthorizedActor(auth, request, agentName as string); const isAgentSelfUnclaim = auth.type === "bearer" && actor === agentName; diff --git a/src/middleware.test.ts b/src/middleware.test.ts index 76fef56d..cf064e4a 100644 --- a/src/middleware.test.ts +++ b/src/middleware.test.ts @@ -18,6 +18,8 @@ function clearAll() { delete process.env.DISPATCH_AUTH_USERNAME; delete process.env.DISPATCH_AUTH_PASSWORD; delete process.env.DISPATCH_AGENT_TOKEN; + delete process.env.DISPATCH_MAINTAINER_TOKEN; + delete process.env.DISPATCH_WORKER_TOKEN; delete process.env.AUTH_URL; delete process.env.NEXTAUTH_URL; delete process.env.NEXTAUTH_SECRET; @@ -76,6 +78,79 @@ describe("middleware auth protection", () => { }); }); +describe("bearer tier tokens in basic mode", () => { + beforeEach(() => { + clearAll(); + resetAuthCaches(); + mocks.getToken.mockReset(); + }); + + afterEach(() => { + clearAll(); + resetAuthCaches(); + }); + + it("accepts a Bearer with the DISPATCH_AGENT_TOKEN (maintainer tier)", async () => { + process.env.DISPATCH_AUTH_MODE = "basic"; + process.env.DISPATCH_AUTH_USERNAME = "operator"; + process.env.DISPATCH_AUTH_PASSWORD = "s3cret"; + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-token"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + + const res = await middleware(makeRequest("/api/issues/claim", { + Authorization: "Bearer agent-token", + })); + + expect(res.status).toBe(200); + }); + + it("accepts a Bearer with the DISPATCH_WORKER_TOKEN (worker tier)", async () => { + process.env.DISPATCH_AUTH_MODE = "basic"; + process.env.DISPATCH_AUTH_USERNAME = "operator"; + process.env.DISPATCH_AUTH_PASSWORD = "s3cret"; + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-token"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + + const res = await middleware(makeRequest("/api/issues/claim", { + Authorization: "Bearer worker-token", + })); + + expect(res.status).toBe(200); + }); + + it("accepts a Bearer with the DISPATCH_MAINTAINER_TOKEN alias (maintainer tier)", async () => { + process.env.DISPATCH_AUTH_MODE = "basic"; + process.env.DISPATCH_AUTH_USERNAME = "operator"; + process.env.DISPATCH_AUTH_PASSWORD = "s3cret"; + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-token"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + + const res = await middleware(makeRequest("/api/issues/claim", { + Authorization: "Bearer maintainer-token", + })); + + expect(res.status).toBe(200); + }); + + it("rejects a Bearer with an unconfigured token", async () => { + process.env.DISPATCH_AUTH_MODE = "basic"; + process.env.DISPATCH_AUTH_USERNAME = "operator"; + process.env.DISPATCH_AUTH_PASSWORD = "s3cret"; + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_MAINTAINER_TOKEN = "maintainer-token"; + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + + const res = await middleware(makeRequest("/api/issues/claim", { + Authorization: "Bearer wrong-token", + })); + + expect(res.status).toBe(401); + }); +}); + describe("basic-auth rate limiting", () => { beforeEach(() => { clearAll(); diff --git a/src/middleware.ts b/src/middleware.ts index d5cdc0fb..e8369ffa 100644 --- a/src/middleware.ts +++ b/src/middleware.ts @@ -1,7 +1,7 @@ import { NextResponse } from "next/server"; import type { NextRequest } from "next/server"; import { getToken } from "next-auth/jwt"; -import { safeEqual } from "@/lib/dispatch-env"; +import { isAuthorizedBearerToken, safeEqual } from "@/lib/dispatch-env"; import { enforceRateLimit, resetRateLimitKey } from "@/lib/rate-limit"; type AuthMode = "basic" | "oidc" | "disabled" | undefined; @@ -121,11 +121,8 @@ function shouldUseSecureAuthCookie(request: NextRequest): boolean { function isBearerAuthorized(authHeader: string | null): boolean { - const token = process.env.DISPATCH_AGENT_TOKEN; - if (!token) return false; - const match = /^Bearer\s+(.+)$/i.exec(authHeader ?? ""); - return match ? safeEqual(match[1].trim(), token) : false; + return match ? isAuthorizedBearerToken(match[1].trim()) : false; } function parseBasicCredentials(authHeader: string | null): { username: string; password: string } | null { @@ -210,7 +207,8 @@ function isBasicAttempt(authHeader: string | null): boolean { * * Auth mode behavior: * - "basic" : HTTP Basic Auth required for UI routes. API routes also allow - * DISPATCH_AGENT_TOKEN Bearer auth for agents and workers. + * Bearer auth with any configured tier token for agents and + * workers; the middleware gates, the route handler enforces tier. * - "oidc" : OIDC session required for UI routes. API routes authorize via * route handlers so Bearer auth and session cookies both work. * - "disabled" : No auth enforcement at all. @@ -308,8 +306,10 @@ export async function middleware(request: NextRequest) { // a successful password authentication resets the rate-limit counter on any // route (UI or API), and so that a syntactically valid Basic attempt with // a wrong password is the only thing counted toward the lockout. Bearer is - // retained as the API-route credential (DISPATCH_AGENT_TOKEN) for agents - // and workers that do not speak Basic. + // retained as the API-route credential for agents and workers that do not + // speak Basic. The gate accepts any configured tier token + // (DISPATCH_AGENT_TOKEN, DISPATCH_MAINTAINER_TOKEN, DISPATCH_WORKER_TOKEN); + // enforcing the tier itself is the route handler's job. if (isBasicAuthorized(authHeader)) { resetRateLimitKey(`basic-auth:${clientIp(request)}`); const response = NextResponse.next(); From 97a06262490331a7641646219f33a749cabaf3f7 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 19:33:32 +0000 Subject: [PATCH 06/10] fix(auth): fail closed on cross-tier token collisions Resolve a bearer token value configured in both DISPATCH_WORKER_TOKEN and a maintainer token env var to the LOWER worker tier (fail-closed) instead of maintainer, warn once naming the colliding vars (never values), and surface the misconfiguration at boot via instrumentation. Trim env token values at table construction (closing the padded-value masking path), deny groomer-token grants that duplicate the worker token, document the semantics, and fix the missing trailing newline in auth-mode.ts. --- .env.example | 3 +- AGENTS.md | 2 +- docs/worker-execution-contract.md | 2 +- src/instrumentation.ts | 9 ++++ src/lib/auth-mode.ts | 2 +- src/lib/auth.test.ts | 39 ++++++++++++++ src/lib/auth.ts | 10 +++- src/lib/dispatch-env.test.ts | 90 ++++++++++++++++++++++++++++++- src/lib/dispatch-env.ts | 66 ++++++++++++++++++----- 9 files changed, 204 insertions(+), 19 deletions(-) diff --git a/.env.example b/.env.example index 71bcb6b8..f1e071a5 100644 --- a/.env.example +++ b/.env.example @@ -39,7 +39,8 @@ DISPATCH_AGENT_TOKEN="your_agent_token_here" # DISPATCH_AGENT_TOKEN). # DISPATCH_MAINTAINER_TOKEN="your_maintainer_token_here" # Worker-tier bearer token for autonomous executors (e.g. worker harnesses). -# Unset by default; use a different value from DISPATCH_AGENT_TOKEN. +# Unset by default; must be distinct from the maintainer tokens — a duplicate +# is treated as worker-tier only (fail-closed) and logs a warning at boot. # DISPATCH_WORKER_TOKEN="your_worker_token_here" # Operator / UI authentication (optional) diff --git a/AGENTS.md b/AGENTS.md index 2620f6e7..80ee4239 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ npm run db:deploy # Deploy migrations (prod) | `GITHUB_TOKEN` | Yes | GitHub Personal Access Token | | `DISPATCH_AGENT_TOKEN` | Yes | Bearer token for agent API (maintainer tier — full rights) | | `DISPATCH_MAINTAINER_TOKEN` | No | Optional alias for the maintainer-tier bearer token (same rights as `DISPATCH_AGENT_TOKEN`) | -| `DISPATCH_WORKER_TOKEN` | No | Worker-tier bearer token for autonomous executors (explicit allowlist: next-task, tasks/report, heartbeat, active-work, queue, work-summary, agent-work start/checkpoint/finish + GET listing, non-force claim / unclaim (assignment-scoped), issue state/status, PR-fix queue reads + FIXED/BLOCKED/STALE marks); agent-work operator release/reassign and sweep stay maintainer-only; other routes return 403 | +| `DISPATCH_WORKER_TOKEN` | No | Worker-tier bearer token for autonomous executors (explicit allowlist: next-task, tasks/report, heartbeat, active-work, queue, work-summary, agent-work start/checkpoint/finish + GET listing, non-force claim / unclaim (assignment-scoped), issue state/status, PR-fix queue reads + FIXED/BLOCKED/STALE marks); agent-work operator release/reassign and sweep stay maintainer-only; other routes return 403; a value duplicating a maintainer token resolves to worker tier (fail-closed) with a boot warning | | `GITHUB_REPOSITORIES` | Yes | **One-time** bootstrap seed for tracked repos (comma or newline separated). Read only when `AutomationRepo` is empty. After first seed, manage via `/automation` UI or `POST /api/repos` / `POST /api/automation/repos`. Seeded repos carry `source: "env"`; UI-added repos carry `source: "user"`. | | `DISPATCH_URL` | No | Base URL of your Dispatch instance (used by outbound clients and MCP bridge) | | `DISPATCH_DATABASE_URL` | No | Alternative database URL alias — used if `DATABASE_URL` is not set | diff --git a/docs/worker-execution-contract.md b/docs/worker-execution-contract.md index 17e04571..40842743 100644 --- a/docs/worker-execution-contract.md +++ b/docs/worker-execution-contract.md @@ -30,7 +30,7 @@ Dispatch bearer tokens have two tiers. A **worker** token (`DISPATCH_WORKER_TOKE - `GET /api/issues`, `GET /api/pr-fix-queue/queued`, `GET /api/pr-fix-queue/history` - `POST /api/pr-fix-queue/mark` with `FIXED`, `BLOCKED`, or `STALE` (generation required, as today) -The **maintainer** token (`DISPATCH_AGENT_TOKEN`, or the `DISPATCH_MAINTAINER_TOKEN` alias) keeps full rights. A worker token calling a maintainer-only route gets an HTTP 403 naming the required tier: force claims, `QUEUED`/`IGNORED` marks, and `POST /api/pr-fix-queue/requeue` are maintainer-only. Unclaim is not tier-gated — the target agent comes from the request body and is only bounded by the assignment check; cryptographic token→agent-name binding is a known follow-up. +The **maintainer** token (`DISPATCH_AGENT_TOKEN`, or the `DISPATCH_MAINTAINER_TOKEN` alias) keeps full rights. A worker token calling a maintainer-only route gets an HTTP 403 naming the required tier: force claims, `QUEUED`/`IGNORED` marks, and `POST /api/pr-fix-queue/requeue` are maintainer-only. Unclaim is not tier-gated — the target agent comes from the request body and is only bounded by the assignment check; cryptographic token→agent-name binding is a known follow-up. A `DISPATCH_WORKER_TOKEN` value that duplicates a maintainer token resolves to the lower worker tier (fail-closed) and logs a one-time boot warning — set it to a distinct value. --- diff --git a/src/instrumentation.ts b/src/instrumentation.ts index 7b2387c3..5adde8e3 100644 --- a/src/instrumentation.ts +++ b/src/instrumentation.ts @@ -16,6 +16,15 @@ export async function register() { // of as an opaque NextAuth error at first login. Node runtime only — the // edge runtime has no process to fail and the check is redundant there. if (process.env.NEXT_RUNTIME !== "nodejs") return; + + // Build the bearer token→tier table at boot so a cross-tier token + // misconfiguration (e.g. DISPATCH_WORKER_TOKEN duplicating a maintainer + // token) surfaces with its console warning at startup rather than on the + // first authenticated request. The warning fires once per module instance, + // so isolated chunk graphs (see the note above) may warn once per runtime. + const { getAcceptedTokenTiers } = await import("@/lib/dispatch-env"); + getAcceptedTokenTiers(); + if (process.env.DISPATCH_AUTH_MODE === "oidc") { const { validateOidcConfig } = await import("@/lib/auth"); validateOidcConfig(); diff --git a/src/lib/auth-mode.ts b/src/lib/auth-mode.ts index 5283f5e6..0b0ea5ec 100644 --- a/src/lib/auth-mode.ts +++ b/src/lib/auth-mode.ts @@ -38,4 +38,4 @@ export function getAuthMode(): "basic" | "oidc" | "disabled" | undefined { */ export function resetAuthModeCache(): void { _cachedAuthMode = undefined; -} \ No newline at end of file +} diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 79ac8105..b45af18b 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -602,6 +602,18 @@ describe("bearer token tiers (#1111)", () => { }); }); + it("does not escalate a groomer token that duplicates the worker token to maintainer", async () => { + // The groomer token value is also the configured worker token: the tier + // table resolves it to the lower worker tier, so the groomer route must + // stay forbidden (fail-closed) rather than grant maintainer. + process.env.DISPATCH_GROOMER_TOKEN = WORKER_TOKEN; + await expect(authorizeGroomerRequest(workerRequest("/api/groomer/run", "POST"))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + }); + it("authErrorResponse maps a tier denial to 403 naming the maintainer tier", async () => { const res = authErrorResponse({ authorized: false, forbidden: true, requiredTier: "maintainer" }); expect(res.status).toBe(403); @@ -646,6 +658,33 @@ describe("bearer token tiers (#1111)", () => { }); }); + it("fails closed on a cross-tier duplicate: a token in both tiers cannot authenticate as maintainer", async () => { + // Same value in the worker and maintainer env vars — must resolve to the + // LOWER worker tier, never escalate to maintainer. + process.env.DISPATCH_AGENT_TOKEN = WORKER_TOKEN; + const request = new Request("http://localhost/api/agent-work/sweep", { + method: "POST", + headers: { Authorization: `Bearer ${WORKER_TOKEN}` }, + }); + await expect(authorizeRequest(request)).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + // The tier denial was audited once (fresh throttle window per test). + expect(mocks.auditCreate).toHaveBeenCalledTimes(1); + + // The same token still works at worker tier on an allowlisted route. + const workerRouteRequest = new Request("http://localhost/api/agents/saffron/next-task", { + headers: { Authorization: `Bearer ${WORKER_TOKEN}` }, + }); + await expect(authorizeRequest(workerRouteRequest)).resolves.toMatchObject({ + authorized: true, + type: "bearer", + tier: "worker", + }); + }); + it("resolves non-bearer modes to maintainer even with only a worker token configured", async () => { process.env.DISPATCH_AUTH_MODE = "disabled"; const request = new Request("http://localhost/api/sync", { method: "POST" }); diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 2020fb7e..e71461bb 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -409,7 +409,11 @@ export function authErrorResponse( /** * Authorize a request for the hosted groomer route. - * Accepts standard auth (agent token, basic, oidc) OR the dedicated groomer token. + * Accepts standard auth (agent token, basic, oidc) OR the dedicated groomer + * token. The groomer token must be distinct from DISPATCH_WORKER_TOKEN: a + * groomer token value that duplicates the worker token is denied here — the + * tier table resolves it to the lower "worker" tier (fail-closed) — and the + * standard result (including a worker-tier 403 `forbidden`) is returned. */ export async function authorizeGroomerRequest(request: Request): Promise { const standard = await authorizeRequest(request); @@ -420,6 +424,10 @@ export async function authorizeGroomerRequest(request: Request): Promise { expect(mod.getBearerTokenTier("any-token")).toBeNull(); }); - it("maintainer wins when the same value is configured for multiple tiers", async () => { + it("trims env values at table construction: a padded worker env duplicates the agent env and warns", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + process.env.DISPATCH_AGENT_TOKEN = "shared"; + process.env.DISPATCH_WORKER_TOKEN = " shared "; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("shared")).toBe("worker"); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(String(warnSpy.mock.calls[0][0])).toContain("DISPATCH_AGENT_TOKEN"); + } finally { + warnSpy.mockRestore(); + } + }); + + it("treats a whitespace-only env value as unset", async () => { + process.env.DISPATCH_AGENT_TOKEN = "agent-token"; + process.env.DISPATCH_WORKER_TOKEN = " "; + const mod = await import("./dispatch-env"); + expect(mod.getAcceptedTokenTiers()).toEqual([{ token: "agent-token", tier: "maintainer" }]); + expect(mod.getBearerTokenTier(" ")).toBeNull(); + }); + + it("trims the presented token before comparing it to table entries", async () => { + process.env.DISPATCH_WORKER_TOKEN = "worker-token"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier(" worker-token ")).toBe("worker"); + }); + + it('resolves a value shared between DISPATCH_AGENT_TOKEN and DISPATCH_WORKER_TOKEN to "worker"', async () => { process.env.DISPATCH_AGENT_TOKEN = "shared-value"; process.env.DISPATCH_WORKER_TOKEN = "shared-value"; const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("shared-value")).toBe("worker"); + }); + + it('resolves a value shared between DISPATCH_MAINTAINER_TOKEN and DISPATCH_WORKER_TOKEN to "worker"', async () => { + process.env.DISPATCH_MAINTAINER_TOKEN = "shared-value"; + process.env.DISPATCH_WORKER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); + expect(mod.getBearerTokenTier("shared-value")).toBe("worker"); + }); + + it('keeps "maintainer" for a value shared only between the two maintainer aliases', async () => { + process.env.DISPATCH_AGENT_TOKEN = "shared-value"; + process.env.DISPATCH_MAINTAINER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); expect(mod.getBearerTokenTier("shared-value")).toBe("maintainer"); }); + + it("warns once (without token values) when the worker token duplicates a maintainer token", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + process.env.DISPATCH_AGENT_TOKEN = "shared-value"; + process.env.DISPATCH_WORKER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); + mod.getAcceptedTokenTiers(); + mod.getAcceptedTokenTiers(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(String(warnSpy.mock.calls[0][0])).not.toContain("shared-value"); + // The warning names the colliding env var, not just "a maintainer token". + expect(String(warnSpy.mock.calls[0][0])).toContain("DISPATCH_AGENT_TOKEN"); + } finally { + warnSpy.mockRestore(); + } + }); + + it("warns once (without token values) when the worker token duplicates DISPATCH_MAINTAINER_TOKEN", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + process.env.DISPATCH_MAINTAINER_TOKEN = "shared-value"; + process.env.DISPATCH_WORKER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); + mod.getAcceptedTokenTiers(); + mod.getAcceptedTokenTiers(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(String(warnSpy.mock.calls[0][0])).not.toContain("shared-value"); + expect(String(warnSpy.mock.calls[0][0])).toContain("DISPATCH_MAINTAINER_TOKEN"); + } finally { + warnSpy.mockRestore(); + } + }); + + it("does not warn when the two maintainer aliases share a value", async () => { + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + process.env.DISPATCH_AGENT_TOKEN = "shared-value"; + process.env.DISPATCH_MAINTAINER_TOKEN = "shared-value"; + const mod = await import("./dispatch-env"); + mod.getAcceptedTokenTiers(); + expect(warnSpy).not.toHaveBeenCalled(); + } finally { + warnSpy.mockRestore(); + } + }); }); describe("getAcceptedTokenTiers", () => { diff --git a/src/lib/dispatch-env.ts b/src/lib/dispatch-env.ts index 7cbe3761..dc2fc95c 100644 --- a/src/lib/dispatch-env.ts +++ b/src/lib/dispatch-env.ts @@ -98,26 +98,54 @@ export type TokenTier = "worker" | "maintainer"; let _tokenTiers: Array<{ token: string; tier: TokenTier }> | undefined; /** - * Return the canonical token→tier table built from the environment: + * Return the canonical token→tier table built from the environment (values + * are trimmed; empty-after-trim is treated as unset): * - DISPATCH_AGENT_TOKEN → "maintainer" * - DISPATCH_MAINTAINER_TOKEN → "maintainer" * - DISPATCH_WORKER_TOKEN → "worker" * - * Entries are ordered maintainer-first, and the tier lookup - * (`getBearerTokenTier`) resolves duplicates with maintainer winning. + * Entries are ordered maintainer-first. When the same value is configured in + * multiple tiers, the tier lookup (`getBearerTokenTier`) resolves it with the + * LOWER "worker" privilege winning (fail-closed). A one-time console warning + * is emitted here when the table is built in that misconfigured state; token + * values are never logged. */ export function getAcceptedTokenTiers(): Array<{ token: string; tier: TokenTier }> { if (_tokenTiers !== undefined) return _tokenTiers; const tiers: Array<{ token: string; tier: TokenTier }> = []; - const agentToken = process.env.DISPATCH_AGENT_TOKEN; + // Trim env values at table construction: presented bearer tokens are + // trimmed before comparison, so a whitespace-padded value is the same + // token, and an empty-after-trim value is treated as unset. + const agentToken = process.env.DISPATCH_AGENT_TOKEN?.trim(); if (agentToken) tiers.push({ token: agentToken, tier: "maintainer" }); - const maintainerToken = process.env.DISPATCH_MAINTAINER_TOKEN; + const maintainerToken = process.env.DISPATCH_MAINTAINER_TOKEN?.trim(); if (maintainerToken) tiers.push({ token: maintainerToken, tier: "maintainer" }); - const workerToken = process.env.DISPATCH_WORKER_TOKEN; - if (workerToken) tiers.push({ token: workerToken, tier: "worker" }); + const workerToken = process.env.DISPATCH_WORKER_TOKEN?.trim(); + if (workerToken) { + tiers.push({ token: workerToken, tier: "worker" }); + // Fail-closed misconfiguration check: a worker token value that is also + // configured as a maintainer token resolves to the LOWER worker tier. + // Surface that once (the table is cached, so this runs once per module + // instance) without ever logging a token value. A duplicate between the + // two maintainer aliases is fine and needs no warning. + const collidesWithAgent = agentToken !== undefined && safeEqual(workerToken, agentToken); + const collidesWithMaintainer = + maintainerToken !== undefined && safeEqual(workerToken, maintainerToken); + if (collidesWithAgent || collidesWithMaintainer) { + const collidingVars = [ + collidesWithAgent ? "DISPATCH_AGENT_TOKEN" : null, + collidesWithMaintainer ? "DISPATCH_MAINTAINER_TOKEN" : null, + ] + .filter((v): v is string => v !== null) + .join(" and "); + console.warn( + `Token tier misconfiguration: DISPATCH_WORKER_TOKEN has the same value as ${collidingVars}; it will be treated as worker-tier only. Set DISPATCH_WORKER_TOKEN to a distinct value.`, + ); + } + } _tokenTiers = tiers; return _tokenTiers; @@ -141,19 +169,31 @@ export function getAcceptedAgentTokens(): string[] { /** * Resolve the tier of a bearer token using timing-safe comparison against each - * configured token. Returns null when the token matches no configured token. - * If the same value is configured for multiple tiers, "maintainer" wins. + * configured token. The presented token is trimmed before comparison (env + * values are trimmed at table construction); a whitespace-only token resolves + * to null. Returns null when the token matches no configured token. + * If the same value is configured for multiple tiers, "worker" wins + * (fail-closed): an ambiguous cross-tier token never resolves to the higher + * "maintainer" privilege. The misconfiguration is surfaced by a one-time + * console warning when the token table is built (token values are never + * logged). */ export function getBearerTokenTier(token: string | null | undefined): TokenTier | null { if (!token) return null; + const trimmed = token.trim(); + if (!trimmed) return null; + let maintainerMatch = false; let workerMatch = false; for (const { token: configured, tier } of getAcceptedTokenTiers()) { - if (!safeEqual(configured, token)) continue; - if (tier === "maintainer") return "maintainer"; - workerMatch = true; + if (!safeEqual(configured, trimmed)) continue; + if (tier === "maintainer") maintainerMatch = true; + else workerMatch = true; } - return workerMatch ? "worker" : null; + // Worker is the lower privilege tier, so it wins any cross-tier duplicate. + if (workerMatch) return "worker"; + if (maintainerMatch) return "maintainer"; + return null; } /** From d244d6bc8fc3f9124a16cfd3fd1f20c27ec8d435 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 19:57:37 +0000 Subject: [PATCH 07/10] test(auth): lock worker-tier enforcement in basic and oidc modes Tier enforcement on the bearer path is mode-independent (the tier resolves before the mode branch), but it was only tested under legacy mode. Add regression tests pinning worker-token forbidden-on-maintainer and allowlisted-at-worker-tier in both basic and oidc modes, including an assertion that the OIDC session fallback never rescues a tier denial. --- src/lib/auth.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index b45af18b..886c40c3 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -693,4 +693,51 @@ describe("bearer token tiers (#1111)", () => { tier: "maintainer", }); }); + + it("enforces the worker allowlist in basic mode, not just legacy", async () => { + // basic mode: authenticateRequest parses the bearer and resolves its tier + // before the mode branch, so the tier table must apply identically here + // (regression lock — previously only legacy mode was covered). + process.env.DISPATCH_AUTH_MODE = "basic"; + process.env.DISPATCH_AUTH_USERNAME = "operator"; + process.env.DISPATCH_AUTH_PASSWORD = "s3cret"; + + // Maintainer-only route: worker token is forbidden, not silently rejected. + await expect(authorizeRequest(workerRequest("/api/sync", "POST"))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + + // Worker-allowlisted route: same token resolves at worker tier. + await expect(authorizeRequest(workerRequest("/api/agents/saffron/next-task"))).resolves.toMatchObject({ + authorized: true, + type: "bearer", + tier: "worker", + }); + }); + + it("enforces the worker allowlist in oidc mode, not just legacy", async () => { + // oidc mode: the bearer path is structurally identical to legacy — no + // OIDC env vars are needed for bearer resolution (validateOidcConfig is + // a startup-only check) and the NextAuth session lookup is only reached + // when header auth fails. The worker allowlist must still apply. + process.env.DISPATCH_AUTH_MODE = "oidc"; + + // Maintainer-only route: worker token is forbidden, and the OIDC session + // fallback must not rescue it. + await expect(authorizeRequest(workerRequest("/api/agent-work/sweep", "POST"))).resolves.toEqual({ + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }); + expect(mocks.auth).not.toHaveBeenCalled(); + + // Worker-allowlisted route: same token resolves at worker tier. + await expect(authorizeRequest(workerRequest("/api/agents/saffron/next-task"))).resolves.toMatchObject({ + authorized: true, + type: "bearer", + tier: "worker", + }); + }); }); From 3de0eb3759192162e0bbfc9bc2c8fc072d4c045a Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 20:17:32 +0000 Subject: [PATCH 08/10] test(auth): failure-inject the best-effort tier-denial audit path Pin the invariant that a rejecting audit write or a throwing rate-limiter never changes the tier-denial decision: worker denials still resolve to the forbidden result (403 via authErrorResponse) in both cases. Wraps the real ./rate-limit in a vi.fn with the real implementation as default so throttle tests keep using it. --- src/lib/auth.test.ts | 50 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 886c40c3..454e4664 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -19,6 +19,7 @@ const { mocks } = vi.hoisted(() => ({ mocks: { auth: vi.fn(), auditCreate: vi.fn(), + checkRateLimit: vi.fn(), }, })); @@ -36,6 +37,20 @@ vi.mock("./prisma", () => ({ }, })); +// The tier-denial audit path is throttled through ./rate-limit. Wrap the +// real limiter in a vi.fn so individual tests can inject one-shot failures +// (mocks.checkRateLimit.mockImplementationOnce). The real implementation +// stays the default behavior, which the throttling tests below depend on — +// so the block's beforeEach must NOT mockReset this one. +vi.mock("./rate-limit", async (importOriginal) => { + const actual = await importOriginal(); + mocks.checkRateLimit.mockImplementation(actual.checkRateLimit); + return { + ...actual, + checkRateLimit: mocks.checkRateLimit, + }; +}); + function clearAll() { delete process.env.DISPATCH_AUTH_MODE; delete process.env.DISPATCH_AUTH_USERNAME; @@ -528,6 +543,41 @@ describe("bearer token tiers (#1111)", () => { expect(call.data.success).toBe(false); }); + it("a failing audit write never changes the tier-denial decision", async () => { + // The audit write is best-effort and sits inside a try/catch: a DB + // failure must not turn the 403 denial into a thrown/500 outcome. + mocks.auditCreate.mockRejectedValueOnce(new Error("db down")); + const forbidden: { authorized: false; forbidden: true; requiredTier: "maintainer" } = { + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }; + const result = await authorizeRequest(workerRequest("/api/sync", "POST")); + expect(result).toEqual(forbidden); + // The write was attempted (then failed) and the result still maps to 403. + expect(mocks.auditCreate).toHaveBeenCalledTimes(1); + expect(authErrorResponse(forbidden).status).toBe(403); + }); + + it("a throwing rate-limiter never changes the tier-denial decision", async () => { + // One-shot override on the wrapped limiter: the next checkRateLimit call + // throws. Everything after reverts to the real limiter (the mock's + // default implementation), so no restoration is needed in beforeEach. + mocks.checkRateLimit.mockImplementationOnce(() => { + throw new Error("limiter down"); + }); + const forbidden: { authorized: false; forbidden: true; requiredTier: "maintainer" } = { + authorized: false, + forbidden: true, + requiredTier: "maintainer", + }; + const result = await authorizeRequest(workerRequest("/api/sync", "POST")); + expect(result).toEqual(forbidden); + // The throw short-circuits before the prisma import: no audit row. + expect(mocks.auditCreate).not.toHaveBeenCalled(); + expect(authErrorResponse(forbidden).status).toBe(403); + }); + it("writes no audit row when the worker token is accepted", async () => { await authorizeRequest(workerRequest("/api/issues/claim", "POST")); expect(mocks.auditCreate).not.toHaveBeenCalled(); From 4ad8a40d33354454fc2b8e8657b0dbdff15967b0 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 21:01:01 +0000 Subject: [PATCH 09/10] test(auth): reset-proof limiter mock and route-level denial audit tests Re-assert the stashed real checkRateLimit as the vi.fn default in the tier beforeEach so a future mockReset refactor cannot strip it, and pin the best-effort audit invariant at the route level: worker force-claim and QUEUED/IGNORED mark denials still return 403 with no mutation when the audit write rejects. --- src/app/api/issues/claim/route.test.ts | 15 +++++++++++++++ src/app/api/pr-fix-queue/mark/route.test.ts | 15 +++++++++++++++ src/lib/auth.test.ts | 21 +++++++++++++++++---- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/src/app/api/issues/claim/route.test.ts b/src/app/api/issues/claim/route.test.ts index bdb968db..bb82ef91 100644 --- a/src/app/api/issues/claim/route.test.ts +++ b/src/app/api/issues/claim/route.test.ts @@ -499,6 +499,21 @@ describe("POST /api/issues/claim — worker tier (#1111)", () => { }); }); + it("still returns 403 when the worker denial audit write fails", async () => { + // The 403 path's audit row is best-effort: a failing write must not + // mask the denial or leak into label/lease state. + mocks.createAuditLog.mockRejectedValueOnce(new Error("db down")); + const res = await POST(workerRequest({ force: true })); + expect(res.status).toBe(403); + expect((await res.json()).error).toBe("Force claim requires a maintainer token"); + // The write was attempted (then failed); nothing else was touched. + expect(mocks.createAuditLog).toHaveBeenCalledTimes(1); + expect(mocks.addIssueLabel).not.toHaveBeenCalled(); + expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); + expect(mocks.updateIssue).not.toHaveBeenCalled(); + expect(mocks.leaseCreate).not.toHaveBeenCalled(); + }); + it("allows a worker to claim without force", async () => { const res = await POST(workerRequest()); expect(res.status).toBe(200); diff --git a/src/app/api/pr-fix-queue/mark/route.test.ts b/src/app/api/pr-fix-queue/mark/route.test.ts index d0d78a39..45c4b40e 100644 --- a/src/app/api/pr-fix-queue/mark/route.test.ts +++ b/src/app/api/pr-fix-queue/mark/route.test.ts @@ -287,6 +287,21 @@ describe("POST /api/pr-fix-queue/mark — worker tier (#1111)", () => { }); }); + it("still returns 403 when the worker denial audit write fails", async () => { + mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "QUEUED", expectedGeneration: 2 }); + // The 403 path's audit row is best-effort: a failing write must not + // mask the denial or settle the queue item. + mocks.auditLogCreate.mockRejectedValueOnce(new Error("db down")); + + const res = await workerPost({ repo: "org/repo", pr: 42, status: "QUEUED", generation: 2 }); + + expect(res.status).toBe(403); + expect((await res.json()).error).toBe("Marking an item QUEUED or IGNORED requires a maintainer token"); + // The write was attempted (then failed); the queue item is untouched. + expect(mocks.auditLogCreate).toHaveBeenCalledTimes(1); + expect(mocks.markPrFixItem).not.toHaveBeenCalled(); + }); + it("returns 403 when a worker marks an item IGNORED", async () => { mocks.parseMarkPrFixInput.mockReturnValue({ repo: "org/repo", pr: 42, status: "IGNORED", expectedGeneration: 2 }); diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 454e4664..d0e11960 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -13,13 +13,16 @@ import { validateOidcConfig, authorizeGroomerRequest, } from "./auth"; -import { resetRateLimits } from "./rate-limit"; +import { resetRateLimits, type RateLimitOptions, type RateLimitResult } from "./rate-limit"; const { mocks } = vi.hoisted(() => ({ mocks: { auth: vi.fn(), auditCreate: vi.fn(), checkRateLimit: vi.fn(), + // Stashed by the vi.mock factory below so the tier describe's + // beforeEach can re-assert the real limiter as the mock's default. + realCheckRateLimit: null as unknown as (key: string, opts: RateLimitOptions) => RateLimitResult, }, })); @@ -39,11 +42,15 @@ vi.mock("./prisma", () => ({ // The tier-denial audit path is throttled through ./rate-limit. Wrap the // real limiter in a vi.fn so individual tests can inject one-shot failures -// (mocks.checkRateLimit.mockImplementationOnce). The real implementation -// stays the default behavior, which the throttling tests below depend on — -// so the block's beforeEach must NOT mockReset this one. +// (mocks.checkRateLimit.mockImplementationOnce). The real implementation is +// stashed on mocks.realCheckRateLimit here and re-asserted as the mock's +// default in the tier describe's beforeEach — a reset-proof pattern: a +// mockReset() in that beforeEach can never strip the default the +// throttling tests depend on, while a per-test mockImplementationOnce +// still wins (it is queued ahead of the default). vi.mock("./rate-limit", async (importOriginal) => { const actual = await importOriginal(); + mocks.realCheckRateLimit = actual.checkRateLimit; mocks.checkRateLimit.mockImplementation(actual.checkRateLimit); return { ...actual, @@ -469,6 +476,12 @@ describe("bearer token tiers (#1111)", () => { resetRateLimits(); mocks.auth.mockReset(); mocks.auditCreate.mockReset(); + // Reset-proofing: re-assert the real limiter (stashed on + // mocks.realCheckRateLimit by the vi.mock factory) as the mock's + // default implementation on every test, so a mockReset() here can + // never strip it. Per-test mockImplementationOnce overrides still + // win, as they are queued ahead of the default. + mocks.checkRateLimit.mockImplementation(mocks.realCheckRateLimit); process.env.DISPATCH_WORKER_TOKEN = WORKER_TOKEN; }); afterEach(() => { From a9c86e67f3d446066048a6b0193a21861fba5c02 Mon Sep 17 00:00:00 2001 From: Courier Date: Mon, 28 Sep 2026 21:24:59 +0000 Subject: [PATCH 10/10] test(auth): assert denial audit payloads and flush one-shot limiter overrides Pin the denial audit record contents (action, success, errorMessage, actor, target) in the claim and mark audit-failure tests, and reset the limiter mock before re-asserting the real implementation so an unconsumed mockImplementationOnce can never leak across tests. --- src/app/api/issues/claim/route.test.ts | 13 ++++++++++++- src/app/api/pr-fix-queue/mark/route.test.ts | 13 ++++++++++++- src/lib/auth.test.ts | 19 +++++++++++-------- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/app/api/issues/claim/route.test.ts b/src/app/api/issues/claim/route.test.ts index bb82ef91..df39d846 100644 --- a/src/app/api/issues/claim/route.test.ts +++ b/src/app/api/issues/claim/route.test.ts @@ -506,8 +506,19 @@ describe("POST /api/issues/claim — worker tier (#1111)", () => { const res = await POST(workerRequest({ force: true })); expect(res.status).toBe(403); expect((await res.json()).error).toBe("Force claim requires a maintainer token"); - // The write was attempted (then failed); nothing else was touched. + // The write was attempted (then failed) with the denial payload; + // nothing else was touched. expect(mocks.createAuditLog).toHaveBeenCalledTimes(1); + expect(mocks.createAuditLog).toHaveBeenCalledWith({ + data: expect.objectContaining({ + action: "claim_issue", + success: false, + errorMessage: "Force claim requires a maintainer token", + actor: "worker-agent", + repoFullName: "org/repo", + issueNumber: 42, + }), + }); expect(mocks.addIssueLabel).not.toHaveBeenCalled(); expect(mocks.removeIssueLabel).not.toHaveBeenCalled(); expect(mocks.updateIssue).not.toHaveBeenCalled(); diff --git a/src/app/api/pr-fix-queue/mark/route.test.ts b/src/app/api/pr-fix-queue/mark/route.test.ts index 45c4b40e..4ce8d498 100644 --- a/src/app/api/pr-fix-queue/mark/route.test.ts +++ b/src/app/api/pr-fix-queue/mark/route.test.ts @@ -297,8 +297,19 @@ describe("POST /api/pr-fix-queue/mark — worker tier (#1111)", () => { expect(res.status).toBe(403); expect((await res.json()).error).toBe("Marking an item QUEUED or IGNORED requires a maintainer token"); - // The write was attempted (then failed); the queue item is untouched. + // The write was attempted (then failed) with the denial payload; the + // queue item is untouched. expect(mocks.auditLogCreate).toHaveBeenCalledTimes(1); + expect(mocks.auditLogCreate).toHaveBeenCalledWith({ + data: expect.objectContaining({ + action: "pr_fix_mark", + success: false, + errorMessage: "Marking an item QUEUED or IGNORED requires a maintainer token", + actor: "worker-agent", + repoFullName: "org/repo", + issueNumber: null, + }), + }); expect(mocks.markPrFixItem).not.toHaveBeenCalled(); }); diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index d0e11960..094f26d7 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -44,10 +44,10 @@ vi.mock("./prisma", () => ({ // real limiter in a vi.fn so individual tests can inject one-shot failures // (mocks.checkRateLimit.mockImplementationOnce). The real implementation is // stashed on mocks.realCheckRateLimit here and re-asserted as the mock's -// default in the tier describe's beforeEach — a reset-proof pattern: a -// mockReset() in that beforeEach can never strip the default the -// throttling tests depend on, while a per-test mockImplementationOnce -// still wins (it is queued ahead of the default). +// default in the tier describe's beforeEach, immediately after a +// mockReset() (which also flushes any queued one-shots) — a reset-proof +// pattern, while a per-test mockImplementationOnce still wins (it is +// queued ahead of the default). vi.mock("./rate-limit", async (importOriginal) => { const actual = await importOriginal(); mocks.realCheckRateLimit = actual.checkRateLimit; @@ -476,11 +476,14 @@ describe("bearer token tiers (#1111)", () => { resetRateLimits(); mocks.auth.mockReset(); mocks.auditCreate.mockReset(); - // Reset-proofing: re-assert the real limiter (stashed on - // mocks.realCheckRateLimit by the vi.mock factory) as the mock's - // default implementation on every test, so a mockReset() here can - // never strip it. Per-test mockImplementationOnce overrides still + // Reset-proofing: mockReset() clears both the implementation and any + // queued mockImplementationOnce entries (so a one-shot left over from an + // earlier test cannot leak into this one), which is why the real limiter + // (stashed on mocks.realCheckRateLimit by the vi.mock factory) is + // re-asserted as the mock's default immediately after. Per-test + // mockImplementationOnce overrides (set in the test, after this) still // win, as they are queued ahead of the default. + mocks.checkRateLimit.mockReset(); mocks.checkRateLimit.mockImplementation(mocks.realCheckRateLimit); process.env.DISPATCH_WORKER_TOKEN = WORKER_TOKEN; });