feat(cli-auth): sliding board-key renewal + expiry visibility - #579
Conversation
Board API keys expire 30 days after the interactive cli-auth mint and nothing could renew them, so every consumer 401s in lockstep when the TTL lapses (fleet-wide outage on 2026-07-02). - GET /api/cli-auth/me now returns the key's expiresAt so clients can see expiry coming instead of discovering it via 401. - POST /api/cli-auth/refresh lets a still-valid board key extend itself to a fresh 30-day window (no browser challenge; the valid token is the proof of identity). Never shortens long-lived service keys, never resurrects expired/revoked keys, no-ops on never-expires keys. - Abandoned machines keep the old security property: no renewal means the key still dies within 30 days. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
|
Hey @kkroo! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
…rtion Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Looks good — reviewed the full diff (auth middleware, board-auth.ts, access.ts routes, OpenAPI registry, and the new cli-auth-refresh.test.ts suite). No Critical or Important findings.
The core guarded-UPDATE logic (refreshBoardApiKeyExpiry) is sound: isNull(revokedAt) blocks resurrecting revoked keys, gt(expiresAt, now) blocks resurrecting already-expired keys (which can't reach this handler anyway since findBoardApiKeyByToken already excludes them from auth), isNotNull(expiresAt) correctly no-ops never-expires keys, and lt(expiresAt, target) correctly prevents shortening long-lived (365d) keys. Verified against findBoardApiKeyByToken/getBoardApiKeyForUser in board-auth.ts — the WHERE-clause scoping matches how the token was authenticated, so there's no cross-user key-refresh path. Test suite's six cases line up with the actual guard conditions.
Suggestions (3)
- [code]
server/src/routes/access.ts:2766-2767— Whenrefreshedis null (never-expires or long-lived key), the handler does a second DB round-trip viagetBoardApiKeyForUserto report the current expiry.req.actor.keyExpiresAtwas already populated byactorMiddlewarefrom the same key row that authenticated this request, so it can be used directly on the "not refreshed" branch instead of re-querying. - [quality]
server/src/routes/access.ts:2735/server/src/services/board-auth.ts:157— No rate limit or per-key debounce onPOST /cli-auth/refresh. Since the sliding-window guard (expiresAt < now + 30d) re-satisfies on almost every call once a key is in its renewal window, a tight retry loop would fan out aboard_api_key.refreshedlogActivitywrite per company membership on every request (existing pattern fromboard_api_key.created/.revoked, so not a new class of risk, but this is a hotter path than those). A cheap debounce (e.g. skip re-refresh if last refresh was <1h ago) would keep the audit log from getting noisy under a misbehaving client. - [tests]
server/src/__tests__/cli-auth-refresh.test.ts— Good coverage of the guard conditions, but there's no test for calling/cli-auth/refreshtwice in quick succession to confirm the second call still extends further (documenting the sliding-window-under-repeat behavior noted above) or a test asserting/me'sexpiresAtisnullfor non-board-key (session) actors.
Strengths
- Zero-extra-query design for
/me'sexpiresAt— reuses the key row already selected during auth instead of adding a query. - Guarded conditional UPDATE is the right primitive here: atomic, race-safe, and self-documenting via the WHERE clause rather than a read-then-write.
resolveBoardActivityCompanyIds+ per-companylogActivityfan-out matches the existing convention used byboard_api_key.created/.revoked, so the audit trail stays consistent.
Recommended Action
- No blockers — safe to merge as-is.
- Consider the
keyExpiresAtreuse and debounce suggestions opportunistically.
Thinking Path
Linked Issues or Issue Description
No pre-existing issue — incident-driven. Inline issue description following
.github/ISSUE_TEMPLATE/bug_report.yml:What happened?
All board-key consumers began receiving
401 {"error":"invalid or expired token"}from the ccrotate auth proxy and board API on 2026-07-02 ~10:28–12:38 UTC. Every board API key minted in a June-2 rotation hit its hard 30-day TTL in the same window; there is no renewal API, no expiry surface on/api/cli-auth/me, and recovery requires the interactive browser challenge flow on every affected machine. Impact: ccrotate model proxy, all paperclip-fronted MCP servers, the penstock-serve-anthropic vault credential, and k8s opencode agents.Expected behavior
Actively-used credentials should not mass-expire with no warning and no non-interactive recovery path — clients should be able to see expiry coming and renew a still-valid key.
Steps to reproduce
/api/cli-auth/megave no advance signal.Paperclip version or commit
432f0de78(master, 2026-07-02)What Changed
GET /api/cli-auth/menow returns the board key'sexpiresAt(null for session actors and never-expires keys); the auth middleware carrieskeyExpiresAton the actor from the key row it already selects — zero extra queriesPOST /api/cli-auth/refresh(board-key bearers only): sliding renewal to a fresh 30-day window via a single guarded conditional UPDATE — never shortens keys that already outlive a fresh TTL (365d service-account keys pass through), never resurrects expired/revoked keys, no-ops on never-expires keysboard_api_key.refreshedBOARD_ONLY_OPERATIONSupdated for the new routeVerification
server/src/__tests__/cli-auth-refresh.test.ts(embedded postgres, realactorMiddleware+accessRoutes, six cases:/meexpiry surface; near-expiry extend + activity log + DB assertion; never-shorten long-lived key; never-expires no-op; expired-token 401; non-board-key actor 400)pnpm run typecheckclean; openapi route-consistency tests passRisks
expiresAtadded to/me)Model Used
Claude Fable 5 (claude-fable-5) via Claude Code
🤖 Generated with Claude Code