Skip to content

Commit a03e095

Browse files
jesseturner21claude
andcommitted
fix(dev): detect and skip container runtime shims that masquerade as real runtimes
The container runtime detection previously only checked `which` + `--version` to validate a runtime. Toolbox shims (e.g., ~/.toolbox/bin/finch wrapping ada) pass both checks but don't support container operations like build/run. Add a `build --help` probe after the version check. Skip the runtime if: - The probe exits with non-zero code, OR - The probe exits 0 but stderr contains "unknown command" or "not found" (handles shims that always exit 0 regardless of errors) Constraint: Cannot use `docker info` as it requires daemon access and triggers password prompts Rejected: Only checking exit code | ada shim exits 0 for all commands Confidence: high Scope-risk: narrow Not-tested: Shims that swallow stderr entirely (would need stdout content validation) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 92f0b62 commit a03e095

2 files changed

Lines changed: 34 additions & 13 deletions

File tree

src/cli/external-requirements/__tests__/detect.test.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ describe('detectContainerRuntime', () => {
2020
mockCheckSubprocess.mockResolvedValue(true);
2121
mockRunSubprocessCapture.mockImplementation((_bin: string, args: string[]) => {
2222
if (args[0] === '--version') return Promise.resolve({ code: 0, stdout: 'Docker version 24.0.0\n', stderr: '' });
23+
if (args[0] === 'build') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
2324
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
2425
});
2526

@@ -36,6 +37,7 @@ describe('detectContainerRuntime', () => {
3637
mockRunSubprocessCapture.mockImplementation((bin: string, args: string[]) => {
3738
if (bin === 'podman' && args[0] === '--version')
3839
return Promise.resolve({ code: 0, stdout: 'podman version 4.5.0\n', stderr: '' });
40+
if (bin === 'podman' && args[0] === 'build') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
3941
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
4042
});
4143

@@ -57,6 +59,7 @@ describe('detectContainerRuntime', () => {
5759
if (bin === 'docker' && args[0] === '--version') return Promise.resolve({ code: 1, stdout: '', stderr: 'error' });
5860
if (bin === 'podman' && args[0] === '--version')
5961
return Promise.resolve({ code: 0, stdout: 'podman version 4.5.0\n', stderr: '' });
62+
if (bin === 'podman' && args[0] === 'build') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
6063
// finch --version also fails
6164
if (bin === 'finch' && args[0] === '--version') return Promise.resolve({ code: 1, stdout: '', stderr: 'error' });
6265
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
@@ -71,6 +74,7 @@ describe('detectContainerRuntime', () => {
7174
mockRunSubprocessCapture.mockImplementation((_bin: string, args: string[]) => {
7275
if (args[0] === '--version')
7376
return Promise.resolve({ code: 0, stdout: 'Docker version 24.0.0\nExtra info line\n', stderr: '' });
77+
if (args[0] === 'build') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
7478
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
7579
});
7680

@@ -82,6 +86,7 @@ describe('detectContainerRuntime', () => {
8286
mockCheckSubprocess.mockResolvedValue(true);
8387
mockRunSubprocessCapture.mockImplementation((_bin: string, args: string[]) => {
8488
if (args[0] === '--version') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
89+
if (args[0] === 'build') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
8590
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
8691
});
8792

@@ -90,20 +95,30 @@ describe('detectContainerRuntime', () => {
9095
expect(result.runtime?.version).toBe('');
9196
});
9297

93-
it('does not call docker info to check daemon status', async () => {
98+
it('skips runtime when build --help check fails with non-zero exit', async () => {
9499
mockCheckSubprocess.mockResolvedValue(true);
95100
mockRunSubprocessCapture.mockImplementation((_bin: string, args: string[]) => {
96-
if (args[0] === '--version') return Promise.resolve({ code: 0, stdout: 'Docker version 24.0.0\n', stderr: '' });
101+
if (args[0] === '--version') return Promise.resolve({ code: 0, stdout: '1.0.0\n', stderr: '' });
102+
if (args[0] === 'build') return Promise.resolve({ code: 1, stdout: '', stderr: 'unknown command "build"' });
97103
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
98104
});
99105

100-
await detectContainerRuntime();
106+
const result = await detectContainerRuntime();
107+
expect(result.runtime).toBeNull();
108+
});
109+
110+
it('skips runtime when build --help exits 0 but stderr indicates unknown command (shim/wrapper)', async () => {
111+
mockCheckSubprocess.mockResolvedValue(true);
112+
mockRunSubprocessCapture.mockImplementation((_bin: string, args: string[]) => {
113+
// Shim exits 0 for everything but prints error to stderr
114+
if (args[0] === '--version') return Promise.resolve({ code: 0, stdout: '1.0.0\n', stderr: '' });
115+
if (args[0] === 'build')
116+
return Promise.resolve({ code: 0, stdout: '', stderr: 'Error: unknown command "build" for "ada"' });
117+
return Promise.resolve({ code: 0, stdout: '', stderr: '' });
118+
});
101119

102-
// Verify 'info' was never called — this is the key behavioral change
103-
const infoCalls = mockRunSubprocessCapture.mock.calls.filter(
104-
(call: unknown[]) => (call[1] as string[])[0] === 'info'
105-
);
106-
expect(infoCalls).toHaveLength(0);
120+
const result = await detectContainerRuntime();
121+
expect(result.runtime).toBeNull();
107122
});
108123
});
109124

@@ -112,6 +127,7 @@ describe('requireContainerRuntime', () => {
112127
mockCheckSubprocess.mockResolvedValue(true);
113128
mockRunSubprocessCapture.mockImplementation((_bin: string, args: string[]) => {
114129
if (args[0] === '--version') return Promise.resolve({ code: 0, stdout: 'Docker version 24.0.0\n', stderr: '' });
130+
if (args[0] === 'build') return Promise.resolve({ code: 0, stdout: '', stderr: '' });
115131
return Promise.resolve({ code: 1, stdout: '', stderr: '' });
116132
});
117133

src/cli/external-requirements/detect.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,8 @@ export interface DetectionResult {
2020

2121
/**
2222
* Detect available container runtime.
23-
* Checks docker, podman, finch in order; returns the first that is installed.
24-
* Does not probe the daemon (e.g., `docker info`) — that would require socket
25-
* access and can trigger OS password prompts on systems where the user is not
26-
* in the docker group. Actual daemon availability is validated when the runtime
27-
* is used (build, run, etc.).
23+
* Checks docker, podman, finch in order; returns the first that is installed
24+
* and capable of running container operations.
2825
*/
2926
export async function detectContainerRuntime(): Promise<DetectionResult> {
3027
for (const runtime of CONTAINER_RUNTIMES) {
@@ -36,6 +33,14 @@ export async function detectContainerRuntime(): Promise<DetectionResult> {
3633
const result = await runSubprocessCapture(runtime, ['--version']);
3734
if (result.code !== 0) continue;
3835

36+
// Validate the binary actually supports container operations.
37+
// Some environments have shims (e.g., toolbox wrappers) that respond to
38+
// --version but don't support build/run commands. These shims may exit 0
39+
// even on failure, so also check stderr for error indicators.
40+
const buildCheck = await runSubprocessCapture(runtime, ['build', '--help']);
41+
if (buildCheck.code !== 0) continue;
42+
if (buildCheck.stderr && /unknown command|not found/i.test(buildCheck.stderr)) continue;
43+
3944
const version = result.stdout.trim().split('\n')[0] ?? 'unknown';
4045
return { runtime: { runtime, binary: runtime, version } };
4146
}

0 commit comments

Comments
 (0)