HarperFast / HarperFast/harper
MCP session: an in-flight put can resurrect a client-DELETEd session (unconditional save)
- Dominant language
- JavaScript
- Stars
- 89
- Forks
- 10
- Avg merge
- 2d 6h
- Merged PRs (30d)
- 200
Description
## Summary
An MCP session record can be **resurrected** after a client `DELETE` by an in-flight request on the same session, because the server persists sessions with an unconditional `put` (create-or-update) while `DELETE` is a plain delete. A `put` that lands after the `delete` recreates the row, keeping a terminated session usable until its TTL eviction.
Surfaced by a Codex review of #1350 (the `logging/setLevel` save), but the root is **pre-existing and request-wide** — not specific to logging.
## Where
- `components/mcp/session.ts`
- `saveSession(record)` → `table.put(record)` (unconditional create-or-update)
- `touchSession(record)` → `saveSession({ ...record, lastActivity })`
- `deleteSession(id)` → `table.delete(id)` (gated by `mcp.session.allowClientDelete` at the transport layer)
- `components/mcp/transport.ts` `handlePost` runs `session = await touchSession(session)` on **every authenticated POST**, so the resurrecting `put` is on the hot path for all request types (`tools/list`, `tools/call`, `logging/setLevel`, etc.) — `touchSession` is the first resurrection point in a request; later saves (`handleInitialized`, `dispatchSetLevel`) are subsequent `put`s.
## Race
1. Request A (any POST) loads session S and passes `touchSession` (or is about to `put`).
2. Request B issues `DELETE /mcp` for S → `deleteSession(S)` removes the row.
3. Request A's `put` (touchSession / handleInitialized / setLevel) lands → S is recreated.
4. S is usable again until idle-TTL eviction, despite the explicit client termination.
Only reachable when `mcp.session.allowClientDelete` is enabled; the window is the request's lifetime.
## Proposed fix
Make session persistence non-resurrecting:
- Use a conditional write — update-only-if-exists / compare-and-set — for `saveSession`/`touchSession`, so a `put` after a `delete` is a no-op rather than a recreate. (Check what Harper's `Table` API offers: a conditional update, an existence guard, or a version/`If-Match`-style precondition.)
- Apply at the `session.ts` layer so it covers `touchSession`, `handleInitialized`, and `logging/setLevel` uniformly.
## Acceptance
- [ ] A `put` racing a concurrent `deleteSession` does not recreate the row.
- [ ] `touchSession` / `handleInitialized` / `logging/setLevel` cannot resurrect a deleted session.
- [ ] Regression test covering delete-then-late-save.
## Notes
- TTL is keyed off the record's put timestamp, not the `lastActivity` field, so a resurrected row also gets a fresh idle window — the session genuinely lives on.
- Tracked alongside the broader MCP surface in #1349.
🤖 Drafted by an LLM (Claude) from a Codex review finding; grounded in the linked code. Reviewed by Kyle.
Contributor guide
Assessment
This issue has not been assessed yet.