From 4505471149b06a5c4d7c39f140deeeca2c2677ed Mon Sep 17 00:00:00 2001 From: Gabriel Brown Date: Mon, 13 Jul 2026 10:36:25 -0400 Subject: [PATCH] fix(worker): keep-id compat, zombie tmux clients, nvm in box image - Box init + chownInBox exec explicitly as root: under podman keep-id the container's default user is the mapped uid, which cannot useradd/chown. - Init renames an existing uid-1000 passwd entry (podman keep-id injects one named after the HOST user) instead of adding a duplicate that would win the getpwuid lookup and make whoami report the wrong name. - Terminal connections tag their shell with a per-connection marker and reap the process group on WebSocket close: closing the attach socket never killed the exec'd shell, so every disconnect leaked an attached tmux client (session stuck at the smallest zombie's size; enough dead clients starve tmux rendering for live ones). - Job image: nvm 0.39.5 at /usr/local/nvm (user-writable) with /etc/profile.d/nvm.sh, matching what user dotfiles expect; docs for the box user + ownership model. --- apps/agent-worker/src/box-init.ts | 3 ++ apps/agent-worker/src/runtime/box-user.ts | 19 +++++++---- apps/agent-worker/src/runtime/docker.ts | 4 +++ apps/agent-worker/src/terminal.ts | 32 ++++++++++++++++--- apps/agent-worker/tests/unit/box-user.test.ts | 10 ++++-- .../tests/unit/terminal-resize.test.ts | 15 +++++++++ docker/agent-job.Dockerfile | 10 ++++++ docs/agent-terminal.md | 25 +++++++++++++++ 8 files changed, 106 insertions(+), 12 deletions(-) diff --git a/apps/agent-worker/src/box-init.ts b/apps/agent-worker/src/box-init.ts index f70faad..7213bd7 100644 --- a/apps/agent-worker/src/box-init.ts +++ b/apps/agent-worker/src/box-init.ts @@ -27,6 +27,9 @@ const execRoot = async ( redact, timeoutMs: 120_000, input, + // Explicit: under podman keep-id the container's DEFAULT user is the + // mapped uid (1000), not root, and useradd/chpasswd/sudoers need root. + user: 'root', }); if (result.exitCode !== 0) { throw new Error(`Box ${label} failed: ${result.output}`); diff --git a/apps/agent-worker/src/runtime/box-user.ts b/apps/agent-worker/src/runtime/box-user.ts index 320601a..215a968 100644 --- a/apps/agent-worker/src/runtime/box-user.ts +++ b/apps/agent-worker/src/runtime/box-user.ts @@ -18,11 +18,15 @@ export const linuxUsername = (spoonUsername: string): string => { return /^[a-z_]/.test(cleaned) ? cleaned : `u${cleaned}`.slice(0, 32); }; -// Idempotent root init: create the user at the (already mounted) home, grant -// wheel, and chown the home ONCE (marker-guarded — later host-side writes are -// repaired by targeted chowns, so a big home is never re-walked). `-o` allows -// the uid even if the image ever gains a uid-1000 user; `-M` skips skeleton -// copy since the mounted home persists across containers. +// Idempotent root init: ensure a SINGLE uid-1000 passwd entry with our name, +// grant wheel, and chown the home ONCE (marker-guarded — later host-side +// writes are repaired by targeted chowns, so a big home is never re-walked). +// +// The uid-1000 entry may already exist: podman --userns=keep-id injects one +// named after the HOST user. A second entry at the same uid would win the +// getpwuid lookup (whoami would report the host username), so an existing +// entry is RENAMED rather than duplicated. `-M` skips skeleton copy since the +// mounted home persists across containers. export const buildBoxInitScript = (args: { username: string; home: string; @@ -32,7 +36,10 @@ export const buildBoxInitScript = (args: { const marker = shellQuote(`${args.home}/.spoon/chown-v1`); return [ 'set -e', - `if ! id -u ${user} >/dev/null 2>&1; then`, + `existing="$(getent passwd ${BOX_UID} | cut -d: -f1 || true)"`, + `if [ -n "$existing" ] && [ "$existing" != ${user} ]; then`, + ` usermod -l ${user} -d ${home} -s /bin/bash "$existing"`, + `elif [ -z "$existing" ]; then`, ` useradd -u ${BOX_UID} -o -M -d ${home} -s /bin/bash ${user}`, 'fi', `usermod -aG wheel ${user}`, diff --git a/apps/agent-worker/src/runtime/docker.ts b/apps/agent-worker/src/runtime/docker.ts index d4ac50e..fd08455 100644 --- a/apps/agent-worker/src/runtime/docker.ts +++ b/apps/agent-worker/src/runtime/docker.ts @@ -445,6 +445,10 @@ export const chownInBox = async (args: { containerRuntime(), [ 'exec', + // Explicit root: under podman keep-id the container default user is the + // mapped uid, which cannot chown files it does not own. + '-u', + 'root', args.containerName, 'chown', '-R', diff --git a/apps/agent-worker/src/terminal.ts b/apps/agent-worker/src/terminal.ts index 86dc9cf..a2d097a 100644 --- a/apps/agent-worker/src/terminal.ts +++ b/apps/agent-worker/src/terminal.ts @@ -1,3 +1,4 @@ +import { randomUUID } from 'node:crypto'; import type { Server } from 'node:http'; import type { Duplex } from 'node:stream'; import type { WebSocket } from 'ws'; @@ -7,7 +8,11 @@ import type { BoxHandle } from './user-container'; import { boxHomePaths } from './box'; import { env } from './env'; import { linuxUsername } from './runtime/box-user'; -import { chownInBox, getDockerClient } from './runtime/docker'; +import { + chownInBox, + getDockerClient, + killBoxProcessesByMarker, +} from './runtime/docker'; import { openExecStream } from './runtime/exec-stream'; import { parseBoxTokenUsername, @@ -53,10 +58,19 @@ const closeWithReason = (ws: WebSocket, code: number, reason: string) => { // The login-shell command shared by the job and box terminals: prefer a // resumable tmux session, else fall back to a plain interactive login shell. -const SHELL_CMD = [ +// +// The marker comment keeps a per-connection tag in the wrapper bash's argv so +// the bridge can kill THIS connection's process group when the WebSocket +// closes. Without it, every disconnect leaks an attached tmux client (the +// exec process survives the closed attach socket); the session then stays +// sized to the smallest zombie client, and once enough dead clients' ptys +// fill up, tmux stops rendering for live ones entirely. tmux/bash must NOT be +// exec'd — that would replace the marker-carrying argv. +export const buildTerminalShellCommand = (marker: string): string[] => [ '/bin/bash', '-lc', - 'if command -v tmux >/dev/null 2>&1; then exec tmux new-session -A -s spoon; else exec bash -il; fi', + `# ${marker} +if command -v tmux >/dev/null 2>&1; then tmux new-session -A -s spoon; else bash -il; fi`, ]; type ShellBridgeOptions = { @@ -107,6 +121,7 @@ const runShellBridge = async (ws: WebSocket, opts: ShellBridgeOptions) => { else pendingInput.push(data); }); + const marker = `spoon-term-${randomUUID()}`; const handleHolder: { current?: BoxHandle } = {}; let released = false; // Read through a function so TS doesn't narrow `released` to a constant — the @@ -117,6 +132,15 @@ const runShellBridge = async (ws: WebSocket, opts: ShellBridgeOptions) => { released = true; execHolder.current?.stream.end(); execHolder.current?.stream.destroy(); + // Closing the attach socket does NOT kill the exec'd shell — reap this + // connection's process group so dead tmux clients don't accumulate. The + // tmux server daemonized into its own group and survives. + const boxName = handleHolder.current?.boxName; + if (boxName && execHolder.current) { + void killBoxProcessesByMarker({ containerName: boxName, marker }).catch( + () => undefined, + ); + } handleHolder.current?.release(); }; ws.on('close', cleanup); @@ -166,7 +190,7 @@ const runShellBridge = async (ws: WebSocket, opts: ShellBridgeOptions) => { AttachStderr: true, Tty: true, User: opts.user, - Cmd: SHELL_CMD, + Cmd: buildTerminalShellCommand(marker), Env: ['TERM=xterm-256color', ...opts.envFlags], WorkingDir: opts.cwd, }); diff --git a/apps/agent-worker/tests/unit/box-user.test.ts b/apps/agent-worker/tests/unit/box-user.test.ts index cd6e0c8..ef2b630 100644 --- a/apps/agent-worker/tests/unit/box-user.test.ts +++ b/apps/agent-worker/tests/unit/box-user.test.ts @@ -31,8 +31,14 @@ describe('buildBoxInitScript', () => { username: 'gabriel', home: '/home/Gabriel', }); - it('creates the user idempotently with uid 1000 at the given home', () => { - expect(script).toContain("id -u 'gabriel'"); + it('renames an existing uid-1000 entry instead of duplicating it', () => { + // podman --userns=keep-id injects a passwd entry named after the HOST user + // at the mapped uid; a second uid-1000 entry would win the getpwuid lookup + // and make whoami report the host username. + expect(script).toContain('getent passwd 1000'); + expect(script).toContain("usermod -l 'gabriel'"); + }); + it('creates the user with uid 1000 at the given home when none exists', () => { expect(script).toContain('useradd'); expect(script).toContain('-u 1000'); expect(script).toContain("-d '/home/Gabriel'"); diff --git a/apps/agent-worker/tests/unit/terminal-resize.test.ts b/apps/agent-worker/tests/unit/terminal-resize.test.ts index 4a7d1fe..7eb907d 100644 --- a/apps/agent-worker/tests/unit/terminal-resize.test.ts +++ b/apps/agent-worker/tests/unit/terminal-resize.test.ts @@ -44,3 +44,18 @@ describe('parseResizeMessage', () => { expect(parseResizeMessage(msg, false)).toEqual({ cols: 1000, rows: 1 }); }); }); + +describe('buildTerminalShellCommand', () => { + test('embeds the connection marker without exec-ing the shell away', async () => { + const { buildTerminalShellCommand } = await load(); + const command = buildTerminalShellCommand('spoon-term-abc-123'); + expect(command[0]).toBe('/bin/bash'); + const script = command[2] ?? ''; + // Marker must stay in the wrapper's argv for pgrep-based cleanup, so the + // inner shell/tmux must NOT be exec'd (exec would replace the argv). + expect(script).toContain('# spoon-term-abc-123'); + expect(script).not.toContain('exec tmux'); + expect(script).not.toContain('exec bash'); + expect(script).toContain('tmux new-session -A -s spoon'); + }); +}); diff --git a/docker/agent-job.Dockerfile b/docker/agent-job.Dockerfile index 0ea3eae..0a1af7e 100644 --- a/docker/agent-job.Dockerfile +++ b/docker/agent-job.Dockerfile @@ -50,6 +50,16 @@ RUN dnf install -y --setopt=install_weak_deps=False --nodocs \ RUN npm install -g pnpm yarn bun@1.3.10 opencode-ai@1.17.9 @openai/codex@0.142.0 @anthropic-ai/claude-code@2.1.207 \ && npm cache clean --force +# nvm, system-wide: user dotfiles source /etc/profile.d/nvm.sh (workstation +# parity), and NVM_DIR must be writable by the box user (uid 1000) so +# `nvm install` works from the shell. PROFILE=/dev/null stops the installer +# from editing root's rc files. +ENV NVM_DIR=/usr/local/nvm +RUN mkdir -p "$NVM_DIR" \ + && wget -qO- https://raw.githubusercontent.com/nvm-sh/nvm/v0.39.5/install.sh | PROFILE=/dev/null bash \ + && printf 'export NVM_DIR=/usr/local/nvm\n[ -s "$NVM_DIR/nvm.sh" ] && . "$NVM_DIR/nvm.sh"\n[ -s "$NVM_DIR/bash_completion" ] && . "$NVM_DIR/bash_completion"\n' > /etc/profile.d/nvm.sh \ + && chown -R 1000:1000 "$NVM_DIR" + # oh-my-posh prompt (binary only; we ship our own /etc/spoon/omp.json theme). RUN curl -fsSL https://ohmyposh.dev/install.sh | bash -s -- -d /usr/local/bin \ && oh-my-posh version diff --git a/docs/agent-terminal.md b/docs/agent-terminal.md index f96b8ba..a977733 100644 --- a/docs/agent-terminal.md +++ b/docs/agent-terminal.md @@ -108,6 +108,31 @@ Docker mode. Prod (Docker socket mounted) works as-is. is intended for the single-user self-hosted deployment; do not expose the worker domain without TLS, and keep the deployment single-tenant. +## The box user + +Terminals and agent turns run as a named non-root user (uid 1000), not root — +Claude Code refuses some operations as root, and a real machine has a named +account. The Linux username mirrors your Spoon username (lowercased/sanitized; +the passwd entry may differ from the home folder name), the hostname is +`-box`, and the account is in `wheel`. + +**sudo & password:** with no password set, sudo works without prompting (a +NOPASSWD drop-in at `/etc/sudoers.d/spoon-box`). Set a password from the +**/machine → Box user** card: it is stored encrypted in Convex, applied live to +a running box (`chpasswd` over stdin via `POST /box/set-password`), and +re-applied at every box creation — at which point the NOPASSWD drop-in is +removed and wheel membership makes sudo prompt. Clearing the password reverts +to passwordless sudo. Passwords are 8-128 chars and write-only (the UI only +ever learns whether one is set). + +**File ownership:** the worker writes into homes from the host (clones, +dotfiles, the file editor). Under dev rootless podman the box runs with +`--userns=keep-id:uid=1000,gid=1000` so the host user IS the box user and +ownership just works (`SPOON_AGENT_BOX_USERNS` overrides; empty disables). +Under prod rootful docker the worker repairs ownership with targeted +`chown -R 1000:1000` execs after each host-side write, plus a one-time +marker-guarded (`~/.spoon/chown-v1`) home chown at first box creation. + ## Tools in the shell The box image ships `bash`, `tmux`, `neovim`, `git`, `ripgrep`, `jq`, `python3`,