security: /api/long-running has no authentication — anyone can start test runs, scans and KB population, or cancel any operation #17010

Activity

1. mrveiss

   added this to the v0.9.0 milestone on Sep 18, 2026
2. mrveiss

   added

   bugSomething isn't workingSomething isn't working

   priority: critical

   security

   on Sep 18, 2026
3. mrveiss

   added a sub-issue on Sep 18, 2026

   • long-running: wire the indexing, KB-population, security-scan and code-analysis operations to their existing implementations, or retire them — no parallel implementation #17023
4. github-actions

   added

   backend

   on Sep 18, 2026
5. mrveiss commented on Sep 23, 2026

   

   mrveiss

   on Sep 23, 2026

   OwnerAuthor

   More actions

   AC verification against merged main — 3 of 4 met, issue stays OPEN

   Verified from origin/main at 31310392df, file autobot-backend/api/long_running_operations.py.
   Branch verdict: no branch carries this work — it landed and its branch is gone. The handover note
   that this issue was "referenced only" was right that it was never closed, and wrong that it was
   never done.

   • A router-level Depends(get_current_user). NOT MET AS SPECIFIED — the outcome is there,
     the mechanism is not, and the difference is load-bearing.
     Every one of the ten routes is gated, enumerated rather than sampled: the five start routes
     by dependencies=_ADMIN (:81), GET /{operation_id}, GET /, POST /{id}/cancel and
     POST /{id}/resume by current_user: dict = Depends(get_current_user) in the signature,
     and @router.websocket("/{operation_id}/progress") by open_authenticated_ws(...) at
     :407. So no route is anonymous today.
     But the router is APIRouter(tags=[...]) with no dependencies=, so a route added
     tomorrow without a Depends is anonymous and nothing would catch it: the repo-wide
     guard cannot see this router at all —
     enumerate_routers reports api.long_running_operations as NOT REGISTERED, because
     router_auth_enumerator.REGISTRY reads only core_routers.py while this router is
     registered as a string tuple in feature_routers.py:418. That is #16375, where I have
     posted the measurement; #16286 is the "one gated route hides the rest" half.
     This criterion asked for router-level precisely because per-route gating does not hold
     itself.
   • The five work-starting routes require admin. /codebase/index, /testing/comprehensive,
     /knowledge-base/populate, /security/scan and /migrate/existing all carry
     dependencies=_ADMIN, i.e. Depends(check_admin_permission).
   • Status, list, cancel, resume and the progress WebSocket are scoped — and better than this
     criterion asked. It settled for admin-only because operations recorded no creator. They
     do now, so _may_see (:99) admits creator-or-admin, _owned_operation (:108) returns
     404 rather than 403 so existence is not disclosed, and _may_see_id (:120) is the
     socket's own check. The criterion's own fallback is superseded, not unmet.
   • Tests. PARTIAL — the first clause has no test.
     autobot-backend/api/long_running_operations_17017_test.py (15 passed locally) covers
     non-admin refused on a work-starting route (test_a_non_admin_cannot_start_work), another
     user's operation refused without disclosure
     (test_another_users_or_an_unowned_operation_is_refused_without_disclosing_it), and creator
     and admin allowed (test_the_creator_reads_and_cancels_their_own,
     test_an_admin_reads_and_cancels_anyones).
     Not covered: no test asserts an unauthenticated request is refused on any of these
     routes — the first clause, and the one this issue is titled after. And "non-admin refused on
     each work-starting route" is one route, not five; the other four answer 501 before an
     auth decision would be observable, so a non-admin hitting them is untested.

   Verdict: the hole this issue was filed for is closed — /api/long-running is not anonymous.
   Two things keep it open: the gate is per-route in a router no guard watches, and the
   unauthenticated case is asserted nowhere. Both are small; neither is verified by this evidence,
   so neither is ticked.
6. mrveiss

   added a commit that references this issue on Sep 23, 2026

   test(api): no route on /api/long-running can be added without an auth…

   ...

   3f0d4b3
