Skip to content

Commit 29e79e7

Browse files
rluo8github-actions[bot]prekshivyas
authored
fix(onboard): isolate WSL Docker public pulls (#10470)
<!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 plain sentences: what changes and why. Describe before-and-after behavior when it applies. Follow the NemoClaw Writing Guide: https://github.com/NVIDIA/NemoClaw/blob/main/WRITING.md. Do not add unrelated prose cleanup. --> On WSL 2, default managed-image onboarding no longer fails when the Docker Desktop credential helper is unavailable in a headless session. NemoClaw uses a temporary credential-free Docker client config for public managed-image and probe pulls on the default Docker context. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Fixes #10349 ## Changes <!-- List concrete changes. If this adds an abstraction, configuration, fallback, migration, or compatibility path, name its current requirement and consumer, explain why a direct change is insufficient, and identify the test that protects it. --> - Detect an unavailable Docker Desktop credential helper on WSL 2 and create a temporary credential-free Docker config. - Apply the isolated config to managed-image creation, GPU probe pulls, and local-inference probe containers. - Preserve non-default Docker contexts and avoid copying unrelated host environment variables into isolated Docker commands. - Clean up temporary Docker configs after each operation. - Add tests for environment isolation, helper availability, default-context handling, and managed GPU image pulls. A prebuild-only fix is insufficient because managed-image onboarding and local-inference probes use separate Docker paths. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check one tests line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests 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 approval](#10470 (review)) covers credential isolation, environment minimization, Docker authority preservation, temporary-file permissions, cleanup/error handling, and security regression coverage. - [x] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: Final-head WSL manual E2E stopped before candidate execution because of the tracked Windows artifact-verification defect; accepted in [maintainer approval](#10470 (review)) and tracked in [#10569](#10569). Five non-required Advisor specialists were also unavailable after repeated infrastructure retries; completed specialists and manual review found no blocker. ## DGX Station Hardware Evidence <!-- Required only when scripts/prepare-dgx-station-host.sh changes. Maintainers must review the linked evidence before approving or merging. This is human-reviewed evidence, not authenticated hardware provenance. Exceptional bypasses use existing repository governance and must be documented on the PR. --> - [ ] Tested on DGX Station - Tested commit: N/A - Station profile/scenario: N/A - Result: N/A - Supporting evidence: N/A ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [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 — 113 tests passed across five affected suites at exact head `0a4c2fe415f7093a3549575c64eeb6a3fba54a51`. A/B onboarding on ipp1-1141 succeeded with the credential-helper stub still failing. - [x] Applicable broad gate passed — Final-head [CI](https://github.com/NVIDIA/NemoClaw/actions/runs/33207527032) passed all 12 CLI shards, aggregate `cli-tests`/`checks`, build/typecheck, installer integration, static/plugin/security/audit gates. Exact [managed-image E2E](https://github.com/NVIDIA/NemoClaw/actions/runs/33207527052) passed direct startup for all three agents, real all-agent managed activation, and two independent OpenClaw MCP discovery passes. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] 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) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Rui Luo <ruluo@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Docker isolation during local inference and sandbox setup. * Prevented sensitive credentials from being exposed during Docker image builds and probes. * Improved handling of Docker contexts, BuildKit, and WSL environments. * Improved sandbox resume behavior and validation for previously verified sandboxes. * Added clearer failure reporting and cleanup when Docker setup cannot be completed. * **Tests** * Expanded coverage for credential isolation, environment merging, BuildKit configuration, and local inference validation. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Rui Luo <ruluo@nvidia.com> Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
1 parent c166756 commit 29e79e7

15 files changed

Lines changed: 1221 additions & 462 deletions

src/lib/actions/sandbox/rebuild-custom-image-preflight.test.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ describe("preflightRebuildImage", () => {
316316
isolatedConfig = String(options.env?.DOCKER_CONFIG);
317317
expect(isolatedConfig).toContain("nemoclaw-wsl-buildkit-docker-config-");
318318
expect(isolatedConfig).not.toBe(dockerConfig);
319-
expect(options.env?.DOCKER_HOST).toBe("unix:///selected-docker.sock");
319+
expect(options.env?.DOCKER_HOST).toBeUndefined();
320320
expect(options.env?.DOCKER_CONTEXT).toBeUndefined();
321321
expect(options.env?.DOCKER_BUILDKIT).toBe("1");
322322
expect(
@@ -327,11 +327,10 @@ describe("preflightRebuildImage", () => {
327327
removeImage: vi.fn(() => ({ status: 0 }) as never),
328328
env: {
329329
DOCKER_CONFIG: dockerConfig,
330-
DOCKER_CONTEXT: "ambient-remote",
331-
DOCKER_HOST: "unix:///selected-docker.sock",
332330
WSL_DISTRO_NAME: "Ubuntu",
333331
},
334332
credentialHelperResponds,
333+
dockerContextIsDefault: () => true,
335334
isWslHost: true,
336335
}),
337336
);
@@ -377,8 +376,12 @@ describe("preflightRebuildImage", () => {
377376
[
378377
"#!/bin/sh",
379378
"set -eu",
379+
'if [ "$1" = "context" ] && [ "$2" = "show" ]; then',
380+
" printf 'default\\n'",
381+
" exit 0",
382+
"fi",
380383
'if [ "$1" = "build" ]; then',
381-
' [ "$DOCKER_HOST" = "unix:///selected-docker.sock" ]',
384+
' [ -z "${DOCKER_HOST+x}" ]',
382385
' [ -z "${DOCKER_CONTEXT+x}" ]',
383386
' [ -n "${DOCKER_CONFIG:-}" ]',
384387
` [ "$DOCKER_CONFIG" != "${dockerConfig}" ]`,
@@ -389,10 +392,7 @@ describe("preflightRebuildImage", () => {
389392
].join("\n"),
390393
{ mode: 0o700 },
391394
);
392-
vi.stubEnv(
393-
"PATH",
394-
`${executableRoot}${path.delimiter}${String(process.env.PATH ?? "")}`,
395-
);
395+
vi.stubEnv("PATH", `${executableRoot}${path.delimiter}${String(process.env.PATH ?? "")}`);
396396

397397
try {
398398
const result = successful(
@@ -410,8 +410,6 @@ describe("preflightRebuildImage", () => {
410410
})),
411411
env: {
412412
DOCKER_CONFIG: dockerConfig,
413-
DOCKER_CONTEXT: "ambient-remote",
414-
DOCKER_HOST: "unix:///selected-docker.sock",
415413
WSL_DISTRO_NAME: "Ubuntu",
416414
},
417415
isWslHost: true,

src/lib/actions/sandbox/rebuild-custom-image-preflight.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,18 @@
44
import path from "node:path";
55

66
import { dockerBuild, dockerRmi } from "../../adapters/docker";
7+
import {
8+
prepareDockerBuildEnvironment,
9+
type DockerBuildEnvironmentInput,
10+
warnIfDockerBuildEnvironmentCleanupFailed,
11+
} from "../../adapters/docker/client-isolation";
712
import { fingerprintBuildContext } from "../../adapters/fs/build-context-fingerprint";
813
import type { AgentDefinition } from "../../agent/defs";
914
import { createAgentSandbox } from "../../agent/onboard";
1015
import type { WebSearchConfig } from "../../inference/web-search";
1116
import type { SandboxMessagingPlan } from "../../messaging";
1217
import { stageCreateSandboxBuildContext } from "../../onboard/build-context-stage";
1318
import { patchStagedDockerfileMessagingPlan } from "../../onboard/dockerfile-patch";
14-
import {
15-
prepareDockerBuildEnvironment,
16-
type DockerBuildEnvironmentInput,
17-
} from "../../onboard/sandbox-prebuild";
1819
import {
1920
applyReasoningEffortEnv,
2021
REASONING_EFFORT_ENV,
@@ -108,6 +109,7 @@ function buildReplacementImage(
108109
const preparedEnvironment = (deps.prepareBuildEnvironment ?? prepareDockerBuildEnvironment)({
109110
env: deps.env,
110111
credentialHelperResponds: deps.credentialHelperResponds,
112+
dockerContextIsDefault: deps.dockerContextIsDefault,
111113
isWslHost: deps.isWslHost,
112114
allowCredentialIsolation: origin === "generated",
113115
});
@@ -119,7 +121,10 @@ function buildReplacementImage(
119121
stdio: ["ignore", "pipe", "pipe"],
120122
});
121123
} finally {
122-
preparedEnvironment.cleanup();
124+
warnIfDockerBuildEnvironmentCleanupFailed(
125+
preparedEnvironment.cleanup(),
126+
`rebuild image '${imageTag}'`,
127+
);
123128
}
124129
}
125130

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { spawnSync } from "node:child_process";
5+
import fs from "node:fs";
6+
import os from "node:os";
7+
import path from "node:path";
8+
9+
import { DOCKER_DESKTOP_CREDENTIAL_STORE_NAMES } from "../../domain/docker-host";
10+
import { isWsl } from "../../platform";
11+
import { buildSubprocessEnv } from "../../subprocess-env";
12+
import {
13+
dockerDesktopCredentialHelperResponds,
14+
readDockerCredentialStore,
15+
} from "./credential-store";
16+
import { dockerSpawnSync } from "./exec";
17+
18+
const DOCKER_ENV_NAMES = [
19+
"CONTAINERS_CONF",
20+
"DOCKER_API_VERSION",
21+
"DOCKER_CERT_PATH",
22+
"DOCKER_CONFIG",
23+
"DOCKER_CONTEXT",
24+
"DOCKER_HOST",
25+
"DOCKER_TLS_VERIFY",
26+
] as const;
27+
28+
export interface DockerBuildEnvironmentInput {
29+
env?: NodeJS.ProcessEnv;
30+
credentialHelperResponds?: (credsStore: string) => boolean;
31+
dockerContextIsDefault?: (env: NodeJS.ProcessEnv) => boolean;
32+
isWslHost?: boolean;
33+
allowCredentialIsolation?: boolean;
34+
}
35+
36+
export interface PreparedDockerBuildEnvironment {
37+
env: NodeJS.ProcessEnv;
38+
isolatedCredentialConfig: boolean;
39+
cleanup(): DockerBuildEnvironmentCleanupResult;
40+
}
41+
42+
export type DockerBuildEnvironmentCleanupResult =
43+
| { ok: true }
44+
| { ok: false; directory: string; error: string };
45+
46+
export function createCredentialFreeDockerConfig(purpose: "portable" | "wsl-buildkit"): string {
47+
const directory = fs.mkdtempSync(path.join(os.tmpdir(), `nemoclaw-${purpose}-docker-config-`));
48+
fs.chmodSync(directory, 0o700);
49+
fs.writeFileSync(path.join(directory, "config.json"), '{"auths":{}}\n', {
50+
encoding: "utf-8",
51+
flag: "wx",
52+
mode: 0o600,
53+
});
54+
return directory;
55+
}
56+
57+
/** Restrict the host Docker build to environment values used by Docker itself. */
58+
export function dockerBuildSubprocessEnv(
59+
sourceEnv: NodeJS.ProcessEnv = process.env,
60+
): Record<string, string> {
61+
const env = buildSubprocessEnv();
62+
for (const key of DOCKER_ENV_NAMES) {
63+
const value = sourceEnv[key];
64+
// sourceEnv owns Docker daemon and client selection. Do not let a Docker
65+
// variable omitted by the caller leak back in from the parent process.
66+
if (value === undefined) delete env[key];
67+
else env[key] = value;
68+
}
69+
for (const key of Object.keys(env)) {
70+
if (
71+
key === "KUBECONFIG" ||
72+
key === "SSH_AUTH_SOCK" ||
73+
key === "RUST_LOG" ||
74+
key === "RUST_BACKTRACE" ||
75+
key.startsWith("OPENSHELL_") ||
76+
key.startsWith("GRPC_")
77+
) {
78+
delete env[key];
79+
}
80+
}
81+
// Match the runner and Docker probe contract: an explicitly selected host
82+
// owns daemon authority, so an ambient context must not redirect the build.
83+
// Keep DOCKER_CONFIG because the selected daemon can still require registry
84+
// credentials or client certificates from that configuration.
85+
if (env.DOCKER_HOST !== undefined) {
86+
delete env.DOCKER_CONTEXT;
87+
}
88+
return env;
89+
}
90+
91+
function requiresCredentialFreeWslBuildConfig(
92+
env: NodeJS.ProcessEnv,
93+
helperResponds: (credsStore: string) => boolean,
94+
isWslHost?: boolean,
95+
): boolean {
96+
if (!isWsl({ env, isWsl: isWslHost })) return false;
97+
const { credsStore } = readDockerCredentialStore(env, fs.readFileSync);
98+
return (
99+
credsStore !== undefined &&
100+
DOCKER_DESKTOP_CREDENTIAL_STORE_NAMES.has(credsStore) &&
101+
!helperResponds(credsStore)
102+
);
103+
}
104+
105+
function dockerDesktopCredentialHelperRespondsFromBuild(
106+
credsStore: string,
107+
env: NodeJS.ProcessEnv,
108+
): boolean {
109+
return dockerDesktopCredentialHelperResponds(credsStore, (command, options) => {
110+
const [executable, ...args] = command;
111+
if (!executable) return null;
112+
const result = spawnSync(executable, args, {
113+
encoding: "utf-8",
114+
env: dockerBuildSubprocessEnv(env),
115+
shell: false,
116+
stdio: ["ignore", "pipe", "ignore"],
117+
timeout: options?.timeout,
118+
});
119+
return result.error || result.status !== 0 ? null : result.stdout;
120+
});
121+
}
122+
123+
function dockerContextIsDefaultFromBuild(env: NodeJS.ProcessEnv): boolean {
124+
// Any explicit endpoint owns daemon authority, including alternate Unix
125+
// sockets. Preserve its client configuration and registry credentials.
126+
if (env.DOCKER_HOST) return false;
127+
const result = dockerSpawnSync(["context", "show"], {
128+
encoding: "utf-8",
129+
env: dockerBuildSubprocessEnv(env),
130+
shell: false,
131+
stdio: ["ignore", "pipe", "ignore"],
132+
timeout: 5_000,
133+
});
134+
return !result.error && result.status === 0 && String(result.stdout).trim() === "default";
135+
}
136+
137+
/**
138+
* Prepare the Docker environment shared by normal creation and rebuild image
139+
* preflight. Docker Desktop can leave WSL pointing at a Windows credential
140+
* helper that is unavailable from the current session; generated image builds
141+
* do not need registry credentials, so isolate that case without modifying the
142+
* user's Docker config.
143+
*/
144+
export function prepareDockerBuildEnvironment(
145+
input: DockerBuildEnvironmentInput = {},
146+
): PreparedDockerBuildEnvironment {
147+
const sourceEnv = input.env ?? process.env;
148+
const helperResponds =
149+
input.credentialHelperResponds ??
150+
((credsStore: string) => dockerDesktopCredentialHelperRespondsFromBuild(credsStore, sourceEnv));
151+
const contextIsDefault = input.dockerContextIsDefault ?? dockerContextIsDefaultFromBuild;
152+
const credentialFreeConfig =
153+
input.allowCredentialIsolation !== false &&
154+
contextIsDefault(sourceEnv) &&
155+
requiresCredentialFreeWslBuildConfig(sourceEnv, helperResponds, input.isWslHost)
156+
? createCredentialFreeDockerConfig("wsl-buildkit")
157+
: null;
158+
return {
159+
env: {
160+
...dockerBuildSubprocessEnv(sourceEnv),
161+
DOCKER_BUILDKIT: "1",
162+
...(credentialFreeConfig ? { DOCKER_CONFIG: credentialFreeConfig } : {}),
163+
},
164+
isolatedCredentialConfig: credentialFreeConfig !== null,
165+
cleanup: () => {
166+
if (credentialFreeConfig === null) return { ok: true };
167+
try {
168+
fs.rmSync(credentialFreeConfig, { recursive: true, force: true });
169+
return { ok: true };
170+
} catch (error) {
171+
return {
172+
ok: false,
173+
directory: credentialFreeConfig,
174+
error: error instanceof Error ? error.message : String(error),
175+
};
176+
}
177+
},
178+
};
179+
}
180+
181+
function boundedCleanupDiagnostic(value: string): string {
182+
return value.replace(/[^\x20-\x7E]/gu, "?").slice(0, 240);
183+
}
184+
185+
/** Warn without replacing the Docker operation result when temporary cleanup fails. */
186+
export function warnIfDockerBuildEnvironmentCleanupFailed(
187+
result: DockerBuildEnvironmentCleanupResult,
188+
operation: string,
189+
warn: (message: string) => void = console.warn,
190+
): void {
191+
if (result.ok) return;
192+
const directory = boundedCleanupDiagnostic(result.directory);
193+
const operationLabel = boundedCleanupDiagnostic(operation);
194+
const detail = boundedCleanupDiagnostic(result.error);
195+
try {
196+
warn(
197+
` Warning: failed to remove credential-free Docker config '${directory}' after ${operationLabel}: ${detail}. It contains no credentials and can be removed after Docker no longer uses it.`,
198+
);
199+
} catch {
200+
// Cleanup diagnostics must never replace the Docker operation result.
201+
}
202+
}
203+
204+
/** Overlay a credential-free Docker client config onto a host subprocess env. */
205+
export function mergeIsolatedDockerClientEnv(
206+
targetEnv: NodeJS.ProcessEnv,
207+
prepared: PreparedDockerBuildEnvironment,
208+
): NodeJS.ProcessEnv {
209+
const isolatedConfig = prepared.isolatedCredentialConfig ? prepared.env.DOCKER_CONFIG : undefined;
210+
return isolatedConfig === undefined ? targetEnv : { ...targetEnv, DOCKER_CONFIG: isolatedConfig };
211+
}

src/lib/adapters/docker/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ export * from "./exec";
66
export * from "./pull";
77
export * from "./info";
88
export * from "./runtime";
9+
export * from "./client-isolation";
910
export * from "./inspect";
1011
export * from "./image";
1112
export * from "./container";

src/lib/adapters/docker/runtime.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,12 @@ export function detectContainerRuntimeFromDockerInfo(
2828
const timeout = Math.max(1, Math.floor(opts.timeoutMs ?? DOCKER_INFO_RUNTIME_PROBE_TIMEOUT_MS));
2929
const probe = opts.dockerInfoImpl ?? dockerInfo;
3030

31-
return retryUntil(
32-
() => inferContainerRuntime(probe({ ignoreError: true, timeout })),
33-
{
34-
accept: (runtime) => runtime !== "unknown",
35-
retryDelaysMs: Array.from(
36-
{ length: attempts - 1 },
37-
() => DOCKER_INFO_RUNTIME_PROBE_RETRY_DELAY_MS,
38-
),
39-
sleep: opts.sleep ?? sleepMs,
40-
},
41-
);
31+
return retryUntil(() => inferContainerRuntime(probe({ ignoreError: true, timeout })), {
32+
accept: (runtime) => runtime !== "unknown",
33+
retryDelaysMs: Array.from(
34+
{ length: attempts - 1 },
35+
() => DOCKER_INFO_RUNTIME_PROBE_RETRY_DELAY_MS,
36+
),
37+
sleep: opts.sleep ?? sleepMs,
38+
});
4239
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
import { describe, expect, it, vi } from "vitest";
5+
6+
import { validateLocalProvider } from "./local";
7+
8+
describe("local inference Docker client isolation", () => {
9+
it("passes only the isolated Docker config to local-inference probe commands (#10349)", () => {
10+
const cleanup = vi.fn(() => ({ ok: true as const }));
11+
const isolatedConfig = "/tmp/nemoclaw-isolated-docker-config";
12+
const calls: Array<{ cmd: readonly string[]; env: NodeJS.ProcessEnv | undefined }> = [];
13+
const outputs = ['{"models":[]}', "200"];
14+
const mockCapture = (
15+
cmd: readonly string[],
16+
opts?: { ignoreError?: boolean; env?: NodeJS.ProcessEnv },
17+
) => {
18+
calls.push({ cmd, env: opts?.env });
19+
return outputs[calls.length - 1] ?? "";
20+
};
21+
22+
const result = validateLocalProvider(
23+
"ollama-local",
24+
mockCapture,
25+
() => {},
26+
() => ({
27+
env: { DOCKER_CONFIG: isolatedConfig },
28+
isolatedCredentialConfig: true,
29+
cleanup,
30+
}),
31+
);
32+
33+
expect(result.ok).toBe(true);
34+
expect(calls[0]?.cmd[0]).toBe("curl");
35+
expect(calls[0]?.env).toBeUndefined();
36+
expect(calls[1]?.cmd[0]).toBe("docker");
37+
expect(calls[1]?.env).toEqual({ DOCKER_CONFIG: isolatedConfig });
38+
expect(calls[1]?.env).not.toHaveProperty("NVIDIA_INFERENCE_API_KEY");
39+
expect(cleanup).toHaveBeenCalledOnce();
40+
});
41+
});

0 commit comments

Comments
 (0)