[codex] fix(security): harden sandbox command execution

bin/lib/onboard.js

@@ -18,7 +18,16 @@ function envInt(name, fallback) {
   const n = Number(raw);
   return Number.isFinite(n) ? Math.max(0, Math.round(n)) : fallback;
 }
-const { ROOT, SCRIPTS, redact, run, runCapture, shellQuote } = require("./runner");
+const {
+  ROOT,
+  SCRIPTS,
+  redact,
+  run,
+  runCapture,
+  runFile,
+  shellQuote,
+  validateName,
+} = require("./runner");
 const {
   getDefaultOllamaModel,
   getBootstrapOllamaModelOptions,
@@ -1044,12 +1053,18 @@ async function promptBraveSearchApiKey() {
   }
 }
 
-async function ensureValidatedBraveSearchCredential() {
-  let apiKey = getCredential(webSearch.BRAVE_API_KEY_ENV);
-  let usingSavedKey = Boolean(apiKey);
+async function ensureValidatedBraveSearchCredential(nonInteractive = isNonInteractive()) {
+  const savedApiKey = getCredential(webSearch.BRAVE_API_KEY_ENV);
+  let apiKey = savedApiKey || normalizeCredentialValue(process.env[webSearch.BRAVE_API_KEY_ENV]);
+  let usingSavedKey = Boolean(savedApiKey);
 
   while (true) {
     if (!apiKey) {
+      if (nonInteractive) {
+        throw new Error(
+          "Brave Search requires BRAVE_API_KEY or a saved Brave Search credential in non-interactive mode.",
+        );
+      }
       apiKey = await promptBraveSearchApiKey();
       usingSavedKey = false;
     }
@@ -1069,6 +1084,13 @@ async function ensureValidatedBraveSearchCredential() {
       console.error(`  ${validation.message}`);
     }
 
+    if (nonInteractive) {
+      throw new Error(
+        validation.message ||
+          "Brave Search API key validation failed in non-interactive mode.",
+      );
+    }
+
     const action = await promptBraveSearchRecovery(validation);
     if (action === "skip") {
       console.log("  Skipping Brave Web Search setup.");
@@ -2389,15 +2411,14 @@ async function promptValidatedSandboxName() {
       "NEMOCLAW_SANDBOX_NAME",
       "my-assistant",
     );
-    const sandboxName = (nameAnswer || "my-assistant").trim().toLowerCase();
+    const sandboxName = (nameAnswer || "my-assistant").trim();
 
-    // Validate: RFC 1123 subdomain — lowercase alphanumeric and hyphens,
-    // must start and end with alphanumeric (required by Kubernetes/OpenShell)
-    if (/^[a-z0-9]([a-z0-9-]*[a-z0-9])?$/.test(sandboxName)) {
-      return sandboxName;
+    try {
+      return validateName(sandboxName, "sandbox name");
+    } catch (error) {
+      console.error(`  ${error.message}`);
     }
 
-    console.error(`  Invalid sandbox name: '${sandboxName}'`);
     console.error("  Names must be lowercase, contain only letters, numbers, and hyphens,");
     console.error("  and must start and end with a letter or number.");
 
@@ -2424,7 +2445,10 @@ async function createSandbox(
 ) {
   step(5, 7, "Creating sandbox");
 
-  const sandboxName = sandboxNameOverride || (await promptValidatedSandboxName());
+  const sandboxName = validateName(
+    sandboxNameOverride ?? (await promptValidatedSandboxName()),
+    "sandbox name",
+  );
   const chatUiUrl = process.env.CHAT_UI_URL || `http://127.0.0.1:${CONTROL_UI_PORT}`;
 
   // Reconcile local registry state with the live OpenShell gateway state.
@@ -2583,11 +2607,11 @@ async function createSandbox(
   // or seeing 502/503 errors during initial load.
   console.log("  Waiting for NemoClaw dashboard to become ready...");
   for (let i = 0; i < 15; i++) {
-    const readyMatch = runCapture(
-      `openshell sandbox exec ${sandboxName} curl -sf http://localhost:18789/ 2>/dev/null || echo "no"`,
+    const readyMatch = runCaptureOpenshell(
+      ["sandbox", "exec", sandboxName, "curl", "-sf", `http://localhost:${CONTROL_UI_PORT}/`],
       { ignoreError: true },
     );
-    if (readyMatch && !readyMatch.includes("no")) {
+    if (readyMatch) {
       console.log("  ✓ Dashboard is live");
       break;
     }
@@ -2612,10 +2636,9 @@ async function createSandbox(
   // DNS proxy — run a forwarder in the sandbox pod so the isolated
   // sandbox namespace can resolve hostnames (fixes #626).
   console.log("  Setting up sandbox DNS proxy...");
-  run(
-    `bash "${path.join(SCRIPTS, "setup-dns-proxy.sh")}" ${GATEWAY_NAME} "${sandboxName}" 2>&1 || true`,
-    { ignoreError: true },
-  );
+  runFile("bash", [path.join(SCRIPTS, "setup-dns-proxy.sh"), GATEWAY_NAME, sandboxName], {
+    ignoreError: true,
+  });
 
   console.log(`  ✓ Sandbox '${sandboxName}' created`);
   return sandboxName;
@@ -4036,35 +4059,13 @@ async function onboard(opts = {}) {
       break;
     }
 
-    if (webSearchConfig) {
-      note("  [resume] Revalidating Brave Search configuration.");
-      const braveApiKey = await ensureValidatedBraveSearchCredential();
-      if (braveApiKey) {
-        webSearchConfig = { fetchEnabled: true };
-        onboardSession.updateSession((current) => {
-          current.webSearchConfig = webSearchConfig;
-          return current;
-        });
-        note("  [resume] Reusing Brave Search configuration.");
-      } else {
-        webSearchConfig = await configureWebSearch(null);
-        onboardSession.updateSession((current) => {
-          current.webSearchConfig = webSearchConfig;
-          return current;
-        });
-      }
-    } else {
-      webSearchConfig = await configureWebSearch(webSearchConfig);
-      onboardSession.updateSession((current) => {
-        current.webSearchConfig = webSearchConfig;
-        return current;
-      });
-    }
-
     const sandboxReuseState = getSandboxReuseState(sandboxName);
     const resumeSandbox =
       resume && session?.steps?.sandbox?.status === "complete" && sandboxReuseState === "ready";
     if (resumeSandbox) {
+      if (webSearchConfig) {
+        note("  [resume] Reusing Brave Search configuration already baked into the sandbox.");
+      }
       skippedStepMessage("sandbox", sandboxName);
     } else {
       if (resume && session?.steps?.sandbox?.status === "complete") {
@@ -4080,16 +4081,34 @@ async function onboard(opts = {}) {
           }
         }
       }
+      let nextWebSearchConfig = webSearchConfig;
+      if (nextWebSearchConfig) {
+        note("  [resume] Revalidating Brave Search configuration for sandbox recreation.");
+        const braveApiKey = await ensureValidatedBraveSearchCredential();
+        nextWebSearchConfig = braveApiKey ? { fetchEnabled: true } : null;
+        if (nextWebSearchConfig) {
+          note("  [resume] Reusing Brave Search configuration.");
+        }
+      } else {
+        nextWebSearchConfig = await configureWebSearch(null);
+      }
       startRecordedStep("sandbox", { sandboxName, provider, model });
       sandboxName = await createSandbox(
         gpu,
         model,
         provider,
         preferredInferenceApi,
         sandboxName,
-        webSearchConfig,
+        nextWebSearchConfig,
       );
-      onboardSession.markStepComplete("sandbox", { sandboxName, provider, model, nimContainer });
+      webSearchConfig = nextWebSearchConfig;
+      onboardSession.markStepComplete("sandbox", {
+        sandboxName,
+        provider,
+        model,
+        nimContainer,
+        webSearchConfig,
+      });
     }
 
     const resumeOpenclaw = resume && sandboxName && isOpenclawReady(sandboxName);
@@ -4158,7 +4177,9 @@ module.exports = {
   buildSandboxConfigSyncScript,
   copyBuildContextDir,
   classifySandboxCreateFailure,
+  configureWebSearch,
   createSandbox,
+  ensureValidatedBraveSearchCredential,
   getFutureShellPathHint,
   getGatewayStartEnv,
   getGatewayReuseState,

bin/lib/runner.js

@@ -17,9 +17,8 @@ if (dockerHost) {
  * Run a shell command via bash, streaming stdout/stderr (redacted) to the terminal.
  * Exits the process on failure unless opts.ignoreError is true.
  */
-function run(cmd, opts = {}) {
-  const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"];
-  const result = spawnSync("bash", ["-c", cmd], {
+function spawnAndHandle(file, args, opts = {}, stdio, renderedCommand) {
+  const result = spawnSync(file, args, {
     ...opts,
     stdio,
     cwd: ROOT,
@@ -29,32 +28,40 @@ function run(cmd, opts = {}) {
     writeRedactedResult(result, stdio);
   }
   if (result.status !== 0 && !opts.ignoreError) {
-    console.error(`  Command failed (exit ${result.status}): ${redact(cmd).slice(0, 80)}`);
+    console.error(
+      `  Command failed (exit ${result.status}): ${redact(renderedCommand).slice(0, 80)}`,
+    );
     process.exit(result.status || 1);
   }
   return result;
 }
 
+function run(cmd, opts = {}) {
+  const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"];
+  return spawnAndHandle("bash", ["-c", cmd], opts, stdio, cmd);
+}
+
 /**
  * Run a shell command interactively (stdin inherited) while capturing and redacting stdout/stderr.
  * Exits the process on failure unless opts.ignoreError is true.
  */
 function runInteractive(cmd, opts = {}) {
   const stdio = opts.stdio ?? ["inherit", "pipe", "pipe"];
-  const result = spawnSync("bash", ["-c", cmd], {
-    ...opts,
-    stdio,
-    cwd: ROOT,
-    env: { ...process.env, ...opts.env },
-  });
-  if (!opts.suppressOutput) {
-    writeRedactedResult(result, stdio);
-  }
-  if (result.status !== 0 && !opts.ignoreError) {
-    console.error(`  Command failed (exit ${result.status}): ${redact(cmd).slice(0, 80)}`);
-    process.exit(result.status || 1);
+  return spawnAndHandle("bash", ["-c", cmd], opts, stdio, cmd);
+}
+
+/**
+ * Run a program directly with argv-style arguments, bypassing shell parsing.
+ * Exits the process on failure unless opts.ignoreError is true.
+ */
+function runFile(file, args = [], opts = {}) {
+  if (opts.shell) {
+    throw new Error("runFile does not allow opts.shell=true");
   }
-  return result;
+  const stdio = opts.stdio ?? ["ignore", "pipe", "pipe"];
+  const normalizedArgs = args.map((arg) => String(arg));
+  const rendered = [shellQuote(file), ...normalizedArgs.map((arg) => shellQuote(arg))].join(" ");
+  return spawnAndHandle(file, normalizedArgs, { ...opts, shell: false }, stdio, rendered);
 }
 
 /**
@@ -200,6 +207,7 @@ module.exports = {
   redact,
   run,
   runCapture,
+  runFile,
   runInteractive,
   shellQuote,
   validateName,

src/lib/onboard-session.test.ts

@@ -120,6 +120,20 @@ describe("onboard session", () => {
     expect(loaded.metadata.token).toBeUndefined();
   });
 
+  it("persists and clears web search config through safe session updates", () => {
+    session.saveSession(session.createSession());
+    session.markStepComplete("provider_selection", {
+      webSearchConfig: { fetchEnabled: true },
+    });
+
+    let loaded = session.loadSession();
+    expect(loaded.webSearchConfig).toEqual({ fetchEnabled: true });
+
+    session.completeSession({ webSearchConfig: null });
+    loaded = session.loadSession();
+    expect(loaded.webSearchConfig).toBeNull();
+  });
+
   it("does not clear existing metadata when updates omit whitelisted metadata fields", () => {
     session.saveSession(session.createSession({ metadata: { gatewayName: "nemoclaw" } }));
     session.markStepComplete("provider_selection", {