7. mrveiss commented on Sep 23, 2026

   

   mrveiss

   on Sep 23, 2026

   OwnerAuthor

   More actions

   #17344 merged (2026-09-23T21:54), adding the guard that catches a future ungated route on this router. It does not close this issue, and AC1 specifically is worth reopening as a question rather than treating as impossible.

   The PR's claim, and where it is broader than what is proven. #17344 reports that a router-level Depends(get_current_user) cannot be written, because a router-level dependency also applies to the WebSocket route /{operation_id}/progress (long_running_operations.py:398) and get_current_user takes a Request (auth_middleware.py:859) that FastAPI does not supply for a socket. That is true as narrowly stated: a single APIRouter covering both the HTTP routes and the WS route cannot carry that dependency.

   It does not follow that a router-level gate is impossible. A router split was not considered:

   • keep the 9 HTTP routes on one APIRouter, mounted with include_router(http_router, dependencies=[Depends(get_current_user)])
   • put the single websocket route on a second bare APIRouter under the same /api/long-running prefix, still gated inline by open_authenticated_ws exactly as today

   That satisfies AC1 literally for the entire HTTP surface and leaves the socket no less protected than it is now. It is a production-code change, which is why it was correctly out of scope for a test-only PR — but "the router-level dependency cannot be written" should be narrowed in the record to "a single unified router cannot carry it", so the next person does not read AC1 as closed off.

   What #17344 does deliver, and it is worth keeping: a guard that parses the module's own source with ast rather than walking the live route table, so it is immune to registration style — which matters here, because router_auth_enumerator.py:45 reads only core_routers.py and this router is registered in feature_routers.py:418, meaning the repo-wide sweep genuinely cannot see it. The guard also carries _MIN_ROUTES_SEEN = 10, verified exact against the 10 route decorators on base, so a silent-empty enumeration fails loudly instead of passing.

   Two weaknesses in that guard are filed as #17348 — it matches parameter names and the presence of a dependencies= keyword rather than their values, and its RED case was mutation-tested by hand but never committed. Neither leaves today's routes unchecked.

   So this issue stays open on AC1, and the concrete next step is the router split above rather than further work on the guard.
8. mrveiss

   marked this as blocked on Sep 23, 2026

   • tech-debt(guard): the LRO route-auth guard matches names, not semantics, and its RED case is not committed #17348
9. mrveiss commented on Sep 25, 2026

   

   mrveiss

   on Sep 25, 2026

   OwnerAuthor

   More actions

   All four acceptance criteria are met on current origin/main. Closing with evidence rather than reopening work — this was delivered and never closed.

   Verified against merged code, not against a PR diff. All ten routes on the router, enumerated by parsing the module:

   Route	Gate
   POST /codebase/index :132	dependencies=_ADMIN
   POST /testing/comprehensive :139	dependencies=_ADMIN
   POST /knowledge-base/populate :208	dependencies=_ADMIN
   POST /security/scan :215	dependencies=_ADMIN
   POST /migrate/existing :222	dependencies=_ADMIN
   GET /{operation_id} :271	Depends(get_current_user) + _owned_operation
   GET / :290	Depends(get_current_user), non-admin sees only their own
   POST /{operation_id}/cancel :335	Depends(get_current_user) + _owned_operation
   POST /{operation_id}/resume :360	Depends(get_current_user) + _owned_operation
   WS /{operation_id}/progress :398	open_authenticated_ws(..., allow=_may_see_id)

   Ten routes, ten gates, zero ungated.

   • AC1 — unauthenticated refused on every route. Satisfied by a different mechanism than the criterion named. The AC asked for a router-level Depends(get_current_user); the implementation gates each route and adds test_every_route_on_this_router_declares_a_gate (long_running_operations_17017_test.py:367) to enforce it structurally. Ticking because the criterion's stated property — "unauthenticated requests are refused on every route" — holds, and the guard is stronger than the router-level dependency would have been: a router-level dep cannot catch a future route declaring the wrong gate, and this does.
   • AC2 — work-starting routes require admin. _ADMIN = [Depends(check_admin_permission)] (:81, comment cites this issue) on all five. Test: test_a_non_admin_cannot_start_work (:239).
   • AC3 — status/list/cancel/resume/WS go to creator or admin. The AC said operations record no creator and that long-running: the four start routes cannot create an operation — create_operation gets estimated_items/context it does not accept (TypeError → 500) #17017 had to land first. It has: operations now carry metadata["created_by"] (test_the_test_suite_route_creates_an_operation_that_records_its_creator, :229), so these went to creator-or-admin rather than the admin-only fallback the AC allowed for — the stronger outcome.
   • AC4 — tests. Unauthenticated, non-admin, another user's operation, creator, and admin are each covered: :239, :243, :259, :268 (refused without disclosing the operation exists), :280.

   Worth recording because it is rarer than it should be: the guard at :367 is mutation-proved by three tests that assert it fails when it must — test_the_guard_names_an_ungated_route (:454), test_a_dependencies_list_without_an_auth_dependency_does_not_count (:459), and test_a_parameter_merely_named_current_user_does_not_count (:464). The last one is the interesting case: a parameter named current_user without the dependency would satisfy a naive grep, and the guard refuses it. There is also test_the_auth_vocabulary_matches_what_the_module_imports (:484), which keeps the guard's list of recognised gates tied to the module's real imports rather than to a hand-maintained copy that would drift.

   Nothing further is needed here. #17017 remains the home for anything outstanding on the create path.
10. mrveiss

    closed this as completedon Sep 25, 2026