Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions src/lib/cluster-image-patch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,9 +363,15 @@ describe("ensurePatchedClusterImage", () => {
expect(buildCall?.opts).toMatchObject({ suppressOutput: true });
});

it("throws ClusterImagePatchError on docker build failure", () => {
let upstreamInspectCount = 0;
expect(() =>
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({
upstreamImage: UPSTREAM,
runCaptureImpl: (cmd) => {
Expand All @@ -375,11 +381,24 @@ describe("ensurePatchedClusterImage", () => {
}
return "";
},
runImpl: (cmd) => (cmd[1] === "build" ? { status: 2 } : { status: 0 }),
runImpl: (cmd) =>
cmd[1] === "build"
? {
status: null,
error: new Error(spawnCause),
stderr: `Authorization: Basic ${basicCredential}\nfailed to resolve source metadata: Bearer ${token}`,
}
: { status: 0 },
logger: () => {},
fsImpl: createMockFs(),
tmpdirImpl: () => "/tmp",
}),
).toThrow(ClusterImagePatchError);
});
};

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);
});
});
24 changes: 20 additions & 4 deletions src/lib/cluster-image-patch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -51,6 +53,13 @@ export interface RunOpts {
suppressOutput?: boolean;
}

export interface RunResult {
status: number | null;
error?: unknown;
stdout?: unknown;
stderr?: unknown;
}

export interface EnsurePatchedClusterImageOpts {
/** Upstream OpenShell cluster image reference, e.g. `ghcr.io/nvidia/openshell/cluster:0.0.36`. */
upstreamImage: string;
Expand All @@ -59,7 +68,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). */
Expand Down Expand Up @@ -330,9 +339,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 {
Expand Down Expand Up @@ -393,11 +404,16 @@ 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,
error: result.error,
stdout: result.stdout,
stderr: result.stderr,
};
}
28 changes: 28 additions & 0 deletions src/lib/sandbox-base-image-resolution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
59 changes: 59 additions & 0 deletions src/lib/sandbox-base-image.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 <REDACTED>");
expect(output).toContain("Proxy-Authorization: Digest <REDACTED>");
expect(output).toContain("Cookie: <REDACTED>");
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"),
Expand Down
35 changes: 28 additions & 7 deletions src/lib/sandbox-base-image.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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";
Expand All @@ -35,26 +39,43 @@ 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");
return String(stream);
})
.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(), "<tmp>"],
] 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 {
Expand Down Expand Up @@ -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;
Expand Down