Skip to content

api: keep earlier attach tokens valid when a session create is replayed - #809

Closed
NancyKwongPerplexity wants to merge 2 commits into
session-tokens-tablefrom
session-replay-adds-token
Closed

NancyKwongPerplexity wants to merge 2 commits into
session-tokens-tablefrom
session-replay-adds-token

Conversation

@NancyKwongPerplexity

@NancyKwongPerplexity NancyKwongPerplexity commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Proposal, pending agreement on the replay-versus-renewal contract. The review of #79 and the review of #91 asked that command-create replay be defined separately from credential renewal, without committing to overlapping validity, and that the smallest implementation of that contract be chosen. This PR implements one option for discussion. Alternatives:

  • an explicit credential-renew endpoint, leaving command-create replay unchanged;
  • a one-generation previous_token_hash grace column, so only the token before the latest stays valid.

The revoked-session guard alone is split out as #811.

With this change, replaying a data-plane session's create with the same idempotency key adds a token instead of replacing the session's only one. Tokens returned earlier stay valid until their own expiry. Replaying the create of a revoked or closed session returns 409 conflict instead of a token that cannot be used.

At the moment a session keeps one attach token, in data_plane_sessions.token_hash. A replay (same method, path, body and key, within the key's 24 hours) mints a new token and overwrites that hash. Every token returned earlier stops working immediately:

  • Ten concurrent same-key replays of a command create leave nine callers holding tokens that fail with a non-retryable 401.
  • Port and protocol sessions check the token on every proxied request. A caller that renews a port-session token by replaying the key before it expires gets 401s on requests still in flight on the previous token.
  • Replay does not check the session's status, so replaying a revoked session's create returns 201 and a token.

What changes:

  • Issuing (API). Create and replay each insert a data_plane_session_tokens row (expires_at = now + 5 minutes) in the statement that writes the session, inside the existing idempotency lock. Replay then deletes the session's expired rows and keeps at most 16 unexpired tokens (MAX_LIVE_ATTACH_TOKENS), evicting those that expire first and never the one it returns. Both still write token_hash/token_expires_at as the latest token. Create and replay are still six statements each.

  • Revoked or closed sessions. The replay's refresh only matches a session with revocation_status = 'active' and a state other than closed, revoked or failed. When nothing matches, the replay returns the existing conflict code (409, retryable: false) with the detail {"key": "reason", "value": "session_revoked"}, and no new error code is added. The row lock taken by that update also orders a replay against a concurrent revoke.

  • Revocation. revokeSession deletes the session's token rows in the transaction that clears the column.

  • Validation (internal API). Route resolution looks up the presented token's row and uses that token's own expiry, both for attach and for the per-request port/protocol ingress check. A token without a row falls back to the existing constant-time comparison against token_hash/token_expires_at. A row counts only while token_hash is set, so a revoke performed by a server that predates the table (it clears only the column) still ends every token.

  • Docs. docs/api/index.md gains "Renew session tokens":

    • replaying adds a 5-minute token, and earlier ones stay valid until their own expires_at
    • a session keeps at most 16 unexpired tokens
    • attached streams are not cut at token expiry, but reconnects and each port/protocol request need an unexpired token
    • renewal ends when the session expires, 24 hours after creation
    • replaying a revoked or closed session returns the 409

    docs/native-api-hardening.md describes this as a proposed contract pending that agreement, links both reviews, and lists the alternatives above.

  • Smoke. native_contract.command_replay_credentials asserts that the original credential still attaches after a replay. It is ignored on main pending the contract decision. This proposal runs it in normal selection, which would commit the API to the overlap guarantee. Its assertions are unchanged.

Validation

At 652743e (on #808). The head since adds only the documentation rewording in 623c5ae:

  • cargo fmt --all --check: pass.
  • cargo clippy -p pplx-space-api -p pplx-space-internal-api-server -p pplx-space-public-api -p pplx-space-db-migrate --all-features --all-targets -- -D warnings: pass.
  • cargo nextest run --config 'profile.test.package.pplx-space-api.opt-level=0' --all-features -p pplx-space-api -p pplx-space-internal-api-server -p pplx-space-public-api -p pplx-space-db-migrate with nextest's Postgres and Redis: 2385 passed. The one skipped test is the #[ignore]d benchmark_single_node_occupancy.
  • cargo arch with the base branch's baseline and ARCHITECTURE.toml: 0 required findings. It reports one advisory: crates/libs/public/api/src/lib.rs grew past its recorded size, partly from the two detail constants added here.
  • python3 docs/book/test.py after scripts/build-guide-docs.sh: pass. space_smoke/tests: 568 passed, 1 skipped.

New tests:

  • pplx-space-api:
    • 10 concurrent replays each add a distinct live token, and the original stays live.
    • Replay prunes an expired row and keeps the 16 newest of 21 tokens.
    • Revoke deletes every token, and the next replay is refused and mints nothing.
    • Replaying a closed, revoked or failed session is refused.
    • The refusal renders as a non-retryable 409 conflict with the session_revoked detail.
  • pplx-space-internal-api-server: route resolution accepts the latest and an earlier table token. It rejects an expired token, another session's token and an unknown token. It accepts a column-only token (the mixed-version fallback) and rejects every token once the column is cleared (an older server's revoke).

Each new test was mutation-checked by breaking the code it covers, one mutation at a time. 14 of 15 mutations fail a new test. These include the token insert on create and on replay, the status guard, the revoke delete, the cap bound, expired-row pruning, protection of the returned token, the detail value, the table lookup and each of its three predicates, the column fallback, and using the row's own expiry. The surviving mutation feeds the ingress check the session row's expiry instead of the presented token's. Attach validation has already rejected an expired presented token, and the row always holds the latest expiry, so in practice the two behave the same and no test distinguishes them.

E2E on Kind: a local cluster per docs/installation.md (install-fc-vendor.sh, bootstrap.sh). It was first built entirely from main (da5bfd2), then redeployed from this branch, and the migrator applied #808. A disposable script drove the public API on 127.0.0.1:8080/:8090:

Check main this branch
Command session, key replayed 10× concurrently, attach with each replay's token 1 × 200, 9 × 401 10 × 200
Same session, attach with the original create's token 401 200
Port session, request loop on the original token while the key is replayed every 5 s (8 renewals in 40 s) 200 until the first renewal at t=5.0 s, then 401 (731 of 806 requests) 200 for all 626 requests
Port session, a request with each renewed token afterwards only the last works (7 × 401) 8 × 200
Replay a revoked command session's create 201, state: revoked, with a token 409 conflict, retryable: false, details: [{key: reason, value: session_revoked}]
Three tokens from a create and two replays, attach after revoking the session not run all three 401, immediately and 2 s later. In an earlier run, one token that had been resolved under a second before still got a 200 from the route cache.

native_contract.command_replay_credentials fails on main for the expected reason (original_status: 401, replay_status: 200) and passes here.

Typed smoke on the branch cluster: bash scripts/tests/smoke-kind-typed.sh native ran 27 scenarios, all passed (SUITE: 27 passed, 0 warned, 0 failed, 0 inconclusive). These include native.m1_generated_sdk_conformance, native_contract.token_scope, native_contract.command_replay_credentials, native_concurrency.command_idempotency and the native recovery scenarios.

Not validated: the 409 for a closed or failed session through the API (no public path sets those states, so only the store test covers it); a rolling deploy with old and new pods serving together (covered by the fallback tests, not on Kind).

Severity and risk

Severity: moderate. Concurrent or renewing callers receive tokens that are rejected with a non-retryable 401, and port/protocol traffic on a token that was just renewed fails. Sessions are not exposed across tenants or after revocation. On the revoked-replay path the current code hands out a token for a dead session.

Risk: the create, replay and revoke statements change. A bug there affects every native command, terminal, port and protocol session create. Token validation now reads the presented token's row first. A wrong rejection would surface as data-plane 401s, and a wrong acceptance is limited to tokens minted for the same session. Callers that replayed a revoked session's create and used the 201 now get a 409. To roll back, revert this PR. Tokens minted only into the table then stop working, while the latest token per session keeps working through the column.

Rollout

Deploy #808 first. Old and new releases interoperate in either order. An old validator honours only the latest token (the current behavior). An old issuer's tokens validate through the column fallback. A revoke from an old server ends every token, because rows count only while the column is set. Revoking a session from a new server ends every token at once. A token resolved in the last second can still be served from the data plane's one-second route cache, as it can today.

Stack

  1. db: add a table of data-plane session attach tokens #808: the data_plane_session_tokens table (base main).
  2. This PR, based on db: add a table of data-plane session attach tokens #808. Merge after it.

#674 (signed route handover) compares the presented token against the stored token_hash. It needs the same lookup, or tokens from earlier replays will be rejected on that path.

Not covered

  • Dropping data_plane_sessions.token_hash/token_expires_at and the column fallback, in a later migration once no deployed release reads or writes them. No issue is filed yet.
  • The tokenless data-plane route refresh still bounds port/protocol ingress by the session's latest token expiry, as it does today.

AI assistance: Claude Code drafted the code, tests, and this description; the author reviewed them.

A data-plane session stored one attach token, so replaying its create with
the same idempotency key overwrote the token every earlier response
returned. Concurrent same-key replays left all but one caller with a token
that failed with a non-retryable 401, and renewing a port session's token
by replay could 401 requests still in flight on the previous one. Replay
also ignored the session's status and returned a token for a revoked
session.

Create and replay now each add a row to data_plane_session_tokens with its
own five-minute expiry, in the statement that writes the session and under
the existing idempotency lock. Replay deletes expired rows and keeps at
most 16 unexpired tokens per session, evicting those that expire first but
never the one it returns. The session row still names the latest token so
validators that predate the table keep working.

Route resolution uses the presented token's row and its own expiry for
attach and for each port or protocol request, and falls back to the
session row's token for tokens minted before the table. A row counts only
while the session row still names a token, so a revoke by an older server
still ends every token. Revoking deletes the session's rows. Replaying the
create of a revoked or closed session returns the existing conflict code
with the detail reason=session_revoked instead of a token.

The replay-credential smoke scenario, parked until this contract was
decided, runs in normal selection again.
Overlapping credential validity after a replay is not yet agreed. Mark the
section and its coverage row as a proposed contract, link the reviews that
deferred the decision, and list the alternatives: an explicit renew
endpoint, or a single previous-token grace column.
@NancyKwongPerplexity

Copy link
Copy Markdown
Collaborator Author

closing: the replay-vs-renewal contract stays as decided on #79/#91, and the clear bug from this work landed separately in #811 (a replay of a revoked or ended session now gets a 409). can reopen if we revisit renewal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant