Fix auth-none-only cleanup#98788
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed July 1, 2026, 7:41 PM ET / 23:41 UTC. Summary PR surface: Source +6, Tests +3. Total +9 across 2 files. Reproducibility: yes. at source level for the review blocker: current Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Retarget this to the actual token-auth Do we have a high-confidence way to reproduce the issue? Yes at source level for the review blocker: current Is this the best way to solve the issue? No. The auth-none guard split may be reasonable cleanup, but the best fix for the linked regression needs to exercise the actual token-auth Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against cdaafe198cb7. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +6, Tests +3. Total +9 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
46265cc to
893abf6
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
4101e83 to
e972df9
Compare
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
2b1c580 to
8fca119
Compare
|
Filed #98614 — thanks for picking it up, but I don't think this PR fixes the case reported there. Our environment uses token auth ( ClawSweeper's own review flagged the same gap (confidence 0.9): the closing reference can auto-close #98614 without the token-auth regression being fixed or proven. Given the maintainer options ClawSweeper listed, would it make sense to either:
Happy to help verify against a live token-auth deployment once there's a build that changes that path. |
…-auth check Auth mode "none" independently satisfies two concerns that were previously conflated behind requireLocalBackendSharedAuth: 1. Shared-auth check (hasLocalBackendSharedAuth) — when the caller explicitly requests local-backend shared auth, auth-none fulfills it without a token. 2. Device-identity omission — auth-none always omits device identity so the server-side device-less self-pairing bypass preserves requested scopes without requiring paired-device checks. Previously isAuthNone was gated behind requireLocalBackendSharedAuth, so callers that did not set the flag would still resolve device identity for auth-none deployments. Compute isAuthNone independently and pass it to both shouldOmitDeviceIdentityForGatewayCall and resolveDeviceIdentityForGatewayCall.
36812ea to
cd0b19e
Compare






What Problem This Solves
When auth mode is
"none"(e.g., Google Chat loopback deployments),sessions_spawnand other gateway calls requiringoperator.writescope fail withmissing scope: operator.write.Root cause: Commit
65b460f234introducedallowAuthNonewhich was tied torequireLocalBackendSharedAuth— a flag only set by the nodes CLI path. WhencallSubagentGateway(the actualsessions_spawncode path) callscallGatewaywithoutrequireLocalBackendSharedAuth,allowAuthNoneevaluates tofalseeven when auth mode is"none". This causesshouldOmitDeviceIdentityForGatewayCallto returnfalse(no token →hasDirectLocalBackendAuthis false), sodeviceIdentityis resolved from disk instead of being set tonull. The server receives a device identity without a paired token, clears requested scopes to[], and theagentmethod fails withmissing scope: operator.write.Why This Change Was Made
Two concerns are now cleanly separated:
hasLocalBackendSharedAuth): tied torequireLocalBackendSharedAuth— only matters for callers that explicitly request local backend shared auth (e.g., nodes CLI)omitDeviceIdentity):isAuthNoneis computed independently ofrequireLocalBackendSharedAuth, so auth-none always triggers device identity omission for all callers, includingcallSubagentGatewayUser Impact
sessions_spawnand all subagent gateway calls no longer fail withmissing scope: operator.writeisAuthNoneisfalse)isLoopbackGatewayUrlgates the omission)mode/clientName)Evidence
Changed Files
src/gateway/call.tsisAuthNoneindependently, pass to bothshouldOmitDeviceIdentityForGatewayCallandresolveDeviceIdentityForGatewayCallsrc/gateway/call.test.tsrequireLocalBackendSharedAuth(simulatescallSubagentGatewaypath)Unit Tests
New test:
omits device identity for auth-none without requireLocalBackendSharedAuth (callSubagentGateway path)— verifies that auth-none triggers device identity omission even whenrequireLocalBackendSharedAuthis not set, covering the actualcallSubagentGateway→callGatewayspawn path.Real-Machine Verification — Auth-None (the fix)
Gateway auth mode=none explicitly configurede2bc5456⇄ res ✓ agent 53mse2bc5456ended with stopReason=stopNo
missing scopeorshared auth requireserrors.Real-Machine Verification — Token-Auth (no regression)
Config:
"auth": { "mode": "token" }, loopback, local mode."mode": "token"1cae301f⇄ res ✓ agent 195ms1cae301fended with stopReason=stopNo regression on the token-auth path.
Before/After Comparison
✗ sessions.spawn ... missing scope: operator.admin✓ agent 53ms ... stopReason=stopGatewayLocalBackendSharedAuthUnavailableErrorEnvironment
🤖 Generated with Claude Code