diff --git a/src/servers/api/terminal/pty-sidecar.mjs b/src/servers/api/terminal/pty-sidecar.mjs index f2b4f5a1..72cc72d6 100644 --- a/src/servers/api/terminal/pty-sidecar.mjs +++ b/src/servers/api/terminal/pty-sidecar.mjs @@ -10,24 +10,10 @@ process.on('SIGINT', () => { }); process.on('SIGTERM', () => process.emit('SIGINT')); -import { existsSync } from 'node:fs'; -import { cp, mkdir } from 'node:fs/promises'; -import { join, dirname } from 'node:path'; -import { fileURLToPath } from 'node:url'; -import { execFile } from 'node:child_process'; +import { join } from 'node:path'; import WebSocket from 'ws'; import * as pty from 'node-pty'; -const run = (cmd, args, opts = {}) => - new Promise((resolve) => { - const proc = execFile(cmd, args, { stdio: 'ignore', ...opts }, () => resolve()); - proc.on('error', () => resolve()); - }); - -const __dirname = dirname(fileURLToPath(import.meta.url)); - -const templateDir = join(__dirname, 'templates'); - import 'dotenv/config'; const API_URL = process.env.API_URL ?? `ws://127.0.0.1:${process.env.PORT ?? '5000'}`; @@ -36,7 +22,31 @@ const REGISTER_URL = `${API_URL}/api/sidecar/register`; const BUFFER_MAX = 50 * 1024; const RECONNECT_DELAYS = [200, 500, 1000, 2000, 4000, 8000, 15000]; -/** @type {Map} */ +// ── What kind of shell this process runs ── +// +// These used to arrive inside every pty:init, which meant officer chose the owner's shell and read the +// owner's HOME to do it. They are this process's business: it is the one that spawns the thing. +// +// HOME_DIR mirrors `data-path.ts:getOwnerHomeDir` — on a host where the owner's real login home differs +// from this process's HOME, the shell should open in the former, like every other host-executing surface. +const HOME_DIR = process.env.HOME_DIR ?? process.env.HOME ?? process.cwd(); +const SHELL = { command: process.env.SHELL ?? '/bin/zsh', args: ['-i'] }; +// A terminal is always a plain host shell: the owner is the only account and it is their own machine +// (`platform/CLAUDE.md` — do not add a jail without being asked). There used to be a second branch here +// for a bwrap sandbox, selected by `config.host`, which officer hardcoded to true. Nothing ever built the +// bwrap command it expected, on either side, so it could not have run — it is in git history if the +// decision is ever revisited, and the `ensureUserFiles` half of it duplicated +// `api/users/provision.ts:seedShellConfigs`, which is the live seeder of those templates. + +const resolveCwd = (cwd) => { + if (!cwd || cwd === '~') return HOME_DIR; + if (cwd.startsWith('~/')) return join(HOME_DIR, cwd.slice(2)); + if (cwd.startsWith('/')) return cwd; + // Relative paths have no meaning here — this process's cwd is the repo, not the user's folder. + return HOME_DIR; +}; + +/** @type {Map} */ const sessions = new Map(); // ── Helpers ── @@ -59,32 +69,6 @@ const sendJson = (msg) => { } }; -const ensureUserFiles = async (homeDir) => { - await mkdir(homeDir, { recursive: true }); - await mkdir(join(homeDir, '.config'), { recursive: true }); - await mkdir(join(homeDir, '.local', 'bin'), { recursive: true }); - - const zshrcPath = join(homeDir, '.zshrc'); - if (!existsSync(zshrcPath)) { - await cp(join(templateDir, '.zshrc'), zshrcPath); - } - - const tmuxconfPath = join(homeDir, '.tmux.conf'); - if (!existsSync(tmuxconfPath)) { - await cp(join(templateDir, '.tmux.conf'), tmuxconfPath); - } - - const starshipPath = join(homeDir, '.config', 'starship-officer.toml'); - if (!existsSync(starshipPath)) { - await cp(join(templateDir, 'starship-officer.toml'), starshipPath); - } - - const ohMyZshPath = join(homeDir, '.oh-my-zsh'); - if (!existsSync(ohMyZshPath)) { - await run('git', ['clone', '--depth=1', 'https://github.com/ohmyzsh/ohmyzsh.git', ohMyZshPath]); - } -}; - const appendBuffer = (session, data) => { session.buffer += data; if (session.buffer.length > BUFFER_MAX) { @@ -130,47 +114,18 @@ async function handleCommand(msg) { } // New session — spawn PTY - const shell = config.shell ?? { command: '/bin/bash', args: ['-i'] }; - const cwd = config.cwd ?? process.cwd(); - const homeDir = config.homeDir ?? process.cwd(); - const userLabel = config.userLabel ?? 'officer'; + const cwd = resolveCwd(config.cwd); const cols = config.cols ?? 80; const rows = config.rows ?? 24; - const isHost = !!config.host; - - let spawnCommand; - let spawnArgs; - let ptyEnv; - - if (isHost) { - spawnCommand = shell.command; - spawnArgs = shell.args ?? []; - ptyEnv = { ...process.env, TERM: 'xterm-256color', ...(config.env ?? {}) }; - } else { - // Sandboxed mode: shell config contains the full bwrap command - spawnCommand = shell.command; - spawnArgs = shell.args ?? []; - - try { - await ensureUserFiles(homeDir); - } catch (err) { - console.error('[pty-sidecar] ensureUserFiles failed:', err); - } - - // bwrap sets env vars internally via --setenv, so use minimal host env - ptyEnv = { TERM: 'xterm-256color' }; - } - - const ptyCwd = isHost ? cwd : undefined; let term; try { - term = pty.spawn(spawnCommand, spawnArgs, { + term = pty.spawn(SHELL.command, SHELL.args, { name: 'xterm-256color', cols, rows, - cwd: ptyCwd, - env: ptyEnv, + cwd, + env: { ...process.env, TERM: 'xterm-256color' }, }); } catch (err) { const message = err instanceof Error ? err.message : 'Failed to start terminal'; @@ -179,13 +134,7 @@ async function handleCommand(msg) { return; } - const session = { - term, - buffer: '', - cols, - rows, - initConfig: { shell, cwd, homeDir, userLabel }, - }; + const session = { term, buffer: '', cols, rows }; sessions.set(sessionId, session); term.onData((output) => { diff --git a/src/servers/api/terminal/pty-sidecar.test.ts b/src/servers/api/terminal/pty-sidecar.test.ts index ba774e01..000de990 100644 --- a/src/servers/api/terminal/pty-sidecar.test.ts +++ b/src/servers/api/terminal/pty-sidecar.test.ts @@ -81,31 +81,28 @@ describe('pty sidecar', () => { const port = officer.port; child = Bun.spawn(['node', SIDECAR], { - env: { ...process.env, API_URL: `ws://127.0.0.1:${port}` }, + env: { + ...process.env, + API_URL: `ws://127.0.0.1:${port}`, + // The sidecar now chooses the shell and the home itself, so the test pins both rather than + // spawning the owner's interactive zsh (which would read their rc files and their history). + SHELL: '/bin/sh', + HOME_DIR: '/tmp', + ENV: '/dev/null', + }, stdout: 'ignore', stderr: 'ignore', }); await officer.await((f) => f.type === 'register' && f.capabilities?.includes('terminal')); - // A plain non-interactive shell: no rc files, no prompt, so the assertions below are about output - // the test itself caused. - officer.send({ - type: 'pty:init', - id: 'i1', - sessionId, - config: { - sessionId, - host: true, - shell: { command: '/bin/sh', args: [] }, - cwd: '/tmp', - homeDir: '/tmp', - cols: 80, - rows: 24, - }, - }); + // `~` is resolved by the sidecar against its own HOME_DIR, not by officer. + officer.send({ type: 'pty:init', id: 'i1', sessionId, config: { sessionId, cwd: '~', cols: 80, rows: 24 } }); await officer.await((f) => f.type === 'pty:ready' && f.sessionId === sessionId); + officer.send({ type: 'pty:input', id: 'in0', sessionId, data: 'pwd\n' }); + await officer.await(isOutput(sessionId, '/tmp')); + officer.send({ type: 'pty:input', id: 'in1', sessionId, data: 'echo before-restart\n' }); await officer.await(isOutput(sessionId, 'before-restart')); @@ -121,7 +118,7 @@ describe('pty sidecar', () => { // Re-attaching replays the scrollback, marked as history so the client can rebuild rather than // append — and it contains what happened on both sides of the restart. - officer.send({ type: 'pty:init', id: 'i2', sessionId, config: { sessionId, host: true, cols: 80, rows: 24 } }); + officer.send({ type: 'pty:init', id: 'i2', sessionId, config: { sessionId, cols: 80, rows: 24 } }); const replay = await officer.await((f) => f.type === 'pty:replay' && f.sessionId === sessionId); expect(String(replay.data)).toContain('before-restart'); expect(String(replay.data)).toContain('after-restart'); diff --git a/src/servers/api/terminal/websocket.ts b/src/servers/api/terminal/websocket.ts index 045fe9e0..97dcb95e 100644 --- a/src/servers/api/terminal/websocket.ts +++ b/src/servers/api/terminal/websocket.ts @@ -1,5 +1,4 @@ import type { ServerWebSocket } from 'bun'; -import { join } from 'node:path'; import { sendPtyCommand, sendPtyCommandAsync, on, isTerminalConnected } from '@@/sidecar-registry'; import type { PtyInitConfig } from '../../sidecar/protocol'; @@ -34,13 +33,6 @@ const sendOutput = (ws: ServerWebSocket, data: string) => { } }; -const resolveCwd = (home: string, cwd?: string) => { - if (!cwd || cwd === '~') return home; - if (cwd.startsWith('~/')) return join(home, cwd.slice(2)); - if (cwd.startsWith('/')) return cwd; - return home; -}; - export const terminalWebsocket = { async open(ws: ServerWebSocket) { const { email, username } = ws.data; @@ -54,17 +46,10 @@ export const terminalWebsocket = { const sessionId = ws.data.sessionId ?? `host-${ws.data.userId}`; - // The server owner is the only account, so the terminal is always a plain host shell. - const config: PtyInitConfig = { - sessionId, - host: true, - shell: { command: process.env.SHELL ?? '/bin/zsh', args: ['-i'] }, - cwd: resolveCwd(process.env.HOME!, ws.data.cwd), - homeDir: process.env.HOME!, - userLabel: email, - cols: ws.data.cols, - rows: ws.data.rows, - }; + // Everything this bridge knows: which session, which folder the panel was opened on, and how big the + // client's viewport is. The shell, its arguments and the home directory are the sidecar's — it is the + // process that spawns them, and officer has no business reading the owner's SHELL and HOME to guess. + const config: PtyInitConfig = { sessionId, cwd: ws.data.cwd, cols: ws.data.cols, rows: ws.data.rows }; // The sidecar emits one global stream, so each frame is filtered down to this session and relabelled. const relay = (event: 'pty:output' | 'pty:replay' | 'pty:exit', clientType: string) => @@ -118,16 +103,9 @@ export const terminalWebsocket = { }); } break; - case 'cwd': - if (msg.path) { - sendPtyCommand({ - type: 'pty:input', - id: nextId(), - sessionId: session.sessionId, - data: `cd ${JSON.stringify(msg.path)}\r`, - }); - } - break; + // There was a 'cwd' case here that typed `cd \r` into the user's shell. No frontend sends + // that message — the browser composes its own `cd` (Terminal.tsx / CommandTerminalWrapper.tsx) — + // so it was unreachable, and synthesizing keystrokes is not a thing a proxy should do. } } catch { // ignore malformed messages diff --git a/src/servers/sidecar/protocol.ts b/src/servers/sidecar/protocol.ts index 522df93d..f9177f72 100644 --- a/src/servers/sidecar/protocol.ts +++ b/src/servers/sidecar/protocol.ts @@ -130,16 +130,15 @@ export type VncSessionInfo = { // ── PTY types ── +// What officer knows about a terminal, and nothing more. The shell, its arguments, the home directory and +// whether the shell is sandboxed are the sidecar's own decisions — they used to travel in here, which is +// how officer ended up reading the owner's SHELL and HOME and hardcoding `host: true`. export type PtyInitConfig = { sessionId: string; - shell?: { command: string; args?: string[] }; + /** The folder the panel was opened on. `~`, `~/x` and absolute paths only; resolved by the sidecar. */ cwd?: string; - homeDir?: string; - userLabel?: string; - host?: boolean; cols?: number; rows?: number; - env?: Record; }; // PTY commands (API → PTY sidecar)