HarperFast / HarperFast/oauth

Session invalidation is not atomic vs a concurrent token refresh — logout/expiry can be resurrected (no CAS)

Open
#212 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
JavaScript
Stars
1
Forks
1
Avg merge
2d 16h
Merged PRs (30d)
12

Description

Surfaced during the #211 (logout-invalidation) cross-model review. **Verified in code; not yet reproduced** — needs a PoC before a fix is designed.

### Problem

Session writes are read-modify-write with no optimistic concurrency (no CAS/version check). `session.update(...)` is a full-replace `put` on the `hdb_session` record keyed by `id`, so **last write wins**:

- `src/lib/handlers.ts` `clearOAuthSession` persists `session.update({ user: null })` (logout / expiry).
- `src/lib/sessionValidator.ts` (~L92, ~L169) persists `session.update(session)` (full write-back of refreshed tokens) on the auto-refresh path.

**Race:** request A snapshots the authenticated session and awaits token validation/refresh; request B logs the user out and commits `{ user: null }`; A then completes and full-PUTs its stale snapshot (original non-null `user` + fresh `oauth`) under the same id. The logged-out session is **resurrected** and keeps authenticating. (A refresh-vs-refresh variant can also revoke a just-succeeded refresh.)

### Scope / severity

Narrow timing window; requires a concurrent in-flight refresh at the logout instant. Pre-existing race class (refresh-vs-refresh already has it); #211's logout fix now participates because logout now (correctly) persists. Hardening-class, not a clean default-exploit.

### Direction (to design after reproduction)

Per-session serialization/singleflight on session writes, or optimistic concurrency (version/CAS) on the `hdb_session` put, or a tombstone that stale writers cannot overwrite. Reproduce first (two concurrent requests, one logout + one refresh) to pin the window.

Priority suggestion: P2.

Contributor guide

Open the contributing guide

Research direction

Start in src/lib/handlers.ts at clearOAuthSession and in src/lib/sessionValidator.ts around the refresh write-back paths. Reproduce the stated concurrent logout/refresh race with two requests, then document the result and identify the concurrency behavior needed to prevent a stale full write from restoring the session.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
authentication, backend
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.