Repository navigation
fix(desktop): never replay a refresh token after a timeout; jitter relay lease renewal - #18719
Conversation
…lay lease renewal
Two independent desktop hardening changes that share one incident date.
1. Refresh-token replay. A refresh POST that aborts client-side leaves the
token's fate unknown: the server may already have rotated it before the
reply was lost. Every caller above this module (the relay auth coordinator
most of all) then re-entered and resent the same stored token, which the
server reads as reuse and answers by revoking the whole token family. On
2026-09-04 that turned a slow refresh endpoint into 21,605 sign-outs.
- The refresh endpoint now gets one 60s attempt instead of a 30s attempt
plus a replayed retry.
- A failure with no status line is ambiguous: it is never retried, and the
token is recorded so a re-entry within 30s is refused outright rather
than replayed. The block is bounded, not permanent -- the token is only
possibly spent, and a permanent block would sign out every desktop whose
refresh merely timed out.
- Before any retry or replay the stored session is re-read; if another
caller already rotated it, that result is adopted and the old token is
never sent again.
- Only a definitive 5xx, which proves the server rejected without
rotating, is retried, and exactly once.
- A 401 on a token whose earlier attempt never answered now logs
orca_cloud_refresh_possible_replay so support can tell a real revocation
from a sign-out we caused ourselves. clearCloudSessionIfUnchanged
semantics are unchanged and still emit the invalidation event.
2. Lease renewal jitter. Both desktop renewal timers took the mean of a
60s-wide window before expiry. A cell recreate reconnects a whole cohort
inside one second, so every host in it renewed inside the same second ~54
minutes later, re-bursting the fleet every ~54 minutes. Renewal now
carries full +/-10% jitter, spreading the same cohort over ~10 minutes.
Early renewal is free on the server side: the rebind branch resets the
full 55-minute TTL from whenever it arrives, with no minimum-age or
early-renewal restriction (cloud/apps/relay/src/host-session-registry.ts
:736-743, and :794 for a fresh session). Only lateness is fatal -- :997
drains and closes a lease that has expired. The base interval is therefore
shrunk to fit the upward jitter rather than clipping the jittered value at
the margin, which keeps the distribution unbiased and keeps every sample at
least 90s before expiry. No wire change.
📝 WalkthroughWalkthroughThe cloud client now supports request options, configurable timeouts, and ambiguous failure classification. Session refresh logic records ambiguous attempts, blocks short-term token replay, retries definitive 5xx responses once, handles sessions rotated elsewhere, and warns on possible replay. Relay renewal scheduling now uses a shared jitter helper with a safety margin and bounded randomness. Tests cover refresh replay behavior and renewal timing. Merge Risk: 🟡 Moderate · up to A concurrent session removal can still lead to reuse of a stale refresh token, potentially revoking the token family and forcing reconnection. This should be fixed before merge. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description clearly explains the problem, implementation, scope, and tests. However, it omits several required template sections, including the linked issue, Visual Proof or an explicit N/A rationale, AI Disclosure, Review, Agent skill upstream boundary, Notes, and the Checklist. Resolution Add all missing template sections. Include a valid
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Team
Run ID: 239b71e8-0f2e-41f4-b76d-ce5237275166
📒 Files selected for processing (10)
src/main/orca-profiles/profile-cloud-client.tssrc/main/orca-profiles/profile-cloud-refresh-replay-guard.tssrc/main/orca-profiles/profile-cloud-service-auth-retry.test.tssrc/main/orca-profiles/profile-cloud-service-refresh.test.tssrc/main/orca-profiles/profile-cloud-session-refresh.test.tssrc/main/orca-profiles/profile-cloud-session-refresh.tssrc/main/runtime/relay/relay-origin-pool.tssrc/main/runtime/relay/relay-renewal-jitter.test.tssrc/main/runtime/relay/relay-renewal-jitter.tssrc/main/runtime/relay/relay-session-broker.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 5 remain after this review.
| const current = readOrcaCloudSession(active.profile.id, userDataPath) | ||
| if (current.status === 'found' && current.session.refreshToken !== session.refreshToken) { | ||
| return { status: 'rotated-elsewhere', session: current.session } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Stop the retry when the stored session no longer exists.
If the first refresh returns 5xx and another caller clears or invalidates the session, readOrcaCloudSession can return a status other than found. Lines 143-145 then fall through and issue the retry with session.refreshToken. The mutation snapshot can reject the later save, but it cannot prevent the second credential-bearing request.
Treat every non-found reread as a stale or reconnect-required session before retrying. Add a test for a 503 followed by a missing session-store result.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The replay-protection core is sound and well-tested, but two gaps undercut the PR's own stated goals: the possible-replay diagnostic never reaches disk, and the org-selection refresh path still replays unguarded tokens. Both are detailed below.
Reviewed changes
- Refresh-token replay guard — adds a bounded 30s replay block, a single definitive-5xx retry, and a store re-read to
profile-cloud-session-refresh.ts, plus a 60s refresh timeout (vs 30s for other calls) inprofile-cloud-client.ts. - Possible-replay diagnostics — logs
orca_cloud_refresh_possible_replayon a 401 that follows an ambiguous attempt, without printing the token. - Relay renewal jitter —
relayRenewalDelayMsreplaces the 60s-wide renewal window in both the control-lease and relay-JWT schedulers with full ±10% jitter.
⚠️ The replay guard and single-flight don't cover the direct refresh in org selection
blocksAmbiguousRefreshReplay and the inflightCloudSessionRefreshes single-flight both live inside refreshStoredCloudSession, so only refreshes that flow through readFreshOrcaCloudSession are protected. selectCloudOrgWithMutationFence calls refreshOrcaCloudSession directly on a 401 (profile-cloud-org-selection.ts:48). A timeout there re-throws without recording an ambiguous attempt and leaves the stale token in the store, so a later guarded refresh will still POST that same token once — and a concurrent guarded refresh is not single-flighted against it either. This is the same replay the PR is meant to prevent, just through a lower-frequency entry point.
Technical details
# org-selection refresh bypasses replay guard and single-flight
## Affected sites
- src/main/orca-profiles/profile-cloud-org-selection.ts:48 — direct `refreshOrcaCloudSession` call on 401
- src/main/orca-profiles/profile-cloud-session-refresh.ts:117-152 — `attemptCloudSessionRefresh` records/guards only this path
## Required outcome
- Every refresh-token POST that can race a rotation is either recorded as ambiguous on timeout or single-flighted with the guarded path, so a lost reply cannot become a later replay of the same token.
## Suggested approach
- Route the org-selection 401 recovery through `refreshStoredCloudSession` (or a shared single-flight + record wrapper) rather than `refreshOrcaCloudSession` directly.
## Open questions for the human
- Is a concurrent/delayed replay through org selection in scope for this fix, or acceptable as a documented follow-up? It is far lower frequency than the relay-coordinator loop that caused the 21,605 sign-outs.DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
| if (!wasRefreshTokenAmbiguouslyAttempted(key, failed.refreshToken)) { | ||
| return | ||
| } | ||
| console.warn( |
There was a problem hiding this comment.
console.warn never reaches an on-disk artifact in a packaged build — main-process logs have no file tee and only surface via Console.app / --enable-logging. So the PR's stated goal, "support can now separate a genuine revocation from a sign-out we caused ourselves", is not actually achievable from this line. Persist the marker through recordDurableCrashBreadcrumb (or the observability tracer) instead.
Technical details
# orca_cloud_refresh_possible_replay is not persisted
## Affected sites
- src/main/orca-profiles/profile-cloud-session-refresh.ts:104-106 — `warnIfPossibleRefreshReplay` emits the marker via `console.warn` only
## Required outcome
- The possible-replay marker reaches the on-disk diagnostics bundle so support can correlate it with a sign-out.
- The token itself is never persisted (matches the PR's "never log the token" constraint).
## Suggested approach
- Use `recordDurableCrashBreadcrumb('orca_cloud_refresh_possible_replay', { ... })` from `src/main/crash-reporting/durable-crash-breadcrumb.ts`, which writes a redacted span to the NDJSON trace and flushes it. Do not include the refresh token or profile id in the data payload.
…lay lease renewal (stablyai#18719) Two independent desktop hardening changes that share one incident date. 1. Refresh-token replay. A refresh POST that aborts client-side leaves the token's fate unknown: the server may already have rotated it before the reply was lost. Every caller above this module (the relay auth coordinator most of all) then re-entered and resent the same stored token, which the server reads as reuse and answers by revoking the whole token family. On 2026-09-04 that turned a slow refresh endpoint into 21,605 sign-outs. - The refresh endpoint now gets one 60s attempt instead of a 30s attempt plus a replayed retry. - A failure with no status line is ambiguous: it is never retried, and the token is recorded so a re-entry within 30s is refused outright rather than replayed. The block is bounded, not permanent -- the token is only possibly spent, and a permanent block would sign out every desktop whose refresh merely timed out. - Before any retry or replay the stored session is re-read; if another caller already rotated it, that result is adopted and the old token is never sent again. - Only a definitive 5xx, which proves the server rejected without rotating, is retried, and exactly once. - A 401 on a token whose earlier attempt never answered now logs orca_cloud_refresh_possible_replay so support can tell a real revocation from a sign-out we caused ourselves. clearCloudSessionIfUnchanged semantics are unchanged and still emit the invalidation event. 2. Lease renewal jitter. Both desktop renewal timers took the mean of a 60s-wide window before expiry. A cell recreate reconnects a whole cohort inside one second, so every host in it renewed inside the same second ~54 minutes later, re-bursting the fleet every ~54 minutes. Renewal now carries full +/-10% jitter, spreading the same cohort over ~10 minutes. Early renewal is free on the server side: the rebind branch resets the full 55-minute TTL from whenever it arrives, with no minimum-age or early-renewal restriction (cloud/apps/relay/src/host-session-registry.ts :736-743, and :794 for a fresh session). Only lateness is fatal -- :997 drains and closes a lease that has expired. The base interval is therefore shrunk to fit the upward jitter rather than clipping the jittered value at the margin, which keeps the distribution unbiased and keeps every sample at least 90s before expiry. No wire change.
Two small main-process hardening changes that share one incident date.
A. Never replay a refresh token after an ambiguous attempt
Why. A refresh POST that aborts client-side leaves the token's fate unknown: the server may already have rotated it before the reply was lost. Every caller above
profile-cloud-session-refresh.tsthen re-entered and resent the same stored token — most damaginglyRelayAuthCoordinator.scheduleRetry, whoseshouldRetryRelayConnectionErrortreats any non-RelayHttpError(including a refreshTimeoutError) as retryable and re-runsreadContext()→readFreshOrcaCloudSession(). The server reads the second POST as reuse and answers by revoking the whole token family. On 2026-09-04 that turned a slow refresh endpoint into 21,605 sign-outs.What changed.
CLOUD_REFRESH_TIMEOUT_MS = 60_000; every other cloud call keeps the 30 s deadline.postJsonnow takes an options object so the two can differ.isAmbiguousCloudRequestFailureis true for anything that is not anOrcaCloudRequestError: only a status line proves the server rejected the request without consuming what was in it. An abort, a dropped socket, or a 200 we could not parse all leave the token possibly spent.profile-cloud-refresh-replay-guard.ts. A re-entry with that same token insideAMBIGUOUS_REFRESH_REPLAY_DELAY_MS(30 s) is refused withorca_cloud_refresh_replay_blockedinstead of replayed — that window is exactly where the coordinator's exponential backoff would otherwise fire several times. The block is bounded, not permanent: the token is only possibly spent, and a permanent block would sign out every desktop whose refresh merely timed out.[orca-cloud] orca_cloud_refresh_possible_replay: refresh rejected 401 for a token whose earlier attempt never answered. No token is ever printed. Support can now separate a genuine revocation from a sign-out we caused ourselves.Semantics kept.
clearCloudSessionIfUnchangedstill only clears when the store still holds the failed token, still tombstones, and still firesemitOrcaCloudSessionInvalidated()(PR #18694). It additionally forgets the ambiguous-attempt record, so a later unrelated 401 is not mislabelled. The 401 path is untouched for callers: no extra store read, no behavior change for the existing "stays silent when a concurrent rotation already replaced the failed session" case.B. Jitter the control-lease renewal
Why. Both desktop renewal timers computed
earlyMs = 60_000 + random()*60_001— a 60 s-wide window before expiry. A cell recreate reconnects a whole cohort inside one second, so every host in it took its lease from the same second and renewed inside the same second ~54 minutes later, re-bursting the fleet every ~54 minutes (the 1006/4408 cohort waves).relayRenewalDelayMsin the newrelay-renewal-jitter.tsapplies full ±10 % jitter, spreading the same cohort over ~10 minutes. It is used by bothRelayOriginPool.scheduleControlRotation(the 55-minute control lease) andRelaySessionBroker.scheduleRefresh(the relay-JWT authorization), which are cohort-synchronized for the same reason.Server acceptance window — early renewals are accepted. In
cloud/apps/relay/src/host-session-registry.ts::736-743— the rebind branch (if (rebind && existing)) setsexisting.leaseExpiresAt = this.now() + 55 * 60 * 1000unconditionally. There is no minimum-age check, no "too early" rejection, and no cap: a rebind at any point in the lease resets the full 55-minute TTL from when it arrives.:794— a fresh host session gets the same55 * 60 * 1000lease, so both paths share one TTL.:997— the only lease enforcement is in the heartbeat:if (now > session.leaseExpiresAt)sendsdrainand closes withDRAINING. Lateness is fatal; earliness is free.That asymmetry is why the base interval is shrunk to fit the upward jitter (
base = (remaining - margin) / 1.1) rather than clipping the jittered value at the margin: clipping would bias the distribution toward the margin, while shrinking keeps it unbiased and keeps every sample at leastRELAY_RENEWAL_SAFETY_MARGIN_MS(90 s) before expiry. On a 55-minute lease that renews at ~49.1 min ±10 % (44.2–54.0 min), versus 53–54 min today; the cost is one extra rebind per ~9 leases, which the server charges nothing for.No wire change. Nothing new is sent or parsed; only the local timer changes.
Tests
src/main/orca-profiles/profile-cloud-session-refresh.test.ts(fake refresh client):refreshOrcaCloudSessionstill called onceorca_cloud_refresh_possible_replay, still clears viaclearCloudSessionIfUnchanged, still emits the invalidation eventsrc/main/runtime/relay/relay-renewal-jitter.test.ts:random()0 / 0.5 / 1), immediate renewal once inside the margin, delay measured fromnowNo branch depends on
process.platform. No renderer copy, so no i18n catalogs touched.Gates
pnpm tc·pnpm typecheck:tsc:node·pnpm test src/main/orca-profiles src/main/runtime/relay src/main/artifacts/artifact-cloud-service.test.ts src/main/skills(870 passed) ·pnpm run check:code-quality:changed(0 new findings) ·pnpm run check:react-doctor:changed(0 issues) — all green.