fix(worker): harden device token revalidation - #1206
Conversation
|
Codex review: found issues before merge. Reviewed July 29, 2026, 1:11 PM ET / 17:11 UTC. ClawSweeper reviewWhat this changesThe Worker caches successful device-token GitHub account and organization/team revalidation for 60 seconds, distinguishes expired or unusable pairing grants from revoked device tokens, and adds documentation and regression coverage. Merge readinessKeep this PR open for an explicit maintainer decision on the authorization boundary: successful remote GitHub membership checks become reusable for 60 seconds, so a remote membership removal is not observed immediately even though local revocation and failed refreshes remain immediate/fail-closed. Priority: P2 Review scores
Verification
How this fits togetherCrabbox’s Worker authenticates device-token requests before exposing lease data and coordinator operations. It combines durable device-token state and local policy with GitHub account and membership checks, then authorizes the request to the fleet coordinator. flowchart LR
A[Device token request] --> B[Durable token and grant checks]
B --> C[Local revoked-user and org policy]
C --> D{Recent successful membership check?}
D -->|Yes, under 60 seconds| E[Reuse bounded authorization result]
D -->|No or expired| F[GitHub account and membership revalidation]
F --> G[Authorize lease and coordinator access]
E --> G
F --> H[Reauth or deny response]
Decision needed
Why: This PR intentionally changes when a remote authorization revocation becomes effective; source tests can establish the bounded behavior but cannot select the repository’s acceptable authorization window. Before merge
Findings
Agent review detailsSecurityNeeds attention: The patch does not add a new secret path or third-party dependency, but it deliberately changes the remote authorization freshness boundary and needs explicit approval. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Obtain explicit maintainer approval for the documented 60-second stale-membership window, retain the immediate local revocation and fail-closed refresh semantics, and remove the release-owned changelog entry before landing. Do we have a high-confidence way to reproduce the issue? Yes, for the proposed behavior: the PR provides a local Wrangler HTTP run showing one GitHub validation on the first request, a cache hit on the second, immediate device revocation, and expired-grant reauthentication. The policy question is not a failing reproduction; it is whether the 60-second authorization window is acceptable. Is this the best way to solve the issue? Unclear until maintainers choose the remote-revocation latency. The cache is narrowly scoped and documented, but making a positive remote authorization result reusable is a security-policy choice rather than a mechanically provable bug fix. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f7c915578a6c. LabelsLabel justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
Summary
pairing_reauth_requiredwhen the stored grant expires, cannot be opened, or GitHub rejects its credential, while device revocation remainsdevice_token_invalidImplements #1200.
Verification
The full Go race fan-out ran on Blacksmith Testbox. Every package outside the Testbox shared-filesystem permission fixture passed. The two affected packages were then rerun locally on the same commit with native filesystem semantics:
The real CLI also built and ran:
Local Wrangler HTTP proof
The Worker ran under local Wrangler and used a real active GitHub/org membership. All credentials remained off argv and were redacted from output.
Source-blind behavior validation passed all four contract clauses: repeated-request caching, immediate warm-token revocation, distinct expired-grant reauthentication, and credential-redacted evidence. Autoreview was clean with no accepted/actionable findings.
Security and configuration
The stored sealed OAuth credential model from device pairing is unchanged. No new secrets or configuration are required. The explicit tradeoff is that removed GitHub org/team membership can remain readable for at most about 60 seconds; lookup failures after cache expiry fail closed, and explicit device-token revocation remains immediate.