1 Commits

Author SHA1 Message Date
Sunshine
a956bbc030
fix(runs): reject cancel actions on GET stream joins (#5092)
* fix(runs): reject cancel actions on GET stream joins

stream_existing_run is registered for both GET and POST, and its
?action=interrupt|rollback branch cancels the run. The CSRF middleware
exempts GET, so a session-authenticated browser could be forced
cross-site (img/script/top-level navigation) into
GET /api/threads/{id}/runs/{run_id}/stream?action=interrupt|rollback —
a state-changing GET that bypasses the CSRF protection guarding the
POST variant. Introduced with the dual registration in #1403.

The handler's docstring already documents cancel-then-stream as
POST-only (the LangGraph SDK's joinStream/useStream stop button uses
POST); enforce it: GET with an action answers 405, action-less GET
joins and POST cancel-then-stream are unchanged.

Regression drives the real router: GET+action is 405 with the run left
running, plain GET join still streams, POST+action still cancels.

* fix(runs): scope the 405 detail to the action requirement

"GET is a read-only stream join" overstates the current main: on a
locally-owned run with the default on_disconnect=cancel, a GET join's
disconnect can still trigger cancellation. That observer-disconnect
vector is closed by #5041; the detail here should only claim what this
guard enforces.

* fix(runs): harden GET stream action rejection

* fix(runs): align stream schema with method contract

* test(runs): pin GET stream action 405 through the production stack

Review follow-up (defence-in-depth): the GET-action suite drove bare
FastAPI() apps, so nothing pinned that a session-authenticated
cross-site GET reaches the route gate at all once CSRF exempts the
safe method. test_pat_auth.py already assembles the production
middleware order (AuthMiddleware inner, CSRFMiddleware outer), so its
mirror app now registers the real _reject_get_stream_action
dependency on a GET join route.

The new case pins the end-to-end premise: an authenticated GET
?action=interrupt is answered 405 + Allow: POST by the production
route dependency, while the same unauthenticated GET dies at
AuthMiddleware's 401 before any route logic runs.

Validation: focused suites (test_pat_auth, test_stream_get_action,
test_csrf_middleware) — 62 passed; ruff check + format clean; the new
case errors on the pre-fix baseline (guard absent), confirming the
pin.
2026-09-01 15:51:37 +08:00