fix(orchestrator): use provider-specific credential environment variable by dumko2001 · Pull Request #191 · NVIDIA/NemoClaw

dumko2001 · 2026-03-17T10:24:34Z

Rationale

Using a hardcoded ENV variable for all providers prevented multi-provider configurations (e.g. NVIDIA and NIM) from coexisting cleanly.

Changes

Dynamically resolved the correct environment variable name based on the provider type in the orchestrator dispatch.

Verification Results

Automated Tests: Passed all 52 core tests via npm test.
Manual Audit: Verified correct variable interpolation across multiple test profiles.
Security Review: Verified no sensitive data leaks and correct permission enforcement.

Summary by CodeRabbit

Bug Fixes
- Enhanced credential environment resolution for improved multi-provider support. NVIDIA-based AI providers now correctly use their dedicated credentials instead of defaulting to generic settings. Dynamic credential configuration has been improved to ensure proper handling across different provider types and their respective authentication requirements.

ericksoa

LGTM — correct fix for credential env var passthrough.

dumko2001 · 2026-03-18T18:53:12Z

Hey @ericksoa ,

I've been contributing to the GWS CLI recently, and the developer there is using an OpenClaw bot and a separate Gemini bot to review pull requests and push PRs and thats accelerating the development of the project.

I want to propose we use something like that here as a reviewer here for this repo. It would help us review PRs more easily and accelerate getting more commits in. Maybe we can have a bot that pushes PR's too .Since I'm already working on the GWS CLI and this repo, I'd like to help build this or even take charge of more responsibilities around the agent/reviewer side .

Is this something we can explore for this repo?

… in argv Fixes: NVIDIA#325 (API key exposed in process list via ps aux) Supersedes: PRs NVIDIA#191, NVIDIA#330 The root cause: all three execution layers passed the actual credential VALUE as --credential KEY=VALUE, making it visible to any local user via `ps aux` or /proc/<pid>/cmdline. Safe pattern: set the secret in the child's inherited env, then pass only the env-var NAME to --credential (openshell env-lookup form). nemoclaw/src/commands/onboard.ts - process.env[credentialEnv] = apiKey before execOpenShell - --credential arg: credentialEnv (name only, not KEY=VALUE) - applies to both provider create and provider update paths nemoclaw-blueprint/orchestrator/runner.py - Rename credential_env -> target_cred_env with type-based fallback (nvidia -> NVIDIA_API_KEY, openai -> OPENAI_API_KEY) when not set in the blueprint profile. Supersedes PR NVIDIA#191's partial fix. - os.environ[target_cred_env] = credential before run_cmd - --credential arg: target_cred_env (name only) nemoclaw-blueprint/blueprint.yaml - Add credential_env: NVIDIA_API_KEY to the default profile. Without this field the type-based fallback would silently use OPENAI_API_KEY for the nvidia provider_type, causing auth failure. nemoclaw/src/onboard/config.ts - writeFileSync for config.json now passes mode: 0o600 so the file containing endpoint/model/credentialEnv metadata is not world-readable. test/credential-exposure.test.js (new file) - Static source scan: asserts no --credential KEY=VALUE pattern in any of the 3 execution layer files (allowlists dummy/ollama stubs) - Layer-specific structural checks (process.env set, os.environ set, blueprint default profile has credential_env) - Runtime injection PoC: proves old bash -c IS vulnerable; new runCaptureArgv IS NOT All 84 tests pass.

During openshell provider create, the NVIDIA API key was interpolated directly into shell command strings, making it visible via ps aux, /proc/PID/cmdline, and process monitoring tools on shared systems. Fixes: - bin/lib/onboard.js: Write credential to temp file (mode 0600, exclusive create) and use $(cat ...) substitution so the key only exists in bash's memory during expansion, not in the parent's command string. File is cleaned up in a finally block. - scripts/setup.sh: Same temp file + cat pattern with trap EXIT cleanup. - bin/nemoclaw.js: Use sudo --preserve-env instead of interpolating the key into the command string. - nemoclaw/src/commands/onboard.ts: Added comment noting the remaining /proc exposure requires openshell --credential-file or --credential-from-env support (see PR NVIDIA#191). The credential still appears briefly in the openshell child process arguments. Full mitigation requires openshell to support reading credentials from a file or environment variable rather than CLI args. Fixes NVIDIA#325 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

cv · 2026-03-21T19:30:04Z

Hi @dumko2001 — using provider-specific credential env vars is cleaner and more correct. A rebase onto the latest main is needed before we can review though. Mind updating? Thanks for all the contributions!

coderabbitai · 2026-03-21T21:10:50Z

📝 Walkthrough

Walkthrough

The action_apply function in the orchestrator runner now dynamically resolves credential environment variables based on the provider type. It defaults to NVIDIA_API_KEY for NVIDIA providers and OPENAI_API_KEY otherwise, enabling flexible credential handling across multiple provider types with fallback support.

Changes

Cohort / File(s)	Summary
Credential Resolution Enhancement `nemoclaw-blueprint/orchestrator/runner.py`	Modified credential environment variable resolution to use a computed `target_cred_env` that adapts based on provider type (NVIDIA vs. default). Updated credential lookup and command argument generation to use the dynamic key name, and added clarifying comment for endpoint handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hops through credential streams,
Now NVIDIA dreams and OpenAI schemes,
Dynamic keys dance with provider grace,
Environment resolved in the right place! 🐰✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(orchestrator): use provider-specific credential environment variable' directly describes the main change in the changeset—dynamically resolving provider-specific credential environment variables instead of using hardcoded values.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

nemoclaw-blueprint/orchestrator/runner.py (1)

192-199: ⚠️ Potential issue | 🟠 Major

Normalize provider type before fallback credential-env selection.

Line 199 uses a case-sensitive check (provider_type == "nvidia"). Mixed-case values (e.g., NVIDIA) will incorrectly fall back to OPENAI_API_KEY, which can break auth at runtime.

Proposed fix

-    provider_type: str = inference_cfg.get("provider_type", "openai")
+    provider_type_raw = str(inference_cfg.get("provider_type", "openai"))
+    provider_type: str = provider_type_raw.strip().lower()
@@
-    if not target_cred_env:
-        target_cred_env = "NVIDIA_API_KEY" if provider_type == "nvidia" else "OPENAI_API_KEY"
+    if not target_cred_env:
+        target_cred_env = "NVIDIA_API_KEY" if provider_type == "nvidia" else "OPENAI_API_KEY"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@nemoclaw-blueprint/orchestrator/runner.py` around lines 192 - 199, The
fallback credential selection uses a case-sensitive check on provider_type;
update the logic so provider_type is normalized before comparison (e.g., call
.lower() or .casefold() on the value returned from inference_cfg.get or when
performing the comparison) so the target_cred_env assignment (the branch that
sets "NVIDIA_API_KEY" vs "OPENAI_API_KEY") correctly handles mixed-case inputs;
modify the code that assigns provider_type or the conditional that sets
target_cred_env (referencing provider_type, target_cred_env, and inference_cfg)
to use the normalized string for the comparison.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@nemoclaw-blueprint/orchestrator/runner.py`:
- Around line 192-199: The fallback credential selection uses a case-sensitive
check on provider_type; update the logic so provider_type is normalized before
comparison (e.g., call .lower() or .casefold() on the value returned from
inference_cfg.get or when performing the comparison) so the target_cred_env
assignment (the branch that sets "NVIDIA_API_KEY" vs "OPENAI_API_KEY") correctly
handles mixed-case inputs; modify the code that assigns provider_type or the
conditional that sets target_cred_env (referencing provider_type,
target_cred_env, and inference_cfg) to use the normalized string for the
comparison.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b9e10803-3cb8-44ed-bc3f-d2543a50fd96

📥 Commits

Reviewing files that changed from the base of the PR and between 255e606 and 29ec306.

📒 Files selected for processing (1)

nemoclaw-blueprint/orchestrator/runner.py

dumko2001 · 2026-03-21T21:13:11Z

@cv rebased.

During openshell provider create, the NVIDIA API key was interpolated directly into shell command strings, making it visible via ps aux, /proc/PID/cmdline, and process monitoring tools on shared systems. Fixes: - bin/lib/onboard.js: Write credential to temp file (mode 0600, exclusive create) and use $(cat ...) substitution so the key only exists in bash's memory during expansion, not in the parent's command string. File is cleaned up in a finally block. - scripts/setup.sh: Same temp file + cat pattern with trap EXIT cleanup. - bin/nemoclaw.js: Use sudo --preserve-env instead of interpolating the key into the command string. - nemoclaw/src/commands/onboard.ts: Added comment noting the remaining /proc exposure requires openshell --credential-file or --credential-from-env support (see PR NVIDIA#191). The credential still appears briefly in the openshell child process arguments. Full mitigation requires openshell to support reading credentials from a file or environment variable rather than CLI args. Fixes NVIDIA#325 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…d remove openclaw (NVIDIA#191)

cv · 2026-03-30T18:49:36Z

We no longer have a nemoclaw-blueprint/orchestrator/runner.py

ericksoa mentioned this pull request Mar 18, 2026

[Security] NVIDIA API key exposed in process list when creating inference provider #325

Closed

ericksoa approved these changes Mar 18, 2026

View reviewed changes

dumko2001 mentioned this pull request Mar 18, 2026

fix(security): eliminate API key leakage via ps aux across all three execution layers dumko2001/NemoClaw#1

Open

dumko2001 mentioned this pull request Mar 19, 2026

fix(security): harden process execution — shell injection + credential leak via ps aux #384

Closed

dumko2001 mentioned this pull request Mar 19, 2026

fix(security): harden process execution — shell injection + credential leak via ps aux #387

Closed

pjt222 mentioned this pull request Mar 19, 2026

security: hide API credentials from process argument list #394

Closed

5 tasks

wscurran added the enhancement: feature Use this label to identify requests for new capabilities in NemoClaw. label Mar 19, 2026

fix(orchestrator): use provider-specific credential environment variable

29ec306

dumko2001 force-pushed the fix/c3-credential-env-nvidia-provider branch from 01bcad3 to 29ec306 Compare March 21, 2026 21:10

coderabbitai bot reviewed Mar 21, 2026

View reviewed changes

mafueee pushed a commit to mafueee/NemoClaw that referenced this pull request Mar 28, 2026

fix(containers): remediate high-severity container vulnerabilities an…

a83109c

…d remove openclaw (NVIDIA#191)

cv closed this Mar 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(orchestrator): use provider-specific credential environment variable#191

fix(orchestrator): use provider-specific credential environment variable#191
dumko2001 wants to merge 1 commit intoNVIDIA:mainfrom
dumko2001:fix/c3-credential-env-nvidia-provider

dumko2001 commented Mar 17, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

ericksoa left a comment

Uh oh!

dumko2001 commented Mar 18, 2026

Uh oh!

cv commented Mar 21, 2026

Uh oh!

coderabbitai bot commented Mar 21, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Uh oh!

dumko2001 commented Mar 21, 2026

Uh oh!

cv commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dumko2001 commented Mar 17, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale

Changes

Verification Results

Summary by CodeRabbit

Uh oh!

ericksoa left a comment

Choose a reason for hiding this comment

Uh oh!

dumko2001 commented Mar 18, 2026

Uh oh!

cv commented Mar 21, 2026

Uh oh!

coderabbitai bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

dumko2001 commented Mar 21, 2026

Uh oh!

cv commented Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dumko2001 commented Mar 17, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 21, 2026 •

edited

Loading