test(scope): guard the /api/v1 GET surface against unscoped routes (#1178)

[Sriram-PR]

Refs #1178.

Fixing #1166/#1167 took three passes because the /api/v1 GET surface was enumerated by hand and each pass turned up another door that wasn't scope-gated. This adds a test that walks the production chi router instead of trusting a hand-kept list.

What it does:

• walks s.Router() and collects every GET under /api/v1
• requires each route to be classified as refused, filtered, or open, with a written reason next to it
• a new GET route added without a classification fails the test, so a door can't ship unclassified
• drives each route with a real read-only agent token scoped to one server and checks the verdict holds: refused routes answer the fixed 403 (and carry the scope-layer denial body, so an unrelated 403 doesn't count), filtered and open routes stay reachable
• floors the route count so a walk that matches nothing (say after a router refactor moves the prefix) can't pass silently

There's already a similar walk for the server edition (tenant_allowlist_walk_test.go, //go:build server) covering session principals. This is the personal-edition agent-token counterpart, which is the axis #1166/#1167 were about and didn't have a guard.

Also adds a Doctor method to the shared scope-test management mock so /diagnostics runs through the same path production uses, plus TestDiagnostics_ScopedCallerDoesNotSeeHiddenServer backing that route's classification.

Verification:

• go test -race ./internal/httpapi/
• golangci-lint run --config .github/.golangci.yml ./internal/httpapi/ (0 issues)

[Sriram-PR]

Good catches, both. You're right that "not 403" was doing almost nothing for the rows that never reach the handler, I confirmed the same 11: the scan subroutes and the /{id} detail routes 404 on the synthetic id before canSeeServer, connect 503s (not wired in the fixture), and index/search 400s with no query.

Reworked it: routes that actually reach the handler now assert 2xx, and the pre-gate ones are in an explicit shortCircuitCodes map with a reason each, so a route that starts reaching the handler trips it. Also fixed the routeParamSubstitutions comment, which was just wrong about the scan routes.

On diagnostics, yes, the old test only exercised the mock's own filter. I swapped it to run the httpapi handler with withManagement:false so it drives the real visibleServers path in the legacy branch, which had no coverage before. Removing that call now fails the test. For the management path I'm citing TestDoctor_ScopedTokenSeesOnlyAllowedServers in the map instead of pretending this file covers it.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Sriram-PR]

Thanks for the reviews and for resolving the threads, @Dumbris. I think everything's addressed on my end and CI's green; anything else you'd like changed, or is this good to merge?

[Dumbris]

Thanks a lot @Sriram-PR, this is a great guard, and it proved itself right away 🙌

Since you branched, main gained four new /api/v1 GET routes, and TestScopeRouteTableGuard failed on the merge commit because none of them were classified. That's exactly the job this test was written for. I merged main into your branch and pushed one small follow-up (05a7b84):

• /api/v1/attention → scopeFiltered (same rule as /servers; TestHandleGetAttention_BranchesOnScopedCaller)
• /api/v1/review → scopeFiltered (canSeeServer per row, then a recount; TestReviewQueueFiltersAgentAndUserScopesAndRecounts)
• /api/v1/servers/{id}/review → scopeFiltered (the scopedServerSubtree gate; TestServerReviewUsesScopedSubtreeGuard)
• /api/v1/catalog/search → scopeOpen (searches remote registries; only the per-entry added flag is scope-filtered)
• Both review routes are also in shortCircuitCodes as 503, because the test fixture has no review service.

Verified locally: go test -race ./internal/httpapi/ passes, the guard passes with -count=3 and also with -tags server, and golangci-lint v2 finds nothing new in either tag set. Auto-merge (squash) is on, so this lands once CI is green. Thanks again for the contribution!