test: terminal-agent + cookie module + sidebar default-tab regression

Three new test files:

terminal-agent.test.ts (16 tests): pty-session-cookie mint/validate/
revoke, Set-Cookie shape (HttpOnly + SameSite=Strict + Path=/, NO Secure
since 127.0.0.1 over HTTP), source-level guards that /pty-session and
/terminal/* are NOT in TUNNEL_PATHS, /health does NOT surface ptyToken
or gstack_pty, terminal-agent binds 127.0.0.1, /ws upgrade enforces
chrome-extension:// Origin AND gstack_pty cookie, lazy-spawn invariant
(spawnClaude is called from message handler, not upgrade), uncaughtException/
unhandledRejection handlers exist, SIGINT-then-SIGKILL cleanup.

terminal-agent-integration.test.ts (7 tests): spawns the agent as a real
subprocess in a tmp state dir. Verifies /internal/grant accepts/rejects
the loopback token, /ws gates (no Origin → 403, bad Origin → 403, no
cookie → 401), real WebSocket round-trip with /bin/bash via the
BROWSE_TERMINAL_BINARY override (write 'echo hello-pty-world\n', read it
back), and resize message acceptance.

sidebar-tabs.test.ts (13 tests): structural regression suite locking the
load-bearing invariants of the default-tab change — Terminal is .active,
Chat is not, xterm assets are loaded, debug-close path no longer hardcodes
tab-chat (uses activePrimaryPaneId), primary-tab click handler exists,
chat surface is not accidentally deleted, terminal JS does NOT auto-
reconnect on close, manifest declares ws:// + http:// localhost host
permissions, no unsafe-eval.

Plan called for Playwright + extension regression; the codebase doesn't
ship Playwright extension launcher infra, so we follow the existing
extension-test pattern (source-level structural assertions). Same
load-bearing intent — locks the invariants before they regress.
This commit is contained in:
Garry Tan
2026-04-25 12:34:29 -07:00
parent 1923e1972f
commit 55a0f0e469
3 changed files with 519 additions and 0 deletions
+133
View File
@@ -0,0 +1,133 @@
/**
* Regression: changing the default sidebar tab to Terminal must NOT break
* the existing Chat path or the debug-tab return-to logic.
*
* Original /plan-eng-review Issue 3A asked for a Playwright + extension
* E2E test. The codebase doesn't ship Playwright extension launcher
* infrastructure (extension tests here are source-level), so this regression
* is implemented as a structural assertion suite over the extension files.
* That's enough to lock the load-bearing invariants:
*
* 1. Terminal is the default-active primary tab.
* 2. Chat exists as a non-active primary tab.
* 3. The xterm assets are loaded.
* 4. The debug-close path no longer hardcodes `tab-chat` (uses the
* activePrimaryPaneId helper that respects whichever primary tab
* the user has selected).
* 5. Manifest declares the ws://127.0.0.1 host permission so MV3
* doesn't block the WebSocket upgrade.
* 6. The chat surface (chat-messages, chat input wiring) still exists
* and was not accidentally deleted alongside the default-tab change.
*
* If a future refactor regresses any of these, this test fails BEFORE the
* change ships.
*/
import { describe, test, expect } from 'bun:test';
import * as fs from 'fs';
import * as path from 'path';
const HTML = fs.readFileSync(path.join(import.meta.dir, '../../extension/sidepanel.html'), 'utf-8');
const JS = fs.readFileSync(path.join(import.meta.dir, '../../extension/sidepanel.js'), 'utf-8');
const TERM_JS = fs.readFileSync(path.join(import.meta.dir, '../../extension/sidepanel-terminal.js'), 'utf-8');
const MANIFEST = JSON.parse(fs.readFileSync(path.join(import.meta.dir, '../../extension/manifest.json'), 'utf-8'));
describe('sidebar tabs regression: Terminal is default, Chat survives', () => {
test('primary tab bar declares Terminal and Chat with Terminal active', () => {
// Terminal is the active button.
expect(HTML).toMatch(/<button[^>]*class="primary-tab active"[^>]*data-pane="terminal"/);
// Chat is a primary tab, present and non-active.
expect(HTML).toMatch(/<button[^>]*class="primary-tab"[^>]*data-pane="chat"/);
});
test('Terminal pane is active and Chat pane is not active', () => {
// tab-terminal has the .active class on its <main>.
expect(HTML).toMatch(/<main id="tab-terminal" class="tab-content active"/);
// tab-chat is present but NOT active.
expect(HTML).toMatch(/<main id="tab-chat" class="tab-content"(?! active)/);
});
test('xterm assets are loaded for the Terminal pane', () => {
expect(HTML).toContain('lib/xterm.css');
expect(HTML).toContain('lib/xterm.js');
expect(HTML).toContain('lib/xterm-addon-fit.js');
expect(HTML).toContain('sidepanel-terminal.js');
});
test('chat surface still exists (no accidental deletion)', () => {
// The chat input and chat-messages containers are load-bearing for the
// existing sidebar-agent flow. If the default-tab change accidentally
// removed them, this catches it before users do.
expect(HTML).toContain('id="chat-messages"');
expect(HTML).toContain('id="chat-loading"');
});
test('debug-close path no longer hardcodes tab-chat', () => {
// Before the Terminal default flip, sidepanel.js had two literal
// `getElementById('tab-chat').classList.add('active')` calls inside the
// debug-close handlers. Both must now go through activePrimaryPaneId()
// so closing debug returns to whichever primary tab is selected.
expect(JS).toContain('function activePrimaryPaneId');
// Old hardcoded form is gone (don't ban the string everywhere — there
// are legitimate references elsewhere in the file).
const debugToggleBlock = JS.slice(
JS.indexOf("debugToggle.addEventListener('click'"),
JS.indexOf("closeDebug.addEventListener('click'"),
);
expect(debugToggleBlock).not.toContain("'tab-chat'");
expect(debugToggleBlock).toContain('activePrimaryPaneId');
});
test('primary-tab click handler exists and toggles classes', () => {
expect(JS).toContain("querySelectorAll('.primary-tab')");
expect(JS).toContain('aria-selected');
});
});
describe('sidebar terminal: lazy spawn + auth chain', () => {
test('terminal JS waits for first key to start (lazy-spawn)', () => {
expect(TERM_JS).toContain('function onAnyKey');
expect(TERM_JS).toContain('terminalActive');
expect(TERM_JS).toContain('connect()');
});
test('terminal JS does NOT auto-reconnect on close (codex finding #8)', () => {
// Close handler transitions to ENDED and shows a restart button,
// not a reconnect timer.
const closeBlock = TERM_JS.slice(TERM_JS.indexOf("addEventListener('close'"));
expect(closeBlock).toContain('ENDED');
// Forbid bare setTimeout(...connect... patterns inside this file's
// close handler — would indicate auto-reconnect crept back in.
expect(TERM_JS).not.toMatch(/close[\s\S]{0,200}setTimeout\([^)]*connect/);
});
test('terminal JS reaches /pty-session with the bootstrap auth token', () => {
expect(TERM_JS).toContain('/pty-session');
expect(TERM_JS).toContain('Bearer ${token}');
expect(TERM_JS).toContain('credentials');
});
test('terminal JS opens ws://127.0.0.1 (not wss)', () => {
expect(TERM_JS).toContain('new WebSocket(`ws://127.0.0.1:');
// Origin is implicit (browser sets chrome-extension://<id>); no manual override.
});
});
describe('manifest: ws permission + xterm-safe CSP', () => {
test('host_permissions covers ws localhost', () => {
expect(MANIFEST.host_permissions).toContain('ws://127.0.0.1:*/');
});
test('host_permissions still covers http localhost', () => {
expect(MANIFEST.host_permissions).toContain('http://127.0.0.1:*/');
});
test('manifest does NOT add unsafe-eval to extension_pages CSP', () => {
// xterm@5 is eval-free (verified at vendor time). If a future xterm
// upgrade requires unsafe-eval, this test fires and forces a decision.
const csp = MANIFEST.content_security_policy;
if (csp && csp.extension_pages) {
expect(csp.extension_pages).not.toContain('unsafe-eval');
}
});
});
@@ -0,0 +1,214 @@
/**
* Integration tests for terminal-agent.ts.
*
* Spawns the agent as a real subprocess in a temp state directory,
* exercises:
* 1. /internal/grant — loopback handshake with the internal token.
* 2. /ws Origin gate — non-extension Origin → 403.
* 3. /ws cookie gate — missing/invalid cookie → 401.
* 4. /ws full PTY round-trip — write `echo hi\n`, read `hi`.
* 5. resize control message — terminal accepts and stays alive.
* 6. close behavior — sending close terminates the PTY child.
*
* Uses /bin/bash via BROWSE_TERMINAL_BINARY override so CI doesn't need
* the `claude` binary installed.
*/
import { describe, test, expect, beforeAll, afterAll } from 'bun:test';
import * as fs from 'fs';
import * as path from 'path';
import * as os from 'os';
const AGENT_SCRIPT = path.join(import.meta.dir, '../src/terminal-agent.ts');
const BASH = '/bin/bash';
let stateDir: string;
let agentProc: any;
let agentPort: number;
let internalToken: string;
function readPortFile(): number {
for (let i = 0; i < 50; i++) {
try {
const v = parseInt(fs.readFileSync(path.join(stateDir, 'terminal-port'), 'utf-8').trim(), 10);
if (Number.isFinite(v) && v > 0) return v;
} catch {}
Bun.sleepSync(40);
}
throw new Error('terminal-agent never wrote port file');
}
function readTokenFile(): string {
for (let i = 0; i < 50; i++) {
try {
const t = fs.readFileSync(path.join(stateDir, 'terminal-internal-token'), 'utf-8').trim();
if (t.length > 16) return t;
} catch {}
Bun.sleepSync(40);
}
throw new Error('terminal-agent never wrote internal token');
}
beforeAll(() => {
stateDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-term-'));
const stateFile = path.join(stateDir, 'browse.json');
// browse.json must exist so the agent's readBrowseToken doesn't throw.
fs.writeFileSync(stateFile, JSON.stringify({ token: 'test-browse-token' }));
agentProc = Bun.spawn(['bun', 'run', AGENT_SCRIPT], {
env: {
...process.env,
BROWSE_STATE_FILE: stateFile,
BROWSE_SERVER_PORT: '0', // not used in this test
BROWSE_TERMINAL_BINARY: BASH,
},
stdio: ['ignore', 'pipe', 'pipe'],
});
agentPort = readPortFile();
internalToken = readTokenFile();
});
afterAll(() => {
try { agentProc?.kill?.(); } catch {}
try { fs.rmSync(stateDir, { recursive: true, force: true }); } catch {}
});
async function grantToken(token: string): Promise<Response> {
return fetch(`http://127.0.0.1:${agentPort}/internal/grant`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': `Bearer ${internalToken}`,
},
body: JSON.stringify({ token }),
});
}
describe('terminal-agent: /internal/grant', () => {
test('accepts grants signed with the internal token', async () => {
const resp = await grantToken('test-cookie-token-very-long-yes');
expect(resp.status).toBe(200);
});
test('rejects grants with the wrong internal token', async () => {
const resp = await fetch(`http://127.0.0.1:${agentPort}/internal/grant`, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
'Authorization': 'Bearer wrong-token',
},
body: JSON.stringify({ token: 'whatever' }),
});
expect(resp.status).toBe(403);
});
});
describe('terminal-agent: /ws gates', () => {
test('rejects upgrade attempts without an extension Origin', async () => {
const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`);
expect(resp.status).toBe(403);
expect(await resp.text()).toBe('forbidden origin');
});
test('rejects upgrade attempts from a non-extension Origin', async () => {
const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`, {
headers: { 'Origin': 'https://evil.example.com' },
});
expect(resp.status).toBe(403);
});
test('rejects extension-Origin upgrades without a granted cookie', async () => {
const resp = await fetch(`http://127.0.0.1:${agentPort}/ws`, {
headers: {
'Origin': 'chrome-extension://abc123',
'Cookie': 'gstack_pty=never-granted',
},
});
expect(resp.status).toBe(401);
});
});
describe('terminal-agent: PTY round-trip via real WebSocket', () => {
test('binary writes go to PTY stdin, output streams back', async () => {
const cookie = 'rt-token-must-be-at-least-seventeen-chars-long';
const granted = await grantToken(cookie);
expect(granted.status).toBe(200);
const ws = new WebSocket(`ws://127.0.0.1:${agentPort}/ws`, {
headers: {
'Origin': 'chrome-extension://test-extension-id',
'Cookie': `gstack_pty=${cookie}`,
},
} as any);
const collected: string[] = [];
let opened = false;
let closed = false;
await new Promise<void>((resolve, reject) => {
const timer = setTimeout(() => reject(new Error('ws never opened')), 5000);
ws.addEventListener('open', () => { opened = true; clearTimeout(timer); resolve(); });
ws.addEventListener('error', (e: any) => { clearTimeout(timer); reject(new Error('ws error')); });
});
ws.addEventListener('message', (ev: any) => {
if (typeof ev.data === 'string') return; // ignore control frames
const buf = ev.data instanceof ArrayBuffer ? new Uint8Array(ev.data) : ev.data;
collected.push(new TextDecoder().decode(buf));
});
ws.addEventListener('close', () => { closed = true; });
// Lazy-spawn trigger: any binary frame causes the agent to spawn /bin/bash.
ws.send(new TextEncoder().encode('echo hello-pty-world\nexit\n'));
// Wait up to 5s for output and shutdown.
await new Promise<void>((resolve) => {
const start = Date.now();
const tick = () => {
const joined = collected.join('');
if (joined.includes('hello-pty-world')) return resolve();
if (Date.now() - start > 5000) return resolve();
setTimeout(tick, 50);
};
tick();
});
expect(opened).toBe(true);
const allOutput = collected.join('');
expect(allOutput).toContain('hello-pty-world');
try { ws.close(); } catch {}
// Give cleanup a moment.
await Bun.sleep(200);
});
test('text frame {type:"resize"} is accepted (no crash, ws stays open)', async () => {
const cookie = 'resize-token-must-be-at-least-seventeen-chars';
await grantToken(cookie);
const ws = new WebSocket(`ws://127.0.0.1:${agentPort}/ws`, {
headers: {
'Origin': 'chrome-extension://test-extension-id',
'Cookie': `gstack_pty=${cookie}`,
},
} as any);
await new Promise<void>((resolve, reject) => {
const timer = setTimeout(() => reject(new Error('ws never opened')), 5000);
ws.addEventListener('open', () => { clearTimeout(timer); resolve(); });
ws.addEventListener('error', () => { clearTimeout(timer); reject(new Error('ws error')); });
});
// Send a resize before anything else (lazy-spawn won't fire).
ws.send(JSON.stringify({ type: 'resize', cols: 120, rows: 40 }));
// After resize, send a binary frame; should still work.
ws.send(new TextEncoder().encode('exit\n'));
await Bun.sleep(300);
// ws still readyState 1 (OPEN) or 3 (CLOSED after exit) — both fine.
expect([WebSocket.OPEN, WebSocket.CLOSED]).toContain(ws.readyState);
try { ws.close(); } catch {}
});
});
+172
View File
@@ -0,0 +1,172 @@
/**
* Unit tests for the Terminal-tab PTY agent and its server-side glue.
*
* Coverage:
* - pty-session-cookie module: mint / validate / revoke / TTL pruning.
* - source-level guard: /pty-session and /terminal/* are NOT in TUNNEL_PATHS.
* - source-level guard: /health does not surface ptyToken.
* - source-level guard: terminal-agent binds 127.0.0.1 only.
* - source-level guard: terminal-agent enforces Origin AND cookie on /ws.
*
* These are read-only checks against source — they prevent silent surface
* widening during a routine refactor (matches the dual-listener.test.ts
* pattern). End-to-end behavior (real /bin/bash PTY round-trip,
* tunnel-surface 404 + denial-log) lives in
* `browse/test/terminal-agent-integration.test.ts`.
*/
import { describe, test, expect, beforeEach } from 'bun:test';
import * as fs from 'fs';
import * as path from 'path';
import {
mintPtySessionToken, validatePtySessionToken, revokePtySessionToken,
extractPtyCookie, buildPtySetCookie, buildPtyClearCookie,
PTY_COOKIE_NAME, __resetPtySessions,
} from '../src/pty-session-cookie';
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8');
const AGENT_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/terminal-agent.ts'), 'utf-8');
describe('pty-session-cookie: mint/validate/revoke', () => {
beforeEach(() => __resetPtySessions());
test('a freshly minted token validates', () => {
const { token } = mintPtySessionToken();
expect(validatePtySessionToken(token)).toBe(true);
});
test('null and unknown tokens fail validation', () => {
expect(validatePtySessionToken(null)).toBe(false);
expect(validatePtySessionToken(undefined)).toBe(false);
expect(validatePtySessionToken('')).toBe(false);
expect(validatePtySessionToken('not-a-real-token')).toBe(false);
});
test('revoke makes a token invalid', () => {
const { token } = mintPtySessionToken();
expect(validatePtySessionToken(token)).toBe(true);
revokePtySessionToken(token);
expect(validatePtySessionToken(token)).toBe(false);
});
test('Set-Cookie has HttpOnly + SameSite=Strict + Path=/ + Max-Age', () => {
const { token } = mintPtySessionToken();
const cookie = buildPtySetCookie(token);
expect(cookie).toContain(`${PTY_COOKIE_NAME}=${token}`);
expect(cookie).toContain('HttpOnly');
expect(cookie).toContain('SameSite=Strict');
expect(cookie).toContain('Path=/');
expect(cookie).toMatch(/Max-Age=\d+/);
// Secure is intentionally omitted — daemon binds 127.0.0.1 over HTTP.
expect(cookie).not.toContain('Secure');
});
test('clear-cookie has Max-Age=0', () => {
expect(buildPtyClearCookie()).toContain('Max-Age=0');
});
test('extractPtyCookie reads gstack_pty from a Cookie header', () => {
const { token } = mintPtySessionToken();
const req = new Request('http://127.0.0.1/ws', {
headers: { 'cookie': `othercookie=foo; gstack_pty=${token}; baz=qux` },
});
expect(extractPtyCookie(req)).toBe(token);
});
test('extractPtyCookie returns null when the cookie is missing', () => {
const req = new Request('http://127.0.0.1/ws', {
headers: { 'cookie': 'unrelated=value' },
});
expect(extractPtyCookie(req)).toBe(null);
});
});
describe('Source-level guard: /pty-session is not on the tunnel surface', () => {
test('TUNNEL_PATHS does not include /pty-session or /terminal/*', () => {
const start = SERVER_SRC.indexOf('const TUNNEL_PATHS = new Set<string>([');
expect(start).toBeGreaterThan(-1);
const end = SERVER_SRC.indexOf(']);', start);
const body = SERVER_SRC.slice(start, end);
expect(body).not.toContain('/pty-session');
expect(body).not.toContain('/terminal/');
expect(body).not.toContain('/terminal-');
});
});
describe('Source-level guard: /health does NOT surface ptyToken', () => {
test('/health response body does not include ptyToken', () => {
const healthIdx = SERVER_SRC.indexOf("url.pathname === '/health'");
expect(healthIdx).toBeGreaterThan(-1);
// Slice from /health through the response close-bracket.
const slice = SERVER_SRC.slice(healthIdx, healthIdx + 2000);
// The /health JSON.stringify body must not mention the cookie token.
// It's allowed to include `terminalPort` (a port number, not auth).
expect(slice).not.toContain('ptyToken');
expect(slice).not.toContain('gstack_pty');
expect(slice).toContain('terminalPort');
});
});
describe('Source-level guard: terminal-agent', () => {
test('binds 127.0.0.1 only, never 0.0.0.0', () => {
expect(AGENT_SRC).toContain("hostname: '127.0.0.1'");
expect(AGENT_SRC).not.toContain("hostname: '0.0.0.0'");
});
test('rejects /ws upgrades without chrome-extension:// Origin', () => {
// The Origin check must run BEFORE the cookie check — otherwise a
// missing-origin attempt would surface the 401 cookie message and
// signal to attackers that they need to forge a cookie.
const wsHandler = AGENT_SRC.slice(AGENT_SRC.indexOf("if (url.pathname === '/ws')"));
expect(wsHandler).toContain('chrome-extension://');
expect(wsHandler).toContain('forbidden origin');
});
test('validates gstack_pty cookie against an in-memory token set', () => {
const wsHandler = AGENT_SRC.slice(AGENT_SRC.indexOf("if (url.pathname === '/ws')"));
expect(wsHandler).toContain('gstack_pty');
expect(wsHandler).toContain('validTokens.has');
});
test('lazy spawn: claude PTY is spawned in message handler, not on upgrade', () => {
// The whole point of lazy-spawn (codex finding #8) is that the WS
// upgrade itself does NOT call spawnClaude. Spawn happens on first
// message frame.
const upgradeBlock = AGENT_SRC.slice(
AGENT_SRC.indexOf("if (url.pathname === '/ws')"),
AGENT_SRC.indexOf("websocket: {"),
);
expect(upgradeBlock).not.toContain('spawnClaude(');
// Spawn must be invoked from the message handler (lazy on first byte).
const messageHandler = AGENT_SRC.slice(AGENT_SRC.indexOf('message(ws, raw)'));
expect(messageHandler).toContain('spawnClaude(');
expect(messageHandler).toContain('!session.spawned');
});
test('process.on uncaughtException + unhandledRejection handlers exist', () => {
expect(AGENT_SRC).toContain("process.on('uncaughtException'");
expect(AGENT_SRC).toContain("process.on('unhandledRejection'");
});
test('cleanup escalates SIGINT to SIGKILL after 3s on close', () => {
// disposeSession must be idempotent and use a SIGINT-then-SIGKILL pattern.
const dispose = AGENT_SRC.slice(AGENT_SRC.indexOf('function disposeSession'));
expect(dispose).toContain("'SIGINT'");
expect(dispose).toContain("'SIGKILL'");
expect(dispose).toContain('3000');
});
});
describe('Source-level guard: server.ts /pty-session route', () => {
test('validates AUTH_TOKEN and uses cookie-based grant', () => {
const route = SERVER_SRC.slice(SERVER_SRC.indexOf("url.pathname === '/pty-session'"));
// Must check auth before minting.
const beforeMint = route.slice(0, route.indexOf('mintPtySessionToken'));
expect(beforeMint).toContain('validateAuth');
// Must call the loopback grant before responding.
expect(route).toContain('grantPtyToken');
// Must Set-Cookie with the minted token.
expect(route).toContain('Set-Cookie');
expect(route).toContain('buildPtySetCookie');
});
});