mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 14:09:42 +02:00
fix(docker): stop requiring the CLI on the host for a container session
Attaching a container, picking claude and hitting Run gave one line — `execvp(3) failed.: No such file or directory` — and the run-mode menu offered every mode. Three separate defects, found on a real deployment. TmuxManager.createSession resolved the CLI directory without distinguishing a docker session, so a host with no claude threw, the catch fell back to a direct PTY, and that PTY exec'd the CLI on the HOST. The failure surfaced as a bare execvp error naming nothing. A docker session runs its CLI inside the container; the host does not need it. All eight modes now sit behind a cliRunsInContainer guard, and whether the container has the CLI is settled by the adoption preflight or the image gate before launch. The running check used a bare double quote and command substitution. The whole chain is embedded in an outer `bash -c "…"`, so the unescaped quote closed that string early and the remainder was re-tokenized. It is now a `grep -qx` pipeline using only the single-quote form every other line in the builder already uses. Claude Code refuses --dangerously-skip-permissions as root. Our base image runs a non-root user, so an owned container never hit this; an adopted container's user belongs to its owner and is frequently root, and keeping the flag killed the pane with a message visible only inside the container. The preflight now reports runsAsRoot and the launch chain drops the flag for it. The menu also showed every mode because the container CLI probe only started when the menu opened. It is warmed when the case is selected instead.
This commit is contained in:
+15
-3
@@ -160,11 +160,16 @@ export function dockerContainerName(caseName: string): string {
|
||||
}
|
||||
|
||||
/** Default pane command per CLI mode (mirror of defaultRemoteCommandForMode). */
|
||||
export function defaultDockerCommandForMode(mode: SessionMode): string {
|
||||
export function defaultDockerCommandForMode(mode: SessionMode, runsAsRoot = false): string {
|
||||
const commands: Record<DockerCommandMode, string> = {
|
||||
shell: 'exec bash -l',
|
||||
// Mirror the LOCAL claude default so the in-container agent runs non-interactively.
|
||||
claude: 'exec claude --dangerously-skip-permissions',
|
||||
// Mirror the LOCAL claude default so the in-container agent runs
|
||||
// non-interactively — EXCEPT as root, where Claude Code refuses the flag
|
||||
// outright ("cannot be used with root/sudo privileges"). Our base image runs
|
||||
// a non-root user so an owned container never hits this; an adopted
|
||||
// container's user belongs to its owner and is frequently root, and keeping
|
||||
// the flag there kills the pane with a message only visible inside it.
|
||||
claude: runsAsRoot ? 'exec claude' : 'exec claude --dangerously-skip-permissions',
|
||||
opencode: 'exec opencode',
|
||||
codex: 'exec codex',
|
||||
gemini: 'exec gemini',
|
||||
@@ -1056,6 +1061,8 @@ export interface AdoptedContainerProbe {
|
||||
availableModes?: SessionMode[];
|
||||
/** Whether the requested working directory exists INSIDE the container. */
|
||||
workdirExists?: boolean;
|
||||
/** Whether the container's exec user is root (uid 0). */
|
||||
runsAsRoot?: boolean;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
@@ -1179,6 +1186,10 @@ export async function probeAdoptableContainer(
|
||||
// --workdir <missing>` fails with an OCI chdir error the pane surfaces as a bare
|
||||
// "execvp failed", so it is resolved here into an actionable message.
|
||||
if (containerWorkdir) steps.push(`[ -d ${shellescape(containerWorkdir)} ] && echo __workdir__`);
|
||||
// Claude Code REFUSES --dangerously-skip-permissions as root. Our own base
|
||||
// image runs a non-root user so an owned container never hits it; an adopted
|
||||
// container's user belongs to its owner and is frequently root.
|
||||
steps.push(`[ "$(id -u)" = 0 ] && echo __root__`);
|
||||
const script = `${steps.join('; ')}; exit 0`;
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
@@ -1220,6 +1231,7 @@ export async function probeAdoptableContainer(
|
||||
tmuxPath: 'tmux',
|
||||
availableModes: modes.filter((m) => m === 'shell' || found.has(binaryFor(m))),
|
||||
workdirExists,
|
||||
runsAsRoot: found.has('__root__'),
|
||||
};
|
||||
} catch (err) {
|
||||
const msg = err instanceof Error ? err.message : String(err);
|
||||
|
||||
+26
-12
@@ -1290,7 +1290,8 @@ export function buildDockerLaunchCommand(opts: DockerLaunchOptions): string {
|
||||
const dkrName = dockerTmuxSessionName(sessionId);
|
||||
const sid = sessionId.slice(0, 8);
|
||||
|
||||
let modeCommand = docker.commands?.[mode as DockerCommandMode] || defaultDockerCommandForMode(mode);
|
||||
let modeCommand =
|
||||
docker.commands?.[mode as DockerCommandMode] || defaultDockerCommandForMode(mode, !!docker.runsAsRoot);
|
||||
if (mode === 'claude') {
|
||||
modeCommand = claudeDockerPaneCommand(modeCommand, sessionId, resumeSessionId);
|
||||
} else if (resumeSessionId) {
|
||||
@@ -1327,10 +1328,10 @@ export function buildDockerLaunchCommand(opts: DockerLaunchOptions): string {
|
||||
const startFailMsg = shellescape(`Codeman: container ${docker.containerName} failed to start (docker daemon down?)`);
|
||||
|
||||
const notFoundMsg = shellescape(
|
||||
`Codeman: container ${docker.containerName} not found. Adopted containers are never created by Codeman — start it yourself, then reopen this session.`
|
||||
`Codeman: container ${docker.containerName} not found. Adopted containers are never created by Codeman - start it yourself, then reopen this session.`
|
||||
);
|
||||
const notRunningMsg = shellescape(
|
||||
`Codeman: container ${docker.containerName} is not running. Codeman never starts a container it does not own — start it yourself, then reopen this session.`
|
||||
`Codeman: container ${docker.containerName} is not running. Codeman never starts a container it does not own - start it yourself, then reopen this session.`
|
||||
);
|
||||
|
||||
const imageCheck = adopted
|
||||
@@ -1340,8 +1341,13 @@ export function buildDockerLaunchCommand(opts: DockerLaunchOptions): string {
|
||||
const ensure = adopted
|
||||
? `${base} inspect ${name} >/dev/null 2>&1 || { echo ${notFoundMsg}; exit 1; }`
|
||||
: `${base} inspect ${name} >/dev/null 2>&1 || ${base} ${createArgs}`;
|
||||
// ⚠️ No double quotes and no `$(…)` here. This whole chain is embedded in an
|
||||
// outer `bash -c "…"`, so an unescaped `"` closes that string early, the rest
|
||||
// is re-tokenized, and tmux fails to exec with a bare `execvp(3) failed`. A
|
||||
// `grep -qx` pipeline reads the same answer using only the single-quoted form
|
||||
// every other line in this builder already uses.
|
||||
const start = adopted
|
||||
? `[ "$(${base} inspect -f '{{.State.Running}}' ${name} 2>/dev/null)" = true ] || { echo ${notRunningMsg}; exit 1; }`
|
||||
? `${base} inspect -f ${shellescape('{{.State.Running}}')} ${name} 2>/dev/null | grep -qx true || { echo ${notRunningMsg}; exit 1; }`
|
||||
: `${base} start ${name} >/dev/null 2>&1 || { echo ${startFailMsg}; exit 1; }`;
|
||||
// Seed writable credential config from read-only host mounts ONCE per container
|
||||
// (guarded by [ -e ] so reconnects never clobber in-container config; `cp -a` for
|
||||
@@ -2115,29 +2121,37 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
|
||||
// from the resolvers (formatCliNotFoundMessage) so the error names WHERE it
|
||||
// looked — server PATH, login shell, checked directories — instead of just
|
||||
// asserting the CLI is missing (the classic systemd/launchd PATH trap).
|
||||
//
|
||||
// ⚠️ A DOCKER session runs its CLI INSIDE the container, so the host does not
|
||||
// need it at all. Demanding it here threw for a host without the binary, the
|
||||
// catch fell back to a direct PTY, and that PTY tried to exec the CLI on the
|
||||
// HOST — surfacing as a bare `execvp(3) failed: No such file or directory`
|
||||
// with nothing pointing at the real cause. The container's own CLIs are
|
||||
// verified by the adoption preflight / image gate before launch instead.
|
||||
const { pathExport, dir: cliDir } = this.buildPathExport(mode);
|
||||
if (mode === 'claude' && !cliDir) {
|
||||
const cliRunsInContainer = !!docker;
|
||||
if (!cliRunsInContainer && mode === 'claude' && !cliDir) {
|
||||
throw new Error(getClaudeNotFoundMessage());
|
||||
}
|
||||
if (mode === 'opencode' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'opencode' && !cliDir) {
|
||||
throw new Error(getOpenCodeNotFoundMessage());
|
||||
}
|
||||
if (mode === 'codex' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'codex' && !cliDir) {
|
||||
throw new Error(getCodexNotFoundMessage());
|
||||
}
|
||||
if (mode === 'gemini' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'gemini' && !cliDir) {
|
||||
throw new Error(getGeminiNotFoundMessage());
|
||||
}
|
||||
if (mode === 'antigravity' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'antigravity' && !cliDir) {
|
||||
throw new Error(getAntigravityNotFoundMessage());
|
||||
}
|
||||
if (mode === 'pi' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'pi' && !cliDir) {
|
||||
throw new Error(getPiNotFoundMessage());
|
||||
}
|
||||
if (mode === 'deepseek' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'deepseek' && !cliDir) {
|
||||
throw new Error(getDeepSeekNotFoundMessage());
|
||||
}
|
||||
if (mode === 'grok' && !cliDir) {
|
||||
if (!cliRunsInContainer && mode === 'grok' && !cliDir) {
|
||||
throw new Error(getGrokNotFoundMessage());
|
||||
}
|
||||
|
||||
|
||||
@@ -47,15 +47,7 @@ export type ClaudeMode = 'dangerously-skip-permissions' | 'auto' | 'normal' | 'a
|
||||
|
||||
/** Session mode: which CLI backend a session runs */
|
||||
export type SessionMode =
|
||||
| 'claude'
|
||||
| 'shell'
|
||||
| 'opencode'
|
||||
| 'codex'
|
||||
| 'gemini'
|
||||
| 'antigravity'
|
||||
| 'pi'
|
||||
| 'grok'
|
||||
| 'deepseek';
|
||||
'claude' | 'shell' | 'opencode' | 'codex' | 'gemini' | 'antigravity' | 'pi' | 'grok' | 'deepseek';
|
||||
|
||||
export type RemoteCommandMode = Extract<
|
||||
SessionMode,
|
||||
@@ -302,6 +294,13 @@ export interface SessionDocker {
|
||||
extraExecArgs?: string[];
|
||||
/** Stable hash of the drift-relevant create args (recreate-on-drift detection). */
|
||||
configHash?: string;
|
||||
/**
|
||||
* Whether the container's exec user is root. Claude Code REFUSES
|
||||
* `--dangerously-skip-permissions` as root, and an adopted container's user
|
||||
* belongs to its owner, so the flag is omitted rather than letting the pane
|
||||
* die with a message only visible inside the container.
|
||||
*/
|
||||
runsAsRoot?: boolean;
|
||||
/**
|
||||
* Mirror of `DockerCase.owned`, flattened onto the live session so every
|
||||
* lifecycle decision (launch chain, drift, stop, remove) can see it without
|
||||
|
||||
@@ -187,6 +187,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
this.closeCasePicker();
|
||||
this.updateDirDisplayForCase(select.value);
|
||||
this.updateMobileCaseLabel(select.value);
|
||||
// Warm the container's CLI list HERE rather than when the run menu opens.
|
||||
// The probe is a `docker exec` round trip, so gating it on the menu meant the
|
||||
// menu painted every mode first and only narrowed a moment later — which
|
||||
// reads as "it shows all of them" and lets a mode be picked that the
|
||||
// container does not have.
|
||||
const picked = (this.cases || []).find((c) => c.name === select.value);
|
||||
if (picked?.location === 'docker') void this._probeDockerCaseModes(picked, null);
|
||||
if (save) {
|
||||
this.saveLastUsedCase(select.value);
|
||||
}
|
||||
|
||||
@@ -2968,6 +2968,9 @@ export function registerSessionRoutes(
|
||||
if (!probe.ok) {
|
||||
return createErrorResponse(ApiErrorCode.OPERATION_FAILED, probe.error || 'container is not usable');
|
||||
}
|
||||
// The probe already exec'd into the container; carry its facts onto the
|
||||
// live session so the launch chain does not have to re-ask.
|
||||
sessionDocker.runsAsRoot = probe.runsAsRoot;
|
||||
if (mode !== 'shell' && !probe.availableModes?.includes(mode)) {
|
||||
return createErrorResponse(
|
||||
ApiErrorCode.OPERATION_FAILED,
|
||||
|
||||
@@ -12,6 +12,7 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import {
|
||||
defaultDockerCommandForMode,
|
||||
toSessionDocker,
|
||||
isAdoptedContainer,
|
||||
removeDockerContainer,
|
||||
@@ -113,6 +114,18 @@ describe('adopted container: the launch chain never mutates lifecycle', () => {
|
||||
expect(adopted).toMatch(/not running.*never starts a container it does not own/i);
|
||||
});
|
||||
|
||||
it('uses no double quote and no command substitution in the launch chain', () => {
|
||||
// The whole chain is embedded in an outer `bash -c "…"`. An unescaped `"`
|
||||
// closes that string early, the remainder is re-tokenized, and tmux fails to
|
||||
// exec with a bare `execvp(3) failed: No such file or directory` — no hint
|
||||
// that the command was ever malformed. `$(…)` is banned with it because it
|
||||
// is then evaluated by the wrong shell at the wrong time.
|
||||
expect(adopted).not.toContain('"');
|
||||
expect(adopted).not.toContain('$(');
|
||||
// Every other line already quotes with the single-quote helper.
|
||||
expect(adopted).toContain("grep -qx true");
|
||||
});
|
||||
|
||||
it('skips the base-image gate, which describes an image adoption never uses', () => {
|
||||
expect(owned).toContain('image inspect');
|
||||
expect(adopted).not.toContain('image inspect');
|
||||
@@ -129,6 +142,43 @@ describe('adopted container: the launch chain never mutates lifecycle', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: claude as root', () => {
|
||||
it('drops --dangerously-skip-permissions when the container runs as root', () => {
|
||||
// Claude Code refuses the flag as root ("cannot be used with root/sudo
|
||||
// privileges"), so keeping it kills the pane with a message only visible
|
||||
// inside the container. Our base image runs a non-root user, which is why an
|
||||
// owned container never hit this.
|
||||
expect(defaultDockerCommandForMode('claude', true)).toBe('exec claude');
|
||||
expect(defaultDockerCommandForMode('claude', false)).toContain('--dangerously-skip-permissions');
|
||||
expect(defaultDockerCommandForMode('claude')).toContain('--dangerously-skip-permissions');
|
||||
});
|
||||
|
||||
it('leaves every other mode unchanged as root', () => {
|
||||
for (const mode of ['codex', 'shell', 'pi'] as const) {
|
||||
expect(defaultDockerCommandForMode(mode, true)).toBe(defaultDockerCommandForMode(mode, false));
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: the host is not required to have the CLI', () => {
|
||||
const src = readFileSync(new URL('../src/tmux-manager.ts', import.meta.url), 'utf8');
|
||||
|
||||
it('skips every host CLI requirement for a docker session', () => {
|
||||
// A docker session runs its CLI inside the container. Demanding it on the
|
||||
// host threw, the catch fell back to a direct PTY, and that PTY tried to
|
||||
// exec the CLI on the HOST — surfacing as a bare `execvp(3) failed` with
|
||||
// nothing naming the real cause.
|
||||
const guarded = src.match(/!cliRunsInContainer && mode === '/g) || [];
|
||||
const unguarded = src.match(/\n if \(mode === '[a-z]+' && !cliDir\)/g) || [];
|
||||
expect(guarded.length).toBeGreaterThanOrEqual(7);
|
||||
expect(unguarded).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('derives the flag from the docker metadata the session already carries', () => {
|
||||
expect(src).toContain('const cliRunsInContainer = !!docker;');
|
||||
});
|
||||
});
|
||||
|
||||
describe('adopted container: mutating verbs fail closed at the builder', () => {
|
||||
const docker = toSessionDocker(HOST, caseFor(false));
|
||||
|
||||
@@ -202,7 +252,7 @@ describe('adopted container: run modes come from the CONTAINER, not the host', (
|
||||
const refreshFn = (src) => {
|
||||
const start = src.indexOf('_refreshRunModeAvailability(menu) {');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
return src.slice(start, start + 1600);
|
||||
return src.slice(start, start + 2000);
|
||||
};
|
||||
|
||||
it('gates a docker case on availableModes instead of host CLI probes', () => {
|
||||
|
||||
Reference in New Issue
Block a user