From 5b0434e0c49f9f86b8772f2412f7c321d6031297 Mon Sep 17 00:00:00 2001 From: Chengjie Wang Date: Fri, 10 Jul 2026 11:37:37 +0800 Subject: [PATCH 1/3] fix(onboard): surface cluster image build failures Signed-off-by: Chengjie Wang --- src/lib/cluster-image-patch.test.ts | 30 +++++++++++++++++++++++++++++ src/lib/cluster-image-patch.ts | 18 +++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/lib/cluster-image-patch.test.ts b/src/lib/cluster-image-patch.test.ts index 2be76645c3c..c68901db19b 100644 --- a/src/lib/cluster-image-patch.test.ts +++ b/src/lib/cluster-image-patch.test.ts @@ -382,4 +382,34 @@ describe("ensurePatchedClusterImage", () => { }), ).toThrow(ClusterImagePatchError); }); + + it("surfaces redacted docker build diagnostics on failure (#6622)", () => { + const token = ["sk", "abcdef0123456789abcdef0123456789abcdef0123456789"].join("-"); + const reproduce = () => { + let upstreamInspectCount = 0; + ensurePatchedClusterImage({ + upstreamImage: UPSTREAM, + runCaptureImpl: (cmd) => { + if (inspectAt(cmd) === "upstream") { + upstreamInspectCount += 1; + return upstreamInspectCount === 1 ? "" : "sha256:abcd"; + } + return ""; + }, + runImpl: (cmd) => + cmd[1] === "build" + ? { + status: 1, + stderr: `failed to resolve source metadata: Bearer ${token}`, + } + : { status: 0 }, + logger: () => {}, + fsImpl: createMockFs(), + tmpdirImpl: () => "/tmp", + }); + }; + + expect(reproduce).toThrowError(/failed to resolve source metadata/); + expect(reproduce).not.toThrowError(token); + }); }); diff --git a/src/lib/cluster-image-patch.ts b/src/lib/cluster-image-patch.ts index 9ba03b7ed18..520d38af8eb 100644 --- a/src/lib/cluster-image-patch.ts +++ b/src/lib/cluster-image-patch.ts @@ -26,6 +26,8 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { formatBuildFailureDiagnostics } from "./sandbox-base-image"; + export type SnapshotterChoice = "fuse-overlayfs" | "native"; export const DEFAULT_SNAPSHOTTER: SnapshotterChoice = "fuse-overlayfs"; @@ -51,6 +53,12 @@ export interface RunOpts { suppressOutput?: boolean; } +export interface RunResult { + status: number | null; + stdout?: unknown; + stderr?: unknown; +} + export interface EnsurePatchedClusterImageOpts { /** Upstream OpenShell cluster image reference, e.g. `ghcr.io/nvidia/openshell/cluster:0.0.36`. */ upstreamImage: string; @@ -59,7 +67,7 @@ export interface EnsurePatchedClusterImageOpts { /** Captures stdout from a command (used to probe `docker image inspect`). */ runCaptureImpl?: (cmd: readonly string[], opts?: RunOpts) => string; /** Streams a command's stdio (used for `docker pull` and `docker build`). */ - runImpl?: (cmd: readonly string[], opts?: RunOpts) => { status: number | null }; + runImpl?: (cmd: readonly string[], opts?: RunOpts) => RunResult; /** Logger for human-readable progress lines. Defaults to console.error. */ logger?: (msg: string) => void; /** Filesystem implementation (testing seam). */ @@ -330,9 +338,11 @@ export function ensurePatchedClusterImage(opts: EnsurePatchedClusterImageOpts): ); if (buildResult.status !== 0) { + const diagnostics = formatBuildFailureDiagnostics(buildResult); throw new ClusterImagePatchError( `failed to build patched cluster image ` + - `(docker build exit ${buildResult.status}; timeout ${buildTimeoutMs} ms)`, + `(docker build exit ${buildResult.status}; timeout ${buildTimeoutMs} ms)` + + (diagnostics ? `\n${diagnostics}` : ""), ); } } finally { @@ -393,11 +403,11 @@ function defaultRunCapture(cmd: readonly string[], opts: RunOpts = {}): string { }); } -function defaultRun(cmd: readonly string[], opts: RunOpts = {}): { status: number | null } { +function defaultRun(cmd: readonly string[], opts: RunOpts = {}): RunResult { const result = runner.run(cmd, { ignoreError: opts.ignoreError, suppressOutput: opts.suppressOutput, ...(opts.timeoutMs !== undefined ? { timeout: opts.timeoutMs } : {}), }); - return { status: result.status ?? null }; + return { status: result.status ?? null, stdout: result.stdout, stderr: result.stderr }; } From c88c1c10ebf2f480b25001d82ac5edf16df81c96 Mon Sep 17 00:00:00 2001 From: Chengjie Wang Date: Fri, 10 Jul 2026 11:47:45 +0800 Subject: [PATCH 2/3] test(onboard): keep cluster image regression linear Signed-off-by: Chengjie Wang --- src/lib/cluster-image-patch.test.ts | 26 ++------------------------ 1 file changed, 2 insertions(+), 24 deletions(-) diff --git a/src/lib/cluster-image-patch.test.ts b/src/lib/cluster-image-patch.test.ts index c68901db19b..6d56f2dc1a9 100644 --- a/src/lib/cluster-image-patch.test.ts +++ b/src/lib/cluster-image-patch.test.ts @@ -363,26 +363,6 @@ describe("ensurePatchedClusterImage", () => { expect(buildCall?.opts).toMatchObject({ suppressOutput: true }); }); - it("throws ClusterImagePatchError on docker build failure", () => { - let upstreamInspectCount = 0; - expect(() => - ensurePatchedClusterImage({ - upstreamImage: UPSTREAM, - runCaptureImpl: (cmd) => { - if (inspectAt(cmd) === "upstream") { - upstreamInspectCount += 1; - return upstreamInspectCount === 1 ? "" : "sha256:abcd"; - } - return ""; - }, - runImpl: (cmd) => (cmd[1] === "build" ? { status: 2 } : { status: 0 }), - logger: () => {}, - fsImpl: createMockFs(), - tmpdirImpl: () => "/tmp", - }), - ).toThrow(ClusterImagePatchError); - }); - it("surfaces redacted docker build diagnostics on failure (#6622)", () => { const token = ["sk", "abcdef0123456789abcdef0123456789abcdef0123456789"].join("-"); const reproduce = () => { @@ -398,10 +378,7 @@ describe("ensurePatchedClusterImage", () => { }, runImpl: (cmd) => cmd[1] === "build" - ? { - status: 1, - stderr: `failed to resolve source metadata: Bearer ${token}`, - } + ? { status: 2, stderr: `failed to resolve source metadata: Bearer ${token}` } : { status: 0 }, logger: () => {}, fsImpl: createMockFs(), @@ -409,6 +386,7 @@ describe("ensurePatchedClusterImage", () => { }); }; + expect(reproduce).toThrowError(ClusterImagePatchError); expect(reproduce).toThrowError(/failed to resolve source metadata/); expect(reproduce).not.toThrowError(token); }); From 5769ce161fe7ed51e99fbf9df28a5a5cd497afcf Mon Sep 17 00:00:00 2001 From: Charan Jagwani Date: Fri, 10 Jul 2026 10:10:03 -0700 Subject: [PATCH 3/3] fix(onboard): harden cluster build diagnostics Signed-off-by: Charan Jagwani --- src/lib/cluster-image-patch.test.ts | 13 +++- src/lib/cluster-image-patch.ts | 8 ++- src/lib/sandbox-base-image-resolution.test.ts | 28 +++++++++ src/lib/sandbox-base-image.test.ts | 59 +++++++++++++++++++ src/lib/sandbox-base-image.ts | 35 ++++++++--- 5 files changed, 134 insertions(+), 9 deletions(-) diff --git a/src/lib/cluster-image-patch.test.ts b/src/lib/cluster-image-patch.test.ts index 6d56f2dc1a9..9a1632aa1fc 100644 --- a/src/lib/cluster-image-patch.test.ts +++ b/src/lib/cluster-image-patch.test.ts @@ -365,6 +365,11 @@ describe("ensurePatchedClusterImage", () => { it("surfaces redacted docker build diagnostics on failure (#6622)", () => { const token = ["sk", "abcdef0123456789abcdef0123456789abcdef0123456789"].join("-"); + const basicCredential = Buffer.from( + ["build-user", "build-password"].join(":"), + "utf8", + ).toString("base64"); + const spawnCause = ["spawn", "docker", "EACCES"].join(" "); const reproduce = () => { let upstreamInspectCount = 0; ensurePatchedClusterImage({ @@ -378,7 +383,11 @@ describe("ensurePatchedClusterImage", () => { }, runImpl: (cmd) => cmd[1] === "build" - ? { status: 2, stderr: `failed to resolve source metadata: Bearer ${token}` } + ? { + status: null, + error: new Error(spawnCause), + stderr: `Authorization: Basic ${basicCredential}\nfailed to resolve source metadata: Bearer ${token}`, + } : { status: 0 }, logger: () => {}, fsImpl: createMockFs(), @@ -388,6 +397,8 @@ describe("ensurePatchedClusterImage", () => { expect(reproduce).toThrowError(ClusterImagePatchError); expect(reproduce).toThrowError(/failed to resolve source metadata/); + expect(reproduce).toThrowError(spawnCause); expect(reproduce).not.toThrowError(token); + expect(reproduce).not.toThrowError(basicCredential); }); }); diff --git a/src/lib/cluster-image-patch.ts b/src/lib/cluster-image-patch.ts index 520d38af8eb..e41bab33b9a 100644 --- a/src/lib/cluster-image-patch.ts +++ b/src/lib/cluster-image-patch.ts @@ -55,6 +55,7 @@ export interface RunOpts { export interface RunResult { status: number | null; + error?: unknown; stdout?: unknown; stderr?: unknown; } @@ -409,5 +410,10 @@ function defaultRun(cmd: readonly string[], opts: RunOpts = {}): RunResult { suppressOutput: opts.suppressOutput, ...(opts.timeoutMs !== undefined ? { timeout: opts.timeoutMs } : {}), }); - return { status: result.status ?? null, stdout: result.stdout, stderr: result.stderr }; + return { + status: result.status ?? null, + error: result.error, + stdout: result.stdout, + stderr: result.stderr, + }; } diff --git a/src/lib/sandbox-base-image-resolution.test.ts b/src/lib/sandbox-base-image-resolution.test.ts index 22c63829537..23929cd4874 100644 --- a/src/lib/sandbox-base-image-resolution.test.ts +++ b/src/lib/sandbox-base-image-resolution.test.ts @@ -275,6 +275,34 @@ describe("sandbox base-image warm resolution", () => { error.mockRestore(); }); + it("redacts a local rebuild spawn error before logging it", () => { + const token = ["spawn", "secret", "token"].join("-"); + sourceMocks.inputsChanged.mockReturnValue(true); + dockerMocks.imageInspect.mockReturnValue({ status: 1 }); + dockerMocks.pull.mockReturnValue({ status: 1 }); + dockerMocks.build.mockReturnValue({ + status: null, + error: new Error(`spawn docker EACCES: Bearer ${token}`), + }); + const error = vi.spyOn(console, "error").mockImplementation(() => {}); + + expect(() => + resolveSandboxBaseImage({ + ...resolutionOptions(), + env: { + ...resolutionOptions().env, + NEMOCLAW_SANDBOX_BASE_LOCAL_BUILD: "1", + }, + }), + ).toThrow(SandboxBaseImageResolutionError); + + const logged = error.mock.calls.flat().join("\n"); + expect(logged).toContain("spawn docker EACCES"); + expect(logged).toContain("process launch failed"); + expect(logged).not.toContain(token); + error.mockRestore(); + }); + it("rebuilds dirty base inputs before considering published or existing local candidates (#4680)", () => { sourceMocks.inputsDirty.mockReturnValue(true); dockerMocks.imageInspect.mockReturnValue({ status: 0 }); diff --git a/src/lib/sandbox-base-image.test.ts b/src/lib/sandbox-base-image.test.ts index cfae8f69707..07a795870ef 100644 --- a/src/lib/sandbox-base-image.test.ts +++ b/src/lib/sandbox-base-image.test.ts @@ -1,6 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import os from "node:os"; +import path from "node:path"; + import { describe, expect, it } from "vitest"; import { formatBuildFailureDiagnostics } from "./sandbox-base-image"; @@ -46,6 +49,62 @@ describe("sandbox base-image build diagnostics", () => { expect(output).not.toContain("sk-abcdef0123456789abcdef0123456789abcdef0123456789"); }); + it("redacts structured credentials and credentialed URLs in build output", () => { + const basicCredential = Buffer.from( + ["build-user", "build-password"].join(":"), + "utf8", + ).toString("base64"); + const digestResponse = ["digest", "response", "secret"].join("-"); + const cookieValue = ["session", "cookie-secret"].join("="); + const urlPassword = ["registry", "password"].join("-"); + const queryToken = ["query", "token", "secret"].join("-"); + const terminalLink = ["https://", "terminal.example.test", "/hidden"].join(""); + const homePath = path.join(os.homedir(), ".docker", "config.json"); + const temporaryPath = path.join(os.tmpdir(), "nemoclaw-build", "metadata.json"); + const output = formatBuildFailureDiagnostics({ + stderr: [ + `Authorization: Basic ${basicCredential}`, + `Proxy-Authorization: Digest username="build-user", response="${digestResponse}"`, + `Cookie: ${cookieValue}`, + `failed to fetch https://build-user:${urlPassword}@registry.example.test/v2/layer?token=${queryToken}`, + `terminal link: \u001b]8;;${terminalLink}\u0007open\u001b]8;;\u0007`, + `home config: ${homePath}`, + `temporary metadata: ${temporaryPath}`, + ].join("\n"), + }); + + expect(output).not.toContain(basicCredential); + expect(output).not.toContain(digestResponse); + expect(output).not.toContain(cookieValue); + expect(output).not.toContain(urlPassword); + expect(output).not.toContain(queryToken); + expect(output).not.toContain(terminalLink); + expect(output).not.toContain("\u001b"); + expect(output).not.toContain(homePath); + expect(output).not.toContain(temporaryPath); + expect(output).toContain("Authorization: Basic "); + expect(output).toContain("Proxy-Authorization: Digest "); + expect(output).toContain("Cookie: "); + expect(output).toContain("****"); + }); + + it("bounds captured build diagnostics before returning them", () => { + const output = formatBuildFailureDiagnostics({ stderr: "x".repeat(10_000) }); + + expect(output.length).toBeLessThan(8_100); + expect(output.endsWith("[diagnostic truncated]")).toBe(true); + }); + + it("surfaces a redacted spawn failure cause", () => { + const token = ["spawn", "secret", "token"].join("-"); + const output = formatBuildFailureDiagnostics({ + error: new Error(`spawn docker EACCES: Bearer ${token}`), + }); + + expect(output).toContain("spawn docker EACCES"); + expect(output).not.toContain(token); + }); + it("accepts Buffer streams from spawnSync", () => { const output = formatBuildFailureDiagnostics({ stderr: Buffer.from("buffered build error", "utf8"), diff --git a/src/lib/sandbox-base-image.ts b/src/lib/sandbox-base-image.ts index 7fb15189b2b..cbcb59071f0 100644 --- a/src/lib/sandbox-base-image.ts +++ b/src/lib/sandbox-base-image.ts @@ -1,6 +1,9 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +import os from "node:os"; +import path from "node:path"; +import { stripVTControlCharacters } from "node:util"; import { dockerBuild, dockerImageInspect, @@ -26,6 +29,7 @@ import { SANDBOX_BASE_TAG, type SandboxBaseImageResolution, } from "./sandbox-base-image/types"; +import { redactFull } from "./security/redact"; import { addTraceEvent } from "./trace"; export * from "./sandbox-base-image/image-compatibility"; @@ -35,18 +39,22 @@ export * from "./sandbox-base-image/resolution-metadata"; export * from "./sandbox-base-image/source-identity"; export * from "./sandbox-base-image/types"; +const BUILD_FAILURE_DIAGNOSTIC_LIMIT = 8_000; +const BUILD_FAILURE_TRUNCATED_SUFFIX = "\n[diagnostic truncated]"; + /** * Combine stderr + stdout from a captured `dockerBuild` failure and pass them - * through the runner's redaction so secrets in build output never reach the - * terminal. BuildKit splits diagnostics across both streams depending on the - * backend and progress mode, so taking only stderr can hide the actual reason - * a build failed. + * through the complete diagnostic redaction pipeline so secrets, host paths, + * and terminal control sequences never reach the terminal. BuildKit splits + * diagnostics across both streams depending on the backend and progress mode, + * so taking only stderr can hide the actual reason a build failed. */ export function formatBuildFailureDiagnostics(buildResult: { + error?: unknown; stderr?: unknown; stdout?: unknown; }): string { - const streams = [buildResult.stderr, buildResult.stdout] + const streams = [buildResult.error, buildResult.stderr, buildResult.stdout] .map((stream) => { if (stream == null) return ""; if (Buffer.isBuffer(stream)) return stream.toString("utf8"); @@ -54,7 +62,20 @@ export function formatBuildFailureDiagnostics(buildResult: { }) .map((text) => text.trim()) .filter((text) => text.length > 0); - return streams.length > 0 ? redact(streams.join("\n")) : ""; + if (streams.length === 0) return ""; + + let diagnostics = redact(redactFull(stripVTControlCharacters(streams.join("\n")))); + for (const [prefix, replacement] of [ + [process.env.HOME, "~"], + [os.homedir(), "~"], + [os.tmpdir(), ""], + ] as const) { + if (!prefix || prefix === path.parse(prefix).root) continue; + diagnostics = diagnostics.replaceAll(prefix, replacement); + } + return diagnostics.length > BUILD_FAILURE_DIAGNOSTIC_LIMIT + ? `${diagnostics.slice(0, BUILD_FAILURE_DIAGNOSTIC_LIMIT)}${BUILD_FAILURE_TRUNCATED_SUFFIX}` + : diagnostics; } function localBuildAllowed(env: NodeJS.ProcessEnv = process.env): boolean { @@ -194,7 +215,7 @@ function resolveLocalCandidate( const diagnostics = formatBuildFailureDiagnostics(buildResult); if (diagnostics) console.error(diagnostics); const detail = buildResult.error - ? `: ${buildResult.error.message}` + ? " (process launch failed)" : ` (exit ${buildResult.status ?? "unknown"})`; console.error(` Failed to build ${label}${detail}`); return null;