Skip to content

fix(onboard): don't enforce nvapi- prefix on non-NVIDIA API keys#2389

Merged
ericksoa merged 3 commits intoNVIDIA:mainfrom
robobryce:fix/anthropic-api-key-validation
Apr 27, 2026
Merged

fix(onboard): don't enforce nvapi- prefix on non-NVIDIA API keys#2389
ericksoa merged 3 commits intoNVIDIA:mainfrom
robobryce:fix/anthropic-api-key-validation

Conversation

@robobryce
Copy link
Copy Markdown
Contributor

@robobryce robobryce commented Apr 23, 2026

Summary

  • Users selecting Anthropic (or any non-NVIDIA provider) in the onboarding wizard could see Invalid key. Must start with nvapi- when re-entering a valid Anthropic API key after an auth-failure retry.
  • Root cause: the retry path in promptValidationRecovery (bin/lib/onboard.js) routed all credentials through validateNvidiaApiKeyValue, which unconditionally rejected anything without the NVIDIA nvapi- prefix.
  • Fix: make validateNvidiaApiKeyValue provider-aware via an optional credentialEnv argument — the nvapi- check only applies when credentialEnv === "NVIDIA_API_KEY". The retry path now passes credentialEnv through, so Anthropic/OpenAI/Gemini keys are accepted on retry.
  • Clarified the error strings (Invalid NVIDIA API key. Must start with nvapi-) so users who do hit the check understand they are at the NVIDIA prompt.

Test plan

  • npx vitest run src/lib/validation.test.ts — added cases for Anthropic/OpenAI/Gemini credentialEnv values
  • npx vitest run test/credentials.test.js — updated error-string assertion
  • npx vitest run test/onboard-selection.test.js — existing Anthropic/OpenAI/Gemini retry flows still pass
  • npm run build:cli — TypeScript compiles
  • Manual reasoning: Anthropic retry path now calls validateNvidiaApiKeyValue(key, "ANTHROPIC_API_KEY") which returns null, so replaceNamedCredential accepts the key

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Validation now reports clearer, provider-specific messages and enforces the NVIDIA key prefix only when using the NVIDIA credential.
    • Recovery flow always wires a validator so non-NVIDIA credentials are validated with provider-aware rules.
  • Tests

    • Expanded and updated tests to cover provider-aware credential validation and to assert the more specific NVIDIA error messages.

Signed-off-by: Aaron Erickson aerickson@nvidia.com

@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 23, 2026

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

📝 Walkthrough

Walkthrough

Validation was made provider-aware: validateNvidiaApiKeyValue gained an optional credentialEnv parameter and enforces NVIDIA-specific prefix/messages only when credentialEnv indicates NVIDIA; related error text and validator wiring were updated in credentials/onboard and tests.

Changes

Cohort / File(s) Summary
Validation Core
src/lib/validation.ts
Added credentialEnv: string = "NVIDIA_API_KEY" param; derive isNvidia and apply NVIDIA-only prefix requirement and NVIDIA-specific required/error messages.
Credential Usage
src/lib/credentials.ts, src/lib/onboard.ts
Error message updated to reference "NVIDIA API key"; promptValidationRecovery now passes credentialEnv into the validator wrapper so validation is provider-aware.
Tests — Validation Behaviour
src/lib/validation.test.ts
Extended tests to exercise credentialEnv branch: non-nvapi- keys accepted when credentialEnvNVIDIA_API_KEY, empty keys still rejected.
Tests — Expectation Updates
test/credentials.test.ts, test/onboard-selection.test.ts, test/credential-exposure.test.ts
Updated assertions and validator-hoisting expectation to reflect provider-aware validation and the more specific "Invalid NVIDIA API key" messaging.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through keys both short and long,
I learned when nvapi- must sing along.
For NVIDIA I guard the precious key,
For others I nod and set them free. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: making the nvapi- prefix validation conditional for non-NVIDIA providers during the onboarding retry flow.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

Users selecting Anthropic (or any non-NVIDIA provider) in the
onboarding flow could see "Must start with nvapi-" when re-entering a
valid Anthropic API key after an auth-failure retry, because the retry
path in promptValidationRecovery routed all credentials through a
validator that unconditionally rejected anything without the NVIDIA
nvapi- prefix.

Make validateNvidiaApiKeyValue provider-aware via an optional
credentialEnv argument: the nvapi- check only applies when
credentialEnv === "NVIDIA_API_KEY". The retry path in onboard.ts now
passes credentialEnv through, so Anthropic/OpenAI/Gemini keys are
accepted on retry. Also clarify the error strings to name NVIDIA
explicitly so a user who does hit the check understands they are at
the NVIDIA prompt.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@robobryce robobryce force-pushed the fix/anthropic-api-key-validation branch from bb7e71d to 916e1c4 Compare April 24, 2026 13:43
@brycelelbach
Copy link
Copy Markdown
Contributor

image

Resolves a merge conflict in test/onboard-selection.test.ts, where main
had retyped the lambda parameter to (line: string) while this PR
clarified the assertion text to "Invalid NVIDIA API key. Must start
with nvapi-". Combined: typed parameter + the new assertion string.

Also updates test/credential-exposure.test.ts:114 ("api-key paste-guard
uses extensible prefix list and regex fallback") to match the rewritten
validator line in src/lib/onboard.ts. The original test (added by PR
NVIDIA#1313) asserted the source contained `const validator = credentialEnv
=== "NVIDIA_API_KEY"`; this PR refactors that to a one-line provider-
aware delegate `(key) => validateNvidiaApiKeyValue(key, credentialEnv)`,
so the regression test's regex needs to match the new shape. The
underlying single-definition invariant from NVIDIA#1313 is preserved.

Pushed by maintainer (Allow edits by maintainers is enabled on this PR).

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Copy link
Copy Markdown
Contributor

@ericksoa ericksoa left a comment

Choose a reason for hiding this comment

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

Reviewed the diff and the merge resolution. The PR's framing oversells the safety impact (Anthropic keys already worked on retry — pre-PR validator was already provider-aware), but the actual changes are sound: (1) clearer error message — Invalid NVIDIA API key vs Invalid key — useful on the NVIDIA paths that legitimately reject non-nvapi values; (2) refactor consolidating provider-awareness into validateNvidiaApiKeyValue itself; (3) tests for the new 2-arg signature. Net positive, low regression risk. Pushed merge commit 655b22e to resolve the conflict and unbreak macos-e2e (test/credential-exposure.test.ts source-text regex needed updating for the new validator line shape; #1313's single-definition invariant is preserved).

@ericksoa ericksoa merged commit 6f7f0c6 into NVIDIA:main Apr 27, 2026
11 of 15 checks passed
@miyoungc miyoungc mentioned this pull request Apr 28, 2026
13 tasks
miyoungc added a commit that referenced this pull request Apr 28, 2026
## Summary
Refreshes user-facing docs for the last 24 hours of merged NemoClaw
history and bumps the docs metadata to 0.0.29, the next version after
v0.0.28. The updates are limited to behavior supported by merged PR
descriptions and diffs.

## Changes
- `docs/reference/commands.md`: documented `nemoclaw <name> policy-add
--from-file` and `--from-dir`, including custom preset review guidance,
from #2077 / commit `7720b175`.
- `docs/deployment/deploy-to-remote-gpu.md`: clarified that non-loopback
`CHAT_UI_URL` disables OpenClaw device pairing for remote browser-only
deployments, from #2449 / commit `f5ee8a4d`.
- `docs/inference/inference-options.md`: documented provider-aware
credential retry validation and the NVIDIA-only `nvapi-` prefix check,
from #2389 / commit `6f7f0c6d`.
- `docs/inference/switch-inference-providers.md`: documented
`NEMOCLAW_INFERENCE_INPUTS` for text/image-capable model metadata baked
into `openclaw.json`, from #2441 / commit `f4391892`.
- `docs/reference/troubleshooting.md`: added the Git certificate
verification entry for proxy CA propagation through `GIT_SSL_CAINFO`,
`GIT_SSL_CAPATH`, `CURL_CA_BUNDLE`, and `REQUESTS_CA_BUNDLE`, from #2345
/ commit `fa0dc1ab`.
- `docs/versions1.json` and `docs/project.json`: promoted docs version
`0.0.29`; `docs/versions1.json` omits unpublished `0.0.26`, `0.0.27`,
and `0.0.28` entries.
- `.agents/skills/nemoclaw-user-*`: regenerated derived user skill
references from the updated docs.
- Reviewed with no extra doc changes: #2575 / `d392ec07`, #2565 /
`a3231049`, #1965 / `db1ef3ca`, #1990 / `db665834`, #2495 / `7da86fa3`,
#2496 / `3192f4f4`, #2490 / `8c209058`, #2487 / `1f615e2f`, #2483 /
`5653d33a`, #2482 / `31c782c0`, #2464 / `23bb5703`, #2472 / `a54f9a34`,
and #2437 / `6bc860d7`.
- Skipped per docs policy: #2420 / `7b76df6b` touched the experimental
sandbox config path listed in `docs/.docs-skip`; #2466 / `cc15689c`
touched a skipped term and CI-only sandbox image files.

## Type of Change
- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [ ] Doc only (prose changes, no code sample modifications)
- [x] Doc only (includes code sample changes)

## Verification
<!-- Check each item you ran and confirmed. Leave unchecked items you
skipped. -->
- [x] `npx prek run --all-files` passes
- [ ] `npm test` passes — failed locally in installer-integration tests
and one onboard helper timeout; the doc-scoped hook test projects passed
under `prek`.
- [ ] Tests added or updated for new or changed behavior
- [x] No secrets, API keys, or credentials committed
- [x] Docs updated for user-facing behavior changes
- [ ] `make docs` builds without warnings (doc changes only) — build
succeeded, but local Sphinx emitted the existing version-switcher file
read message.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)

## AI Disclosure
<!-- If an AI agent authored or co-authored this PR, check the box and
name the tool. Remove this section for fully human-authored PRs. -->
- [x] AI-assisted — tool: Codex

---
<!-- DCO sign-off required by CI. Run: git config user.name && git
config user.email -->
Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Support for custom YAML presets in policy configuration via
--from-file and --from-dir.
* New build-time inference input option to declare accepted modalities
(text or text,image).

* **Improvements**
* Credential validation now offers interactive recovery: re-enter key,
retry, choose another provider, or exit.
* Clarified provider-specific API key prefix handling (nvapi- only
applies to NVIDIA keys).

* **Documentation**
  * TLS certificate troubleshooting for inspected networks.
* Clarified remote dashboard security/device-pairing behavior; command
docs updated; docs version bumped.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Miyoung Choi <miyoungc@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants