mirror of https://github.com/garrytan/gstack.git
feat(browse): pty-session-lease registry — stable sessionId + lease lifecycle
Foundation for Commit 2 of the long-lived-sidebar PR. Separates two
concerns that pre-v1.44 were conflated under one token:
* sessionId — stable, non-secret identifier for a single PTY session.
Safe to log, safe in URLs, safe in DevTools. Identifies "this terminal,"
not "you're allowed to use this terminal."
* lease — server-side bookkeeping that maps sessionId → expiresAt.
Re-attach within the lease window resumes the same PTY; expiry tears
it down.
The companion attach-token primitive (short-lived 30s bearer) reuses the
existing browse/src/pty-session-cookie.ts module unchanged — the lease
adds a name-space alongside, it doesn't replace anything.
Codex outside-voice (T1 of the eng review) flagged the original D4
"token IS sessionId" design as conflating identity with auth. The fix
is this lease registry: re-attach URLs carry the stable sessionId
(loggable), the short-lived attachToken stays out of logs.
API:
* mintLease() → { sessionId, expiresAt }
* validateLease(sessionId) → { ok: true, expiresAt } | { ok: false }
* refreshLease(sessionId) — validate-first, never resurrects expired
leases. Security-critical: the 30-min TTL is what bounds blast
radius for a leaked attachToken whose lease should have GC'd.
* revokeLease(sessionId) — explicit dispose path.
* leaseCount() — observability helper.
* __resetLeases() — test-only.
TTL env knob (GSTACK_PTY_LEASE_TTL_MS) lets v1.44 e2e tests compress
the detach window to 1s instead of waiting 30 minutes per assertion.
Server.ts wiring + /pty-session shape change + /pty-restart + /pty-dispose
+ /pty-session/reattach all land in subsequent commits in this branch.
Test (browse/test/pty-session-lease.test.ts):
* 8 cases pinning mint uniqueness, validate-first refresh contract,
revoke idempotency, null/undefined tolerance, and the negative case
that refresh never resurrects a revoked lease (same code path as
expired-and-pruned).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
5d648e4568
commit
3aada48bf9
|
|
@ -0,0 +1,137 @@
|
|||
/**
|
||||
* PTY session lease registry (v1.44+).
|
||||
*
|
||||
* Separates two concerns that pre-v1.44 were conflated under one token:
|
||||
*
|
||||
* - **sessionId** — stable, non-secret identifier for a single PTY session.
|
||||
* Safe to log, safe to include in URLs and server access logs, safe to
|
||||
* keep in DevTools. Identifies "this terminal," not "you're allowed to
|
||||
* use this terminal."
|
||||
*
|
||||
* - **attachToken** — secret, short-lived (30 s) bearer credential that
|
||||
* grants the WS upgrade for ONE attach attempt against a session. Minted
|
||||
* on every /pty-session and /pty-session/reattach call; revoked when
|
||||
* the WS upgrade consumes it. Kept out of logs.
|
||||
*
|
||||
* - **lease** — server-side bookkeeping that maps sessionId → expiresAt.
|
||||
* Re-attach within the lease window resumes the same PTY (and replays
|
||||
* the ring buffer from terminal-agent). Lease expiry tears down the
|
||||
* session.
|
||||
*
|
||||
* Codex outside-voice (T1 of the eng review) pushed for this separation:
|
||||
* "the auth token IS the session id" collapsed identity into a secret,
|
||||
* meaning re-attach URLs and logs carry the bearer credential. The lease
|
||||
* model fixes that without changing the user experience.
|
||||
*
|
||||
* Mint cadence:
|
||||
* - Initial /pty-session: mint sessionId + lease + attachToken (one round trip).
|
||||
* - /pty-session/reattach: validate sessionId/lease, mint fresh attachToken.
|
||||
* - /pty-restart: revoke old lease, mint fresh sessionId + lease + attachToken.
|
||||
* - /pty-dispose: revoke lease (and the terminal-agent disposes the PTY).
|
||||
*
|
||||
* Lease TTL is env-overridable so v1.44 e2e tests can compress detach
|
||||
* windows to 1 s instead of waiting 30 minutes per assertion.
|
||||
*/
|
||||
import * as crypto from 'crypto';
|
||||
|
||||
interface Lease {
|
||||
createdAt: number;
|
||||
expiresAt: number;
|
||||
}
|
||||
|
||||
const LEASE_TTL_MS = parseInt(
|
||||
process.env.GSTACK_PTY_LEASE_TTL_MS || `${30 * 60 * 1000}`,
|
||||
10,
|
||||
); // 30 minutes default; covers idle-but-engaged user sessions
|
||||
const MAX_LEASES = 10_000;
|
||||
const leases = new Map<string, Lease>();
|
||||
|
||||
/**
|
||||
* Mint a fresh sessionId + lease. Returns the non-secret sessionId and
|
||||
* the expiry timestamp (caller surfaces both to the client). Never throws.
|
||||
*/
|
||||
export function mintLease(): { sessionId: string; expiresAt: number } {
|
||||
const sessionId = crypto.randomBytes(32).toString('base64url');
|
||||
const now = Date.now();
|
||||
const expiresAt = now + LEASE_TTL_MS;
|
||||
leases.set(sessionId, { createdAt: now, expiresAt });
|
||||
pruneExpired(now);
|
||||
return { sessionId, expiresAt };
|
||||
}
|
||||
|
||||
/**
|
||||
* Check whether a lease is still valid (exists AND not expired). Returns
|
||||
* the current expiresAt for valid leases; null otherwise. Lazily prunes
|
||||
* stale entries.
|
||||
*/
|
||||
export function validateLease(sessionId: string | null | undefined): { ok: true; expiresAt: number } | { ok: false } {
|
||||
if (!sessionId) return { ok: false };
|
||||
const lease = leases.get(sessionId);
|
||||
if (!lease) {
|
||||
pruneExpired(Date.now());
|
||||
return { ok: false };
|
||||
}
|
||||
if (Date.now() > lease.expiresAt) {
|
||||
leases.delete(sessionId);
|
||||
pruneExpired(Date.now());
|
||||
return { ok: false };
|
||||
}
|
||||
return { ok: true, expiresAt: lease.expiresAt };
|
||||
}
|
||||
|
||||
/**
|
||||
* Extend the lease's expiresAt to `now + LEASE_TTL_MS`. Caller should
|
||||
* gate refresh on `expiresAt - now < REFRESH_THRESHOLD` (D10 lazy
|
||||
* refresh: avoid refreshing on every keepalive when the lease is
|
||||
* comfortably far from expiry).
|
||||
*
|
||||
* Returns `{ ok: true, expiresAt }` on success, `{ ok: false }` if the
|
||||
* lease is unknown or already expired (the agent must close the WS and
|
||||
* surface auth-invalid). Critical security invariant: never resurrect
|
||||
* an expired lease — the 30-min TTL is what bounds blast radius for a
|
||||
* leaked attach token whose lease should have been GC'd.
|
||||
*/
|
||||
export function refreshLease(sessionId: string | null | undefined): { ok: true; expiresAt: number } | { ok: false } {
|
||||
if (!sessionId) return { ok: false };
|
||||
const lease = leases.get(sessionId);
|
||||
if (!lease) return { ok: false };
|
||||
const now = Date.now();
|
||||
if (now > lease.expiresAt) {
|
||||
leases.delete(sessionId);
|
||||
return { ok: false };
|
||||
}
|
||||
lease.expiresAt = now + LEASE_TTL_MS;
|
||||
return { ok: true, expiresAt: lease.expiresAt };
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop a lease. Called on explicit dispose (/pty-dispose, /pty-restart,
|
||||
* WS close with code 4001) and on session timeout in terminal-agent.
|
||||
*/
|
||||
export function revokeLease(sessionId: string | null | undefined): void {
|
||||
if (!sessionId) return;
|
||||
leases.delete(sessionId);
|
||||
}
|
||||
|
||||
/** Returns the lease count — test + observability helper. */
|
||||
export function leaseCount(): number {
|
||||
return leases.size;
|
||||
}
|
||||
|
||||
/** Test-only reset. */
|
||||
export function __resetLeases(): void {
|
||||
leases.clear();
|
||||
}
|
||||
|
||||
function pruneExpired(now: number): void {
|
||||
let checked = 0;
|
||||
for (const [sessionId, lease] of leases) {
|
||||
if (checked++ >= 20) break;
|
||||
if (lease.expiresAt <= now) leases.delete(sessionId);
|
||||
}
|
||||
while (leases.size > MAX_LEASES) {
|
||||
const first = leases.keys().next().value;
|
||||
if (!first) break;
|
||||
leases.delete(first);
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,98 @@
|
|||
import { describe, test, expect, beforeEach } from 'bun:test';
|
||||
|
||||
// pty-session-lease registers a sessionId space distinct from the pre-v1.44
|
||||
// attach-token space (browse/src/pty-session-cookie.ts). These tests pin
|
||||
// the validate-first contract that codex outside-voice flagged as critical:
|
||||
// refreshLease MUST NOT resurrect expired leases, otherwise the 30-min TTL
|
||||
// stops bounding leaked-token blast radius.
|
||||
|
||||
import {
|
||||
mintLease,
|
||||
validateLease,
|
||||
refreshLease,
|
||||
revokeLease,
|
||||
leaseCount,
|
||||
__resetLeases,
|
||||
} from '../src/pty-session-lease';
|
||||
|
||||
beforeEach(() => {
|
||||
__resetLeases();
|
||||
});
|
||||
|
||||
describe('pty-session-lease: mint/validate/revoke', () => {
|
||||
test('mintLease returns a fresh non-secret sessionId + future expiresAt', () => {
|
||||
const a = mintLease();
|
||||
const b = mintLease();
|
||||
expect(a.sessionId).toBeTruthy();
|
||||
expect(b.sessionId).toBeTruthy();
|
||||
expect(a.sessionId).not.toBe(b.sessionId);
|
||||
expect(a.expiresAt).toBeGreaterThan(Date.now());
|
||||
// base64url alphabet: characters in [A-Za-z0-9_-].
|
||||
expect(a.sessionId).toMatch(/^[A-Za-z0-9_-]+$/);
|
||||
expect(leaseCount()).toBe(2);
|
||||
});
|
||||
|
||||
test('validateLease ok for fresh lease, false for unknown', () => {
|
||||
const { sessionId } = mintLease();
|
||||
const ok = validateLease(sessionId);
|
||||
expect(ok.ok).toBe(true);
|
||||
if (ok.ok) expect(ok.expiresAt).toBeGreaterThan(Date.now());
|
||||
expect(validateLease('not-a-real-session-id').ok).toBe(false);
|
||||
expect(validateLease(null).ok).toBe(false);
|
||||
expect(validateLease(undefined).ok).toBe(false);
|
||||
});
|
||||
|
||||
test('revokeLease removes the lease; subsequent validate returns false', () => {
|
||||
const { sessionId } = mintLease();
|
||||
expect(validateLease(sessionId).ok).toBe(true);
|
||||
revokeLease(sessionId);
|
||||
expect(validateLease(sessionId).ok).toBe(false);
|
||||
expect(leaseCount()).toBe(0);
|
||||
});
|
||||
|
||||
test('revokeLease tolerates unknown sessionId without throwing', () => {
|
||||
expect(() => revokeLease('phantom')).not.toThrow();
|
||||
expect(() => revokeLease(null)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe('pty-session-lease: refresh contract (validate-first)', () => {
|
||||
test('refreshLease extends expiresAt for a valid lease', () => {
|
||||
const { sessionId, expiresAt: initial } = mintLease();
|
||||
// Sleep micro-tick — Date.now() is ms-grain so a synchronous extend
|
||||
// may not move the integer. Use a tight async wait instead.
|
||||
return new Promise<void>((resolve) => {
|
||||
setTimeout(() => {
|
||||
const r = refreshLease(sessionId);
|
||||
expect(r.ok).toBe(true);
|
||||
if (r.ok) expect(r.expiresAt).toBeGreaterThan(initial);
|
||||
resolve();
|
||||
}, 5);
|
||||
});
|
||||
});
|
||||
|
||||
test('refreshLease rejects unknown sessionId (validate-first invariant)', () => {
|
||||
const r = refreshLease('never-minted');
|
||||
expect(r.ok).toBe(false);
|
||||
});
|
||||
|
||||
test('refreshLease never resurrects an expired lease', async () => {
|
||||
// Force TTL down to 5ms for this assertion by minting + waiting past expiry.
|
||||
// Lease internals use Date.now() so the easiest way to expire one is
|
||||
// to artificially backdate via revoke+remint cycle. Simpler: mint, then
|
||||
// wait for the registry's own expiry check to trip.
|
||||
//
|
||||
// We can't backdate without breaking encapsulation, so this test exercises
|
||||
// the negative-validate path: minted lease, then prove that refresh after
|
||||
// explicit revoke still returns ok:false (same as expired-and-pruned).
|
||||
const { sessionId } = mintLease();
|
||||
revokeLease(sessionId);
|
||||
const r = refreshLease(sessionId);
|
||||
expect(r.ok).toBe(false);
|
||||
});
|
||||
|
||||
test('refreshLease tolerates null / undefined sessionId', () => {
|
||||
expect(refreshLease(null).ok).toBe(false);
|
||||
expect(refreshLease(undefined).ok).toBe(false);
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue