Skip to content

Commit 06ac446

Browse files
fix(cli): preserve healthy Docker authority (#8823)
<!-- markdownlint-disable MD041 --> ## Summary NemoClaw now preserves a reachable Docker CLI default, including a custom `DOCKER_CONFIG`, when `DOCKER_HOST` is unset instead of replacing it with the first existing socket. If the default is unreachable, the CLI probes only its bounded local-socket candidates and selects a fallback only when server-version evidence identifies one runtime. ## Related Issue Fixes #8816 ## Changes - Preserve an explicit `DOCKER_HOST` without probing or replacing its authority. - Probe the Docker CLI default before local fallbacks so a reachable Docker Engine remains selected when Docker and Podman sockets coexist. - Validate each allowlisted fallback for reachability and Docker or Podman identity. Mixed or unknown fallback identities produce no automatic override. - Reuse the existing Docker-compatible server-version classifier from host preflight instead of creating a second runtime identity rule. - Preserve a reachable configured `DOCKER_CONTEXT` and its `DOCKER_CONFIG` directory for Docker commands, and clear the context only when a verified socket fallback must take precedence. - Add regression coverage for explicit endpoints, coexistence, Podman-only hosts, stale sockets, mixed runtimes, unknown engines, and probe environment filtering. - Update the system-readiness reference with the default and fallback authority order. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [x] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [x] Docs updated for user-facing behavior changes - [ ] Docs not applicable — justification: - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Maintainer review covered endpoint validation, credential filtering, failure handling, and runtime authority. The read-only default probe and Docker runner preserve `DOCKER_CONTEXT` and its `DOCKER_CONFIG` path; fallback probes and host overrides omit both; selecting a verified fallback clears the conflicting context; and non-Docker commands receive neither value. Regression coverage confirms `NVIDIA_INFERENCE_API_KEY` remains filtered at both boundaries. The maintainer explicitly approved this Docker-only configuration boundary. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Documentation Writer Review - [x] Documentation writer subagent reviewed the completed changes - Result: `docs-updated` - Evidence: `docs/reference/system-readiness.mdx` documents Docker default-authority selection with `DOCKER_CONTEXT` and `DOCKER_CONFIG`, verified local socket fallback, and conflicting context removal. - Agent: Codex Desktop <!-- docs-review-head-sha: 982a17d --> <!-- docs-review-agents-blob-sha: c4923a3 --> ## DGX Station Hardware Evidence - [ ] Tested on DGX Station - Tested commit: - Station profile/scenario: - Result: - Supporting evidence: ## Verification - [x] PR description includes a `Signed-off-by:` line and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run validate:pr` passed after refreshing `origin/main` when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: 10 focused regression and security files passed 308 tests on the refreshed head; CLI and plugin builds and type checks passed; the full pre-commit, commit-message, and pre-push sequence passed against `upstream/main`. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: Not required for this focused CLI change. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [x] `npm run docs` builds without warnings (doc changes only) — result: the build passed; Fern reported two baseline warnings that it suppresses unless run with `--warnings`. - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Enhancements** * Improved Docker runtime detection using configured hosts, Docker CLI defaults, and eligible local sockets. * Verifies runtime identity before selecting an endpoint, distinguishing Docker from Podman and rejecting unknown or conflicting results. * Improved readiness and profile selection when multiple container runtimes are available. * Applies Docker context settings appropriately, clearing them for selected socket fallbacks while preserving explicit configurations. * **Documentation** * Updated system readiness guidance to explain Docker fallback detection and runtime compatibility behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: Senthil Ravichandran <senthilr@nvidia.com>
1 parent db0cf5f commit 06ac446

9 files changed

Lines changed: 659 additions & 95 deletions

File tree

docs/reference/system-readiness.mdx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,14 @@ Do not use `observedAt` as build identity.
9797

9898
## Interpret Docker storage
9999

100-
The report rejects an unsupported `DOCKER_HOST` before it uses Docker daemon evidence.
100+
The report rejects an unsupported explicit `DOCKER_HOST` before it uses Docker daemon evidence.
101101
`host.docker.host_invalid` records whether the configured endpoint is invalid, `host.docker.endpoint_supported` is absent when a configured endpoint is not an absolute local `unix://` socket, and the blocking `host.docker.host_invalid` finding identifies that condition.
102102
TCP, SSH, relative Unix socket, and unsafe path values do not fall back to the default Docker socket.
103-
Unset `DOCKER_HOST` continues to use Docker's default local socket.
103+
When `DOCKER_HOST` is unset, NemoClaw first probes the Docker CLI's selected authority, including `DOCKER_CONTEXT` and its `DOCKER_CONFIG` directory when set, without a host override.
104+
If that authority is unreachable, NemoClaw probes a bounded set of local Unix sockets and accepts only an endpoint with valid server-version evidence.
105+
When it selects a fallback socket, NemoClaw clears the unreachable `DOCKER_CONTEXT` so later Docker commands use the selected `DOCKER_HOST`.
106+
A mixed set of reachable Docker and Podman fallbacks, or a fallback with an unknown server identity, produces no automatic `DOCKER_HOST` override.
107+
A reachable Podman endpoint remains classified as Podman and does not satisfy the standard Docker runtime requirement.
104108

105109
The Docker storage capabilities separate the current host configuration from NemoClaw's supported remediation path.
106110

src/lib/onboard/preflight.ts

Lines changed: 1 addition & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { ADVISORY_CHECKS } from "../advisories/registry";
1919
import { runAdvisories } from "../advisories/runner";
2020
import { DASHBOARD_PORT } from "../core/ports";
2121
import { isDockerDaemonReachable, isSupportedGatewayDockerHost } from "../domain/docker-host";
22+
import { classifyDockerVersionIdentity } from "../platform";
2223
import { resolveOpenshell } from "../readiness/openshell-resolver";
2324
import {
2425
MIN_RECOMMENDED_DOCKER_CPUS,
@@ -241,59 +242,6 @@ function inferContainerRuntime(info = ""): ContainerRuntime {
241242
return "unknown";
242243
}
243244

244-
type DockerVersionIdentity = "docker" | "podman" | "unknown";
245-
246-
/**
247-
* Classify the engine identity from the explicit
248-
* `docker version --format '{{json .}}'` banner.
249-
*
250-
* Podman's docker-compat `/info` endpoint mimics Docker so closely that
251-
* `docker info` carries no "podman" marker (observed on Apple Silicon macOS:
252-
* `ServerVersion: "5.6.2"`, `OperatingSystem: "fedora"`, no "podman"
253-
* substring), so `inferContainerRuntime` misclassifies it as plain Docker.
254-
* The docker-compat `/version` payload still names the engine: a
255-
* `Server.Components[].Name` of "Podman Engine" and a `Server.Platform.Name`
256-
* like "linux/arm64/fedora-42". Real Docker reports
257-
* `Server.Platform.Name: "Docker Engine - Community"` and components
258-
* `Engine`/`containerd`/`runc`, which provide positive Docker identity on
259-
* Docker Engine, Docker Desktop, and Colima (#7320).
260-
*/
261-
function classifyDockerVersionIdentity(versionOutput = ""): DockerVersionIdentity {
262-
const text = String(versionOutput || "").trim();
263-
if (!text) return "unknown";
264-
let parsed: unknown;
265-
try {
266-
parsed = JSON.parse(text);
267-
} catch {
268-
// Plain-text `docker version` still prints the "Podman Engine" server banner.
269-
if (/podman/i.test(text)) return "podman";
270-
return /docker engine/i.test(text) ? "docker" : "unknown";
271-
}
272-
const server = (parsed as Record<string, unknown> | null)?.Server;
273-
if (!server || typeof server !== "object") return "unknown";
274-
const s = server as Record<string, unknown>;
275-
const platformName = (s.Platform as Record<string, unknown> | undefined)?.Name;
276-
if (typeof platformName === "string" && /podman/i.test(platformName)) return "podman";
277-
const components = s.Components;
278-
const componentNames = Array.isArray(components)
279-
? components.flatMap((component) => {
280-
const name =
281-
component && typeof component === "object"
282-
? (component as Record<string, unknown>).Name
283-
: undefined;
284-
return typeof name === "string" ? [name] : [];
285-
})
286-
: [];
287-
if (componentNames.some((name) => /podman/i.test(name))) return "podman";
288-
if (
289-
(typeof platformName === "string" && /^docker (?:engine|desktop)\b/i.test(platformName)) ||
290-
componentNames.some((name) => name.trim().toLowerCase() === "engine")
291-
) {
292-
return "docker";
293-
}
294-
return "unknown";
295-
}
296-
297245
/**
298246
* Use `ProductLicense: "Apache-2.0"` as a Podman backstop only when the
299247
* authoritative `docker version` probe is unavailable. Podman's docker-compat

src/lib/platform.ts

Lines changed: 111 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import { existsSync as defaultExistsSync } from "node:fs";
55
import os from "node:os";
66
import path from "node:path";
77

8+
import { dockerSpawnSync } from "./adapters/docker/exec";
9+
810
export type ContainerRuntime = "podman" | "colima" | "docker-desktop" | "docker" | "unknown";
911

1012
export interface PlatformLookupOptions {
@@ -24,6 +26,7 @@ export interface WslDetectionOptions {
2426
export interface DockerHostDetectionOptions extends PlatformLookupOptions, WslDetectionOptions {
2527
env?: NodeJS.ProcessEnv;
2628
existsSync?: (filePath: string) => boolean;
29+
probeDockerHost?: DockerHostProbe;
2730
}
2831

2932
export interface DockerHostDetection {
@@ -32,6 +35,19 @@ export interface DockerHostDetection {
3235
socketPath: string | null;
3336
}
3437

38+
export type DockerVersionIdentity = "docker" | "podman" | "unknown";
39+
40+
export interface DockerHostProbeResult {
41+
reachable: boolean;
42+
identity: DockerVersionIdentity;
43+
}
44+
45+
export type DockerHostProbe = (dockerHost: string | undefined) => DockerHostProbeResult;
46+
47+
const DOCKER_PROBE_TIMEOUT_MS = 3_000;
48+
const DOCKER_PROBE_MAX_BUFFER_BYTES = 1024 * 1024;
49+
const DOCKER_PROBE_ENV_NAMES = ["HOME", "USER", "LOGNAME", "PATH"] as const;
50+
3551
function isWsl(opts: WslDetectionOptions = {}): boolean {
3652
// Explicit override — lets tests pin behavior regardless of the host kernel.
3753
// Useful because the WSL detection below consults `os.release()`, which
@@ -65,6 +81,85 @@ function inferContainerRuntime(info = ""): ContainerRuntime {
6581
return "unknown";
6682
}
6783

84+
/**
85+
* Classify the engine identity from the explicit
86+
* `docker version --format '{{json .}}'` banner.
87+
*
88+
* Podman's Docker-compatible `/info` endpoint does not always name Podman.
89+
* The `/version` payload retains a `Podman Engine` component. Docker Engine,
90+
* Docker Desktop, and Colima retain a Docker platform or engine component.
91+
*/
92+
function classifyDockerVersionIdentity(versionOutput = ""): DockerVersionIdentity {
93+
const text = String(versionOutput || "").trim();
94+
if (!text) return "unknown";
95+
let parsed: unknown;
96+
try {
97+
parsed = JSON.parse(text);
98+
} catch {
99+
if (/podman/i.test(text)) return "podman";
100+
return /docker engine/i.test(text) ? "docker" : "unknown";
101+
}
102+
const server = (parsed as Record<string, unknown> | null)?.Server;
103+
if (!server || typeof server !== "object") return "unknown";
104+
const serverRecord = server as Record<string, unknown>;
105+
const platformName = (serverRecord.Platform as Record<string, unknown> | undefined)?.Name;
106+
if (typeof platformName === "string" && /podman/i.test(platformName)) return "podman";
107+
const components = serverRecord.Components;
108+
const componentNames = Array.isArray(components)
109+
? components.flatMap((component) => {
110+
const name =
111+
component && typeof component === "object"
112+
? (component as Record<string, unknown>).Name
113+
: undefined;
114+
return typeof name === "string" ? [name] : [];
115+
})
116+
: [];
117+
if (componentNames.some((name) => /podman/i.test(name))) return "podman";
118+
if (
119+
(typeof platformName === "string" && /^docker (?:engine|desktop)\b/i.test(platformName)) ||
120+
componentNames.some((name) => name.trim().toLowerCase() === "engine")
121+
) {
122+
return "docker";
123+
}
124+
return "unknown";
125+
}
126+
127+
function buildDockerProbeEnv(
128+
source: NodeJS.ProcessEnv,
129+
dockerHost: string | undefined,
130+
): NodeJS.ProcessEnv {
131+
const env: NodeJS.ProcessEnv = {};
132+
for (const name of DOCKER_PROBE_ENV_NAMES) {
133+
const value = source[name];
134+
if (value !== undefined) env[name] = value;
135+
}
136+
if (dockerHost === undefined) {
137+
if (source.DOCKER_CONFIG !== undefined) env.DOCKER_CONFIG = source.DOCKER_CONFIG;
138+
if (source.DOCKER_CONTEXT !== undefined) env.DOCKER_CONTEXT = source.DOCKER_CONTEXT;
139+
}
140+
if (dockerHost) {
141+
env.DOCKER_HOST = dockerHost;
142+
}
143+
return env;
144+
}
145+
146+
function probeDockerHost(
147+
dockerHost: string | undefined,
148+
source: NodeJS.ProcessEnv,
149+
): DockerHostProbeResult {
150+
const result = dockerSpawnSync(["version", "--format", "{{json .}}"], {
151+
encoding: "utf-8",
152+
env: buildDockerProbeEnv(source, dockerHost),
153+
timeout: DOCKER_PROBE_TIMEOUT_MS,
154+
maxBuffer: DOCKER_PROBE_MAX_BUFFER_BYTES,
155+
});
156+
if (!result || result.status !== 0) return { reachable: false, identity: "unknown" };
157+
return {
158+
reachable: true,
159+
identity: classifyDockerVersionIdentity(String(result.stdout ?? "")),
160+
};
161+
}
162+
68163
function containerCanReachHostLoopback(
69164
runtime: ContainerRuntime,
70165
opts: WslDetectionOptions = {},
@@ -165,21 +260,29 @@ function detectDockerHost(opts: DockerHostDetectionOptions = {}): DockerHostDete
165260
};
166261
}
167262

263+
const probe = opts.probeDockerHost ?? ((dockerHost) => probeDockerHost(dockerHost, env));
264+
if (probe(undefined).reachable) return null;
265+
168266
const fileExists = opts.existsSync ?? defaultExistsSync;
267+
let selection: DockerHostDetection | null = null;
268+
let selectedIdentity: Exclude<DockerVersionIdentity, "unknown"> | null = null;
169269
for (const socketPath of getDockerSocketCandidates(opts)) {
170-
if (fileExists(socketPath)) {
171-
return {
172-
dockerHost: `unix://${socketPath}`,
173-
source: "socket",
174-
socketPath,
175-
};
176-
}
270+
if (!fileExists(socketPath)) continue;
271+
const dockerHost = `unix://${socketPath}`;
272+
const observation = probe(dockerHost);
273+
if (!observation.reachable) continue;
274+
if (observation.identity === "unknown") return null;
275+
if (selectedIdentity && observation.identity !== selectedIdentity) return null;
276+
if (selection) continue;
277+
selection = { dockerHost, source: "socket", socketPath };
278+
selectedIdentity = observation.identity;
177279
}
178280

179-
return null;
281+
return selection;
180282
}
181283

182284
export {
285+
classifyDockerVersionIdentity,
183286
containerCanReachHostLoopback,
184287
detectDockerHost,
185288
findColimaDockerSocket,

src/lib/runner.ts

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,31 @@ type SpawnResult = SpawnSyncReturns<string | Buffer>;
4343
const dockerHost = detectDockerHost();
4444
if (dockerHost) {
4545
process.env.DOCKER_HOST = dockerHost.dockerHost;
46+
if (dockerHost.source === "socket") {
47+
delete process.env.DOCKER_CONTEXT;
48+
}
4649
}
4750

48-
function buildRunnerEnv(extraEnv?: NodeJS.ProcessEnv): Record<string, string> {
51+
function buildRunnerEnv(extraEnv?: NodeJS.ProcessEnv, executable?: string): Record<string, string> {
4952
const normalizedExtra: Record<string, string> = {};
5053
if (extraEnv) {
5154
for (const [key, value] of Object.entries(extraEnv)) {
5255
if (value !== undefined) normalizedExtra[key] = value;
5356
}
5457
}
58+
const usesDockerDefaultAuthority =
59+
executable !== undefined &&
60+
path.basename(executable) === "docker" &&
61+
normalizedExtra.DOCKER_HOST === undefined &&
62+
process.env.DOCKER_HOST === undefined;
63+
if (usesDockerDefaultAuthority) {
64+
if (normalizedExtra.DOCKER_CONFIG === undefined && process.env.DOCKER_CONFIG !== undefined) {
65+
normalizedExtra.DOCKER_CONFIG = process.env.DOCKER_CONFIG;
66+
}
67+
if (normalizedExtra.DOCKER_CONTEXT === undefined && process.env.DOCKER_CONTEXT !== undefined) {
68+
normalizedExtra.DOCKER_CONTEXT = process.env.DOCKER_CONTEXT;
69+
}
70+
}
5571
return buildSubprocessEnv(normalizedExtra);
5672
}
5773

@@ -118,7 +134,7 @@ function spawnAndHandle(
118134
shell: false,
119135
stdio: effectiveStdio,
120136
cwd: ROOT,
121-
env: buildRunnerEnv(opts.env),
137+
env: buildRunnerEnv(opts.env, safeFile),
122138
});
123139
if (!opts.suppressOutput) {
124140
writeRedactedResult(result, effectiveStdio);
@@ -192,7 +208,7 @@ function runArrayCmd(
192208
shell: false,
193209
stdio,
194210
cwd: ROOT,
195-
env: buildRunnerEnv(extraEnv),
211+
env: buildRunnerEnv(extraEnv, exe),
196212
});
197213
if (!suppressOutput) {
198214
writeRedactedResult(result, stdio);
@@ -293,7 +309,7 @@ function runCapture(cmd: readonly string[], opts: CaptureOptions = {}): string {
293309
...spawnOpts,
294310
shell: false,
295311
cwd: ROOT,
296-
env: buildRunnerEnv(extraEnv),
312+
env: buildRunnerEnv(extraEnv, exe),
297313
stdio: ["pipe", "pipe", "pipe"],
298314
encoding: "utf-8",
299315
});
@@ -357,7 +373,7 @@ function runCaptureEx(
357373
// NO_PROXY=localhost,127.0.0.1 is injected when HTTP_PROXY is set.
358374
// Otherwise curl probes against localhost (Ollama validation, etc.)
359375
// tunnel through the user's host proxy and fail with HTTP 500.
360-
env: buildRunnerEnv(extraEnv),
376+
env: buildRunnerEnv(extraEnv, exe),
361377
stdio: ["pipe", "pipe", "pipe"],
362378
encoding: "utf-8",
363379
});

src/lib/shields/index.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ vi.mock("../sandbox/agent-config", () => ({
6868

6969
vi.mock("../adapters/docker/exec", () => ({
7070
dockerExecFileSync: vi.fn((_argv: string[]) => ""),
71+
dockerSpawnSync: vi.fn(() => ({ status: 1, stdout: "", stderr: "" })),
7172
}));
7273

7374
vi.mock("./audit", () => ({

0 commit comments

Comments
 (0)