Skip to content

fix(scan): precise 403/404 access codes + already-running guard on POST /api/scan#85

Merged
LanNguyenSi merged 1 commit into
masterfrom
fix/scan-endpoint-hardening
Jun 26, 2026
Merged

fix(scan): precise 403/404 access codes + already-running guard on POST /api/scan#85
LanNguyenSi merged 1 commit into
masterfrom
fix/scan-endpoint-hardening

Conversation

@LanNguyenSi

Copy link
Copy Markdown
Owner

What

Two backend fixes to POST /api/scan + scanRepository.

Why

Surfaced by the reviewer of the depsight_rescan MCP tool. The route turned every access failure into a 500, and scanRepository created a new RUNNING scan on every call with no debounce, so a loop (web Rescan button or depsight_rescan) could spawn many concurrent synchronous scans and burn the user's GitHub API quota.

How

  • Precise access codes: scanRepository throws a typed ScanAccessError(status): not-found 404, not-owned 403, owned-but-untracked 404. The POST handler maps it before the generic 500 fallback, so a genuine scan crash still returns 500.
  • Already-running guard: before creating a RUNNING scan, short-circuit and return the in-flight scan (scanId, alreadyRunning:true) when one already exists for the repo within SCAN_RUNNING_WINDOW_MS (default 5 min, positive-finite override only). The success response surfaces status running vs completed.

Tests

  • Scanner unit tests: 404/403/404 access cases, already-running guard (asserts scan.create NOT called and pins the guard query shape), and a negative control (fresh scan proceeds).
  • Route tests: 404/403/500 mapping, 200 completed vs running, 401/400 guards.
  • tsc, eslint, vitest (270) green. Route tests live in a separate file because vi.mock is file-scoped.

Notes / follow-up

  • The guard is a debounce, not a hard mutex (findFirst-then-create is non-atomic; no DB unique constraint). Acceptable for the stated goal; a partial unique index would be needed for true concurrency safety.
  • 403 for a not-owned repo reveals its existence to a non-owner, an intended task-requested tradeoff; repo IDs are non-enumerable cuids.
  • Downstream callers reacting to the new alreadyRunning contract (export route 409 edge, depsight_rescan message) tracked in follow-up f62a0fbf.

Refs: agent-tasks 5e9a27bc

…cate running scans

scanRepository now distinguishes not-found (404), not-owned (403), and
not-tracked (404) via a typed ScanAccessError instead of a generic Error that
the route turned into a 500, so callers (web Rescan button, depsight_rescan MCP)
can tell a permission problem from a scan crash. It also short-circuits when a
RUNNING scan already exists for the repo within SCAN_RUNNING_WINDOW_MS (default
5 min), returning the in-flight scan instead of spawning a duplicate that burns
GitHub API quota. Genuine scan failures still return 500.

Adds scanner + route unit tests (12) covering each status code, the guard, and
a negative control.

Refs: agent-tasks 5e9a27bc

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LanNguyenSi LanNguyenSi merged commit 27c761f into master Jun 26, 2026
4 checks passed
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.

2 participants