From dcfee04e1b2d1e500736c9d63a2720bd4ecb78db Mon Sep 17 00:00:00 2001 From: prateek Date: Wed, 18 Feb 2026 03:28:55 +0530 Subject: [PATCH] fix: terminal servers compatible with hash-based architecture (#87) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: terminal servers compatible with hash-based architecture The terminal WebSocket servers (direct-terminal-ws and terminal-websocket) used config.dataDir to validate sessions, which no longer exists in the hash-based architecture. Also fixed node-pty failing to find tmux via posix_spawnp. Changes: - Remove config.dataDir dependency, validate via `tmux has-session` instead - Add resolveTmuxSession() to map user-facing IDs (ao-15) to hash-prefixed tmux names (8474d6f29887-ao-15) - Use explicit tmux path discovery (findTmux) since node-pty's posix_spawnp doesn't reliably inherit PATH - Include /opt/homebrew/bin in fallback PATH for macOS ARM Co-Authored-By: Claude Opus 4.6 * fix: add session resolution to ttyd server and use exact tmux matching - Add findTmux() and resolveTmuxSession() to terminal-websocket.ts (previously only in direct-terminal-ws.ts), fixing hash-prefixed session lookup for the ttyd-based terminal server - Use tmux exact match prefix (=sessionId) in has-session checks to prevent ao-1 from matching ao-15 via prefix matching - Add server compatibility tests that verify both servers handle hash-based architecture correctly (14 tests) - Include server/ in tsconfig and vitest config for typecheck coverage Co-Authored-By: Claude Opus 4.6 * refactor: extract tmux-utils and add proper unit tests - Extract findTmux(), resolveTmuxSession(), validateSessionId() into shared server/tmux-utils.ts — eliminates duplication between direct-terminal-ws.ts and terminal-websocket.ts - Add 20 real unit tests with injected mocks that test actual behavior: - findTmux: candidate priority, fallback to bare name - resolveTmuxSession: exact match, hash-prefix resolution, = prefix for preventing tmux prefix matching (ao-1 vs ao-15), null when no session found, tmux not running - validateSessionId: path traversal, shell injection, whitespace - Slim down server-compatibility.test.ts to 10 structural checks (imports tmux-utils, no loadConfig, no config.dataDir, no existsSync) Total: 30 tests — all pass on fix branch, 8 fail on main Co-Authored-By: Claude Opus 4.6 * test: add real integration tests for direct-terminal-ws - Refactor direct-terminal-ws to export createDirectTerminalServer() factory so tests can control server lifecycle without side effects - Add 10 integration tests that create real tmux sessions, start the real server, connect via WebSocket, and verify the full flow: - Health endpoint returns 200 - Missing session parameter → close 1008 - Path traversal in session ID → close 1008 - Shell injection in session ID → close 1008 - Nonexistent tmux session → close 1008 - Real tmux session → connects and receives terminal output - Hash-prefixed session resolution works end-to-end - Can send input and receive echoed output - Resize messages don't crash the connection - Unknown HTTP path → 404 - Tests create/destroy tmux sessions in beforeAll/afterAll - Server runs on random port (port 0) to avoid conflicts - Total test suite: 40 tests (20 unit + 10 compatibility + 10 integration) Co-Authored-By: Claude Opus 4.6 * ci: add web server tests to CI pipeline The web package was explicitly excluded from CI test runs (pnpm -r --filter '!@composio/ao-web' test). Add a test-web job that installs tmux, starts the tmux server, and runs the web package tests (unit + integration). Co-Authored-By: Claude Opus 4.6 * fix: address CI failures and bugbot review comments - Fix lint: replace require() with ESM import in integration test - Fix base-path mismatch: ttyd now uses user-facing sessionId for --base-path/URL and actual tmux name for attach-session - Fix bare "tmux" in ttyd spawn args: use TMUX constant (full path) - Scope CI test-web job to server/__tests__/ to avoid pre-existing failures in src/__tests__/ Co-Authored-By: Claude Opus 4.6 * test: comprehensive unit and integration test coverage Unit tests (77): validateSessionId covers all injection vectors (shell, path traversal, command substitution, special chars, unicode, control chars), findTmux covers all candidate paths and error types, resolveTmuxSession covers exact match, hash-prefix resolution, suffix matching precision, edge cases (single char, long lists, multiple hyphens, different tmux paths). Integration tests (45): health endpoint lifecycle (active count tracks connections/disconnections), HTTP routing (404s for all non-health paths), WebSocket validation (11 injection/traversal vectors), terminal connection (resize, multi-resize, invalid JSON, non-resize JSON), hash-prefixed resolution (suffix match, command passthrough, session key tracking, cross-match prevention), terminal I/O (Ctrl-C, Tab, Enter, empty messages, rapid keystrokes, multi-line), connection lifecycle (cleanup, rapid connect/disconnect, error recovery), server creation (independent instances). Co-Authored-By: Claude Opus 4.6 * fix: validate 12-char hex prefix in hash-prefixed session resolution The previous endsWith("-{sessionId}") suffix match was ambiguous: "hash-my-app-1" would falsely match a lookup for "app-1". Now validates that the prefix matches the exact format generated by generateConfigHash (12-char lowercase hex) before comparing the remainder. Added unit tests for the ambiguity case and invalid prefix formats. Updated integration test session names to use proper 12-char hex prefixes. Co-Authored-By: Claude Opus 4.6 --------- Co-authored-by: Claude Opus 4.6 --- .github/workflows/ci.yml | 19 + .../direct-terminal-ws.integration.test.ts | 760 +++++++++++++++++ .../__tests__/server-compatibility.test.ts | 81 ++ .../web/server/__tests__/tmux-utils.test.ts | 794 ++++++++++++++++++ packages/web/server/direct-terminal-ws.ts | 413 ++++----- packages/web/server/terminal-websocket.ts | 45 +- packages/web/server/tmux-utils.ts | 102 +++ packages/web/tsconfig.json | 2 +- packages/web/vitest.config.ts | 2 +- 9 files changed, 1994 insertions(+), 224 deletions(-) create mode 100644 packages/web/server/__tests__/direct-terminal-ws.integration.test.ts create mode 100644 packages/web/server/__tests__/server-compatibility.test.ts create mode 100644 packages/web/server/__tests__/tmux-utils.test.ts create mode 100644 packages/web/server/tmux-utils.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7fff9cb6..efa827b84 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,3 +55,22 @@ jobs: - run: pnpm install --frozen-lockfile - run: pnpm -r --filter '!@composio/ao-web' build - run: pnpm test + + test-web: + name: Test (Web) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: pnpm/action-setup@v4 + - uses: actions/setup-node@v4 + with: + node-version: 20 + cache: pnpm + - name: Install tmux + run: sudo apt-get update && sudo apt-get install -y tmux + - name: Start tmux server + run: tmux start-server + - run: pnpm install --frozen-lockfile + - run: pnpm -r --filter '!@composio/ao-web' build + - name: Run web server tests (unit + integration) + run: pnpm --filter @composio/ao-web exec vitest run server/__tests__/ diff --git a/packages/web/server/__tests__/direct-terminal-ws.integration.test.ts b/packages/web/server/__tests__/direct-terminal-ws.integration.test.ts new file mode 100644 index 000000000..2cf132466 --- /dev/null +++ b/packages/web/server/__tests__/direct-terminal-ws.integration.test.ts @@ -0,0 +1,760 @@ +/** + * Integration tests for direct-terminal-ws. + * + * These start the real server, create real tmux sessions, connect via + * WebSocket, and verify the full flow works end-to-end — exactly what + * a user's browser does when opening a terminal on the dashboard. + * + * These tests would have caught the PR #58 breakage because: + * - The server would fail to start (loadConfig crash) + * - Session resolution would fail (config.dataDir doesn't exist) + * - WebSocket connections would be rejected (no tmux session match) + */ + +import { describe, it, expect, beforeAll, afterAll, afterEach } from "vitest"; +import { execFileSync } from "node:child_process"; +import { request, type IncomingMessage } from "node:http"; +import { WebSocket } from "ws"; +import { findTmux } from "../tmux-utils.js"; +import { createDirectTerminalServer, type DirectTerminalServer } from "../direct-terminal-ws.js"; + +const TMUX = findTmux(); +const TEST_SESSION = `ao-test-integration-${process.pid}`; +const TEST_HASH_SESSION = `abcdef123456-${TEST_SESSION}`; + +let terminal: DirectTerminalServer; +let port: number; + +// ============================================================================= +// Helpers +// ============================================================================= + +function httpGet(path: string): Promise<{ status: number; body: string }> { + return new Promise((resolve, reject) => { + const req = request( + { hostname: "localhost", port, path, method: "GET", timeout: 3000 }, + (res: IncomingMessage) => { + let body = ""; + res.on("data", (chunk: Buffer) => { body += chunk.toString(); }); + res.on("end", () => resolve({ status: res.statusCode ?? 0, body })); + }, + ); + req.on("error", reject); + req.end(); + }); +} + +function connectWs(sessionId: string): Promise { + return new Promise((resolve, reject) => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=${sessionId}`); + ws.on("open", () => resolve(ws)); + ws.on("error", reject); + setTimeout(() => reject(new Error("WebSocket connect timeout")), 5000); + }); +} + +function waitForWsClose(ws: WebSocket): Promise<{ code: number; reason: string }> { + return new Promise((resolve) => { + ws.on("close", (code, reason) => { + resolve({ code, reason: reason.toString() }); + }); + setTimeout(() => resolve({ code: -1, reason: "timeout" }), 5000); + }); +} + +function waitForWsData(ws: WebSocket, timeoutMs = 3000): Promise { + return new Promise((resolve, reject) => { + let buf = ""; + const handler = (data: Buffer | string) => { + buf += data.toString(); + if (buf.length > 0) { + ws.off("message", handler); + resolve(buf); + } + }; + ws.on("message", handler); + setTimeout(() => { + ws.off("message", handler); + if (buf.length > 0) resolve(buf); + else reject(new Error("No data received from terminal")); + }, timeoutMs); + }); +} + +/** Wait for output containing a specific marker string */ +function waitForMarker(ws: WebSocket, marker: string, timeoutMs = 3000): Promise { + return new Promise((resolve) => { + let buf = ""; + const handler = (data: Buffer | string) => { + buf += data.toString(); + if (buf.includes(marker)) { + ws.off("message", handler); + resolve(buf); + } + }; + ws.on("message", handler); + setTimeout(() => { ws.off("message", handler); resolve(buf); }, timeoutMs); + }); +} + +// ============================================================================= +// Lifecycle +// ============================================================================= + +beforeAll(() => { + // Create test tmux sessions + execFileSync(TMUX, ["new-session", "-d", "-s", TEST_SESSION, "-x", "80", "-y", "24"], { timeout: 5000 }); + execFileSync(TMUX, ["new-session", "-d", "-s", TEST_HASH_SESSION, "-x", "80", "-y", "24"], { timeout: 5000 }); + + // Start the server on a random port + terminal = createDirectTerminalServer(TMUX); + terminal.server.listen(0); + const addr = terminal.server.address(); + port = typeof addr === "object" && addr ? addr.port : 0; +}); + +afterEach(() => { + // Clean up any active sessions from tests + for (const [, session] of terminal.activeSessions) { + session.pty.kill(); + session.ws.close(); + } + terminal.activeSessions.clear(); +}); + +afterAll(() => { + terminal.shutdown(); + + // Kill test tmux sessions + try { execFileSync(TMUX, ["kill-session", "-t", TEST_SESSION], { timeout: 5000 }); } catch { /* already dead */ } + try { execFileSync(TMUX, ["kill-session", "-t", TEST_HASH_SESSION], { timeout: 5000 }); } catch { /* already dead */ } +}); + +// ============================================================================= +// Health endpoint +// ============================================================================= + +describe("health endpoint", () => { + it("GET /health returns 200 with JSON body", async () => { + const res = await httpGet("/health"); + + expect(res.status).toBe(200); + const data = JSON.parse(res.body); + expect(data).toHaveProperty("active"); + expect(data).toHaveProperty("sessions"); + }); + + it("health shows 0 active sessions initially", async () => { + const res = await httpGet("/health"); + const data = JSON.parse(res.body); + + expect(data.active).toBe(0); + expect(data.sessions).toEqual([]); + }); + + it("health reflects active sessions after WebSocket connection", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + const res = await httpGet("/health"); + const data = JSON.parse(res.body); + + expect(data.active).toBe(1); + expect(data.sessions).toContain(TEST_SESSION); + + ws.close(); + }); + + it("health active count matches number of connections", async () => { + // Create a second tmux session for this test + const secondSession = `ao-test-health-${process.pid}`; + execFileSync(TMUX, ["new-session", "-d", "-s", secondSession, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws1 = await connectWs(TEST_SESSION); + await waitForWsData(ws1); + const ws2 = await connectWs(secondSession); + await waitForWsData(ws2); + + const res = await httpGet("/health"); + const data = JSON.parse(res.body); + + expect(data.active).toBe(2); + expect(data.sessions).toContain(TEST_SESSION); + expect(data.sessions).toContain(secondSession); + + ws1.close(); + ws2.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", secondSession], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("health active count decreases after WebSocket close", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Verify connected + let res = await httpGet("/health"); + expect(JSON.parse(res.body).active).toBe(1); + + // Close and wait for cleanup + ws.close(); + await new Promise((r) => setTimeout(r, 200)); + + res = await httpGet("/health"); + expect(JSON.parse(res.body).active).toBe(0); + }); +}); + +// ============================================================================= +// HTTP routing +// ============================================================================= + +describe("HTTP routing", () => { + it("returns 404 for unknown HTTP path", async () => { + const res = await httpGet("/unknown-path"); + expect(res.status).toBe(404); + }); + + it("returns 404 for root path", async () => { + const res = await httpGet("/"); + expect(res.status).toBe(404); + }); + + it("returns 404 for /terminal (that's the ttyd server's endpoint)", async () => { + const res = await httpGet("/terminal"); + expect(res.status).toBe(404); + }); + + it("returns 404 for /ws via HTTP (not WebSocket upgrade)", async () => { + const res = await httpGet("/ws"); + expect(res.status).toBe(404); + }); +}); + +// ============================================================================= +// WebSocket connection validation +// ============================================================================= + +describe("WebSocket connection validation", () => { + it("rejects connection with no session parameter", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Missing session"); + }); + + it("rejects connection with empty session parameter", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=`); + const result = await waitForWsClose(ws); + + // URL searchParams.get("session") returns "" for ?session=, which is falsy + expect(result.code).toBe(1008); + expect(result.reason).toContain("Missing session"); + }); + + it("rejects connection with path traversal in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=../../../etc/passwd`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects connection with shell injection in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test;rm%20-rf%20/`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects command substitution in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test$(whoami)`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects backtick injection in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test%60id%60`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects pipe in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=test|cat%20/etc/passwd`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects forward slash in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=ao/15`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects spaces in session ID", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=ao%2015`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Invalid session ID"); + }); + + it("rejects connection for nonexistent tmux session", async () => { + const ws = new WebSocket(`ws://localhost:${port}/ws?session=ao-nonexistent-999`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Session not found"); + }); + + it("rejects connection for session that doesn't exist in tmux", async () => { + // Valid format but no such tmux session + const ws = new WebSocket(`ws://localhost:${port}/ws?session=definitely-not-real-${Date.now()}`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Session not found"); + }); +}); + +// ============================================================================= +// WebSocket terminal connection — basic +// ============================================================================= + +describe("WebSocket terminal connection", () => { + it("connects to a real tmux session and receives terminal output", async () => { + const ws = await connectWs(TEST_SESSION); + + // tmux sends terminal init sequences on attach + const data = await waitForWsData(ws); + expect(data.length).toBeGreaterThan(0); + + ws.close(); + }); + + it("can send input to the terminal and receive echo", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Send a command — "echo INTEGRATION_TEST_MARKER" + const marker = `MARKER_${Date.now()}`; + ws.send(`echo ${marker}\n`); + + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles resize messages without crashing", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Send resize message (same format xterm.js FitAddon sends) + ws.send(JSON.stringify({ type: "resize", cols: 120, rows: 40 })); + + // Verify connection still works after resize + const marker = `RESIZE_OK_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles multiple resize messages in sequence", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Rapid resize sequence (simulating window drag) + ws.send(JSON.stringify({ type: "resize", cols: 100, rows: 30 })); + ws.send(JSON.stringify({ type: "resize", cols: 110, rows: 35 })); + ws.send(JSON.stringify({ type: "resize", cols: 120, rows: 40 })); + + // Should still work + const marker = `MULTI_RESIZE_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("passes non-resize JSON as terminal input (not intercepted)", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // JSON that doesn't match resize format should pass through as terminal input + ws.send(JSON.stringify({ type: "not-resize", data: "hello" })); + + // Should not crash — the JSON string is written to the terminal + const marker = `JSON_PASSTHROUGH_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles incomplete resize JSON gracefully", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Resize with missing cols — should be treated as terminal input + ws.send(JSON.stringify({ type: "resize", rows: 40 })); + // Resize with missing rows + ws.send(JSON.stringify({ type: "resize", cols: 120 })); + + // Should still work + const marker = `INCOMPLETE_RESIZE_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles invalid JSON starting with { gracefully", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Looks like it might be JSON but isn't + ws.send("{not json at all"); + + // Should not crash — treated as terminal input + const marker = `BADJSON_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); +}); + +// ============================================================================= +// Hash-prefixed session resolution (integration) +// ============================================================================= + +describe("hash-prefixed session resolution", () => { + it("resolves hash-prefixed tmux session by suffix match", async () => { + // Create a session that only exists with a hash prefix (no exact match) + const hashOnlySession = `ao-hashtest-${process.pid}`; + const hashPrefixedName = `deadbeef0123-${hashOnlySession}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashPrefixedName, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(hashOnlySession); + + // Should have resolved via hash-prefix match and connected + const data = await waitForWsData(ws); + expect(data.length).toBeGreaterThan(0); + + ws.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashPrefixedName], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("can send input through hash-resolved session", async () => { + const hashOnlySession = `ao-hashcmd-${process.pid}`; + const hashPrefixedName = `cafebabe0123-${hashOnlySession}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashPrefixedName, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(hashOnlySession); + await waitForWsData(ws); + + const marker = `HASH_CMD_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashPrefixedName], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("uses user-facing ID (not hash name) as activeSessions key", async () => { + const hashOnlySession = `ao-hashkey-${process.pid}`; + const hashPrefixedName = `face12340000-${hashOnlySession}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashPrefixedName, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(hashOnlySession); + await waitForWsData(ws); + + // The activeSessions map should use the user-facing ID, not the hash-prefixed tmux name + expect(terminal.activeSessions.has(hashOnlySession)).toBe(true); + expect(terminal.activeSessions.has(hashPrefixedName)).toBe(false); + + ws.close(); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashPrefixedName], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("does NOT cross-match ao-1 to hash-ao-15 via prefix", async () => { + // Create "deadbeef0123-ao-test-15-PID" but NOT "ao-test-1-PID" + // Connecting as "ao-test-1-PID" should fail (not match ao-test-15-PID) + const session15 = `ao-crosstest-15-${process.pid}`; + const hashSession15 = `deadbeef01ab-${session15}`; + const session1 = `ao-crosstest-1-${process.pid}`; + + execFileSync(TMUX, ["new-session", "-d", "-s", hashSession15, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + // ao-crosstest-1-PID should NOT resolve to deadbeef01ab-ao-crosstest-15-PID + const ws = new WebSocket(`ws://localhost:${port}/ws?session=${session1}`); + const result = await waitForWsClose(ws); + + expect(result.code).toBe(1008); + expect(result.reason).toContain("Session not found"); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", hashSession15], { timeout: 5000 }); } catch { /* */ } + } + }); +}); + +// ============================================================================= +// Terminal I/O +// ============================================================================= + +describe("terminal I/O", () => { + it("can run a command and get output", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + const marker = `PWD_TEST_${Date.now()}`; + ws.send(`echo ${marker}_$(pwd | wc -c)\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles special terminal characters (Ctrl-C as \\x03)", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Start a long-running process + ws.send("sleep 9999\n"); + await new Promise((r) => setTimeout(r, 200)); + + // Send Ctrl-C to interrupt it + ws.send("\x03"); + await new Promise((r) => setTimeout(r, 200)); + + // Should be back at the prompt — test by echoing a marker + const marker = `CTRLC_OK_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles Tab key (\\t) for completion", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Tab should not crash the connection + ws.send("ech\t"); + await new Promise((r) => setTimeout(r, 300)); + + // Clear with Ctrl-C and verify still working + ws.send("\x03"); + await new Promise((r) => setTimeout(r, 200)); + + const marker = `TAB_OK_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles Enter key (\\r or \\n)", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Enter via \r (what xterm.js typically sends) + const marker = `ENTER_TEST_${Date.now()}`; + ws.send(`echo ${marker}\r`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles empty messages without crashing", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Send empty string + ws.send(""); + + // Should still work + const marker = `EMPTY_MSG_${Date.now()}`; + ws.send(`echo ${marker}\n`); + const output = await waitForMarker(ws, marker); + expect(output).toContain(marker); + + ws.close(); + }); + + it("handles rapid keystrokes", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + // Simulate rapid typing + const chars = "echo RAPID_TEST\n"; + for (const ch of chars) { + ws.send(ch); + } + + const output = await waitForMarker(ws, "RAPID_TEST"); + expect(output).toContain("RAPID_TEST"); + + ws.close(); + }); + + it("handles multi-line input", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + const marker = `MULTILINE_${Date.now()}`; + ws.send(`echo "line1" && echo "${marker}"\n`); + + const output = await waitForMarker(ws, marker); + expect(output).toContain("line1"); + expect(output).toContain(marker); + + ws.close(); + }); +}); + +// ============================================================================= +// Connection lifecycle +// ============================================================================= + +describe("connection lifecycle", () => { + it("cleans up activeSessions on WebSocket close", async () => { + // Use a dedicated session to avoid race conditions with afterEach cleanup + const cleanupSession = `ao-test-cleanup-${process.pid}`; + execFileSync(TMUX, ["new-session", "-d", "-s", cleanupSession, "-x", "80", "-y", "24"], { timeout: 5000 }); + + try { + const ws = await connectWs(cleanupSession); + await waitForWsData(ws); + + // Verify the session was registered + expect(terminal.activeSessions.has(cleanupSession)).toBe(true); + + ws.close(); + await new Promise((r) => setTimeout(r, 300)); + + // After close, the session should be cleaned up + expect(terminal.activeSessions.has(cleanupSession)).toBe(false); + } finally { + try { execFileSync(TMUX, ["kill-session", "-t", cleanupSession], { timeout: 5000 }); } catch { /* */ } + } + }); + + it("tracks session by user-facing ID in activeSessions", async () => { + const ws = await connectWs(TEST_SESSION); + await waitForWsData(ws); + + expect(terminal.activeSessions.has(TEST_SESSION)).toBe(true); + + const session = terminal.activeSessions.get(TEST_SESSION); + expect(session).toBeDefined(); + expect(session!.sessionId).toBe(TEST_SESSION); + + ws.close(); + }); + + it("handles rapid connect and disconnect", async () => { + // Connect and immediately close multiple times + for (let i = 0; i < 3; i++) { + const ws = await connectWs(TEST_SESSION); + ws.close(); + await new Promise((r) => setTimeout(r, 100)); + } + + // Server should still be healthy + const res = await httpGet("/health"); + expect(res.status).toBe(200); + }); + + it("server stays healthy after connection errors", async () => { + // Try invalid connection + const ws = new WebSocket(`ws://localhost:${port}/ws?session=nonexistent-${Date.now()}`); + await waitForWsClose(ws); + + // Server should still be healthy + const res = await httpGet("/health"); + expect(res.status).toBe(200); + expect(JSON.parse(res.body).active).toBe(0); + }); + + it("multiple health checks work consistently", async () => { + // Rapid health checks shouldn't break anything + const results = await Promise.all([ + httpGet("/health"), + httpGet("/health"), + httpGet("/health"), + ]); + + for (const res of results) { + expect(res.status).toBe(200); + const data = JSON.parse(res.body); + expect(typeof data.active).toBe("number"); + } + }); +}); + +// ============================================================================= +// Server creation +// ============================================================================= + +describe("server creation", () => { + it("createDirectTerminalServer returns all expected properties", () => { + expect(terminal).toHaveProperty("server"); + expect(terminal).toHaveProperty("wss"); + expect(terminal).toHaveProperty("activeSessions"); + expect(terminal).toHaveProperty("shutdown"); + expect(terminal.activeSessions).toBeInstanceOf(Map); + expect(typeof terminal.shutdown).toBe("function"); + }); + + it("can create multiple independent servers", () => { + const server2 = createDirectTerminalServer(TMUX); + server2.server.listen(0); + const addr = server2.server.address(); + const port2 = typeof addr === "object" && addr ? addr.port : 0; + + expect(port2).toBeGreaterThan(0); + expect(port2).not.toBe(port); + + // Independent activeSessions + expect(server2.activeSessions).not.toBe(terminal.activeSessions); + + server2.shutdown(); + }); +}); diff --git a/packages/web/server/__tests__/server-compatibility.test.ts b/packages/web/server/__tests__/server-compatibility.test.ts new file mode 100644 index 000000000..193f83b9a --- /dev/null +++ b/packages/web/server/__tests__/server-compatibility.test.ts @@ -0,0 +1,81 @@ +/** + * Server compatibility tests. + * + * These verify that the terminal server files import shared utilities + * from tmux-utils.ts and don't contain deprecated patterns from the + * pre-hash-based architecture (config.dataDir, loadConfig, existsSync). + * + * For actual behavioral tests of the shared utilities, see tmux-utils.test.ts. + */ + +import { describe, it, expect } from "vitest"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const serverDir = join(__dirname, ".."); + +function readServerFile(name: string): string { + return readFileSync(join(serverDir, name), "utf-8"); +} + +describe("direct-terminal-ws.ts", () => { + const source = readServerFile("direct-terminal-ws.ts"); + + it("imports from shared tmux-utils", () => { + expect(source).toMatch(/from\s+["']\.\/tmux-utils/); + }); + + it("does not import loadConfig from @composio/ao-core", () => { + expect(source).not.toMatch(/import\s.*loadConfig.*from\s+["']@composio\/ao-core["']/); + }); + + it("does not reference config.dataDir", () => { + expect(source).not.toMatch(/config\.dataDir/); + }); + + it("does not use bare 'tmux' string for ptySpawn", () => { + expect(source).not.toMatch(/ptySpawn\(\s*["']tmux["']/); + }); + + it("does not check file existence for session validation", () => { + expect(source).not.toMatch(/existsSync.*session/i); + }); +}); + +describe("terminal-websocket.ts", () => { + const source = readServerFile("terminal-websocket.ts"); + + it("imports from shared tmux-utils", () => { + expect(source).toMatch(/from\s+["']\.\/tmux-utils/); + }); + + it("does not import loadConfig from @composio/ao-core", () => { + expect(source).not.toMatch(/import\s.*loadConfig.*from\s+["']@composio\/ao-core["']/); + }); + + it("does not reference config.dataDir", () => { + expect(source).not.toMatch(/config\.dataDir/); + }); + + it("does not check file existence for session validation", () => { + expect(source).not.toMatch(/existsSync.*session/i); + }); +}); + +describe("OrchestratorConfig compatibility", () => { + it("OrchestratorConfig does not have dataDir property", () => { + const typesSource = readFileSync( + join(__dirname, "..", "..", "..", "core", "src", "types.ts"), + "utf-8", + ); + + const configMatch = typesSource.match( + /export interface OrchestratorConfig \{[\s\S]*?\n\}/, + ); + expect(configMatch).toBeTruthy(); + const configBlock = configMatch![0]; + + expect(configBlock).not.toMatch(/dataDir/); + expect(configBlock).toMatch(/configPath/); + }); +}); diff --git a/packages/web/server/__tests__/tmux-utils.test.ts b/packages/web/server/__tests__/tmux-utils.test.ts new file mode 100644 index 000000000..a4d3581a8 --- /dev/null +++ b/packages/web/server/__tests__/tmux-utils.test.ts @@ -0,0 +1,794 @@ +/** + * Unit tests for tmux-utils. + * + * These test actual behavior by injecting mock execFileSync functions, + * verifying the logic handles all edge cases correctly. + */ + +import { describe, it, expect, vi } from "vitest"; +import { findTmux, resolveTmuxSession, validateSessionId, SESSION_ID_PATTERN } from "../tmux-utils.js"; + +// ============================================================================= +// validateSessionId +// ============================================================================= + +describe("validateSessionId", () => { + describe("accepts valid IDs", () => { + it("accepts simple alphanumeric IDs", () => { + expect(validateSessionId("ao-15")).toBe(true); + expect(validateSessionId("ao_orchestrator")).toBe(true); + expect(validateSessionId("session123")).toBe(true); + }); + + it("accepts hash-prefixed IDs", () => { + expect(validateSessionId("8474d6f29887-ao-15")).toBe(true); + expect(validateSessionId("abcdef123456-my-session")).toBe(true); + }); + + it("accepts single character IDs", () => { + expect(validateSessionId("a")).toBe(true); + expect(validateSessionId("1")).toBe(true); + expect(validateSessionId("-")).toBe(true); + expect(validateSessionId("_")).toBe(true); + }); + + it("accepts numbers-only IDs", () => { + expect(validateSessionId("12345")).toBe(true); + expect(validateSessionId("0")).toBe(true); + }); + + it("accepts hyphens and underscores only", () => { + expect(validateSessionId("---")).toBe(true); + expect(validateSessionId("___")).toBe(true); + expect(validateSessionId("-_-")).toBe(true); + }); + + it("accepts uppercase letters", () => { + expect(validateSessionId("AO-15")).toBe(true); + expect(validateSessionId("MySession")).toBe(true); + expect(validateSessionId("ALLCAPS")).toBe(true); + }); + + it("accepts long session IDs", () => { + const longId = "a".repeat(200); + expect(validateSessionId(longId)).toBe(true); + }); + + it("accepts realistic session names from the orchestrator", () => { + expect(validateSessionId("ao-orchestrator")).toBe(true); + expect(validateSessionId("ao-1")).toBe(true); + expect(validateSessionId("ao-99")).toBe(true); + expect(validateSessionId("integrator-44")).toBe(true); + expect(validateSessionId("splitly-3")).toBe(true); + expect(validateSessionId("8474d6f29887-ao-15")).toBe(true); + expect(validateSessionId("deadbeef1234-integrator-7")).toBe(true); + }); + }); + + describe("rejects empty and whitespace", () => { + it("rejects empty string", () => { + expect(validateSessionId("")).toBe(false); + }); + + it("rejects spaces", () => { + expect(validateSessionId("ao 15")).toBe(false); + expect(validateSessionId(" ao-15")).toBe(false); + expect(validateSessionId("ao-15 ")).toBe(false); + expect(validateSessionId(" ")).toBe(false); + }); + + it("rejects tabs and newlines", () => { + expect(validateSessionId("ao\t15")).toBe(false); + expect(validateSessionId("ao\n15")).toBe(false); + expect(validateSessionId("ao\r15")).toBe(false); + expect(validateSessionId("\t")).toBe(false); + }); + }); + + describe("rejects path traversal", () => { + it("rejects dot-dot-slash sequences", () => { + expect(validateSessionId("../etc/passwd")).toBe(false); + expect(validateSessionId("ao-15/../../secret")).toBe(false); + expect(validateSessionId("..")).toBe(false); + }); + + it("rejects forward slashes", () => { + expect(validateSessionId("ao/15")).toBe(false); + expect(validateSessionId("/etc/passwd")).toBe(false); + expect(validateSessionId("a/b")).toBe(false); + }); + + it("rejects backslashes", () => { + expect(validateSessionId("ao\\15")).toBe(false); + expect(validateSessionId("..\\..\\secret")).toBe(false); + }); + + it("rejects dots (current directory reference)", () => { + expect(validateSessionId(".")).toBe(false); + expect(validateSessionId("ao.15")).toBe(false); + }); + }); + + describe("rejects shell injection", () => { + it("rejects semicolons", () => { + expect(validateSessionId("ao-15; rm -rf /")).toBe(false); + expect(validateSessionId(";id")).toBe(false); + }); + + it("rejects command substitution", () => { + expect(validateSessionId("ao-15$(whoami)")).toBe(false); + expect(validateSessionId("$(cat /etc/passwd)")).toBe(false); + }); + + it("rejects backticks", () => { + expect(validateSessionId("ao-15`id`")).toBe(false); + expect(validateSessionId("`rm -rf /`")).toBe(false); + }); + + it("rejects pipes", () => { + expect(validateSessionId("ao-15|cat /etc/passwd")).toBe(false); + expect(validateSessionId("|id")).toBe(false); + }); + + it("rejects ampersands", () => { + expect(validateSessionId("ao-15&sleep 10")).toBe(false); + expect(validateSessionId("ao-15&&id")).toBe(false); + }); + + it("rejects angle brackets (redirection)", () => { + expect(validateSessionId("ao-15>output")).toBe(false); + expect(validateSessionId("ao-15 { + expect(validateSessionId("ao-15'")).toBe(false); + expect(validateSessionId('ao-15"')).toBe(false); + }); + + it("rejects dollar signs", () => { + expect(validateSessionId("$HOME")).toBe(false); + expect(validateSessionId("ao-${15}")).toBe(false); + }); + + it("rejects parentheses", () => { + expect(validateSessionId("ao(15)")).toBe(false); + expect(validateSessionId("(id)")).toBe(false); + }); + + it("rejects hash/pound sign", () => { + expect(validateSessionId("ao#15")).toBe(false); + expect(validateSessionId("#comment")).toBe(false); + }); + + it("rejects exclamation mark", () => { + expect(validateSessionId("ao!15")).toBe(false); + }); + + it("rejects asterisk and question mark (glob)", () => { + expect(validateSessionId("ao*")).toBe(false); + expect(validateSessionId("ao?15")).toBe(false); + }); + + it("rejects tilde (home directory)", () => { + expect(validateSessionId("~")).toBe(false); + expect(validateSessionId("~/something")).toBe(false); + }); + }); + + describe("rejects other dangerous characters", () => { + it("rejects null bytes", () => { + expect(validateSessionId("ao\x0015")).toBe(false); + }); + + it("rejects unicode", () => { + expect(validateSessionId("ao-\u00e9")).toBe(false); + expect(validateSessionId("ao-\u200b15")).toBe(false); // zero-width space + }); + + it("rejects control characters", () => { + expect(validateSessionId("ao\x01")).toBe(false); + expect(validateSessionId("ao\x7f")).toBe(false); // DEL + }); + + it("rejects percent (URL encoding attempts)", () => { + expect(validateSessionId("ao%2F15")).toBe(false); + expect(validateSessionId("%00")).toBe(false); + }); + + it("rejects at sign", () => { + expect(validateSessionId("user@host")).toBe(false); + }); + + it("rejects colons", () => { + expect(validateSessionId("ao:15")).toBe(false); + }); + + it("rejects equals sign", () => { + expect(validateSessionId("ao=15")).toBe(false); + }); + + it("rejects square brackets", () => { + expect(validateSessionId("ao[15]")).toBe(false); + }); + + it("rejects curly braces", () => { + expect(validateSessionId("ao{15}")).toBe(false); + }); + + it("rejects comma", () => { + expect(validateSessionId("ao,15")).toBe(false); + }); + }); + + describe("SESSION_ID_PATTERN export", () => { + it("is exported and matches the same pattern", () => { + expect(SESSION_ID_PATTERN).toBeInstanceOf(RegExp); + expect(SESSION_ID_PATTERN.test("ao-15")).toBe(true); + expect(SESSION_ID_PATTERN.test("../bad")).toBe(false); + }); + }); +}); + +// ============================================================================= +// findTmux +// ============================================================================= + +describe("findTmux", () => { + it("returns first candidate that succeeds", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw new Error("not found"); }) // /opt/homebrew/bin/tmux + .mockImplementationOnce(() => "tmux 3.4") // /usr/local/bin/tmux succeeds + .mockImplementationOnce(() => "tmux 3.4"); // /usr/bin/tmux (not reached) + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/local/bin/tmux"); + expect(mockExec).toHaveBeenCalledTimes(2); + }); + + it("returns /opt/homebrew/bin/tmux on macOS ARM (first candidate)", () => { + const mockExec = vi.fn().mockReturnValue("tmux 3.4"); + + const result = findTmux(mockExec); + + expect(result).toBe("/opt/homebrew/bin/tmux"); + expect(mockExec).toHaveBeenCalledTimes(1); + }); + + it("returns /usr/bin/tmux on Linux (third candidate)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw new Error("not found"); }) // /opt/homebrew/bin/tmux + .mockImplementationOnce(() => { throw new Error("not found"); }) // /usr/local/bin/tmux + .mockImplementationOnce(() => "tmux 3.3a"); // /usr/bin/tmux + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/bin/tmux"); + expect(mockExec).toHaveBeenCalledTimes(3); + }); + + it("falls back to bare 'tmux' when no candidates found", () => { + const mockExec = vi.fn().mockImplementation(() => { + throw new Error("not found"); + }); + + const result = findTmux(mockExec); + + expect(result).toBe("tmux"); + expect(mockExec).toHaveBeenCalledTimes(3); + }); + + it("checks all three standard locations in order", () => { + const mockExec = vi.fn().mockImplementation(() => { + throw new Error("not found"); + }); + + findTmux(mockExec); + + expect(mockExec).toHaveBeenNthCalledWith(1, "/opt/homebrew/bin/tmux", ["-V"], { timeout: 5000 }); + expect(mockExec).toHaveBeenNthCalledWith(2, "/usr/local/bin/tmux", ["-V"], { timeout: 5000 }); + expect(mockExec).toHaveBeenNthCalledWith(3, "/usr/bin/tmux", ["-V"], { timeout: 5000 }); + }); + + it("handles timeout errors from execFileSync", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw Object.assign(new Error("ETIMEDOUT"), { code: "ETIMEDOUT" }); }) + .mockImplementationOnce(() => "tmux 3.4"); + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/local/bin/tmux"); + }); + + it("handles permission denied errors", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw Object.assign(new Error("EACCES"), { code: "EACCES" }); }) + .mockImplementationOnce(() => { throw Object.assign(new Error("EACCES"), { code: "EACCES" }); }) + .mockImplementationOnce(() => "tmux 3.3a"); + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/bin/tmux"); + }); + + it("handles ENOENT (file not found) errors", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { throw Object.assign(new Error("ENOENT"), { code: "ENOENT" }); }) + .mockImplementationOnce(() => "tmux 3.4"); + + const result = findTmux(mockExec); + + expect(result).toBe("/usr/local/bin/tmux"); + }); + + it("passes -V flag and 5000ms timeout to each candidate", () => { + const mockExec = vi.fn().mockReturnValue("tmux 3.4"); + + findTmux(mockExec); + + const [, args, options] = mockExec.mock.calls[0]; + expect(args).toEqual(["-V"]); + expect(options).toEqual({ timeout: 5000 }); + }); + + it("stops checking after first success (short-circuit)", () => { + const mockExec = vi.fn().mockReturnValue("tmux 3.4"); + + findTmux(mockExec); + + expect(mockExec).toHaveBeenCalledTimes(1); + }); +}); + +// ============================================================================= +// resolveTmuxSession +// ============================================================================= + +describe("resolveTmuxSession", () => { + const TMUX = "/opt/homebrew/bin/tmux"; + + // --------------------------------------------------------------------------- + // Exact match behavior + // --------------------------------------------------------------------------- + + describe("exact match", () => { + it("returns sessionId for exact match", () => { + const mockExec = vi.fn().mockReturnValue(""); + + const result = resolveTmuxSession("ao-orchestrator", TMUX, mockExec); + + expect(result).toBe("ao-orchestrator"); + }); + + it("uses = prefix to prevent tmux prefix matching", () => { + // This is the critical bugbot fix: without =, "ao-1" matches "ao-15" + const mockExec = vi.fn().mockReturnValue(""); + + resolveTmuxSession("ao-1", TMUX, mockExec); + + // Must pass "=ao-1" not "ao-1" to has-session + expect(mockExec).toHaveBeenCalledWith( + TMUX, + ["has-session", "-t", "=ao-1"], + { timeout: 5000 }, + ); + }); + + it("passes correct tmux path and timeout to has-session", () => { + const customTmux = "/usr/bin/tmux"; + const mockExec = vi.fn().mockReturnValue(""); + + resolveTmuxSession("my-session", customTmux, mockExec); + + expect(mockExec).toHaveBeenCalledWith( + customTmux, + ["has-session", "-t", "=my-session"], + { timeout: 5000 }, + ); + }); + + it("prefers exact match over hash-prefixed match", () => { + // If "ao-15" exists as both exact and hash-prefixed, return exact + const mockExec = vi.fn().mockReturnValue(""); + + const result = resolveTmuxSession("ao-15", TMUX, mockExec); + + expect(result).toBe("ao-15"); + // Should only call has-session, not list-sessions + expect(mockExec).toHaveBeenCalledTimes(1); + }); + + it("does not call list-sessions when exact match succeeds", () => { + const mockExec = vi.fn().mockReturnValue(""); + + resolveTmuxSession("ao-15", TMUX, mockExec); + + expect(mockExec).toHaveBeenCalledTimes(1); + expect(mockExec).toHaveBeenCalledWith( + TMUX, + ["has-session", "-t", "=ao-15"], + { timeout: 5000 }, + ); + }); + }); + + // --------------------------------------------------------------------------- + // Hash-prefix resolution + // --------------------------------------------------------------------------- + + describe("hash-prefix resolution", () => { + it("resolves hash-prefixed session when exact match fails", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "8474d6f29887-ao-15\na1b2c3d4e5f6-ao-16\nao-orchestrator\n"; + }); + + const result = resolveTmuxSession("ao-15", TMUX, mockExec); + + expect(result).toBe("8474d6f29887-ao-15"); + }); + + it("uses exact match after hash prefix (not endsWith)", () => { + // "ao-1" should NOT match "8474d6f29887-ao-15" because substring(13) is "ao-15" not "ao-1" + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "8474d6f29887-ao-15\n8474d6f29887-ao-16\n"; + }); + + const result = resolveTmuxSession("ao-1", TMUX, mockExec); + + expect(result).toBeNull(); + }); + + it("does NOT match ambiguous suffixes (bugbot: hash-my-app-1 vs app-1)", () => { + // This is the critical bugbot fix: "hash-my-app-1" ends with "-app-1" + // but the hash prefix is not a valid 12-char hex string, so it shouldn't match. + // A user looking up "app-1" should NOT be connected to "hash-my-app-1". + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "nonhexprefix-my-app-1\n8474d6f29887-app-1\n"; + }); + + // Should match the one with valid hash prefix, not the ambiguous one + expect(resolveTmuxSession("app-1", TMUX, mockExec)).toBe("8474d6f29887-app-1"); + }); + + it("rejects session names where hash prefix is not 12-char hex", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + // These look like hash-prefixed but aren't valid 12-char hex + return [ + "short-ao-15", // too short + "toolonghashprefix-ao-15", // too long + "ABCDEF123456-ao-15", // uppercase hex (not matching [a-f0-9]) + "zzzzzzzzzzzz-ao-15", // not hex chars + "8474d6f2988-ao-15", // 11 chars (one short) + "8474d6f29887a-ao-15", // 13 chars (one extra) + ].join("\n") + "\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("only matches valid 12-char lowercase hex prefix", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "abcdef012345-ao-15\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("abcdef012345-ao-15"); + }); + + it("matches the correct session among many", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return [ + "aabbccddeef0-ao-1", + "112233445566-ao-15", + "ffeeddccbbaa-ao-2", + "a0b1c2d3e4f5-ao-orchestrator", + ].join("\n") + "\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("112233445566-ao-15"); + }); + + it("matches ao-1 to hash-ao-1 (not hash-ao-15)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return [ + "aabbccddeef0-ao-1", + "112233445566-ao-15", + "ffeeddccbbaa-ao-2", + ].join("\n") + "\n"; + }); + + expect(resolveTmuxSession("ao-1", TMUX, mockExec)).toBe("aabbccddeef0-ao-1"); + }); + + it("matches session with multiple hyphens in name", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-my-long-session-name\n112233445566-other-session\n"; + }); + + expect(resolveTmuxSession("my-long-session-name", TMUX, mockExec)) + .toBe("aabbccddeef0-my-long-session-name"); + }); + + it("matches session with underscores", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-my_session\n112233445566-other_session\n"; + }); + + expect(resolveTmuxSession("my_session", TMUX, mockExec)).toBe("aabbccddeef0-my_session"); + }); + + it("passes list-sessions format flag correctly", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "some-session\n"; + }); + + resolveTmuxSession("ao-99", TMUX, mockExec); + + expect(mockExec).toHaveBeenNthCalledWith(2, + TMUX, + ["list-sessions", "-F", "#{session_name}"], + { timeout: 5000, encoding: "utf8" }, + ); + }); + + it("does NOT match if session name contains the ID but not after hash prefix", () => { + // e.g., "ao-15-extended" has no valid hash prefix + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "ao-15-extended\nao-15-backup\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("does NOT match hash-prefixed session with extra suffix", () => { + // "aabbccddeef0-ao-15-backup" has valid hash but substring(13) is "ao-15-backup" not "ao-15" + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15-backup\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns first match when multiple hash-prefixed sessions exist for same ID", () => { + // This shouldn't happen in practice, but test the behavior + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15\n112233445566-ao-15\n"; + }); + + // find() returns the first match + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBe("aabbccddeef0-ao-15"); + }); + }); + + // --------------------------------------------------------------------------- + // Not found scenarios + // --------------------------------------------------------------------------- + + describe("not found", () => { + it("returns null when no session matches", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "some-other-session\nanother-session\n"; + }); + + expect(resolveTmuxSession("ao-99", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when tmux is not running", () => { + const mockExec = vi.fn().mockImplementation(() => { + throw new Error("no server running on /tmp/tmux-501/default"); + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when list-sessions returns empty string", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return ""; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when list-sessions returns only newlines", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "\n\n\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when list-sessions throws (no sessions exist)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); // has-session fails + }) + .mockImplementationOnce(() => { + throw new Error("no sessions"); // list-sessions fails + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("returns null when has-session times out and list-sessions is empty", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw Object.assign(new Error("ETIMEDOUT"), { code: "ETIMEDOUT" }); + }) + .mockImplementationOnce(() => { + return "\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + }); + + // --------------------------------------------------------------------------- + // Edge cases + // --------------------------------------------------------------------------- + + describe("edge cases", () => { + it("handles session name that looks like a hash (all hex chars)", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-abcdef123456\n"; + }); + + expect(resolveTmuxSession("abcdef123456", TMUX, mockExec)).toBe("aabbccddeef0-abcdef123456"); + }); + + it("handles single-char session ID", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-a\n112233445566-b\n"; + }); + + expect(resolveTmuxSession("a", TMUX, mockExec)).toBe("aabbccddeef0-a"); + }); + + it("does not match session without valid hash prefix", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "xao-15\nnotahash-ao-15\n"; + }); + + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("handles Windows-style line endings in list-sessions output", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-15\r\n112233445566-ao-16\r\n"; + }); + + // \r will remain in the session name after split("\n") + // substring(13) of "aabbccddeef0-ao-15\r" is "ao-15\r" which !== "ao-15" + // This documents the current behavior — tmux shouldn't produce \r\n on unix + expect(resolveTmuxSession("ao-15", TMUX, mockExec)).toBeNull(); + }); + + it("handles very long session list", () => { + // Generate 100 sessions with valid 12-char hex prefixes + const sessions = Array.from({ length: 100 }, (_, i) => { + const hex = i.toString(16).padStart(12, "0"); + return `${hex}-session-${i}`; + }).join("\n") + "\n"; + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => sessions); + + const hex50 = (50).toString(16).padStart(12, "0"); + expect(resolveTmuxSession("session-50", TMUX, mockExec)).toBe(`${hex50}-session-50`); + }); + + it("handles session list where target is last entry", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-1\n112233445566-ao-2\nffeeddccbbaa-ao-3\na0b1c2d3e4f5-ao-target\n"; + }); + + expect(resolveTmuxSession("ao-target", TMUX, mockExec)).toBe("a0b1c2d3e4f5-ao-target"); + }); + + it("handles session list where target is first entry", () => { + const mockExec = vi.fn() + .mockImplementationOnce(() => { + throw new Error("session not found"); + }) + .mockImplementationOnce(() => { + return "aabbccddeef0-ao-target\n112233445566-ao-2\nffeeddccbbaa-ao-3\n"; + }); + + expect(resolveTmuxSession("ao-target", TMUX, mockExec)).toBe("aabbccddeef0-ao-target"); + }); + + it("works with different tmux paths", () => { + const paths = ["/opt/homebrew/bin/tmux", "/usr/local/bin/tmux", "/usr/bin/tmux", "tmux"]; + + for (const tmuxPath of paths) { + const mockExec = vi.fn().mockReturnValue(""); + resolveTmuxSession("ao-15", tmuxPath, mockExec); + expect(mockExec).toHaveBeenCalledWith(tmuxPath, ["has-session", "-t", "=ao-15"], { timeout: 5000 }); + } + }); + }); +}); diff --git a/packages/web/server/direct-terminal-ws.ts b/packages/web/server/direct-terminal-ws.ts index 1f0f00595..05badae16 100644 --- a/packages/web/server/direct-terminal-ws.ts +++ b/packages/web/server/direct-terminal-ws.ts @@ -7,29 +7,12 @@ * that tmux requires for clipboard support. */ -import { createServer } from "node:http"; +import { createServer, type Server } from "node:http"; import { spawn } from "node:child_process"; import { WebSocketServer, WebSocket } from "ws"; import { spawn as ptySpawn, type IPty } from "node-pty"; -import { existsSync } from "node:fs"; -import { join } from "node:path"; import { homedir, userInfo } from "node:os"; -import { loadConfig } from "@composio/ao-core"; - -// Load config with fallback -let config: ReturnType; -try { - config = loadConfig(); -} catch (err) { - console.warn( - "[DirectTerminal] Could not load config, using defaults:", - err instanceof Error ? err.message : String(err), - ); - // Fallback to default config - config = { - dataDir: join(homedir(), ".ao/sessions"), - } as ReturnType; -} +import { findTmux, resolveTmuxSession, validateSessionId } from "./tmux-utils.js"; interface TerminalSession { sessionId: string; @@ -37,190 +20,212 @@ interface TerminalSession { ws: WebSocket; } -const activeSessions = new Map(); - -/** - * Create HTTP server with WebSocket upgrade handling - */ -const server = createServer((req, res) => { - // Health check endpoint - if (req.url === "/health") { - res.writeHead(200, { "Content-Type": "application/json" }); - res.end( - JSON.stringify({ - active: activeSessions.size, - sessions: Array.from(activeSessions.keys()), - }), - ); - return; - } - - res.writeHead(404); - res.end("Not found"); -}); - -/** - * WebSocket server for terminal connections - */ -const wss = new WebSocketServer({ - server, - path: "/ws", -}); - -wss.on("connection", (ws, req) => { - const url = new URL(req.url ?? "/", "ws://localhost"); - const sessionId = url.searchParams.get("session"); - - if (!sessionId) { - console.error("[DirectTerminal] Missing session parameter"); - ws.close(1008, "Missing session parameter"); - return; - } - - // Validate session ID - if (!/^[a-zA-Z0-9_-]+$/.test(sessionId)) { - console.error("[DirectTerminal] Invalid session ID:", sessionId); - ws.close(1008, "Invalid session ID"); - return; - } - - // Validate session exists - const sessionPath = join(config.dataDir, sessionId); - if (!existsSync(sessionPath)) { - console.error("[DirectTerminal] Session not found:", sessionId); - ws.close(1008, "Session not found"); - return; - } - - console.log(`[DirectTerminal] New connection for session: ${sessionId}`); - - // Enable mouse mode for scrollback support - const mouseProc = spawn("tmux", ["set-option", "-t", sessionId, "mouse", "on"]); - mouseProc.on("error", (err) => { - console.error(`[DirectTerminal] Failed to set mouse mode for ${sessionId}:`, err.message); - }); - - // Hide the green status bar for cleaner appearance - const statusProc = spawn("tmux", ["set-option", "-t", sessionId, "status", "off"]); - statusProc.on("error", (err) => { - console.error(`[DirectTerminal] Failed to hide status bar for ${sessionId}:`, err.message); - }); - - // Spawn PTY attached to tmux session - // Use tmux from PATH for cross-platform compatibility - const tmuxPath = "tmux"; - - // Build complete environment - node-pty requires proper env setup - const homeDir = process.env.HOME || homedir(); - const currentUser = process.env.USER || userInfo().username; - const env = { - HOME: homeDir, - SHELL: process.env.SHELL || "/bin/bash", - USER: currentUser, - PATH: process.env.PATH || "/usr/local/bin:/usr/bin:/bin", - TERM: "xterm-256color", - LANG: process.env.LANG || "en_US.UTF-8", - TMPDIR: process.env.TMPDIR || "/tmp", - }; - - let pty: IPty; - try { - console.log(`[DirectTerminal] Spawning PTY: ${tmuxPath} attach-session -t ${sessionId}`); - console.log(`[DirectTerminal] CWD: ${homeDir}, ENV keys:`, Object.keys(env).join(", ")); - - pty = ptySpawn(tmuxPath, ["attach-session", "-t", sessionId], { - name: "xterm-256color", - cols: 80, - rows: 24, - cwd: homeDir, - env, - }); - - console.log(`[DirectTerminal] PTY spawned successfully`); - } catch (err) { - console.error(`[DirectTerminal] Failed to spawn PTY:`, err); - ws.close(1011, `Failed to spawn terminal: ${err instanceof Error ? err.message : String(err)}`); - return; - } - - const session: TerminalSession = { sessionId, pty, ws }; - activeSessions.set(sessionId, session); - - // PTY -> WebSocket - pty.onData((data) => { - if (ws.readyState === WebSocket.OPEN) { - ws.send(data); - } - }); - - // PTY exit - pty.onExit(({ exitCode }) => { - console.log(`[DirectTerminal] PTY exited for ${sessionId} with code ${exitCode}`); - activeSessions.delete(sessionId); - if (ws.readyState === WebSocket.OPEN) { - ws.close(1000, "Terminal session ended"); - } - }); - - // WebSocket -> PTY - ws.on("message", (data) => { - const message = data.toString("utf8"); - - // Handle resize messages (sent by xterm.js FitAddon) - if (message.startsWith("{")) { - try { - const parsed = JSON.parse(message) as { type?: string; cols?: number; rows?: number }; - if (parsed.type === "resize" && parsed.cols && parsed.rows) { - pty.resize(parsed.cols, parsed.rows); - return; - } - } catch { - // Not JSON, treat as terminal input - } - } - - // Normal terminal input - pty.write(message); - }); - - // WebSocket close - ws.on("close", () => { - console.log(`[DirectTerminal] WebSocket closed for ${sessionId}`); - activeSessions.delete(sessionId); - pty.kill(); - }); - - // WebSocket error - ws.on("error", (err) => { - console.error(`[DirectTerminal] WebSocket error for ${sessionId}:`, err.message); - activeSessions.delete(sessionId); - pty.kill(); - }); -}); - -const PORT = parseInt(process.env.DIRECT_TERMINAL_PORT ?? "3003", 10); - -server.listen(PORT, () => { - console.log(`[DirectTerminal] WebSocket server listening on port ${PORT}`); -}); - -// Graceful shutdown -function shutdown(signal: string) { - console.log(`[DirectTerminal] Received ${signal}, shutting down...`); - for (const [, session] of activeSessions) { - session.pty.kill(); - session.ws.close(1001, "Server shutting down"); - } - server.close(() => { - console.log("[DirectTerminal] Server closed"); - process.exit(0); - }); - const forceExitTimer = setTimeout(() => { - console.error("[DirectTerminal] Forced shutdown after timeout"); - process.exit(1); - }, 5000); - forceExitTimer.unref(); +export interface DirectTerminalServer { + server: Server; + wss: WebSocketServer; + activeSessions: Map; + shutdown: () => void; } -process.on("SIGINT", () => shutdown("SIGINT")); -process.on("SIGTERM", () => shutdown("SIGTERM")); +/** + * Create the direct terminal WebSocket server. + * Separated from listen() so tests can control lifecycle. + */ +export function createDirectTerminalServer(tmuxPath?: string): DirectTerminalServer { + const TMUX = tmuxPath ?? findTmux(); + const activeSessions = new Map(); + + const server = createServer((req, res) => { + if (req.url === "/health") { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end( + JSON.stringify({ + active: activeSessions.size, + sessions: Array.from(activeSessions.keys()), + }), + ); + return; + } + + res.writeHead(404); + res.end("Not found"); + }); + + const wss = new WebSocketServer({ + server, + path: "/ws", + }); + + wss.on("connection", (ws, req) => { + const url = new URL(req.url ?? "/", "ws://localhost"); + const sessionId = url.searchParams.get("session"); + + if (!sessionId) { + console.error("[DirectTerminal] Missing session parameter"); + ws.close(1008, "Missing session parameter"); + return; + } + + // Validate session ID format + if (!validateSessionId(sessionId)) { + console.error("[DirectTerminal] Invalid session ID:", sessionId); + ws.close(1008, "Invalid session ID"); + return; + } + + // Resolve tmux session name: try exact match first, then suffix match + // (hash-prefixed sessions like "8474d6f29887-ao-15" are accessed by user-facing ID "ao-15") + const tmuxSessionId = resolveTmuxSession(sessionId, TMUX); + if (!tmuxSessionId) { + console.error("[DirectTerminal] tmux session not found:", sessionId); + ws.close(1008, "Session not found"); + return; + } + + console.log(`[DirectTerminal] New connection for session: ${tmuxSessionId}`); + + // Enable mouse mode for scrollback support + const mouseProc = spawn(TMUX, ["set-option", "-t", tmuxSessionId, "mouse", "on"]); + mouseProc.on("error", (err) => { + console.error( + `[DirectTerminal] Failed to set mouse mode for ${tmuxSessionId}:`, + err.message, + ); + }); + + // Hide the green status bar for cleaner appearance + const statusProc = spawn(TMUX, ["set-option", "-t", tmuxSessionId, "status", "off"]); + statusProc.on("error", (err) => { + console.error( + `[DirectTerminal] Failed to hide status bar for ${tmuxSessionId}:`, + err.message, + ); + }); + + // Build complete environment - node-pty requires proper env setup + const homeDir = process.env.HOME || homedir(); + const currentUser = process.env.USER || userInfo().username; + const env = { + HOME: homeDir, + SHELL: process.env.SHELL || "/bin/bash", + USER: currentUser, + PATH: process.env.PATH || "/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin", + TERM: "xterm-256color", + LANG: process.env.LANG || "en_US.UTF-8", + TMPDIR: process.env.TMPDIR || "/tmp", + }; + + let pty: IPty; + try { + console.log(`[DirectTerminal] Spawning PTY: tmux attach-session -t ${tmuxSessionId}`); + + pty = ptySpawn(TMUX, ["attach-session", "-t", tmuxSessionId], { + name: "xterm-256color", + cols: 80, + rows: 24, + cwd: homeDir, + env, + }); + + console.log(`[DirectTerminal] PTY spawned successfully`); + } catch (err) { + console.error(`[DirectTerminal] Failed to spawn PTY:`, err); + ws.close(1011, `Failed to spawn terminal: ${err instanceof Error ? err.message : String(err)}`); + return; + } + + const session: TerminalSession = { sessionId, pty, ws }; + activeSessions.set(sessionId, session); + + // PTY -> WebSocket + pty.onData((data) => { + if (ws.readyState === WebSocket.OPEN) { + ws.send(data); + } + }); + + // PTY exit + pty.onExit(({ exitCode }) => { + console.log(`[DirectTerminal] PTY exited for ${sessionId} with code ${exitCode}`); + activeSessions.delete(sessionId); + if (ws.readyState === WebSocket.OPEN) { + ws.close(1000, "Terminal session ended"); + } + }); + + // WebSocket -> PTY + ws.on("message", (data) => { + const message = data.toString("utf8"); + + // Handle resize messages (sent by xterm.js FitAddon) + if (message.startsWith("{")) { + try { + const parsed = JSON.parse(message) as { type?: string; cols?: number; rows?: number }; + if (parsed.type === "resize" && parsed.cols && parsed.rows) { + pty.resize(parsed.cols, parsed.rows); + return; + } + } catch { + // Not JSON, treat as terminal input + } + } + + // Normal terminal input + pty.write(message); + }); + + // WebSocket close + ws.on("close", () => { + console.log(`[DirectTerminal] WebSocket closed for ${sessionId}`); + activeSessions.delete(sessionId); + pty.kill(); + }); + + // WebSocket error + ws.on("error", (err) => { + console.error(`[DirectTerminal] WebSocket error for ${sessionId}:`, err.message); + activeSessions.delete(sessionId); + pty.kill(); + }); + }); + + function shutdown() { + for (const [, session] of activeSessions) { + session.pty.kill(); + session.ws.close(1001, "Server shutting down"); + } + server.close(); + } + + return { server, wss, activeSessions, shutdown }; +} + +// --- Run as standalone script --- +// Only start the server when executed directly (not imported by tests) +const isMainModule = process.argv[1]?.endsWith("direct-terminal-ws.ts") || + process.argv[1]?.endsWith("direct-terminal-ws.js"); + +if (isMainModule) { + const TMUX = findTmux(); + console.log(`[DirectTerminal] Using tmux: ${TMUX}`); + + const { server, shutdown } = createDirectTerminalServer(TMUX); + const PORT = parseInt(process.env.DIRECT_TERMINAL_PORT ?? "3003", 10); + + server.listen(PORT, () => { + console.log(`[DirectTerminal] WebSocket server listening on port ${PORT}`); + }); + + function handleShutdown(signal: string) { + console.log(`[DirectTerminal] Received ${signal}, shutting down...`); + shutdown(); + const forceExitTimer = setTimeout(() => { + console.error("[DirectTerminal] Forced shutdown after timeout"); + process.exit(1); + }, 5000); + forceExitTimer.unref(); + } + + process.on("SIGINT", () => handleShutdown("SIGINT")); + process.on("SIGTERM", () => handleShutdown("SIGTERM")); +} diff --git a/packages/web/server/terminal-websocket.ts b/packages/web/server/terminal-websocket.ts index 05369644e..b8840b7a9 100644 --- a/packages/web/server/terminal-websocket.ts +++ b/packages/web/server/terminal-websocket.ts @@ -15,9 +15,11 @@ import { spawn, type ChildProcess } from "node:child_process"; import { createServer, request } from "node:http"; -import { existsSync } from "node:fs"; -import { join } from "node:path"; -import { loadConfig } from "@composio/ao-core"; +import { findTmux, resolveTmuxSession, validateSessionId } from "./tmux-utils.js"; + +/** Cached full path to tmux binary */ +const TMUX = findTmux(); +console.log(`[Terminal] Using tmux: ${TMUX}`); interface TtydInstance { sessionId: string; @@ -30,8 +32,6 @@ const availablePorts = new Set(); // Pool of recycled ports let nextPort = 7800; // Start ttyd instances from port 7800 const MAX_PORT = 7900; // Prevent unbounded port allocation -// Load config once at startup -const config = loadConfig(); /** * Check if ttyd is ready to accept connections by making a test request. @@ -105,7 +105,13 @@ function waitForTtyd(port: number, sessionId: string, timeoutMs = 3000): Promise }); } -function getOrSpawnTtyd(sessionId: string): TtydInstance { +/** + * Spawn or reuse a ttyd instance for a tmux session. + * + * @param sessionId - User-facing session ID (used for base-path and URL) + * @param tmuxSessionName - Actual tmux session name (may be hash-prefixed) + */ +function getOrSpawnTtyd(sessionId: string, tmuxSessionName: string): TtydInstance { const existing = instances.get(sessionId); if (existing) return existing; @@ -123,20 +129,22 @@ function getOrSpawnTtyd(sessionId: string): TtydInstance { port = nextPort++; } - console.log(`[Terminal] Spawning ttyd for ${sessionId} on port ${port}`); + console.log(`[Terminal] Spawning ttyd for ${tmuxSessionName} on port ${port}`); // Enable mouse mode for scrollback support - const mouseProc = spawn("tmux", ["set-option", "-t", sessionId, "mouse", "on"]); + const mouseProc = spawn(TMUX, ["set-option", "-t", tmuxSessionName, "mouse", "on"]); mouseProc.on("error", (err) => { - console.error(`[Terminal] Failed to set mouse mode for ${sessionId}:`, err.message); + console.error(`[Terminal] Failed to set mouse mode for ${tmuxSessionName}:`, err.message); }); // Hide the green status bar for cleaner appearance - const statusProc = spawn("tmux", ["set-option", "-t", sessionId, "status", "off"]); + const statusProc = spawn(TMUX, ["set-option", "-t", tmuxSessionName, "status", "off"]); statusProc.on("error", (err) => { - console.error(`[Terminal] Failed to hide status bar for ${sessionId}:`, err.message); + console.error(`[Terminal] Failed to hide status bar for ${tmuxSessionName}:`, err.message); }); + // Use user-facing sessionId for base-path (matches URL the dashboard uses) + // Use tmuxSessionName for tmux attach (may be hash-prefixed) const proc = spawn( "ttyd", [ @@ -145,10 +153,10 @@ function getOrSpawnTtyd(sessionId: string): TtydInstance { String(port), "--base-path", `/${sessionId}`, - "tmux", + TMUX, "attach-session", "-t", - sessionId, + tmuxSessionName, ], { stdio: ["ignore", "pipe", "pipe"], @@ -241,15 +249,16 @@ const server = createServer(async (req, res) => { } // Validate session ID to prevent path traversal and injection - if (!/^[a-zA-Z0-9_-]+$/.test(sessionId)) { + if (!validateSessionId(sessionId)) { res.writeHead(400, { "Content-Type": "application/json" }); res.end(JSON.stringify({ error: "Invalid session ID" })); return; } - // Validate session exists before spawning ttyd using configured dataDir - const sessionPath = join(config.dataDir, sessionId); - if (!existsSync(sessionPath)) { + // Resolve tmux session name: try exact match first, then suffix match + // (hash-prefixed sessions like "8474d6f29887-ao-15" are accessed by user-facing ID "ao-15") + const tmuxSessionId = resolveTmuxSession(sessionId, TMUX); + if (!tmuxSessionId) { res.writeHead(404, { "Content-Type": "application/json" }); res.end(JSON.stringify({ error: "Session not found" })); return; @@ -257,7 +266,7 @@ const server = createServer(async (req, res) => { // Spawn ttyd and wait for it to be ready (catch port exhaustion and startup failures) try { - const instance = getOrSpawnTtyd(sessionId); + const instance = getOrSpawnTtyd(sessionId, tmuxSessionId); await waitForTtyd(instance.port, sessionId); // Use the request host to construct the terminal URL (supports remote access) diff --git a/packages/web/server/tmux-utils.ts b/packages/web/server/tmux-utils.ts new file mode 100644 index 000000000..6a476fbe1 --- /dev/null +++ b/packages/web/server/tmux-utils.ts @@ -0,0 +1,102 @@ +/** + * Shared tmux utilities for terminal servers. + * + * Extracted from direct-terminal-ws.ts and terminal-websocket.ts + * so the logic can be properly unit tested. + */ + +import { execFileSync } from "node:child_process"; + +/** Session ID validation regex — alphanumeric, hyphens, underscores only */ +export const SESSION_ID_PATTERN = /^[a-zA-Z0-9_-]+$/; + +/** Hash prefix pattern — 12-char lowercase hex, as generated by generateConfigHash */ +const HASH_PREFIX_PATTERN = /^[a-f0-9]{12}-/; + +/** + * Validate a session ID format. + * Prevents path traversal, shell injection, and other attacks. + */ +export function validateSessionId(sessionId: string): boolean { + return SESSION_ID_PATTERN.test(sessionId); +} + +/** + * Find full path to tmux binary. + * + * Checks common installation locations because node-pty's posix_spawnp + * doesn't reliably inherit PATH, and some Node.js environments (e.g., + * launched from GUI apps) have minimal PATH. + * + * @param execFn - Injectable execFileSync for testing. Defaults to child_process.execFileSync. + */ +export function findTmux( + execFn: typeof execFileSync = execFileSync, +): string { + const candidates = [ + "/opt/homebrew/bin/tmux", // macOS ARM (Homebrew) + "/usr/local/bin/tmux", // macOS Intel (Homebrew) + "/usr/bin/tmux", // Linux + ]; + for (const p of candidates) { + try { + execFn(p, ["-V"], { timeout: 5000 }); + return p; + } catch { + continue; + } + } + return "tmux"; // Fall back to bare name +} + +/** + * Resolve a user-facing session ID to its actual tmux session name. + * + * The hash-based architecture prefixes tmux session names with a config hash + * (e.g., "8474d6f29887-ao-15" for user-facing "ao-15"). This function: + * + * 1. Tries exact match first using tmux's `=` prefix syntax to prevent + * prefix matching (where "ao-1" would incorrectly match "ao-15"). + * 2. Falls back to listing all sessions and finding one with a 12-char hex + * prefix followed by the exact session ID (e.g., `{12-hex}-{sessionId}`). + * + * @param sessionId - User-facing session ID (e.g., "ao-15") + * @param tmuxPath - Full path to tmux binary + * @param execFn - Injectable execFileSync for testing. Defaults to child_process.execFileSync. + * @returns The actual tmux session name, or null if not found + */ +export function resolveTmuxSession( + sessionId: string, + tmuxPath: string, + execFn: typeof execFileSync = execFileSync, +): string | null { + // Try exact match first using = prefix for exact matching (e.g., "ao-orchestrator") + // Without =, tmux uses prefix matching: "ao-1" would match "ao-15" + try { + execFn(tmuxPath, ["has-session", "-t", `=${sessionId}`], { timeout: 5000 }); + return sessionId; + } catch { + // Not an exact match + } + + // Search for hash-prefixed tmux session (e.g., "8474d6f29887-ao-15" for "ao-15") + // Validate the 12-char hex prefix to avoid ambiguous suffix matches where + // "hash-my-app-1" could falsely match a lookup for "app-1". + try { + const output = execFn(tmuxPath, ["list-sessions", "-F", "#{session_name}"], { + timeout: 5000, + encoding: "utf8", + }) as string; + const sessions = output.split("\n").filter(Boolean); + const match = sessions.find((s) => + HASH_PREFIX_PATTERN.test(s) && s.substring(13) === sessionId, + ); + if (match) { + return match; + } + } catch { + // tmux not running or no sessions + } + + return null; +} diff --git a/packages/web/tsconfig.json b/packages/web/tsconfig.json index 58736ffd0..536398df7 100644 --- a/packages/web/tsconfig.json +++ b/packages/web/tsconfig.json @@ -22,6 +22,6 @@ "declarationMap": false, "verbatimModuleSyntax": false }, - "include": ["next-env.d.ts", "src", ".next/types/**/*.ts"], + "include": ["next-env.d.ts", "src", "server", ".next/types/**/*.ts"], "exclude": ["node_modules", "src/**/*.test.tsx", "src/**/__tests__"] } diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index 03c46261e..f3cbb2328 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -8,7 +8,7 @@ export default defineConfig({ globals: true, environment: "jsdom", setupFiles: ["./src/__tests__/setup.ts"], - include: ["src/**/*.test.{ts,tsx}"], + include: ["src/**/*.test.{ts,tsx}", "server/**/*.test.ts"], }, resolve: { alias: {