diff --git a/nemoclaw/src/blueprint/runner-test-fixtures.ts b/nemoclaw/src/blueprint/runner-test-fixtures.ts new file mode 100644 index 00000000000..08bd16cadaf --- /dev/null +++ b/nemoclaw/src/blueprint/runner-test-fixtures.ts @@ -0,0 +1,84 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +export function minimalBlueprint(overrides?: Record): Record { + return { + version: "1.0", + components: { + inference: { + profiles: { + default: { + provider_type: "openai", + provider_name: "my-provider", + endpoint: "https://api.example.com/v1", + model: "gpt-4", + credential_env: "MY_API_KEY", + }, + }, + }, + sandbox: { + image: "openclaw", + name: "test-sandbox", + forward_ports: [18789], + }, + policy: { additions: {} }, + }, + ...overrides, + }; +} + +export function routedBlueprint(): Record { + return { + version: "1.0", + components: { + inference: { + profiles: { + routed: { + provider_type: "openai", + provider_name: "nvidia-router", + endpoint: "http://localhost:4000/v1", + model: "routed", + credential_env: "NVIDIA_INFERENCE_API_KEY", + credential_default: "router-local", + timeout_secs: 180, + }, + }, + }, + sandbox: { + image: "openclaw", + name: "test-sandbox", + forward_ports: [18789], + }, + router: { + enabled: true, + port: 4000, + pool_config_path: "router/pool-config.yaml", + }, + policy: { additions: {} }, + }, + }; +} + +export function blueprintWithPolicyAdditions( + additions: Record, +): Record { + const blueprint = minimalBlueprint(); + const components = blueprint.components as Record; + return { + ...blueprint, + components: { + ...components, + policy: { additions }, + }, + }; +} + +export function resultForCommandFailure( + args: readonly string[], + command: readonly [string, string], + stderr: string, +): { exitCode: number; stdout: string; stderr: string } { + return args[0] === command[0] && args[1] === command[1] + ? { exitCode: 1, stdout: "", stderr } + : { exitCode: 0, stdout: "", stderr: "" }; +} diff --git a/nemoclaw/src/blueprint/runner.test.ts b/nemoclaw/src/blueprint/runner.test.ts index 22f533eea9f..7535e113b14 100644 --- a/nemoclaw/src/blueprint/runner.test.ts +++ b/nemoclaw/src/blueprint/runner.test.ts @@ -1,9 +1,15 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import type fs from "node:fs"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import YAML from "yaml"; +import { + blueprintWithPolicyAdditions, + minimalBlueprint, + resultForCommandFailure, + routedBlueprint, +} from "./runner-test-fixtures.js"; // ── In-memory filesystem ──────────────────────────────────────── @@ -114,80 +120,10 @@ function capturedJsonOutput(): T { return JSON.parse(json) as T; } -function minimalBlueprint(overrides?: Record): Record { - return { - version: "1.0", - components: { - inference: { - profiles: { - default: { - provider_type: "openai", - provider_name: "my-provider", - endpoint: "https://api.example.com/v1", - model: "gpt-4", - credential_env: "MY_API_KEY", - }, - }, - }, - sandbox: { - image: "openclaw", - name: "test-sandbox", - forward_ports: [18789], - }, - policy: { additions: {} }, - }, - ...overrides, - }; -} - -function routedBlueprint(): Record { - return { - version: "1.0", - components: { - inference: { - profiles: { - routed: { - provider_type: "openai", - provider_name: "nvidia-router", - endpoint: "http://localhost:4000/v1", - model: "routed", - credential_env: "NVIDIA_INFERENCE_API_KEY", - credential_default: "router-local", - timeout_secs: 180, - }, - }, - }, - sandbox: { - image: "openclaw", - name: "test-sandbox", - forward_ports: [18789], - }, - router: { - enabled: true, - port: 4000, - pool_config_path: "router/pool-config.yaml", - }, - policy: { additions: {} }, - }, - }; -} - function seedBlueprintFile(bp?: Record): void { addFile("blueprint.yaml", YAML.stringify(bp ?? minimalBlueprint())); } -function blueprintWithPolicyAdditions(additions: Record): Record { - const bp = minimalBlueprint(); - const components = bp.components as Record; - return { - ...bp, - components: { - ...components, - policy: { additions }, - }, - }; -} - function mockCurrentPolicy(stdout: string): void { mockExeca.mockImplementation(async (_cmd: string, args: string[]) => { if ( @@ -638,6 +574,71 @@ describe("runner", () => { ); }); + const hasPlanJson = (): boolean => [...store.keys()].some((k) => k.endsWith("plan.json")); + + it("rejects without persisting a plan when provider create fails (#6703)", async () => { + const credential = "provider-secret-value"; + process.env.MY_API_KEY = credential; + mockExeca.mockImplementation(async (_cmd: string, args: string[]) => + resultForCommandFailure( + args, + ["provider", "create"], + `provider setup failed\nOPENAI_API_KEY=${credential}\nAuthorization: Bearer opaque-bearer`, + ), + ); + + try { + const error = await actionApply("default", minimalBlueprint()).then( + () => new Error("expected provider creation to fail"), + (cause: unknown) => cause, + ); + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toMatch( + /Failed to create inference provider 'my-provider'.*provider setup failed/i, + ); + expect((error as Error).message).toContain("OPENAI_API_KEY="); + expect((error as Error).message).toContain("Authorization: Bearer "); + expect((error as Error).message).not.toContain(credential); + expect((error as Error).message).not.toContain("opaque-bearer"); + expect(hasPlanJson()).toBe(false); + expect(stdoutText()).not.toContain("Apply complete"); + expect(stdoutText()).not.toContain("PROGRESS:70"); + expect(stdoutText()).not.toContain("PROGRESS:100"); + } finally { + delete process.env.MY_API_KEY; + } + }); + + it("reuses an already-existing provider instead of failing (#6703)", async () => { + mockExeca.mockImplementation(async (_cmd: string, args: string[]) => + resultForCommandFailure( + args, + ["provider", "create"], + "provider 'my-provider' already exists", + ), + ); + + // Matches the sandbox-create contract: already-existing is a reuse, so the + // apply proceeds and completes. + await actionApply("default", minimalBlueprint()); + expect(hasPlanJson()).toBe(true); + expect(stdoutText()).toContain("Apply complete"); + }); + + it("rejects without persisting a plan when inference set fails (#6703)", async () => { + mockExeca.mockImplementation(async (_cmd: string, args: string[]) => + resultForCommandFailure(args, ["inference", "set"], "inference route rejected"), + ); + + await expect(actionApply("default", minimalBlueprint())).rejects.toThrow( + /Failed to set inference route .*model 'gpt-4'.*inference route rejected/i, + ); + + expect(hasPlanJson()).toBe(false); + expect(stdoutText()).not.toContain("Apply complete"); + expect(stdoutText()).not.toContain("PROGRESS:100"); + }); + it("applies blueprint policy additions by merging into the base policy", async () => { const bp = minimalBlueprint({ components: { diff --git a/nemoclaw/src/blueprint/runner.ts b/nemoclaw/src/blueprint/runner.ts index a0a6d6c8516..ba7b488411b 100644 --- a/nemoclaw/src/blueprint/runner.ts +++ b/nemoclaw/src/blueprint/runner.ts @@ -24,8 +24,8 @@ import { DASHBOARD_PORT } from "../lib/ports.js"; import { buildSubprocessEnv } from "../lib/subprocess-env.js"; import { isPlainObject, type UnknownRecord } from "../shared/object-record.js"; import * as importedOpenShellPolicyBoundary from "../shared/openshell-policy-boundary.cjs"; -import { actionSnapshots } from "./snapshot-command.js"; import type { SnapshotCommandOptions } from "./snapshot-command.js"; +import { actionSnapshots } from "./snapshot-command.js"; import { safeEndpointUrlForDownstream, validateEndpointUrl } from "./ssrf.js"; // The compiled plugin exposes named CommonJS exports. Source-mode tsx maps the @@ -78,6 +78,29 @@ function isAction(value: string | undefined): value is Action { return value === "plan" || value === "apply" || value === "status" || value === "rollback"; } +// Redact credential-shaped output before bounding OpenShell stderr to a compact, +// single-line diagnostic. (#6703) +const MAX_COMMAND_ERROR_CHARS = 500; +const SENSITIVE_ERROR_ASSIGNMENT = + /(\b[A-Z][A-Z0-9_]*(?:KEY|TOKEN|SECRET|PASSWORD|CREDENTIAL)[A-Z0-9_]*\s*)[:=]\s*(?:"[^"]*"|'[^']*'|[^\s,;]+)/gi; + +function boundedCommandError(stderr: string, secretValues: readonly string[] = []): string { + let redacted = stderr; + for (const secret of [...new Set(secretValues)] + .filter(Boolean) + .sort((a, b) => b.length - a.length)) { + redacted = redacted.split(secret).join(""); + } + redacted = redacted + .replace(SENSITIVE_ERROR_ASSIGNMENT, "$1=") + .replace(/\b(Bearer)\s+\S+/gi, "$1 "); + const collapsed = redacted.replace(/\s+/g, " ").trim(); + if (collapsed.length === 0) return "no error output"; + return collapsed.length > MAX_COMMAND_ERROR_CHARS + ? `${collapsed.slice(0, MAX_COMMAND_ERROR_CHARS)}…` + : collapsed; +} + function isOptionalString(value: unknown): value is string | undefined { return value === undefined || typeof value === "string"; } @@ -749,12 +772,27 @@ export async function actionApply( providerArgs.push("--config", `OPENAI_BASE_URL=${endpoint}`); } - await execa(providerArgs[0], providerArgs.slice(1), { + const providerResult = await execa(providerArgs[0], providerArgs.slice(1), { reject: false, stdout: "pipe", stderr: "pipe", env: buildSubprocessEnv(credEnv), }); + // A required mutation: a silently-ignored failure would persist plan.json and + // report a ready sandbox that cannot perform inference. Mirror the + // sandbox-create contract above — tolerate an already-existing provider as a + // reuse (keeps re-apply idempotent) and fail on any other non-zero result. + // The credential is passed via env (never argv); redact it from stderr before + // surfacing bounded diagnostic context. (#6703) + if (providerResult.exitCode !== 0) { + if (providerResult.stderr.includes("already exists")) { + log(`Provider '${providerName}' already exists, reusing.`); + } else { + throw new Error( + `Failed to create inference provider '${providerName}': ${boundedCommandError(providerResult.stderr, [credential])}`, + ); + } + } progress(70, "Setting inference route"); const inferenceArgs = [ @@ -769,7 +807,14 @@ export async function actionApply( if (inferenceCfg.timeout_secs !== undefined) { inferenceArgs.push("--timeout", String(inferenceCfg.timeout_secs)); } - await runCmd(inferenceArgs, { reject: false }); + const inferenceResult = await runCmd(inferenceArgs, { reject: false }); + // Another required mutation: without a routed provider the sandbox cannot + // perform inference, so a non-zero result must abort the apply. (#6703) + if (inferenceResult.exitCode !== 0) { + throw new Error( + `Failed to set inference route (provider '${providerName}', model '${model}'): ${boundedCommandError(inferenceResult.stderr)}`, + ); + } if (Object.keys(policyAdditions).length > 0) { progress(78, "Applying policy additions");