diff --git a/.agents/skills/nemoclaw-user-configure-security/SKILL.md b/.agents/skills/nemoclaw-user-configure-security/SKILL.md index e8850aef16d..e6cd9522d54 100644 --- a/.agents/skills/nemoclaw-user-configure-security/SKILL.md +++ b/.agents/skills/nemoclaw-user-configure-security/SKILL.md @@ -1,6 +1,6 @@ --- name: "nemoclaw-user-configure-security" -description: "Presents a risk framework for every configurable security control in NemoClaw. Use when evaluating security posture, reviewing sandbox security defaults, or assessing control trade-offs. Trigger keywords - nemoclaw security best practices, sandbox security controls risk framework, nemoclaw credential storage, credentials.json, api key security, openclaw security controls, nemoclaw security boundary, prompt injection, tool access control." +description: "Presents a risk framework for every configurable security control in NemoClaw. Use when evaluating security posture, reviewing sandbox security defaults, or assessing control trade-offs. Trigger keywords - nemoclaw security best practices, sandbox security controls risk framework, nemoclaw credential storage, openshell provider, api key security, openclaw security controls, nemoclaw security boundary, prompt injection, tool access control." --- @@ -12,4 +12,4 @@ description: "Presents a risk framework for every configurable security control - **Load [references/best-practices.md](references/best-practices.md)** when evaluating security posture, reviewing sandbox security defaults, or assessing control trade-offs. Presents a risk framework for every configurable security control in NemoClaw. - **Load [references/openclaw-controls.md](references/openclaw-controls.md)** when reviewing the security boundary between NemoClaw and OpenClaw or assessing what NemoClaw does not cover. Lists OpenClaw security controls that operate independently of NemoClaw, including prompt injection detection, tool access control, rate limiting, environment variable policy, audit framework, supply chain scanning, messaging access policy, context visibility, and safe regex. -- **Load [references/credential-storage.md](references/credential-storage.md)** when reviewing how credentials are handled, locating a specific credential file, or assessing the risk of the unencrypted-at-rest default. Covers where NemoClaw stores provider credentials, the file permissions applied, and the trade-offs of plaintext local storage. +- **Load [references/credential-storage.md](references/credential-storage.md)** when reviewing how credentials are handled, locating a stored credential, or assessing the storage threat model. Covers where NemoClaw stores provider credentials, why nothing is persisted to host disk, and how the OpenShell gateway acts as the single system of record. diff --git a/.agents/skills/nemoclaw-user-configure-security/references/credential-storage.md b/.agents/skills/nemoclaw-user-configure-security/references/credential-storage.md index 8ff7c78f537..970f8e4f81b 100644 --- a/.agents/skills/nemoclaw-user-configure-security/references/credential-storage.md +++ b/.agents/skills/nemoclaw-user-configure-security/references/credential-storage.md @@ -2,112 +2,107 @@ # Credential Storage -NemoClaw stores operator-provided host-side credentials under `~/.nemoclaw/`. -These credentials are used during onboarding and host-side lifecycle operations. -They are not encrypted at rest by NemoClaw. -Instead, NemoClaw relies on local filesystem ownership and Unix permissions to limit access. +NemoClaw does not persist provider credentials to host disk. +The OpenShell gateway is the only system of record for stored credentials. -## Location and Permissions +When you provide a provider credential — interactively during `nemoclaw onboard` or via an environment variable — NemoClaw holds the value in memory only long enough to register it with the OpenShell gateway through `openshell provider create` or `openshell provider update`. +The gateway stores the credential and the OpenShell L7 proxy substitutes it into outbound requests at egress, so sandboxed agents see placeholders instead of the raw secret. -By default, NemoClaw stores credentials in: +`nemoclaw config rotate-token` is a separate flow that rotates a sandbox-side OpenClaw auth token; it is not a provider-credential upsert and is documented under Commands (use the `nemoclaw-user-reference` skill). -```text -~/.nemoclaw/credentials.json -``` +## Where Credentials Live -When NemoClaw creates this state directory, it uses owner-only permissions: +Provider credentials live in the OpenShell gateway store. +List what is registered with: -- `~/.nemoclaw/` is created with mode `0700` -- `~/.nemoclaw/credentials.json` is written with mode `0600` +```console +$ openshell provider list +``` -That means only the local account that owns the files should be able to read or modify them. +Or, equivalently, through NemoClaw: -NemoClaw also refuses to use obviously unsafe `HOME` paths such as `/tmp`, `/var/tmp`, `/dev/shm`, or `/` for credential storage. -If `HOME` points to one of those locations, onboarding exits with an error instead of writing secrets there. +```console +$ nemoclaw credentials list +``` -## Plaintext Storage Warning +Both surface the provider names that the gateway holds credentials for. The values themselves cannot be read back from the CLI; this is a deliberate property of OpenShell. -The credential file is plaintext JSON. -NemoClaw does **not** currently encrypt the file or integrate with the host operating system keychain. +NemoClaw still keeps non-secret operational state under `~/.nemoclaw/` (such as the sandbox registry). +That directory is created with mode `0700` and contains no credential material. -A typical file looks like this: +## Environment Variables Take Precedence -```json -{ - "NVIDIA_API_KEY": "nvapi-...", - "GITHUB_TOKEN": "ghp_...", - "OPENAI_API_KEY": "sk-..." -} -``` +When a NemoClaw command needs a credential value during a single run (for example to forward it to an `openshell provider` registration), it reads from `process.env` first. +This means you can: -Treat this file like any other local secret material. -Anyone who can read it can reuse those credentials with the upstream provider. +- Prefix any command with the credential to override the gateway-stored value: `NVIDIA_API_KEY=nvapi-... nemoclaw onboard` +- Use short-lived or rotated credentials in CI by exporting them once per pipeline run +- Avoid registering credentials in the gateway entirely if your environment supplies them -## Precedence and Scope +## Deploy Reads from Environment Only -When NemoClaw looks up a credential, it checks environment variables first. -If the corresponding environment variable is set, NemoClaw uses that value instead of the stored file. +`nemoclaw deploy` (which provisions a remote Brev box) cannot read secrets back from the gateway, so it requires every credential to be present in the host environment at invocation time. +A typical deploy invocation looks like: -This behavior is useful for: +```console +$ NVIDIA_API_KEY=nvapi-... \ + TELEGRAM_BOT_TOKEN=... \ + nemoclaw deploy my-instance +``` -- CI or automation where you do not want to persist secrets to disk -- temporary overrides during testing -- short-lived or rotated credentials +If a required credential is missing the deploy aborts before any remote work begins. -For interactive local use, `nemoclaw onboard` can save credentials into `~/.nemoclaw/credentials.json` so future runs do not prompt again. +## GitHub Tokens -## Security Recommendations +NemoClaw never persists `GITHUB_TOKEN` itself. +When a private repo requires authentication NemoClaw runs `gh auth token`, which returns whatever the GitHub CLI has stored — without caring about the storage backend. -Use the following practices to reduce the risk of credential exposure. +The GitHub CLI prefers an OS keychain when one is reachable: macOS Keychain on macOS, Windows Credential Manager on Windows, and Linux Secret Service (libsecret + a running D-Bus session) on Linux. +On hosts where no keychain is reachable (CI runners, headless launches, WSL without a session bus, macOS contexts where Keychain access is blocked, etc.) `gh auth login` falls back to a `gh`-managed file under `~/.config/gh/` with mode `0600`. +NemoClaw treats both backends identically: `gh auth token` returns the value, and NemoClaw stages it in `process.env` for the current run only. -1. Keep your home directory private and owned by your user account. -2. Exclude `~/.nemoclaw/` from cloud-sync folders, shared folders, and broad backup exports unless those systems are already approved for secret storage. -3. Prefer short-lived or low-scope provider credentials where the upstream service supports them. -4. Rotate keys after suspected exposure, machine transfer, or account changes. -5. Prefer environment variables for ephemeral automation instead of persisting long-lived secrets locally. -6. Do not copy `credentials.json` into container images, Git repositories, bug reports, or support bundles. +If `gh` is not installed or not logged in, NemoClaw prompts for a personal access token for that single run; the prompted value is held in process memory and is not written to host disk. +Run `gh auth login` if you want a persistent backing store (whichever one applies on your host) so future runs do not prompt. -## Inspect and Repair Permissions +## Migration From Earlier Releases -To inspect the current permissions: +Earlier NemoClaw releases stored credentials as plaintext JSON in `~/.nemoclaw/credentials.json` with mode `0600`. +On first `nemoclaw onboard` after upgrading, NemoClaw automatically: -```console -$ ls -ld ~/.nemoclaw ~/.nemoclaw/credentials.json -``` +1. Reads the legacy file. +2. Stages allowlisted credential values into `process.env` for the rest of the run. +3. Re-registers each value with the OpenShell gateway through the normal onboarding path. +4. Securely overwrites and deletes `~/.nemoclaw/credentials.json` only after every staged value has been verified as migrated to the gateway. -Expected output should show a private directory and file, for example: +You will see a one-line stderr notice the first time this happens. +Credential lookup paths such as rebuild also stage allowlisted legacy values so interrupted upgrades can keep working, but those staging-only paths do not delete the plaintext file because they cannot prove every legacy value was registered with the gateway. +If `~/.nemoclaw/credentials.json` remains after a rebuild or other credential lookup, run `nemoclaw onboard` to complete the verified gateway migration and cleanup. -```text -drwx------ ... ~/.nemoclaw --rw------- ... ~/.nemoclaw/credentials.json -``` +## Rotate or Remove a Stored Credential -If the permissions are broader than expected, tighten them: +The simplest way to replace a stored value is to rerun onboarding with the new value in your environment: ```console -$ chmod 700 ~/.nemoclaw -$ chmod 600 ~/.nemoclaw/credentials.json +$ NVIDIA_API_KEY=nvapi-new-value nemoclaw onboard ``` -## Rotate or Remove Stored Credentials - -The simplest way to replace a stored provider key is to rerun onboarding and provide the new value when prompted: +To remove a credential from the gateway entirely: ```console -$ nemoclaw onboard +$ nemoclaw credentials reset ``` -To remove the stored file entirely: +`` is the OpenShell provider name (run `nemoclaw credentials list` first if you are not sure). +On the next run NemoClaw prompts again unless the credential is supplied through the environment. -```console -$ rm -f ~/.nemoclaw/credentials.json -``` +## Security Recommendations -On the next run, NemoClaw prompts again unless the credential is supplied through the environment. +1. Prefer short-lived or low-scope provider credentials where the upstream service supports them. +2. Rotate keys after suspected exposure, machine transfer, or account changes. +3. Prefer environment variables for ephemeral automation rather than registering long-lived secrets in the gateway. +4. Do not copy any host-side NemoClaw state into container images, Git repositories, bug reports, or support bundles. Even though credentials no longer live on disk, the surrounding configuration may reveal which providers you have registered. +5. Keep your home directory private and owned by your user account. ## Related Files -Other NemoClaw host-side state also lives under `~/.nemoclaw/`, such as sandbox registry metadata. -These files are operational state, not provider secrets, but they should still remain in a user-owned home directory. - For the broader sandbox security model and operational trade-offs, see Security Best Practices (use the `nemoclaw-user-configure-security` skill) and Architecture (use the `nemoclaw-user-reference` skill). diff --git a/.agents/skills/nemoclaw-user-get-started/SKILL.md b/.agents/skills/nemoclaw-user-get-started/SKILL.md index ecc2e494632..f7fb9e665ea 100644 --- a/.agents/skills/nemoclaw-user-get-started/SKILL.md +++ b/.agents/skills/nemoclaw-user-get-started/SKILL.md @@ -191,7 +191,7 @@ After you enter the sandbox name, the wizard prints a review summary and asks fo ────────────────────────────────────────────────── Provider: nvidia-api Model: nvidia/nemotron-3-super-120b-a12b - API key: NVIDIA_API_KEY (stored in ~/.nemoclaw/credentials.json) + API key: NVIDIA_API_KEY (registered with the OpenShell gateway) Web search: disabled Messaging: none Sandbox name: my-assistant @@ -307,15 +307,15 @@ Refer to Switch inference providers (use the `nemoclaw-user-configure-inference` ### Reset a Stored Credential -If an API key was entered incorrectly during onboarding, clear the stored value and re-enter it on the next onboard run: +If a provider credential was entered incorrectly during onboarding, clear the gateway-registered value and re-enter it on the next onboard run: ```console -$ nemoclaw credentials list # see which keys are stored -$ nemoclaw credentials reset # clear a single key, for example NVIDIA_API_KEY -$ nemoclaw onboard # re-run to re-enter the cleared key +$ nemoclaw credentials list # see which providers are registered +$ nemoclaw credentials reset # clear a single provider, for example nvidia-prod +$ nemoclaw onboard # re-run to re-enter the cleared provider ``` -The credentials command is documented in full at `nemoclaw credentials reset ` (use the `nemoclaw-user-reference` skill). +The credentials command is documented in full at `nemoclaw credentials reset ` (use the `nemoclaw-user-reference` skill). ### Rebuild a Sandbox While Preserving Workspace State diff --git a/.agents/skills/nemoclaw-user-reference/references/architecture.md b/.agents/skills/nemoclaw-user-reference/references/architecture.md index c4d736a4e7d..f54bdd83b14 100644 --- a/.agents/skills/nemoclaw-user-reference/references/architecture.md +++ b/.agents/skills/nemoclaw-user-reference/references/architecture.md @@ -223,13 +223,18 @@ Agent (sandbox) ──▶ OpenShell gateway ──▶ NVIDIA Endpoint (build Refer to Inference Options (use the `nemoclaw-user-configure-inference` skill) for provider configuration details. +## Provider Credential Storage + +Provider credentials live in the OpenShell gateway store, not on the host filesystem. +NemoClaw never writes them to host disk; the OpenShell L7 proxy injects values at egress. +See Credential Storage (use the `nemoclaw-user-configure-security` skill) for the inspection, rotation, and migration flow. + ## Host-Side State and Config -NemoClaw keeps its operator-facing state on the host rather than inside the sandbox. +NemoClaw keeps non-secret operator-facing state on the host rather than inside the sandbox. | Path | Purpose | |---|---| -| `~/.nemoclaw/credentials.json` | Provider credentials saved during onboarding. Stored as plaintext JSON protected by local filesystem permissions; see Credential Storage (use the `nemoclaw-user-configure-security` skill). | | `~/.nemoclaw/sandboxes.json` | Registered sandbox metadata, including the default sandbox selection. | | `~/.openclaw/openclaw.json` | Host OpenClaw configuration that NemoClaw snapshots or restores during migration flows. | diff --git a/.agents/skills/nemoclaw-user-reference/references/commands.md b/.agents/skills/nemoclaw-user-reference/references/commands.md index 5370e634125..ac9f407a8ea 100644 --- a/.agents/skills/nemoclaw-user-reference/references/commands.md +++ b/.agents/skills/nemoclaw-user-reference/references/commands.md @@ -59,7 +59,7 @@ $ NEMOCLAW_SINGLE_SESSION=1 curl -fsSL https://www.nvidia.com/nemoclaw.sh | bash The wizard prompts for a provider first, then collects the provider credential if needed. Supported non-experimental choices include NVIDIA Endpoints, OpenAI, Anthropic, Google Gemini, and compatible OpenAI or Anthropic endpoints. -Credentials are stored in `~/.nemoclaw/credentials.json`. For file permissions, plaintext storage behavior, and hardening guidance, see Credential Storage (use the `nemoclaw-user-configure-security` skill). +Credentials are registered with the OpenShell gateway and never persisted to host disk. See Credential Storage (use the `nemoclaw-user-configure-security` skill) for details on inspection, rotation, and migration from earlier releases. The legacy `nemoclaw setup` command is deprecated; use `nemoclaw onboard` instead. After provider selection, the wizard prompts for a **policy tier** that controls the default set of network policy presets applied to the sandbox. @@ -391,7 +391,7 @@ $ nemoclaw my-assistant channels list ### `nemoclaw channels add ` Store credentials for a messaging channel (`telegram`, `discord`, or `slack`) and rebuild the sandbox so the image picks up the new channel. -The command prompts for any missing token, persists it under `~/.nemoclaw/credentials.json`, then asks whether to rebuild immediately. +The command prompts for any missing token, registers it with the OpenShell gateway, then asks whether to rebuild immediately. Running `add` for an already-configured channel simply overwrites the stored tokens — the operation is idempotent. ```console @@ -424,7 +424,9 @@ Host-side removal is the supported path because `/sandbox/.openclaw/openclaw.jso ### `nemoclaw channels stop ` -Pause a single messaging bridge (`telegram`, `discord`, or `slack`) without clearing its credentials. The channel is marked disabled in the per-sandbox registry, and the sandbox is rebuilt so the onboard step skips registering the bridge with the gateway. Credentials stay in `~/.nemoclaw/credentials.json`, so a later `channels start` brings the bridge back without re-entering tokens. +Pause a single messaging bridge (`telegram`, `discord`, or `slack`) without clearing its credentials. +The channel is marked disabled in the per-sandbox registry, and the sandbox is rebuilt so the onboard step skips registering the bridge with the gateway. +The provider stays registered with the OpenShell gateway, so a later `channels start` brings the bridge back without re-entering tokens. ```console $ nemoclaw my-assistant channels stop telegram @@ -673,20 +675,21 @@ If `--output` is set and the tarball cannot be written (for example, the destina ### `nemoclaw credentials list` -List the names of all credentials stored in `~/.nemoclaw/credentials.json`. +List the provider credentials registered with the OpenShell gateway. Values are not printed. ```console $ nemoclaw credentials list ``` -### `nemoclaw credentials reset ` +### `nemoclaw credentials reset ` -Remove a stored credential by name. -After removal, re-running `nemoclaw onboard` re-prompts for that key. +Remove a provider credential from the OpenShell gateway by provider name. +After removal, re-running `nemoclaw onboard` re-prompts for that provider's credential. +Run `nemoclaw credentials list` first if you are not sure of the provider name. ```console -$ nemoclaw credentials reset NVIDIA_API_KEY +$ nemoclaw credentials reset nvidia-prod ``` | Flag | Description | diff --git a/.agents/skills/nemoclaw-user-reference/references/troubleshooting.md b/.agents/skills/nemoclaw-user-reference/references/troubleshooting.md index 27e2afd94da..daf0c142aae 100644 --- a/.agents/skills/nemoclaw-user-reference/references/troubleshooting.md +++ b/.agents/skills/nemoclaw-user-reference/references/troubleshooting.md @@ -568,7 +568,7 @@ $ nemoclaw channels add $ nemoclaw channels remove ``` -`channels add` stores credentials under `~/.nemoclaw/credentials.json` and `channels remove` clears them; both offer to rebuild the sandbox so the image reflects the new channel set. +`channels add` registers credentials with the OpenShell gateway and `channels remove` clears them; both offer to rebuild the sandbox so the image reflects the new channel set. In non-interactive mode (`NEMOCLAW_NON_INTERACTIVE=1`), the commands stage the change and leave the rebuild to a follow-up `nemoclaw rebuild`. ### `nemoclaw config set` refuses a key that does not currently exist diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index 7245385e62e..c8b78bacd4d 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -15,6 +15,9 @@ # probe → live inference. Validates the multi-agent architecture. # skip-permissions-e2e Validates --dangerously-skip-permissions activates the permissive # policy (not stuck in Pending) and sandbox egress works (not 403). +# credential-migration-e2e Validates legacy ~/.nemoclaw/credentials.json migration to the +# OpenShell gateway, secure zero-fill on unlink, allowlist filter +# on non-credential env keys, and symlink-safe deletion. # gpu-e2e Local Ollama inference on an NVKS ephemeral GPU runner. # gpu-double-onboard-e2e Ollama proxy token consistency after re-onboard (#2553). # notify-on-failure Auto-creates a GitHub issue when any E2E job fails. @@ -43,12 +46,12 @@ on: token-rotation-e2e, sandbox-survival-e2e, issue-2478-crash-loop-recovery-e2e, hermes-e2e, skip-permissions-e2e, sandbox-operations-e2e, inference-routing-e2e, network-policy-e2e, - deployment-services-e2e, diagnostics-e2e, snapshot-commands-e2e, - shields-config-e2e, rebuild-openclaw-e2e, upgrade-stale-sandbox-e2e, - rebuild-hermes-e2e, double-onboard-e2e, onboard-repair-e2e, - onboard-resume-e2e, runtime-overrides-e2e, + deployment-services-e2e, diagnostics-e2e, credential-migration-e2e, + snapshot-commands-e2e, shields-config-e2e, rebuild-openclaw-e2e, + upgrade-stale-sandbox-e2e, rebuild-hermes-e2e, double-onboard-e2e, + onboard-repair-e2e, onboard-resume-e2e, runtime-overrides-e2e, credential-sanitization-e2e, telegram-injection-e2e, - overlayfs-autofix-e2e, gpu-e2e + overlayfs-autofix-e2e, gpu-e2e, gpu-double-onboard-e2e required: false type: string default: "" @@ -526,6 +529,42 @@ jobs: path: test-diagnostics-*.log if-no-files-found: ignore + # ── Credential migration E2E ──────────────────────────────── + # Validates the host-side credential storage hardening: pre-fix plaintext + # credentials.json is migrated into the OpenShell gateway during onboard, + # securely zero-filled and unlinked, non-allowlisted keys from a tampered + # file are not honored, and a planted symlink at the credentials path is + # link-only-unlinked without touching its target. + credential-migration-e2e: + if: >- + github.repository == 'NVIDIA/NemoClaw' && + (github.event_name != 'workflow_dispatch' || + inputs.jobs == '' || + contains(format(',{0},', inputs.jobs), ',credential-migration-e2e,')) + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Run credential migration E2E test + env: + NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} + NEMOCLAW_NON_INTERACTIVE: "1" + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + NEMOCLAW_SANDBOX_NAME: "e2e-cred-migration" + NEMOCLAW_RECREATE_SANDBOX: "1" + GITHUB_TOKEN: ${{ github.token }} + run: bash test/e2e/test-credential-migration.sh + + - name: Upload install log on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: install-log-credential-migration + path: /tmp/nemoclaw-e2e-install.log + if-no-files-found: ignore + # ── Snapshot commands E2E ──────────────────────────────────── # Validates snapshot create/list/restore lifecycle: create a snapshot, # list it, delete state, restore from snapshot, verify state recovered. @@ -689,8 +728,6 @@ jobs: path: /tmp/nemoclaw-e2e-install.log if-no-files-found: ignore - # ── Docker 26+ overlayfs nested-mount auto-fix (#2481) ────── - # TEMPORARY: validates the auto-fix in src/lib/cluster-image-patch.ts. # ── Double Onboard / Lifecycle Recovery E2E ────────────────── double-onboard-e2e: if: >- @@ -924,6 +961,8 @@ jobs: # Remove this job — and the matching notify-on-failure entry — in the # same PR that deletes cluster-image-patch.ts when the OpenShell # roadmap migration off k3s (NVIDIA/OpenShell#873) lands. + # ── Docker 26+ overlayfs nested-mount auto-fix (#2481) ────── + # TEMPORARY: validates the auto-fix in src/lib/cluster-image-patch.ts. overlayfs-autofix-e2e: if: >- github.repository == 'NVIDIA/NemoClaw' && @@ -1085,6 +1124,7 @@ jobs: network-policy-e2e, deployment-services-e2e, diagnostics-e2e, + credential-migration-e2e, snapshot-commands-e2e, shields-config-e2e, rebuild-openclaw-e2e, diff --git a/docs/get-started/quickstart.md b/docs/get-started/quickstart.md index 49ab8669967..b6345ddf11e 100644 --- a/docs/get-started/quickstart.md +++ b/docs/get-started/quickstart.md @@ -215,7 +215,7 @@ After you enter the sandbox name, the wizard prints a review summary and asks fo ────────────────────────────────────────────────── Provider: nvidia-api Model: nvidia/nemotron-3-super-120b-a12b - API key: NVIDIA_API_KEY (stored in ~/.nemoclaw/credentials.json) + API key: NVIDIA_API_KEY (registered with the OpenShell gateway) Web search: disabled Messaging: none Sandbox name: my-assistant @@ -331,15 +331,15 @@ Refer to [Switch inference providers](../inference/switch-inference-providers.md ### Reset a Stored Credential -If an API key was entered incorrectly during onboarding, clear the stored value and re-enter it on the next onboard run: +If a provider credential was entered incorrectly during onboarding, clear the gateway-registered value and re-enter it on the next onboard run: ```console -$ nemoclaw credentials list # see which keys are stored -$ nemoclaw credentials reset # clear a single key, for example NVIDIA_API_KEY -$ nemoclaw onboard # re-run to re-enter the cleared key +$ nemoclaw credentials list # see which providers are registered +$ nemoclaw credentials reset # clear a single provider, for example nvidia-prod +$ nemoclaw onboard # re-run to re-enter the cleared provider ``` -The credentials command is documented in full at [`nemoclaw credentials reset `](../reference/commands.md#nemoclaw-credentials-reset-key). +The credentials command is documented in full at [`nemoclaw credentials reset `](../reference/commands.md#nemoclaw-credentials-reset-provider). ### Rebuild a Sandbox While Preserving Workspace State diff --git a/docs/reference/architecture.md b/docs/reference/architecture.md index c240a9c9d0d..ca2a41803fc 100644 --- a/docs/reference/architecture.md +++ b/docs/reference/architecture.md @@ -243,13 +243,18 @@ Agent (sandbox) ──▶ OpenShell gateway ──▶ NVIDIA Endpoint (build Refer to [Inference Options](../inference/inference-options.md) for provider configuration details. +## Provider Credential Storage + +Provider credentials live in the OpenShell gateway store, not on the host filesystem. +NemoClaw never writes them to host disk; the OpenShell L7 proxy injects values at egress. +See [Credential Storage](../security/credential-storage.md) for the inspection, rotation, and migration flow. + ## Host-Side State and Config -NemoClaw keeps its operator-facing state on the host rather than inside the sandbox. +NemoClaw keeps non-secret operator-facing state on the host rather than inside the sandbox. | Path | Purpose | |---|---| -| `~/.nemoclaw/credentials.json` | Provider credentials saved during onboarding. Stored as plaintext JSON protected by local filesystem permissions; see [Credential Storage](../security/credential-storage.md). | | `~/.nemoclaw/sandboxes.json` | Registered sandbox metadata, including the default sandbox selection. | | `~/.openclaw/openclaw.json` | Host OpenClaw configuration that NemoClaw snapshots or restores during migration flows. | diff --git a/docs/reference/commands.md b/docs/reference/commands.md index d32c9d744ed..fc58aef3d4d 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -16,7 +16,7 @@ status: published --- @@ -81,7 +81,7 @@ $ NEMOCLAW_SINGLE_SESSION=1 curl -fsSL https://www.nvidia.com/nemoclaw.sh | bash The wizard prompts for a provider first, then collects the provider credential if needed. Supported non-experimental choices include NVIDIA Endpoints, OpenAI, Anthropic, Google Gemini, and compatible OpenAI or Anthropic endpoints. -Credentials are stored in `~/.nemoclaw/credentials.json`. For file permissions, plaintext storage behavior, and hardening guidance, see [Credential Storage](../security/credential-storage.md). +Credentials are registered with the OpenShell gateway and never persisted to host disk. See [Credential Storage](../security/credential-storage.md) for details on inspection, rotation, and migration from earlier releases. The legacy `nemoclaw setup` command is deprecated; use `nemoclaw onboard` instead. After provider selection, the wizard prompts for a **policy tier** that controls the default set of network policy presets applied to the sandbox. @@ -421,7 +421,7 @@ $ nemoclaw my-assistant channels list ### `nemoclaw channels add ` Store credentials for a messaging channel (`telegram`, `discord`, or `slack`) and rebuild the sandbox so the image picks up the new channel. -The command prompts for any missing token, persists it under `~/.nemoclaw/credentials.json`, then asks whether to rebuild immediately. +The command prompts for any missing token, registers it with the OpenShell gateway, then asks whether to rebuild immediately. Running `add` for an already-configured channel simply overwrites the stored tokens — the operation is idempotent. ```console @@ -454,7 +454,9 @@ Host-side removal is the supported path because `/sandbox/.openclaw/openclaw.jso ### `nemoclaw channels stop ` -Pause a single messaging bridge (`telegram`, `discord`, or `slack`) without clearing its credentials. The channel is marked disabled in the per-sandbox registry, and the sandbox is rebuilt so the onboard step skips registering the bridge with the gateway. Credentials stay in `~/.nemoclaw/credentials.json`, so a later `channels start` brings the bridge back without re-entering tokens. +Pause a single messaging bridge (`telegram`, `discord`, or `slack`) without clearing its credentials. +The channel is marked disabled in the per-sandbox registry, and the sandbox is rebuilt so the onboard step skips registering the bridge with the gateway. +The provider stays registered with the OpenShell gateway, so a later `channels start` brings the bridge back without re-entering tokens. ```console $ nemoclaw my-assistant channels stop telegram @@ -711,20 +713,21 @@ If `--output` is set and the tarball cannot be written (for example, the destina ### `nemoclaw credentials list` -List the names of all credentials stored in `~/.nemoclaw/credentials.json`. +List the provider credentials registered with the OpenShell gateway. Values are not printed. ```console $ nemoclaw credentials list ``` -### `nemoclaw credentials reset ` +### `nemoclaw credentials reset ` -Remove a stored credential by name. -After removal, re-running `nemoclaw onboard` re-prompts for that key. +Remove a provider credential from the OpenShell gateway by provider name. +After removal, re-running `nemoclaw onboard` re-prompts for that provider's credential. +Run `nemoclaw credentials list` first if you are not sure of the provider name. ```console -$ nemoclaw credentials reset NVIDIA_API_KEY +$ nemoclaw credentials reset nvidia-prod ``` | Flag | Description | diff --git a/docs/reference/troubleshooting.md b/docs/reference/troubleshooting.md index 15ca312f60c..a3459a3029b 100644 --- a/docs/reference/troubleshooting.md +++ b/docs/reference/troubleshooting.md @@ -598,7 +598,7 @@ $ nemoclaw channels add $ nemoclaw channels remove ``` -`channels add` stores credentials under `~/.nemoclaw/credentials.json` and `channels remove` clears them; both offer to rebuild the sandbox so the image reflects the new channel set. +`channels add` registers credentials with the OpenShell gateway and `channels remove` clears them; both offer to rebuild the sandbox so the image reflects the new channel set. In non-interactive mode (`NEMOCLAW_NON_INTERACTIVE=1`), the commands stage the change and leave the rebuild to a follow-up `nemoclaw rebuild`. ### `nemoclaw config set` refuses a key that does not currently exist diff --git a/docs/security/credential-storage.md b/docs/security/credential-storage.md index f27d5023ee8..24a07bf1b87 100644 --- a/docs/security/credential-storage.md +++ b/docs/security/credential-storage.md @@ -3,11 +3,11 @@ title: page: "NemoClaw Credential Storage" nav: "Credential Storage" description: - main: "Learn where NemoClaw stores credentials, what filesystem protections it applies, and how to secure or rotate stored secrets." - agent: "Covers where NemoClaw stores provider credentials, the file permissions applied, and the trade-offs of plaintext local storage. Use when reviewing how credentials are handled, locating a specific credential file, or assessing the risk of the unencrypted-at-rest default." -keywords: ["nemoclaw credential storage", "credentials.json", "api key security"] + main: "Learn where NemoClaw stores credentials, what protections it applies, and how to inspect or rotate stored secrets." + agent: "Covers where NemoClaw stores provider credentials, why nothing is persisted to host disk, and how the OpenShell gateway acts as the single system of record. Use when reviewing how credentials are handled, locating a stored credential, or assessing the storage threat model." +keywords: ["nemoclaw credential storage", "openshell provider", "api key security"] topics: ["generative_ai", "ai_agents"] -tags: ["security", "credentials", "filesystem", "secrets"] +tags: ["security", "credentials", "openshell", "secrets"] content: type: reference difficulty: technical_beginner @@ -22,112 +22,107 @@ status: published # Credential Storage -NemoClaw stores operator-provided host-side credentials under `~/.nemoclaw/`. -These credentials are used during onboarding and host-side lifecycle operations. -They are not encrypted at rest by NemoClaw. -Instead, NemoClaw relies on local filesystem ownership and Unix permissions to limit access. +NemoClaw does not persist provider credentials to host disk. +The OpenShell gateway is the only system of record for stored credentials. -## Location and Permissions +When you provide a provider credential — interactively during `nemoclaw onboard` or via an environment variable — NemoClaw holds the value in memory only long enough to register it with the OpenShell gateway through `openshell provider create` or `openshell provider update`. +The gateway stores the credential and the OpenShell L7 proxy substitutes it into outbound requests at egress, so sandboxed agents see placeholders instead of the raw secret. -By default, NemoClaw stores credentials in: +`nemoclaw config rotate-token` is a separate flow that rotates a sandbox-side OpenClaw auth token; it is not a provider-credential upsert and is documented under [Commands](../reference/commands.md). -```text -~/.nemoclaw/credentials.json -``` +## Where Credentials Live -When NemoClaw creates this state directory, it uses owner-only permissions: +Provider credentials live in the OpenShell gateway store. +List what is registered with: -- `~/.nemoclaw/` is created with mode `0700` -- `~/.nemoclaw/credentials.json` is written with mode `0600` +```console +$ openshell provider list +``` -That means only the local account that owns the files should be able to read or modify them. +Or, equivalently, through NemoClaw: -NemoClaw also refuses to use obviously unsafe `HOME` paths such as `/tmp`, `/var/tmp`, `/dev/shm`, or `/` for credential storage. -If `HOME` points to one of those locations, onboarding exits with an error instead of writing secrets there. +```console +$ nemoclaw credentials list +``` -## Plaintext Storage Warning +Both surface the provider names that the gateway holds credentials for. The values themselves cannot be read back from the CLI; this is a deliberate property of OpenShell. -The credential file is plaintext JSON. -NemoClaw does **not** currently encrypt the file or integrate with the host operating system keychain. +NemoClaw still keeps non-secret operational state under `~/.nemoclaw/` (such as the sandbox registry). +That directory is created with mode `0700` and contains no credential material. -A typical file looks like this: +## Environment Variables Take Precedence -```json -{ - "NVIDIA_API_KEY": "nvapi-...", - "GITHUB_TOKEN": "ghp_...", - "OPENAI_API_KEY": "sk-..." -} -``` +When a NemoClaw command needs a credential value during a single run (for example to forward it to an `openshell provider` registration), it reads from `process.env` first. +This means you can: -Treat this file like any other local secret material. -Anyone who can read it can reuse those credentials with the upstream provider. +- Prefix any command with the credential to override the gateway-stored value: `NVIDIA_API_KEY=nvapi-... nemoclaw onboard` +- Use short-lived or rotated credentials in CI by exporting them once per pipeline run +- Avoid registering credentials in the gateway entirely if your environment supplies them -## Precedence and Scope +## Deploy Reads from Environment Only -When NemoClaw looks up a credential, it checks environment variables first. -If the corresponding environment variable is set, NemoClaw uses that value instead of the stored file. +`nemoclaw deploy` (which provisions a remote Brev box) cannot read secrets back from the gateway, so it requires every credential to be present in the host environment at invocation time. +A typical deploy invocation looks like: -This behavior is useful for: +```console +$ NVIDIA_API_KEY=nvapi-... \ + TELEGRAM_BOT_TOKEN=... \ + nemoclaw deploy my-instance +``` -- CI or automation where you do not want to persist secrets to disk -- temporary overrides during testing -- short-lived or rotated credentials +If a required credential is missing the deploy aborts before any remote work begins. -For interactive local use, `nemoclaw onboard` can save credentials into `~/.nemoclaw/credentials.json` so future runs do not prompt again. +## GitHub Tokens -## Security Recommendations +NemoClaw never persists `GITHUB_TOKEN` itself. +When a private repo requires authentication NemoClaw runs `gh auth token`, which returns whatever the GitHub CLI has stored — without caring about the storage backend. -Use the following practices to reduce the risk of credential exposure. +The GitHub CLI prefers an OS keychain when one is reachable: macOS Keychain on macOS, Windows Credential Manager on Windows, and Linux Secret Service (libsecret + a running D-Bus session) on Linux. +On hosts where no keychain is reachable (CI runners, headless launches, WSL without a session bus, macOS contexts where Keychain access is blocked, etc.) `gh auth login` falls back to a `gh`-managed file under `~/.config/gh/` with mode `0600`. +NemoClaw treats both backends identically: `gh auth token` returns the value, and NemoClaw stages it in `process.env` for the current run only. -1. Keep your home directory private and owned by your user account. -2. Exclude `~/.nemoclaw/` from cloud-sync folders, shared folders, and broad backup exports unless those systems are already approved for secret storage. -3. Prefer short-lived or low-scope provider credentials where the upstream service supports them. -4. Rotate keys after suspected exposure, machine transfer, or account changes. -5. Prefer environment variables for ephemeral automation instead of persisting long-lived secrets locally. -6. Do not copy `credentials.json` into container images, Git repositories, bug reports, or support bundles. +If `gh` is not installed or not logged in, NemoClaw prompts for a personal access token for that single run; the prompted value is held in process memory and is not written to host disk. +Run `gh auth login` if you want a persistent backing store (whichever one applies on your host) so future runs do not prompt. -## Inspect and Repair Permissions +## Migration From Earlier Releases -To inspect the current permissions: +Earlier NemoClaw releases stored credentials as plaintext JSON in `~/.nemoclaw/credentials.json` with mode `0600`. +On first `nemoclaw onboard` after upgrading, NemoClaw automatically: -```console -$ ls -ld ~/.nemoclaw ~/.nemoclaw/credentials.json -``` +1. Reads the legacy file. +2. Stages allowlisted credential values into `process.env` for the rest of the run. +3. Re-registers each value with the OpenShell gateway through the normal onboarding path. +4. Securely overwrites and deletes `~/.nemoclaw/credentials.json` only after every staged value has been verified as migrated to the gateway. -Expected output should show a private directory and file, for example: +You will see a one-line stderr notice the first time this happens. +Credential lookup paths such as rebuild also stage allowlisted legacy values so interrupted upgrades can keep working, but those staging-only paths do not delete the plaintext file because they cannot prove every legacy value was registered with the gateway. +If `~/.nemoclaw/credentials.json` remains after a rebuild or other credential lookup, run `nemoclaw onboard` to complete the verified gateway migration and cleanup. -```text -drwx------ ... ~/.nemoclaw --rw------- ... ~/.nemoclaw/credentials.json -``` +## Rotate or Remove a Stored Credential -If the permissions are broader than expected, tighten them: +The simplest way to replace a stored value is to rerun onboarding with the new value in your environment: ```console -$ chmod 700 ~/.nemoclaw -$ chmod 600 ~/.nemoclaw/credentials.json +$ NVIDIA_API_KEY=nvapi-new-value nemoclaw onboard ``` -## Rotate or Remove Stored Credentials - -The simplest way to replace a stored provider key is to rerun onboarding and provide the new value when prompted: +To remove a credential from the gateway entirely: ```console -$ nemoclaw onboard +$ nemoclaw credentials reset ``` -To remove the stored file entirely: +`` is the OpenShell provider name (run `nemoclaw credentials list` first if you are not sure). +On the next run NemoClaw prompts again unless the credential is supplied through the environment. -```console -$ rm -f ~/.nemoclaw/credentials.json -``` +## Security Recommendations -On the next run, NemoClaw prompts again unless the credential is supplied through the environment. +1. Prefer short-lived or low-scope provider credentials where the upstream service supports them. +2. Rotate keys after suspected exposure, machine transfer, or account changes. +3. Prefer environment variables for ephemeral automation rather than registering long-lived secrets in the gateway. +4. Do not copy any host-side NemoClaw state into container images, Git repositories, bug reports, or support bundles. Even though credentials no longer live on disk, the surrounding configuration may reveal which providers you have registered. +5. Keep your home directory private and owned by your user account. ## Related Files -Other NemoClaw host-side state also lives under `~/.nemoclaw/`, such as sandbox registry metadata. -These files are operational state, not provider secrets, but they should still remain in a user-owned home directory. - For the broader sandbox security model and operational trade-offs, see [Security Best Practices](./best-practices.md) and [Architecture](../reference/architecture.md). diff --git a/src/lib/config-io.ts b/src/lib/config-io.ts index fa7f3e3e4ac..2dfc8e7c8a1 100644 --- a/src/lib/config-io.ts +++ b/src/lib/config-io.ts @@ -102,9 +102,10 @@ export class ConfigPermissionError extends Error { * Reject a path if it — or any ancestor up to the user's home — is a symlink. * This prevents an attacker from planting e.g. ~/.nemoclaw as a symlink to an * attacker-controlled directory, which would cause credentials to be written - * to the wrong location. + * to the wrong location. Throws when a planted symlink is found; returns + * normally otherwise. */ -function rejectSymlinksOnPath(dirPath: string): void { +export function rejectSymlinksOnPath(dirPath: string): void { const home = process.env.HOME || os.homedir(); const resolved = path.resolve(dirPath); const resolvedHome = path.resolve(home); diff --git a/src/lib/credentials.ts b/src/lib/credentials.ts index 77d2b15cd61..391b328ad36 100644 --- a/src/lib/credentials.ts +++ b/src/lib/credentials.ts @@ -1,5 +1,12 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 +// +// Host-side credential helpers. +// +// The OpenShell gateway is the system of record for provider credentials. +// This module holds them only in the current process environment so they +// can be passed through to `openshell provider create/update --credential KEY` +// during onboarding. Nothing is written to disk. import { execFileSync } from "node:child_process"; import fs from "node:fs"; @@ -7,18 +14,51 @@ import os from "node:os"; import path from "node:path"; import readline from "node:readline"; -import { readConfigFile, writeConfigFile } from "./config-io"; +import { rejectSymlinksOnPath } from "./config-io"; import { isErrnoException } from "./errno"; const UNSAFE_HOME_PATHS = new Set(["/tmp", "/var/tmp", "/dev/shm", "/"]); type CredentialInput = string | null | undefined; +// Credential env keys NemoClaw knows how to round-trip. listCredentialKeys() +// projects the in-process env through this set; entries not in the set are +// invisible to `nemoclaw credentials list` even if exported. +// Exported so tests can import the same source-of-truth list and stay in +// sync without a second hand-maintained copy. +export const KNOWN_CREDENTIAL_ENV_KEYS: readonly string[] = [ + "NVIDIA_API_KEY", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "GEMINI_API_KEY", + "COMPATIBLE_API_KEY", + "COMPATIBLE_ANTHROPIC_API_KEY", + "BRAVE_API_KEY", + "GITHUB_TOKEN", + "TELEGRAM_BOT_TOKEN", + "ALLOWED_CHAT_IDS", + "DISCORD_BOT_TOKEN", + "SLACK_BOT_TOKEN", + "SLACK_APP_TOKEN", +]; + +// Hard upper bound on the legacy credentials.json size we are willing to +// read into memory. The largest realistic credential set NemoClaw has ever +// shipped is well under 1 KiB; the cap exists purely so an attacker who +// can write to ~/.nemoclaw/ cannot OOM the next onboard by planting a +// huge file. 1 MiB leaves plenty of headroom over any plausible mutation. +const LEGACY_CREDS_FILE_MAX_BYTES = 1 * 1024 * 1024; + +/** + * Resolve the user's home directory and reject obviously unsafe choices + * (e.g. `/tmp`, `/`) so we never use a world-readable path for state. + * Throws if `HOME` cannot be determined or resolves to an unsafe location. + */ export function resolveHomeDir(): string { const raw = process.env.HOME || os.homedir(); if (!raw) { throw new Error( - "Cannot determine safe home directory for credential storage. " + + "Cannot determine safe home directory. " + "Set the HOME environment variable to a user-owned directory.", ); } @@ -27,10 +67,10 @@ export function resolveHomeDir(): string { const real = fs.realpathSync(home); if (UNSAFE_HOME_PATHS.has(real)) { throw new Error( - "Cannot store credentials: HOME resolves to '" + + "Cannot use HOME='" + real + - "' which is world-readable. " + - "Set the HOME environment variable to a user-owned directory.", + "': resolves to a world-readable path. " + + "Set HOME to a user-owned directory.", ); } } catch (error) { @@ -43,10 +83,10 @@ export function resolveHomeDir(): string { } if (UNSAFE_HOME_PATHS.has(home)) { throw new Error( - "Cannot store credentials: HOME resolves to '" + + "Cannot use HOME='" + home + - "' which is world-readable. " + - "Set the HOME environment variable to a user-owned directory.", + "': resolves to a world-readable path. " + + "Set HOME to a user-owned directory.", ); } return home; @@ -54,78 +94,298 @@ export function resolveHomeDir(): string { let _cachedHome: string | null = null; let _credsDir: string | null = null; -let _credsFile: string | null = null; +let _legacyCredsFile: string | null = null; +/** Return `~/.nemoclaw`, resolving and validating `HOME` once per process. */ export function getCredsDir(): string { const home = resolveHomeDir(); if (_cachedHome !== home) { _cachedHome = home; _credsDir = path.join(home, ".nemoclaw"); - _credsFile = null; + _legacyCredsFile = null; } return _credsDir || path.join(home, ".nemoclaw"); } +/** + * Path of the pre-migration plaintext credentials file. Retained only so + * stageLegacyCredentialsToEnv() / removeLegacyCredentialsFile() can find it. + * New code must NOT write to this path; the gateway is the system of record. + */ export function getCredsFile(): string { const dir = getCredsDir(); - if (!_credsFile) _credsFile = path.join(dir, "credentials.json"); - return _credsFile; -} - -export function loadCredentials(): Record { - return readConfigFile>(getCredsFile(), {}); + if (!_legacyCredsFile) _legacyCredsFile = path.join(dir, "credentials.json"); + return _legacyCredsFile; } +/** Trim whitespace and strip CR characters that shells often append on paste. */ export function normalizeCredentialValue(value: CredentialInput): string { if (typeof value !== "string") return ""; return value.replace(/\r/g, "").trim(); } +/** + * Stage a credential for the current process. The OpenShell upsert that + * follows in onboarding (`openshell provider create/update --credential KEY`) + * reads the value from this env entry. Nothing is persisted to disk. + * An empty/whitespace value clears the env entry instead of staging blanks. + * + * NOTE for tests: this mutates `process.env` directly (not via vitest's + * `vi.stubEnv`), so callers that pollute the env in a unit test must + * clean up themselves — see `test/credentials.test.ts` for the + * `clearTrackedEnv` pattern. + */ export function saveCredential(key: string, value: CredentialInput): void { - const creds = loadCredentials(); - creds[key] = normalizeCredentialValue(value); - writeConfigFile(getCredsFile(), creds); + const normalized = normalizeCredentialValue(value); + if (normalized) { + process.env[key] = normalized; + } else { + delete process.env[key]; + } } +/** Return the staged value for `key` from the current process env, or null. */ export function getCredential(key: string): string | null { - const envValue = normalizeCredentialValue(process.env[key]); - if (envValue) return envValue; - const creds = loadCredentials(); - const value = normalizeCredentialValue(creds[key]); - return value || null; + const raw = process.env[key]; + if (!raw) return null; + const normalized = normalizeCredentialValue(raw); + return normalized || null; } /** - * Canonical entry point for provider credential resolution. - * Resolves from process.env or ~/.nemoclaw/credentials.json via getCredential(), - * and populates process.env so downstream code reading process.env directly - * sees the value. Returns the resolved value or null. + * Canonical entry point for provider credential resolution (PR #2306). + * Resolves the credential for `envName` from `process.env`, falling back + * to a one-time on-demand stage of any pre-fix `~/.nemoclaw/credentials.json`, + * and writes the resolved value back into `process.env` so downstream + * code that reads `process.env[envName]` directly sees it. * - * This replaces the dual-pattern of hydrateCredentialEnv() + process.env[key] - * checks. See #2306. + * Returns the resolved value, or `null` if neither env nor the legacy + * file produced one. + * + * Note: this used to read the credentials file directly via + * `loadCredentials()` after #2306 landed on main, but that path is + * incompatible with the env-only contract introduced for the + * credentials-gateway-only security fix. The legacy file is now + * accessed only through `stageLegacyCredentialsToEnv()`, which + * allowlists keys, refuses ancestor symlinks, fstats by descriptor, + * caps file size, and is gated by the per-key fill-only-if-missing + * guard inside the staging helper itself. */ export function resolveProviderCredential(envName: string): string | null { - const value = getCredential(envName); + let value = getCredential(envName); + if (!value) { + stageLegacyCredentialsToEnv(); + value = getCredential(envName); + } if (value) { process.env[envName] = value; } return value || null; } +/** Clear the staged credential from the current process env. */ export function deleteCredential(key: string): boolean { - const file = getCredsFile(); - if (!fs.existsSync(file)) return false; - const creds = loadCredentials(); - if (!Object.prototype.hasOwnProperty.call(creds, key)) return false; - delete creds[key]; - writeConfigFile(file, creds); + if (!(key in process.env)) return false; + delete process.env[key]; return true; } +/** + * Snapshot of credentials currently staged in `process.env`, projected + * through `KNOWN_CREDENTIAL_ENV_KEYS`. Unrelated env entries are not exposed. + */ +export function loadCredentials(): Record { + const result: Record = {}; + for (const key of KNOWN_CREDENTIAL_ENV_KEYS) { + const raw = process.env[key]; + if (!raw) continue; + const normalized = normalizeCredentialValue(raw); + if (normalized) result[key] = normalized; + } + return result; +} + +/** Sorted list of credential env-var names currently staged in this process. */ export function listCredentialKeys(): string[] { return Object.keys(loadCredentials()).sort(); } +/** + * Best-effort secure unlink: zero the file's bytes, fsync, then unlink. + * Refuses to follow symlinks (lstat + O_NOFOLLOW) so a planted symlink + * cannot redirect the zero-fill onto an unrelated file. Does not defeat + * copy-on-write filesystems or prior backup snapshots, but removes the + * cleartext from the typical ext4/HFS+/APFS-without-snapshot path that + * backup tools and same-user processes tend to read. + */ +function secureUnlink(filePath: string): void { + try { + const stat = fs.lstatSync(filePath); + if (stat.isSymbolicLink()) { + // The credentials path was a symlink; remove the link itself without + // touching whatever it pointed at. + fs.unlinkSync(filePath); + return; + } + if (!stat.isFile()) return; + if (stat.size > 0) { + const fd = fs.openSync(filePath, fs.constants.O_RDWR | fs.constants.O_NOFOLLOW); + try { + const chunkSize = Math.min(stat.size, 64 * 1024); + const zeros = Buffer.alloc(chunkSize); + let written = 0; + while (written < stat.size) { + const len = Math.min(chunkSize, stat.size - written); + fs.writeSync(fd, zeros, 0, len, written); + written += len; + } + fs.fsyncSync(fd); + } finally { + fs.closeSync(fd); + } + } + } catch { + // best effort + } + try { + fs.unlinkSync(filePath); + } catch { + // best effort + } +} + +/** + * Stage credential values from a pre-fix plaintext credentials.json into + * `process.env` (non-destructive). Restricted to `KNOWN_CREDENTIAL_ENV_KEYS` + * so a stale or tampered file cannot inject unrelated variables (`PATH`, + * `NODE_OPTIONS`, `OPENSHELL_GATEWAY`, etc.) into later child processes. + * + * The file is intentionally NOT removed here. The unlink runs only after + * onboarding successfully registers the credentials with the OpenShell + * gateway — see {@link removeLegacyCredentialsFile}. + * + * @returns Sorted list of credential keys that were staged, or `[]`. + */ +export function stageLegacyCredentialsToEnv(): string[] { + const legacyFile = getCredsFile(); + + // O_NOFOLLOW only protects the *final* path component. Walk every + // ancestor between HOME and ~/.nemoclaw and refuse if any of them is + // a symlink — otherwise a planted directory symlink at ~/.nemoclaw/ + // would redirect the read into an attacker-controlled location even + // though the credentials.json open itself looks safe. config-io's + // rejectSymlinksOnPath throws when a planted link is found; treat + // that the same as "no migratable file" and bail. + try { + rejectSymlinksOnPath(path.dirname(legacyFile)); + } catch (error) { + console.error( + ` Refusing to migrate legacy credentials: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return []; + } + + // Pin the file by descriptor before doing any checks. O_NOFOLLOW makes + // the open() itself fail when the final path component is a symlink, + // and fstat/read both target the same inode, so an attacker cannot + // swap the file between checks (TOCTOU) or redirect us through a + // symlink planted at the credentials path. + let fd: number; + try { + fd = fs.openSync(legacyFile, fs.constants.O_RDONLY | fs.constants.O_NOFOLLOW); + } catch { + return []; + } + + let raw: string; + try { + const stat = fs.fstatSync(fd); + if (!stat.isFile()) return []; + if (stat.size > LEGACY_CREDS_FILE_MAX_BYTES) { + console.error( + ` Refusing to migrate ${legacyFile}: file is ${String(stat.size)} bytes, ` + + `exceeding the ${String(LEGACY_CREDS_FILE_MAX_BYTES)}-byte sanity cap. ` + + `Inspect the file manually and remove it if it does not contain credentials.`, + ); + return []; + } + raw = fs.readFileSync(fd, "utf-8"); + } catch { + return []; + } finally { + try { + fs.closeSync(fd); + } catch { + /* fd already closed; ignore */ + } + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return []; + } + + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + return []; + } + + const allowed = new Set(KNOWN_CREDENTIAL_ENV_KEYS); + const staged: string[] = []; + for (const [key, value] of Object.entries(parsed as Record)) { + if (!allowed.has(key)) continue; + if (typeof value !== "string") continue; + const normalized = normalizeCredentialValue(value); + if (!normalized) continue; + // Defer to env values that were already set (e.g. by the user) so the + // file cannot silently override an explicit override. Use getCredential + // for the existence check so a blank or whitespace-only env entry — + // which `getCredential` normalizes to null — counts as unset and the + // legacy value is staged. Track only the keys we actually imported + // from the file — `staged.length > 0` is the signal callers use to + // decide it is safe to delete the legacy file. + if (!getCredential(key)) { + process.env[key] = normalized; + staged.push(key); + } + } + return staged.sort(); +} + +/** + * Securely remove the legacy plaintext credentials.json. Call this only + * after the gateway has accepted the migrated values, so an interrupted or + * failed onboard cannot leave the user with no copy of their credentials. + * + * `secureUnlink` is itself missing-file-tolerant and uses `lstatSync`, so + * we deliberately do NOT pre-check with `existsSync` — that would follow a + * planted symlink and skip cleanup of a dangling link. Walk the ancestor + * directories between HOME and ~/.nemoclaw first so a planted directory + * symlink can't redirect the zero-fill into an unrelated tree. + */ +export function removeLegacyCredentialsFile(): void { + const legacyFile = getCredsFile(); + try { + rejectSymlinksOnPath(path.dirname(legacyFile)); + } catch (error) { + console.error( + ` Refusing to remove legacy credentials: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return; + } + secureUnlink(legacyFile); +} + +/** + * Read a secret value from a TTY without echoing typed characters + * (asterisks are written instead). Resolves to the trimmed answer or + * rejects with `code: "SIGINT"` on Ctrl-C. + */ export function promptSecret(question: string): Promise { return new Promise((resolve, reject) => { const input = process.stdin; @@ -213,6 +473,11 @@ export function promptSecret(question: string): Promise { }); } +/** + * Prompt the user on stderr and resolve to their trimmed answer. Pass + * `{ secret: true }` to mask input on a TTY (falls back to plain readline + * when stdin/stderr is non-interactive, e.g. in CI). + */ export function prompt(question: string, opts: { secret?: boolean } = {}): Promise { return new Promise((resolve, reject) => { const silent = opts.secret === true && process.stdin.isTTY && process.stderr.isTTY; @@ -269,6 +534,12 @@ export function prompt(question: string, opts: { secret?: boolean } = {}): Promi }); } +/** + * Ensure `NVIDIA_API_KEY` is staged for this process. Returns immediately + * if it is already in env, otherwise prompts interactively (validating + * the `nvapi-` prefix) and stages the result. Onboarding registers the + * value with the OpenShell gateway later in the flow. + */ export async function ensureApiKey(): Promise { let key = getCredential("NVIDIA_API_KEY"); if (key) { @@ -306,10 +577,16 @@ export async function ensureApiKey(): Promise { saveCredential("NVIDIA_API_KEY", key); process.env.NVIDIA_API_KEY = key; console.log(""); - console.log(" Key saved to ~/.nemoclaw/credentials.json (mode 600)"); + console.log(" Key staged for the OpenShell gateway. It is held in process memory only;"); + console.log(" onboarding registers it with the gateway and nothing is written to disk."); console.log(""); } +/** + * Return true if `/` is a private GitHub repository, using + * `gh api`. Returns false on any failure (no `gh`, not authenticated, + * network error) — callers must treat the result as a hint, not a proof. + */ export function isRepoPrivate(repo: string): boolean { try { const json = execFileSync("gh", ["api", `repos/${repo}`, "--jq", ".private"], { @@ -322,6 +599,14 @@ export function isRepoPrivate(repo: string): boolean { } } +/** + * Ensure `GITHUB_TOKEN` is staged for this process when a private repo + * needs it. Tries `gh auth token` first (which returns whatever the + * GitHub CLI has stored — system keychain when reachable, otherwise a + * gh-managed file); falls back to a session-only PAT prompt if `gh` is + * unavailable or not logged in. The token is never persisted to host + * disk by NemoClaw itself. + */ export async function ensureGithubToken(): Promise { let token = getCredential("GITHUB_TOKEN"); if (token) { @@ -329,6 +614,7 @@ export async function ensureGithubToken(): Promise { return; } + // Preferred path: gh CLI keeps tokens in the OS keychain. try { token = execFileSync("gh", ["auth", "token"], { encoding: "utf-8", @@ -339,16 +625,19 @@ export async function ensureGithubToken(): Promise { return; } } catch { - /* ignored */ + /* gh not available or not logged in */ } console.log(""); - console.log(" ┌──────────────────────────────────────────────────┐"); - console.log(" │ GitHub token required (private repo detected) │"); - console.log(" │ │"); - console.log(" │ Option A: gh auth login (if you have gh CLI) │"); - console.log(" │ Option B: Paste a PAT with read:packages scope │"); - console.log(" └──────────────────────────────────────────────────┘"); + console.log(" ┌────────────────────────────────────────────────────────────────┐"); + console.log(" │ GitHub token required (private repo detected) │"); + console.log(" │ │"); + console.log(" │ Recommended: run 'gh auth login'. NemoClaw picks up whatever │"); + console.log(" │ the GitHub CLI stores (system keychain when reachable; a │"); + console.log(" │ gh-managed file otherwise). │"); + console.log(" │ │"); + console.log(" │ Otherwise, paste a PAT below for this run only. │"); + console.log(" └────────────────────────────────────────────────────────────────┘"); console.log(""); token = await prompt(" GitHub Token: ", { secret: true }); @@ -361,6 +650,8 @@ export async function ensureGithubToken(): Promise { saveCredential("GITHUB_TOKEN", token); process.env.GITHUB_TOKEN = token; console.log(""); - console.log(" Token saved to ~/.nemoclaw/credentials.json (mode 600)"); + console.log(" Token loaded for this session only. Run 'gh auth login' to let"); + console.log(" the GitHub CLI persist it (system keychain when reachable;"); + console.log(" gh-managed file otherwise) so future runs do not prompt."); console.log(""); } diff --git a/src/lib/onboard-session.ts b/src/lib/onboard-session.ts index cd6bdc948f7..f5597965da8 100644 --- a/src/lib/onboard-session.ts +++ b/src/lib/onboard-session.ts @@ -77,6 +77,18 @@ export interface Session { webSearchConfig: WebSearchConfig | null; policyPresets: string[] | null; messagingChannels: string[] | null; + // SHA-256 hex digest of every legacy credential value successfully + // written to the OpenShell gateway during this onboard session, keyed by + // env-name. Persisted across process restarts so a `--resume` run that + // skips already-completed upserts still knows the migration completed + // earlier and can safely remove ~/.nemoclaw/credentials.json on the + // final completeSession. Storing the hash (not just the env-name) lets + // us detect when the legacy file value was edited between runs, when + // the gateway provider was reset out-of-band, or when an unrelated + // session is found on disk — in any of those cases the in-memory + // migrated set is NOT seeded from the persisted record, so the cleanup + // gate keeps the file until the *current* value is actually re-migrated. + migratedLegacyValueHashes: Record | null; metadata: SessionMetadata; steps: Record; } @@ -107,6 +119,7 @@ export interface SessionUpdates { webSearchConfig?: WebSearchConfig | null; policyPresets?: string[]; messagingChannels?: string[]; + migratedLegacyValueHashes?: Record; metadata?: { gatewayName?: string; fromDockerfile?: string | null }; } @@ -172,6 +185,17 @@ function readStringArray(value: SessionJsonValue | undefined): string[] | null { return value.filter((entry): entry is string => typeof entry === "string"); } +function readStringRecord( + value: SessionJsonValue | undefined, +): Record | null { + if (!isObject(value)) return null; + const result: Record = {}; + for (const [k, v] of Object.entries(value)) { + if (typeof k === "string" && typeof v === "string") result[k] = v; + } + return result; +} + function isStepStatus(value: string): value is StepStatus { return VALID_STEP_STATES.has(value); } @@ -261,6 +285,9 @@ export function createSession(overrides: Partial = {}): Session { overrides.webSearchConfig?.fetchEnabled === true ? { fetchEnabled: true } : null, policyPresets: readStringArray(overrides.policyPresets), messagingChannels: readStringArray(overrides.messagingChannels), + migratedLegacyValueHashes: overrides.migratedLegacyValueHashes + ? readStringRecord(overrides.migratedLegacyValueHashes) + : null, metadata: { gatewayName: overrides.metadata?.gatewayName ?? "nemoclaw", fromDockerfile: overrides.metadata?.fromDockerfile ?? null, @@ -292,6 +319,7 @@ export function normalizeSession(data: Session | SessionJsonValue | undefined): webSearchConfig: parseWebSearchConfig(data.webSearchConfig), policyPresets: readStringArray(data.policyPresets), messagingChannels: readStringArray(data.messagingChannels), + migratedLegacyValueHashes: readStringRecord(data.migratedLegacyValueHashes), lastStepStarted: readString(data.lastStepStarted), lastCompletedStep: readString(data.lastCompletedStep), failure: sanitizeFailure(isObject(data.failure) ? data.failure : null), @@ -598,6 +626,13 @@ export function filterSafeUpdates(updates: SessionUpdates): Partial { if (Array.isArray(updates.messagingChannels)) { safe.messagingChannels = updates.messagingChannels.filter((value) => typeof value === "string"); } + if (isObject(updates.migratedLegacyValueHashes)) { + const cleaned: Record = {}; + for (const [k, v] of Object.entries(updates.migratedLegacyValueHashes)) { + if (typeof k === "string" && typeof v === "string") cleaned[k] = v; + } + safe.migratedLegacyValueHashes = cleaned; + } if (isObject(updates.metadata) && typeof updates.metadata.gatewayName === "string") { safe.metadata = { gatewayName: updates.metadata.gatewayName, diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index f7e534be19a..037879ba6c9 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -146,9 +146,11 @@ const { prompt, ensureApiKey, getCredential, + stageLegacyCredentialsToEnv, + removeLegacyCredentialsFile, normalizeCredentialValue, - saveCredential, resolveProviderCredential, + saveCredential, } = credentials; const registry: typeof import("./registry") = require("./registry"); const nim: typeof import("./nim") = require("./nim"); @@ -689,8 +691,24 @@ const { parsePolicyPresetEnv, } = urlUtils; +/** + * Resolve a credential into `process.env[envName]` so subsequent gateway + * upserts can read it via `--credential `. Idempotently stages any + * pre-fix plaintext credentials.json (non-destructively) so callers that + * reach a credential check from outside the onboard entry point — such as + * rebuild preflight — can still find legacy values. The file itself is + * removed only after a full successful onboard, so an interrupted run can + * be retried without losing the user's only copy. + * + * @param envName Credential env variable name, e.g. `NVIDIA_API_KEY`. + * @returns The resolved value, or `null` if `envName` is empty/unstaged. + */ function hydrateCredentialEnv(envName: string | null | undefined): string | null { if (!envName) return null; + // Thin wrapper for back-compat. resolveProviderCredential() (introduced + // by PR #2306 as the canonical entry point) now performs the staging + // dance internally — env first, then a one-time on-demand legacy stage, + // then write back into process.env for downstream code. return resolveProviderCredential(envName); } @@ -759,7 +777,7 @@ async function replaceNamedCredential( saveCredential(envName, key); process.env[envName] = key; console.log(""); - console.log(` Key saved to ~/.nemoclaw/credentials.json (mode 600)`); + console.log(` ${envName} staged. Onboarding will register it with the OpenShell gateway.`); console.log(""); return key; } @@ -858,8 +876,92 @@ async function promptValidationRecovery( // Provider CRUD — thin wrappers that inject runOpenshell to avoid circular deps. const { buildProviderArgs } = onboardProviders; + +// Snapshot of legacy {env-key → value} pairs that stageLegacyCredentialsToEnv() +// imported from ~/.nemoclaw/credentials.json at the start of this run. +// Captured by the onboard() entry point; consulted by the upsertProvider / +// upsertMessagingProviders wrappers below to decide whether a successful +// gateway upsert actually migrated the *legacy* value (vs. e.g. a vllm/ollama +// branch that upserts a placeholder under the same env-key name). +const stagedLegacyValues: Map = new Map(); + +// Env-keys whose successful gateway upsert actually used the staged legacy +// value. Seeded from the persisted onboard session at the start of every +// run so a `--resume` invocation that skips already-completed upserts still +// remembers the migrations the prior attempt committed. The post-onboard +// legacy-file cleanup is gated on `stagedLegacyKeys ⊆ migratedLegacyKeys` +// so picking a local inference provider, disabling a preselected messaging +// channel, or any other path that upserts a different value under the same +// env-key name leaves the file alone instead of stranding the user's only +// copy. +const migratedLegacyKeys: Set = new Set(); + +// SHA-256 hex digest of `value`. Used to fingerprint migrated legacy +// secrets in the persisted onboard session so a later `--resume` can +// detect when the legacy file value was edited between runs (or another +// session is on disk with stale entries) and refuse to inherit a stale +// "migrated" mark. +function legacyValueHash(value: string): string { + return crypto.createHash("sha256").update(value).digest("hex"); +} + +// Mirror the in-memory `migratedLegacyKeys` set into the persisted onboard +// session along with each entry's value hash. `--resume` invocations that +// skip the upsert wrappers entirely use this to inherit migration state +// from the previous attempt — but only when the staged value at restore +// time still hashes to the same digest, so an edit to the legacy file or +// an out-of-band gateway reset cannot satisfy the cleanup gate. +function persistMigratedLegacyKeys(): void { + try { + const hashes: Record = {}; + for (const key of migratedLegacyKeys) { + const stagedValue = stagedLegacyValues.get(key); + if (stagedValue !== undefined) { + hashes[key] = legacyValueHash(stagedValue); + } + } + onboardSession.updateSession((current: Session) => { + current.migratedLegacyValueHashes = hashes; + return current; + }); + } catch { + // updateSession can throw if the session file isn't yet writable + // (e.g. very early in the run before lockless state is established). + // The cleanup gate in this same process still consults the in-memory + // set, so a missed write only matters if THIS run later crashes and + // a future --resume needs the persisted value. Best effort. + } +} + function upsertProvider(name: string, type: string, credentialEnv: string, baseUrl: string | null, env: NodeJS.ProcessEnv = {}) { - return onboardProviders.upsertProvider(name, type, credentialEnv, baseUrl, env, runOpenshell); + const result = onboardProviders.upsertProvider(name, type, credentialEnv, baseUrl, env, runOpenshell); + if (result.ok && credentialEnv) { + const stagedValue = stagedLegacyValues.get(credentialEnv); + if (stagedValue !== undefined) { + // openshell receives `--credential ` and reads the value from the + // `env` block passed here, falling back to the inherited process.env. + // Use getCredential() for the env-fallback branch (per the + // no-direct-credential-env eslint rule from PR #2306) — it mirrors + // openshell's resolution order while the staging contract has + // already populated the same value into process.env. + const upsertedValue = env[credentialEnv] ?? getCredential(credentialEnv); + if (upsertedValue === stagedValue) { + // The gateway received the staged legacy value verbatim — count + // this key as migrated. + migratedLegacyKeys.add(credentialEnv); + } else { + // A later upsert under the same env-key wrote a different value + // (e.g. a retry-loop after validation failure replaced the legacy + // key with a freshly entered one, or a placeholder like "dummy" + // for vllm-local). The gateway no longer holds the staged legacy + // value under this env-key, so withdraw the migration mark — the + // cleanup gate must keep the legacy file intact. + migratedLegacyKeys.delete(credentialEnv); + } + persistMigratedLegacyKeys(); + } + } + return result; } type MessagingTokenDef = { name: string; envKey: string; token: string | null }; @@ -878,7 +980,30 @@ type SelectionDrift = { }; function upsertMessagingProviders(tokenDefs: MessagingTokenDef[]) { - return onboardProviders.upsertMessagingProviders(tokenDefs, runOpenshell); + const upserted = onboardProviders.upsertMessagingProviders(tokenDefs, runOpenshell); + // upsertMessagingProviders process.exits on failure, so reaching this + // point means every entry in tokenDefs that had a token was registered. + // Mark migrated only when the registered token equals the staged legacy + // value — a token rotated since staging (or a fresh prompt) is not a + // legacy migration even if it happens to use the same env-key name. + // Mirror upsertProvider's withdrawal logic so a later messaging upsert + // that replaces the legacy value with something else cannot leave the + // mark stuck on. + let mutated = false; + for (const def of tokenDefs) { + if (!def.token || !def.envKey) continue; + const stagedValue = stagedLegacyValues.get(def.envKey); + if (stagedValue === undefined) continue; + if (def.token === stagedValue) { + migratedLegacyKeys.add(def.envKey); + mutated = true; + } else { + migratedLegacyKeys.delete(def.envKey); + mutated = true; + } + } + if (mutated) persistMigratedLegacyKeys(); + return upserted; } function providerExistsInGateway(name: string) { return onboardProviders.providerExistsInGateway(name, runOpenshell); @@ -3174,7 +3299,7 @@ function formatOnboardConfigSummary({ const webSearch = webSearchConfig && webSearchConfig.fetchEnabled === true ? "enabled" : "disabled"; const apiKeyLine = credentialEnv - ? ` API key: ${credentialEnv} (stored in ~/.nemoclaw/credentials.json)` + ? ` API key: ${credentialEnv} (staged for OpenShell gateway registration)` : ` API key: (not required for ${provider ?? "this provider"})`; const noteLines = (Array.isArray(notes) ? notes : []) .filter((n) => typeof n === "string" && n.length > 0) @@ -4251,7 +4376,7 @@ async function setupNim(gpu: ReturnType): Promise<{ } } - // Hydrate from saved credentials (~/.nemoclaw/credentials.json) + // Hydrate from credential env vars set earlier in this process // before checking env, so rebuild and other non-interactive callers // can resolve keys stored during the original interactive onboard. // See #2273. @@ -6820,6 +6945,47 @@ async function onboard(opts: OnboardOptions = {}): Promise { process.exit(1); } + // Stage any pre-fix plaintext credentials.json into process.env so the + // provider upserts later in this run can pick the values up. The file is + // NOT removed here — the secure unlink runs only after onboarding + // completes successfully and only when every staged value was actually + // pushed to the gateway in this run. + stagedLegacyValues.clear(); + migratedLegacyKeys.clear(); + + const stagedLegacyKeys = stageLegacyCredentialsToEnv(); + for (const key of stagedLegacyKeys) { + const value = process.env[key]; + if (value) stagedLegacyValues.set(key, value); + } + + // Only carry forward migration state across processes when the user is + // explicitly continuing the same attempt via `--resume`. Even then, + // validate each persisted entry against the *current* staged value: if + // the legacy file was edited between runs (so the staged secret no + // longer matches what the gateway holds), the hash mismatch drops that + // key from migratedLegacyKeys and the cleanup gate forces a fresh + // upsert before the file can be removed. A fresh / non-resume run + // ignores prior persisted state entirely so a stale or unrelated + // session record cannot satisfy the cleanup gate. + if (resume) { + const previousSession = onboardSession.loadSession(); + const persistedHashes = previousSession?.migratedLegacyValueHashes ?? {}; + for (const [key, hash] of Object.entries(persistedHashes)) { + if (typeof key !== "string" || typeof hash !== "string") continue; + const currentValue = stagedLegacyValues.get(key); + if (currentValue === undefined) continue; + if (legacyValueHash(currentValue) !== hash) continue; + migratedLegacyKeys.add(key); + } + } + + if (stagedLegacyKeys.length > 0) { + console.error( + ` Staged ${String(stagedLegacyKeys.length)} legacy credential(s) for migration to the OpenShell gateway.`, + ); + } + let lockReleased = false; const releaseOnboardLock = () => { if (lockReleased) return; @@ -7086,9 +7252,9 @@ async function onboard(opts: OnboardOptions = {}): Promise { .toLowerCase(); if (answer === "n" || answer === "no") { console.log(" Aborted. Re-run `nemoclaw onboard` to start over."); - console.log(" Credentials entered so far are stored in ~/.nemoclaw/credentials.json —"); + console.log(" Credentials entered so far were only staged in memory for this run."); console.log( - " clear them with `nemoclaw credentials reset ` if you no longer want them.", + " No new gateway credential was registered because onboarding stopped here.", ); process.exit(0); } @@ -7314,6 +7480,30 @@ async function onboard(opts: OnboardOptions = {}): Promise { onboardSession.completeSession(toSessionUpdates({ sandboxName, provider, model })); completed = true; + // Onboarding finished successfully. Delete the legacy plaintext + // credentials.json only when every staged *value* was actually pushed + // to the gateway in this run. A successful upsert under the same + // env-key name with a different value (e.g. vllm-local upserting + // `OPENAI_API_KEY: "dummy"` while the legacy file held a real + // `sk-…` cloud key) does not count as a migration — the gateway + // never received the legacy secret, so unlinking the file would + // strand the user's only copy. + const allStagedMigrated = + stagedLegacyKeys.length > 0 && + stagedLegacyKeys.every((k) => migratedLegacyKeys.has(k)); + if (allStagedMigrated) { + removeLegacyCredentialsFile(); + } else if (stagedLegacyKeys.length > 0) { + const unmigrated = stagedLegacyKeys.filter( + (k) => !migratedLegacyKeys.has(k), + ); + console.error( + ` Kept ~/.nemoclaw/credentials.json: ${String(unmigrated.length)} ` + + `legacy credential(s) were not migrated verbatim to the gateway in this run ` + + `(${unmigrated.join(", ")}). Re-run onboard with the relevant ` + + `providers/channels enabled to migrate them, then the file is removed automatically.`, + ); + } printDashboard(sandboxName, model, provider, nimContainer, agent); } finally { releaseOnboardLock(); diff --git a/src/lib/sandbox-config.ts b/src/lib/sandbox-config.ts index 527d60c8311..2a867a533bf 100644 --- a/src/lib/sandbox-config.ts +++ b/src/lib/sandbox-config.ts @@ -670,10 +670,11 @@ async function configRotateToken(sandboxName: string, opts: RotateTokenOpts = {} process.exit(1); } - // 4. Save credential locally + // 4. Stage the new value in the current process so the openshell update + // that follows can read it via --credential . The OpenShell + // gateway becomes the system of record once the update succeeds. const { saveCredential } = require("./credentials"); saveCredential(credentialEnv, newToken); - console.log(" Credential saved to ~/.nemoclaw/credentials.json"); // 5. Update the openshell provider console.log(" Updating openshell provider..."); diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index 18dbf13d83c..42c45f65e62 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -34,13 +34,12 @@ const { startGatewayForRecovery, pruneKnownHostsEntries, ensureOllamaAuthProxy, + hydrateCredentialEnv, isNonInteractive, } = require("./lib/onboard"); const { parseGatewayTokenArgs, runGatewayTokenCommand } = require("./lib/gateway-token-command"); const { getCredential, - deleteCredential, - listCredentialKeys, prompt: askPrompt, } = require("./lib/credentials"); const registry = require("./lib/registry"); @@ -95,6 +94,7 @@ import { knownChannelNames, persistChannelTokens, } from "./lib/sandbox-channels"; +const onboardProviders = require("./lib/onboard-providers"); // ── Global commands (derived from command registry) ────────────── @@ -1184,6 +1184,29 @@ function uninstall(args: string[]) { }); } +// Suffixes that mark a per-sandbox messaging integration in the gateway's +// provider list, not a NemoClaw-managed credential. The bridge providers are +// created during onboarding (see src/lib/onboard.ts:3203,3208,3218) and torn +// down by the channels/sandbox-delete flows. `nemoclaw credentials list` +// hides them and `nemoclaw credentials reset` refuses to touch them so +// users cannot accidentally break a live integration via the credentials +// surface. +const BRIDGE_PROVIDER_SUFFIXES: readonly string[] = [ + "-telegram-bridge", + "-discord-bridge", + "-slack-bridge", + // Slack registers a second provider for the App-Level Token (used for + // Socket Mode). bridgeProviderName() emits `${sandbox}-slack-app` for + // SLACK_APP_TOKEN, so the guardrails must match that suffix too — + // otherwise the slack-app provider shows up as an ordinary credential + // and `credentials reset` would happily delete it. + "-slack-app", +]; + +function isBridgeProviderName(name: string): boolean { + return BRIDGE_PROVIDER_SUFFIXES.some((suffix) => name.endsWith(suffix)); +} + async function credentialsCommand(args: string[]): Promise { const sub = args[0]; if (!sub || sub === "help" || sub === "--help" || sub === "-h") { @@ -1191,54 +1214,102 @@ async function credentialsCommand(args: string[]): Promise { console.log(" Usage: nemoclaw credentials "); console.log(""); console.log(" Subcommands:"); - console.log(" list List stored credential keys (values are not printed)"); - console.log(" reset [--yes] Remove a stored credential so onboard re-prompts"); + console.log(" list List provider credentials registered with the OpenShell gateway"); + console.log(" reset [--yes] Remove a provider credential so onboard re-prompts"); console.log(""); - console.log(" Stored at ~/.nemoclaw/credentials.json (mode 600)"); + console.log(" Credentials live in the OpenShell gateway. Inspect with `openshell provider list`."); + console.log(" Nothing is persisted to host disk; deploy/non-onboard commands read from env vars."); console.log(""); return; } if (sub === "list") { - const keys = listCredentialKeys(); - if (keys.length === 0) { - console.log(" No stored credentials."); - return; + // Pin to the NemoClaw gateway so a different active gateway cannot make + // us list (or later delete) providers from the wrong place. + const recovery = await recoverNamedGatewayRuntime(); + if (!recovery.recovered) { + console.error(" Could not query the NemoClaw OpenShell gateway. Is it running?"); + console.error(" Run 'openshell gateway start --name nemoclaw' or 'nemoclaw onboard' first."); + process.exit(1); } - console.log(" Stored credentials:"); - for (const k of keys) { - console.log(` ${k}`); + const result = runOpenshell(["provider", "list", "--names"], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) { + console.error(" Could not query OpenShell gateway. Is it running?"); + console.error(" Run 'openshell gateway start --name nemoclaw' or 'nemoclaw onboard' first."); + process.exit(1); + } + const allNames = String(result.stdout || "") + .split("\n") + .map((s) => s.trim()) + .filter((s) => s.length > 0); + // Show only credential providers. Per-sandbox messaging bridges are + // live integrations managed by the channels surface; surfacing them + // here would invite users to "reset" what looks like a credential and + // accidentally destroy a running bridge. + const credentialNames = allNames.filter((n) => !isBridgeProviderName(n)).sort(); + const bridgeNames = allNames.filter((n) => isBridgeProviderName(n)); + if (credentialNames.length === 0) { + console.log(" No provider credentials registered."); + } else { + console.log(" Providers registered with the OpenShell gateway:"); + for (const name of credentialNames) { + console.log(` ${name}`); + } + } + if (bridgeNames.length > 0) { + console.log(""); + console.log( + ` ${String(bridgeNames.length)} per-sandbox messaging bridge(s) are also registered.`, + ); + console.log( + " Manage those with `nemoclaw channels list/remove/stop` — not this command.", + ); } return; } if (sub === "reset") { const key = args[1]; - // Validate that is a real positional argument, not a flag like - // `--yes` that the user passed without a key. Without this guard, the - // missing-key path would mistakenly look up '--yes' as a credential. + // Validate that is a real positional argument, not a flag like + // `--yes` that the user passed without a key. if (!key || key.startsWith("-")) { - console.error(" Usage: nemoclaw credentials reset [--yes]"); - console.error(" Run 'nemoclaw credentials list' to see stored keys."); + console.error(" Usage: nemoclaw credentials reset [--yes]"); + console.error(" KEY is an OpenShell provider name. Run 'nemoclaw credentials list' first."); process.exit(1); } // Reject unknown trailing arguments to keep scripted use predictable. const extraArgs = args.slice(2).filter((arg) => arg !== "--yes" && arg !== "-y"); if (extraArgs.length > 0) { console.error(` Unknown argument(s) for credentials reset: ${extraArgs.join(", ")}`); - console.error(" Usage: nemoclaw credentials reset [--yes]"); + console.error(" Usage: nemoclaw credentials reset [--yes]"); process.exit(1); } - // Only consult the persisted credentials file — getCredential() falls back - // to process.env, which would let an env-only key pass this check even - // though there is nothing on disk to delete. - if (!listCredentialKeys().includes(key)) { - console.error(` No stored credential found for '${key}'.`); + // Refuse to delete a per-sandbox messaging bridge — those are live + // integrations created/destroyed by the channels surface, not + // NemoClaw-managed credentials. Without this guard, scripting against + // the gateway provider list could tear down a running bridge and + // leave the sandbox in a half-configured state. + if (isBridgeProviderName(key)) { + console.error( + ` '${key}' is a per-sandbox messaging bridge, not a credential.`, + ); + console.error( + " Use `nemoclaw channels remove ` to retire", + ); + console.error( + " the integration (it tears down the bridge provider and rebuilds the sandbox),", + ); + console.error( + " or `nemoclaw channels stop <…>` to pause it without clearing tokens.", + ); process.exit(1); } const skipPrompt = args.includes("--yes") || args.includes("-y"); if (!skipPrompt) { - const answer = (await askPrompt(` Remove stored credential '${key}'? [y/N]: `)) + const answer = (await askPrompt(` Remove provider '${key}' from the OpenShell gateway? [y/N]: `)) .trim() .toLowerCase(); if (answer !== "y" && answer !== "yes") { @@ -1246,12 +1317,40 @@ async function credentialsCommand(args: string[]): Promise { return; } } - const removed = deleteCredential(key); - if (removed) { - console.log(` Removed '${key}' from ~/.nemoclaw/credentials.json`); + // Pin to the NemoClaw gateway so we cannot accidentally delete a + // provider from a different active gateway. We deliberately do NOT + // touch process.env here — `key` is an OpenShell provider name, and + // calling deleteCredential on it would silently strip an unrelated + // env entry whenever a provider name happens to share the shape of + // a credential env variable. + const recovery = await recoverNamedGatewayRuntime(); + if (!recovery.recovered) { + console.error(" Could not reach the NemoClaw OpenShell gateway. Is it running?"); + console.error(" Run 'openshell gateway start --name nemoclaw' or 'nemoclaw onboard' first."); + process.exit(1); + } + const result = runOpenshell(["provider", "delete", key], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status === 0) { + console.log(` Removed provider '${key}' from the OpenShell gateway.`); console.log(" Re-run 'nemoclaw onboard' to enter a new value."); } else { - console.error(` No stored credential found for '${key}'.`); + console.error(` Could not remove provider '${key}'.`); + // Earlier releases accepted a credential env-var name (e.g. + // NVIDIA_API_KEY) here; the API now takes an OpenShell provider + // name (nvidia-prod, openai-api, telegram-bridge, …). Surface the + // rename to anyone whose script is still passing the old shape. + if (/^[A-Z][A-Z0-9_]+$/.test(key)) { + console.error(""); + console.error(` '${key}' looks like a credential env variable name.`); + console.error(" As of this release, 'credentials reset' takes an OpenShell"); + console.error(" provider name. Run 'nemoclaw credentials list' to see the"); + console.error(" registered providers, then retry with one of those names."); + } + const stderr = String(result.stderr || "").trim(); + if (stderr) console.error(` ${stderr}`); process.exit(1); } return; @@ -2055,6 +2154,108 @@ function sandboxChannelsList(sandboxName: string) { console.log(""); } +// Map a channel + token-env-key to the OpenShell provider name onboarding +// uses for it. Mirrors the names in src/lib/onboard.ts:3201-3221 so a +// channels-add upsert collides with (i.e. updates) the same provider that +// a later rebuild would have created from scratch. +function bridgeProviderName(sandboxName: string, channelName: string, envKey: string): string { + if (channelName === "slack" && envKey === "SLACK_APP_TOKEN") { + return `${sandboxName}-slack-app`; + } + return `${sandboxName}-${channelName}-bridge`; +} + +// Push channel tokens to the OpenShell gateway and add the channel to the +// sandbox registry's messagingChannels list. Done eagerly at `channels +// add` time (not deferred to rebuild) because the host-side credential +// helpers are env-only after the fix — without an immediate gateway +// upsert plus registry update, a "rebuild later" answer would drop the +// queued change since process.env disappears when the CLI exits. +async function applyChannelAddToGatewayAndRegistry( + sandboxName: string, + channelName: string, + acquired: Record, +): Promise { + const recovery = await recoverNamedGatewayRuntime(); + if (!recovery.recovered) { + console.error(" Could not reach the NemoClaw OpenShell gateway. Tokens were staged"); + console.error(" in env for this run only — re-run after starting the gateway, or run"); + console.error(" 'openshell gateway start --name nemoclaw' manually."); + process.exit(1); + } + const tokenDefs = Object.entries(acquired).map(([envKey, token]) => ({ + name: bridgeProviderName(sandboxName, channelName, envKey), + envKey, + token, + })); + // upsertMessagingProviders handles create-or-update and process.exits on + // failure, so reaching the next line means every entry is registered. + onboardProviders.upsertMessagingProviders(tokenDefs, runOpenshell); + + // Persist the enabled-channels list in the registry so a deferred + // `nemoclaw rebuild` knows the channel set without needing + // tokens on disk. + const entry = registry.getSandbox(sandboxName); + if (entry) { + const enabled = new Set(entry.messagingChannels || []); + enabled.add(channelName); + const disabled = (entry.disabledChannels || []).filter((c: string) => c !== channelName); + registry.updateSandbox(sandboxName, { + messagingChannels: Array.from(enabled).sort(), + disabledChannels: disabled, + }); + } +} + +// Remove a channel's bridge providers from the gateway and drop it from the +// registry's messagingChannels list. Mirrors applyChannelAddToGatewayAndRegistry. +async function applyChannelRemoveToGatewayAndRegistry( + sandboxName: string, + channelName: string, + channelTokenKeys: string[], +): Promise { + const recovery = await recoverNamedGatewayRuntime(); + if (!recovery.recovered) { + console.error(" Could not reach the NemoClaw OpenShell gateway to delete the bridge."); + console.error(" Re-run after starting the gateway, or run 'openshell gateway start --name nemoclaw'."); + process.exit(1); + } + // Capture each delete's outcome. If any non-NotFound failure surfaces + // we must NOT update the registry — otherwise NemoClaw would record + // the channel as removed locally while the bridge is still live in + // the gateway, which produces a half-configured sandbox the user + // can't easily recover. + const failed: string[] = []; + for (const envKey of channelTokenKeys) { + const name = bridgeProviderName(sandboxName, channelName, envKey); + const result = runOpenshell(["provider", "delete", name], { + ignoreError: true, + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) { + const output = `${result.stdout || ""}${result.stderr || ""}`; + // Treat "not found" as success-equivalent — a previous run may + // have already deleted the provider. + if (!/\bNotFound\b|not found/i.test(output)) { + failed.push(name); + } + } + } + if (failed.length > 0) { + console.error( + ` Failed to delete bridge provider(s) from the OpenShell gateway: ${failed.join(", ")}.`, + ); + console.error(" Registry not updated; re-run after resolving the gateway error."); + process.exit(1); + } + + const entry = registry.getSandbox(sandboxName); + if (entry) { + const enabled = (entry.messagingChannels || []).filter((c: string) => c !== channelName); + registry.updateSandbox(sandboxName, { messagingChannels: enabled }); + } +} + async function promptAndRebuild(sandboxName: string, actionDesc: string): Promise { if (isNonInteractive()) { console.log(""); @@ -2123,7 +2324,13 @@ async function sandboxChannelsAdd(sandboxName: string, args: string[] = []): Pro } persistChannelTokens(acquired); - console.log(` ${G}✓${R} Saved ${channelArg} credentials.`); + // Push to the gateway and update the registry NOW so that answering + // "rebuild later" (or running non-interactively) does not silently + // discard the change. Pre-fix this was safe because saveCredential() + // wrote credentials.json; with env-only persistence, exiting before + // the rebuild used to drop the queued token. + await applyChannelAddToGatewayAndRegistry(sandboxName, channelArg, acquired); + console.log(` ${G}✓${R} Registered ${channelArg} bridge with the OpenShell gateway.`); await promptAndRebuild(sandboxName, `add '${channelArg}'`); } @@ -2149,7 +2356,16 @@ async function sandboxChannelsRemove(sandboxName: string, args: string[] = []): } clearChannelTokens(channel); - console.log(` ${G}✓${R} Cleared stored ${channelArg} credentials.`); + // Same rationale as channels-add: tear down the gateway providers and + // drop the channel from the registry NOW so a deferred rebuild does + // not leave a stale bridge running against a token NemoClaw has + // already "removed" from the user's perspective. + await applyChannelRemoveToGatewayAndRegistry( + sandboxName, + channelArg, + getChannelTokenKeys(channel), + ); + console.log(` ${G}✓${R} Removed ${channelArg} bridge from the OpenShell gateway.`); await promptAndRebuild(sandboxName, `remove '${channelArg}'`); } @@ -2754,7 +2970,10 @@ async function sandboxRebuild( rebuildCredentialEnv = null; } if (rebuildCredentialEnv) { - const credentialValue = getCredential(rebuildCredentialEnv); + // hydrateCredentialEnv migrates any pre-fix legacy credentials.json + // into process.env once, so users upgrading from a release that wrote + // the plaintext file can still rebuild without re-entering keys. + const credentialValue = hydrateCredentialEnv(rebuildCredentialEnv); log( `Preflight credential check: ${rebuildCredentialEnv} → ${credentialValue ? "present" : "MISSING"}`, ); @@ -2763,7 +2982,7 @@ async function sandboxRebuild( console.error(` ${_RD}Rebuild preflight failed:${R} provider credential not found.`); console.error(` The non-interactive recreate step requires ${rebuildCredentialEnv},`); console.error( - " but it is not set in the environment or saved in ~/.nemoclaw/credentials.json.", + " but it is not set in the environment.", ); console.error(""); console.error(" To fix, do one of:"); @@ -3775,14 +3994,14 @@ function help() { ` ${D}• Change inference model: openshell inference set -g nemoclaw --model --provider ${R}`, ); lines.push(` ${D}• Add network presets: use the policy-add command on your sandbox${R}`); - lines.push(` ${D}• Change credentials: credentials reset , then re-run onboard${R}`); + lines.push(` ${D}• Change credentials: credentials reset , then re-run onboard${R}`); lines.push(` ${D}• openclaw.json is read-only inside the sandbox (Landlock enforced).${R}`); lines.push(` ${D} To change OpenClaw settings, re-run onboard to rebuild the sandbox.${R}`); // ── Footer ── lines.push(""); lines.push(` ${D}Powered by NVIDIA OpenShell · Nemotron · Agent Toolkit`); - lines.push(` Credentials saved in ~/.nemoclaw/credentials.json (mode 600)${R}`); + lines.push(` Credentials registered with the OpenShell gateway${R}`); lines.push(` ${D}https://www.nvidia.com/nemoclaw${R}`); lines.push(""); diff --git a/test/canonical-credential-resolution.test.ts b/test/canonical-credential-resolution.test.ts index e90f6d8bbac..5c5d47ca3de 100644 --- a/test/canonical-credential-resolution.test.ts +++ b/test/canonical-credential-resolution.test.ts @@ -117,6 +117,21 @@ describe("resolveProviderCredential — canonical credential resolution (#2306)" expect(result).toBe("from-env"); }); + it("stages legacy credentials through the resolver without deleting the legacy file", async () => { + const tmpDir = createFixtureHome("NVIDIA_API_KEY", "nvapi-staged-only"); + const legacyFile = path.join(tmpDir, ".nemoclaw", "credentials.json"); + delete process.env["NVIDIA_API_KEY"]; + + const credentials = await importCredentialsModule(tmpDir); + const result = credentials.resolveProviderCredential("NVIDIA_API_KEY"); + + expect(result).toBe("nvapi-staged-only"); + expect(process.env["NVIDIA_API_KEY"]).toBe("nvapi-staged-only"); + // Generic lookup cannot prove every legacy value reached the gateway. + // Only onboard's verified migration gate may remove this plaintext file. + expect(fs.existsSync(legacyFile)).toBe(true); + }); + it("returns null when credential exists nowhere", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2306-missing-")); tmpFixtures.push(tmpDir); @@ -130,14 +145,21 @@ describe("resolveProviderCredential — canonical credential resolution (#2306)" }); it("normalizes whitespace and carriage returns", async () => { - const tmpDir = createFixtureHome("TEST_WHITESPACE_KEY", " nvapi-key \r\n"); + // Uses an allowlisted env-key (`NVIDIA_API_KEY`) so the value can + // actually be staged from the legacy file. The post-#2554 staging + // helper rejects entries that aren't in `KNOWN_CREDENTIAL_ENV_KEYS`, + // which is the security guard that prevents a tampered + // credentials.json from injecting unrelated env vars (e.g. `PATH`, + // `NODE_OPTIONS`); the original test fixture used a fake + // `TEST_WHITESPACE_KEY` that is correctly filtered out. + const tmpDir = createFixtureHome("NVIDIA_API_KEY", " nvapi-whitespace-test \r\n"); const credentials = await importCredentialsModule(tmpDir); - delete process.env["TEST_WHITESPACE_KEY"]; - const result = credentials.resolveProviderCredential("TEST_WHITESPACE_KEY"); + delete process.env["NVIDIA_API_KEY"]; + const result = credentials.resolveProviderCredential("NVIDIA_API_KEY"); - expect(result).toBe("nvapi-key"); - expect(process.env["TEST_WHITESPACE_KEY"]).toBe("nvapi-key"); + expect(result).toBe("nvapi-whitespace-test"); + expect(process.env["NVIDIA_API_KEY"]).toBe("nvapi-whitespace-test"); }); it("does not pollute process.env on null resolve", async () => { diff --git a/test/credentials.test.ts b/test/credentials.test.ts index 2bc9a19c157..79da7d35231 100644 --- a/test/credentials.test.ts +++ b/test/credentials.test.ts @@ -5,7 +5,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { spawnSync } from "node:child_process"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; type CredentialsModule = typeof import("../dist/lib/credentials.js"); @@ -14,10 +14,25 @@ function isCredentialsModule(value: object | null): value is CredentialsModule { value !== null && typeof Reflect.get(value, "loadCredentials") === "function" && typeof Reflect.get(value, "getCredential") === "function" && - typeof Reflect.get(value, "saveCredential") === "function" + typeof Reflect.get(value, "saveCredential") === "function" && + typeof Reflect.get(value, "stageLegacyCredentialsToEnv") === "function" && + typeof Reflect.get(value, "removeLegacyCredentialsFile") === "function" ); } +// Pull the credential-env-key allowlist from the production module so +// future additions only need to be made in one place. Plus a few +// fixture-only names this suite mutates directly. +import { KNOWN_CREDENTIAL_ENV_KEYS } from "../dist/lib/credentials.js"; +const TEST_FIXTURE_ENV_KEYS = ["TEST_API_KEY", "OTHER_KEY", "EMPTY_VALUE", "ZETA", "ALPHA"]; +const TRACKED_ENV_KEYS = [...KNOWN_CREDENTIAL_ENV_KEYS, ...TEST_FIXTURE_ENV_KEYS]; + +function clearTrackedEnv() { + for (const key of TRACKED_ENV_KEYS) { + delete process.env[key]; + } +} + async function importCredentialsModule(home: string): Promise { vi.resetModules(); vi.doUnmock("fs"); @@ -33,47 +48,56 @@ async function importCredentialsModule(home: string): Promise return moduleObject; } +beforeEach(() => { + // The user's shell may export NVIDIA_API_KEY etc.; the credentials module + // now reads exclusively from process.env, so any inherited value would + // contaminate every test. Start each case from a clean process env. + clearTrackedEnv(); +}); + afterEach(() => { + clearTrackedEnv(); vi.restoreAllMocks(); vi.resetModules(); vi.unstubAllEnvs(); }); -describe("credential prompts", () => { - it("loads, normalizes, and saves credentials from disk", async () => { +describe("host-side credential staging", () => { + it("stages values in process.env and never writes to disk", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); const credentials = await importCredentialsModule(home); expect(credentials.loadCredentials()).toEqual({}); - credentials.saveCredential("TEST_API_KEY", " nvapi-saved-key \r\n"); + credentials.saveCredential("NVIDIA_API_KEY", " nvapi-saved-key \r\n"); - expect(credentials.getCredsDir()).toBe(path.join(home, ".nemoclaw")); - expect(credentials.getCredsFile()).toBe(path.join(home, ".nemoclaw", "credentials.json")); - expect(credentials.loadCredentials()).toEqual({ TEST_API_KEY: "nvapi-saved-key" }); - expect(credentials.getCredential("TEST_API_KEY")).toBe("nvapi-saved-key"); + // No plaintext credentials.json — the gateway is the system of record. + const legacyFile = path.join(home, ".nemoclaw", "credentials.json"); + expect(fs.existsSync(legacyFile)).toBe(false); - const saved = JSON.parse( - fs.readFileSync(path.join(home, ".nemoclaw", "credentials.json"), "utf-8"), - ); - expect(saved).toEqual({ TEST_API_KEY: "nvapi-saved-key" }); - - const dirMode = fs.statSync(path.join(home, ".nemoclaw")).mode & 0o777; - const fileMode = fs.statSync(path.join(home, ".nemoclaw", "credentials.json")).mode & 0o777; - expect(dirMode).toBe(0o700); - expect(fileMode).toBe(0o600); + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-saved-key"); + expect(credentials.getCredential("NVIDIA_API_KEY")).toBe("nvapi-saved-key"); + expect(credentials.loadCredentials()).toEqual({ NVIDIA_API_KEY: "nvapi-saved-key" }); + expect(credentials.listCredentialKeys()).toEqual(["NVIDIA_API_KEY"]); }); - it("prefers environment credentials and ignores malformed credential files", async () => { + it("getCredential reads only from process.env", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + + // A pre-existing legacy file must NOT bleed into getCredential — the + // module no longer reads cleartext from disk. fs.mkdirSync(path.join(home, ".nemoclaw"), { recursive: true }); - fs.writeFileSync(path.join(home, ".nemoclaw", "credentials.json"), "{not-json"); + fs.writeFileSync( + path.join(home, ".nemoclaw", "credentials.json"), + JSON.stringify({ NVIDIA_API_KEY: "nvapi-from-disk" }), + { mode: 0o600 }, + ); const credentials = await importCredentialsModule(home); - expect(credentials.loadCredentials()).toEqual({}); + expect(credentials.getCredential("NVIDIA_API_KEY")).toBe(null); - vi.stubEnv("TEST_API_KEY", " nvapi-from-env \n"); - expect(credentials.getCredential("TEST_API_KEY")).toBe("nvapi-from-env"); + vi.stubEnv("NVIDIA_API_KEY", " nvapi-from-env \n"); + expect(credentials.getCredential("NVIDIA_API_KEY")).toBe("nvapi-from-env"); }); it("returns null for missing or blank credential values", async () => { @@ -81,48 +105,337 @@ describe("credential prompts", () => { const credentials = await importCredentialsModule(home); credentials.saveCredential("EMPTY_VALUE", " \r\n "); - expect(credentials.getCredential("MISSING_VALUE")).toBe(null); expect(credentials.getCredential("EMPTY_VALUE")).toBe(null); + expect(credentials.getCredential("NVIDIA_API_KEY")).toBe(null); }); - it("deleteCredential removes a stored key and leaves the file mode intact", async () => { + it("deleteCredential clears the staged value without touching disk", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); - // Isolate from real environment so getCredential only checks the file. - vi.stubEnv("NVIDIA_API_KEY", ""); - vi.stubEnv("OTHER_KEY", ""); const credentials = await importCredentialsModule(home); credentials.saveCredential("NVIDIA_API_KEY", "nvapi-bad-key"); - credentials.saveCredential("OTHER_KEY", "other-value"); - const credsFile = path.join(home, ".nemoclaw", "credentials.json"); - expect(fs.statSync(credsFile).mode & 0o777).toBe(0o600); - expect(credentials.listCredentialKeys()).toEqual(["NVIDIA_API_KEY", "OTHER_KEY"]); + credentials.saveCredential("OPENAI_API_KEY", "sk-other"); + + expect(credentials.listCredentialKeys()).toEqual(["NVIDIA_API_KEY", "OPENAI_API_KEY"]); + expect(fs.existsSync(path.join(home, ".nemoclaw", "credentials.json"))).toBe(false); expect(credentials.deleteCredential("NVIDIA_API_KEY")).toBe(true); - expect(fs.statSync(credsFile).mode & 0o777).toBe(0o600); expect(credentials.getCredential("NVIDIA_API_KEY")).toBe(null); - expect(credentials.listCredentialKeys()).toEqual(["OTHER_KEY"]); - expect(credentials.getCredential("OTHER_KEY")).toBe("other-value"); + expect(credentials.listCredentialKeys()).toEqual(["OPENAI_API_KEY"]); + expect(credentials.getCredential("OPENAI_API_KEY")).toBe("sk-other"); - // Removing the same key twice is a no-op that returns false. + // Idempotent. expect(credentials.deleteCredential("NVIDIA_API_KEY")).toBe(false); }); - it("deleteCredential returns false when no credentials file exists", async () => { + it("deleteCredential returns false when nothing is staged", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); const credentials = await importCredentialsModule(home); expect(credentials.deleteCredential("ANYTHING")).toBe(false); }); - it("listCredentialKeys returns sorted key names without exposing values", async () => { + it("listCredentialKeys reports staged known keys, sorted, without exposing values", async () => { const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); const credentials = await importCredentialsModule(home); expect(credentials.listCredentialKeys()).toEqual([]); - credentials.saveCredential("ZETA", "z"); - credentials.saveCredential("ALPHA", "a"); - expect(credentials.listCredentialKeys()).toEqual(["ALPHA", "ZETA"]); + + credentials.saveCredential("ANTHROPIC_API_KEY", "z"); + credentials.saveCredential("OPENAI_API_KEY", "a"); + expect(credentials.listCredentialKeys()).toEqual(["ANTHROPIC_API_KEY", "OPENAI_API_KEY"]); + }); +}); + +describe("legacy credentials.json migration (two-phase: stage then remove)", () => { + it("stages allowlisted keys into env without touching the file", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + fs.writeFileSync( + legacyFile, + JSON.stringify({ + NVIDIA_API_KEY: "nvapi-legacy", + TELEGRAM_BOT_TOKEN: "tg-legacy", + IGNORED_NON_STRING: 42 as unknown as string, + }), + { mode: 0o600 }, + ); + + const credentials = await importCredentialsModule(home); + const staged = credentials.stageLegacyCredentialsToEnv(); + + expect(staged).toEqual(["NVIDIA_API_KEY", "TELEGRAM_BOT_TOKEN"]); + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-legacy"); + expect(process.env.TELEGRAM_BOT_TOKEN).toBe("tg-legacy"); + + // The file MUST still exist after staging — it is removed only after a + // successful gateway write so an interrupted onboard can be retried. + expect(fs.existsSync(legacyFile)).toBe(true); + }); + + it("ignores keys outside the credential allowlist (PATH, NODE_OPTIONS, etc.)", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + // Capture what the runner already exports so the assertions don't + // assume `undefined` on hosts that legitimately set NODE_OPTIONS or + // OPENSHELL_GATEWAY (CI runners, dev shells with debug flags, etc.). + const originalPath = process.env.PATH; + const originalNodeOptions = process.env.NODE_OPTIONS; + const originalOpenshellGateway = process.env.OPENSHELL_GATEWAY; + fs.writeFileSync( + legacyFile, + JSON.stringify({ + PATH: "/attacker/bin:/usr/bin", + NODE_OPTIONS: "--require=/tmp/evil.js", + OPENSHELL_GATEWAY: "evil-gw", + NVIDIA_API_KEY: "nvapi-legitimate", + }), + { mode: 0o600 }, + ); + + const credentials = await importCredentialsModule(home); + const staged = credentials.stageLegacyCredentialsToEnv(); + + expect(staged).toEqual(["NVIDIA_API_KEY"]); + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-legitimate"); + expect(process.env.PATH).toBe(originalPath); + expect(process.env.NODE_OPTIONS).toBe(originalNodeOptions); + expect(process.env.OPENSHELL_GATEWAY).toBe(originalOpenshellGateway); + }); + + it("returns [] when no legacy file is present", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credentials = await importCredentialsModule(home); + expect(credentials.stageLegacyCredentialsToEnv()).toEqual([]); + }); + + it("does not override env values that the user explicitly set", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + fs.mkdirSync(credsDir, { recursive: true }); + fs.writeFileSync( + path.join(credsDir, "credentials.json"), + JSON.stringify({ NVIDIA_API_KEY: "nvapi-from-disk" }), + { mode: 0o600 }, + ); + + vi.stubEnv("NVIDIA_API_KEY", "nvapi-from-env"); + const credentials = await importCredentialsModule(home); + const staged = credentials.stageLegacyCredentialsToEnv(); + + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-from-env"); + // The legacy value was skipped, so it must NOT be reported as staged. + // Onboard uses the staged length to decide whether to delete the file; + // a false-positive entry here would unlink credentials we never + // actually migrated. + expect(staged).toEqual([]); + expect(fs.existsSync(path.join(credsDir, "credentials.json"))).toBe(true); + }); + + it("staging is a no-op once the file is gone (idempotent across runs)", async () => { + // Subsequent CLI invocations after the legacy file has been + // unlinked must short-circuit without rebuilding env from disk. + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credentials = await importCredentialsModule(home); + expect(credentials.stageLegacyCredentialsToEnv()).toEqual([]); + expect(process.env.NVIDIA_API_KEY).toBeUndefined(); }); + it("treats a blank/whitespace env entry as unset and stages the legacy value", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + fs.mkdirSync(credsDir, { recursive: true }); + fs.writeFileSync( + path.join(credsDir, "credentials.json"), + JSON.stringify({ NVIDIA_API_KEY: "nvapi-from-disk" }), + { mode: 0o600 }, + ); + + // A whitespace-only env entry — for example a CI step that exports + // an empty value — must not block staging the legacy file value, or + // rebuild/onboard preflight will fail with a credential the user + // demonstrably has on disk. + vi.stubEnv("NVIDIA_API_KEY", " "); + const credentials = await importCredentialsModule(home); + const staged = credentials.stageLegacyCredentialsToEnv(); + + expect(staged).toEqual(["NVIDIA_API_KEY"]); + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-from-disk"); + }); + + it("stages nothing from a corrupt legacy file and leaves it untouched", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + fs.writeFileSync(legacyFile, "{not-json", { mode: 0o600 }); + + const credentials = await importCredentialsModule(home); + expect(credentials.stageLegacyCredentialsToEnv()).toEqual([]); + // Corrupt input must not silently disappear — leave it for inspection. + expect(fs.existsSync(legacyFile)).toBe(true); + expect(process.env.NVIDIA_API_KEY).toBeUndefined(); + }); + + it("refuses to migrate an oversized legacy file (DoS guard)", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + // Two megabytes of valid JSON, well above the 1 MiB sanity cap. + const filler = "x".repeat(2 * 1024 * 1024); + fs.writeFileSync( + legacyFile, + JSON.stringify({ NVIDIA_API_KEY: `nvapi-${filler}` }), + { mode: 0o600 }, + ); + + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const credentials = await importCredentialsModule(home); + + try { + expect(credentials.stageLegacyCredentialsToEnv()).toEqual([]); + expect(process.env.NVIDIA_API_KEY).toBeUndefined(); + // File is left in place so the user can inspect or delete it. + expect(fs.existsSync(legacyFile)).toBe(true); + // The user gets a diagnostic on stderr explaining the refusal. + const messages = errorSpy.mock.calls.map((call) => String(call[0])).join("\n"); + expect(messages).toMatch(/sanity cap/); + } finally { + errorSpy.mockRestore(); + } + }); + + it("refuses to follow a symlink at the legacy path (no value reads past the link)", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + + // A real credentials file at an unrelated path; the attacker plants a + // symlink at credentials.json that points at it. + const realFile = path.join(home, "real-creds.json"); + fs.writeFileSync( + realFile, + JSON.stringify({ NVIDIA_API_KEY: "nvapi-attacker-controlled" }), + ); + fs.symlinkSync(realFile, legacyFile); + + const credentials = await importCredentialsModule(home); + expect(credentials.stageLegacyCredentialsToEnv()).toEqual([]); + expect(process.env.NVIDIA_API_KEY).toBeUndefined(); + // The pointee is intact; we never read or modified it. + expect(fs.existsSync(realFile)).toBe(true); + }); + + it("survives a crash between stage and remove (interrupted-onboard regression)", async () => { + // Simulates: process A stages legacy values into env then dies before + // completeSession + removeLegacyCredentialsFile run. Process B starts + // fresh (no env) and must successfully re-stage from the still-present + // file, then cleanly remove it on its own success path. + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + fs.writeFileSync( + legacyFile, + JSON.stringify({ NVIDIA_API_KEY: "nvapi-survives-crash" }), + { mode: 0o600 }, + ); + + // --- Process A: stage, then "crash" (we just abandon the env). --- + { + const credentials = await importCredentialsModule(home); + const stagedA = credentials.stageLegacyCredentialsToEnv(); + expect(stagedA).toEqual(["NVIDIA_API_KEY"]); + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-survives-crash"); + // Mid-onboard crash — file MUST still exist. + expect(fs.existsSync(legacyFile)).toBe(true); + } + + // Wipe env so nothing carries over from "process A" into "process B". + delete process.env.NVIDIA_API_KEY; + + // --- Process B: fresh start, re-stage idempotently, then succeed. --- + { + const credentials = await importCredentialsModule(home); + const stagedB = credentials.stageLegacyCredentialsToEnv(); + expect(stagedB).toEqual(["NVIDIA_API_KEY"]); + expect(process.env.NVIDIA_API_KEY).toBe("nvapi-survives-crash"); + credentials.removeLegacyCredentialsFile(); + expect(fs.existsSync(legacyFile)).toBe(false); + } + }); + + it("removeLegacyCredentialsFile zero-fills the file before unlinking", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + const cleartext = JSON.stringify({ NVIDIA_API_KEY: "nvapi-secret-payload" }); + fs.writeFileSync(legacyFile, cleartext, { mode: 0o600 }); + + // Capture the pre-unlink content via a wrapper that intercepts the unlink + // call. After secureUnlink finishes the zero-fill but before the unlink + // runs, the file should be all-zero bytes of the original size. + // The capture lives on a holder object so TypeScript doesn't narrow the + // closure-mutated slot to `never`. + const originalUnlink = fs.unlinkSync; + const captured: { bytes: Buffer | null } = { bytes: null }; + const spy = vi.spyOn(fs, "unlinkSync").mockImplementation((p) => { + if (typeof p === "string" && p === legacyFile && captured.bytes === null) { + try { + captured.bytes = fs.readFileSync(p); + } catch { + /* file already gone */ + } + } + return originalUnlink(p); + }); + + try { + const credentials = await importCredentialsModule(home); + credentials.removeLegacyCredentialsFile(); + } finally { + spy.mockRestore(); + } + + const bytesAtUnlink = captured.bytes; + expect(bytesAtUnlink).not.toBeNull(); + if (bytesAtUnlink !== null) { + expect(bytesAtUnlink.length).toBe(Buffer.byteLength(cleartext)); + expect(bytesAtUnlink.every((b) => b === 0)).toBe(true); + } + expect(fs.existsSync(legacyFile)).toBe(false); + }); + + it("removeLegacyCredentialsFile refuses to follow symlinks (deletes the link, not the target)", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-creds-")); + const credsDir = path.join(home, ".nemoclaw"); + const legacyFile = path.join(credsDir, "credentials.json"); + fs.mkdirSync(credsDir, { recursive: true }); + + // The "victim" file is unrelated content the attacker wants overwritten. + const victimFile = path.join(home, "victim.txt"); + const victimPayload = "important data the attacker should not touch"; + fs.writeFileSync(victimFile, victimPayload); + + // Plant the symlink at the credentials path. + fs.symlinkSync(victimFile, legacyFile); + + const credentials = await importCredentialsModule(home); + credentials.removeLegacyCredentialsFile(); + + // The symlink itself is gone, but the victim file is intact. + expect(fs.existsSync(legacyFile)).toBe(false); + expect(fs.existsSync(victimFile)).toBe(true); + expect(fs.readFileSync(victimFile, "utf-8")).toBe(victimPayload); + }); +}); + +describe("prompt machinery (unchanged)", () => { it("exits cleanly when answers are staged through a pipe", () => { const script = ` set -euo pipefail diff --git a/test/e2e/test-credential-migration.sh b/test/e2e/test-credential-migration.sh new file mode 100755 index 00000000000..c2939782357 --- /dev/null +++ b/test/e2e/test-credential-migration.sh @@ -0,0 +1,297 @@ +#!/bin/bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +# Credential Migration E2E +# +# Validates the host-side credential storage hardening: +# +# 1. A pre-existing plaintext ~/.nemoclaw/credentials.json from an earlier +# release is staged into process.env at onboard time and the value is +# registered with the OpenShell gateway. The legacy file is then +# securely removed (zero-filled, then unlinked) — only after a +# successful onboard, so an interrupted run can be retried without +# losing the user's only copy. +# +# 2. The migration loop is gated on KNOWN_CREDENTIAL_ENV_KEYS so a stale +# or tampered credentials.json cannot inject unrelated variables (PATH, +# NODE_OPTIONS, OPENSHELL_GATEWAY) into the onboard process. +# +# 3. After a normal env-var-driven onboard, no plaintext credentials.json +# exists under ~/.nemoclaw/. +# +# 4. `nemoclaw credentials list` reports providers from the OpenShell +# gateway, not from disk. +# +# 5. If ~/.nemoclaw/credentials.json exists as a symlink to an unrelated +# file, the secure-unlink path removes the symlink without touching +# the target. +# +# This test deliberately lays down legacy state under the runner's HOME, so +# it should run on an ephemeral CI runner. Local dev runs are destructive +# to ~/.nemoclaw/ — set NEMOCLAW_E2E_KEEP_SANDBOX=1 to skip the teardown +# and inspect post-mortem. +# +# Prerequisites: +# - Docker running +# - openshell + nemoclaw on PATH +# - NVIDIA_API_KEY set (used as the migrated value) +# +# Usage: +# NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ +# NVIDIA_API_KEY=nvapi-... bash test/e2e/test-credential-migration.sh + +set -uo pipefail + +export NEMOCLAW_E2E_DEFAULT_TIMEOUT=2400 +SCRIPT_DIR_TIMEOUT="$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd)" +# shellcheck source=test/e2e/e2e-timeout.sh +source "${SCRIPT_DIR_TIMEOUT}/e2e-timeout.sh" + +PASS=0 +FAIL=0 +TOTAL=0 + +pass() { + ((PASS++)) + ((TOTAL++)) + printf '\033[32m PASS: %s\033[0m\n' "$1" +} +fail() { + ((FAIL++)) + ((TOTAL++)) + printf '\033[31m FAIL: %s\033[0m\n' "$1" +} +section() { + echo "" + printf '\033[1;36m=== %s ===\033[0m\n' "$1" +} +info() { printf '\033[1;34m [info]\033[0m %s\n' "$1"; } +indent() { awk '{print " " $0}'; } + +# Resolve repo root the same way the other E2E scripts do. +if [ -d /workspace ] && [ -f /workspace/install.sh ]; then + REPO="/workspace" +elif [ -f "$(cd "$(dirname "$0")/../.." && pwd)/install.sh" ]; then + REPO="$(cd "$(dirname "$0")/../.." && pwd)" +else + echo "ERROR: Cannot find repo root." + exit 1 +fi + +SANDBOX_NAME="${NEMOCLAW_SANDBOX_NAME:-e2e-cred-migration}" + +# shellcheck source=test/e2e/lib/sandbox-teardown.sh +. "$(dirname "${BASH_SOURCE[0]}")/lib/sandbox-teardown.sh" +register_sandbox_for_teardown "$SANDBOX_NAME" + +# ══════════════════════════════════════════════════════════════════ +# Phase 0: Prerequisites +# ══════════════════════════════════════════════════════════════════ +section "Phase 0: Prerequisites" + +if [ -z "${NVIDIA_API_KEY:-}" ]; then + fail "NVIDIA_API_KEY not set" + exit 1 +fi +pass "NVIDIA_API_KEY is set" + +if ! command -v openshell >/dev/null 2>&1; then + info "openshell not found; running install" + bash "$REPO/install.sh" --yes-i-accept-third-party-software \ + >/tmp/nemoclaw-e2e-install.log 2>&1 || { + fail "install.sh failed; see /tmp/nemoclaw-e2e-install.log" + exit 1 + } +fi + +command -v openshell >/dev/null 2>&1 || { + fail "openshell still missing after install" + exit 1 +} +command -v nemoclaw >/dev/null 2>&1 || { + fail "nemoclaw still missing after install" + exit 1 +} +pass "openshell + nemoclaw on PATH" + +REAL_API_KEY="$NVIDIA_API_KEY" +NEMOCLAW_DIR="$HOME/.nemoclaw" +LEGACY_FILE="$NEMOCLAW_DIR/credentials.json" + +# ══════════════════════════════════════════════════════════════════ +# Phase 1: Pre-seed a legacy credentials.json and verify migration +# ══════════════════════════════════════════════════════════════════ +section "Phase 1: Legacy credentials.json migration" + +# Start from a clean ~/.nemoclaw to avoid interference from prior runs. +rm -rf "$NEMOCLAW_DIR" +mkdir -p "$NEMOCLAW_DIR" +chmod 700 "$NEMOCLAW_DIR" + +# Tampered fixture: includes an unrelated key the migrator must ignore. +cat >"$LEGACY_FILE" </dev/null || stat -f '%i' "$LEGACY_FILE" 2>/dev/null || echo "") +[ -n "$LEGACY_INODE_BEFORE" ] && info "Legacy file inode before onboard: $LEGACY_INODE_BEFORE" + +# Run onboard WITHOUT NVIDIA_API_KEY in the env. The only place the value +# can come from is the legacy credentials.json — exactly the migration +# path we want to exercise. +ONBOARD_LOG="$(mktemp)" +( + unset NVIDIA_API_KEY + NEMOCLAW_NON_INTERACTIVE=1 \ + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ + NEMOCLAW_SANDBOX_NAME="$SANDBOX_NAME" \ + NEMOCLAW_RECREATE_SANDBOX=1 \ + nemoclaw onboard --non-interactive >"$ONBOARD_LOG" 2>&1 +) & +ONBOARD_PID=$! +wait "$ONBOARD_PID" +ONBOARD_EXIT=$? + +if [ "$ONBOARD_EXIT" -eq 0 ]; then + pass "nemoclaw onboard succeeded with only the legacy file as the credential source" +else + fail "nemoclaw onboard failed (exit $ONBOARD_EXIT); see log below" + tail -50 "$ONBOARD_LOG" || true + rm -f "$ONBOARD_LOG" + exit 1 +fi + +if grep -q "Staged .* legacy credential" "$ONBOARD_LOG"; then + pass "Migration notice was emitted to stderr" +else + fail "Expected migration notice on stderr; not found in onboard log" + tail -30 "$ONBOARD_LOG" || true +fi +rm -f "$ONBOARD_LOG" + +# After a successful onboard, the legacy file must be gone. +if [ -e "$LEGACY_FILE" ]; then + fail "Legacy credentials.json still exists after successful onboard" +else + pass "Legacy credentials.json was removed after onboard" +fi + +# ══════════════════════════════════════════════════════════════════ +# Phase 2: Verify the value reached the OpenShell gateway +# ══════════════════════════════════════════════════════════════════ +section "Phase 2: Gateway provider registration" + +if ! PROVIDERS_OUT=$(openshell -g nemoclaw provider list --names 2>&1); then + fail "openshell -g nemoclaw provider list --names failed" + printf '%s\n' "$PROVIDERS_OUT" | indent + exit 1 +fi +info "Providers in nemoclaw gateway:" +printf '%s\n' "$PROVIDERS_OUT" | indent + +# The legacy NVIDIA_API_KEY should have been registered as one of the +# inference providers (nvidia-prod, nvidia-nim, etc. — the exact name +# depends on what onboarding chose). Just assert that at least one +# provider was registered. +PROVIDER_COUNT=$(echo "$PROVIDERS_OUT" | grep -E -c '^[a-zA-Z][a-zA-Z0-9_-]*$' || true) +if [ "$PROVIDER_COUNT" -ge 1 ]; then + pass "At least one provider is registered with the gateway ($PROVIDER_COUNT total)" +else + fail "No providers registered with the gateway after migration" +fi + +# Negative assertion: the unrelated keys from the tampered file must not +# have leaked anywhere observable. The strongest check available without +# spawning another nemoclaw process is to verify they are NOT registered +# as gateway provider names — since `openshell provider create +# --credential KEY` would have failed for non-allowlisted keys, but a bug +# could conceivably push them through. +if echo "$PROVIDERS_OUT" | grep -q "OPENSHELL_GATEWAY\|NODE_OPTIONS"; then + fail "A non-allowlisted key from the tampered file appears as a gateway provider" +else + pass "Non-allowlisted keys from the tampered file did not become providers" +fi + +# ══════════════════════════════════════════════════════════════════ +# Phase 3: nemoclaw credentials list reads from the gateway, not disk +# ══════════════════════════════════════════════════════════════════ +section "Phase 3: nemoclaw credentials list" + +if ! CREDS_LIST_OUT=$(nemoclaw credentials list 2>&1); then + fail "nemoclaw credentials list failed" + printf '%s\n' "$CREDS_LIST_OUT" | indent + exit 1 +fi +info "Output:" +printf '%s\n' "$CREDS_LIST_OUT" | indent + +if echo "$CREDS_LIST_OUT" | grep -q "Providers registered with the OpenShell gateway"; then + pass "credentials list surfaces gateway-registered providers" +else + fail "credentials list did not produce the expected gateway header" +fi + +# The disk should still have NO plaintext credentials.json regardless of +# what the gateway holds. +if [ -e "$LEGACY_FILE" ]; then + fail "credentials.json reappeared on disk after credentials list" +else + pass "No plaintext credentials.json on disk after credentials list" +fi + +# ══════════════════════════════════════════════════════════════════ +# Phase 4: Symlink-safe secure unlink +# ══════════════════════════════════════════════════════════════════ +section "Phase 4: Symlink-safe secure unlink" + +# Plant a symlink at the credentials path pointing at an unrelated victim +# file. A naive secureUnlink would zero-fill and unlink the target; the +# hardened path must remove the symlink itself and leave the target +# intact. +VICTIM_FILE="$(mktemp)" +VICTIM_PAYLOAD="important data the attacker should not touch" +printf '%s' "$VICTIM_PAYLOAD" >"$VICTIM_FILE" +ln -s "$VICTIM_FILE" "$LEGACY_FILE" + +# Drive removeLegacyCredentialsFile() directly via a tiny node one-liner. +# Using the compiled module from dist/ matches what the CLI imports. +node -e " +const { removeLegacyCredentialsFile } = require('${REPO}/dist/lib/credentials.js'); +removeLegacyCredentialsFile(); +" >/dev/null 2>&1 || { + fail "node invocation of removeLegacyCredentialsFile failed" +} + +if [ -L "$LEGACY_FILE" ] || [ -e "$LEGACY_FILE" ]; then + fail "Symlink at credentials path was not removed" +else + pass "Symlink at credentials path was removed" +fi + +if [ ! -e "$VICTIM_FILE" ]; then + fail "Victim file was deleted; secureUnlink followed the symlink" +elif [ "$(cat "$VICTIM_FILE")" != "$VICTIM_PAYLOAD" ]; then + fail "Victim file contents were modified; secureUnlink wrote through the symlink" +else + pass "Victim file is untouched (link removed without following the target)" +fi +rm -f "$VICTIM_FILE" + +# ══════════════════════════════════════════════════════════════════ +# Summary +# ══════════════════════════════════════════════════════════════════ +section "Summary" +echo " Total: $TOTAL" +echo " Passed: $PASS" +echo " Failed: $FAIL" + +if [ "$FAIL" -gt 0 ]; then + exit 1 +fi +exit 0 diff --git a/test/e2e/test-onboard-repair.sh b/test/e2e/test-onboard-repair.sh index 8d8223d68b5..071aea423bc 100755 --- a/test/e2e/test-onboard-repair.sh +++ b/test/e2e/test-onboard-repair.sh @@ -125,11 +125,8 @@ else exit 1 fi -node -e ' -const { saveCredential } = require(process.argv[1]); -saveCredential("NVIDIA_API_KEY", process.argv[2]); -' "$REPO/dist/lib/credentials.js" "$RESTORE_API_KEY" -pass "Stored NVIDIA_API_KEY in ~/.nemoclaw/credentials.json for resume hydration" +export NVIDIA_API_KEY="$RESTORE_API_KEY" +pass "Exported NVIDIA_API_KEY for the repair run (host writes nothing to disk; OpenShell gateway is the system of record)" # ══════════════════════════════════════════════════════════════════ # Phase 2: Create interrupted resumable state diff --git a/test/e2e/test-onboard-resume.sh b/test/e2e/test-onboard-resume.sh index 5f1162617b8..c4be7b4c064 100755 --- a/test/e2e/test-onboard-resume.sh +++ b/test/e2e/test-onboard-resume.sh @@ -135,11 +135,8 @@ else exit 1 fi -node -e ' -const { saveCredential } = require(process.argv[1]); -saveCredential("NVIDIA_API_KEY", process.argv[2]); -' "$REPO/dist/lib/credentials.js" "$RESTORE_API_KEY" -pass "Stored NVIDIA_API_KEY in ~/.nemoclaw/credentials.json for resume hydration" +export NVIDIA_API_KEY="$RESTORE_API_KEY" +pass "Exported NVIDIA_API_KEY for the resume run (host writes nothing to disk; OpenShell gateway is the system of record)" # ══════════════════════════════════════════════════════════════════ # Phase 2: First onboard (forced failure after sandbox creation) diff --git a/test/onboard.test.ts b/test/onboard.test.ts index 7a57f851c48..2d7c6997b32 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -2648,7 +2648,7 @@ const { setupInference } = require(${onboardPath}); assert.equal(typeof streamSandboxCreate, "function"); }); - it("hydrates stored provider credentials when setupInference runs without process env set", () => { + it("migrates a legacy credentials.json into env so setupInference can register the provider", () => { const repoRoot = path.join(import.meta.dirname, ".."); const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-resume-cred-")); const fakeBin = path.join(tmpDir, "bin"); @@ -2656,22 +2656,31 @@ const { setupInference } = require(${onboardPath}); const onboardPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "onboard.js")); const runnerPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "runner.js")); const registryPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "registry.js")); - const credentialsPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "credentials.js")); + // Pre-seed a pre-fix plaintext credentials.json. hydrateCredentialEnv + // stages it non-destructively into process.env via + // stageLegacyCredentialsToEnv(); the secure unlink only runs from the + // post-onboard cleanup gate when the staged values are confirmed + // migrated, so the legacy file must still exist after this test's + // setupInference call (asserted further down). + const legacyDir = path.join(tmpDir, ".nemoclaw"); + fs.mkdirSync(legacyDir, { recursive: true, mode: 0o700 }); + fs.writeFileSync( + path.join(legacyDir, "credentials.json"), + JSON.stringify({ OPENAI_API_KEY: "sk-stored-secret" }), + { mode: 0o600 }, + ); fs.mkdirSync(fakeBin, { recursive: true }); fs.writeFileSync(path.join(fakeBin, "openshell"), "#!/usr/bin/env bash\nexit 0\n", { mode: 0o755, }); + const legacyFilePath = JSON.stringify(path.join(legacyDir, "credentials.json")); const script = String.raw` const runner = require(${runnerPath}); const _n = (c) => (Array.isArray(c) ? c.join(" ") : String(c)).replace(/'/g, ""); const registry = require(${registryPath}); -const credentials = require(${credentialsPath}); -const childProcess = require("node:child_process"); -const { EventEmitter } = require("node:events"); const fs = require("node:fs"); -const path = require("node:path"); const commands = []; runner.run = (command, opts = {}) => { @@ -2693,14 +2702,17 @@ runner.runCapture = (command) => { }; registry.updateSandbox = () => true; -credentials.saveCredential("OPENAI_API_KEY", "sk-stored-secret"); delete process.env.OPENAI_API_KEY; const { setupInference } = require(${onboardPath}); (async () => { await setupInference("test-box", "gpt-5.4", "openai-api", "https://api.openai.com/v1", "OPENAI_API_KEY"); - console.log(JSON.stringify({ commands, openai: process.env.OPENAI_API_KEY || null })); + console.log(JSON.stringify({ + commands, + openai: process.env.OPENAI_API_KEY || null, + legacyFileGone: !fs.existsSync(${legacyFilePath}), + })); })().catch((error) => { console.error(error); process.exit(1); @@ -2722,8 +2734,18 @@ const { setupInference } = require(${onboardPath}); const payload = parseStdoutJson<{ openai: string; commands: CommandEntry[]; + legacyFileGone: boolean; }>(result.stdout); assert.equal(payload.openai, "sk-stored-secret"); + // setupInference's hydrateCredentialEnv only stages the legacy file + // (non-destructive). The secure unlink runs only after a full successful + // onboard, so an interrupted run can be retried without losing the + // user's only copy of their credentials. + assert.equal( + payload.legacyFileGone, + false, + "legacy credentials.json must survive the staging-only hydrate path", + ); // commands[0]=gateway select, [1]=provider get, [2]=provider update const providerUpdate = payload.commands[2]; assert.ok(providerUpdate, "expected provider update command"); @@ -6145,8 +6167,8 @@ const { createSandbox } = require(${onboardPath}); assert.ok(summary.includes("gemini-api"), "summary includes provider"); assert.ok(summary.includes("gemini-2.5-flash"), "summary includes model"); assert.ok( - summary.includes("GEMINI_API_KEY (stored in ~/.nemoclaw/credentials.json)"), - "summary shows API key env var + storage location", + summary.includes("GEMINI_API_KEY (staged for OpenShell gateway registration)"), + "summary shows API key env var + staging state", ); assert.ok(summary.includes("enabled"), "summary includes web-search enabled"); assert.ok(summary.includes("telegram, slack"), "summary lists enabled channels"); diff --git a/test/rebuild-credential-hydration.test.ts b/test/rebuild-credential-hydration.test.ts index 8cc7ed811b0..ab4cce41cc6 100644 --- a/test/rebuild-credential-hydration.test.ts +++ b/test/rebuild-credential-hydration.test.ts @@ -3,14 +3,13 @@ /** * Tests for issue #2273 Layer 1: non-interactive provider selection - * resolves credentials from ~/.nemoclaw/credentials.json. + * can stage pre-fix legacy credentials from ~/.nemoclaw/credentials.json. * * Verifies that the non-interactive code path in setupNim() hydrates - * provider credentials from saved storage (via hydrateCredentialEnv) - * BEFORE checking process.env. This prevents the bug where rebuild - * calls onboard non-interactively and fails because the API key was - * entered interactively during the original onboard and only exists - * in credentials.json, not in the current process environment. + * provider credentials through the canonical resolver + * (via hydrateCredentialEnv) before checking process.env. This preserves + * rebuild compatibility for users who still have a pre-fix legacy + * credentials.json while keeping new credential persistence env-only. * * This test covers all remote providers in REMOTE_PROVIDER_CONFIG. * @@ -30,6 +29,19 @@ import { afterEach, describe, expect, it } from "vitest"; const REPO_ROOT = path.join(import.meta.dirname, ".."); const tmpFixtures: string[] = []; +function parsePositiveInt(value: string | undefined): number | null { + if (!value) return null; + const parsed = Number.parseInt(value, 10); + return Number.isFinite(parsed) && parsed > 0 ? parsed : null; +} + +const CHILD_PROCESS_TIMEOUT_MS = Math.max( + 10_000, + parsePositiveInt(process.env.NEMOCLAW_EXEC_TIMEOUT) ?? 0, + parsePositiveInt(process.env.NEMOCLAW_TEST_TIMEOUT) ?? 0, +); +const TEST_TIMEOUT_MS = Math.max(30_000, CHILD_PROCESS_TIMEOUT_MS + 10_000); + afterEach(() => { for (const dir of tmpFixtures.splice(0)) { try { @@ -42,7 +54,7 @@ afterEach(() => { /** * Parametric test: for a given credentialEnv, verify that onboard - * non-interactive mode can resolve the key from credentials.json + * non-interactive mode can resolve a pre-fix legacy credentials.json key * when process.env does NOT have it set. * * We run a small script that: @@ -58,7 +70,7 @@ function verifyCredentialHydration(credentialEnv: string, credentialValue: strin const nemoclawDir = path.join(tmpDir, ".nemoclaw"); fs.mkdirSync(nemoclawDir, { recursive: true, mode: 0o700 }); - // Save credential in credentials.json + // Seed a pre-fix legacy credentials.json. fs.writeFileSync( path.join(nemoclawDir, "credentials.json"), JSON.stringify({ [credentialEnv]: credentialValue }), @@ -76,7 +88,7 @@ const { hydrateCredentialEnv } = require(onboardPath); // Ensure the env var is NOT set delete process.env[${JSON.stringify(credentialEnv)}]; -// Hydrate from credentials.json +// Hydrate through the canonical resolver. const result = hydrateCredentialEnv(${JSON.stringify(credentialEnv)}); // Report @@ -96,13 +108,13 @@ process.stdout.write(JSON.stringify(payload)); PATH: path.dirname(process.execPath) + ":/usr/bin:/bin", NO_COLOR: "1", }, - timeout: 10_000, + timeout: CHILD_PROCESS_TIMEOUT_MS, }); return { result, tmpDir }; } -describe("Issue #2273 Layer 1: credential hydration from saved storage", () => { +describe("Issue #2273 Layer 1: credential hydration from legacy storage", () => { // Test each provider's credential env to ensure parametric coverage const providers = [ { name: "NVIDIA Endpoints", credentialEnv: "NVIDIA_API_KEY", value: "nvapi-test-hydrate" }, @@ -115,8 +127,8 @@ describe("Issue #2273 Layer 1: credential hydration from saved storage", () => { for (const { name, credentialEnv, value } of providers) { it( - `hydrates ${credentialEnv} (${name}) from credentials.json when not in process.env`, - { timeout: 30_000 }, + `hydrates ${credentialEnv} (${name}) from legacy credentials.json when not in process.env`, + { timeout: TEST_TIMEOUT_MS }, () => { const { result } = verifyCredentialHydration(credentialEnv, value);