fix(gateway): enforce chat.abort owner-authorization — reject non-owner aborts (#2721)#2729
Merged
Merged
Conversation
…er aborts (#2721) chat.abort never read client identity, so any authenticated client could abort another client's in-flight run (cross-client denial of in-flight work, HIGH). Restore the owner-authorization control, self-contained to chat.ts: - resolveChatAbortRequester identifies the requesting client (connId / deviceId / operator.admin scope). - canRequesterAbortChatRun authorizes the requester against the run's owner: admin bypass, owner-by-deviceId (paired device across reconnects), owner-by-connId; unowned runs stay abortable by any authenticated client (no legacy regression). - Explicit-run aborts from a non-owner are rejected as "unauthorized". - Session-scoped aborts cancel only requester-owned runs (resolveAuthorizedRunIdsForSession + abortAuthorizedSessionRunsWithPartials). - chat.send populates ownerConnId / ownerDeviceId at run-start so the check has data. Un-skips the two covering tests in chat.abort-authorization.test.ts. Behavior tightening: previously-accepted non-owner aborts now return unauthorized. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
chat.abortnever read client identity, so any authenticated client could abort another client's in-flight chat run — cross-client denial of in-flight work (HIGH). This restores the owner-authorization control, self-contained tosrc/gateway/server-methods/chat.ts(no shared-core edits).Closes #2721.
What changed
resolveChatAbortRequester— identifies the requesting client:connId,deviceId(connect.device.id), andoperator.adminscope.canRequesterAbortChatRun— authorizes a requester against the run's recorded owner:operator.admin→ bypass;ownerConnId/ownerDeviceId) → abortable by any authenticated client (no regression for legacy / no-identity runs);deviceId(a paired device keeps abort rights across reconnects) or owner-by-connId(originating connection).{ code: "INVALID_REQUEST", message: "unauthorized" }; the run is left untouched.resolveAuthorizedRunIdsForSession+abortAuthorizedSessionRunsWithPartials, preserving partial-transcript persistence).chat.sendpopulatesownerConnId/ownerDeviceIdat run-start so the check has data in production.chat.abort-authorization.test.ts.Behavior change (intended tightening)
Previously-accepted non-owner aborts now return
unauthorized. The owner (same connection, or same paired device after reconnect) andoperator.adminretain full abort capability; unowned runs are unaffected.Test plan
chat.abort-authorization.test.ts— 4/4 (the 2 previouslyit.skipcases un-skipped and passing).pnpm test:gateway— 1554 passed / 0 failed (140 files), incl.chat.abortpersistence (client: null+ unowned runs) and bothserver.chat.gateway-server-chat*e2e suites (same-wssend → abort).acp/translator.cancel-scoping.test.ts— 7/7.pnpm check(oxfmt + tsgo + oxlint + css-class-drift) clean; throwing-stub-callers gate 0 violations.Merge
Per the dispatch directive, this PR is intentionally not merged — merging is handled by the orchestrator once CI is green.
🤖 Generated with Claude Code