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
15 changes: 12 additions & 3 deletions bin/lib/onboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -1386,11 +1386,15 @@ async function createSandbox(gpu, model, provider, preferredInferenceApi = null)
console.log(` Creating sandbox '${sandboxName}' (this takes a few minutes on first run)...`);
const chatUiUrl = process.env.CHAT_UI_URL || "http://127.0.0.1:18789";
patchStagedDockerfile(stagedDockerfile, model, chatUiUrl, String(Date.now()), provider, preferredInferenceApi);
// Only pass non-sensitive env vars to the sandbox. NVIDIA_API_KEY is NOT
// needed inside the sandbox — inference is proxied through the OpenShell
// gateway which injects the stored credential server-side. The gateway
// also strips any Authorization headers sent by the sandbox client.
// See: crates/openshell-sandbox/src/proxy.rs (header stripping),
// crates/openshell-router/src/backend.rs (server-side auth injection).
const envArgs = [formatEnvAssignment("CHAT_UI_URL", chatUiUrl)];
const sandboxEnv = { ...process.env };
if (process.env.NVIDIA_API_KEY) {
sandboxEnv.NVIDIA_API_KEY = process.env.NVIDIA_API_KEY;
}
delete sandboxEnv.NVIDIA_API_KEY;
const discordToken = getCredential("DISCORD_BOT_TOKEN") || process.env.DISCORD_BOT_TOKEN;
if (discordToken) {
sandboxEnv.DISCORD_BOT_TOKEN = discordToken;
Expand Down Expand Up @@ -2142,6 +2146,11 @@ async function onboard(opts = {}) {
process.env.NEMOCLAW_OPENSHELL_BIN = getOpenshellBinary();
await startGateway(gpu);
await setupInference(GATEWAY_NAME, model, provider, endpointUrl, credentialEnv);
// The key is now stored in openshell's provider config. Clear it from our
// process environment so new child processes don't inherit it. Note: this
// does NOT clear /proc/pid/environ (kernel snapshot is immutable after exec),
// but it prevents run()'s { ...process.env } from propagating the key.
delete process.env.NVIDIA_API_KEY;
const sandboxName = await createSandbox(gpu, model, provider, preferredInferenceApi);
if (nimContainer) {
registry.updateSandbox(sandboxName, { nimContainer });
Expand Down
4 changes: 2 additions & 2 deletions bin/nemoclaw.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ async function setup() {
}

async function setupSpark() {
await ensureApiKey();
run(`sudo -E NVIDIA_API_KEY=${shellQuote(process.env.NVIDIA_API_KEY)} bash "${SCRIPTS}/setup-spark.sh"`);
// setup-spark.sh configures Docker cgroups — it does not use NVIDIA_API_KEY.
run(`sudo bash "${SCRIPTS}/setup-spark.sh"`);
}

async function deploy(instanceName) {
Expand Down
8 changes: 6 additions & 2 deletions scripts/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -133,10 +133,12 @@ fi
info "Setting up inference providers..."

# nvidia-nim (build.nvidia.com)
# Use env-name-only form so openshell reads the value from the environment
# internally — the literal key value never appears in the process argument list.
upsert_provider \
"nvidia-nim" \
"openai" \
"NVIDIA_API_KEY=$NVIDIA_API_KEY" \
"NVIDIA_API_KEY" \
"OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1"

# vllm-local (if vLLM is installed or running)
Expand Down Expand Up @@ -193,9 +195,11 @@ rm -rf "$BUILD_CTX/nemoclaw/node_modules"
# detect failures. The raw log is kept on failure for debugging.
CREATE_LOG=$(mktemp /tmp/nemoclaw-create-XXXXXX.log)
set +e
# NVIDIA_API_KEY is NOT passed into the sandbox. Inference is proxied through
# the OpenShell gateway which injects the stored credential server-side.
openshell sandbox create --from "$BUILD_CTX/Dockerfile" --name "$SANDBOX_NAME" \
--provider nvidia-nim \
-- env NVIDIA_API_KEY="$NVIDIA_API_KEY" >"$CREATE_LOG" 2>&1
>"$CREATE_LOG" 2>&1
Comment on lines +198 to +202
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

nl -ba scripts/setup.sh | sed -n '132,210p'
rg -n 'unset NVIDIA_API_KEY|env -u NVIDIA_API_KEY|openshell sandbox create' scripts/setup.sh

Repository: NVIDIA/NemoClaw

Length of output: 101


🏁 Script executed:

#!/bin/bash
# Re-check without nl command
cat -n scripts/setup.sh | sed -n '132,210p'
echo "---"
rg -n 'unset NVIDIA_API_KEY|env -u NVIDIA_API_KEY|openshell sandbox create' scripts/setup.sh

Repository: NVIDIA/NemoClaw

Length of output: 3759


Unset or scrub NVIDIA_API_KEY before sandbox creation to prevent child-process environment inheritance.

Although the command no longer explicitly passes -- env, the variable remains in the shell's exported environment and will be inherited by the sandbox process. Add either unset NVIDIA_API_KEY before the sandbox creation call, or use env -u NVIDIA_API_KEY as a prefix to the command.

Suggested patch
 upsert_provider \
   "nvidia-nim" \
   "openai" \
   "NVIDIA_API_KEY" \
   "OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1"
+
+# Credential is now stored server-side; prevent further child-process inheritance.
+unset NVIDIA_API_KEY
@@
-openshell sandbox create --from "$BUILD_CTX/Dockerfile" --name "$SANDBOX_NAME" \
+env -u NVIDIA_API_KEY openshell sandbox create --from "$BUILD_CTX/Dockerfile" --name "$SANDBOX_NAME" \
   --provider nvidia-nim \
   >"$CREATE_LOG" 2>&1
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup.sh` around lines 198 - 202, The openshell sandbox creation call
(openshell sandbox create ... >"$CREATE_LOG" 2>&1) can inherit the exported
NVIDIA_API_KEY from the parent shell; before invoking openshell (or as a
prefix), remove that env var to avoid leaking credentials — either run unset
NVIDIA_API_KEY in the script just before the openshell sandbox create invocation
or invoke the command with env -u NVIDIA_API_KEY to run it with that variable
removed; update the section around the SANDBOX_NAME/CREATE_LOG invocation
accordingly.

CREATE_RC=$?
set -e
rm -rf "$BUILD_CTX"
Expand Down
8 changes: 4 additions & 4 deletions scripts/walkthrough.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,7 @@ if ! command -v tmux >/dev/null 2>&1; then
echo ""
echo " Terminal 2 (Agent):"
echo " openshell sandbox connect nemoclaw"
echo ' export NVIDIA_API_KEY=<your-key>'
echo " nemoclaw-start"
echo " openclaw agent --agent main --local --session-id live"
echo " nemoclaw-start openclaw agent --agent main --local --session-id live"
exit 0
fi

Expand All @@ -87,8 +85,10 @@ tmux kill-session -t "$SESSION" 2>/dev/null || true
tmux new-session -d -s "$SESSION" -x 200 -y 50 "openshell term"

# Split right pane for the agent
# NVIDIA_API_KEY is not needed inside the sandbox — inference is proxied
# through the OpenShell gateway which injects credentials server-side.
tmux split-window -h -t "$SESSION" \
"openshell sandbox connect nemoclaw -- bash -c 'export NVIDIA_API_KEY=$NVIDIA_API_KEY && nemoclaw-start openclaw agent --agent main --local --session-id live'"
"openshell sandbox connect nemoclaw -- bash -c 'nemoclaw-start openclaw agent --agent main --local --session-id live'"

# Even split
tmux select-layout -t "$SESSION" even-horizontal
Expand Down
68 changes: 68 additions & 0 deletions test/runner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,72 @@ describe("regression guards", () => {
expect(src.includes("validateName(SANDBOX")).toBeTruthy();
expect(src.includes("execSync")).toBeFalsy();
});

describe("credential exposure guards (#429)", () => {
it("onboard createSandbox does not pass NVIDIA_API_KEY to sandbox env", () => {
const fs = require("fs");
const src = fs.readFileSync(path.join(__dirname, "..", "bin", "lib", "onboard.js"), "utf-8");
// Find the envArgs block in createSandbox — it should not contain NVIDIA_API_KEY
const envArgsMatch = src.match(/const envArgs = \[[\s\S]*?\];/);
expect(envArgsMatch).toBeTruthy();
expect(envArgsMatch[0].includes("NVIDIA_API_KEY")).toBe(false);
});

it("onboard clears NVIDIA_API_KEY from process.env after setupInference", () => {
const fs = require("fs");
const src = fs.readFileSync(path.join(__dirname, "..", "bin", "lib", "onboard.js"), "utf-8");
expect(src.includes("delete process.env.NVIDIA_API_KEY")).toBeTruthy();
});

it("setup.sh uses env-name-only form for nvidia-nim credential", () => {
const fs = require("fs");
const src = fs.readFileSync(path.join(__dirname, "..", "scripts", "setup.sh"), "utf-8");
// Should use "NVIDIA_API_KEY" (name only), not "NVIDIA_API_KEY=$NVIDIA_API_KEY" (value)
const lines = src.split("\n");
for (const line of lines) {
if (line.includes("upsert_provider") || line.includes("--credential")) continue;
if (line.trim().startsWith("#")) continue;
// Check credential argument lines passed to upsert_provider
if (line.includes('"NVIDIA_API_KEY=')) {
// Allow "NVIDIA_API_KEY" alone but not "NVIDIA_API_KEY=$..."
expect(line.includes("NVIDIA_API_KEY=$")).toBe(false);
}
}
});

it("setup.sh does not pass NVIDIA_API_KEY in sandbox create env args", () => {
const fs = require("fs");
const src = fs.readFileSync(path.join(__dirname, "..", "scripts", "setup.sh"), "utf-8");
// Find sandbox create command — should not have env NVIDIA_API_KEY
const createLines = src.split("\n").filter((l) => l.includes("sandbox create"));
for (const line of createLines) {
expect(line.includes("NVIDIA_API_KEY")).toBe(false);
}
});

it("setupSpark does not pass NVIDIA_API_KEY to sudo", () => {
const fs = require("fs");
const src = fs.readFileSync(path.join(__dirname, "..", "bin", "nemoclaw.js"), "utf-8");
// Find the run() call inside setupSpark — it should not contain the key
const sparkLines = src.split("\n").filter(
(l) => l.includes("setup-spark") && l.includes("run(")
);
for (const line of sparkLines) {
expect(line.includes("NVIDIA_API_KEY")).toBe(false);
}
});

it("walkthrough.sh does not embed NVIDIA_API_KEY in tmux or sandbox commands", () => {
const fs = require("fs");
const src = fs.readFileSync(path.join(__dirname, "..", "scripts", "walkthrough.sh"), "utf-8");
// Check only executable lines (tmux spawn, openshell connect) — not comments/docs
const cmdLines = src.split("\n").filter(
(l) => !l.trim().startsWith("#") && !l.trim().startsWith("echo") &&
(l.includes("tmux") || l.includes("openshell sandbox connect"))
);
for (const line of cmdLines) {
expect(line.includes("NVIDIA_API_KEY")).toBe(false);
}
});
Comment on lines +282 to +293
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Credential regression guard is too narrow for walkthrough script.

This check only inspects tmux/connect command lines, so a direct env dependency/exposure path can slip through undetected.

🧪 Proposed test hardening
     it("walkthrough.sh does not embed NVIDIA_API_KEY in tmux or sandbox commands", () => {
       const fs = require("fs");
       const src = fs.readFileSync(path.join(__dirname, "..", "scripts", "walkthrough.sh"), "utf-8");
+      // Guard against runtime env preconditions that force secret presence.
+      expect(src.includes('[ -n "${NVIDIA_API_KEY:-}" ]')).toBe(false);
       // Check only executable lines (tmux spawn, openshell connect) — not comments/docs
       const cmdLines = src.split("\n").filter(
         (l) => !l.trim().startsWith("#") && !l.trim().startsWith("echo") &&
                (l.includes("tmux") || l.includes("openshell sandbox connect"))
       );
       for (const line of cmdLines) {
         expect(line.includes("NVIDIA_API_KEY")).toBe(false);
       }
     });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/runner.test.js` around lines 263 - 274, The current test "walkthrough.sh
does not embed NVIDIA_API_KEY in tmux or sandbox commands" only scans
tmux/openshell lines (cmdLines) so other exposures can slip through; update the
test to scan the full script content (src) — or at least all
non-comment/non-echo executable lines — for the string "NVIDIA_API_KEY" and
assert no matches, replacing the filtered cmdLines logic with a broader filter
over src.split("\n") (reusing variables like src, fs, path) so any direct env
usage anywhere in walkthrough.sh fails the test.

});
});
Loading