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.
This commit is contained in:
@@ -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}`);
|
||||
|
||||
@@ -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}`,
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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'");
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user