Round two of adversarial review. The skeptic confirmed the session_activity finding
with extra evidence from this host and raised three things the first pass missed.
SECURITY — `tmux -t <name>` resolves exact name, then NAME PREFIX, then fnmatch.
The whole module rests on "we only ever touch `web_` + a UUID v4, so a tmux session
the user made for their own work is never enumerated and never offered for
deletion". Prefix matching goes around that: a session named `web_<uuid>_mine` is
refused by parseSessionList (its suffix is not a UUID) and so never appears in the
listing — but `-t web_<uuid>` matches it. So a DELETE aimed at an id that does not
exist would end the user's session, and a preview would print its screen.
Verified on tmux 3.6a, isolated socket:
has-session -t web_<uuid> → succeeds against web_<uuid>_MINE
has-session -t =web_<uuid> → "can't find session"
capture-pane -p -t web_<uuid> → printed the DECOY's screen
capture-pane -p -t =web_<uuid>: → "can't find session"
Two target forms are needed, because `=` only qualifies the session part of a
target: session targets (has-session, kill-session) take `=name`, while
capture-pane takes a PANE and rejects a bare `=name` outright ("can't find pane"),
so it needs `=name:`. Both are now the only way a name reaches tmux, with an
integration test that stands up a real decoy and asserts the listing omits it and
both the preview and the DELETE report 404 while it stays alive.
CORRECTNESS — take max(window_activity, session_activity), not window_activity
alone. The previous commit swapped one clock for the other, but they are not
ordered: window_activity tracks pane output while an attach with no output bumps
only session_activity. On this host one session's session_activity was 4 s NEWER
than its window_activity (and another's window_activity was 25 days newer). The
question is "has anything happened here", so the answer is the later of the two.
Also documents the caveat that window_activity resolves against the session's
current window only — exact for the single-window sessions this app creates, and
bounded by the max for anything else.
CORRECTNESS — parseSessionList accepted an empty numeric field. Number('') is 0
and 0 is finite, so a blank clock parsed as "created at the epoch, idle ever
since": instantly eligible for the idle cleanup. Fields must now be actual digits.
SAFETY — bulk cleanup re-reads the world immediately before killing. The candidate
list is a snapshot and the kill loop takes time, so a session could be attached, or
adopted into the table, inside that window. This is the irreversible path; it now
verifies each candidate against a fresh listing rather than trusting the snapshot.
PERFORMANCE — captureOrphan no longer probes with hasSession first. That was a
second tmux spawn per thumbnail, and a SYNCHRONOUS one on the path a whole grid
refreshes, buying nothing: capture-pane already fails cleanly on a missing session
and we already turn that into null. The guard splits into a pure half
(mayActOnOrphan: UUID shape + not in the table) used by both paths, and the
existence probe, which only the destructive path needs.
Tests: 2213 unit, 9 orphan integration (incl. the decoy regression). Confirmed
`has-session -t =<name>` still resolves the 69 real sessions, so the Case 3.5
re-attach path is unaffected.
Note: test/integration/server.test.ts "H1 shell state survives a server restart"
flakes ~1 run in 3 when all 27 run together on a loaded machine, and passes 4/4 in
isolation. That is the pre-existing real-PTY timing flake already recorded in
PROGRESS_LOG, not this change — it exercises Case 3.5, which is verified above.
245 lines
8.1 KiB
TypeScript
245 lines
8.1 KiB
TypeScript
/**
|
|
* Integration test for the orphan-session routes (A):
|
|
* GET /orphan-sessions
|
|
* GET /orphan-sessions/:id/preview
|
|
* DELETE /orphan-sessions/:id
|
|
*
|
|
* SAFETY: this file NEVER issues `DELETE /orphan-sessions` (the bulk cleanup),
|
|
* because these tests run against the host's real tmux server and would end the
|
|
* developer's own long-running sessions. Bulk cleanup is covered with mocks in
|
|
* test/manager-orphans.test.ts. The only session touched here is one this file
|
|
* creates under a fresh UUID and tears down itself.
|
|
*/
|
|
|
|
import net from 'node:net'
|
|
import { execFileSync } from 'node:child_process'
|
|
import { randomUUID } from 'node:crypto'
|
|
|
|
import { afterAll, beforeAll, describe, expect, it } from 'vitest'
|
|
|
|
import { loadConfig } from '../../src/config.js'
|
|
import { startServer } from '../../src/server.js'
|
|
import type { OrphanSessionInfo } from '../../src/types.js'
|
|
|
|
const TMUX_AVAILABLE = (() => {
|
|
try {
|
|
execFileSync('tmux', ['-V'], { stdio: 'ignore' })
|
|
return true
|
|
} catch {
|
|
return false
|
|
}
|
|
})()
|
|
const itTmux = TMUX_AVAILABLE ? it : it.skip
|
|
|
|
function getFreePort(): Promise<number> {
|
|
return new Promise((resolve, reject) => {
|
|
const srv = net.createServer()
|
|
srv.listen(0, '127.0.0.1', () => {
|
|
const addr = srv.address()
|
|
if (addr === null || typeof addr === 'string') {
|
|
srv.close()
|
|
reject(new Error('unexpected address type'))
|
|
return
|
|
}
|
|
const port = addr.port
|
|
srv.close(() => resolve(port))
|
|
})
|
|
srv.on('error', reject)
|
|
})
|
|
}
|
|
|
|
/* ── tmux off: the whole feature must be inert ──────────────────────────────── */
|
|
|
|
describe('orphan sessions — tmux disabled', () => {
|
|
let port: number
|
|
let handle: { close(): Promise<void> }
|
|
const origin = (): string => `http://127.0.0.1:${port}`
|
|
|
|
beforeAll(async () => {
|
|
port = await getFreePort()
|
|
handle = startServer(
|
|
loadConfig({
|
|
PORT: String(port),
|
|
BIND_HOST: '127.0.0.1',
|
|
SHELL_PATH: process.env['SHELL'] ?? '/bin/zsh',
|
|
ALLOWED_ORIGINS: `http://127.0.0.1:${port}`,
|
|
USE_TMUX: '0',
|
|
IDLE_TTL: '86400',
|
|
}),
|
|
)
|
|
await new Promise<void>((r) => setTimeout(r, 100))
|
|
})
|
|
|
|
afterAll(async () => {
|
|
await handle.close()
|
|
})
|
|
|
|
it('lists nothing, and does not require an Origin header (read-only)', async () => {
|
|
const res = await fetch(`${origin()}/orphan-sessions`)
|
|
expect(res.status).toBe(200)
|
|
expect(await res.json()).toEqual([])
|
|
})
|
|
|
|
it('404s a preview instead of shelling out to tmux', async () => {
|
|
const res = await fetch(`${origin()}/orphan-sessions/${randomUUID()}/preview`)
|
|
expect(res.status).toBe(404)
|
|
})
|
|
|
|
it('404s a kill', async () => {
|
|
const res = await fetch(`${origin()}/orphan-sessions/${randomUUID()}`, {
|
|
method: 'DELETE',
|
|
headers: { Origin: origin() },
|
|
})
|
|
expect(res.status).toBe(404)
|
|
})
|
|
})
|
|
|
|
/* ── tmux on: real session, real capture, real kill ─────────────────────────── */
|
|
|
|
describe('orphan sessions — tmux enabled', () => {
|
|
let port: number
|
|
let handle: { close(): Promise<void> }
|
|
const origin = (): string => `http://127.0.0.1:${port}`
|
|
|
|
/** Our throwaway session. Never any other. */
|
|
const id = randomUUID()
|
|
const name = `web_${id}`
|
|
let created = false
|
|
|
|
beforeAll(async () => {
|
|
port = await getFreePort()
|
|
handle = startServer(
|
|
loadConfig({
|
|
PORT: String(port),
|
|
BIND_HOST: '127.0.0.1',
|
|
SHELL_PATH: process.env['SHELL'] ?? '/bin/zsh',
|
|
ALLOWED_ORIGINS: `http://127.0.0.1:${port}`,
|
|
USE_TMUX: '1',
|
|
IDLE_TTL: '86400',
|
|
}),
|
|
)
|
|
await new Promise<void>((r) => setTimeout(r, 100))
|
|
|
|
if (TMUX_AVAILABLE) {
|
|
// Detached, printing a known marker so capture-pane has something to find.
|
|
execFileSync(
|
|
'tmux',
|
|
['new-session', '-d', '-s', name, `printf 'ORPHAN_MARKER_%s' OK; sleep 300`],
|
|
{ stdio: 'ignore' },
|
|
)
|
|
created = true
|
|
await new Promise<void>((r) => setTimeout(r, 400))
|
|
}
|
|
})
|
|
|
|
afterAll(async () => {
|
|
if (created) {
|
|
try {
|
|
execFileSync('tmux', ['kill-session', '-t', name], { stdio: 'ignore' })
|
|
} catch {
|
|
// the kill test already removed it — fine
|
|
}
|
|
}
|
|
await handle.close()
|
|
})
|
|
|
|
itTmux('lists our untracked tmux session as an orphan', async () => {
|
|
const res = await fetch(`${origin()}/orphan-sessions`)
|
|
expect(res.status).toBe(200)
|
|
const body = (await res.json()) as OrphanSessionInfo[]
|
|
const mine = body.find((o) => o.id === id)
|
|
expect(mine).toBeDefined()
|
|
expect(mine!.attached).toBe(false) // created with -d, nobody attached
|
|
expect(mine!.cols).toBeGreaterThan(0)
|
|
expect(mine!.lastActivityAt).toBeGreaterThan(0)
|
|
})
|
|
|
|
itTmux('previews the live screen without attaching to it', async () => {
|
|
const res = await fetch(`${origin()}/orphan-sessions/${id}/preview`)
|
|
expect(res.status).toBe(200)
|
|
const body = (await res.json()) as { id: string; data: string }
|
|
expect(body.id).toBe(id)
|
|
expect(body.data).toContain('ORPHAN_MARKER_OK')
|
|
|
|
// The capture must not have left a client behind: still unattached.
|
|
const after = execFileSync('tmux', ['ls', '-F', '#{session_name} #{session_attached}'], {
|
|
encoding: 'utf8',
|
|
})
|
|
expect(after).toContain(`${name} 0`)
|
|
})
|
|
|
|
itTmux('rejects a non-UUID id with 400, never reaching tmux', async () => {
|
|
for (const bad of ['-t', 'not-a-uuid', '../../etc/passwd']) {
|
|
const res = await fetch(`${origin()}/orphan-sessions/${encodeURIComponent(bad)}/preview`)
|
|
expect(res.status).toBe(400)
|
|
}
|
|
})
|
|
|
|
itTmux('refuses a kill with a foreign Origin (CSRF guard)', async () => {
|
|
const res = await fetch(`${origin()}/orphan-sessions/${id}`, {
|
|
method: 'DELETE',
|
|
headers: { Origin: 'http://evil.example' },
|
|
})
|
|
expect(res.status).toBe(403)
|
|
// and it is still alive
|
|
expect(
|
|
execFileSync('tmux', ['ls', '-F', '#{session_name}'], { encoding: 'utf8' }),
|
|
).toContain(name)
|
|
})
|
|
|
|
itTmux('cannot reach a look-alike session by tmux prefix matching', async () => {
|
|
// tmux -t resolves exact name, then NAME PREFIX, then fnmatch. `web_<uuid>_mine`
|
|
// is refused by parseSessionList (suffix is not a UUID) and so is never listed —
|
|
// but a bare `-t web_<uuid>` prefix-matches it, which would let a DELETE aimed at
|
|
// a non-existent id end a session the user created for their own work.
|
|
const decoyId = randomUUID()
|
|
const decoy = `web_${decoyId}_mine`
|
|
execFileSync('tmux', ['new-session', '-d', '-s', decoy, 'sleep 300'], { stdio: 'ignore' })
|
|
try {
|
|
// It must not appear in the listing…
|
|
const list = (await (await fetch(`${origin()}/orphan-sessions`)).json()) as OrphanSessionInfo[]
|
|
expect(list.some((o) => o.id === decoyId)).toBe(false)
|
|
|
|
// …and the exact id must be reported as absent, not silently matched to it.
|
|
const preview = await fetch(`${origin()}/orphan-sessions/${decoyId}/preview`)
|
|
expect(preview.status).toBe(404)
|
|
|
|
const del = await fetch(`${origin()}/orphan-sessions/${decoyId}`, {
|
|
method: 'DELETE',
|
|
headers: { Origin: origin() },
|
|
})
|
|
expect(del.status).toBe(404)
|
|
|
|
// The decoy is still alive — that is the whole point.
|
|
expect(
|
|
execFileSync('tmux', ['ls', '-F', '#{session_name}'], { encoding: 'utf8' }),
|
|
).toContain(decoy)
|
|
} finally {
|
|
try {
|
|
execFileSync('tmux', ['kill-session', '-t', `=${decoy}`], { stdio: 'ignore' })
|
|
} catch {
|
|
// already gone
|
|
}
|
|
}
|
|
})
|
|
|
|
itTmux('kills it, and 404s the second time', async () => {
|
|
const first = await fetch(`${origin()}/orphan-sessions/${id}`, {
|
|
method: 'DELETE',
|
|
headers: { Origin: origin() },
|
|
})
|
|
expect(first.status).toBe(204)
|
|
|
|
const listing = execFileSync('tmux', ['ls', '-F', '#{session_name}'], {
|
|
encoding: 'utf8',
|
|
}).split('\n')
|
|
expect(listing).not.toContain(name)
|
|
|
|
const second = await fetch(`${origin()}/orphan-sessions/${id}`, {
|
|
method: 'DELETE',
|
|
headers: { Origin: origin() },
|
|
})
|
|
expect(second.status).toBe(404)
|
|
})
|
|
})
|