Repository navigation
Add a route-table guard test so new /api/v1 GET routes cannot ship unscoped #1178
Description
Summary
Closing #1166 took three passes over the route table, and each pass found routes the previous one missed:
- The issue named
/api/v1/serversand/api/v1/config. - An adversarial review found
/api/v1/tools,/api/v1/index/search,/api/v1/profiles,/api/v1/annotations/coverage,/api/v1/security/scansand/api/v1/status. - A cross-model review then found the entire
/api/v1/servers/{id}/*read subtree (tools, logs, tool-calls, scan/*, integrity, tools/export), the whole/api/v1/activityand/api/v1/tool-callsfamily,/api/v1/sessions, and/api/v1/stats/tokens.
Hand-enumerating a route table does not converge. The fix that ships gates the subtree with a chi middleware so new sub-resources inherit it, but nothing stops a new top-level route being added unscoped.
Proposal
A test that walks the production router (chi.Walk over s.router), collects every GET under /api/v1, and asserts each is either:
- demonstrably scope-filtered (exercised by a scoped-caller test), or
- on an explicit allowlist with a written reason (e.g. "admin-only, denied by requireAdminRead", "carries no server identity").
A new route then fails the build until someone classifies it, and the allowlist becomes the reviewable record of what is deliberately unscoped.
Guard against vacuity: the test must assert it found a non-trivial number of routes, or a chi.Walk that silently matches nothing passes forever.
Why it is worth the effort
This is the durable fix for the enumerate-and-patch problem. Two of the routes in pass 3 (/servers/{id}/logs, /activity) leaked strictly more than the enumeration the original issue was about — server stdout/stderr, and tool-call arguments and responses.
Activity
Sriram-PR commentedon Sep 22, 2026
i can take this one. went through how the /api/v1 routes get scoped for agent tokens, the deny cases go through requireAdminRead, and the rest get filtered by visibleServers / the scopedServerSubtree middleware, so the chi.Walk idea fits well here.
plan: walk s.router, collect every GET under /api/v1, and assert each is either driven by a scoped-token test that proves it's filtered/denied, or on an explicit allowlist with a written reason next to it. a new route that's neither fails until someone classifies it. plus the min-count check so an empty walk can't pass silently.
one thing worth flagging: there's already a chi.Walk guard for the server edition (tenant sessions) in tenant_allowlist_walk_test.go, but nothing equivalent for the personal-edition agent-token scope, which is where #1166 actually bit. so this fills that gap rather than duplicating it.
test(scope): guard the /api/v1 GET surface against unscoped routes (#…
Dumbris commentedon Oct 6, 2026
Triage on 2026-10-05 against main 8442428: Implemented by merged PR #1351 as internal/httpapi/scope_route_table_guard_test.go: a chi.Walk route table with a minGetRoutes=40 vacuity floor and an allowlist with reasons. The issue stayed open only because the PR had no closing keyword.