Files
web-terminal/test/tmux.test.ts
Yaojia Wang d39a0ab8d1 fix(sessions): tmux -t prefix-matching let orphan ops reach the user's own sessions
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.
2026-07-30 11:46:51 +02:00

232 lines
8.7 KiB
TypeScript
Raw Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

/**
* test/tmux.test.ts (H1) — thin tmux CLI wrappers.
*
* execFileSync is mocked so no real tmux binary runs: every wrapper is
* best-effort and must NOT throw (a throwing exec → false / no-op).
*/
import { describe, it, expect, vi, beforeEach } from 'vitest'
const mockExec = vi.fn()
// promisify(execFile) reads the custom symbol, so give it one that returns our mock.
const mockExecAsync = vi.fn()
const fakeExecFile = Object.assign(
(...a: unknown[]) => mockExecAsync(...a),
{ [Symbol.for('nodejs.util.promisify.custom')]: (...a: unknown[]) => mockExecAsync(...a) },
)
vi.mock('node:child_process', () => ({
execFileSync: (...a: unknown[]) => mockExec(...a),
execFile: fakeExecFile,
}))
const { tmuxAvailable, tmuxName, hasSession, killSession, parseSessionList, listSessions, capturePane } =
await import('../src/session/tmux.js')
beforeEach(() => {
mockExec.mockReset()
mockExecAsync.mockReset()
})
describe('tmuxName', () => {
it('prefixes the session id with web_', () => {
expect(tmuxName('abc')).toBe('web_abc')
})
})
describe('tmuxAvailable', () => {
it('returns true when `tmux -V` succeeds', () => {
mockExec.mockReturnValue(Buffer.from('tmux 3.4'))
expect(tmuxAvailable()).toBe(true)
expect(mockExec).toHaveBeenCalledWith('tmux', ['-V'], expect.objectContaining({ stdio: 'ignore' }))
})
it('returns false when tmux is missing (exec throws)', () => {
mockExec.mockImplementation(() => {
throw new Error('ENOENT')
})
expect(tmuxAvailable()).toBe(false)
})
})
describe('hasSession', () => {
it('returns true when has-session exits 0', () => {
mockExec.mockReturnValue(Buffer.from(''))
expect(hasSession('web_x')).toBe(true)
expect(mockExec).toHaveBeenCalledWith(
'tmux',
['has-session', '-t', '=web_x'],
expect.objectContaining({ stdio: 'ignore' }),
)
})
it('returns false when has-session throws (no such session)', () => {
mockExec.mockImplementation(() => {
throw new Error("can't find session")
})
expect(hasSession('web_gone')).toBe(false)
})
})
describe('killSession', () => {
it('targets the EXACT name so it cannot prefix-match another session', () => {
mockExec.mockReturnValue(Buffer.from(''))
killSession('web_x')
expect(mockExec).toHaveBeenCalledWith(
'tmux',
['kill-session', '-t', '=web_x'],
expect.objectContaining({ stdio: 'ignore' }),
)
})
it('swallows errors when the session is already gone', () => {
mockExec.mockImplementation(() => {
throw new Error('no session')
})
expect(() => killSession('web_gone')).not.toThrow()
})
})
/* ── A: enumerate + preview tmux sessions the server does not track ─────────── */
const U1 = '05caa88b-1f5f-45d9-bbbe-fc5955314870'
const U2 = '3a63f3c7-acfe-4081-992f-c4dbd55b3d6b'
describe('parseSessionList', () => {
it('parses tmux rows into summaries, converting seconds to ms', () => {
const out = `web_${U1}\t1783911991\t1783911992\t1783911990\t0\t80\t23\n`
expect(parseSessionList(out)).toEqual([
{
id: U1,
createdAtMs: 1783911991_000,
lastActivityAtMs: 1783911992_000,
attached: false,
cols: 80,
rows: 23,
},
])
})
it('sorts most-recently-active first', () => {
const out = [`web_${U1}\t100\t100\t100\t0\t80\t24`, `web_${U2}\t100\t900\t100\t1\t80\t24`].join('\n')
expect(parseSessionList(out).map((s) => s.id)).toEqual([U2, U1])
})
it('reports a session attached from a real terminal', () => {
const out = `web_${U1}\t100\t100\t100\t1\t80\t24`
expect(parseSessionList(out)[0]!.attached).toBe(true)
})
it('ignores sessions that are not ours — never touch the users own tmux', () => {
const out = ['mywork\t100\t100\t100\t0\t80\t24', `web_${U1}\t100\t100\t100\t0\t80\t24`].join('\n')
expect(parseSessionList(out).map((s) => s.id)).toEqual([U1])
})
it('ignores a web_-prefixed name whose suffix is not a UUID v4', () => {
// Only ids the protocol could have produced are actionable: anything else is
// someone else's session that merely starts with web_.
const out = ['web_../../etc\t100\t100\t100\t0\t80\t24', 'web_-t\t100\t100\t100\t0\t80\t24'].join('\n')
expect(parseSessionList(out)).toEqual([])
})
it('skips malformed and blank rows instead of throwing', () => {
const out = [
'',
'garbage',
`web_${U1}\tNaN\t100\t100\t0\t80\t24`,
`web_${U2}\t100\t100\t100\t0\t80\t24`,
].join('\n')
expect(parseSessionList(out).map((s) => s.id)).toEqual([U2])
})
it('rejects an EMPTY numeric field instead of reading it as 0', () => {
// Number('') is 0 and 0 is finite, so a blank clock would parse as "created at
// the epoch, idle ever since" — instantly eligible for the idle cleanup.
const out = `web_${U1}\t\t100\t100\t0\t80\t24`
expect(parseSessionList(out)).toEqual([])
})
it('takes the NEWER of the two activity clocks', () => {
// They are not ordered: window_activity tracks pane output, session_activity is
// bumped by an attach that produces none. "Has anything happened" is the max.
const winNewer = `web_${U1}\t100\t900\t100\t0\t80\t24`
const sessNewer = `web_${U2}\t100\t100\t900\t0\t80\t24`
expect(parseSessionList(winNewer)[0]!.lastActivityAtMs).toBe(900_000)
expect(parseSessionList(sessNewer)[0]!.lastActivityAtMs).toBe(900_000)
})
it('returns an empty list for empty output (no tmux server)', () => {
expect(parseSessionList('')).toEqual([])
})
})
describe('listSessions', () => {
it('asks tmux for a machine-readable list', async () => {
mockExecAsync.mockResolvedValue({ stdout: `web_${U1}\t100\t100\t100\t0\t80\t24\n`, stderr: '' })
expect((await listSessions()).map((s) => s.id)).toEqual([U1])
const [file, args] = mockExecAsync.mock.calls[0] as [string, string[]]
expect(file).toBe('tmux')
expect(args[0]).toBe('list-sessions')
expect(args).toContain('-F')
})
it('asks for window_activity, NOT session_activity', async () => {
// session_activity is a CLIENT clock: it never advances for a detached session
// however much its shell prints. Using it would make idle-cleanup kill exactly
// the unattended, actively-working sessions this feature exists to protect.
mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' })
await listSessions()
const [, args] = mockExecAsync.mock.calls[0] as [string, string[]]
const fmt = args[args.indexOf('-F') + 1]!
expect(fmt).toContain('#{window_activity}')
// session_activity is also requested — the parser takes the max — but
// window_activity is the one that must never be missing.
expect(fmt).toContain('#{session_activity}')
})
it('bounds the call so a wedged tmux cannot hang the server', async () => {
mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' })
await listSessions()
const opts = mockExecAsync.mock.calls[0]![2] as { timeout?: number; maxBuffer?: number }
expect(opts.timeout).toBeGreaterThan(0)
expect(opts.maxBuffer).toBeGreaterThan(0)
})
it('returns [] when there is no tmux server (exec rejects)', async () => {
mockExecAsync.mockRejectedValue(new Error('no server running'))
expect(await listSessions()).toEqual([])
})
})
describe('capturePane', () => {
it('captures the current screen WITHOUT attaching', async () => {
mockExecAsync.mockResolvedValue({ stdout: 'hello\n', stderr: '' })
expect(await capturePane('web_x')).toBe('hello\n')
const [file, args] = mockExecAsync.mock.calls[0] as [string, string[]]
expect(file).toBe('tmux')
expect(args[0]).toBe('capture-pane')
expect(args).toContain('-p') // print to stdout — read-only, no client
expect(args).toContain('-e') // keep escape sequences so colour survives
// '=web_x:' forces an EXACT session match for a target-PANE. A bare target
// prefix-matches, which leaks the screen of a session parseSessionList
// deliberately refuses to enumerate; a plain '=web_x' is rejected by tmux
// outright ("can't find pane"), because '=' only qualifies the session part.
expect(args).toContain('=web_x:')
expect(args).not.toContain('web_x')
expect(args).not.toContain('=web_x')
// Attaching would resize the session and SIGWINCH whatever is running in it.
expect(args).not.toContain('attach-session')
})
it('returns null when the session is gone', async () => {
mockExecAsync.mockRejectedValue(new Error("can't find session"))
expect(await capturePane('web_gone')).toBeNull()
})
it('bounds the call', async () => {
mockExecAsync.mockResolvedValue({ stdout: '', stderr: '' })
await capturePane('web_x')
const opts = mockExecAsync.mock.calls[0]![2] as { timeout?: number }
expect(opts.timeout).toBeGreaterThan(0)
})
})