fix(security): require sign-in on seven ungated routers, and admin where they change shared state or reach outside (#16375)

[mrveiss]

Refs #16375

Single-issue rationale: this gates seven of #16375's routers, in one change of one kind: a router-level sign-in dependency plus per-route admin. api.long_running_operations goes in the next PR, because it needs its own extraction to get under its size ceiling, and its websocket needs its own gate. api.chat_knowledge waits for #16345, which edits that file.

Thinking Path

• security(api): 44 config-registered routers declare no auth gate, triaged by exposure (found by widening the router-auth guard, #16368) #16375 found 44 routers mounted with no auth gate.
  • app_factory.py:83,90 mounts every registry router with include_router(router, prefix, tags) and no dependencies=.
  • AuthenticationMiddleware is never installed.
  • service_auth_enforcement.py only blocks SERVICE_ONLY_PATHS.
• The coordinator took the riskiest into one PR, following fix(security): authenticate every skills route, and require admin to change, fetch or execute (#16368) #16370's pattern. Before gating, I traced what an anonymous caller could reach on base a12b4bf79. The keyword signals in security(api): 44 config-registered routers declare no auth gate, triaged by exposure (found by widening the router-auth guard, #16368) #16375 were leads, not proof. The trace is below, per router.
• The two routers first dropped as "gated" were ungated.
  • knowledge_extract has no dependency at all.
  • long_running_operations' only dependency is a 503 availability check.
  • An enumerator false match: an import from api.ws_security counts as a gate, and the whole module is judged as one unit. Filed as a separate enumerator bug.
  • knowledge_extract is in this PR. long_running_operations is next.

What an anonymous caller could reach (before this PR)

Router	Route(s)	What it reached
api.ide_integration (/api/ide)	PUT /config	Changed the one engine every client on that worker shares. It could switch off the SQL-injection, hardcoded-secret and eval/exec rules for everyone (update_configuration).
	POST /analyze, /quickfix, /hover, /batch-analyze, /completion; GET /rules, /categories, /severities	In-memory regex and AST analysis of submitted code, plus Redis cache writes (the completion and context caches). No subprocess, file or outbound access: the "shell/exec" hits are regex strings in the lint rules.
api.knowledge_crawl	POST /api/knowledge/crawl	BFS-crawled caller-chosen seeds. KB writes are on by default (ingest=True → store_fact, Redis and ChromaDB). An unguarded robots.txt GET went to every seed and linked host (SSRF, #16378). A scrape_template_id reached the headless template runner, whose own router needs admin.
api.knowledge_scrape	POST /api/knowledge/scrape	A guarded fetch of any public URL, returned to the caller: an open fetch proxy. ingest=true wrote to the autobot_docs ChromaDB collection.
api.knowledge_site_map	POST /api/knowledge/site-map	An unguarded GET of <caller scheme://host:port/path>/sitemap.xml, plus every child <loc> of a sitemap index, with <loc> values reflected back. That's SSRF to internal hosts (#16378).
api.knowledge_extract	POST /api/knowledge/extract	A guarded fetch, then an LLM call with the caller's JSON Schema (LLM spend), then an optional write to autobot_docs.
api.run_jwt_router	POST /api/runs/{run_id}/jwt/refresh	Not usable without a valid run JWT: the handler checks the signature, audience, denylist and run_id binding. But a refreshed token has no absolute lifetime (#16379).
api.web_research_settings (/api/web-research)	PUT /settings	Persistent config writes to settings.json, including require_user_confirmation and the rate limits.
	POST /test	Drove the headless research browser with the caller's query, using up the shared rate limit.
	POST /reset-circuit-breakers, /clear-cache	Switched off the breaker's protection, and cleared the cache.
	POST /enable, /disable	Always 500 today on a broken import (#16380), so nothing changed.
	GET /status, /settings, /usage-stats	Status, config and stats, with no secrets.

What Changed

• Router-level Depends(get_current_user) on all seven routers, so a route added later is gated by default.

• Per-route Depends(check_admin_permission) on every route that changes shared config or drives the research browser:

  Level	Routes
  Admin (7)	PUT /api/ide/config; POST /api/web-research/enable, /disable, /test, /clear-cache, /reset-circuit-breakers; PUT /api/web-research/settings
  Authenticated (16)	the other eight /api/ide/* routes; POST /api/knowledge/crawl, /scrape, /site-map, /extract; POST /api/runs/{run_id}/jwt/refresh; GET /api/web-research/status, /settings, /usage-stats

  Owner decision: the web-research routes (crawl/scrape/site-map/extract) are for any signed-in user — each still reaches outside on a caller-chosen URL, but only the web-research settings mutations (enable, disable, PUT /settings, test, clear-cache, reset-circuit-breakers) stay admin.

• run-JWT refresh stays authenticated-only. get_current_user accepts a run-scoped JWT (SEC-2 feat(security): run-scoped short-lived JWTs to limit blast radius of leaked agent credentials #6473), so a run refreshing its own token still passes. The handler still binds the token to run_id, and revokes it.

• Moves needed to edit at all:

  • The knowledge and run-JWT models move unchanged to the new api/schemas_knowledge_web.py and api/schemas_run_jwt.py. The no-local-schemas hook (feat(lint): add pre-commit hook to block new local BaseModel definitions in non-schema API endpoint files #6056) blocks any edit of a router that still defines its own BaseModel, and schemas_knowledge.py is at its ceiling.
  • CrawlPageEntry had no user and isn't re-imported.

• Room under the size ceiling: PATTERN_RULES (184 lines) moves unchanged to api/ide_pattern_rules.py, and api.ide_integration re-imports it.

  • ide_integration.py goes 1110 → 928 lines, and its ceiling is lowered in both repo_tests/python_file_size_ratchet_baseline.py and scripts/python_file_size_known_large.py.
  • The moved list was checked AST-identical to the original.

• Changelog: changelog/unreleased/16375-router-auth.md.

Behaviour change, stated plainly:

• Non-admin users keep the Scrape, Crawl, Site map and Extract tabs. (composables/knowledge/useWebResearch.ts:107-119.) Any signed-in user can reach these four routes, per the owner's decision above.
• Non-admin users lose the web-research toggles. (stores/useWebResearchStore.ts: enable/disable, settings, test, clear-cache, reset-circuit-breakers.) A non-admin now gets 403 there.
• No SLM-frontend or service-to-service callers of these paths were found.

Verification

• CI: pending.
• autobot-backend/tests/api/test_router_auth_16375.py tests with the real dependencies, on fix(security): authenticate every skills route, and require admin to change, fetch or execute (#16368) #16370's pattern. Under pytest auth_middleware is the conftest stub, which grants admin, so the test overrides exactly the objects each router bound with the real functions from real_auth_middleware.
  • Every route is classified: the pinned policy table equals the seven routers' 23 routes.
  • Anonymous callers get 401 on all 23.
  • A non-admin gets 403 on all 7 admin routes.
  • A non-admin gets 200 on the four research POSTs (crawl, scrape, site-map, extract — outbound I/O stubbed, so only the auth gate is under test) and on GET /api/ide/categories (the pre-existing control).
• Existing tests that import these routers (tests/api/test_web_research_settings_guard.py, tests/unit/api/test_knowledge_crawl.py, api/knowledge_scrape_test.py, api/web_research_status_timeout_12751_test.py) run under the admin-granting stub. The new dependencies resolve to it there, as with fix(security): authenticate every skills route, and require admin to change, fetch or execute (#16368) #16370.
• Local checks: pre-commit and pre-push hooks only. I executed no code from the repo.

Sequencing: #16370's router-auth guard adds a baseline of the ungated routers, with a stale-entry check. Once it merges, base gets merged here and these seven routers' entries get removed.

Filed while tracing: #16378 (SSRF in the robots.txt and sitemap fetches, which auth doesn't close), #16379 (unbounded run-JWT refresh), #16380 (web-research enable/disable always 500), #16381 (IDE completion cache key and unbounded input).

Model Used

Claude Opus 5 (claude-opus-5)

🤖 Generated with Claude Code

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: d59bbe4c-d580-4f89-9e35-d85574cefe3f

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Notice: 29 open PRs — past the runaway threshold (25)

There is no PR queue limit, and this is not a request to defer this PR. Work proceeds one issue at a time without a cap on open PRs; review capacity is the constraint.

This notice only means the count is high enough to be worth a glance for a runaway — something opening PRs in a loop, or a merge pipeline that has stalled so nothing is draining.

Currently open:

• chore(ci): merge train 2026-09-12-a, test vehicle, do not merge (#15937) #16417 chore(ci): merge train 2026-09-12-a, test vehicle, do not merge (Enable the merge queue — no PR merges against a stale base #15937)
• feat(preflight): run every reproducible required check through the workflow's own command (#15933) #16416 feat(preflight): run every reproducible required check through the workflow's own command (Local preflight must cover the 10 required checks #15933)
• fix(guards): fold shell continuations in the hooksPath guard via one shared helper (#15961) #16414 fix(guards): fold shell continuations in the hooksPath guard via one shared helper (core.hooksPath override silently disables every hook in a worktree #15961)
• fix(security): deny an SLM token when its revocation check cannot run, HS256 and RS256 (#16387, #16412) #16413 fix(security): deny an SLM token when its revocation check cannot run, HS256 and RS256 (security(slm-auth): token revocation checks fail open when Redis errors, so a revoked token is accepted #16387, security(slm-auth): RS256 authority-token revocation check fails open when Redis errors #16412)
• fix(security): generate the vault root key from 32 random bytes, not alphanumeric characters (#16405) #16410 fix(security): generate the vault root key from 32 random bytes, not alphanumeric characters (security(secrets): the vault root key is base64 of 32 alphanumeric chars (~190 bits), not 32 random bytes #16405)
• chore(docker): retire the four agent Dockerfiles whose modules no longer exist; agents run in-process (#16361) #16409 chore(docker): retire the four agent Dockerfiles whose modules no longer exist; agents run in-process (chore(docker): four agent Dockerfiles under autobot-infrastructure/autobot-backend/docker/ are built by nothing: wire in or retire #16361)
• chore(deps): carry dependabot's npm security bumps onto Dev_new_gui (hono 4.13.7 and transitive; supersedes #16171) #16408 chore(deps): carry dependabot's npm security bumps onto Dev_new_gui (hono 4.13.7 and transitive; supersedes chore(deps): bump the npm_and_yarn group across 6 directories with 7 updates #16171)
• fix(tests): stop gating a disk-bound model load on a CPU work budget, and prove it stays offline and import-free (#15055, #15235) #16404 fix(tests): stop gating a disk-bound model load on a CPU work budget, and prove it stays offline and import-free (test(perf): startup benchmarks assert absolute wall-clock budgets and fail on runner load — passed and failed on the same code minutes apart #15055, test(perf): three calibrated budgets are drifting or disk-bound — the startup margin is 1.4x, not the 1.9x its note records #15235)
• fix(ci): keep the drift-checked SLM agent mirror out of the duplication scan (#16401) #16402 fix(ci): keep the drift-checked SLM agent mirror out of the duplication scan (ci(duplication-guard): the SLM scope counts the drift-checked agent mirror, so every agent change fails the absolute pin #16401)
• fix(ci): bring CI's pytest environment up to its declared floors, and gate on them (#16264) #16391 fix(ci): bring CI's pytest environment up to its declared floors, and gate on them (ci: the pytest environment sits below two declared floors, and the floor banner tells CI readers the opposite #16264)
• fix(security): the dev proxy stops granting admin without a login, and the session check endpoint is tested (#16382, #16385) #16390 fix(security): the dev proxy stops granting admin without a login, and the session check endpoint is tested (SLM Vite dev-server proxy injects X-Internal-API-Key on /autobot-api with no session check #16382, SLM backend: add direct test coverage for GET /api/auth/me #16385)
• fix(security): the SLM proxy checks the caller's SLM session before it adds the backend key (#16374) #16386 fix(security): the SLM proxy checks the caller's SLM session before it adds the backend key (security(slm): the SLM nginx /autobot-api/ proxy adds the internal API key for every caller, so anyone reaching the SLM host is a backend admin #16374)
• fix(ci): retire the CodeQL suite file nothing loads, and record the real guard behind alert #1030 (#16283, #16302) #16377 fix(ci): retire the CodeQL suite file nothing loads, and record the real guard behind alert feat: Voice conversation mode — Hands-free with VAD (Tier 2) #1030 (ci(security): codeql.yml is disabled, so the reviewed codeql-config.yml exclusions apply to nothing #16283, ci(security): codeql-config.yml's file-scoped query-filters exclude nothing, under either setup #16302)
• fix(security): write the service-key export only to its configured directory, never into a checkout, and keep one (#16348) #16372 fix(security): write the service-key export only to its configured directory, never into a checkout, and keep one (security(service-auth): generate_service_keys.py writes its live-key export relative to the working directory, so a run inside a checkout drops keys into the work tree #16348)
• fix(security): authenticate every skills route, and require admin to change, fetch or execute (#16368) #16370 fix(security): authenticate every skills route, and require admin to change, fetch or execute (security(api): every route in api/skills.py (catalog fetch/install, enable, execute) is unauthenticated, and the router-auth guards can't see feature routers #16368)
• fix(ci): let the system-package retry recover from its own timeout, and fetch before unpacking (#15515) #16367 fix(ci): let the system-package retry recover from its own timeout, and fetch before unpacking (ci(runner): apt provisioning times out (exit 124) three attempts in a row and fails the shard before any test runs #15515)
• fix(guards): tell a baseline entry that matches nothing apart from one that under-matches (#16334) #16365 fix(guards): tell a baseline entry that matches nothing apart from one that under-matches (bug(guards): --audit-baseline says "no longer match anything" for an entry that still matches, so the reader deletes a live exemption #16334)
• fix(ci): stop the regen bot's approved runs cancelling the job that approved them (#16360) #16363 fix(ci): stop the regen bot's approved runs cancelling the job that approved them (ci: the regen bot's own push cancels the job that approves its parked runs, so a PR's CI can silently never run #16360)
• fix(ci): keep line numbers out of the secrets baseline, so a line move no longer rewrites it (#16353) #16362 fix(ci): keep line numbers out of the secrets baseline, so a line move no longer rewrites it (ci(secrets): .secrets.baseline stores a line number per finding, so every PR that shifts lines in a file with findings rewrites it and conflicts with every other such PR #16353)
• test(guards): name the deliberate fixed sleeps, and stop a random temp name deciding a language test (#16255, #16341) #16358 test(guards): name the deliberate fixed sleeps, and stop a random temp name deciding a language test (tests: 32 more fixed sleeps are followed by an assertion, and some decide the outcome by the clock #16255, test(voice): test_no_filename_hint flakes when tempfile's random name forms a language-code token (e.g. _lv_) #16341)
• fix(ci): gate the frontend npm audit on one report, retry it, and report 'could not check' apart from advisories (#16337) #16357 fix(ci): gate the frontend npm audit on one report, retry it, and report 'could not check' apart from advisories (ci(security): the npm audit gate fails an audit-endpoint error exactly like a high advisory — retry, and report 'could not check' distinctly #16337)
• fix(docker): retry apt fetches and fail apt-get update on a lost index, set once for every image (#16291) #16356 fix(docker): retry apt fetches and fail apt-get update on a lost index, set once for every image (ci(docker): an unreachable Ubuntu mirror surfaces as a misleading 'held broken packages' error; apt steps neither retry nor fail on index errors #16291)
• feat(slm): Redis control and detailed health live in the SLM only; remove the unreachable user-GUI copies (#16245, #16252) #16352 feat(slm): Redis control and detailed health live in the SLM only; remove the unreachable user-GUI copies (tech-debt(frontend): move SettingsPanel's detailed system health and Redis service management into the SLM #16245, bug(frontend): RedisServiceControl calls /api/service-monitor/services/* and no mounted router serves those paths #16252)
• fix(code-sync): delete files removed from source through the updater, and check drift across every file (#16310, #16322) #16351 fix(code-sync): delete files removed from source through the updater, and check drift across every file (deploy: the builtin updater never removes files deleted from source, so deleted modules pile up on every host #16310, deploy: ansible synchronize never deletes files removed from source on remote fleet nodes #16322)
• fix(pre-commit): run detect-secrets serially and refresh the baseline's line numbers (#16343) #16349 fix(pre-commit): run detect-secrets serially and refresh the baseline's line numbers (pre-commit: detect-secrets runs in parallel batches that race on .secrets.baseline, so large commits and base merges can't commit #16343)
• fix(kb): make three summarisation paths do what they claim (#14840, #15700, #16110) #16345 fix(kb): make three summarisation paths do what they claim (context-overflow summarization can never succeed — _get_gateway imports a module and a symbol that do not exist #14840, sec(kb): the chat-to-knowledge summariser interpolates whole transcripts unframed, and the result is persisted to the knowledge base #15700, bug(kb): summary quality is scored against text[:200] — a 300-word summary judged on a 200-character window #16110)
• ci(python-suite): queue the shards first in, first out across PRs, with a per-shard job-level queue: max group (#16320) #16328 ci(python-suite): queue the shards first in, first out across PRs, with a per-shard job-level queue: max group (ci: queue python-suite shards FIFO across PRs (queue: max), so one PR's suite finishes before the next starts #16320)
• docs(env): move the generated env-var table into ENV_VARS.md and drop its count line, so env-var PRs stop conflicting (#16317) #16318 docs(env): move the generated env-var table into ENV_VARS.md and drop its count line, so env-var PRs stop conflicting (docs: the generated env-var table and its count line live in CLAUDE_RULES.md, so every env-var PR conflicts with every other #16317)
• fix(terminal): render TerminalModals in TerminalWindow, with each action reporting the parent's real outcome (#16285) #16313 fix(terminal): render TerminalModals in TerminalWindow, with each action reporting the parent's real outcome (fix(terminal): TerminalModals reports success after a fixed delay, and nothing renders it; TerminalWindow still has the modals inline #16285)

If the queue is draining normally, ignore this. Otherwise:

1. Check whether CI is dispatching at all — see the ci-dispatch-watchdog status on these PRs
2. Merge the ones whose CI has finished and review has passed: gh pr merge <number> --squash --delete-branch
3. Look for a loop opening near-identical PRs

Warn-only runaway detector — .github/workflows/pr-queue-gate.yml. It never blocks a merge.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

[mrveiss]

CI red at c651fb43f: this PR's own new completeness test, cause not fully isolated. test_router_auth_16375.py::test_the_policy_table_covers_every_route asserts _routes(_app()) == set(_POLICY); the actual side (_routes(_app())) comes back completely empty, so all 15 _POLICY entries read as missing.

_app() builds a bare FastAPI() and calls include_router(module.router, prefix=...) for each of 7 directly-imported modules (import api.ide_integration as ide_api, etc. — not going through the app factory or router registry). I ruled out autobot-backend/conftest.py stubbing api.* broadly (it doesn't, unlike the SLM conftest's AST-stubbing that caused a similar-shaped empty-result bug on #16236) — so that's not the cause here.

I could not pin the exact reason _app() collects zero routes with the budget I had. Worth checking next: whether these 7 router modules define their routes via a decorator that needs the app's own lifespan/startup to run first (so a bare FastAPI() never sees them), or whether module.router is being shadowed by something these modules do at import time in this test's collection order specifically.

[mrveiss]

Security review at 92b29ef0d: APPROVE WITH NITS. Refs #16375 is correct: seven of the issue's routers are gated here, not all of them.

Verified at this head:

• Each of the seven modules gates at router level with APIRouter(dependencies=[Depends(get_current_user)]):
  • ide_integration.py:82
  • knowledge_crawl.py:54
  • knowledge_extract.py:61
  • knowledge_scrape.py:59
  • knowledge_site_map.py:46
  • run_jwt_router.py:26
  • web_research_settings.py:39
• Admin checks sit on the mutating routes: check_admin_permission appears twice in ide_integration.py and 7 times in web_research_settings.py.
• No overlap. security(auth): admin-gate the open routers, settings, teams and user-account routes (#15745, #16278, #16276, #16277, #15738, #16279) #16240's f5fe68888 gated developer, frontend_config, redis, settings and settings_config. Open security(routers): gate knowledge_search, knowledge_search_aggregator, knowledge_suggestions and voice_stream (#16507) #16508 gates knowledge_search, knowledge_search_aggregator, knowledge_suggestions and voice_stream. Neither list shares a module with this PR, so nothing is double-gated and there is no conflicting dependency.
• The 07:40Z red at c651fb43f is fixed. 92b29ef0d reads routes per router through effective_routes, because walking the mounted app on FastAPI ≥0.139 returned nothing. At this head the latest checks show nothing red and nothing pending.
• tests/api/test_router_auth_16375.py uses the real_auth_middleware fixture, not the stubbed admin grant.
• From the review pass, not re-derived by me:
  • The tests assert 401 for anonymous callers on all 23 routes and 403 for non-admins on the 7 admin routes.
  • The policy table covers every route in the seven modules.
  • None of them has a websocket route.

Severity	Where	Note
Nit	tests/api/test_router_auth_16375.py	It mounts the routers on a bare FastAPI(), not through the real registry. So a prefix or dependency drift in feature_routers.py would not be caught here. That is acceptable under the FastAPI ≥0.139 constraint, and #16370's config-router guard covers the registry side once it lands.
Info	repo_tests/python_file_size_ratchet_baseline.py, scripts/python_file_size_known_large.py	Open #16508 also edits both files. The review pass read the lines as non-adjacent, but whichever PR merges second should re-check the counts.

[mrveiss]

Delta 92b29ef0d..2ebcd96b0 (the rebase onto main plus the list fix): approve. The change is list removals only.

• The only file whose change differs from the approved net diff is repo_tests/config_router_auth_coverage_test.py. Its net diff removes exactly six KNOWN_UNGATED entries: api.ide_integration, api.knowledge_crawl, api.knowledge_scrape, api.knowledge_site_map, api.run_jwt_router and api.web_research_settings. Each of the six is on main's list. Nothing is added.
• api.knowledge_extract was never on main's KNOWN_UNGATED, so there was nothing to remove for the seventh module.
• The six router modules, their tests and the ratchet files are unchanged from the approved version; the other commits are the rebased originals.
• This fixes the batch-2a red on test_the_known_ungated_list_has_not_gone_stale, which landed after my approval.

The branch is 0 behind main, and CI has 23 checks pending with none red.