Repository navigation
feat(auth): scope renewable outside-agent OAuth grants - #558
Conversation
|
What: Independent review of P2: refresh-token reuse does not revoke the grant
Cleared in the inspected scope
Optional, not blocking
Proof limitsI read the full diff, #553 and comment 6275891763, and the consumers named above. I ran no tests. I inspected the author's logs: Reviewed by Claude Opus 5.5 ( |
|
What: Fixed the review's P2 in Finding accepted and independently checked against the compare-and-swap failure branch. The new regression failed before the fix because the successor access token still authenticated. After the fix it proves stale refresh rejection, successor access/refresh rejection, and absence from active clients. A separate concurrent-refresh test proves one successful rotation followed by revocation of the raced grant. The manual-revocation test remains separate and retains wrong-client/resource denials followed by a successful legitimate renewal. User guidance now tells clients to serialize renewal and never reuse replaced tokens. Focused proof: Optional notes are deferred: no demonstrated client failure requires changing the refresh scope-input policy, and switching the new native form inputs to primitives is outside this security correction. Requirements and who asked: #553's renewable OAuth grant, with the independent review's P2 requiring compromised rotation authority to be cut off. |
|
What: Independent review of P2: concurrent refresh from the stock MCP SDK revokes the grant
Prior P2 (replay without revocation): fixed
Grant ceiling boundaries re-checked on this head: cleared
Optional, still not blockingNative Proof limitsI read #553, comment 6275891763, the full PR diff at Reviewed by Claude Opus 5.5 ( |
|
What: Fixed the concurrent-SDK-refresh P2 in Finding accepted after reading the pinned SDK's The successor nonce is derived with a separate HMAC domain from the presented token. We persist only the current and immediate predecessor hashes plus a fixed five-second deadline. A duplicate within that deadline returns the same successor; it neither rotates again nor extends the deadline. Older-token reuse or reuse at/after the deadline still revokes the shared session. This state survives reopening the database, and the schema adds the two nullable retry fields when an earlier candidate table exists. Proof on this candidate:
Requirements and who asked: #553's unattended self-renewal plus independent review's replay and SDK-concurrency findings. |
|
What: Independent review of Prior findings, adjudicated on this head
Security tradeoff, acceptedA thief who presents the same stale token within five seconds of the real rotation receives the same successor instead of triggering revocation. Because successors are deterministic, both holders then share one chain. The next time either one presents a superseded token more than five seconds after a rotation, the grant is revoked. The thief cannot read the legitimate client's timing, so detection is delayed by at most the window, not lost. That is the RFC 9700 §4.14.2 tradeoff proposed in the prior review and documented in Checked and cleared
Optional, still deferredNative Proof limitsI read #553, comment 6275891763, both prior reviews, the full diff Reviewed by Claude Opus 5.5 ( |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
|
What: PR #558 hit the previously reported Grok fast-wake idle assertion at unchanged head Failed job: Unchanged local command: Requirements and who asked: User #553 requires focused proof and an independent different-model review; repository CI supplies complete checks. |
What: Outside agents can be approved until revoked, with project-id and named-operation limits enforced by the server.
Why: Unattended services otherwise lose sign-in after 30 days, and outside agents can currently reach every project.
So what: Ready for human review against main, with current CI and independent review passed; no merge or deployment is authorized.
Closes #553, including the scoping acceptance criteria.
Approval retains the 30-day default and adds Until revoked, an optional immutable project id, and named operations. Renewable grants return one-hour access tokens and rotating refresh tokens bound to the client and resource. Refresh authority has a separate signing domain; only the current and previous hashes are persisted. Renewal copies the signed ceiling and the existing session owns revocation. The declaration boundary rejects unapproved tools, other projects, remote forwarding and borrowed caller contexts before handlers. Settle permits only settling.
#323's paired-client protocol, peer links, #552 and #554 are unchanged. Renewable outside grants use an absolute deadline to avoid #389's existing year-10000 TTL overflow; the peer helper and existing grants are untouched. This supports the Objective by allowing unattended services to drive Drafters within an approved project.
Elon record
Requirements and who asked: Lukasz requested #553 plus comment 6275891763, self-renewal with immediate revocation, fixed server-side project/operation limits, unchanged 30-day defaults, red tests first, shortened clock, isolated worktree and different-model review.
Deleted: Global TTL changes, monthly reapproval, a separate non-OAuth credential mechanism, and work owned by #323/#552/#554/#379.
Bottleneck: OAuth lacked refresh grants and MCP declarations lacked project/operation limits; browser proof also exposed #389's existing permanent-TTL overflow.
Checked myself: Read the Issue and required comment in full, checked #323's mcp-client exclusion and upstream history/open PRs/Issues, ran red/green authorization and clock tests, and exercised UI approval/revocation and real MCP HTTP requests against synthetic worktree state.
Proof
Focused commands:
Lint ran only the 12 changed TypeScript/TSX files and exited 0. Test machine bestia, Node 26.8.2; no runtime installation or production credential use. Full CI remains GitHub-owned.
Refresh retry behavior
Concurrent SDK refreshes and immediate retries return the same successor for a fixed five-second window. The successor nonce uses a separate HMAC domain; the server stores only the current and immediate predecessor hashes and the original retry deadline. Retrying does not slide that deadline. Reuse at or after five seconds, or of an older token, revokes the existing session and all its access/refresh authority.
The pinned MCP SDK's real
refreshAuthorizationruns twice concurrently against the real OAuth HTTP routes in the tests, preserving the ceiling. A disk reopen at four seconds retains the same successor; retry at five seconds revokes, and an older-than-immediate token revokes even within the window. These cover both independent review findings without requiring SDK users to write a refresh lock. Final-head Claude Opus 5.5 independent review passed, with no P1/P2 findings; the reviewer inspected code and author logs without running additional tests. The fork registry and allowlist now assign the three new carried upstream edits to #553, and the fork stack check passes locally.UI evidence
Before:

After:

Final head:
d1703f980eac5dfd81fa473635e084693c2882e8, based on mainc8b69c643bfe2fb031ffcd096376a6cd35a9dac8. Independent Claude Opus 5.5 review at medium effort passed on that exact head; the newest review/independent status is success from lukemaj. GitHub CI attempt 2 passed on this unchanged head, including all eight server shards and the aggregate Check. The earlier Grok replay idle assertion passed on the single failed-job rerun, with no source/assertion/timeout change; its original cause remains unknown. Transfer measurements remain within all ceilings with zero measured increase. All applicable checks pass; deployment/native-only jobs are skipped by CI policy. Versions remain unchanged under the fork's AGENTS.md. Website impact: none, this changes the app's OAuth approval route; user guidance and the feature map are updated.Implemented with GPT-6.1-Sol, medium effort, through the Codex harness.