Skip to content

chore(auth): require auth on the previously-open GET routes#537

Merged
joryirving merged 2 commits into
mainfrom
chore/guard-open-get-routes
Jul 2, 2026
Merged

chore(auth): require auth on the previously-open GET routes#537
joryirving merged 2 commits into
mainfrom
chore/guard-open-get-routes

Conversation

@joryirving

Copy link
Copy Markdown
Contributor

Summary

  • Guard the GET handlers for repos, automation/repos, automation/sync, agent-runs, and audit with authorizeRequest — they previously allowed unauthenticated reads of repo lists, sync history, and audit logs.

Why it's safe for the dashboard (validated)

  • board / agents pages are server components reading Prisma directly — they don't HTTP-fetch these routes, so guarding them has zero UI impact.
  • The automation page fetches automation/{repos,sync} via authedFetch, which attaches Authorization: Basic … (and the oidc session cookie rides along). authorizeRequest accepts basic/bearer/oidc-session, so the logged-in dashboard passes; only anonymous callers get 401.

Verification

  • Added a 401-when-unauthenticated test to each route with existing GET tests (repos, audit, agent-runs); existing GET tests now send auth. automation/{repos,sync} had no GET tests (POST-only, already authed).
  • tsc → 0; eslint → clean; the five route suites → 51 passing.

Closes #509 · completes the audit umbrella #498

The GET handlers for repos, automation/repos, automation/sync, agent-runs, and
audit allowed unauthenticated reads (repo lists, sync history, audit logs).
Guard them with authorizeRequest, matching every other route.

Safe for the dashboard: the board/agents pages server-render via Prisma (they
don't fetch these), and the automation page fetches automation/{repos,sync} via
authedFetch (sends Authorization), which authorizeRequest accepts (basic/bearer/
oidc-session). Only anonymous callers are now rejected.

Closes #509

@its-saffron its-saffron Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Automated Review

Full PR review.

Analysis engine: MiniMax-M2.7@https://litellm.jory.dev/v1 (anthropic) — routed smart (risk match: public_route_changes)

PR Review: chore(auth): require auth on the previously-open GET routes

Recommendation

Approve. This PR correctly addresses the security issue in PR 509 by adding authorizeRequest authentication guards to all five previously-open GET routes. The implementation is consistent, well-tested, and follows existing patterns in the codebase. CI passed (typecheck, docker build, tests, lint, build).


Change-by-Change Findings

1. src/app/api/repos/route.ts

  • Added authorizeRequest check at the top of the GET handler
  • Returns 401 with { "error": "Unauthorized" } when not authorized
  • Status: ✅ Correct — matches the pattern used elsewhere in the codebase (see agent-work/checkpoint/route.ts:55, agents/[agentName]/heartbeat/route.ts:38, etc.)

2. src/app/api/agent-runs/route.ts

  • Added authorizeRequest check at the top of the GET handler
  • Status: ✅ Correct

3. src/app/api/audit/route.ts

  • Added authorizeRequest check at the top of the GET handler
  • Status: ✅ Correct
  • The diff also shows removal of the "intentionally unauthenticated" NOTE comment from the test file, which is appropriate since the policy is now to require auth

4. src/app/api/automation/repos/route.ts

  • Added authorizeRequest check; also added request: Request parameter (was missing before)
  • Status: ✅ Correct — adds proper typing while guarding the route

5. src/app/api/automation/sync/route.ts

  • Added authorizeRequest check; also added request: Request parameter (was missing before)
  • Status: ✅ Correct

Test Changes

  • route.test.ts files updated to:
    • Add explicit it("401s an unauthenticated request", ...) test for each route
    • Add Authorization: Bearer header to existing GET tests that now require auth
    • Remove outdated "intentionally unauthenticated" NOTE comments
  • Status: ✅ Tests verify the new behavior correctly

Required Checks

✅ verify route access controls are in place

All five routes now have authorizeRequest guards at the entry point of their GET handlers:

  • GET /api/repos
  • GET /api/agent-runs
  • GET /api/audit
  • GET /api/automation/repos
  • GET /api/automation/sync

The guards consistently return 401 with { "error": "Unauthorized" }, matching the pattern established by other secured routes in the repository (e.g., agent-work/checkpoint/route.ts:55, agents/[agentName]/heartbeat/route.ts:38).

✅ check for unintended public endpoints

No new public endpoints were introduced. The PR only adds auth guards to existing routes. The POST handlers on these routes were already protected (the PR body notes that automation/{repos,sync} were POST-only and already authed).


Standards Compliance

  • API routes return appropriate HTTP status codes: ✅ Uses 401 for unauthorized, consistent with existing auth patterns
  • Error handling uses error instanceof Error pattern: ✅ Not applicable here (no error paths modified)
  • No commit of secrets: ✅ No secrets added

Linked Issue Fit

Issue PR 509 required guarding GETs on repos, automation/repos, automation/sync, agent-runs, and audit with authorizeRequest. This PR fulfills that requirement exactly:

Route Before After Status
GET /api/repos Unauthenticated 401 when unauthenticated ✅ Fixed
GET /api/automation/repos Unauthenticated 401 when unauthenticated ✅ Fixed
GET /api/automation/sync Unauthenticated 401 when unauthenticated ✅ Fixed
GET /api/agent-runs Unauthenticated 401 when unauthenticated ✅ Fixed
GET /api/audit Unauthenticated 401 when unauthenticated ✅ Fixed

Evidence Provider Findings

No evidence providers configured; not applicable.


Tool Harness Findings

The tool-calling loop issued no tool calls; this review is based on direct corpus analysis.


Unknowns / Needs Verification

None. The diff is self-contained, CI passed, and the implementation pattern is verified against existing auth-protected routes in the repository.

@joryirving joryirving enabled auto-merge (squash) July 2, 2026 02:49
@joryirving joryirving merged commit 89f5f35 into main Jul 2, 2026
6 checks passed
@joryirving joryirving deleted the chore/guard-open-get-routes branch July 2, 2026 02:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore(auth): make read auth uniform on open-GET routes

1 participant