Skip to content

security: hide API credentials from process argument list #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pjt222 wants to merge 1 commit into from
Closed

security: hide API credentials from process argument list #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 15 additions & 7 deletions bin/lib/onboard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const fs = require("fs");
const os = require("os");
const path = require("path");
const crypto = require("crypto");
const { spawn, spawnSync } = require("child_process");
const { ROOT, SCRIPTS, run, runCapture, shellQuote } = require("./runner");
const {
Expand Down Expand Up @@ -805,13 +806,20 @@ async function setupInference(sandboxName, model, provider) {
step(5, 7, "Setting up inference provider");

if (provider === "nvidia-nim") {
// Create nvidia-nim provider
run(
`openshell provider create --name nvidia-nim --type openai ` +
`--credential ${shellQuote("NVIDIA_API_KEY=" + process.env.NVIDIA_API_KEY)} ` +
`--config "OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1" 2>&1 || true`,
{ ignoreError: true }
);
// Create nvidia-nim provider — credential written to temp file to avoid
// exposing the API key in process arguments visible via ps/proc (#325)
const credPath = path.join(os.tmpdir(), `nemoclaw-cred-${crypto.randomBytes(8).toString("hex")}`);
fs.writeFileSync(credPath, process.env.NVIDIA_API_KEY, { mode: 0o600, flag: "wx" });
try {
Comment on lines +811 to +813
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 22, 2026

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:

#!/bin/bash
# Verifies write occurs inside try block in bin/lib/onboard.js
awk '
/const credPath =/ {cp=NR}
/fs\.writeFileSync\(credPath/ {wf=NR}
/try \{/ && !tr {tr=NR}
END {
  printf "credPath=%s write=%s try=%s\n", cp, wf, tr
  exit !(tr && wf && wf > tr)
}' bin/lib/onboard.js

Repository: NVIDIA/NemoClaw

Length of output: 89

🏁 Script executed:

sed -n '809,825p' bin/lib/onboard.js

Repository: NVIDIA/NemoClaw

Length of output: 918

Move file write inside try/finally to guarantee cleanup.

If fs.writeFileSync() throws after the file is created (e.g., due to disk space or permission issues during write), the file will be left behind because the finally block is never reached if the try block is never entered.

Suggested fix 
 const credPath = path.join(os.tmpdir(), `nemoclaw-cred-${crypto.randomBytes(8).toString("hex")}`);
-fs.writeFileSync(credPath, process.env.NVIDIA_API_KEY, { mode: 0o600, flag: "wx" });
 try {
+  fs.writeFileSync(credPath, process.env.NVIDIA_API_KEY, { mode: 0o600, flag: "wx" });
   run(
     `openshell provider create --name nvidia-nim --type openai ` +
     `--credential "NVIDIA_API_KEY=$(cat ${shellQuote(credPath)})" ` +
     `--config "OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1" 2>&1 || true`,
     { ignoreError: true }
   );
 } finally {
   try { fs.unlinkSync(credPath); } catch {}
 }

Also applies to: 820-822

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 811 - 813, The temporary credential file is
created with fs.writeFileSync(credPath, ...) before the try/finally, so if an
error occurs during or after the write the cleanup in the finally block may not
run; move the fs.writeFileSync call into the try block that wraps the operation
using credPath (the try/finally surrounding the code that deletes the file) so
that the finally always executes and removes credPath, ensuring the temporary
file is always cleaned up; update the other similar spots around the code that
use credPath (the second occurrence noted at lines ~820-822) to follow the same
pattern.

run(
`openshell provider create --name nvidia-nim --type openai ` +
`--credential "NVIDIA_API_KEY=$(cat ${shellQuote(credPath)})" ` +
`--config "OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1" 2>&1 || true`,
Comment on lines +816 to +817
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Credential still reaches openshell as expanded argv content.

Using $(cat <file>) avoids inline interpolation in source strings, but the resulting credential is still passed as a command-line argument to the child process.

What is the recommended secure way in OpenShell to create/update providers without placing credential values in process arguments?
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/lib/onboard.js` around lines 816 - 817, The command currently injects
credentials into the child process argv via the string `--credential
"NVIDIA_API_KEY=$(cat ${shellQuote(credPath)})"` in bin/lib/onboard.js; instead,
read the credential file contents in Node, place the secret into the child
process environment (e.g., set env.NVIDIA_API_KEY = fs.readFileSync(credPath,
'utf8')) when spawning OpenShell, and remove the sensitive `--credential` argv
so the secret is not visible in process arguments; refer to the existing
shellQuote usage and the command template and adjust the spawn/exec options to
pass the env rather than interpolating the credential into the command string.

{ ignoreError: true }
);
} finally {
try { fs.unlinkSync(credPath); } catch {}
}
run(
`openshell inference set --no-verify --provider nvidia-nim --model ${shellQuote(model)} 2>/dev/null || true`,
{ ignoreError: true }
Expand Down
4 changes: 3 additions & 1 deletion bin/nemoclaw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ 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"`);
// Pass API key via environment, not command string, to avoid exposing it
// in process arguments visible via ps/proc (#325)
run(`sudo --preserve-env=NVIDIA_API_KEY bash "${SCRIPTS}/setup-spark.sh"`);
}

async function deploy(instanceName) {
Expand Down
9 changes: 8 additions & 1 deletion scripts/setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,18 @@ fi
# 3. Providers
info "Setting up inference providers..."

# Write API key to temp file to avoid exposing it in process arguments
# visible via ps/proc (#325). The file is cleaned up on exit.
_CRED_FILE=$(mktemp /tmp/nemoclaw-cred-XXXXXX)
chmod 600 "$_CRED_FILE"
printf '%s' "$NVIDIA_API_KEY" > "$_CRED_FILE"
trap 'rm -f "$_CRED_FILE"' EXIT
Comment on lines +134 to +137
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 22, 2026

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:

#!/bin/bash
# Verifies trap is declared before credential write in scripts/setup.sh
trap_line=$(awk '/trap .*_CRED_FILE/ {print NR; exit}' scripts/setup.sh)
write_line=$(awk '/printf .*NVIDIA_API_KEY/ {print NR; exit}' scripts/setup.sh)
echo "trap_line=$trap_line write_line=$write_line"
test -n "$trap_line" -a -n "$write_line" -a "$trap_line" -lt "$write_line"

Repository: NVIDIA/NemoClaw

Length of output: 87

🏁 Script executed:

cat -n scripts/setup.sh | sed -n '130,145p'

Repository: NVIDIA/NemoClaw

Length of output: 674

🏁 Script executed:

head -20 scripts/setup.sh | grep -E "^[[:space:]]*(set|#)" | head -5

Repository: NVIDIA/NemoClaw

Length of output: 279

🏁 Script executed:

head -30 scripts/setup.sh

Repository: NVIDIA/NemoClaw

Length of output: 1047

Install the EXIT trap before any credential write.

With set -e enabled, a failure during mktemp, chmod, or printf exits the script immediately before the trap is registered, leaving the credential file on disk undeleted.

Suggested fix 
_CRED_FILE=$(mktemp /tmp/nemoclaw-cred-XXXXXX)
+trap 'rm -f "${_CRED_FILE:-}"' EXIT
chmod 600 "$_CRED_FILE"
printf '%s' "$NVIDIA_API_KEY" > "$_CRED_FILE"
-trap 'rm -f "$_CRED_FILE"' EXIT
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_CRED_FILE=$(mktemp /tmp/nemoclaw-cred-XXXXXX)
chmod 600 "$_CRED_FILE"
printf '%s' "$NVIDIA_API_KEY" > "$_CRED_FILE"
trap 'rm -f "$_CRED_FILE"' EXIT
_CRED_FILE=$(mktemp /tmp/nemoclaw-cred-XXXXXX)
trap 'rm -f "${_CRED_FILE:-}"' EXIT
chmod 600 "$_CRED_FILE"
printf '%s' "$NVIDIA_API_KEY" > "$_CRED_FILE"
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup.sh` around lines 134 - 137, Move the trap registration to
immediately after creating the temporary filename and before any operations that
can fail: register trap 'rm -f "$_CRED_FILE"' EXIT right after assigning
_CRED_FILE (the mktemp step) so the cleanup is guaranteed even if chmod, printf,
or later commands fail under set -e; ensure _CRED_FILE is referenced in the trap
and only write credentials (printf) after the trap is in place.


# nvidia-nim (build.nvidia.com)
upsert_provider \
"nvidia-nim" \
"openai" \
"NVIDIA_API_KEY=$NVIDIA_API_KEY" \
"NVIDIA_API_KEY=$(cat "$_CRED_FILE")" \
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 22, 2026

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

🌐 Web query:

Does openshell provider createsupport reading credentials from environment variables or a credential file (instead of--credential KEY=VALUE argv), and if so what exact flags/options are available?

💡 Result:

I couldn’t find any authoritative documentation or source code for a CLI command named openshell provider create (with a --credential KEY=VALUE option) in public web search results, so I can’t verify whether it supports environment-variable credentials or a credential file, nor list the exact flags/options for doing so. The “OpenShell” results that are indexed appear to refer to unrelated projects (e.g., the Windows Start Menu app), not this CLI. [1]

If you paste the output of:

openshell provider create --help
# and ideally also:
openshell --version
openshell provider --help

(or share a link to the repo/docs for this openshell), I can tell you exactly what’s supported and what the relevant flags are.

Source

  1. Public search results for "openshell provider create" return unrelated “OpenShell” projects and no docs for this CLI subcommand. [1]

🏁 Script executed:

# First, let's examine the setup.sh file around line 143 to see the full context
wc -l scripts/setup.sh

Repository: NVIDIA/NemoClaw

Length of output: 79

🏁 Script executed:

# Read the file around line 143 to understand the context
sed -n '130,160p' scripts/setup.sh

Repository: NVIDIA/NemoClaw

Length of output: 1174

🏁 Script executed:

# Search for openshell definition and usage in the codebase
rg -i "openshell" --type-list | head -20

Repository: NVIDIA/NemoClaw

Length of output: 448

🏁 Script executed:

# Look for openshell in the repository
fd openshell

Repository: NVIDIA/NemoClaw

Length of output: 116

🏁 Script executed:

# Search for how openshell is invoked or defined
rg "openshell" -A 5 -B 5

Repository: NVIDIA/NemoClaw

Length of output: 50374

🏁 Script executed:

# Find the upsert_provider function definition
rg "upsert_provider\s*\(" -A 10 scripts/setup.sh | head -50

Repository: NVIDIA/NemoClaw

Length of output: 390

🏁 Script executed:

# Search for how openshell provider is invoked
rg "openshell provider" --type sh -A 2 -B 2

Repository: NVIDIA/NemoClaw

Length of output: 688

🏁 Script executed:

# Look for openshell help or documentation within the repo
fd -e md -e txt | xargs grep -l "openshell provider" 2>/dev/null || echo "No docs found"

Repository: NVIDIA/NemoClaw

Length of output: 72

🏁 Script executed:

# Check if there's any help text or documentation in install-openshell.sh
cat scripts/install-openshell.sh | head -100

Repository: NVIDIA/NemoClaw

Length of output: 2357

🏁 Script executed:

# Let's check if there's any reference to openshell --help or version info
rg "openshell.*--help|openshell.*--version" scripts/

Repository: NVIDIA/NemoClaw

Length of output: 626

🏁 Script executed:

# Check if there's any environment variable usage pattern for credentials in the codebase
rg "CREDENTIAL|credential" scripts/ -i -B 2 -A 2 | head -80

Repository: NVIDIA/NemoClaw

Length of output: 954

$(cat "$_CRED_FILE") still materializes the secret in child process argv.

The temp file approach protects the parent shell, but the expanded credential value is passed to openshell provider create --credential "NVIDIA_API_KEY=<key>" where it becomes visible in the spawned process's command line (accessible via /proc/[pid]/cmdline and ps).

Investigate whether openshell provider create supports reading credentials from environment variables, stdin, or credential file paths instead of the --credential KEY=VALUE argv parameter.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/setup.sh` at line 143, The current command substitutes the secret
into the argv via "NVIDIA_API_KEY=$(cat "$_CRED_FILE")", which exposes it to
ps/proc; instead, check whether the openshell CLI supports passing credentials
via environment variables, stdin, or a credential-file option and use that
mechanism; specifically, inspect the openshell provider create help for flags
like --credential-file or reading from stdin, and if available change the
invocation in scripts/setup.sh to supply the path (or pipe the file to stdin) or
export NVIDIA_API_KEY into the environment before running openshell (and
immediately unset it afterward) rather than embedding the literal key in the
command line. Ensure you remove the command-substitution usage and only load the
secret into a non-argv context.

"OPENAI_BASE_URL=https://integrate.api.nvidia.com/v1"

# vllm-local (if vLLM is installed or running)
Expand Down