diff --git a/src/lib/actions/sandbox/sandbox-exec-output.test.ts b/src/lib/actions/sandbox/sandbox-exec-output.test.ts new file mode 100644 index 00000000000..41fcde04941 --- /dev/null +++ b/src/lib/actions/sandbox/sandbox-exec-output.test.ts @@ -0,0 +1,112 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { Buffer } from "node:buffer"; +import { describe, expect, it } from "vitest"; +import { + buildSandboxExecMarkedCommand, + createSandboxExecMarker, + extractSandboxExecCommandStdout, + extractSandboxExecCommandStdoutFromStreams, + SANDBOX_EXEC_STARTED_MARKER, +} from "./sandbox-exec-output"; + +describe("buildSandboxExecMarkedCommand", () => { + it("prints the sentinel before the command for ordinary scripts", () => { + const command = buildSandboxExecMarkedCommand("echo hi"); + expect(command).toBe(`printf '%s\\n' '${SANDBOX_EXEC_STARTED_MARKER}'; echo hi`); + }); + + it("base64-encodes the hermes secret boundary script instead of inlining it", () => { + const script = "python3 validate-hermes-env-secret-boundary.py --check"; + const command = buildSandboxExecMarkedCommand(script); + + expect(command).toContain(`printf '%s\\n' '${SANDBOX_EXEC_STARTED_MARKER}'`); + expect(command).not.toContain(script); + const encoded = Buffer.from(script, "utf8").toString("base64"); + expect(command).toContain(encoded); + }); + + it("creates a fresh shell-safe marker for each exec", () => { + const first = createSandboxExecMarker(); + const second = createSandboxExecMarker(); + + expect(first).toMatch(new RegExp(`^${SANDBOX_EXEC_STARTED_MARKER}_[0-9a-f]{32}$`)); + expect(second).not.toBe(first); + expect(buildSandboxExecMarkedCommand("echo hi", first)).toContain(`'${first}'`); + }); + + it("rejects a custom marker containing shell syntax", () => { + expect(() => + buildSandboxExecMarkedCommand("echo hi", "x'; echo NEMOCLAW_MARKER_INJECTION; '"), + ).toThrow("Invalid sandbox exec marker"); + }); +}); + +describe("extractSandboxExecCommandStdout", () => { + it("returns null for empty output", () => { + expect(extractSandboxExecCommandStdout("")).toBeNull(); + expect(extractSandboxExecCommandStdout(" \n ")).toBeNull(); + }); + + it("returns null when the sentinel never appears", () => { + expect(extractSandboxExecCommandStdout("exec failed\n")).toBeNull(); + }); + + it("extracts stdout after a raw, unframed sentinel", () => { + const output = `${SANDBOX_EXEC_STARTED_MARKER}\nNEMOCLAW_DCODE_PROBE=idle\n`; + expect(extractSandboxExecCommandStdout(output)).toBe("NEMOCLAW_DCODE_PROBE=idle"); + }); + + it("strips the 'stdout: ' frame prefix", () => { + const output = `stdout: ${SANDBOX_EXEC_STARTED_MARKER}\nstdout: NEMOCLAW_DCODE_PROBE=idle\n`; + expect(extractSandboxExecCommandStdout(output)).toBe("NEMOCLAW_DCODE_PROBE=idle"); + }); + + it("strips the '[stdout] ' frame prefix", () => { + const output = `[stdout] ${SANDBOX_EXEC_STARTED_MARKER}\n[stdout] NEMOCLAW_DCODE_PROBE=active\n`; + expect(extractSandboxExecCommandStdout(output)).toBe("NEMOCLAW_DCODE_PROBE=active"); + }); + + it("rejects duplicate sentinel lines as an ambiguous parser boundary", () => { + const output = [ + SANDBOX_EXEC_STARTED_MARKER, + "NEMOCLAW_DCODE_PROBE=idle", + SANDBOX_EXEC_STARTED_MARKER, + "NEMOCLAW_DCODE_PROBE=active", + ].join("\n"); + + expect(extractSandboxExecCommandStdout(output)).toBeNull(); + }); + + it("does not match a sentinel embedded in a preamble line as a substring", () => { + const output = `some login banner ${SANDBOX_EXEC_STARTED_MARKER} noise\n${SANDBOX_EXEC_STARTED_MARKER}\nNEMOCLAW_DCODE_PROBE=idle\n`; + expect(extractSandboxExecCommandStdout(output)).toBe("NEMOCLAW_DCODE_PROBE=idle"); + }); + + it("extracts a framed marker from stderr when stdout does not contain one", () => { + const marker = createSandboxExecMarker(); + expect( + extractSandboxExecCommandStdoutFromStreams( + { + stdout: "OpenShell transport preamble\n", + stderr: `stdout: ${marker}\nstdout: NEMOCLAW_DCODE_PROBE=idle\n`, + }, + marker, + ), + ).toBe("NEMOCLAW_DCODE_PROBE=idle"); + }); + + it("rejects the same marker split across stdout and stderr", () => { + const marker = createSandboxExecMarker(); + expect( + extractSandboxExecCommandStdoutFromStreams( + { + stdout: `${marker}\nNEMOCLAW_DCODE_PROBE=active\n`, + stderr: `stdout: ${marker}\nstdout: NEMOCLAW_DCODE_PROBE=idle\n`, + }, + marker, + ), + ).toBeNull(); + }); +}); diff --git a/src/lib/actions/sandbox/sandbox-exec-output.ts b/src/lib/actions/sandbox/sandbox-exec-output.ts index 9e5b54ede42..96291f8f262 100644 --- a/src/lib/actions/sandbox/sandbox-exec-output.ts +++ b/src/lib/actions/sandbox/sandbox-exec-output.ts @@ -2,16 +2,38 @@ // SPDX-License-Identifier: Apache-2.0 import { Buffer } from "node:buffer"; +import { randomBytes } from "node:crypto"; export const SANDBOX_EXEC_STARTED_MARKER = "__NEMOCLAW_SANDBOX_EXEC_STARTED__"; +const GENERATED_SANDBOX_EXEC_MARKER_PATTERN = new RegExp( + `^${SANDBOX_EXEC_STARTED_MARKER}_[0-9a-f]{32}$`, +); -export function buildSandboxExecMarkedCommand(command: string): string { +function assertSandboxExecMarker(marker: string): void { + if ( + marker === SANDBOX_EXEC_STARTED_MARKER || + GENERATED_SANDBOX_EXEC_MARKER_PATTERN.test(marker) + ) { + return; + } + throw new Error("Invalid sandbox exec marker"); +} + +export function createSandboxExecMarker(): string { + return `${SANDBOX_EXEC_STARTED_MARKER}_${randomBytes(16).toString("hex")}`; +} + +export function buildSandboxExecMarkedCommand( + command: string, + marker = SANDBOX_EXEC_STARTED_MARKER, +): string { + assertSandboxExecMarker(marker); if (!command.includes("validate-hermes-env-secret-boundary.py")) { - return `printf '%s\\n' '${SANDBOX_EXEC_STARTED_MARKER}'; ${command}`; + return `printf '%s\\n' '${marker}'; ${command}`; } const encodedCommand = Buffer.from(command, "utf8").toString("base64"); return [ - `printf '%s\\n' '${SANDBOX_EXEC_STARTED_MARKER}'`, + `printf '%s\\n' '${marker}'`, "command -v base64 >/dev/null 2>&1 || { echo NEMOCLAW_BASE64_MISSING >&2; exit 127; }", `printf '%s' '${encodedCommand}' | base64 -d | sh`, ].join("; "); @@ -32,22 +54,36 @@ function parseSandboxExecStdoutFrame(line: string): { text: string; framed: bool * stdout frame prefixes at this transport boundary so recovery, status, and * Hermes boundary callers keep consuming plain command stdout. * - * Security boundary: the sentinel must occupy its own stdout line after optional - * frame-prefix stripping. A preamble that merely contains the sentinel string is - * rejected so sandbox output cannot move the parser boundary forward. Remove - * this compatibility shim once OpenShell exposes a stable machine-readable exec - * output mode that preserves child stdout/stderr without human framing. + * Security boundary: accept exactly one marker across the captured stdout and + * stderr streams. A duplicate before or after the authentic boundary is + * ambiguous and must fail closed. A fresh marker for each exec also prevents + * fixed preamble text from being mistaken for the current boundary. + * + * Remove this compatibility shim once OpenShell exposes a stable + * machine-readable exec output mode that preserves child stdout/stderr + * without human framing. */ -export function extractSandboxExecCommandStdout(output: string): string | null { - const stdout = output.trim(); - if (!stdout) return null; - const lines = stdout.split(/\r?\n/).map(parseSandboxExecStdoutFrame); - const exactMarkerIndex = lines.findIndex( - (line) => line.text.trim() === SANDBOX_EXEC_STARTED_MARKER, - ); - if (exactMarkerIndex >= 0) { - return lines - .slice(exactMarkerIndex + 1) +export function extractSandboxExecCommandStdoutFromStreams( + streams: { stdout?: string; stderr?: string }, + marker = SANDBOX_EXEC_STARTED_MARKER, +): string | null { + let markerLocation: { lines: Array<{ text: string; framed: boolean }>; index: number } | null = + null; + + for (const output of [streams.stdout ?? "", streams.stderr ?? ""]) { + const normalized = output.trim(); + if (!normalized) continue; + const lines = normalized.split(/\r?\n/).map(parseSandboxExecStdoutFrame); + for (let index = 0; index < lines.length; index += 1) { + if (lines[index].text.trim() !== marker) continue; + if (markerLocation !== null) return null; + markerLocation = { lines, index }; + } + } + + if (markerLocation !== null) { + return markerLocation.lines + .slice(markerLocation.index + 1) .map((line) => line.text) .join("\n") .trim(); @@ -55,3 +91,10 @@ export function extractSandboxExecCommandStdout(output: string): string | null { return null; } + +export function extractSandboxExecCommandStdout( + output: string, + marker = SANDBOX_EXEC_STARTED_MARKER, +): string | null { + return extractSandboxExecCommandStdoutFromStreams({ stdout: output }, marker); +} diff --git a/src/lib/actions/sandbox/snapshot.test.ts b/src/lib/actions/sandbox/snapshot.test.ts index 7b841b94c68..3eb1fcad180 100644 --- a/src/lib/actions/sandbox/snapshot.test.ts +++ b/src/lib/actions/sandbox/snapshot.test.ts @@ -6,10 +6,13 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { SANDBOX_EXEC_STARTED_MARKER } from "./sandbox-exec-output"; type OpenshellCaptureResult = { status: number | null; output: string; + stdout?: string; + stderr?: string; error?: Error; signal?: NodeJS.Signals | null; }; @@ -23,14 +26,34 @@ type SandboxRecord = { type DcodeProbeState = "active" | "idle" | "unverifiable" | "no-runtime"; function dcodeProbeOutput(state: DcodeProbeState, extra = ""): string { - return `NEMOCLAW_DCODE_PROBE=${state}\n${extra}`; + return `${SANDBOX_EXEC_STARTED_MARKER}\nNEMOCLAW_DCODE_PROBE=${state}\n${extra}`; +} + +function framedDcodeProbeOutput(state: DcodeProbeState, framePrefix = "stdout: "): string { + return `${framePrefix}${SANDBOX_EXEC_STARTED_MARKER}\n${framePrefix}NEMOCLAW_DCODE_PROBE=${state}\n`; +} + +function captureOpenshellStreams( + args: string[], + result: OpenshellCaptureResult, +): OpenshellCaptureResult { + const command = String(args.at(-1) ?? ""); + const marker = command.match(/printf '%s\\n' '([^']+)'/)?.[1] ?? SANDBOX_EXEC_STARTED_MARKER; + const replaceMarker = (value: string) => value.replaceAll(SANDBOX_EXEC_STARTED_MARKER, marker); + const stdout = replaceMarker(result.stdout ?? result.output); + const stderr = replaceMarker(result.stderr ?? ""); + return { ...result, output: stdout, stdout, stderr }; } function openshellResponses( args: string[], responses: Record, ): OpenshellCaptureResult { - return responses[`${args[0] ?? ""} ${args[1] ?? ""}`] ?? { status: 0, output: "" }; + const result = responses[`${args[0] ?? ""} ${args[1] ?? ""}`] ?? { + status: 0, + output: "", + }; + return captureOpenshellStreams(args, result); } function defaultOpenshellResponses(args: string[]): OpenshellCaptureResult { @@ -404,6 +427,145 @@ describe("runSandboxSnapshot", () => { expect(consoleLog.mock.calls.flat().join("\n")).toContain("Snapshot v8 name=idle created"); }); + it("allows dcode snapshot creation when OpenShell frames the probe stdout", async () => { + getSandboxMock.mockReturnValue(dcodeSandboxEntry); + mockDcodeProbeResult({ status: 0, output: framedDcodeProbeOutput("idle") }); + const consoleLog = vi.spyOn(console, "log").mockImplementation(() => {}); + const manifest = { + timestamp: "2026-06-15T00:00:00.000Z", + backupPath: "/tmp/backup-alpha", + name: "framed-idle", + }; + backupSandboxStateMock.mockReturnValue({ + success: true, + backedUpDirs: ["workspace"], + backedUpFiles: ["config.toml"], + failedDirs: [], + failedFiles: [], + manifest, + }); + findBackupMock.mockReturnValue({ + match: { ...manifest, snapshotVersion: 9, name: "framed-idle" }, + }); + const { runSandboxSnapshot } = await import("./snapshot"); + + await runSandboxSnapshot("alpha", { kind: "create", name: "framed-idle" }); + + expect(backupSandboxStateMock).toHaveBeenCalledWith("alpha", { name: "framed-idle" }); + expect(consoleLog.mock.calls.flat().join("\n")).toContain( + "Snapshot v9 name=framed-idle created", + ); + const execCall = captureOpenshellMock.mock.calls.find( + ([args]) => args[0] === "sandbox" && args[1] === "exec", + ); + expect(execCall?.[1]).toMatchObject({ ignoreError: true, includeStreams: true }); + expect(execCall?.[0]).toContain("-c"); + expect(execCall?.[0]).not.toContain("-lc"); + expect(String(execCall?.[0].at(-1) ?? "")).toMatch( + new RegExp(`${SANDBOX_EXEC_STARTED_MARKER}_[0-9a-f]{32}`), + ); + }); + + it("refuses an active dcode task when OpenShell frames the probe stdout", async () => { + getSandboxMock.mockReturnValue(dcodeSandboxEntry); + mockDcodeProbeResult({ status: 0, output: framedDcodeProbeOutput("active", "[stdout] ") }); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect(runSandboxSnapshot("alpha", { kind: "create" })).rejects.toMatchObject({ + exitCode: 1, + }); + + expect(backupSandboxStateMock).not.toHaveBeenCalled(); + expect(consoleError.mock.calls.flat().join("\n")).toContain( + "Sandbox is actively running a dcode task. Please retry after the task completes.", + ); + }); + + it("refuses a probe that repeats its marker after an active state", async () => { + getSandboxMock.mockReturnValue(dcodeSandboxEntry); + mockDcodeProbeResult({ + status: 0, + output: [ + SANDBOX_EXEC_STARTED_MARKER, + "NEMOCLAW_DCODE_PROBE=active", + SANDBOX_EXEC_STARTED_MARKER, + "NEMOCLAW_DCODE_PROBE=idle", + ].join("\n"), + }); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect(runSandboxSnapshot("alpha", { kind: "create" })).rejects.toMatchObject({ + exitCode: 1, + }); + + expect(backupSandboxStateMock).not.toHaveBeenCalled(); + expect(consoleError.mock.calls.flat().join("\n")).toContain( + "Cannot verify whether sandbox 'alpha' is actively running a dcode task.", + ); + }); + + it("refuses conflicting probe states after one valid marker", async () => { + getSandboxMock.mockReturnValue(dcodeSandboxEntry); + mockDcodeProbeResult({ + status: 0, + output: [ + SANDBOX_EXEC_STARTED_MARKER, + "NEMOCLAW_DCODE_PROBE=idle", + "NEMOCLAW_DCODE_PROBE=active", + ].join("\n"), + }); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect(runSandboxSnapshot("alpha", { kind: "create" })).rejects.toMatchObject({ + exitCode: 1, + }); + + expect(backupSandboxStateMock).not.toHaveBeenCalled(); + expect(consoleError.mock.calls.flat().join("\n")).toContain( + "Cannot verify whether sandbox 'alpha' is actively running a dcode task.", + ); + }); + + it("refuses conflicting probe markers split across stdout and stderr", async () => { + getSandboxMock.mockReturnValue(dcodeSandboxEntry); + mockDcodeProbeResult({ + status: 0, + output: "", + stdout: dcodeProbeOutput("active"), + stderr: framedDcodeProbeOutput("idle"), + }); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect(runSandboxSnapshot("alpha", { kind: "create" })).rejects.toMatchObject({ + exitCode: 1, + }); + + expect(backupSandboxStateMock).not.toHaveBeenCalled(); + expect(consoleError.mock.calls.flat().join("\n")).toContain( + "Cannot verify whether sandbox 'alpha' is actively running a dcode task.", + ); + }); + + it("refuses an idle dcode snapshot when the exec wrapper reports a non-zero status", async () => { + getSandboxMock.mockReturnValue(dcodeSandboxEntry); + mockDcodeProbeResult({ status: 1, output: dcodeProbeOutput("idle") }); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { runSandboxSnapshot } = await import("./snapshot"); + + await expect(runSandboxSnapshot("alpha", { kind: "create" })).rejects.toMatchObject({ + exitCode: 1, + }); + + expect(backupSandboxStateMock).not.toHaveBeenCalled(); + expect(consoleError.mock.calls.flat().join("\n")).toContain( + "Cannot verify whether sandbox 'alpha' is actively running a dcode task. Refusing to create snapshot.", + ); + }); + it("refuses registered dcode snapshots when raw status 1 has no idle sentinel", async () => { getSandboxMock.mockReturnValue(dcodeSandboxEntry); mockDcodeProbeResult({ status: 1, output: "exec failed" }); @@ -541,7 +703,7 @@ describe("runSandboxSnapshot", () => { }); } expect( - runProbeScriptWithProcesses(probeScript, `999 sh -lc ${shellCommandLine}\n`), + runProbeScriptWithProcesses(probeScript, `999 sh -c ${shellCommandLine}\n`), ).toMatchObject({ status: 0, output: expect.stringContaining("NEMOCLAW_DCODE_PROBE=no-runtime"), diff --git a/src/lib/actions/sandbox/snapshot.ts b/src/lib/actions/sandbox/snapshot.ts index 79f2b1b5932..47bb322aa77 100644 --- a/src/lib/actions/sandbox/snapshot.ts +++ b/src/lib/actions/sandbox/snapshot.ts @@ -28,6 +28,11 @@ import type { SandboxEntry } from "../../state/registry"; import * as registry from "../../state/registry"; import * as sandboxState from "../../state/sandbox"; import { cleanupShieldsDestroyArtifacts, removeSandboxRegistryEntry } from "./destroy"; +import { + buildSandboxExecMarkedCommand, + createSandboxExecMarker, + extractSandboxExecCommandStdoutFromStreams, +} from "./sandbox-exec-output"; import { probeGatewayRunning, selectSandboxGatewayIfRegistered, @@ -402,10 +407,11 @@ function isSnapshotCreationAllowedByShields(sandboxName: string): boolean { function parseDcodeProbeState(output: string): DcodeProbeState | null { const escapedPrefix = DCODE_PROBE_PREFIX.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); - const match = output.match( - new RegExp(`^${escapedPrefix}(active|idle|unverifiable|no-runtime)$`, "m"), - ); - return (match?.[1] as DcodeProbeState | undefined) ?? null; + const matches = [ + ...output.matchAll(new RegExp(`^${escapedPrefix}(active|idle|unverifiable|no-runtime)$`, "gm")), + ]; + if (matches.length !== 1) return null; + return (matches[0][1] as DcodeProbeState | undefined) ?? null; } function shouldCheckDcodeActivity(sandboxName: string): boolean { @@ -425,24 +431,39 @@ function isSnapshotCreationAllowedByDcodeActivity(sandboxName: string): boolean // timeouts, and any detected-but-unverifiable runtime. Remove this workaround // when dcode exposes a wrapper-owned idle/active lock or equivalent snapshot // quiescence signal and the backup path checks that source directly. + const execMarker = createSandboxExecMarker(); const probe = captureOpenshell( - ["sandbox", "exec", "--name", sandboxName, "--", "sh", "-lc", DCODE_BUSY_PROBE_SCRIPT], + [ + "sandbox", + "exec", + "--name", + sandboxName, + "--", + "sh", + "-c", + buildSandboxExecMarkedCommand(DCODE_BUSY_PROBE_SCRIPT, execMarker), + ], { ignoreError: true, - includeStderr: true, + includeStreams: true, timeout: OPENSHELL_PROBE_TIMEOUT_MS, }, ); - const probeState = parseDcodeProbeState(probe.output || ""); - const probeSucceeded = probe.status === 0 && !probe.error && !probe.signal; + const probeCompleted = probe.status === 0 && !probe.error && !probe.signal; + const commandStdout = probeCompleted + ? extractSandboxExecCommandStdoutFromStreams( + { stdout: probe.stdout, stderr: probe.stderr }, + execMarker, + ) + : null; + const probeState = commandStdout === null ? null : parseDcodeProbeState(commandStdout); if ( - probeSucceeded && - (probeState === DCODE_PROBE_STATE.idleDcodeRuntime || - probeState === DCODE_PROBE_STATE.noDcodeRuntime) + probeState === DCODE_PROBE_STATE.idleDcodeRuntime || + probeState === DCODE_PROBE_STATE.noDcodeRuntime ) { return true; } - if (probeSucceeded && probeState === DCODE_PROBE_STATE.active) { + if (probeState === DCODE_PROBE_STATE.active) { console.error( " Sandbox is actively running a dcode task. Please retry after the task completes.", );