From b7f17d5a9cd3bc8023ff86d9e1573041a2dbf22a Mon Sep 17 00:00:00 2001 From: cjagwani Date: Wed, 22 Apr 2026 14:04:47 -0700 Subject: [PATCH 01/17] fix(install): surface Node shell-reload hint adjacent to install line (#2178) (#2298) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #2178 (NV QA UAT: \`node\` still reports v20 after installer claims v22.22.2 was installed). When \`scripts/install.sh\` upgrades Node via nvm, \`nvm use 22\` only takes effect inside the installer's subshell — the user's parent shell still resolves \`node\` to the pre-install version until they reload. The existing generic \`source \` hint at the bottom of the installer is easy to miss. Two layers: ### 1. Loud warning adjacent to install line ``` [INFO] Node.js installed via nvm: v22.22.2 (default alias) [WARN] Your current shell may still resolve `node` to an older version until you reload it. To activate v22.22.2 in this shell: exec "$SHELL" -l # (or) nvm use 22 ``` ### 2. Opt-in auto-activation prompt (new) At the end of \`print_done\`, when the upgrade path actually ran AND the user is on a real TTY AND \`--non-interactive\` is not set: ``` Reload your shell now to activate Node v22.22.2? [Y/n] ``` On accept → \`exec \"\$SHELL\" -l\`, replacing the installer process so \`node --version\` prints v22 in the very next prompt with zero extra user action. On decline → noop (warning + manual command still on screen). ## Gates preserving scriptability - \`NON_INTERACTIVE=1\` / \`--non-interactive\` → skip prompt (keeps CI-style flows quiet) - stdin not a TTY (e.g. \`curl | bash\`) → skip prompt (keeps chained invocations like \`curl | bash && nemoclaw onboard\` working) - Upgrade path didn't run (Node was already >= 22) → skip prompt (nothing to reload for) In all skipped cases the loud warning + manual command remain visible. ## Test plan - [x] \`npx vitest run --project cli test/install-preflight.test.ts\` — two new tests pass (hint presence, helper gating + exec path) - [x] \`bash -n scripts/install.sh\` syntax OK; \`install.sh --help\` unaffected 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Installer now reports the exact Node.js version with nvm-specific messaging, warns that the current shell may still resolve an older Node, and prints an explicit pasteable command to activate the new version in the current session. * **Tests** * Added tests that verify the installer’s upgrade messaging and presence of the activation instructions. --------- Co-authored-by: Claude Opus 4.7 (1M context) --- scripts/install.sh | 13 ++++++++++++- test/install-preflight.test.ts | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/scripts/install.sh b/scripts/install.sh index 966c9e35781..6ddbbf1cc67 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -742,7 +742,18 @@ install_nodejs() { ensure_nvm_loaded --force nvm use 22 --silent nvm alias default 22 2>/dev/null || true - info "Node.js installed: $(node --version)" + local installed_version + installed_version="$(node --version)" + info "Node.js installed via nvm: ${installed_version} (default alias)" + # Surface the shell-reload requirement right next to the install line so the + # user isn't left thinking the new Node is already active in their terminal. + # install.sh runs as a subprocess; the parent shell's PATH genuinely cannot + # be mutated from here, so we print the truth and the exact command. + # See issue #2178. + warn "Your current shell may still resolve \`node\` to an older version until it's reloaded." + printf " Open a new terminal, or run this in your existing shell:\n" + # shellcheck disable=SC2016 # intentional: user pastes this literally; their shell expands the vars + printf ' source "${NVM_DIR:-$HOME/.nvm}/nvm.sh" && nvm use 22\n' } # --------------------------------------------------------------------------- diff --git a/test/install-preflight.test.ts b/test/install-preflight.test.ts index 52fe703ef24..4f6a4889857 100644 --- a/test/install-preflight.test.ts +++ b/test/install-preflight.test.ts @@ -1531,6 +1531,27 @@ fi`, expect(gitCalls).not.toMatch(/clone/); expect(gitCalls).not.toMatch(/fetch/); }); + + // Issue #2178 — when nvm installs a new Node, the user's parent shell still + // resolves `node` to the old version until the shell is reloaded. The + // installer's upgrade path must surface this loudly and adjacent to the + // "Node.js installed" line, not only in the generic bottom-of-output Next + // block where it's easy to miss. + it("install_nodejs upgrade path emits a Node-specific shell-reload hint", () => { + const script = fs.readFileSync(INSTALLER_PAYLOAD, "utf-8"); + const installNodejs = script.match(/install_nodejs\(\)\s*\{[\s\S]*?\n\}/); + expect(installNodejs).not.toBeNull(); + const body = installNodejs![0]; + // Anchor to the actual warn/printf calls (not the comment) so the test + // fails if the executable statements are removed. A child process can't + // mutate the parent's PATH, so the honest fix is printing the exact + // command the user can run in their existing shell (no exec tricks — + // those create a nested shell that masks the problem; see PR #2298). + expect(body).toMatch(/\n\s*warn\s+"Your current shell may still resolve/); + // Single-quoted printf avoids bash expansion of $NVM_DIR / $HOME in the + // printed text — the user gets a literal, env-aware command to paste. + expect(body).toMatch(/\n\s*printf\s+'[^']*NVM_DIR:-\$HOME\/\.nvm[^']*nvm use 22/); + }); }); // --------------------------------------------------------------------------- From 9ed9451742b79907ea2999572efcb71d8fc35dcf Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Wed, 22 Apr 2026 14:33:01 -0700 Subject: [PATCH 02/17] refactor(cli): tighten agent manifest typing (#2143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary This PR tightens typing around agent manifest parsing in `src/lib/agent-defs.ts` by replacing the old catch-all record type with recursive manifest value guards. That keeps the YAML loader and field readers typed more precisely, reduces a hotspot in the manifest loader, and adds coverage for invalid top-level manifest payloads. ## Changes - replace `UnknownRecord` in `src/lib/agent-defs.ts` with recursive `ManifestValue` and `ManifestRecord` types - validate parsed YAML manifests with `isManifestRecord()` while still accepting scalar values emitted by `js-yaml`, including timestamp `Date` objects - add a regression test in `src/lib/agent-defs.test.ts` that rejects non-object manifest payloads ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: OpenAI Codex --- Signed-off-by: Carlos Villela ## Summary by CodeRabbit * **Tests** * Added validation test to ensure agent manifest configuration is properly structured as an object. * **Bug Fixes** * Strengthened agent manifest parsing validation to reject malformed configuration files and enforce stricter type constraints on configuration values. Signed-off-by: Carlos Villela --- src/lib/agent-defs.test.ts | 7 +++++ src/lib/agent-defs.ts | 52 +++++++++++++++++++++++++++----------- 2 files changed, 44 insertions(+), 15 deletions(-) diff --git a/src/lib/agent-defs.test.ts b/src/lib/agent-defs.test.ts index 84b1f4fed60..af91c364c48 100644 --- a/src/lib/agent-defs.test.ts +++ b/src/lib/agent-defs.test.ts @@ -84,6 +84,13 @@ describe("agent definitions", () => { ); }); + it("rejects non-object manifest payloads", () => { + const agentName = `invalid-top-level-manifest-${String(Date.now())}`; + writeTempAgentManifest(agentName, ["- not", "- an", "- object"].join("\n")); + + expect(() => loadAgent(agentName)).toThrow(/YAML object/); + }); + it("rejects invalid forward_ports values in manifests", () => { const agentName = `invalid-forward-port-${String(Date.now())}`; writeTempAgentManifest( diff --git a/src/lib/agent-defs.ts b/src/lib/agent-defs.ts index 19e7ec57d9d..4809166be0e 100644 --- a/src/lib/agent-defs.ts +++ b/src/lib/agent-defs.ts @@ -14,7 +14,9 @@ import { DASHBOARD_PORT } from "./ports"; export const AGENTS_DIR = path.join(ROOT, "agents"); -type UnknownRecord = { [key: string]: unknown }; +type ManifestScalar = string | number | boolean | null | Date; +type ManifestValue = ManifestScalar | ManifestRecord | ManifestValue[]; +type ManifestRecord = { [key: string]: ManifestValue }; type StringMap = { [key: string]: string }; export interface AgentHealthProbe { @@ -59,7 +61,7 @@ export interface AgentDefinition { phone_home_hosts?: string[]; forward_ports?: number[]; health_probe?: AgentHealthProbe; - config?: UnknownRecord; + config?: ManifestRecord; state_dirs?: string[]; messaging_platforms?: { supported?: string[] }; _legacy_paths?: StringMap; @@ -94,26 +96,46 @@ export interface AgentChoice { const _cache = new Map(); -function isRecord(value: unknown): value is UnknownRecord { - return typeof value === "object" && value !== null && !Array.isArray(value); +function isManifestValue(value: unknown): value is ManifestValue { + if (value === null || value instanceof Date) return true; + if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { + return true; + } + if (Array.isArray(value)) { + return value.every((entry) => isManifestValue(entry)); + } + return isManifestRecord(value); +} + +function isManifestRecord(value: unknown): value is ManifestRecord { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + + const prototype = Object.getPrototypeOf(value); + if (prototype !== Object.prototype && prototype !== null) { + return false; + } + + return Object.values(value).every((entry) => isManifestValue(entry)); } -function readString(record: UnknownRecord, key: string): string | undefined { +function readString(record: ManifestRecord, key: string): string | undefined { const value = record[key]; return typeof value === "string" ? value : undefined; } -function readBoolean(record: UnknownRecord, key: string): boolean | undefined { +function readBoolean(record: ManifestRecord, key: string): boolean | undefined { const value = record[key]; return typeof value === "boolean" ? value : undefined; } -function readObject(record: UnknownRecord, key: string): UnknownRecord | undefined { +function readObject(record: ManifestRecord, key: string): ManifestRecord | undefined { const value = record[key]; - return isRecord(value) ? value : undefined; + return isManifestRecord(value) ? value : undefined; } -function readStringArray(record: UnknownRecord, key: string): string[] | undefined { +function readStringArray(record: ManifestRecord, key: string): string[] | undefined { const value = record[key]; if (!Array.isArray(value)) return undefined; return value.filter((entry): entry is string => typeof entry === "string"); @@ -123,7 +145,7 @@ function isValidPort(value: unknown): value is number { return typeof value === "number" && Number.isInteger(value) && value >= 1 && value <= 65535; } -function readPortArray(record: UnknownRecord, key: string): number[] | undefined { +function readPortArray(record: ManifestRecord, key: string): number[] | undefined { const value = record[key]; if (value === undefined) return undefined; if (!Array.isArray(value)) { @@ -142,7 +164,7 @@ function readPortArray(record: UnknownRecord, key: string): number[] | undefined return ports.length > 0 ? ports : undefined; } -function readStringMap(record: UnknownRecord, key: string): StringMap | undefined { +function readStringMap(record: ManifestRecord, key: string): StringMap | undefined { const value = readObject(record, key); if (!value) return undefined; @@ -155,7 +177,7 @@ function readStringMap(record: UnknownRecord, key: string): StringMap | undefine return result; } -function readHealthProbe(record: UnknownRecord): AgentHealthProbe | undefined { +function readHealthProbe(record: ManifestRecord): AgentHealthProbe | undefined { const healthProbe = readObject(record, "health_probe"); if (!healthProbe) return undefined; @@ -185,7 +207,7 @@ function readHealthProbe(record: UnknownRecord): AgentHealthProbe | undefined { return undefined; } -function readMessagingPlatforms(record: UnknownRecord): { supported?: string[] } | undefined { +function readMessagingPlatforms(record: ManifestRecord): { supported?: string[] } | undefined { const messagingPlatforms = readObject(record, "messaging_platforms"); if (!messagingPlatforms) return undefined; @@ -193,9 +215,9 @@ function readMessagingPlatforms(record: UnknownRecord): { supported?: string[] } return supported ? { supported } : {}; } -function loadManifestRecord(manifestPath: string): UnknownRecord { +function loadManifestRecord(manifestPath: string): ManifestRecord { const parsed = yaml.load(fs.readFileSync(manifestPath, "utf8")); - if (!isRecord(parsed)) { + if (!isManifestRecord(parsed)) { throw new Error(`Agent manifest must be a YAML object: ${manifestPath}`); } return parsed; From 7957889581227a8bc07f741c2e262dbeee5181d4 Mon Sep 17 00:00:00 2001 From: Truong Nguyen Date: Thu, 23 Apr 2026 04:35:06 +0700 Subject: [PATCH 03/17] test(e2e): add diagnostics, debug tarball, and credential E2E tests (#2243) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add `test/e2e/test-diagnostics.sh` with 5 end-to-end test cases covering CLI version output, debug snapshots, credential sanitization in debug tarballs, sandbox inference config visibility, and credential list safety. Integrates as `diagnostics-e2e` job in `nightly-e2e.yaml`. ## Related Issue Closes #2242 ## Changes - **Version output (TC-DIAG-04):** Runs `nemoclaw --version`, verifies output matches semver pattern and exits with code 0. No sandbox needed. - **Quick debug snapshot (TC-DIAG-02):** Runs `nemoclaw debug --quick --output `, verifies a non-empty archive is produced within 30 seconds. No sandbox needed. - **Full debug tarball + credential sanitization (TC-DIAG-01):** Runs `nemoclaw debug --output `, extracts the tarball, greps all files for the real API key value and `nvapi-` patterns. Verifies no credentials found in any collected file. - **Sandbox inference config (TC-DIAG-05):** Reads `openclaw.json` inside the sandbox via SSH, verifies the model field is present. Runs `nemoclaw status` from the host and verifies the Model field appears in output. - **Credentials list safety (TC-DIAG-03):** Runs `nemoclaw credentials list`, verifies key names are shown but the real API key value is not exposed. Handles the CI case where the credential store is empty (API key passed via env var). - **Nightly integration:** Added `diagnostics-e2e` job to `nightly-e2e.yaml` with 45-minute timeout, artifact upload on failure, and wired into `notify-on-failure`. ## Type of Change - Code change (feature, bug fix, or refactor) ## Verification - `npx prek run --all-files` passes - Tests added or updated for new or changed behavior - No secrets, API keys, or credentials committed - Verified on CI: all PASS on ubuntu-latest ## AI Disclosure - AI-assisted — tool: Cursor --- Signed-off-by: Truong Nguyen Made with [Cursor](https://cursor.com) ## Summary by CodeRabbit * **Tests** * Added an end-to-end diagnostics test suite covering version checks, debug/export operations, credential handling (preventing secret leakage), sandbox onboarding/status, per-test PASS/FAIL/SKIP reporting, timestamped logs, and overall timeout support. * **Chores** * Nightly CI now runs diagnostics, uploads diagnostic logs on failure, and extends failure notifications so diagnostics failures trigger the existing alert workflow. Signed-off-by: Truong Nguyen --- .github/workflows/nightly-e2e.yaml | 32 +- test/e2e/test-diagnostics.sh | 452 +++++++++++++++++++++++++++++ 2 files changed, 482 insertions(+), 2 deletions(-) create mode 100755 test/e2e/test-diagnostics.sh diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index d622edda520..2e98ed94465 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -437,6 +437,34 @@ jobs: path: test-deployment-services-*.log if-no-files-found: ignore + # ── Diagnostics E2E ───────────────────────────────────────── + # TC-DIAG-04: nemoclaw --version, TC-DIAG-02: debug --quick, + # TC-DIAG-01: debug tarball + credential sanitization, + # TC-DIAG-05: sandbox config, TC-DIAG-03: credentials list + diagnostics-e2e: + if: github.repository == 'NVIDIA/NemoClaw' + runs-on: ubuntu-latest + timeout-minutes: 45 + steps: + - name: Checkout + uses: actions/checkout@v6 + + - name: Run diagnostics E2E test + env: + NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} + NEMOCLAW_NON_INTERACTIVE: "1" + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + NEMOCLAW_RECREATE_SANDBOX: "1" + run: bash test/e2e/test-diagnostics.sh + + - name: Upload test log on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: diagnostics-test-log + path: test-diagnostics-*.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. @@ -637,8 +665,8 @@ jobs: notify-on-failure: runs-on: ubuntu-latest - needs: [cloud-e2e, cloud-experimental-e2e, messaging-providers-e2e, token-rotation-e2e, sandbox-survival-e2e, hermes-e2e, skip-permissions-e2e, sandbox-operations-e2e, inference-routing-e2e, network-policy-e2e, deployment-services-e2e, snapshot-commands-e2e, shields-config-e2e, rebuild-openclaw-e2e, upgrade-stale-sandbox-e2e, rebuild-hermes-e2e, gpu-e2e] - if: ${{ always() && (needs.cloud-e2e.result == 'failure' || needs.cloud-experimental-e2e.result == 'failure' || needs.messaging-providers-e2e.result == 'failure' || needs.token-rotation-e2e.result == 'failure' || needs.sandbox-survival-e2e.result == 'failure' || needs.hermes-e2e.result == 'failure' || needs.skip-permissions-e2e.result == 'failure' || needs.sandbox-operations-e2e.result == 'failure' || needs.inference-routing-e2e.result == 'failure' || needs.network-policy-e2e.result == 'failure' || needs.deployment-services-e2e.result == 'failure' || needs.snapshot-commands-e2e.result == 'failure' || needs.shields-config-e2e.result == 'failure' || needs.rebuild-openclaw-e2e.result == 'failure' || needs.upgrade-stale-sandbox-e2e.result == 'failure' || needs.rebuild-hermes-e2e.result == 'failure' || needs.gpu-e2e.result == 'failure') }} + needs: [cloud-e2e, cloud-experimental-e2e, messaging-providers-e2e, token-rotation-e2e, sandbox-survival-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, gpu-e2e] + if: ${{ always() && (needs.cloud-e2e.result == 'failure' || needs.cloud-experimental-e2e.result == 'failure' || needs.messaging-providers-e2e.result == 'failure' || needs.token-rotation-e2e.result == 'failure' || needs.sandbox-survival-e2e.result == 'failure' || needs.hermes-e2e.result == 'failure' || needs.skip-permissions-e2e.result == 'failure' || needs.sandbox-operations-e2e.result == 'failure' || needs.inference-routing-e2e.result == 'failure' || needs.network-policy-e2e.result == 'failure' || needs.deployment-services-e2e.result == 'failure' || needs.diagnostics-e2e.result == 'failure' || needs.snapshot-commands-e2e.result == 'failure' || needs.shields-config-e2e.result == 'failure' || needs.rebuild-openclaw-e2e.result == 'failure' || needs.upgrade-stale-sandbox-e2e.result == 'failure' || needs.rebuild-hermes-e2e.result == 'failure' || needs.gpu-e2e.result == 'failure') }} permissions: issues: write steps: diff --git a/test/e2e/test-diagnostics.sh b/test/e2e/test-diagnostics.sh new file mode 100755 index 00000000000..49e93909e34 --- /dev/null +++ b/test/e2e/test-diagnostics.sh @@ -0,0 +1,452 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# ============================================================================= +# test-diagnostics.sh +# NemoClaw Diagnostics & Credential E2E Tests +# +# Covers: +# TC-DIAG-04: nemoclaw --version (semver output, exit 0) +# TC-DIAG-02: nemoclaw debug --quick (fast, non-empty archive) +# TC-DIAG-01: nemoclaw debug --output (tarball, no credentials in archive) +# TC-DIAG-05: /nemoclaw status inside sandbox (model + provider) +# TC-DIAG-03: credentials list (no values) + credentials reset +# +# Prerequisites: +# - Docker running +# - NVIDIA_API_KEY set +# ============================================================================= + +set -euo pipefail + +# ── Overall timeout ────────────────────────────────────────────────────────── +if [ -z "${NEMOCLAW_E2E_NO_TIMEOUT:-}" ]; then + export NEMOCLAW_E2E_NO_TIMEOUT=1 + TIMEOUT_SECONDS="${NEMOCLAW_E2E_TIMEOUT_SECONDS:-3600}" + if command -v timeout >/dev/null 2>&1; then + exec timeout -s TERM "$TIMEOUT_SECONDS" bash "$0" "$@" + elif command -v gtimeout >/dev/null 2>&1; then + exec gtimeout -s TERM "$TIMEOUT_SECONDS" bash "$0" "$@" + fi +fi + +# ── Config ─────────────────────────────────────────────────────────────────── +SANDBOX_NAME="e2e-diag" +LOG_FILE="test-diagnostics-$(date +%Y%m%d-%H%M%S).log" +touch "$LOG_FILE" + +if command -v gtimeout >/dev/null 2>&1; then + TIMEOUT_CMD="gtimeout" +elif command -v timeout >/dev/null 2>&1; then + TIMEOUT_CMD="timeout" +else + TIMEOUT_CMD="" +fi + +# ── Colors ─────────────────────────────────────────────────────────────────── +GREEN='\033[0;32m' +RED='\033[0;31m' +YELLOW='\033[1;33m' +CYAN='\033[0;36m' +NC='\033[0m' + +PASS=0 +FAIL=0 +SKIP=0 +TOTAL=0 + +# Log a timestamped message. +log() { echo -e "${CYAN}[$(date +%H:%M:%S)]${NC} $*" | tee -a "$LOG_FILE"; } +# Record a passing assertion. +pass() { + ((PASS += 1)) + ((TOTAL += 1)) + echo -e "${GREEN} PASS${NC} $1" | tee -a "$LOG_FILE" +} +# Record a failing assertion. +fail() { + ((FAIL += 1)) + ((TOTAL += 1)) + echo -e "${RED} FAIL${NC} $1 — $2" | tee -a "$LOG_FILE" +} +# Record a skipped test. +skip() { + ((SKIP += 1)) + ((TOTAL += 1)) + echo -e "${YELLOW} SKIP${NC} $1 — $2" | tee -a "$LOG_FILE" +} + +# ── Resolve repo root ──────────────────────────────────────────────────────── +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)" + +# ── Install NemoClaw if not present ────────────────────────────────────────── +install_nemoclaw() { + export NVM_DIR="${NVM_DIR:-$HOME/.nvm}" + if [ -s "$NVM_DIR/nvm.sh" ]; then + # shellcheck source=/dev/null + . "$NVM_DIR/nvm.sh" + fi + if [ -d "$HOME/.local/bin" ] && [[ ":$PATH:" != *":$HOME/.local/bin:"* ]]; then + export PATH="$HOME/.local/bin:$PATH" + fi + + if command -v nemoclaw >/dev/null 2>&1; then + log "nemoclaw already installed: $(nemoclaw --version 2>/dev/null || echo unknown)" + return + fi + log "=== Installing NemoClaw via install.sh ===" + NEMOCLAW_SANDBOX_NAME="$SANDBOX_NAME" \ + NVIDIA_API_KEY="${NVIDIA_API_KEY:-nvapi-DUMMY-FOR-INSTALL}" \ + NEMOCLAW_NON_INTERACTIVE=1 \ + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ + bash "$REPO_ROOT/install.sh" --non-interactive --yes-i-accept-third-party-software \ + 2>&1 | tee -a "$LOG_FILE" + if [ -f "$HOME/.bashrc" ]; then + # shellcheck source=/dev/null + source "$HOME/.bashrc" 2>/dev/null || true + fi + if ! command -v nemoclaw >/dev/null 2>&1; then + log "ERROR: install.sh failed — nemoclaw not found" + exit 1 + fi +} + +# ── Pre-flight ─────────────────────────────────────────────────────────────── +preflight() { + log "=== Pre-flight checks ===" + if ! docker info >/dev/null 2>&1; then + log "ERROR: Docker is not running." + exit 1 + fi + log "Docker is running" + + local api_key="${NVIDIA_API_KEY:-}" + if [[ -z "$api_key" ]]; then + log "ERROR: NVIDIA_API_KEY not set" + exit 1 + fi + + install_nemoclaw + log "nemoclaw: $(nemoclaw --version 2>/dev/null || echo unknown)" + log "Pre-flight complete" +} + +# Execute a command inside the sandbox via SSH. +sandbox_exec() { + local cmd="$1" + local ssh_cfg + ssh_cfg="$(mktemp)" + if ! openshell sandbox ssh-config "$SANDBOX_NAME" >"$ssh_cfg" 2>/dev/null; then + rm -f "$ssh_cfg" + echo "" + return 1 + fi + local result ssh_exit=0 + result=$(${TIMEOUT_CMD:+$TIMEOUT_CMD 120} ssh -F "$ssh_cfg" \ + -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null \ + -o ConnectTimeout=10 -o LogLevel=ERROR \ + "openshell-${SANDBOX_NAME}" "$cmd" 2>&1) || ssh_exit=$? + rm -f "$ssh_cfg" + echo "$result" + return $ssh_exit +} + +# Onboard a sandbox with default settings. +onboard_sandbox() { + local name="$1" + log " Onboarding sandbox '$name'..." + rm -f "$HOME/.nemoclaw/onboard.lock" 2>/dev/null || true + NEMOCLAW_SANDBOX_NAME="$name" \ + NEMOCLAW_NON_INTERACTIVE=1 \ + NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ + NEMOCLAW_POLICY_TIER="open" \ + ${TIMEOUT_CMD:+$TIMEOUT_CMD 600} nemoclaw onboard --non-interactive --yes-i-accept-third-party-software \ + 2>&1 | tee -a "$LOG_FILE" || { + log "FATAL: Onboard failed for '$name'" + return 1 + } + log " Sandbox '$name' onboarded" +} + +# ============================================================================= +# TC-DIAG-04: nemoclaw --version +# ============================================================================= +test_diag_04_version() { + log "=== TC-DIAG-04: nemoclaw --version ===" + + local version_output version_rc=0 + version_output=$(nemoclaw --version 2>&1) || version_rc=$? + + log " Output: $version_output (exit $version_rc)" + + if [[ $version_rc -ne 0 ]]; then + fail "TC-DIAG-04: Exit code" "nemoclaw --version exited with $version_rc" + return + fi + + if echo "$version_output" | grep -qE '[0-9]+\.[0-9]+\.[0-9]+'; then + pass "TC-DIAG-04: Version output matches semver ($version_output)" + else + fail "TC-DIAG-04: Format" "Output does not match semver pattern: $version_output" + fi +} + +# ============================================================================= +# TC-DIAG-02: nemoclaw debug --quick +# ============================================================================= +test_diag_02_debug_quick() { + log "=== TC-DIAG-02: nemoclaw debug --quick ===" + + local debug_dir + debug_dir=$(mktemp -d) + local output_file="${debug_dir}/quick-debug.tar.gz" + + local start_time + start_time=$(date +%s) + + local debug_output debug_rc=0 + debug_output=$(${TIMEOUT_CMD:+$TIMEOUT_CMD 30} nemoclaw debug --quick --output "$output_file" 2>&1) || debug_rc=$? + + local end_time + end_time=$(date +%s) + local elapsed=$((end_time - start_time)) + + log " Completed in ${elapsed}s (exit $debug_rc)" + log " Output: ${debug_output:0:300}" + + if [[ $debug_rc -ne 0 ]]; then + fail "TC-DIAG-02: Exit code" "debug --quick exited with $debug_rc" + rm -rf "$debug_dir" + return + fi + + if [[ -f "$output_file" ]] && [[ -s "$output_file" ]]; then + pass "TC-DIAG-02: debug --quick produced non-empty archive (${elapsed}s)" + else + fail "TC-DIAG-02: Output" "No archive produced or archive is empty" + fi + + if [[ $elapsed -le 30 ]]; then + pass "TC-DIAG-02: Completed within time limit (${elapsed}s)" + else + fail "TC-DIAG-02: Timing" "Took ${elapsed}s (expected ≤30s)" + fi + + rm -rf "$debug_dir" +} + +# ============================================================================= +# TC-DIAG-01: nemoclaw debug --output (full tarball + credential sanitization) +# ============================================================================= +test_diag_01_debug_tarball() { + log "=== TC-DIAG-01: Full Debug Tarball + Credential Sanitization ===" + + local debug_dir + debug_dir=$(mktemp -d) + local output_file="${debug_dir}/debug-full.tar.gz" + local extract_dir="${debug_dir}/extracted" + + local debug_output debug_rc=0 + debug_output=$(nemoclaw debug --output "$output_file" 2>&1) || debug_rc=$? + log " Debug output (exit $debug_rc): ${debug_output:0:300}" + + if [[ $debug_rc -ne 0 ]] || [[ ! -f "$output_file" ]]; then + fail "TC-DIAG-01: Setup" "debug --output failed or no file produced" + rm -rf "$debug_dir" + return + fi + + pass "TC-DIAG-01: Debug tarball created" + + mkdir -p "$extract_dir" + if ! tar xzf "$output_file" -C "$extract_dir" 2>/dev/null; then + fail "TC-DIAG-01: Extract" "Could not extract tarball" + rm -rf "$debug_dir" + return + fi + + local real_key="${NVIDIA_API_KEY:-}" + if [[ -z "$real_key" ]]; then + skip "TC-DIAG-01: Credential check" "NVIDIA_API_KEY not set" + rm -rf "$debug_dir" + return + fi + + log " Scanning extracted files for credential leaks..." + local leaks + leaks=$(grep -rl "$real_key" "$extract_dir" 2>/dev/null || true) + + if [[ -z "$leaks" ]]; then + pass "TC-DIAG-01: No API key found in debug tarball" + else + fail "TC-DIAG-01: Credential leak" "API key found in: $leaks" + fi + + local pattern_leaks + pattern_leaks=$(grep -rlE "nvapi-[A-Za-z0-9_-]{10,}" "$extract_dir" 2>/dev/null || true) + if [[ -z "$pattern_leaks" ]]; then + pass "TC-DIAG-01: No nvapi- pattern credentials in tarball" + else + fail "TC-DIAG-01: Pattern leak" "nvapi- pattern found in: $pattern_leaks" + fi + + rm -rf "$debug_dir" +} + +# ============================================================================= +# TC-DIAG-05: Sandbox inference config visible inside sandbox +# ============================================================================= +test_diag_05_sandbox_config() { + log "=== TC-DIAG-05: Sandbox Inference Config ===" + + log " Checking openclaw.json config inside sandbox..." + local config_output + config_output=$(sandbox_exec "cat /sandbox/.openclaw/openclaw.json 2>/dev/null" 2>&1) || true + + if [[ -z "$config_output" ]]; then + fail "TC-DIAG-05: Config" "Could not read openclaw.json inside sandbox" + return + fi + + pass "TC-DIAG-05: openclaw.json readable inside sandbox" + + log " Checking nemoclaw status from host..." + local status_output + status_output=$(nemoclaw "$SANDBOX_NAME" status 2>&1) || true + if echo "$status_output" | grep -qiE "Model.*nemotron\|Model.*nvidia\|Model.*llama"; then + pass "TC-DIAG-05: nemoclaw status shows model info" + elif echo "$status_output" | grep -qi "Model"; then + pass "TC-DIAG-05: nemoclaw status shows Model field" + else + fail "TC-DIAG-05: Status" "No model info in nemoclaw status output" + fi +} + +# ============================================================================= +# TC-DIAG-03: credentials list + credentials reset +# ============================================================================= +test_diag_03_credentials() { + log "=== TC-DIAG-03: Credentials List and Reset ===" + + local real_key="${NVIDIA_API_KEY:-}" + + log " Step 1: Running credentials list..." + local list_output list_rc=0 + list_output=$(nemoclaw credentials list 2>&1) || list_rc=$? + log " List output (exit $list_rc): ${list_output:0:400}" + + if [[ $list_rc -ne 0 ]]; then + fail "TC-DIAG-03: List" "credentials list exited with $list_rc" + return + fi + + if echo "$list_output" | grep -qi "No stored credentials"; then + pass "TC-DIAG-03: credentials list works (store empty — API key passed via env on CI)" + + log " Step 2: Verifying credentials list does not leak env var..." + if [[ -n "$real_key" ]] && echo "$list_output" | grep -qF "$real_key"; then + fail "TC-DIAG-03: Value leak" "Real API key visible in credentials list output" + else + pass "TC-DIAG-03: credentials list does not expose env key values" + fi + return + fi + + if echo "$list_output" | grep -qiE "NVIDIA_API_KEY\|nvidia.api"; then + pass "TC-DIAG-03: credentials list shows key name" + else + skip "TC-DIAG-03: Key name" "Expected credential key not found in list" + return + fi + + if [[ -n "$real_key" ]] && echo "$list_output" | grep -qF "$real_key"; then + fail "TC-DIAG-03: Value leak" "Real API key value visible in credentials list" + else + pass "TC-DIAG-03: credentials list does not expose key values" + fi + + log " Step 2: Running credentials reset NVIDIA_API_KEY..." + local reset_output reset_rc=0 + reset_output=$(nemoclaw credentials reset NVIDIA_API_KEY --yes 2>&1) || reset_rc=$? + log " Reset output (exit $reset_rc): ${reset_output:0:300}" + + if [[ $reset_rc -eq 0 ]]; then + pass "TC-DIAG-03: credentials reset completed" + else + fail "TC-DIAG-03: Reset" "credentials reset failed (exit $reset_rc)" + return + fi + + log " Step 3: Verifying key removed from list..." + local post_list + post_list=$(nemoclaw credentials list 2>&1) || true + if echo "$post_list" | grep -qiE "NVIDIA_API_KEY"; then + fail "TC-DIAG-03: Post-reset" "NVIDIA_API_KEY still in list after reset" + else + pass "TC-DIAG-03: NVIDIA_API_KEY removed after reset" + fi +} + +# Clean up sandbox and services on exit. +teardown() { + set +e + rm -f "$HOME/.nemoclaw/onboard.lock" 2>/dev/null || true + nemoclaw "$SANDBOX_NAME" destroy --yes 2>/dev/null || true + set -e +} + +# Print final PASS/FAIL/SKIP counts and exit. +summary() { + echo "" + echo "============================================================" + echo " Diagnostics E2E Results" + echo "============================================================" + echo -e " ${GREEN}PASS: $PASS${NC}" + echo -e " ${RED}FAIL: $FAIL${NC}" + echo -e " ${YELLOW}SKIP: $SKIP${NC}" + echo " TOTAL: $TOTAL" + echo "============================================================" + echo " Log: $LOG_FILE" + echo "============================================================" + echo "" + + if [[ $FAIL -gt 0 ]]; then + exit 1 + fi + exit 0 +} + +# Entry point: preflight → tests → summary. +main() { + echo "" + echo "============================================================" + echo " NemoClaw Diagnostics E2E Tests" + echo " $(date)" + echo "============================================================" + echo "" + + preflight + + # No sandbox needed + test_diag_04_version + test_diag_02_debug_quick + + # Onboard sandbox for remaining tests + log "=== Onboarding sandbox ===" + if ! onboard_sandbox "$SANDBOX_NAME"; then + log "FATAL: Could not onboard sandbox" + exit 1 + fi + + test_diag_01_debug_tarball + test_diag_05_sandbox_config + test_diag_03_credentials # modifies state — runs last + + teardown + trap - EXIT + summary +} + +trap teardown EXIT +main "$@" From 458fe7a079c187c366c279b1ea987ff8fc9afca1 Mon Sep 17 00:00:00 2001 From: Truong Nguyen Date: Thu, 23 Apr 2026 04:36:17 +0700 Subject: [PATCH 04/17] fix(e2e): switch TC-NET-03/04 to non-base-policy endpoints (#2275) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fix TC-NET-03 always skipping on nightly by switching from a base-policy-whitelisted endpoint to one only reachable via preset. ## Related Issue Fixes #2274 ## Changes - **TC-NET-03:** Switched from `api.telegram.org` (whitelisted in base sandbox policy, always reachable) to `slack.com` (only reachable after applying slack preset). The blocked → add preset → reachable flow is now actually tested. - **TC-NET-04:** Switched from `slack.com` to `api.atlassian.com` (jira preset) because TC-NET-03 now applies the slack preset earlier in the suite. Each test uses a unique endpoint to avoid interference. - **Nightly workflow:** Added `NEMOCLAW_POLICY_TIER: restricted` to the `network-policy-e2e` job env to ensure no presets are applied during initial setup. - **install_nemoclaw():** Added `NEMOCLAW_POLICY_TIER=restricted` to the install.sh call so the initial onboard also uses restricted tier. ## Type of Change - Code change (feature, bug fix, or refactor) ## Verification - `npx prek run --all-files` passes - Verified on CI: 10/10 PASS, 0 SKIP on ubuntu-latest ## AI Disclosure - AI-assisted — tool: Cursor --- Signed-off-by: Truong Nguyen Made with [Cursor](https://cursor.com) ## Summary by CodeRabbit * **Tests** * E2E network-policy tests updated to target different service integrations (Slack and Atlassian) and to align their checks and messages with those integrations. * **Chores** * Installation and test environments standardized to use the "restricted" policy tier. Signed-off-by: Truong Nguyen --- .github/workflows/nightly-e2e.yaml | 1 + test/e2e/test-network-policy.sh | 37 +++++++++++++++--------------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index 2e98ed94465..e9fb6e303d5 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -400,6 +400,7 @@ jobs: NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} NEMOCLAW_NON_INTERACTIVE: "1" NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE: "1" + NEMOCLAW_POLICY_TIER: "restricted" run: bash test/e2e/test-network-policy.sh - name: Upload test log on failure diff --git a/test/e2e/test-network-policy.sh b/test/e2e/test-network-policy.sh index 5b667c86f55..2df01c7e23d 100755 --- a/test/e2e/test-network-policy.sh +++ b/test/e2e/test-network-policy.sh @@ -9,7 +9,7 @@ # Covers: # TC-NET-01: Deny-by-default egress (blocked URL returns 403) # TC-NET-02: Whitelisted endpoint access (PyPI reachable via pip) -# TC-NET-03: Live policy-add without restart (telegram preset) +# TC-NET-03: Live policy-add without restart (slack preset) # TC-NET-04: policy-add --dry-run (no changes applied) # TC-NET-05: Hot-reload (policy change without sandbox restart) # TC-NET-06: Permissive policy mode (open all egress) @@ -103,6 +103,7 @@ install_nemoclaw() { NVIDIA_API_KEY="${NVIDIA_API_KEY:-nvapi-DUMMY-FOR-INSTALL}" \ NEMOCLAW_NON_INTERACTIVE=1 \ NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ + NEMOCLAW_POLICY_TIER="restricted" \ bash "$REPO_ROOT/install.sh" --non-interactive --yes-i-accept-third-party-software \ 2>&1 | tee -a "$LOG_FILE" if [ -f "$HOME/.bashrc" ]; then @@ -293,9 +294,9 @@ test_net_02_whitelist_access() { test_net_03_live_policy_add() { log "=== TC-NET-03: Live Policy-Add Without Restart ===" - local target_url="https://api.telegram.org/" + local target_url="https://slack.com/" - log " Step 1: Verify api.telegram.org is blocked before policy-add..." + log " Step 1: Verify slack.com is blocked before policy-add..." local before before=$(sandbox_exec "node -e \" fetch('$target_url', {signal: AbortSignal.timeout(15000)}) @@ -304,18 +305,18 @@ fetch('$target_url', {signal: AbortSignal.timeout(15000)}) \"" 2>&1) || true log " Before policy-add: $before" - if echo "$before" | grep -qE "STATUS_[2-4][0-9][0-9]"; then - skip "TC-NET-03" "api.telegram.org already reachable before policy-add (preset may be pre-applied)" + if echo "$before" | grep -qE "STATUS_[23][0-9][0-9]"; then + skip "TC-NET-03" "slack.com already reachable before policy-add (preset may be pre-applied)" return fi - log " Step 2: Adding telegram preset (interactive mode)..." + log " Step 2: Adding slack preset (interactive mode)..." local interactive_rc=0 - apply_preset_interactive "telegram" || interactive_rc=$? + apply_preset_interactive "slack" || interactive_rc=$? if [[ $interactive_rc -eq 2 ]]; then log " Interactive mode unavailable (expect missing) — falling back to non-interactive..." - if ! apply_preset "telegram"; then - fail "TC-NET-03: Setup" "Could not apply telegram preset" + if ! apply_preset "slack"; then + fail "TC-NET-03: Setup" "Could not apply slack preset" return fi elif [[ $interactive_rc -ne 0 ]]; then @@ -325,7 +326,7 @@ fetch('$target_url', {signal: AbortSignal.timeout(15000)}) sleep 5 - log " Step 3: Verify api.telegram.org is reachable after policy-add..." + log " Step 3: Verify slack.com is reachable after policy-add..." local after after=$(sandbox_exec "node -e \" fetch('$target_url', {signal: AbortSignal.timeout(30000)}) @@ -337,7 +338,7 @@ fetch('$target_url', {signal: AbortSignal.timeout(30000)}) if echo "$after" | grep -qE "STATUS_[2-4][0-9][0-9]"; then pass "TC-NET-03: Endpoint reachable after live policy-add ($after)" elif echo "$after" | grep -qE "ERROR_"; then - fail "TC-NET-03: Live policy-add" "api.telegram.org still proxy-blocked after policy-add ($after)" + fail "TC-NET-03: Live policy-add" "slack.com still proxy-blocked after policy-add ($after)" else fail "TC-NET-03: Live policy-add" "Unexpected response after policy-add ($after)" fi @@ -349,9 +350,9 @@ fetch('$target_url', {signal: AbortSignal.timeout(30000)}) test_net_04_dry_run() { log "=== TC-NET-04: Policy-Add --dry-run ===" - local target_url="https://slack.com/" + local target_url="https://api.atlassian.com/" - log " Step 1: Verify slack.com is blocked..." + log " Step 1: Verify api.atlassian.com is blocked..." local before before=$(sandbox_exec "node -e \" fetch('$target_url', {signal: AbortSignal.timeout(15000)}) @@ -360,18 +361,18 @@ fetch('$target_url', {signal: AbortSignal.timeout(15000)}) \"" 2>&1) || true log " Before dry-run: $before" - log " Step 2: Running policy-add --dry-run slack..." + log " Step 2: Running policy-add --dry-run jira..." local dry_output dry_rc=0 - dry_output=$(nemoclaw "$SANDBOX_NAME" policy-add slack --dry-run 2>&1) || dry_rc=$? + dry_output=$(nemoclaw "$SANDBOX_NAME" policy-add jira --dry-run 2>&1) || dry_rc=$? log " Dry-run output (exit $dry_rc): ${dry_output:0:300}" - if [[ $dry_rc -eq 0 ]] && echo "$dry_output" | grep -qiE "slack\.com|would be opened"; then + if [[ $dry_rc -eq 0 ]] && echo "$dry_output" | grep -qiE "atlassian|would be opened"; then pass "TC-NET-04: Dry-run printed endpoint info" else fail "TC-NET-04: Dry-run output" "Expected endpoint info in output: ${dry_output:0:200}" fi - log " Step 3: Verify slack.com is still blocked after dry-run..." + log " Step 3: Verify api.atlassian.com is still blocked after dry-run..." local after after=$(sandbox_exec "node -e \" fetch('$target_url', {signal: AbortSignal.timeout(15000)}) @@ -383,7 +384,7 @@ fetch('$target_url', {signal: AbortSignal.timeout(15000)}) if echo "$after" | grep -qE "STATUS_403|ERROR_"; then pass "TC-NET-04: Policy unchanged after dry-run (blocked: $after)" elif echo "$after" | grep -qE "STATUS_[23]"; then - fail "TC-NET-04: Dry-run side effect" "slack.com reachable after dry-run (policy was modified)" + fail "TC-NET-04: Dry-run side effect" "api.atlassian.com reachable after dry-run (policy was modified)" else fail "TC-NET-04: Dry-run verification" "Unexpected response ($after)" fi From 59d3115f96ebb00e69d700cd146012b563f7d5b7 Mon Sep 17 00:00:00 2001 From: Tinson Lai Date: Thu, 23 Apr 2026 05:37:29 +0800 Subject: [PATCH 05/17] fix(install): invoke install-openshell.sh from install_nemoclaw (#2279) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit install.sh previously delegated all OpenShell CLI install/upgrade to `nemoclaw onboard`. When onboard is skipped (host preflight blocks, user aborts, interrupted session) the version gate never runs, so a curl|bash upgrade could leave openshell stale even though the new NemoClaw release declares a higher min_openshell_version. Run install-openshell.sh at the tail of install_nemoclaw instead. The script is idempotent — it exits 0 if the installed version is already within [MIN_VERSION, MAX_VERSION], so calling it every install is free on the happy path. The onboard-side check at onboard.ts:2664-2706 stays in place as a safety net for direct `nemoclaw onboard` invocations outside the installer. Fixes #2272. ## Summary ## Related Issue ## Changes ## Type of Change - [X] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [X] `npx prek run --all-files` passes - [X] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [X] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [X] AI-assisted — tool: Claude Code --- Signed-off-by: Tinson Lai ## Summary by CodeRabbit * **Chores** * Installer now runs the OpenShell CLI installation/upgrade during initial setup so it’s available immediately instead of waiting for a later onboarding step. * OpenShell installation/upgrade is attempted on every install to ensure the CLI is present and kept up to date. --------- Signed-off-by: Tinson Lai Co-authored-by: Claude --- scripts/install.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/scripts/install.sh b/scripts/install.sh index 6ddbbf1cc67..de0ac627cfb 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -1032,6 +1032,16 @@ install_nemoclaw() { spin "Building NemoClaw CLI modules" bash -c "cd \"$nemoclaw_src\" && npm run --if-present build:cli" spin "Building NemoClaw plugin" bash -c "cd \"$nemoclaw_src\"/nemoclaw && npm install --ignore-scripts && npm run build" spin "Linking NemoClaw CLI" bash -c "cd \"$nemoclaw_src\" && npm link" + + # Install/upgrade the OpenShell CLI on the GitHub-clone path (curl|bash). + # Without this, install.sh defers the openshell version gate entirely to + # `nemoclaw onboard`, so any later skip of onboard (preflight blocking, + # interrupted session) leaves openshell stale below blueprint's + # min_openshell_version even though the new NemoClaw declared a higher + # floor. The source-checkout branch intentionally skips this — a developer + # running ./scripts/install.sh manages their own openshell. The script is + # idempotent on the happy path. See #2272. + spin "Installing OpenShell CLI" bash "${NEMOCLAW_SOURCE_ROOT}/scripts/install-openshell.sh" fi refresh_path From 3edf6b396593c57735c29e5ddb22305490e7565a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aaron=20Erickson=20=F0=9F=A6=9E?= Date: Wed, 22 Apr 2026 15:13:45 -0700 Subject: [PATCH 06/17] fix(rebuild): forward stored --from Dockerfile path to onboard on rebuild (#2302) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - `sandboxRebuild()` called `onboard({ resume: true })` without passing the session's stored `fromDockerfile`, causing the conflict check to reject the resume (`requestedFrom=null` vs `recordedFrom="/path/to/Dockerfile"`) - This made `nemoclaw rebuild` and `nemoclaw upgrade-sandboxes` fail unconditionally for any sandbox created with `--from` - Fix reads the stored `fromDockerfile` from session metadata and passes it through to `onboard()` Fixes #2301 ## Test plan - [x] New test: rebuild does not hit fromDockerfile conflict when session has a stored `--from` path - [x] Existing #2201 regression tests still pass (agent syncing unaffected) - [x] All 3 rebuild tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Bug Fixes** * Fixed rebuild operations to properly preserve and use the stored Dockerfile source configuration, preventing errors when resuming sessions. Co-authored-by: Claude Opus 4.6 (1M context) --- src/nemoclaw.ts | 8 +++++++- test/repro-2201.test.ts | 32 +++++++++++++++++++++++++++++--- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/nemoclaw.ts b/src/nemoclaw.ts index daa4f55bd96..29337185c14 100644 --- a/src/nemoclaw.ts +++ b/src/nemoclaw.ts @@ -2417,7 +2417,12 @@ async function sandboxRebuild(sandboxName, args = [], opts = {}) { log( `Env: NEMOCLAW_SANDBOX_NAME=${process.env.NEMOCLAW_SANDBOX_NAME}, NEMOCLAW_RECREATE_SANDBOX=${process.env.NEMOCLAW_RECREATE_SANDBOX}`, ); - log("Calling onboard({ resume: true, nonInteractive: true, recreateSandbox: true })"); + + // Forward the stored --from Dockerfile path so onboard --resume uses the + // same custom image. Without this, the conflict check rejects the resume + // because requestedFrom (null) !== recordedFrom (the stored path). (#2301) + const storedFromDockerfile = sessionAfter?.metadata?.fromDockerfile || null; + log(`Calling onboard({ resume: true, nonInteractive: true, recreateSandbox: true, fromDockerfile: ${storedFromDockerfile} })`); const { onboard } = require("./lib/onboard"); await onboard({ @@ -2425,6 +2430,7 @@ async function sandboxRebuild(sandboxName, args = [], opts = {}) { nonInteractive: true, recreateSandbox: true, agent: rebuildAgent, + fromDockerfile: storedFromDockerfile, }); log("onboard() returned successfully"); diff --git a/test/repro-2201.test.ts b/test/repro-2201.test.ts index 17e8a084f96..7847f9fc3c6 100644 --- a/test/repro-2201.test.ts +++ b/test/repro-2201.test.ts @@ -55,9 +55,11 @@ afterEach(() => { function createFixture({ rebuildTarget, lastOnboarded, + fromDockerfile = null, }: { rebuildTarget: { name: string; agent: string | null }; lastOnboarded: { name: string; agent: string | null }; + fromDockerfile?: string | null; }) { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-2201-")); tmpFixtures.push(tmpDir); @@ -96,7 +98,7 @@ function createFixture({ provider: "p", model: "m", endpointUrl: null, credentialEnv: null, preferredInferenceApi: null, nimContainer: null, webSearchConfig: null, policyPresets: [], messagingChannels: null, - metadata: { gatewayName: "nemoclaw", fromDockerfile: null }, + metadata: { gatewayName: "nemoclaw", fromDockerfile: fromDockerfile }, steps: { preflight: { status: "complete", startedAt: null, completedAt: null, error: null }, gateway: { status: "complete", startedAt: null, completedAt: null, error: null }, @@ -188,9 +190,13 @@ function runRebuild(fixture: ReturnType) { ); } -function readSessionAgent(fixture: ReturnType): unknown { +function readSession(fixture: ReturnType): Record { const p = path.join(fixture.nemoclawDir, "onboard-session.json"); - return JSON.parse(fs.readFileSync(p, "utf-8")).agent; + return JSON.parse(fs.readFileSync(p, "utf-8")); +} + +function readSessionAgent(fixture: ReturnType): unknown { + return readSession(fixture).agent; } describe("Issue #2201: rebuild syncs agent from registry, not stale session", () => { @@ -220,3 +226,23 @@ describe("Issue #2201: rebuild syncs agent from registry, not stale session", () expect(readSessionAgent(f)).toBe("hermes"); }); }); + +describe("Issue #2301: rebuild forwards stored --from Dockerfile to onboard", () => { + it("rebuild does not hit fromDockerfile conflict when session has a stored --from path", + { timeout: 60_000 }, () => { + // Scenario: user onboarded with --from /path/to/Dockerfile, then + // runs rebuild. Without the fix, onboard's conflict check sees + // requestedFrom=null vs recordedFrom="/path/to/Dockerfile" and + // exits with a conflict error. + const f = createFixture({ + rebuildTarget: { name: "openclaw", agent: null }, + lastOnboarded: { name: "openclaw", agent: null }, + fromDockerfile: "/tmp/custom/Dockerfile", + }); + const result = runRebuild(f); + // Without fix: exits with "Session was started with --from ..." + // With fix: rebuild proceeds past conflict check (may still fail + // later in the fake-env backup step — that's expected with stubs). + expect(result.stderr).not.toMatch(/Session was started with --from/); + }); +}); From 752bfb3ab37eeacffd679043bb6598e64bb5f05b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aaron=20Erickson=20=F0=9F=A6=9E?= Date: Wed, 22 Apr 2026 15:29:54 -0700 Subject: [PATCH 07/17] fix(sandbox): add WebSocket CONNECT tunnel preload for Discord gateway (#2296) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Add `ws-proxy-fix.ts` preload script that patches `https.request()` to detect WebSocket upgrade requests and inject a CONNECT tunnel agent, fixing Discord gateway connections through the OpenShell L7 proxy - Wire the preload into `nemoclaw-start.sh` (entrypoint + proxy-env.sh persistence for connect sessions) - Add blueprint TypeScript compilation infrastructure (`tsconfig.json`, `build:blueprint` script) Closes #1570 ## Details Node.js 22's `EnvHttpProxyAgent` (activated by `NODE_USE_ENV_PROXY=1`) sends forward proxy requests instead of CONNECT tunnels for HTTPS WebSocket upgrades. The OpenShell L7 proxy correctly rejects these with HTTP 400, breaking the Discord gateway connection. The preload intercepts `https.request()` calls that contain an `Upgrade: websocket` header and replaces the default agent with one that establishes a proper CONNECT tunnel through the proxy, then upgrades to TLS. Non-WebSocket HTTPS requests pass through unchanged. Belt-and-suspenders: if the caller (OpenClaw) already provides a custom agent, the preload steps aside — no double-tunnelling. Works regardless of any upstream OpenClaw fix. ## Test plan - [x] `npx vitest run test/service-env.test.ts` — 39 tests pass (5 new) - [x] `npm run build:blueprint` — clean TS compilation - [x] `npm run typecheck:cli` — clean type-check - [x] `npx prek run --all-files` — all relevant hooks pass (test-cli failures are pre-existing environmental) - [ ] E2E: deploy sandbox with Discord channel, verify gateway connects ## Summary by CodeRabbit * **New Features** * Adds a preload that enables HTTPS-proxy CONNECT tunneling for Discord gateway WebSocket upgrades when a proxy is configured * Start script now auto-applies the preload to launched sessions * **Bug Fixes** * Improved handling of WebSocket-upgrade requests over HTTPS proxies to prevent connection failures and port duplication * **Chores** * Blueprint compilation added to the CLI build * **Tests** * Added unit and e2e tests covering proxy injection and gateway handshake flows --- nemoclaw-blueprint/scripts/ws-proxy-fix.js | 177 ++++++++++++++++ nemoclaw-blueprint/scripts/ws-proxy-fix.ts | 225 +++++++++++++++++++++ nemoclaw-blueprint/tsconfig.json | 15 ++ package.json | 2 +- scripts/nemoclaw-start.sh | 17 ++ test/e2e/test-messaging-providers.sh | 198 ++++++++++++++++++ test/service-env.test.ts | 218 ++++++++++++++++++++ tsconfig.cli.json | 2 +- 8 files changed, 852 insertions(+), 2 deletions(-) create mode 100644 nemoclaw-blueprint/scripts/ws-proxy-fix.js create mode 100644 nemoclaw-blueprint/scripts/ws-proxy-fix.ts create mode 100644 nemoclaw-blueprint/tsconfig.json diff --git a/nemoclaw-blueprint/scripts/ws-proxy-fix.js b/nemoclaw-blueprint/scripts/ws-proxy-fix.js new file mode 100644 index 00000000000..80865ab1aa3 --- /dev/null +++ b/nemoclaw-blueprint/scripts/ws-proxy-fix.js @@ -0,0 +1,177 @@ +"use strict"; +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// ws-proxy-fix.ts — preload script to fix Discord WebSocket connections +// through the OpenShell L7 proxy when HTTPS_PROXY is set. +// +// Problem (NemoClaw#1570): +// The `ws` library (used by OpenClaw's Discord extension via @buape/carbon) +// establishes WebSocket connections by calling https.request() for wss:// URLs. +// Inside the sandbox, HTTPS_PROXY is set and Node.js 22 (with +// NODE_USE_ENV_PROXY=1) routes these through EnvHttpProxyAgent — which sends a +// forward proxy request (GET https://...) instead of a CONNECT tunnel. The +// OpenShell L7 proxy correctly rejects forward proxy HTTPS with HTTP 400. +// Without NODE_USE_ENV_PROXY, ws goes direct, which the sandbox network +// namespace blocks. Either way, the WebSocket handshake fails and the bot +// loops on close code 1006. +// +// Fix: +// Patch https.request() to detect WebSocket upgrade requests to Discord +// gateway hosts (gateway.discord.gg) and inject an agent that issues a proper +// CONNECT request to the proxy, then upgrades the tunnel socket to TLS. +// All other HTTPS requests — including non-Discord WebSockets — pass through +// completely untouched. +// +// Uses only Node.js built-in modules — no external dependencies. +// +// Belt-and-suspenders: works regardless of any upstream OpenClaw changes. +// If the caller already provides a custom (non-default) agent, we step aside +// — no double-tunnelling. +var __importDefault = (this && this.__importDefault) || function (mod) { + return (mod && mod.__esModule) ? mod : { "default": mod }; +}; +Object.defineProperty(exports, "__esModule", { value: true }); +const node_http_1 = __importDefault(require("node:http")); +const node_net_1 = __importDefault(require("node:net")); +const node_tls_1 = __importDefault(require("node:tls")); +const node_https_1 = __importDefault(require("node:https")); +const node_url_1 = require("node:url"); +const _PATCHED = Symbol.for("nemoclaw.wsProxyFix"); +/** + * Self-executing initialiser. Using an IIFE rather than top-level `return` + * keeps the source valid TypeScript while preserving early-exit semantics. + */ +(function wsProxyFixInit() { + const proxyUrl = process.env.HTTPS_PROXY || process.env.https_proxy; + if (!proxyUrl) + return; + if (globalThis[_PATCHED]) + return; + let proxy; + try { + proxy = new node_url_1.URL(proxyUrl); + } + catch { + return; + } + const proxyHost = proxy.hostname; + const proxyPort = parseInt(proxy.port, 10) || 3128; + // ---------- CONNECT tunnel agent ---------------------------------------- + /** + * Create an https.Agent whose createConnection() establishes a CONNECT + * tunnel through the HTTP proxy, then upgrades to TLS — the correct + * behaviour that EnvHttpProxyAgent fails to perform for HTTPS. + */ + function createTunnelAgent(targetHost, targetPort) { + const agent = new node_https_1.default.Agent({ keepAlive: false, maxSockets: 1 }); + // Override createConnection to route through the proxy's CONNECT tunnel. + // The typing is intentionally loosened because the actual Node.js runtime + // signature is broader than what @types/node declares. + agent.createConnection = function (options, callback) { + const connectReq = node_http_1.default.request({ + host: proxyHost, + port: proxyPort, + method: "CONNECT", + path: `${targetHost}:${targetPort}`, + headers: { Host: `${targetHost}:${targetPort}` }, + }); + connectReq.on("connect", (_res, socket, head) => { + if (_res.statusCode !== 200) { + socket.destroy(); + callback(new Error(`ws-proxy-fix: CONNECT ${targetHost}:${targetPort} via proxy failed (${_res.statusCode})`)); + return; + } + // Preserve any bytes already buffered from the tunnel before TLS. + if (head && head.length > 0) { + socket.unshift(head); + } + const tlsSocket = node_tls_1.default.connect({ + socket, + servername: options.servername || targetHost, + }); + callback(null, tlsSocket); + }); + connectReq.on("error", (err) => { + connectReq.destroy(); + callback(err); + }); + connectReq.end(); + // createConnection expects a synchronous return; the real socket arrives + // via the callback. Return a placeholder that Node.js will discard. + return new node_net_1.default.Socket(); + }; + return agent; + } + // ---------- Target check ------------------------------------------------- + /** + * Return true only for WebSocket upgrade requests targeting Discord + * gateway hosts (gateway.discord.gg and regional variants). + */ + function isDiscordWsUpgrade(host, headers) { + if (!host || !headers || typeof headers !== "object") + return false; + const h = host.toLowerCase(); + if (h !== "gateway.discord.gg" && !h.endsWith(".discord.gg")) + return false; + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === "upgrade" && + String(headers[key]).toLowerCase() === "websocket") { + return true; + } + } + return false; + } + // ---------- Patch https.request() --------------------------------------- + // Capture the original — typed as a loose callable so we can invoke it + // with the normalised (options, cb) form without fighting overload resolution. + const origRequest = node_https_1.default.request; + function wsProxyFixedRequest(input, options, callback) { + // --- Normalise arguments (Node.js accepts multiple call signatures) --- + let opts; + let cb; + if (typeof input === "string" || input instanceof node_url_1.URL) { + if (typeof options === "function") { + cb = options; + opts = {}; + } + else { + opts = options || {}; + cb = callback; + } + const url = typeof input === "string" ? new node_url_1.URL(input) : input; + opts = { + protocol: url.protocol, + hostname: url.hostname, + port: url.port, + path: url.pathname + url.search, + ...opts, + }; + } + else { + opts = input || {}; + cb = typeof options === "function" ? options : callback; + } + // opts.host may include a port (e.g. "gateway.discord.gg:443") — strip it + // so the CONNECT path doesn't become "host:443:443". + let host = opts.hostname || undefined; + if (!host && opts.host) { + host = opts.host.replace(/:\d+$/, ""); + } + if (isDiscordWsUpgrade(host, opts.headers)) { + // Discord WebSocket upgrade — inject CONNECT tunnel agent unless the + // caller already provides a custom (non-default) agent. + if (!opts.agent || opts.agent === node_https_1.default.globalAgent) { + const port = parseInt(String(opts.port), 10) || 443; + opts = { ...opts, agent: createTunnelAgent(host, port) }; + } + return origRequest.call(node_https_1.default, opts, cb); + } + // Non-WebSocket — pass through original arguments unchanged. + // eslint-disable-next-line prefer-rest-params + return origRequest.apply(node_https_1.default, arguments); + } + // Replace https.request with our patched version. + node_https_1.default.request = wsProxyFixedRequest; + globalThis[_PATCHED] = true; +})(); diff --git a/nemoclaw-blueprint/scripts/ws-proxy-fix.ts b/nemoclaw-blueprint/scripts/ws-proxy-fix.ts new file mode 100644 index 00000000000..a82f3cdc952 --- /dev/null +++ b/nemoclaw-blueprint/scripts/ws-proxy-fix.ts @@ -0,0 +1,225 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +// ws-proxy-fix.ts — preload script to fix Discord WebSocket connections +// through the OpenShell L7 proxy when HTTPS_PROXY is set. +// +// Problem (NemoClaw#1570): +// The `ws` library (used by OpenClaw's Discord extension via @buape/carbon) +// establishes WebSocket connections by calling https.request() for wss:// URLs. +// Inside the sandbox, HTTPS_PROXY is set and Node.js 22 (with +// NODE_USE_ENV_PROXY=1) routes these through EnvHttpProxyAgent — which sends a +// forward proxy request (GET https://...) instead of a CONNECT tunnel. The +// OpenShell L7 proxy correctly rejects forward proxy HTTPS with HTTP 400. +// Without NODE_USE_ENV_PROXY, ws goes direct, which the sandbox network +// namespace blocks. Either way, the WebSocket handshake fails and the bot +// loops on close code 1006. +// +// Fix: +// Patch https.request() to detect WebSocket upgrade requests to Discord +// gateway hosts (gateway.discord.gg) and inject an agent that issues a proper +// CONNECT request to the proxy, then upgrades the tunnel socket to TLS. +// All other HTTPS requests — including non-Discord WebSockets — pass through +// completely untouched. +// +// Uses only Node.js built-in modules — no external dependencies. +// +// Belt-and-suspenders: works regardless of any upstream OpenClaw changes. +// If the caller already provides a custom (non-default) agent, we step aside +// — no double-tunnelling. + +import http from "node:http"; +import net from "node:net"; +import tls from "node:tls"; +import https from "node:https"; +import { URL } from "node:url"; + +const _PATCHED = Symbol.for("nemoclaw.wsProxyFix"); + +type RequestCallback = (res: http.IncomingMessage) => void; + +/** + * Merged options after normalising the multiple call signatures of + * https.request(). Only fields we inspect are listed. + */ +interface ReqOpts extends https.RequestOptions { + headers?: http.OutgoingHttpHeaders; +} + +/** + * Self-executing initialiser. Using an IIFE rather than top-level `return` + * keeps the source valid TypeScript while preserving early-exit semantics. + */ +(function wsProxyFixInit(): void { + const proxyUrl = process.env.HTTPS_PROXY || process.env.https_proxy; + if (!proxyUrl) return; + + if ((globalThis as Record)[_PATCHED]) return; + + let proxy: URL; + try { + proxy = new URL(proxyUrl); + } catch { + return; + } + + const proxyHost: string = proxy.hostname; + const proxyPort: number = parseInt(proxy.port, 10) || 3128; + + // ---------- CONNECT tunnel agent ---------------------------------------- + + /** + * Create an https.Agent whose createConnection() establishes a CONNECT + * tunnel through the HTTP proxy, then upgrades to TLS — the correct + * behaviour that EnvHttpProxyAgent fails to perform for HTTPS. + */ + function createTunnelAgent( + targetHost: string, + targetPort: number, + ): https.Agent { + const agent = new https.Agent({ keepAlive: false, maxSockets: 1 }); + + // Override createConnection to route through the proxy's CONNECT tunnel. + // The typing is intentionally loosened because the actual Node.js runtime + // signature is broader than what @types/node declares. + (agent as unknown as Record).createConnection = function ( + options: Record, + callback: (err: Error | null, socket?: tls.TLSSocket) => void, + ): net.Socket { + const connectReq = http.request({ + host: proxyHost, + port: proxyPort, + method: "CONNECT", + path: `${targetHost}:${targetPort}`, + headers: { Host: `${targetHost}:${targetPort}` }, + }); + + connectReq.on( + "connect", + (_res: http.IncomingMessage, socket: net.Socket, head: Buffer) => { + if (_res.statusCode !== 200) { + socket.destroy(); + callback( + new Error( + `ws-proxy-fix: CONNECT ${targetHost}:${targetPort} via proxy failed (${_res.statusCode})`, + ), + ); + return; + } + // Preserve any bytes already buffered from the tunnel before TLS. + if (head && head.length > 0) { + socket.unshift(head); + } + const tlsSocket = tls.connect({ + socket, + servername: (options.servername as string) || targetHost, + }); + callback(null, tlsSocket); + }, + ); + + connectReq.on("error", (err: Error) => { + connectReq.destroy(); + callback(err); + }); + connectReq.end(); + + // createConnection expects a synchronous return; the real socket arrives + // via the callback. Return a placeholder that Node.js will discard. + return new net.Socket(); + }; + + return agent; + } + + // ---------- Target check ------------------------------------------------- + + /** + * Return true only for WebSocket upgrade requests targeting Discord + * gateway hosts (gateway.discord.gg and regional variants). + */ + function isDiscordWsUpgrade( + host: string | undefined, + headers: http.OutgoingHttpHeaders | undefined, + ): boolean { + if (!host || !headers || typeof headers !== "object") return false; + const h = host.toLowerCase(); + if (h !== "gateway.discord.gg" && !h.endsWith(".discord.gg")) return false; + for (const key of Object.keys(headers)) { + if ( + key.toLowerCase() === "upgrade" && + String(headers[key]).toLowerCase() === "websocket" + ) { + return true; + } + } + return false; + } + + // ---------- Patch https.request() --------------------------------------- + + // Capture the original — typed as a loose callable so we can invoke it + // with the normalised (options, cb) form without fighting overload resolution. + const origRequest = https.request as ( + options: ReqOpts, + callback?: RequestCallback, + ) => http.ClientRequest; + + function wsProxyFixedRequest( + input: string | URL | ReqOpts, + options?: RequestCallback | ReqOpts, + callback?: RequestCallback, + ): http.ClientRequest { + // --- Normalise arguments (Node.js accepts multiple call signatures) --- + let opts: ReqOpts; + let cb: RequestCallback | undefined; + + if (typeof input === "string" || input instanceof URL) { + if (typeof options === "function") { + cb = options; + opts = {}; + } else { + opts = (options as ReqOpts) || {}; + cb = callback; + } + const url = typeof input === "string" ? new URL(input) : input; + opts = { + protocol: url.protocol, + hostname: url.hostname, + port: url.port, + path: url.pathname + url.search, + ...opts, + }; + } else { + opts = input || {}; + cb = typeof options === "function" ? options : callback; + } + + // opts.host may include a port (e.g. "gateway.discord.gg:443") — strip it + // so the CONNECT path doesn't become "host:443:443". + let host = opts.hostname || undefined; + if (!host && opts.host) { + host = opts.host.replace(/:\d+$/, ""); + } + if (isDiscordWsUpgrade(host, opts.headers)) { + // Discord WebSocket upgrade — inject CONNECT tunnel agent unless the + // caller already provides a custom (non-default) agent. + if (!opts.agent || opts.agent === https.globalAgent) { + const port = parseInt(String(opts.port), 10) || 443; + opts = { ...opts, agent: createTunnelAgent(host!, port) }; + } + return origRequest.call(https, opts, cb); + } + + // Non-WebSocket — pass through original arguments unchanged. + // eslint-disable-next-line prefer-rest-params + return (origRequest as unknown as Function).apply(https, arguments); + } + + // Replace https.request with our patched version. + (https as unknown as Record).request = wsProxyFixedRequest; + + (globalThis as Record)[_PATCHED] = true; +})(); + +export {}; diff --git a/nemoclaw-blueprint/tsconfig.json b/nemoclaw-blueprint/tsconfig.json new file mode 100644 index 00000000000..307ca2f9ad2 --- /dev/null +++ b/nemoclaw-blueprint/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "commonjs", + "lib": ["ES2022"], + "strict": true, + "esModuleInterop": true, + "skipLibCheck": true, + "forceConsistentCasingInFileNames": true, + "declaration": false, + "sourceMap": false, + "types": ["node"] + }, + "include": ["scripts/**/*.ts"] +} diff --git a/package.json b/package.json index 9878a89b353..21272914f1a 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "format": "prettier --write 'bin/**/*.js' 'scripts/**/*.ts' 'test/**/*.{js,ts}'", "format:check": "prettier --check 'bin/**/*.js' 'scripts/**/*.ts' 'test/**/*.{js,ts}'", "typecheck": "tsc -p jsconfig.json", - "build:cli": "tsc -p tsconfig.src.json", + "build:cli": "tsc -p tsconfig.src.json && tsc -p nemoclaw-blueprint/tsconfig.json", "typecheck:cli": "tsc -p tsconfig.cli.json", "validate:configs": "tsx scripts/validate-configs.ts", "migrate:js-to-ts": "tsx scripts/migrate-js-to-ts.ts", diff --git a/scripts/nemoclaw-start.sh b/scripts/nemoclaw-start.sh index b46f3a529fa..09457c338eb 100755 --- a/scripts/nemoclaw-start.sh +++ b/scripts/nemoclaw-start.sh @@ -973,6 +973,19 @@ if [ -f "$_AXIOS_FIX_SCRIPT" ] && [ "${NODE_USE_ENV_PROXY:-}" = "1" ]; then export NODE_OPTIONS="${NODE_OPTIONS:+$NODE_OPTIONS }--require $_AXIOS_FIX_SCRIPT" fi +# WebSocket CONNECT tunnel fix (NemoClaw#1570). +# The `ws` library calls https.request() for wss:// WebSocket upgrades. +# EnvHttpProxyAgent (NODE_USE_ENV_PROXY=1) sends a forward proxy request +# instead of CONNECT — rejected by the L7 proxy with 400. Without +# NODE_USE_ENV_PROXY, ws goes direct — blocked by sandbox netns. +# The preload patches https.request() to inject a CONNECT tunnel agent for +# WebSocket upgrade requests. Activates whenever HTTPS_PROXY is set (the +# script itself guards on the env var). +_WS_FIX_SCRIPT="/opt/nemoclaw-blueprint/scripts/ws-proxy-fix.js" +if [ -f "$_WS_FIX_SCRIPT" ]; then + export NODE_OPTIONS="${NODE_OPTIONS:+$NODE_OPTIONS }--require $_WS_FIX_SCRIPT" +fi + # OpenShell re-injects narrow NO_PROXY/no_proxy=127.0.0.1,localhost,::1 every # time a user connects via `openshell sandbox connect`. The connect path spawns # `/bin/bash -i` (interactive, non-login), which sources ~/.bashrc — NOT @@ -1008,6 +1021,10 @@ PROXYEOF if [ -f "$_AXIOS_FIX_SCRIPT" ] && [ "${NODE_USE_ENV_PROXY:-}" = "1" ]; then echo "export NODE_OPTIONS=\"\${NODE_OPTIONS:+\$NODE_OPTIONS }--require $_AXIOS_FIX_SCRIPT\"" fi + # WebSocket CONNECT tunnel fix for connect sessions. (NemoClaw#1570) + if [ -f "$_WS_FIX_SCRIPT" ]; then + echo "export NODE_OPTIONS=\"\${NODE_OPTIONS:+\$NODE_OPTIONS }--require $_WS_FIX_SCRIPT\"" + fi # Tool cache redirects — generated from _TOOL_REDIRECTS (single source of truth) echo '# Tool cache redirects — /sandbox is Landlock read-only (#804)' for _redir in "${_TOOL_REDIRECTS[@]}"; do diff --git a/test/e2e/test-messaging-providers.sh b/test/e2e/test-messaging-providers.sh index a7046cdf5ef..4d5d13614bd 100755 --- a/test/e2e/test-messaging-providers.sh +++ b/test/e2e/test-messaging-providers.sh @@ -678,6 +678,204 @@ else fi fi +# M13c: Full Discord gateway handshake via ws-proxy-fix CONNECT tunnel (#1570). +# The `ws` library opens WebSocket connections via https.request() with an +# Upgrade: websocket header. The preload patches https.request() to issue a +# CONNECT tunnel for Discord gateway hosts. +# +# This test exercises the real Discord gateway protocol end-to-end: +# 1. https.request with Upgrade: websocket → CONNECT tunnel via proxy +# 2. Receive Discord Hello (opcode 10) with heartbeat_interval +# 3. Send a Heartbeat (opcode 1) back to the gateway +# 4. Receive Heartbeat ACK (opcode 11) +# 5. Send close frame and disconnect cleanly +# +# If the CONNECT tunnel is broken the connection never upgrades (400 from L7 +# proxy) and none of the protocol steps succeed. +dc_ws_tunnel=$(sandbox_exec 'node -e " +const https = require(\"https\"); +const crypto = require(\"crypto\"); + +// --- Minimal WebSocket framing (no ws dependency) --- +function unmaskFrame(buf) { + if (buf.length < 2) return null; + const fin = (buf[0] & 0x80) !== 0; + const opcode = buf[0] & 0x0f; + const masked = (buf[1] & 0x80) !== 0; + let payloadLen = buf[1] & 0x7f; + let offset = 2; + if (payloadLen === 126) { + if (buf.length < 4) return null; + payloadLen = buf.readUInt16BE(2); + offset = 4; + } else if (payloadLen === 127) { + if (buf.length < 10) return null; + payloadLen = Number(buf.readBigUInt64BE(2)); + offset = 10; + } + if (masked) offset += 4; + if (buf.length < offset + payloadLen) return null; + const data = buf.slice(offset, offset + payloadLen); + return { fin, opcode, data, totalLen: offset + payloadLen }; +} + +function makeFrame(opcode, payload) { + const buf = Buffer.from(payload); + const mask = crypto.randomBytes(4); + const masked = Buffer.alloc(buf.length); + for (let i = 0; i < buf.length; i++) masked[i] = buf[i] ^ mask[i % 4]; + let header; + if (buf.length < 126) { + header = Buffer.alloc(6); + header[0] = 0x80 | opcode; + header[1] = 0x80 | buf.length; + mask.copy(header, 2); + } else { + header = Buffer.alloc(8); + header[0] = 0x80 | opcode; + header[1] = 0x80 | 126; + header.writeUInt16BE(buf.length, 2); + mask.copy(header, 4); + } + return Buffer.concat([header, masked]); +} + +function makeCloseFrame(code) { + const payload = Buffer.alloc(2); + payload.writeUInt16BE(code, 0); + return makeFrame(8, payload); +} + +// --- Handshake --- +const results = []; +const done = () => { + console.log(results.join(\"\\n\")); + process.exit(0); +}; +const timer = setTimeout(() => { results.push(\"TIMEOUT\"); done(); }, 20000); + +const key = crypto.randomBytes(16).toString(\"base64\"); +const req = https.request({ + hostname: \"gateway.discord.gg\", + port: 443, + path: \"/?v=10&encoding=json\", + method: \"GET\", + headers: { + \"Connection\": \"Upgrade\", + \"Upgrade\": \"websocket\", + \"Sec-WebSocket-Key\": key, + \"Sec-WebSocket-Version\": \"13\", + }, +}); + +req.on(\"upgrade\", (_res, socket, head) => { + results.push(\"UPGRADED\"); + let pending = head && head.length ? Buffer.from(head) : Buffer.alloc(0); + + socket.on(\"data\", (chunk) => { + pending = Buffer.concat([pending, chunk]); + while (true) { + const frame = unmaskFrame(pending); + if (!frame) break; + pending = pending.slice(frame.totalLen); + + if (frame.opcode === 1) { + let msg; + try { msg = JSON.parse(frame.data.toString()); } catch { continue; } + + if (msg.op === 10) { + const hbInterval = msg.d && msg.d.heartbeat_interval; + results.push(\"HELLO op=10 heartbeat_interval=\" + hbInterval); + + // Send Heartbeat (opcode 1, d: null) + const hb = JSON.stringify({ op: 1, d: null }); + socket.write(makeFrame(1, hb)); + results.push(\"SENT_HEARTBEAT op=1\"); + } else if (msg.op === 11) { + results.push(\"HEARTBEAT_ACK op=11\"); + // Full round-trip complete — close cleanly + socket.write(makeCloseFrame(1000)); + setTimeout(() => { socket.destroy(); clearTimeout(timer); done(); }, 500); + } + } else if (frame.opcode === 8) { + results.push(\"CLOSE_FRAME code=\" + (frame.data.length >= 2 ? frame.data.readUInt16BE(0) : \"none\")); + socket.destroy(); + clearTimeout(timer); + done(); + } + } + }); + + socket.on(\"error\", (e) => { results.push(\"SOCKET_ERROR \" + e.message); }); + socket.on(\"close\", () => { clearTimeout(timer); done(); }); +}); + +req.on(\"response\", (res) => { + results.push(\"HTTP_\" + res.statusCode); + res.resume(); + res.on(\"end\", () => { clearTimeout(timer); done(); }); +}); +req.on(\"error\", (e) => { + results.push(\"ERROR \" + e.message); + clearTimeout(timer); + done(); +}); +req.end(); +"' 2>/dev/null || true) + +info "Discord ws-proxy-fix probe: ${dc_ws_tunnel:0:500}" + +# Check each step of the handshake independently +if echo "$dc_ws_tunnel" | grep -q "UPGRADED"; then + pass "M13c: WebSocket upgrade succeeded via CONNECT tunnel (#1570)" +elif echo "$dc_ws_tunnel" | grep -q "HTTP_400"; then + if [ "$STRICT_DISCORD_GATEWAY" = "1" ]; then + fail "M13c: Discord gateway got 400 — CONNECT tunnel not working" + else + skip "M13c: Discord gateway got 400 — ws-proxy-fix may not be active" + fi +elif echo "$dc_ws_tunnel" | grep -qiE "EAI_AGAIN|getaddrinfo"; then + if [ "$STRICT_DISCORD_GATEWAY" = "1" ]; then + fail "M13c: Discord gateway DNS failure (${dc_ws_tunnel:0:200})" + else + skip "M13c: Discord gateway DNS failure (${dc_ws_tunnel:0:200})" + fi +elif echo "$dc_ws_tunnel" | grep -q "TIMEOUT"; then + if [ "$STRICT_DISCORD_GATEWAY" = "1" ]; then + fail "M13c: Discord gateway CONNECT tunnel timed out" + else + skip "M13c: Discord gateway CONNECT tunnel timed out" + fi +elif echo "$dc_ws_tunnel" | grep -q "ERROR"; then + if [ "$STRICT_DISCORD_GATEWAY" = "1" ]; then + fail "M13c: Discord gateway CONNECT tunnel failed (${dc_ws_tunnel:0:200})" + else + skip "M13c: Discord gateway CONNECT tunnel failed (${dc_ws_tunnel:0:200})" + fi +else + if [ "$STRICT_DISCORD_GATEWAY" = "1" ]; then + fail "M13c: Discord gateway returned unclassified result (${dc_ws_tunnel:0:200})" + else + skip "M13c: Discord gateway returned unclassified result (${dc_ws_tunnel:0:200})" + fi +fi + +if echo "$dc_ws_tunnel" | grep -q "HELLO op=10"; then + pass "M13d: Received Discord Hello (opcode 10) with heartbeat interval" +elif echo "$dc_ws_tunnel" | grep -q "UPGRADED"; then + fail "M13d: Upgraded but never received Discord Hello" +else + skip "M13d: WebSocket upgrade did not complete" +fi + +if echo "$dc_ws_tunnel" | grep -q "HEARTBEAT_ACK op=11"; then + pass "M13e: Sent Heartbeat, received ACK (opcode 11) — full round-trip verified" +elif echo "$dc_ws_tunnel" | grep -q "SENT_HEARTBEAT"; then + fail "M13e: Sent Heartbeat but never received ACK" +else + skip "M13e: Heartbeat exchange did not occur" +fi + # M14 (negative): curl should be blocked by binary restriction curl_reach=$(sandbox_exec "curl -s --max-time 10 https://api.telegram.org/ 2>&1" 2>/dev/null || true) if echo "$curl_reach" | grep -qiE "(blocked|denied|forbidden|refused|not found|no such)"; then diff --git a/test/service-env.test.ts b/test/service-env.test.ts index 5b229bb8c0a..5cce25e1034 100644 --- a/test/service-env.test.ts +++ b/test/service-env.test.ts @@ -632,6 +632,7 @@ describe("service environment", () => { `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, `NODE_USE_ENV_PROXY=1`, `_AXIOS_FIX_SCRIPT="${fakeFixScript}"`, + `_WS_FIX_SCRIPT="/nonexistent/ws-proxy-fix.js"`, `_TOOL_REDIRECTS=()`, "set +u # array expansion safe on macOS bash", persistBlock @@ -682,6 +683,7 @@ describe("service environment", () => { `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, // NODE_USE_ENV_PROXY intentionally NOT set `_AXIOS_FIX_SCRIPT="${fakeFixScript}"`, + `_WS_FIX_SCRIPT="/nonexistent/ws-proxy-fix.js"`, `_TOOL_REDIRECTS=()`, "set +u # array expansion safe on macOS bash", persistBlock @@ -694,8 +696,10 @@ describe("service environment", () => { const envFile = readFileSync(join(fakeDataDir, "proxy-env.sh"), "utf-8"); // NODE_OPTIONS preload should NOT be injected when NODE_USE_ENV_PROXY is not 1 + // and ws fix script does not exist expect(envFile).not.toContain("--require"); expect(envFile).not.toContain("axios-proxy-fix"); + expect(envFile).not.toContain("ws-proxy-fix"); } finally { try { execFileSync("rm", ["-rf", fakeDataDir, tmpFile]); @@ -704,5 +708,219 @@ describe("service environment", () => { } } }); + + it("NemoClaw#1570: proxy-env.sh includes ws-proxy-fix NODE_OPTIONS when fix script exists", () => { + const fakeDataDir = join(tmpdir(), `nemoclaw-ws-fix-test-${process.pid}`); + const fakeWsFixScript = join(fakeDataDir, "ws-proxy-fix.js"); + execFileSync("mkdir", ["-p", fakeDataDir]); + const tmpFile = join(tmpdir(), `nemoclaw-ws-fix-env-${process.pid}.sh`); + try { + const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); + const persistBlock = execFileSync( + "sed", + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + { encoding: "utf-8" }, + ); + if (!persistBlock.trim()) { + throw new Error( + "sed anchors (_PROXY_ENV_FILE…emit_sandbox_sourced_file) not found in nemoclaw-start.sh — test cannot run", + ); + } + const emitHelper = extractEmitHelper(); + const wrapper = [ + "#!/usr/bin/env bash", + "set -euo pipefail", + emitHelper, + `PROXY_HOST="10.200.0.1"`, + `PROXY_PORT="3128"`, + `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, + `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, + `_AXIOS_FIX_SCRIPT="/nonexistent/axios-proxy-fix.js"`, + `_WS_FIX_SCRIPT="${fakeWsFixScript}"`, + `_TOOL_REDIRECTS=()`, + "set +u # array expansion safe on macOS bash", + persistBlock + .trimEnd() + .replaceAll("/tmp/nemoclaw-proxy-env.sh", `${fakeDataDir}/proxy-env.sh`), + ].join("\n"); + writeFileSync(fakeWsFixScript, "// fake", { mode: 0o644 }); + writeFileSync(tmpFile, wrapper, { mode: 0o700 }); + execFileSync("bash", [tmpFile], { encoding: "utf-8" }); + + const envFile = readFileSync(join(fakeDataDir, "proxy-env.sh"), "utf-8"); + expect(envFile).toContain("NODE_OPTIONS"); + expect(envFile).toContain("--require"); + expect(envFile).toContain(fakeWsFixScript); + } finally { + try { + execFileSync("rm", ["-rf", fakeDataDir, tmpFile]); + } catch { + /* ignore */ + } + } + }); + + it("NemoClaw#1570: proxy-env.sh omits ws-proxy-fix when script does not exist", () => { + const fakeDataDir = join(tmpdir(), `nemoclaw-ws-noop-test-${process.pid}`); + execFileSync("mkdir", ["-p", fakeDataDir]); + const tmpFile = join(tmpdir(), `nemoclaw-ws-noop-env-${process.pid}.sh`); + try { + const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); + const persistBlock = execFileSync( + "sed", + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + { encoding: "utf-8" }, + ); + if (!persistBlock.trim()) { + throw new Error("sed anchors not found in nemoclaw-start.sh — test cannot run"); + } + const emitHelper = extractEmitHelper(); + const wrapper = [ + "#!/usr/bin/env bash", + "set -euo pipefail", + emitHelper, + `PROXY_HOST="10.200.0.1"`, + `PROXY_PORT="3128"`, + `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, + `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, + `_AXIOS_FIX_SCRIPT="/nonexistent/axios-proxy-fix.js"`, + `_WS_FIX_SCRIPT="/nonexistent/ws-proxy-fix.js"`, + `_TOOL_REDIRECTS=()`, + "set +u # array expansion safe on macOS bash", + persistBlock + .trimEnd() + .replaceAll("/tmp/nemoclaw-proxy-env.sh", `${fakeDataDir}/proxy-env.sh`), + ].join("\n"); + writeFileSync(tmpFile, wrapper, { mode: 0o700 }); + execFileSync("bash", [tmpFile], { encoding: "utf-8" }); + + const envFile = readFileSync(join(fakeDataDir, "proxy-env.sh"), "utf-8"); + expect(envFile).not.toContain("ws-proxy-fix"); + } finally { + try { + execFileSync("rm", ["-rf", fakeDataDir, tmpFile]); + } catch { + /* ignore */ + } + } + }); + }); + + describe("ws-proxy-fix preload (issue #1570)", () => { + const wsFixPath = join(import.meta.dirname, "../nemoclaw-blueprint/scripts/ws-proxy-fix.js"); + + it("patches https.request when HTTPS_PROXY is set", () => { + const result = execFileSync( + "node", + ["--require", wsFixPath, "-e", "console.log(require('https').request.name)"], + { + encoding: "utf-8", + env: { ...process.env, HTTPS_PROXY: "http://10.200.0.1:3128" }, + }, + ).trim(); + expect(result).toBe("wsProxyFixedRequest"); + }); + + it("is a no-op when HTTPS_PROXY is unset", () => { + const env = { ...process.env }; + delete env.HTTPS_PROXY; + delete env.https_proxy; + const result = execFileSync( + "node", + ["--require", wsFixPath, "-e", "console.log(require('https').request.name)"], + { encoding: "utf-8", env }, + ).trim(); + expect(result).not.toBe("wsProxyFixedRequest"); + }); + + it("is idempotent — loading twice does not double-patch", () => { + const result = execFileSync( + "node", + [ + "--require", + wsFixPath, + "-e", + `require("${wsFixPath}"); console.log(require('https').request.name)`, + ], + { + encoding: "utf-8", + env: { ...process.env, HTTPS_PROXY: "http://10.200.0.1:3128" }, + }, + ).trim(); + expect(result).toBe("wsProxyFixedRequest"); + }); + + it("strips port from opts.host to avoid double-port CONNECT path", () => { + // When callers pass host:"gateway.discord.gg:443" instead of hostname, + // the CONNECT target must be "gateway.discord.gg:443" not + // "gateway.discord.gg:443:443". + const result = execFileSync( + "node", + [ + "--require", + wsFixPath, + "-e", + ` +const https = require("https"); +const http = require("http"); +// Intercept http.request to capture the CONNECT path, then abort immediately +http.request = function(opts) { + if (opts.method === "CONNECT") { + console.log(opts.path); + process.exit(0); + } + return http.__proto__.request.apply(this, arguments); +}; +const req = https.request({ + host: "gateway.discord.gg:443", + path: "/?v=10&encoding=json", + headers: { Connection: "Upgrade", Upgrade: "websocket", "Sec-WebSocket-Key": "dGVzdA==", "Sec-WebSocket-Version": "13" }, +}); +req.on("error", () => {}); +req.end(); + `, + ], + { + encoding: "utf-8", + env: { ...process.env, HTTPS_PROXY: "http://10.200.0.1:3128" }, + }, + ).trim(); + expect(result).toBe("gateway.discord.gg:443"); + expect(result).not.toContain("443:443"); + }); + + it("ignores non-Discord WebSocket upgrades", () => { + const result = execFileSync( + "node", + [ + "--require", + wsFixPath, + "-e", + ` +const https = require("https"); +const http = require("http"); +let sawConnect = false; +http.request = function(opts) { + if (opts.method === "CONNECT") sawConnect = true; + return http.__proto__.request.apply(this, arguments); +}; +const req = https.request({ + hostname: "echo.websocket.org", + path: "/", + headers: { Connection: "Upgrade", Upgrade: "websocket", "Sec-WebSocket-Key": "dGVzdA==", "Sec-WebSocket-Version": "13" }, +}); +req.on("error", () => {}); +req.destroy(); +console.log(sawConnect ? "CONNECT" : "NO_CONNECT"); + `, + ], + { + encoding: "utf-8", + env: { ...process.env, HTTPS_PROXY: "http://10.200.0.1:3128" }, + }, + ).trim(); + // Non-Discord host should NOT trigger the CONNECT tunnel + expect(result).toBe("NO_CONNECT"); + }); }); }); diff --git a/tsconfig.cli.json b/tsconfig.cli.json index 6fe53d6f789..dae891bde31 100644 --- a/tsconfig.cli.json +++ b/tsconfig.cli.json @@ -15,6 +15,6 @@ "moduleDetection": "force", "types": ["node"] }, - "include": ["bin/**/*.ts", "scripts/**/*.ts", "src/**/*.ts", "test/**/*.ts"], + "include": ["bin/**/*.ts", "scripts/**/*.ts", "src/**/*.ts", "test/**/*.ts", "nemoclaw-blueprint/scripts/**/*.ts"], "exclude": ["node_modules", "nemoclaw"] } From 6ffbec10c246ef8d9740bcd01c10964154c1e39e Mon Sep 17 00:00:00 2001 From: "J. Yaunches" Date: Wed, 22 Apr 2026 20:04:06 -0400 Subject: [PATCH 08/17] refactor(sandbox): extract shared entrypoint library to fix Hermes vulnerability (#2277) (#2297) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Extract common entrypoint functions from `scripts/nemoclaw-start.sh` (OpenClaw) and `agents/hermes/start.sh` (Hermes) into a shared shell library (`scripts/lib/sandbox-init.sh`). This fixes an active Hermes vulnerability (same class as #2181), prevents the two entrypoints from drifting further apart, and establishes reusable primitives for future agent types. ## Related Issue Fixes #2277 ## Changes - **`scripts/lib/sandbox-init.sh`** (new): Shared library with security primitives — `emit_sandbox_sourced_file()`, `validate_tmp_permissions()`, `drop_capabilities()`, `verify_config_integrity()`, `lock_rc_files()`, `cleanup_on_signal()`, `validate_config_symlinks()`, `harden_config_symlinks()`, `configure_messaging_channels()` - **`agents/hermes/start.sh`**: Sources shared library. **Security fix**: `.bashrc`/`.profile` now locked to 444 via `lock_rc_files()` (was wide open). Proxy config now uses `emit_sandbox_sourced_file` (root:root 444) instead of inline append to writable `.bashrc`. `validate_tmp_permissions()` gate added before service launch. - **`scripts/nemoclaw-start.sh`**: Sources shared library. Replaced inline capsh block, `verify_config_integrity()`, symlink validation, messaging channels, cleanup, and proxy-env writing with shared versions. Proxy-env.sh now written via `emit_sandbox_sourced_file` (444 instead of 644). - **`test/sandbox-init.test.ts`** (new): 28 tests covering all shared library functions independently - **`test/nemoclaw-start.test.ts`**: Updated tests for shared `cleanup_on_signal` pattern (replaces inline `cleanup()`) - **`test/service-env.test.ts`**: Updated sed extraction anchors for `emit_sandbox_sourced_file` pattern; added 444 permission assertion ### Hermes vulnerability fix summary | Control | Before | After | |---|---|---| | `.bashrc`/`.profile` locked? | **No — wide open** | ✅ chmod 444 via `lock_rc_files` | | Proxy config | **Inline in writable .bashrc** | ✅ Standalone 444 file via `emit_sandbox_sourced_file` | | `/tmp` permission validation | **Missing** | ✅ `validate_tmp_permissions` before service launch | | Future fixes auto-applied? | **No — manual port** | ✅ Both source same library | ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes (shellcheck, shfmt, eslint, prettier, gitleaks all pass; test runner has pre-existing Docker-dependent failures on main) - [x] `npm test` passes (sandbox-init 28/28, service-env 34/34, nemoclaw-start 68/68; pre-existing install-preflight failures unrelated to this PR) - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: Claude Code (pi agent) --- Signed-off-by: Julie Yaunches ## Summary by CodeRabbit * **Chores** * Centralized and hardened sandbox startup logic with stricter /tmp and config permission checks, symlink protections, capability-lowering, improved signal/shutdown handling, and safer proxy env persistence. * **Tests** * Added comprehensive tests covering sandbox init routines, permission enforcement, integrity checks, symlink hardening, capability behavior, and signal/cleanup semantics. --------- Signed-off-by: Julie Yaunches Co-authored-by: Carlos Villela --- Dockerfile | 5 +- agents/hermes/Dockerfile | 3 +- agents/hermes/start.sh | 200 +++++------- scripts/lib/sandbox-init.sh | 340 ++++++++++++++++++++ scripts/nemoclaw-start.sh | 267 +++------------- src/lib/sandbox-build-context.ts | 6 + test/e2e-gateway-isolation.sh | 5 +- test/nemoclaw-start.test.ts | 198 ++---------- test/sandbox-init.test.ts | 516 +++++++++++++++++++++++++++++++ test/service-env.test.ts | 101 +++--- 10 files changed, 1071 insertions(+), 570 deletions(-) create mode 100755 scripts/lib/sandbox-init.sh create mode 100644 test/sandbox-init.test.ts diff --git a/Dockerfile b/Dockerfile index 413b4ec852c..6e239efad33 100644 --- a/Dockerfile +++ b/Dockerfile @@ -149,9 +149,10 @@ RUN set -eu; \ RUN mkdir -p /sandbox/.nemoclaw/blueprints/0.1.0 \ && cp -r /opt/nemoclaw-blueprint/* /sandbox/.nemoclaw/blueprints/0.1.0/ -# Copy startup script +# Copy startup script and shared sandbox initialisation library +COPY scripts/lib/sandbox-init.sh /usr/local/lib/nemoclaw/sandbox-init.sh COPY scripts/nemoclaw-start.sh /usr/local/bin/nemoclaw-start -RUN chmod 755 /usr/local/bin/nemoclaw-start +RUN chmod 755 /usr/local/bin/nemoclaw-start /usr/local/lib/nemoclaw/sandbox-init.sh # Build args for config that varies per deployment. # nemoclaw onboard passes these at image build time. diff --git a/agents/hermes/Dockerfile b/agents/hermes/Dockerfile index 92758326171..3e6027f8d95 100644 --- a/agents/hermes/Dockerfile +++ b/agents/hermes/Dockerfile @@ -34,8 +34,9 @@ RUN chmod 755 /usr/local/bin/nemoclaw-decode-proxy COPY nemoclaw-blueprint/ /opt/nemoclaw-blueprint/ # Copy startup script +COPY scripts/lib/sandbox-init.sh /usr/local/lib/nemoclaw/sandbox-init.sh COPY agents/hermes/start.sh /usr/local/bin/nemoclaw-start -RUN chmod 755 /usr/local/bin/nemoclaw-start +RUN chmod 755 /usr/local/bin/nemoclaw-start /usr/local/lib/nemoclaw/sandbox-init.sh # Build args for config that varies per deployment. ARG NEMOCLAW_MODEL=nvidia/nemotron-3-super-120b-a12b diff --git a/agents/hermes/start.sh b/agents/hermes/start.sh index 2ee1bc719f4..b7baac4ee16 100755 --- a/agents/hermes/start.sh +++ b/agents/hermes/start.sh @@ -16,6 +16,18 @@ set -euo pipefail +# ── Source shared sandbox initialisation library ───────────────── +# Single source of truth for security-sensitive primitives shared with +# scripts/nemoclaw-start.sh (OpenClaw). Ref: #2277 +# Installed location (container): /usr/local/lib/nemoclaw/sandbox-init.sh +# Dev fallback: scripts/lib/sandbox-init.sh relative to this script. +_SANDBOX_INIT="/usr/local/lib/nemoclaw/sandbox-init.sh" +if [ ! -f "$_SANDBOX_INIT" ]; then + _SANDBOX_INIT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/../../scripts/lib/sandbox-init.sh" +fi +# shellcheck source=scripts/lib/sandbox-init.sh +source "$_SANDBOX_INIT" + # Harden: limit process count to prevent fork bombs if ! ulimit -Su 512 2>/dev/null; then echo "[SECURITY] Could not set soft nproc limit (container runtime may restrict ulimit)" >&2 @@ -27,19 +39,8 @@ fi # SECURITY: Lock down PATH export PATH="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" -# ── Drop unnecessary Linux capabilities ────────────────────────── -if [ "${NEMOCLAW_CAPS_DROPPED:-}" != "1" ] && command -v capsh >/dev/null 2>&1; then - if capsh --has-p=cap_setpcap 2>/dev/null; then - export NEMOCLAW_CAPS_DROPPED=1 - exec capsh \ - --drop=cap_net_raw,cap_dac_override,cap_sys_chroot,cap_fsetid,cap_setfcap,cap_mknod,cap_audit_write,cap_net_bind_service \ - -- -c 'exec /usr/local/bin/nemoclaw-start "$@"' -- "$@" - else - echo "[SECURITY] CAP_SETPCAP not available — runtime already restricts capabilities" >&2 - fi -elif [ "${NEMOCLAW_CAPS_DROPPED:-}" != "1" ]; then - echo "[SECURITY WARNING] capsh not available — running with default capabilities" >&2 -fi +# ── Drop unnecessary Linux capabilities (shared) ──────────────── +drop_capabilities /usr/local/bin/nemoclaw-start "$@" # Normalize the self-wrapper bootstrap (same as OpenClaw entrypoint). if [ "${1:-}" = "env" ]; then @@ -83,18 +84,7 @@ HERMES="$(command -v hermes)" # Resolve once, use absolute path everywhere HERMES_IMMUTABLE="/sandbox/.hermes" HERMES_WRITABLE="/sandbox/.hermes-data" -# ── Config integrity check ────────────────────────────────────── -verify_config_integrity() { - local hash_file="${HERMES_IMMUTABLE}/.config-hash" - if [ ! -f "$hash_file" ]; then - echo "[SECURITY] Config hash file missing — refusing to start without integrity verification" >&2 - return 1 - fi - if ! (cd "${HERMES_IMMUTABLE}" && sha256sum -c "$hash_file" --status 2>/dev/null); then - echo "[SECURITY] Hermes config integrity check FAILED — config may have been tampered with" >&2 - return 1 - fi -} +# verify_config_integrity is provided by sandbox-init.sh (parameterized). # Copy verified immutable config into the writable HERMES_HOME so the # gateway process can read it alongside its own state files. @@ -146,66 +136,23 @@ GUARD printf '\n%s\n' "$snippet" >>"$rc_file" fi done + # SECURITY FIX: Lock .bashrc/.profile after all mutations are complete. + # This was missing in Hermes (unlike OpenClaw which had it via #2125), + # leaving rc files writable by the sandbox user. Ref: #2277 + lock_rc_files "$_SANDBOX_HOME" } +# validate_hermes_symlinks / harden_hermes_symlinks — thin wrappers +# around shared library functions for backward compatibility with callsites. validate_hermes_symlinks() { - local entry name target expected - for entry in /sandbox/.hermes/*; do - [ -L "$entry" ] || continue - name="$(basename "$entry")" - target="$(readlink -f "$entry" 2>/dev/null || true)" - expected="/sandbox/.hermes-data/$name" - if [ "$target" != "$expected" ]; then - echo "[SECURITY] Symlink $entry points to unexpected target: $target (expected $expected)" >&2 - return 1 - fi - done + validate_config_symlinks /sandbox/.hermes /sandbox/.hermes-data } harden_hermes_symlinks() { - local entry hardened failed - hardened=0 - failed=0 - - if ! command -v chattr >/dev/null 2>&1; then - echo "[SECURITY] chattr not available — relying on DAC + Landlock for .hermes hardening" >&2 - return 0 - fi - - if chattr +i /sandbox/.hermes 2>/dev/null; then - hardened=$((hardened + 1)) - else - failed=$((failed + 1)) - fi - - for entry in /sandbox/.hermes/*; do - [ -L "$entry" ] || continue - if chattr +i "$entry" 2>/dev/null; then - hardened=$((hardened + 1)) - else - failed=$((failed + 1)) - fi - done - - if [ "$failed" -gt 0 ]; then - echo "[SECURITY] Immutable hardening applied to $hardened path(s); $failed path(s) could not be hardened — continuing with DAC + Landlock" >&2 - elif [ "$hardened" -gt 0 ]; then - echo "[SECURITY] Immutable hardening applied to /sandbox/.hermes and validated symlinks" >&2 - fi + harden_config_symlinks /sandbox/.hermes } -configure_messaging_channels() { - # Channel entries are baked into config.yaml at image build time via - # NEMOCLAW_MESSAGING_CHANNELS_B64. Placeholder tokens flow through to - # the L7 proxy for rewriting at egress. - [ -n "${TELEGRAM_BOT_TOKEN:-}" ] || [ -n "${DISCORD_BOT_TOKEN:-}" ] || [ -n "${SLACK_BOT_TOKEN:-}" ] || return 0 - - echo "[channels] Messaging channels active (baked at build time):" >&2 - [ -n "${TELEGRAM_BOT_TOKEN:-}" ] && echo "[channels] telegram" >&2 - [ -n "${DISCORD_BOT_TOKEN:-}" ] && echo "[channels] discord" >&2 - [ -n "${SLACK_BOT_TOKEN:-}" ] && echo "[channels] slack" >&2 - return 0 -} +# configure_messaging_channels is provided by sandbox-init.sh (shared). print_dashboard_urls() { local local_url @@ -262,16 +209,10 @@ start_decode_proxy() { echo "[gateway] decode-proxy failed to start — placeholder rewriting may not work" >&2 } -# Forward SIGTERM/SIGINT to child processes for graceful shutdown. -cleanup() { - echo "[gateway] received signal, forwarding to children..." >&2 - local gateway_status=0 - kill -TERM "$GATEWAY_PID" 2>/dev/null || true - [ -n "${SOCAT_PID:-}" ] && kill -TERM "$SOCAT_PID" 2>/dev/null || true - [ -n "${DECODE_PROXY_PID:-}" ] && kill -TERM "$DECODE_PROXY_PID" 2>/dev/null || true - wait "$GATEWAY_PID" 2>/dev/null || gateway_status=$? - exit "$gateway_status" -} +# cleanup_on_signal is provided by sandbox-init.sh. It reads +# SANDBOX_CHILD_PIDS (array of all PIDs) and SANDBOX_WAIT_PID (the +# primary process whose exit status is returned). +# Each code path below sets these before registering the trap. # ── Proxy environment ──────────────────────────────────────────── PROXY_HOST="${NEMOCLAW_PROXY_HOST:-10.200.0.1}" @@ -285,18 +226,8 @@ export http_proxy="$_PROXY_URL" export https_proxy="$_PROXY_URL" export no_proxy="$_NO_PROXY_VAL" -_PROXY_MARKER_BEGIN="# nemoclaw-proxy-config begin" -_PROXY_MARKER_END="# nemoclaw-proxy-config end" -_PROXY_SNIPPET="${_PROXY_MARKER_BEGIN} -export HTTP_PROXY=\"$_PROXY_URL\" -export HTTPS_PROXY=\"$_PROXY_URL\" -export NO_PROXY=\"$_NO_PROXY_VAL\" -export http_proxy=\"$_PROXY_URL\" -export https_proxy=\"$_PROXY_URL\" -export no_proxy=\"$_NO_PROXY_VAL\" -export HERMES_HOME=\"${HERMES_WRITABLE}\" -${_PROXY_MARKER_END}" - +# Resolve sandbox home dir early — used by proxy-env writing and +# install_configure_guard before the non-root/root branch below. if [ "$(id -u)" -eq 0 ]; then _SANDBOX_HOME=$(getent passwd sandbox 2>/dev/null | cut -d: -f6) _SANDBOX_HOME="${_SANDBOX_HOME:-/sandbox}" @@ -304,27 +235,24 @@ else _SANDBOX_HOME="${HOME:-/sandbox}" fi -_write_proxy_snippet() { - local target="$1" - if [ -f "$target" ] && grep -qF "$_PROXY_MARKER_BEGIN" "$target" 2>/dev/null; then - local tmp - tmp="$(mktemp)" - awk -v b="$_PROXY_MARKER_BEGIN" -v e="$_PROXY_MARKER_END" \ - '$0==b{s=1;next} $0==e{s=0;next} !s' "$target" >"$tmp" - printf '%s\n' "$_PROXY_SNIPPET" >>"$tmp" - cat "$tmp" >"$target" - rm -f "$tmp" - return 0 - fi - printf '\n%s\n' "$_PROXY_SNIPPET" >>"$target" -} - -# Write proxy snippet — may fail after capsh drops cap_dac_override -# (root can no longer write sandbox-owned files). Non-fatal. -if [ -w "$_SANDBOX_HOME" ]; then - _write_proxy_snippet "${_SANDBOX_HOME}/.bashrc" 2>/dev/null || true - _write_proxy_snippet "${_SANDBOX_HOME}/.profile" 2>/dev/null || true -fi +# SECURITY FIX: Write proxy config to a standalone file via +# emit_sandbox_sourced_file() (root:root 444) instead of appending +# inline to .bashrc/.profile. The old approach left .bashrc writable +# by the sandbox user — same vulnerability class as #2181. +# Ref: https://github.com/NVIDIA/NemoClaw/issues/2277 +_PROXY_ENV_FILE="/tmp/nemoclaw-proxy-env.sh" +{ + cat <&2 exit 1 fi @@ -348,9 +276,14 @@ if [ "$(id -u)" -ne 0 ]; then exec "${NEMOCLAW_CMD[@]}" fi + # TODO(#2277-P2): migrate to shared emit_restricted_log() helper touch /tmp/gateway.log chmod 600 /tmp/gateway.log + # Defence-in-depth: verify /tmp file permissions before launching services. + # shellcheck disable=SC2119 + validate_tmp_permissions + # Start decode proxy and Hermes gateway start_decode_proxy HERMES_HOME="${HERMES_WRITABLE}" \ @@ -361,8 +294,15 @@ if [ "$(id -u)" -ne 0 ]; then nohup "$HERMES" gateway run >/tmp/gateway.log 2>&1 & GATEWAY_PID=$! echo "[gateway] hermes gateway launched (pid $GATEWAY_PID)" >&2 - trap cleanup SIGTERM SIGINT + # NOTE: PIDs are collected after launch; a signal arriving between trap + # registration and the final append is a small race window (same as before + # the shared-library refactor). Acceptable for entrypoint-level cleanup. + SANDBOX_CHILD_PIDS=("$GATEWAY_PID") + [ -n "${DECODE_PROXY_PID:-}" ] && SANDBOX_CHILD_PIDS+=("$DECODE_PROXY_PID") + SANDBOX_WAIT_PID="$GATEWAY_PID" + trap cleanup_on_signal SIGTERM SIGINT start_socat_forwarder + [ -n "${SOCAT_PID:-}" ] && SANDBOX_CHILD_PIDS+=("$SOCAT_PID") print_dashboard_urls wait "$GATEWAY_PID" @@ -371,7 +311,7 @@ fi # ── Root path (full privilege separation via gosu) ───────────── -verify_config_integrity +verify_config_integrity "${HERMES_IMMUTABLE}" deploy_config_to_writable install_configure_guard configure_messaging_channels @@ -381,6 +321,7 @@ if [ ${#NEMOCLAW_CMD[@]} -gt 0 ]; then fi # SECURITY: Protect gateway log from sandbox user tampering +# TODO(#2277-P2): migrate to shared emit_restricted_log() helper touch /tmp/gateway.log chown gateway:gateway /tmp/gateway.log chmod 600 /tmp/gateway.log @@ -391,6 +332,10 @@ validate_hermes_symlinks # Lock .hermes directory after validation. harden_hermes_symlinks +# Defence-in-depth: verify /tmp file permissions before launching services. +# shellcheck disable=SC2119 +validate_tmp_permissions + # Start the gateway as the 'gateway' user. # Start decode proxy and gateway start_decode_proxy @@ -402,8 +347,15 @@ HERMES_HOME="${HERMES_WRITABLE}" \ nohup gosu gateway "$HERMES" gateway run >/tmp/gateway.log 2>&1 & GATEWAY_PID=$! echo "[gateway] hermes gateway launched as 'gateway' user (pid $GATEWAY_PID)" >&2 -trap cleanup SIGTERM SIGINT +# NOTE: PIDs are collected after launch; a signal arriving between trap +# registration and the final append is a small race window (same as before +# the shared-library refactor). Acceptable for entrypoint-level cleanup. +SANDBOX_CHILD_PIDS=("$GATEWAY_PID") +[ -n "${DECODE_PROXY_PID:-}" ] && SANDBOX_CHILD_PIDS+=("$DECODE_PROXY_PID") +SANDBOX_WAIT_PID="$GATEWAY_PID" +trap cleanup_on_signal SIGTERM SIGINT start_socat_forwarder +[ -n "${SOCAT_PID:-}" ] && SANDBOX_CHILD_PIDS+=("$SOCAT_PID") print_dashboard_urls # Keep container running by waiting on the gateway process. diff --git a/scripts/lib/sandbox-init.sh b/scripts/lib/sandbox-init.sh new file mode 100755 index 00000000000..fabdd5af43c --- /dev/null +++ b/scripts/lib/sandbox-init.sh @@ -0,0 +1,340 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +# +# Shared sandbox entrypoint primitives for NemoClaw agent types. +# +# Sourced by scripts/nemoclaw-start.sh (OpenClaw) and agents/hermes/start.sh +# (Hermes) to provide a single source of truth for security-sensitive +# initialisation functions. Prevents drift between entrypoints — every +# security fix applied here protects both agents automatically. +# +# Usage (from an entrypoint script): +# SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# # shellcheck source=scripts/lib/sandbox-init.sh +# source "${SCRIPT_DIR}/../scripts/lib/sandbox-init.sh" # adjust path +# +# Ref: https://github.com/NVIDIA/NemoClaw/issues/2277 + +# Guard against double-sourcing. +[ -z "${_SANDBOX_INIT_LOADED:-}" ] || return 0 +_SANDBOX_INIT_LOADED=1 + +# ── /tmp trust boundary map ────────────────────────────────────── +# Files in /tmp that cross user boundaries. Every file sourced by +# .bashrc/.profile MUST be root-owned 444 in root mode. +# +# File Owner Mode Writer Reader Sourced? +# /tmp/nemoclaw-proxy-env.sh root 444 root sandbox YES (.bashrc/.profile) +# /tmp/gateway.log gateway 600 gateway gateway no +# /tmp/auto-pair.log sandbox 600 sandbox sandbox no +# /tmp/.npm-cache/ sandbox 755 sandbox sandbox no (tool data) +# /tmp/.cache/ sandbox 755 sandbox sandbox no (tool data) +# /tmp/.config/ sandbox 755 sandbox sandbox no (tool data) +# /tmp/.gnupg/ sandbox 700 sandbox sandbox no (key data) +# +# In non-root mode privilege separation is disabled — all files are +# owned by sandbox. chmod 444 is best-effort (owner can chmod back). +# This is an accepted limitation documented in the OpenShell security model. +# +# See also: https://github.com/NVIDIA/NemoClaw/issues/2181 +# ───────────────────────────────────────────────────────────────── + +# ── Secure file helpers ────────────────────────────────────────── +# Centralized primitives for creating files that cross trust boundaries +# in /tmp. Using these helpers instead of ad-hoc chmod/chown ensures +# consistent security posture and prevents the class of bug in #2181. + +# Write a file that the sandbox user can SOURCE but not MODIFY. +# Reads content from stdin. Caller usage: +# emit_sandbox_sourced_file /path <<'EOF' +# export FOO="bar" +# EOF +# +# Or pipe into it: +# generate_content | emit_sandbox_sourced_file /path +# +# Root mode: root:root 444 — sandbox cannot chmod (not owner). +# Non-root: sandbox:sandbox 444 — best-effort (owner can chmod back; +# accepted limitation since privilege separation is disabled). +# +# SECURITY: write to a temp file in the same directory, then atomically rename +# it into place. This closes the rm+recreate race where another user could +# recreate the destination as a symlink between unlink and open. +emit_sandbox_sourced_file() { + local path="$1" + local dir base tmp + dir="$(dirname "$path")" + base="$(basename "$path")" + tmp="$(mktemp "${dir}/.${base}.tmp.XXXXXX")" || return 1 + + if ! cat >"$tmp"; then + rm -f "$tmp" + return 1 + fi + if [ "$(id -u)" -eq 0 ] && ! chown root:root "$tmp"; then + rm -f "$tmp" + return 1 + fi + if ! chmod 444 "$tmp"; then + rm -f "$tmp" + return 1 + fi + if ! mv -f "$tmp" "$path"; then + rm -f "$tmp" + return 1 + fi +} + +# Verify that trust-boundary files in /tmp have the expected permissions +# BEFORE handing off to the sandbox user. Call this after all init work +# and before launching services. Defence-in-depth: catches regressions +# even if a new file is added without using the helper above. +# +# Usage: +# validate_tmp_permissions # default sourced + log files +# validate_tmp_permissions /tmp/custom-sourced.sh # additional sourced files +# +# Positional args are additional sourced files to check (444 required). +# shellcheck disable=SC2120 +validate_tmp_permissions() { + local failed=0 + + # Files sourced by sandbox (.bashrc/.profile) — must not be writable. + local sourced_files=("/tmp/nemoclaw-proxy-env.sh") + sourced_files+=("$@") + + for f in "${sourced_files[@]}"; do + [ -f "$f" ] || continue + local perms owner + perms="$(stat -c '%a' "$f" 2>/dev/null || stat -f '%Lp' "$f" 2>/dev/null || echo "unknown")" + owner="$(stat -c '%U' "$f" 2>/dev/null || stat -f '%Su' "$f" 2>/dev/null || echo "unknown")" + if [ "$(id -u)" -eq 0 ] && { [ "$owner" != "root" ] || [ "$perms" != "444" ]; }; then + echo "[SECURITY] $f has unsafe permissions: owner=$owner mode=$perms (expected root:444)" >&2 + failed=1 + elif [ "$(id -u)" -ne 0 ] && [ "$perms" != "444" ]; then + echo "[SECURITY] $f has unsafe permissions: mode=$perms (expected 444)" >&2 + failed=1 + fi + done + + # Restricted log files — must be 600 + for f in /tmp/gateway.log /tmp/auto-pair.log; do + [ -f "$f" ] || continue + local perms + perms="$(stat -c '%a' "$f" 2>/dev/null || stat -f '%Lp' "$f" 2>/dev/null || echo "unknown")" + if [ "$perms" != "600" ]; then + echo "[SECURITY] $f has unexpected permissions: mode=$perms (expected 600)" >&2 + failed=1 + fi + done + + return $failed +} + +# ── Capability dropping ────────────────────────────────────────── +# CIS Docker Benchmark 5.3: containers should not run with default caps. +# OpenShell manages the container runtime so we cannot pass --cap-drop=ALL +# to docker run. Instead, drop dangerous capabilities from the bounding set +# at startup using capsh. The bounding set limits what caps any child process +# (gateway, sandbox, agent) can ever acquire. +# +# Kept: cap_chown, cap_setuid, cap_setgid, cap_fowner, cap_kill +# — required by the entrypoint for gosu privilege separation and chown. +# Ref: https://github.com/NVIDIA/NemoClaw/issues/797 +# +# Usage: +# drop_capabilities /usr/local/bin/nemoclaw-start "$@" +# +# The first argument is the absolute path to the entrypoint script to +# re-exec via capsh. Remaining arguments are forwarded. +drop_capabilities() { + local entrypoint="$1" + shift + + if [ "${NEMOCLAW_CAPS_DROPPED:-}" != "1" ] && command -v capsh >/dev/null 2>&1; then + # capsh --drop requires CAP_SETPCAP in the bounding set. OpenShell's + # sandbox runtime may strip it, so check before attempting the drop. + if capsh --has-p=cap_setpcap 2>/dev/null; then + export NEMOCLAW_CAPS_DROPPED=1 + exec capsh \ + --drop=cap_net_raw,cap_dac_override,cap_sys_chroot,cap_fsetid,cap_setfcap,cap_mknod,cap_audit_write,cap_net_bind_service \ + -- -c "exec $entrypoint \"\$@\"" -- "$@" + else + echo "[SECURITY] CAP_SETPCAP not available — runtime already restricts capabilities" >&2 + fi + elif [ "${NEMOCLAW_CAPS_DROPPED:-}" != "1" ]; then + echo "[SECURITY WARNING] capsh not available — running with default capabilities" >&2 + fi +} + +# ── Config integrity check ────────────────────────────────────── +# The config hash was pinned at build time. If it doesn't match, +# someone (or something) has tampered with the config. +# +# Usage: +# verify_config_integrity /sandbox/.openclaw # OpenClaw +# verify_config_integrity /sandbox/.hermes # Hermes +# +# The config_dir must contain a .config-hash file with sha256sum output. +verify_config_integrity() { + local config_dir="$1" + local hash_file="${config_dir}/.config-hash" + + if [ ! -f "$hash_file" ]; then + echo "[SECURITY] Config hash file missing (${hash_file}) — refusing to start without integrity verification" >&2 + return 1 + fi + if ! (cd "$config_dir" && sha256sum -c "$hash_file" --status 2>/dev/null); then + echo "[SECURITY] Config integrity check FAILED in ${config_dir} — config may have been tampered with" >&2 + return 1 + fi +} + +# ── RC file locking ────────────────────────────────────────────── +# Lock .bashrc and .profile to 444 after all mutations (proxy snippets, +# configure guard, gateway token export) are complete. This prevents the +# sandbox user from injecting code that runs on every `nemoclaw connect`. +# +# SECURITY: This fixes the Hermes vulnerability where .bashrc/.profile +# were never locked (unlike OpenClaw which had this via #2125). +# +# Usage: +# lock_rc_files /sandbox # locks /sandbox/.bashrc and /sandbox/.profile +lock_rc_files() { + local home_dir="$1" + + for rc_file in "${home_dir}/.bashrc" "${home_dir}/.profile"; do + if [ -f "$rc_file" ]; then + chmod 444 "$rc_file" + fi + done +} + +# ── Cleanup / signal forwarding ────────────────────────────────── +# Forward SIGTERM/SIGINT to child processes for graceful shutdown. +# The entrypoint is PID 1 — without a trap, signals interrupt wait and +# children are orphaned until Docker sends SIGKILL after the grace period. +# +# Usage: +# # After starting processes, register their PIDs: +# SANDBOX_CHILD_PIDS=("$GATEWAY_PID" "$AUTO_PAIR_PID") +# SANDBOX_WAIT_PID="$GATEWAY_PID" +# trap cleanup_on_signal SIGTERM SIGINT +# +# SANDBOX_CHILD_PIDS: array of PIDs to kill on signal (best-effort). +# SANDBOX_WAIT_PID: the primary PID whose exit status is returned. +cleanup_on_signal() { + echo "[gateway] received signal, forwarding to children..." >&2 + local primary_status=0 + + # ${arr[@]+...} guard prevents "unbound variable" under set -u when + # SANDBOX_CHILD_PIDS is empty or unset (bash 3.x / macOS compat). + local _pids=() + # shellcheck disable=SC2206 + _pids=(${SANDBOX_CHILD_PIDS[@]+"${SANDBOX_CHILD_PIDS[@]}"}) + + for pid in "${_pids[@]+"${_pids[@]}"}"; do + kill -TERM "$pid" 2>/dev/null || true + done + + if [ -n "${SANDBOX_WAIT_PID:-}" ]; then + wait "$SANDBOX_WAIT_PID" 2>/dev/null || primary_status=$? + fi + + # Wait for remaining children (best-effort, don't fail on already-exited) + for pid in "${_pids[@]+"${_pids[@]}"}"; do + [ "$pid" = "${SANDBOX_WAIT_PID:-}" ] && continue + wait "$pid" 2>/dev/null || true + done + + exit "$primary_status" +} + +# ── Symlink validation ─────────────────────────────────────────── +# Verify ALL symlinks in a config directory point to the expected +# writable data directory. Dynamic scan so future symlinks are +# covered automatically. +# +# Usage: +# validate_config_symlinks /sandbox/.openclaw /sandbox/.openclaw-data +# validate_config_symlinks /sandbox/.hermes /sandbox/.hermes-data +validate_config_symlinks() { + local config_dir="$1" + local data_dir="$2" + local entry name target expected + + for entry in "${config_dir}"/*; do + [ -L "$entry" ] || continue + name="$(basename "$entry")" + target="$(readlink -f "$entry" 2>/dev/null || true)" + # Resolve expected path too so macOS /var → /private/var doesn't cause + # false positives. Fall back to the unresolved path if readlink fails. + expected="$(readlink -f "${data_dir}/${name}" 2>/dev/null || echo "${data_dir}/${name}")" + if [ "$target" != "$expected" ]; then + echo "[SECURITY] Symlink $entry points to unexpected target: $target (expected $expected)" >&2 + return 1 + fi + done +} + +# Lock a config directory and its symlinks with the immutable flag so +# they cannot be swapped at runtime even if DAC or Landlock are bypassed. +# chattr requires cap_linux_immutable which the entrypoint has as root; +# the sandbox user cannot remove the flag. +# +# Usage: +# harden_config_symlinks /sandbox/.openclaw +# harden_config_symlinks /sandbox/.hermes +harden_config_symlinks() { + local config_dir="$1" + local label="${2:-$(basename "$config_dir")}" + local entry hardened failed + hardened=0 + failed=0 + + if ! command -v chattr >/dev/null 2>&1; then + echo "[SECURITY] chattr not available — relying on DAC + Landlock for ${label} hardening" >&2 + return 0 + fi + + if chattr +i "$config_dir" 2>/dev/null; then + hardened=$((hardened + 1)) + else + failed=$((failed + 1)) + fi + + for entry in "${config_dir}"/*; do + [ -L "$entry" ] || continue + if chattr +i "$entry" 2>/dev/null; then + hardened=$((hardened + 1)) + else + failed=$((failed + 1)) + fi + done + + if [ "$failed" -gt 0 ]; then + echo "[SECURITY] Immutable hardening applied to $hardened path(s); $failed path(s) could not be hardened — continuing with DAC + Landlock" >&2 + elif [ "$hardened" -gt 0 ]; then + echo "[SECURITY] Immutable hardening applied to ${label} and validated symlinks" >&2 + fi +} + +# ── Messaging channels ────────────────────────────────────────── +# Channel entries are baked into the config at image build time via +# NEMOCLAW_MESSAGING_CHANNELS_B64. Placeholder tokens flow through +# to the L7 proxy for rewriting at egress. Real tokens are never +# visible inside the sandbox. +# +# This function just logs which channels are active. Runtime patching +# of config files is not possible — Landlock enforces read-only at +# the kernel level. +configure_messaging_channels() { + [ -n "${TELEGRAM_BOT_TOKEN:-}" ] || [ -n "${DISCORD_BOT_TOKEN:-}" ] || [ -n "${SLACK_BOT_TOKEN:-}" ] || return 0 + + echo "[channels] Messaging channels active (baked at build time):" >&2 + [ -n "${TELEGRAM_BOT_TOKEN:-}" ] && echo "[channels] telegram" >&2 + [ -n "${DISCORD_BOT_TOKEN:-}" ] && echo "[channels] discord" >&2 + [ -n "${SLACK_BOT_TOKEN:-}" ] && echo "[channels] slack" >&2 + return 0 +} diff --git a/scripts/nemoclaw-start.sh b/scripts/nemoclaw-start.sh index 09457c338eb..4db3132b496 100755 --- a/scripts/nemoclaw-start.sh +++ b/scripts/nemoclaw-start.sh @@ -31,26 +31,17 @@ set -euo pipefail -# ── /tmp trust boundary map ────────────────────────────────────── -# Files in /tmp that cross user boundaries. Every file sourced by -# .bashrc/.profile MUST be root-owned 444 in root mode. -# -# File Owner Mode Writer Reader Sourced? -# /tmp/nemoclaw-proxy-env.sh root 444 root sandbox YES (.bashrc/.profile) -# /tmp/gateway.log gateway 600 gateway gateway no -# /tmp/auto-pair.log sandbox 600 sandbox sandbox no -# /tmp/.npm-cache/ sandbox 755 sandbox sandbox no (tool data) -# /tmp/.cache/ sandbox 755 sandbox sandbox no (tool data) -# /tmp/.config/ sandbox 755 sandbox sandbox no (tool data) -# /tmp/.gnupg/ sandbox 700 sandbox sandbox no (key data) -# -# In non-root mode privilege separation is disabled — all files are -# owned by sandbox. chmod 444 is best-effort (owner can chmod back). -# This is an accepted limitation documented in the OpenShell security model. -# -# See also: https://github.com/NVIDIA/NemoClaw/issues/2181 -# Future: adopt s6-overlay fix-attrs.d/ for declarative enforcement. -# ───────────────────────────────────────────────────────────────── +# ── Source shared sandbox initialisation library ───────────────── +# Single source of truth for security-sensitive primitives shared with +# agents/hermes/start.sh. Ref: https://github.com/NVIDIA/NemoClaw/issues/2277 +# Installed location (container): /usr/local/lib/nemoclaw/sandbox-init.sh +# Dev fallback: scripts/lib/sandbox-init.sh relative to this script. +_SANDBOX_INIT="/usr/local/lib/nemoclaw/sandbox-init.sh" +if [ ! -f "$_SANDBOX_INIT" ]; then + _SANDBOX_INIT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/lib/sandbox-init.sh" +fi +# shellcheck source=scripts/lib/sandbox-init.sh +source "$_SANDBOX_INIT" # Harden: limit process count to prevent fork bombs (ref: #809) # Best-effort: some container runtimes (e.g., brev) restrict ulimit @@ -115,99 +106,8 @@ else install -d -m 700 /tmp/.gnupg fi -# ── Secure file helpers ────────────────────────────────────────── -# Centralized primitives for creating files that cross trust boundaries -# in /tmp. Using these helpers instead of ad-hoc chmod/chown ensures -# consistent security posture and prevents the class of bug in #2181. -# -# Future: these map directly to s6-overlay fix-attrs.d/ entries when -# the entrypoint is decomposed. - -# Write a file that the sandbox user can SOURCE but not MODIFY. -# Reads content from stdin. Caller usage: -# emit_sandbox_sourced_file /path <<'EOF' -# export FOO="bar" -# EOF -# -# Root mode: root:root 444 — sandbox cannot chmod (not owner). -# Non-root: sandbox:sandbox 444 — best-effort (owner can chmod back; -# accepted limitation since privilege separation is disabled). -emit_sandbox_sourced_file() { - local path="$1" - # Remove any pre-existing file/symlink to prevent symlink-following attacks. - # rm -f works because: root can remove anything; in non-root mode the owner - # can remove their own file in sticky-bit /tmp. - rm -f "$path" 2>/dev/null || true - cat >"$path" - if [ "$(id -u)" -eq 0 ]; then - chown root:root "$path" - fi - chmod 444 "$path" -} - -# Verify that trust-boundary files in /tmp have the expected permissions -# BEFORE handing off to the sandbox user. Call this after all init work -# and before launching services. Defence-in-depth: catches regressions -# even if a new file is added without using the helper above. -validate_tmp_permissions() { - local failed=0 - - # Files sourced by sandbox (.bashrc/.profile) — must not be writable. - # Single-entry loop is intentional — designed to grow as new sourced files - # are added (e.g., mediator config). See trust boundary map above. - # shellcheck disable=SC2043 - for f in /tmp/nemoclaw-proxy-env.sh; do - [ -f "$f" ] || continue - local perms owner - perms="$(stat -c '%a' "$f" 2>/dev/null || stat -f '%Lp' "$f" 2>/dev/null || echo "unknown")" - owner="$(stat -c '%U' "$f" 2>/dev/null || stat -f '%Su' "$f" 2>/dev/null || echo "unknown")" - if [ "$(id -u)" -eq 0 ] && { [ "$owner" != "root" ] || [ "$perms" != "444" ]; }; then - echo "[SECURITY] $f has unsafe permissions: owner=$owner mode=$perms (expected root:444)" >&2 - failed=1 - elif [ "$(id -u)" -ne 0 ] && [ "$perms" != "444" ]; then - echo "[SECURITY] $f has unsafe permissions: mode=$perms (expected 444)" >&2 - failed=1 - fi - done - - # Restricted log files — must be 600 - for f in /tmp/gateway.log /tmp/auto-pair.log; do - [ -f "$f" ] || continue - local perms - perms="$(stat -c '%a' "$f" 2>/dev/null || stat -f '%Lp' "$f" 2>/dev/null || echo "unknown")" - if [ "$perms" != "600" ]; then - echo "[SECURITY] $f has unexpected permissions: mode=$perms (expected 600)" >&2 - failed=1 - fi - done - - return $failed -} - -# ── Drop unnecessary Linux capabilities ────────────────────────── -# CIS Docker Benchmark 5.3: containers should not run with default caps. -# OpenShell manages the container runtime so we cannot pass --cap-drop=ALL -# to docker run. Instead, drop dangerous capabilities from the bounding set -# at startup using capsh. The bounding set limits what caps any child process -# (gateway, sandbox, agent) can ever acquire. -# -# Kept: cap_chown, cap_setuid, cap_setgid, cap_fowner, cap_kill -# — required by the entrypoint for gosu privilege separation and chown. -# Ref: https://github.com/NVIDIA/NemoClaw/issues/797 -if [ "${NEMOCLAW_CAPS_DROPPED:-}" != "1" ] && command -v capsh >/dev/null 2>&1; then - # capsh --drop requires CAP_SETPCAP in the bounding set. OpenShell's - # sandbox runtime may strip it, so check before attempting the drop. - if capsh --has-p=cap_setpcap 2>/dev/null; then - export NEMOCLAW_CAPS_DROPPED=1 - exec capsh \ - --drop=cap_net_raw,cap_dac_override,cap_sys_chroot,cap_fsetid,cap_setfcap,cap_mknod,cap_audit_write,cap_net_bind_service \ - -- -c 'exec /usr/local/bin/nemoclaw-start "$@"' -- "$@" - else - echo "[SECURITY] CAP_SETPCAP not available — runtime already restricts capabilities" >&2 - fi -elif [ "${NEMOCLAW_CAPS_DROPPED:-}" != "1" ]; then - echo "[SECURITY WARNING] capsh not available — running with default capabilities" >&2 -fi +# ── Drop unnecessary Linux capabilities (shared) ──────────────── +drop_capabilities /usr/local/bin/nemoclaw-start "$@" # Normalize the sandbox-create bootstrap wrapper. Onboard launches the # container as `env CHAT_UI_URL=... nemoclaw-start`, but this script is already @@ -276,23 +176,8 @@ PUBLIC_PORT="$_DASHBOARD_PORT" OPENCLAW="$(command -v openclaw)" # Resolve once, use absolute path everywhere _SANDBOX_HOME="/sandbox" # Home dir for the sandbox user (useradd -d /sandbox in Dockerfile.base) -# ── Config integrity check ────────────────────────────────────── -# The config hash was pinned at build time. If it doesn't match, -# someone (or something) has tampered with the config. - -verify_config_integrity() { - local hash_file="/sandbox/.openclaw/.config-hash" - if [ ! -f "$hash_file" ]; then - echo "[SECURITY] Config hash file missing — refusing to start without integrity verification" >&2 - return 1 - fi - if ! (cd /sandbox/.openclaw && sha256sum -c "$hash_file" --status 2>/dev/null); then - echo "[SECURITY] openclaw.json integrity check FAILED — config may have been tampered with" >&2 - echo "[SECURITY] Expected hash: $(cat "$hash_file")" >&2 - echo "[SECURITY] Actual hash: $(sha256sum /sandbox/.openclaw/openclaw.json)" >&2 - return 1 - fi -} +# ── Config integrity check (delegates to shared library) ──────── +# verify_config_integrity is provided by sandbox-init.sh (parameterized). # ── Runtime model/provider override ────────────────────────────── # Patches openclaw.json at startup when NEMOCLAW_MODEL_OVERRIDE is set, @@ -731,55 +616,17 @@ GUARD done # Final lock after all rc-file mutations (export_gateway_token + this # function) are complete so Landlock read_only enforcement holds. - for rc_file in "${_SANDBOX_HOME}/.bashrc" "${_SANDBOX_HOME}/.profile"; do - [ -f "$rc_file" ] && chmod 444 "$rc_file" - done + lock_rc_files "$_SANDBOX_HOME" } +# validate_openclaw_symlinks / harden_openclaw_symlinks — thin wrappers +# around shared library functions for backward compatibility with callsites. validate_openclaw_symlinks() { - local entry name target expected - for entry in /sandbox/.openclaw/*; do - [ -L "$entry" ] || continue - name="$(basename "$entry")" - target="$(readlink -f "$entry" 2>/dev/null || true)" - expected="/sandbox/.openclaw-data/$name" - if [ "$target" != "$expected" ]; then - echo "[SECURITY] Symlink $entry points to unexpected target: $target (expected $expected)" >&2 - return 1 - fi - done + validate_config_symlinks /sandbox/.openclaw /sandbox/.openclaw-data } harden_openclaw_symlinks() { - local entry hardened failed - hardened=0 - failed=0 - - if ! command -v chattr >/dev/null 2>&1; then - echo "[SECURITY] chattr not available — relying on DAC + Landlock for .openclaw hardening" >&2 - return 0 - fi - - if chattr +i /sandbox/.openclaw 2>/dev/null; then - hardened=$((hardened + 1)) - else - failed=$((failed + 1)) - fi - - for entry in /sandbox/.openclaw/*; do - [ -L "$entry" ] || continue - if chattr +i "$entry" 2>/dev/null; then - hardened=$((hardened + 1)) - else - failed=$((failed + 1)) - fi - done - - if [ "$failed" -gt 0 ]; then - echo "[SECURITY] Immutable hardening applied to $hardened path(s); $failed path(s) could not be hardened — continuing with DAC + Landlock" >&2 - elif [ "$hardened" -gt 0 ]; then - echo "[SECURITY] Immutable hardening applied to /sandbox/.openclaw and validated symlinks" >&2 - fi + harden_config_symlinks /sandbox/.openclaw } # Write an auth profile JSON for the NVIDIA API key so the gateway can authenticate. @@ -812,26 +659,7 @@ harden_auth_profiles() { fi } -configure_messaging_channels() { - # Channel entries are baked into openclaw.json at image build time via - # NEMOCLAW_MESSAGING_CHANNELS_B64 (see Dockerfile). - # - # Telegram/Discord: placeholder tokens (openshell:resolve:env:*) flow through - # to API calls where the L7 proxy rewrites them with real secrets at egress. - # Real tokens are never visible inside the sandbox for these channels. - # - # Slack: apply_slack_token_override (runs before this function) resolves - # SLACK_BOT_TOKEN/SLACK_APP_TOKEN placeholders directly into openclaw.json so - # Bolt's in-process token validation passes. Both env vars are unset before the - # gateway starts (root path) so they do not leak into the sandbox process env. - [ -n "${TELEGRAM_BOT_TOKEN:-}" ] || [ -n "${DISCORD_BOT_TOKEN:-}" ] || [ -n "${SLACK_BOT_TOKEN:-}" ] || return 0 - - echo "[channels] Messaging channels active (baked at build time):" >&2 - [ -n "${TELEGRAM_BOT_TOKEN:-}" ] && echo "[channels] telegram (native)" >&2 - [ -n "${DISCORD_BOT_TOKEN:-}" ] && echo "[channels] discord (native)" >&2 - [ -n "${SLACK_BOT_TOKEN:-}" ] && echo "[channels] slack (native)" >&2 - return 0 -} +# configure_messaging_channels is provided by sandbox-init.sh (shared). # Print the local and remote dashboard URLs, appending the auth token if available. print_dashboard_urls() { @@ -1032,22 +860,10 @@ PROXYEOF done } | emit_sandbox_sourced_file "$_PROXY_ENV_FILE" -# Forward SIGTERM/SIGINT to child processes for graceful shutdown. -# This script is PID 1 — without a trap, signals interrupt wait and -# children are orphaned until Docker sends SIGKILL after the grace period. -cleanup() { - echo "[gateway] received signal, forwarding to children..." >&2 - local gateway_status=0 - kill -TERM "$GATEWAY_PID" 2>/dev/null || true - if [ -n "${AUTO_PAIR_PID:-}" ]; then - kill -TERM "$AUTO_PAIR_PID" 2>/dev/null || true - fi - wait "$GATEWAY_PID" 2>/dev/null || gateway_status=$? - if [ -n "${AUTO_PAIR_PID:-}" ]; then - wait "$AUTO_PAIR_PID" 2>/dev/null || true - fi - exit "$gateway_status" -} +# cleanup_on_signal is provided by sandbox-init.sh. It reads +# SANDBOX_CHILD_PIDS (array of all PIDs) and SANDBOX_WAIT_PID (the +# primary process whose exit status is returned). +# Each code path below sets these before registering the trap. # ── Main ───────────────────────────────────────────────────────── echo 'Setting up NemoClaw...' >&2 @@ -1066,7 +882,7 @@ fi if [ "$(id -u)" -ne 0 ]; then echo "[gateway] Running as non-root (uid=$(id -u)) — privilege separation disabled" >&2 export HOME=/sandbox - if ! verify_config_integrity; then + if ! verify_config_integrity /sandbox/.openclaw; then echo "[SECURITY] Config integrity check failed — refusing to start (non-root mode)" >&2 exit 1 fi @@ -1147,22 +963,32 @@ if [ "$(id -u)" -ne 0 ]; then # In non-root mode, detach gateway stdout/stderr from the sandbox-create # stream so openshell sandbox create can return once the container is ready. + # TODO(#2277-P2): migrate to shared emit_restricted_log() helper touch /tmp/gateway.log chmod 600 /tmp/gateway.log # Separate log for auto-pair in non-root mode as well. + # TODO(#2277-P2): migrate to shared emit_restricted_log() helper touch /tmp/auto-pair.log chmod 600 /tmp/auto-pair.log # Defence-in-depth: verify /tmp file permissions before launching services. + # shellcheck disable=SC2119 validate_tmp_permissions # Start gateway in background, auto-pair, then wait nohup "$OPENCLAW" gateway run --port "${_DASHBOARD_PORT}" >/tmp/gateway.log 2>&1 & GATEWAY_PID=$! echo "[gateway] openclaw gateway launched (pid $GATEWAY_PID)" >&2 - trap cleanup SIGTERM SIGINT start_auto_pair + # NOTE: PIDs are collected after launch; a signal arriving between trap + # registration and the final append is a small race window (same as before + # the shared-library refactor). Acceptable for entrypoint-level cleanup. + SANDBOX_CHILD_PIDS=("$GATEWAY_PID") + [ -n "${AUTO_PAIR_PID:-}" ] && SANDBOX_CHILD_PIDS+=("$AUTO_PAIR_PID") + # shellcheck disable=SC2034 # read by cleanup_on_signal from sandbox-init.sh + SANDBOX_WAIT_PID="$GATEWAY_PID" + trap cleanup_on_signal SIGTERM SIGINT print_dashboard_urls wait "$GATEWAY_PID" @@ -1172,7 +998,7 @@ fi # ── Root path (full privilege separation via gosu) ───────────── # Verify config integrity before starting anything -verify_config_integrity +verify_config_integrity /sandbox/.openclaw apply_model_override apply_cors_override apply_slack_token_override @@ -1184,11 +1010,6 @@ install_configure_guard # BEFORE chattr +i (which locks the config permanently). configure_messaging_channels -# SECURITY: Slack tokens were resolved into openclaw.json by apply_slack_token_override. -# Unset here — before any gosu sandbox child — so neither the sandbox user nor -# the gateway inherits them from the process environment. -unset SLACK_BOT_TOKEN SLACK_APP_TOKEN - # Write auth profile as sandbox user (needs writable .openclaw-data) # and recursively re-tighten any auth-profiles.json files under ~/.openclaw. gosu sandbox bash -c "$(declare -f write_auth_profile harden_auth_profiles); write_auth_profile; harden_auth_profiles" @@ -1199,11 +1020,13 @@ if [ ${#NEMOCLAW_CMD[@]} -gt 0 ]; then fi # SECURITY: Protect gateway log from sandbox user tampering +# TODO(#2277-P2): migrate to shared emit_restricted_log() helper touch /tmp/gateway.log chown gateway:gateway /tmp/gateway.log chmod 600 /tmp/gateway.log # Separate log for auto-pair so sandbox user can write to it +# TODO(#2277-P2): migrate to shared emit_restricted_log() helper touch /tmp/auto-pair.log chown sandbox:sandbox /tmp/auto-pair.log chmod 600 /tmp/auto-pair.log @@ -1220,6 +1043,7 @@ validate_openclaw_symlinks harden_openclaw_symlinks # Defence-in-depth: verify /tmp file permissions before launching services. +# shellcheck disable=SC2119 validate_tmp_permissions # Start the gateway as the 'gateway' user. @@ -1229,9 +1053,16 @@ validate_tmp_permissions nohup gosu gateway "$OPENCLAW" gateway run --port "${_DASHBOARD_PORT}" >/tmp/gateway.log 2>&1 & GATEWAY_PID=$! echo "[gateway] openclaw gateway launched as 'gateway' user (pid $GATEWAY_PID)" >&2 -trap cleanup SIGTERM SIGINT start_auto_pair +# NOTE: PIDs are collected after launch; a signal arriving between trap +# registration and the final append is a small race window (same as before +# the shared-library refactor). Acceptable for entrypoint-level cleanup. +SANDBOX_CHILD_PIDS=("$GATEWAY_PID") +[ -n "${AUTO_PAIR_PID:-}" ] && SANDBOX_CHILD_PIDS+=("$AUTO_PAIR_PID") +# shellcheck disable=SC2034 # read by cleanup_on_signal from sandbox-init.sh +SANDBOX_WAIT_PID="$GATEWAY_PID" +trap cleanup_on_signal SIGTERM SIGINT print_dashboard_urls # Keep container running by waiting on the gateway process. diff --git a/src/lib/sandbox-build-context.ts b/src/lib/sandbox-build-context.ts index 56e61754852..f628867e0d6 100644 --- a/src/lib/sandbox-build-context.ts +++ b/src/lib/sandbox-build-context.ts @@ -63,6 +63,12 @@ function stageOptimizedSandboxBuildContext(rootDir, tmpDir = os.tmpdir()) { path.join(rootDir, "scripts", "nemoclaw-start.sh"), path.join(stagedScriptsDir, "nemoclaw-start.sh"), ); + // Shared sandbox initialisation library sourced by the entrypoint (#2277) + fs.mkdirSync(path.join(stagedScriptsDir, "lib"), { recursive: true }); + fs.copyFileSync( + path.join(rootDir, "scripts", "lib", "sandbox-init.sh"), + path.join(stagedScriptsDir, "lib", "sandbox-init.sh"), + ); return { buildCtx, stagedDockerfile }; } diff --git a/test/e2e-gateway-isolation.sh b/test/e2e-gateway-isolation.sh index bf1b0c48e23..3aebcccbea2 100755 --- a/test/e2e-gateway-isolation.sh +++ b/test/e2e-gateway-isolation.sh @@ -214,8 +214,9 @@ info "14. Entrypoint drops dangerous capabilities from bounding set" # Run capsh directly with the same --drop flags as the entrypoint, then # check CapBnd. This avoids running the full entrypoint which starts # gateway services that fail in CI without a running OpenShell environment. -# Extract the --drop list from the entrypoint to stay in sync. -DROP_LIST=$(run_as_root "grep -oP '(?<=--drop=)[^ \\\\]+' /usr/local/bin/nemoclaw-start") +# Extract the --drop list from the shared sandbox-init library to stay in sync. +# The drop_capabilities() function lives in sandbox-init.sh (not the entrypoint). +DROP_LIST=$(run_as_root "grep -oP '(?<=--drop=)[^ \\\\]+' /usr/local/lib/nemoclaw/sandbox-init.sh") if [ -z "$DROP_LIST" ]; then fail "could not extract --drop list from entrypoint" else diff --git a/test/nemoclaw-start.test.ts b/test/nemoclaw-start.test.ts index a4a3719e5d5..ef682ab01da 100644 --- a/test/nemoclaw-start.test.ts +++ b/test/nemoclaw-start.test.ts @@ -22,8 +22,10 @@ describe("nemoclaw-start non-root fallback", () => { it("exits on config integrity failure in non-root mode", () => { const src = fs.readFileSync(START_SCRIPT, "utf-8"); - // Non-root block must call verify_config_integrity and exit 1 on failure - expect(src).toMatch(/if ! verify_config_integrity; then\s+.*exit 1/s); + const nonRootBlock = src.match(/if \[ "\$\(id -u\)" -ne 0 \]; then([\s\S]*?)# ── Root path/); + expect(nonRootBlock).toBeTruthy(); + // Non-root block must call verify_config_integrity (with config dir) and exit 1 on failure + expect(nonRootBlock[1]).toMatch(/if ! verify_config_integrity\b.*; then\s+.*exit 1/s); // Must not contain the old "proceeding anyway" fallback expect(src).not.toMatch(/proceeding anyway/i); }); @@ -51,9 +53,10 @@ describe("nemoclaw-start non-root fallback", () => { const block = nonRootBlock[1]; // Only check top-level echo lines that are NOT inside { } > file redirects - // or { } | helper piped redirects (e.g., emit_sandbox_sourced_file). - // Filter out lines inside brace-group redirects (proxy-env.sh, etc.) - const braceStripped = block.replace(/\{[\s\S]*?\}\s*(?:>\s*"[^"]*"|[|]\s*\w+[^\n]*)/g, ""); + // or { } | emit_sandbox_sourced_file pipe patterns (proxy-env.sh, etc.) + const braceStripped = block + .replace(/^\s*\{[\s\S]*?^\s*\}\s*>\s*"[^"]*"\s*$/gm, "") + .replace(/^\s*\{[\s\S]*?^\s*\}\s*\|\s*emit_sandbox_sourced_file\b[^\n]*$/gm, ""); const echoLines = braceStripped.match(/^\s*echo\s+.+$/gm) || []; expect(echoLines.length).toBeGreaterThan(0); for (const line of echoLines) { @@ -323,7 +326,6 @@ describe("runtime model override (#759)", () => { expect(fn).toBeTruthy(); // Guard checks all override env vars before returning early expect(fn[1]).toContain("NEMOCLAW_MODEL_OVERRIDE"); - expect(fn[1]).toContain("NEMOCLAW_REASONING"); // shfmt may format `|| return 0` as a standalone `return 0` on its own line expect(fn[1]).toMatch(/\|\|\s*return 0|^\s*return 0/m); }); @@ -410,16 +412,6 @@ describe("runtime model override (#759)", () => { expect(guard).toContain("NEMOCLAW_MAX_TOKENS"); expect(guard).toContain("NEMOCLAW_REASONING"); }); - - it("accesses NEMOCLAW_MODEL_OVERRIDE with :- fallback to avoid unbound variable under set -u", () => { - // NEMOCLAW_CONTEXT_WINDOW/MAX_TOKENS/REASONING are baked into the image ENV and are always - // non-empty, so the guard fires even when the operator never passes NEMOCLAW_MODEL_OVERRIDE. - // Without the :- fallback, set -euo pipefail would abort the entrypoint on every container - // start where only a context-window or reasoning override was intended. - const fn = src.match(/apply_model_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain("${NEMOCLAW_MODEL_OVERRIDE:-}"); - }); }); describe("runtime CORS origin override (#719)", () => { @@ -434,7 +426,7 @@ describe("runtime CORS origin override (#719)", () => { const nonRootBlock = src.match(/if \[ "\$\(id -u\)" -ne 0 \]; then([\s\S]*?)# ── Root path/); expect(nonRootBlock).toBeTruthy(); expect(nonRootBlock[1]).toMatch( - /apply_model_override[\s\S]*?apply_cors_override[\s\S]*?export_gateway_token/, + /apply_model_override[\s\S]*?apply_cors_override[\s\S]*?apply_slack_token_override[\s\S]*?export_gateway_token/, ); const rootBlock = src.match( @@ -483,115 +475,6 @@ describe("runtime CORS origin override (#719)", () => { }); }); -describe("Slack token placeholder resolution (#2085)", () => { - const src = fs.readFileSync(START_SCRIPT, "utf-8"); - - it("defines apply_slack_token_override function", () => { - expect(src).toContain("apply_slack_token_override()"); - expect(src).toContain("SLACK_BOT_TOKEN"); - expect(src).toContain("SLACK_APP_TOKEN"); - }); - - it("calls apply_slack_token_override after apply_cors_override in both paths", () => { - const nonRootBlock = src.match(/if \[ "\$\(id -u\)" -ne 0 \]; then([\s\S]*?)# ── Root path/); - expect(nonRootBlock).toBeTruthy(); - expect(nonRootBlock[1]).toMatch( - /apply_cors_override[\s\S]*?apply_slack_token_override[\s\S]*?export_gateway_token/, - ); - - const rootBlock = src.match( - /# ── Root path[\s\S]*?apply_cors_override\n\s*apply_slack_token_override\n\s*export_gateway_token/, - ); - expect(rootBlock).toBeTruthy(); - }); - - it("is a no-op when SLACK_BOT_TOKEN is not set", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toMatch(/\[ -n "\$\{SLACK_BOT_TOKEN:-\}" \] \|\| return 0/); - }); - - it("only applies override in root mode, fails fast when non-root and token is set", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toMatch(/id -u.*-ne 0/); - expect(fn[1]).toContain("requires a root container"); - // Non-root with SLACK_BOT_TOKEN set must return 1 (not silently skip) - expect(fn[1]).toMatch(/requires a root container[\s\S]*?return 1/); - }); - - it("guards against symlink attacks", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain('-L "$config_file"'); - expect(fn[1]).toContain("Refusing Slack token override"); - }); - - it("validates botToken prefix is xoxb-", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain("xoxb-"); - expect(fn[1]).toContain("does not start with xoxb-"); - }); - - it("validates appToken prefix is xapp-", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain("xapp-"); - expect(fn[1]).toContain("does not start with xapp-"); - }); - - it("warns when SLACK_BOT_TOKEN is set but SLACK_APP_TOKEN is missing", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain("SLACK_APP_TOKEN is missing"); - expect(fn[1]).toContain("Socket Mode requires both tokens"); - }); - - it("recomputes config hash after override", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain("sha256sum openclaw.json"); - expect(fn[1]).toContain("config-hash"); - }); - - it("resolves openshell:resolve:env: placeholders via Python", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toContain("openshell:resolve:env:"); - expect(fn[1]).toContain("botToken"); - expect(fn[1]).toContain("appToken"); - }); - - it("unsets SLACK_BOT_TOKEN and SLACK_APP_TOKEN before first gosu sandbox call in root path", () => { - // unset must appear after configure_messaging_channels and before the first gosu sandbox child - const block = src.match(/configure_messaging_channels\n([\s\S]*?)gosu sandbox bash/); - expect(block).toBeTruthy(); - expect(block[1]).toContain("unset SLACK_BOT_TOKEN SLACK_APP_TOKEN"); - }); - - it("fails fast when SLACK_BOT_TOKEN is set in non-root mode", () => { - // Fail-fast is now folded into apply_slack_token_override itself. - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - // Function must return 1 (not 0) when non-root and SLACK_BOT_TOKEN is set - expect(fn[1]).toMatch(/id -u.*-ne 0[\s\S]*?requires a root container[\s\S]*?return 1/); - - // The non-root call site must not have a separate post-call SLACK_BOT_TOKEN check - const nonRootBlock = src.match(/if \[ "\$\(id -u\)" -ne 0 \]; then([\s\S]*?)# ── Root path/); - expect(nonRootBlock).toBeTruthy(); - expect(nonRootBlock[1]).not.toMatch( - /apply_slack_token_override[\s\S]*?if \[ -n "\$\{SLACK_BOT_TOKEN/, - ); - }); - - it("passes tokens via env prefix, not as positional args", () => { - const fn = src.match(/apply_slack_token_override\(\) \{([\s\S]*?)^}/m); - expect(fn).toBeTruthy(); - expect(fn[1]).toMatch(/SLACK_BOT_TOKEN="\$SLACK_BOT_TOKEN" \\/); - }); -}); - describe("nemoclaw-start auto-pair client whitelisting (#117)", () => { const src = fs.readFileSync(START_SCRIPT, "utf-8"); @@ -656,51 +539,38 @@ describe("nemoclaw-start auto-pair client whitelisting (#117)", () => { describe("nemoclaw-start signal handling", () => { const src = fs.readFileSync(START_SCRIPT, "utf-8"); - it("defines cleanup() as a single top-level function", () => { - const matches = src.match(/^cleanup\(\)/gm); - expect(matches).toHaveLength(1); - }); - - it("cleanup() forwards SIGTERM to both GATEWAY_PID and AUTO_PAIR_PID", () => { - const cleanup = src.match(/cleanup\(\) \{[\s\S]*?^}/m)?.[0]; - expect(cleanup).toBeDefined(); - expect(cleanup).toMatch(/kill -TERM "\$GATEWAY_PID"/); - expect(cleanup).toMatch(/kill -TERM "\$AUTO_PAIR_PID"/); - }); - - it("cleanup() waits for both child processes", () => { - const cleanup = src.match(/cleanup\(\) \{[\s\S]*?^}/m)?.[0]; - expect(cleanup).toMatch(/wait "\$GATEWAY_PID"/); - expect(cleanup).toMatch(/wait "\$AUTO_PAIR_PID"/); - }); - - it("cleanup() exits with the gateway exit status", () => { - const cleanup = src.match(/cleanup\(\) \{[\s\S]*?^}/m)?.[0]; - expect(cleanup).toMatch(/exit "\$gateway_status"/); + it("uses shared cleanup_on_signal from sandbox-init.sh", () => { + // cleanup_on_signal is provided by sandbox-init.sh; the entrypoint + // must NOT define its own cleanup() — it uses the shared version. + const localCleanup = src.match(/^cleanup\(\)/gm); + expect(localCleanup).toBeNull(); + // Must reference cleanup_on_signal in trap registrations + expect(src).toContain("trap cleanup_on_signal SIGTERM SIGINT"); }); - it("registers trap before start_auto_pair in non-root path", () => { - // trap must appear before start_auto_pair within the non-root block. - // Use the Root path comment as boundary instead of ^fi$ which matches - // nested fi inside helper functions. + it("sets SANDBOX_CHILD_PIDS and SANDBOX_WAIT_PID before trap in non-root path", () => { const nonRootBlock = src.match(/if \[ "\$\(id -u\)" -ne 0 \]; then[\s\S]*?# ── Root path/)?.[0]; expect(nonRootBlock).toBeDefined(); - const trapIdx = nonRootBlock.indexOf("trap cleanup SIGTERM SIGINT"); - // Match the call site "start_auto_pair\n" (not the function definition "start_auto_pair() {") - const autoIdx = nonRootBlock.search(/^\s*start_auto_pair\s*$/m); - expect(trapIdx).toBeGreaterThan(-1); - expect(autoIdx).toBeGreaterThan(-1); - expect(trapIdx).toBeLessThan(autoIdx); + expect(nonRootBlock).toContain("SANDBOX_CHILD_PIDS="); + expect(nonRootBlock).toContain("SANDBOX_WAIT_PID="); + const pidsIdx = nonRootBlock.indexOf("SANDBOX_CHILD_PIDS="); + const waitIdx = nonRootBlock.indexOf("SANDBOX_WAIT_PID="); + const trapIdx = nonRootBlock.indexOf("trap cleanup_on_signal"); + expect(waitIdx).toBeGreaterThan(-1); + expect(pidsIdx).toBeLessThan(trapIdx); + expect(waitIdx).toBeLessThan(trapIdx); }); - it("registers trap before start_auto_pair in root path", () => { - // In the root path (after the non-root block), trap must precede start_auto_pair + it("sets SANDBOX_CHILD_PIDS and SANDBOX_WAIT_PID before trap in root path", () => { const rootBlock = src.split(/# ── Root path/)[1] || ""; - const trapIdx = rootBlock.indexOf("trap cleanup SIGTERM SIGINT"); - const autoIdx = rootBlock.indexOf("start_auto_pair"); - expect(trapIdx).toBeGreaterThan(-1); - expect(autoIdx).toBeGreaterThan(-1); - expect(trapIdx).toBeLessThan(autoIdx); + expect(rootBlock).toContain("SANDBOX_CHILD_PIDS="); + expect(rootBlock).toContain("SANDBOX_WAIT_PID="); + const pidsIdx = rootBlock.indexOf("SANDBOX_CHILD_PIDS="); + const waitIdx = rootBlock.indexOf("SANDBOX_WAIT_PID="); + const trapIdx = rootBlock.indexOf("trap cleanup_on_signal"); + expect(waitIdx).toBeGreaterThan(-1); + expect(pidsIdx).toBeLessThan(trapIdx); + expect(waitIdx).toBeLessThan(trapIdx); }); it("captures AUTO_PAIR_PID from background process", () => { diff --git a/test/sandbox-init.test.ts b/test/sandbox-init.test.ts new file mode 100644 index 00000000000..311b34d39a9 --- /dev/null +++ b/test/sandbox-init.test.ts @@ -0,0 +1,516 @@ +// @ts-nocheck +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { execFileSync } from "node:child_process"; +import { + mkdtempSync, + writeFileSync, + readFileSync, + mkdirSync, + symlinkSync, + lstatSync, + chmodSync, + existsSync, + renameSync, + rmSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const SANDBOX_INIT = join(import.meta.dirname, "../scripts/lib/sandbox-init.sh"); + +/** Cross-platform octal permission string (macOS uses -f, Linux uses -c). */ +function getOctalPerms(filePath: string): string { + try { + // Linux: stat -c '%a' file + return execFileSync("stat", ["-c", "%a", filePath], { encoding: "utf-8" }).trim(); + } catch { + // macOS: stat -f '%Lp' file + return execFileSync("stat", ["-f", "%Lp", filePath], { encoding: "utf-8" }).trim(); + } +} + +/** + * Run a bash snippet that sources sandbox-init.sh and executes the given body. + * Returns { stdout, stderr } as trimmed strings. + */ +function runWithLib( + body: string, + opts: { env?: Record; expectFail?: boolean } = {}, +) { + const script = [ + "#!/usr/bin/env bash", + "set -euo pipefail", + `source ${JSON.stringify(SANDBOX_INIT)}`, + body, + ].join("\n"); + const tmpFile = join(tmpdir(), `sandbox-init-test-${process.pid}-${Date.now()}.sh`); + try { + writeFileSync(tmpFile, script, { mode: 0o700 }); + const result = execFileSync("bash", [tmpFile], { + encoding: "utf-8", + env: { ...process.env, ...opts.env }, + stdio: ["pipe", "pipe", "pipe"], + }); + return { stdout: result.trim(), stderr: "" }; + } catch (e: any) { + if (opts.expectFail) { + return { + stdout: (e.stdout || "").toString().trim(), + stderr: (e.stderr || "").toString().trim(), + }; + } + throw e; + } finally { + try { + execFileSync("rm", ["-f", tmpFile]); + } catch { + /* ignore */ + } + } +} + +function pathExists(filePath: string): boolean { + try { + lstatSync(filePath); + return true; + } catch { + return false; + } +} + +function backupTmpArtifacts(paths: string[], backupDir: string): Record { + const backups: Record = {}; + + for (const originalPath of paths) { + if (!pathExists(originalPath)) { + continue; + } + const backupPath = join( + backupDir, + `${originalPath.replaceAll("/", "_").replace(/^_+/, "")}.backup`, + ); + renameSync(originalPath, backupPath); + backups[originalPath] = backupPath; + } + + return backups; +} + +function restoreTmpArtifacts(paths: string[], backups: Record): void { + for (const originalPath of paths) { + if (pathExists(originalPath)) { + rmSync(originalPath, { force: true, recursive: true }); + } + const backupPath = backups[originalPath]; + if (backupPath && pathExists(backupPath)) { + renameSync(backupPath, originalPath); + } + } +} + +describe("scripts/lib/sandbox-init.sh", () => { + describe("emit_sandbox_sourced_file", () => { + let workDir: string; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "sandbox-init-emit-")); + }); + + afterEach(() => { + execFileSync("rm", ["-rf", workDir]); + }); + + it("creates a file with 444 permissions", () => { + const target = join(workDir, "test-sourced.sh"); + runWithLib(`echo 'export FOO=bar' | emit_sandbox_sourced_file ${JSON.stringify(target)}`); + + expect(existsSync(target)).toBe(true); + const content = readFileSync(target, "utf-8"); + expect(content).toContain("export FOO=bar"); + + // Check permissions — 444 in octal + const perms = getOctalPerms(target); + expect(perms).toBe("444"); + }); + + it("overwrites existing file cleanly", () => { + const target = join(workDir, "overwrite.sh"); + writeFileSync(target, "OLD CONTENT"); + runWithLib(`echo 'NEW CONTENT' | emit_sandbox_sourced_file ${JSON.stringify(target)}`); + + const content = readFileSync(target, "utf-8"); + expect(content).toContain("NEW CONTENT"); + expect(content).not.toContain("OLD CONTENT"); + }); + + it("removes symlink before writing (anti-symlink attack)", () => { + const target = join(workDir, "proxy-env.sh"); + const sensitive = join(workDir, "sensitive-data"); + writeFileSync(sensitive, "SECRET_DATA"); + symlinkSync(sensitive, target); + + runWithLib(`echo 'export X=1' | emit_sandbox_sourced_file ${JSON.stringify(target)}`); + + // Target should now be a regular file, not a symlink + const stat = lstatSync(target); + expect(stat.isSymbolicLink()).toBe(false); + // Sensitive file should be untouched + expect(readFileSync(sensitive, "utf-8")).toBe("SECRET_DATA"); + }); + + it("accepts heredoc input", () => { + const target = join(workDir, "heredoc.sh"); + runWithLib(` +emit_sandbox_sourced_file ${JSON.stringify(target)} <<'EOF' +export A="hello" +export B="world" +EOF + `); + + const content = readFileSync(target, "utf-8"); + expect(content).toContain('export A="hello"'); + expect(content).toContain('export B="world"'); + }); + }); + + describe("validate_tmp_permissions", () => { + let workDir: string; + let tmpBackups: Record; + const TMP_ARTIFACTS = [ + "/tmp/nemoclaw-proxy-env.sh", + "/tmp/gateway.log", + "/tmp/auto-pair.log", + ]; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "sandbox-init-validate-")); + tmpBackups = backupTmpArtifacts(TMP_ARTIFACTS, workDir); + }); + + afterEach(() => { + restoreTmpArtifacts(TMP_ARTIFACTS, tmpBackups); + execFileSync("rm", ["-rf", workDir]); + }); + + it("passes when no monitored files exist", () => { + // validate_tmp_permissions should succeed when files don't exist + // (they're skipped via [ -f "$f" ] || continue) + runWithLib(` + validate_tmp_permissions + echo "PASSED" + `); + }); + + it("detects bad permissions on sourced files", () => { + const testFile = join(workDir, "bad-sourced.sh"); + writeFileSync(testFile, "# bad permissions"); + chmodSync(testFile, 0o644); // writable — should fail + + const { stderr } = runWithLib( + `validate_tmp_permissions ${JSON.stringify(testFile)}`, + { expectFail: true }, + ); + expect(stderr).toContain("unsafe permissions"); + }); + + it("passes with correct 444 permissions on sourced files", () => { + const testFile = join(workDir, "good-sourced.sh"); + writeFileSync(testFile, "# good permissions"); + chmodSync(testFile, 0o444); + + runWithLib(` + validate_tmp_permissions ${JSON.stringify(testFile)} + echo "PASSED" + `); + }); + }); + + describe("verify_config_integrity", () => { + let workDir: string; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "sandbox-init-integrity-")); + }); + + afterEach(() => { + execFileSync("rm", ["-rf", workDir]); + }); + + it("fails when hash file is missing", () => { + const { stderr } = runWithLib( + `verify_config_integrity ${JSON.stringify(workDir)}`, + { expectFail: true }, + ); + expect(stderr).toContain("Config hash file missing"); + }); + + it("passes when config matches hash", () => { + const configFile = join(workDir, "config.json"); + writeFileSync(configFile, '{"test": true}'); + // Generate hash + execFileSync("bash", [ + "-c", + `cd ${JSON.stringify(workDir)} && sha256sum config.json > .config-hash`, + ]); + + runWithLib(` + verify_config_integrity ${JSON.stringify(workDir)} + echo "INTEGRITY_OK" + `); + }); + + it("fails when config is tampered", () => { + const configFile = join(workDir, "config.json"); + writeFileSync(configFile, '{"test": true}'); + execFileSync("bash", [ + "-c", + `cd ${JSON.stringify(workDir)} && sha256sum config.json > .config-hash`, + ]); + // Tamper with config + writeFileSync(configFile, '{"test": false, "injected": "malicious"}'); + + const { stderr } = runWithLib( + `verify_config_integrity ${JSON.stringify(workDir)}`, + { expectFail: true }, + ); + expect(stderr).toContain("integrity check FAILED"); + }); + }); + + describe("lock_rc_files", () => { + let workDir: string; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "sandbox-init-lock-")); + }); + + afterEach(() => { + // Need to make writable before cleanup + try { + chmodSync(join(workDir, ".bashrc"), 0o644); + } catch { + /* ignore */ + } + try { + chmodSync(join(workDir, ".profile"), 0o644); + } catch { + /* ignore */ + } + execFileSync("rm", ["-rf", workDir]); + }); + + it("sets .bashrc and .profile to 444", () => { + writeFileSync(join(workDir, ".bashrc"), "# bashrc"); + writeFileSync(join(workDir, ".profile"), "# profile"); + + runWithLib(`lock_rc_files ${JSON.stringify(workDir)}`); + + const bashrcPerms = getOctalPerms(join(workDir, ".bashrc")); + const profilePerms = getOctalPerms(join(workDir, ".profile")); + expect(bashrcPerms).toBe("444"); + expect(profilePerms).toBe("444"); + }); + + it("is a no-op when files do not exist", () => { + // Should not throw + runWithLib(`lock_rc_files ${JSON.stringify(workDir)}`); + }); + }); + + describe("drop_capabilities", () => { + it("function is defined and callable", () => { + // We can't test actual capsh on macOS, but verify the function exists + // and handles the no-capsh case gracefully. Capture stderr via redirect. + const { stdout } = runWithLib( + ` + # Hide capsh from PATH so the function falls through + drop_capabilities /usr/local/bin/fake-entrypoint 2>&1 + echo "FALLTHROUGH_OK" + `, + { env: { PATH: "/usr/bin:/bin", NEMOCLAW_CAPS_DROPPED: "" } }, + ); + expect(stdout).toContain("capsh not available"); + expect(stdout).toContain("FALLTHROUGH_OK"); + }); + + it("skips when NEMOCLAW_CAPS_DROPPED=1", () => { + const { stdout } = runWithLib( + ` + NEMOCLAW_CAPS_DROPPED=1 + drop_capabilities /usr/local/bin/fake-entrypoint + echo "SKIPPED_OK" + `, + ); + expect(stdout).toContain("SKIPPED_OK"); + }); + }); + + describe("validate_config_symlinks", () => { + let workDir: string; + + beforeEach(() => { + workDir = mkdtempSync(join(tmpdir(), "sandbox-init-symlinks-")); + mkdirSync(join(workDir, "config")); + mkdirSync(join(workDir, "data")); + }); + + afterEach(() => { + execFileSync("rm", ["-rf", workDir]); + }); + + it("passes when symlinks point to expected targets", () => { + const dataFile = join(workDir, "data", "agents"); + writeFileSync(dataFile, "data"); + symlinkSync(dataFile, join(workDir, "config", "agents")); + + // validate_config_symlinks resolves both sides via readlink -f, + // so macOS /var → /private/var doesn't cause false positives. + runWithLib(` + validate_config_symlinks ${JSON.stringify(join(workDir, "config"))} ${JSON.stringify(join(workDir, "data"))} + echo "SYMLINKS_OK" + `); + }); + + it("fails when symlink points to unexpected target", () => { + const badTarget = join(workDir, "malicious"); + writeFileSync(badTarget, "evil"); + symlinkSync(badTarget, join(workDir, "config", "agents")); + + const { stderr } = runWithLib( + `validate_config_symlinks ${JSON.stringify(join(workDir, "config"))} ${JSON.stringify(join(workDir, "data"))}`, + { expectFail: true }, + ); + expect(stderr).toContain("unexpected target"); + }); + + it("passes when directory has no symlinks", () => { + writeFileSync(join(workDir, "config", "regular-file"), "not a symlink"); + + runWithLib(` + validate_config_symlinks ${JSON.stringify(join(workDir, "config"))} ${JSON.stringify(join(workDir, "data"))} + echo "NO_SYMLINKS_OK" + `); + }); + }); + + describe("configure_messaging_channels", () => { + it("returns silently when no tokens are set", () => { + const { stderr } = runWithLib("configure_messaging_channels", { + env: { + TELEGRAM_BOT_TOKEN: "", + DISCORD_BOT_TOKEN: "", + SLACK_BOT_TOKEN: "", + }, + }); + expect(stderr).not.toContain("[channels]"); + }); + + it("logs active channels when tokens are present", () => { + // configure_messaging_channels writes to stderr; redirect to stdout to capture it + const { stdout } = runWithLib("configure_messaging_channels 2>&1", { + env: { + TELEGRAM_BOT_TOKEN: "fake-token", + DISCORD_BOT_TOKEN: "", + SLACK_BOT_TOKEN: "fake-slack", + }, + }); + expect(stdout).toContain("telegram"); + expect(stdout).toContain("slack"); + expect(stdout).not.toContain("discord"); + }); + }); + + describe("cleanup_on_signal", () => { + it("function is defined and uses SANDBOX_CHILD_PIDS", () => { + // Verify the function exists and handles empty PID list gracefully + const { stdout } = runWithLib(` + SANDBOX_CHILD_PIDS=() + SANDBOX_WAIT_PID="" + # Override exit so we can test + exit() { echo "EXIT_\$1"; } + cleanup_on_signal + `); + expect(stdout).toContain("EXIT_0"); + }); + }); + + describe("double-source guard", () => { + it("does not redefine functions when sourced twice", () => { + runWithLib(` + # Source again — should be a no-op + source ${JSON.stringify(SANDBOX_INIT)} + # Functions should still work + echo "test" | emit_sandbox_sourced_file /dev/null 2>/dev/null || true + echo "DOUBLE_SOURCE_OK" + `); + }); + }); + + describe("both entrypoints source the shared library", () => { + it("nemoclaw-start.sh sources sandbox-init.sh", () => { + const src = readFileSync( + join(import.meta.dirname, "../scripts/nemoclaw-start.sh"), + "utf-8", + ); + expect(src).toContain("source"); + expect(src).toContain("sandbox-init.sh"); + }); + + it("hermes start.sh sources sandbox-init.sh", () => { + const src = readFileSync( + join(import.meta.dirname, "../agents/hermes/start.sh"), + "utf-8", + ); + expect(src).toContain("source"); + expect(src).toContain("sandbox-init.sh"); + }); + + it("hermes start.sh calls lock_rc_files (vulnerability fix)", () => { + const src = readFileSync( + join(import.meta.dirname, "../agents/hermes/start.sh"), + "utf-8", + ); + expect(src).toContain("lock_rc_files"); + }); + + it("hermes start.sh uses emit_sandbox_sourced_file for proxy config", () => { + const src = readFileSync( + join(import.meta.dirname, "../agents/hermes/start.sh"), + "utf-8", + ); + expect(src).toContain("emit_sandbox_sourced_file"); + // Should NOT contain the old inline _write_proxy_snippet pattern + expect(src).not.toContain("_write_proxy_snippet"); + expect(src).not.toContain("_PROXY_MARKER_BEGIN"); + }); + + it("hermes start.sh calls validate_tmp_permissions", () => { + const src = readFileSync( + join(import.meta.dirname, "../agents/hermes/start.sh"), + "utf-8", + ); + expect(src).toContain("validate_tmp_permissions"); + }); + + it("nemoclaw-start.sh uses emit_sandbox_sourced_file for proxy-env.sh", () => { + const src = readFileSync( + join(import.meta.dirname, "../scripts/nemoclaw-start.sh"), + "utf-8", + ); + expect(src).toContain("emit_sandbox_sourced_file"); + // Should NOT contain old chmod 644 for proxy-env + expect(src).not.toMatch(/chmod 644.*\$_PROXY_ENV_FILE/); + }); + + it("nemoclaw-start.sh uses parameterized verify_config_integrity", () => { + const src = readFileSync( + join(import.meta.dirname, "../scripts/nemoclaw-start.sh"), + "utf-8", + ); + expect(src).toContain("verify_config_integrity /sandbox/.openclaw"); + }); + }); +}); diff --git a/test/service-env.test.ts b/test/service-env.test.ts index 5cce25e1034..71667de12db 100644 --- a/test/service-env.test.ts +++ b/test/service-env.test.ts @@ -201,6 +201,10 @@ describe("service environment", () => { }); describe("proxy environment variables (issue #626)", () => { + // The proxy persistence block calls emit_sandbox_sourced_file from the + // shared library. Wrappers that execute the extracted block must source it. + const sandboxInitSource = `source ${JSON.stringify(join(import.meta.dirname, "../scripts/lib/sandbox-init.sh"))}`; + function extractToolRedirects() { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const block = execFileSync("sed", ["-n", "/^_TOOL_REDIRECTS=/,/^done$/p", scriptPath], { @@ -215,22 +219,6 @@ describe("service environment", () => { return block.trimEnd(); } - function extractEmitHelper() { - const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); - const block = execFileSync( - "sed", - ["-n", "/^emit_sandbox_sourced_file()/,/^}/p", scriptPath], - { encoding: "utf-8" }, - ); - if (!block.trim()) { - throw new Error( - "Failed to extract emit_sandbox_sourced_file from scripts/nemoclaw-start.sh — " + - "the function may have been moved or renamed", - ); - } - return block.trimEnd(); - } - function extractProxyVars(env = {}) { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const proxyBlock = execFileSync( @@ -329,7 +317,7 @@ describe("service environment", () => { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const persistBlock = execFileSync( "sed", - ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file.*\$_PROXY_ENV_FILE/p", scriptPath], { encoding: "utf-8" }, ); if (!persistBlock.trim()) { @@ -339,10 +327,9 @@ describe("service environment", () => { ); } const toolRedirects = extractToolRedirects(); - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", - emitHelper, + sandboxInitSource, toolRedirects, 'PROXY_HOST="10.200.0.1"', 'PROXY_PORT="3128"', @@ -374,16 +361,19 @@ describe("service environment", () => { expect(envFile).toContain("GNUPGHOME=/tmp/.gnupg"); expect(envFile).toContain("PYTHON_HISTORY=/tmp/.python_history"); expect(envFile).toContain("npm_config_prefix=/tmp/npm-global"); - // SECURITY: file must be read-only (#2181) - // Use platform-appropriate stat format: -c '%a' on Linux, -f '%Lp' on macOS - const proxyEnvPath = join(fakeDataDir, "proxy-env.sh"); - let stat; + // Permission should be 444 (hardened via emit_sandbox_sourced_file) + // Cross-platform: Linux uses stat -c '%a', macOS uses stat -f '%Lp' + let perms: string; try { - stat = execFileSync("stat", ["-c", "%a", proxyEnvPath], { encoding: "utf-8" }).trim(); + perms = execFileSync("stat", ["-c", "%a", join(fakeDataDir, "proxy-env.sh")], { + encoding: "utf-8", + }).trim(); } catch { - stat = execFileSync("stat", ["-f", "%Lp", proxyEnvPath], { encoding: "utf-8" }).trim(); + perms = execFileSync("stat", ["-f", "%Lp", join(fakeDataDir, "proxy-env.sh")], { + encoding: "utf-8", + }).trim(); } - expect(stat).toBe("444"); + expect(perms).toBe("444"); } finally { try { unlinkSync(tmpFile); @@ -406,7 +396,7 @@ describe("service environment", () => { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const persistBlock = execFileSync( "sed", - ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file.*\$_PROXY_ENV_FILE/p", scriptPath], { encoding: "utf-8" }, ); if (!persistBlock.trim()) { @@ -416,10 +406,9 @@ describe("service environment", () => { ); } const toolRedirects = extractToolRedirects(); - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", - emitHelper, + sandboxInitSource, toolRedirects, 'PROXY_HOST="10.200.0.1"', 'PROXY_PORT="3128"', @@ -462,7 +451,7 @@ describe("service environment", () => { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const persistBlock = execFileSync( "sed", - ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file.*\$_PROXY_ENV_FILE/p", scriptPath], { encoding: "utf-8" }, ); if (!persistBlock.trim()) { @@ -472,16 +461,15 @@ describe("service environment", () => { ); } const toolRedirects = extractToolRedirects(); - const emitHelper = extractEmitHelper(); const makeWrapper = (host) => [ "#!/usr/bin/env bash", - emitHelper, + sandboxInitSource, toolRedirects, `PROXY_HOST="${host}"`, 'PROXY_PORT="3128"', - `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, - `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, + '_PROXY_URL="http://${PROXY_HOST}:${PROXY_PORT}"', + '_NO_PROXY_VAL="localhost,127.0.0.1,::1,${PROXY_HOST}"', persistBlock .trimEnd() .replaceAll("/tmp/nemoclaw-proxy-env.sh", `${fakeDataDir}/proxy-env.sh`), @@ -519,7 +507,7 @@ describe("service environment", () => { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const persistBlock = execFileSync( "sed", - ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file.*\$_PROXY_ENV_FILE/p", scriptPath], { encoding: "utf-8" }, ); if (!persistBlock.trim()) { @@ -533,10 +521,9 @@ describe("service environment", () => { const proxyEnvPath = join(fakeDataDir, "proxy-env.sh"); execFileSync("ln", ["-sf", sensitiveFile, proxyEnvPath]); const toolRedirects = extractToolRedirects(); - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", - emitHelper, + sandboxInitSource, toolRedirects, 'PROXY_HOST="10.200.0.1"', 'PROXY_PORT="3128"', @@ -613,27 +600,26 @@ describe("service environment", () => { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const persistBlock = execFileSync( "sed", - ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file.*\$_PROXY_ENV_FILE/p", scriptPath], { encoding: "utf-8" }, ); if (!persistBlock.trim()) { throw new Error( - "sed anchors (_PROXY_ENV_FILE…emit_sandbox_sourced_file) not found in nemoclaw-start.sh — test cannot run", + "sed anchors (_PROXY_ENV_FILE=…emit_sandbox_sourced_file) not found in nemoclaw-start.sh — test cannot run", ); } - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", "set -euo pipefail", - emitHelper, - `PROXY_HOST="10.200.0.1"`, - `PROXY_PORT="3128"`, - `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, - `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, - `NODE_USE_ENV_PROXY=1`, + sandboxInitSource, + 'PROXY_HOST="10.200.0.1"', + 'PROXY_PORT="3128"', + '_PROXY_URL="http://${PROXY_HOST}:${PROXY_PORT}"', + '_NO_PROXY_VAL="localhost,127.0.0.1,::1,${PROXY_HOST}"', + 'NODE_USE_ENV_PROXY=1', + '_TOOL_REDIRECTS=()', `_AXIOS_FIX_SCRIPT="${fakeFixScript}"`, `_WS_FIX_SCRIPT="/nonexistent/ws-proxy-fix.js"`, - `_TOOL_REDIRECTS=()`, "set +u # array expansion safe on macOS bash", persistBlock .trimEnd() @@ -666,25 +652,24 @@ describe("service environment", () => { const scriptPath = join(import.meta.dirname, "../scripts/nemoclaw-start.sh"); const persistBlock = execFileSync( "sed", - ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file/p", scriptPath], + ["-n", "/^_PROXY_ENV_FILE=/,/emit_sandbox_sourced_file.*\$_PROXY_ENV_FILE/p", scriptPath], { encoding: "utf-8" }, ); if (!persistBlock.trim()) { throw new Error("sed anchors not found in nemoclaw-start.sh — test cannot run"); } - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", "set -euo pipefail", - emitHelper, - `PROXY_HOST="10.200.0.1"`, - `PROXY_PORT="3128"`, - `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, - `_NO_PROXY_VAL="localhost,127.0.0.1,::1,\${PROXY_HOST}"`, + sandboxInitSource, + 'PROXY_HOST="10.200.0.1"', + 'PROXY_PORT="3128"', + '_PROXY_URL="http://${PROXY_HOST}:${PROXY_PORT}"', + '_NO_PROXY_VAL="localhost,127.0.0.1,::1,${PROXY_HOST}"', // NODE_USE_ENV_PROXY intentionally NOT set + '_TOOL_REDIRECTS=()', `_AXIOS_FIX_SCRIPT="${fakeFixScript}"`, `_WS_FIX_SCRIPT="/nonexistent/ws-proxy-fix.js"`, - `_TOOL_REDIRECTS=()`, "set +u # array expansion safe on macOS bash", persistBlock .trimEnd() @@ -726,11 +711,10 @@ describe("service environment", () => { "sed anchors (_PROXY_ENV_FILE…emit_sandbox_sourced_file) not found in nemoclaw-start.sh — test cannot run", ); } - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", "set -euo pipefail", - emitHelper, + sandboxInitSource, `PROXY_HOST="10.200.0.1"`, `PROXY_PORT="3128"`, `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, @@ -774,11 +758,10 @@ describe("service environment", () => { if (!persistBlock.trim()) { throw new Error("sed anchors not found in nemoclaw-start.sh — test cannot run"); } - const emitHelper = extractEmitHelper(); const wrapper = [ "#!/usr/bin/env bash", "set -euo pipefail", - emitHelper, + sandboxInitSource, `PROXY_HOST="10.200.0.1"`, `PROXY_PORT="3128"`, `_PROXY_URL="http://\${PROXY_HOST}:\${PROXY_PORT}"`, From e9a900b9bcb293a3e9214a5210ee7a789bcacf42 Mon Sep 17 00:00:00 2001 From: paritoshd-nv <153745393+paritoshd-nv@users.noreply.github.com> Date: Wed, 22 Apr 2026 20:04:57 -0400 Subject: [PATCH 09/17] fix(onboard): warn that dashboard URL is printed only once (#2278) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add "save it now — will not be printed again" to the tokenized dashboard URL line in both onboard print paths (OpenClaw in onboard.ts, agent UI in agent-onboard.ts). Treat the URL as a one-shot credential (GitHub-PAT style) rather than exposing it through `nemoclaw status`, which would widen incidental leakage via CI logs and issue-paste output. Fixes #2167 ## Summary ## Related Issue ## Changes ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [x] `npm test` passes - [x] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: --- Signed-off-by: Your Name ## Summary by CodeRabbit ## Release Notes * **Documentation** * Improved guidance in onboarding console messages regarding tokenized URLs. When users encounter the authentication token during the setup process, they are now explicitly instructed to save it immediately and clearly informed that the token will not be printed again in the future. This applies to both agent and gateway UI setup flows. Signed-off-by: Paritosh Dixit --- src/lib/agent-onboard.test.ts | 6 ++++-- src/lib/agent-onboard.ts | 4 +++- src/lib/onboard.ts | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/lib/agent-onboard.test.ts b/src/lib/agent-onboard.test.ts index 0608de714e9..0017e4171e7 100644 --- a/src/lib/agent-onboard.test.ts +++ b/src/lib/agent-onboard.test.ts @@ -76,14 +76,16 @@ describe("printDashboardUi — regression for #2078 (port 8642 is not a chat UI) expect(noteSpy).not.toHaveBeenCalled(); }); - it("keeps the existing tokenized URL wording for UI-kind agents", () => { + it("prints tokenized URL with save-now warning for UI-kind agents", () => { printDashboardUi("sandbox-y", "tok", uiAgent, { note: noteSpy, buildControlUiUrls: buildUrlsLoopback, }); const output = logSpy.mock.calls.map((args) => String(args[0])).join("\n"); - expect(output).toContain("Ficticious UI (tokenized URL; treat it like a password)"); + expect(output).toContain( + "Ficticious UI (tokenized URL; treat it like a password; save it now - it will not be printed again)", + ); expect(output).toContain("Port 19000 must be forwarded before opening this URL."); expect(output).toContain("http://127.0.0.1:19000/#token=tok"); }); diff --git a/src/lib/agent-onboard.ts b/src/lib/agent-onboard.ts index 8d4b67025c1..a8fc6ed7769 100644 --- a/src/lib/agent-onboard.ts +++ b/src/lib/agent-onboard.ts @@ -252,7 +252,9 @@ export function printDashboardUi( } if (token) { - console.log(` ${info.displayName} ${label} (tokenized URL; treat it like a password)`); + console.log( + ` ${info.displayName} ${label} (tokenized URL; treat it like a password; save it now - it will not be printed again)`, + ); console.log(` Port ${info.port} must be forwarded before opening this URL.`); for (const url of deps.buildControlUiUrls(token, info.port)) { console.log(` ${url}`); diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 9804d9cb7bc..1bb1a5a74bc 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -6270,7 +6270,9 @@ function printDashboard(sandboxName, model, provider, nimContainer = null, agent }, }); } else if (token) { - console.log(" OpenClaw UI (tokenized URL; treat it like a password)"); + console.log( + " OpenClaw UI (tokenized URL; treat it like a password; save it now - it will not be printed again)", + ); for (const line of guidanceLines) { console.log(` ${line}`); } From 5ee32b9e23dcb5514cb1be5b04eab5ff0c82e635 Mon Sep 17 00:00:00 2001 From: Guatu <51002668+futhgar@users.noreply.github.com> Date: Wed, 22 Apr 2026 20:08:50 -0400 Subject: [PATCH 10/17] fix: offer Ollama install fallback when cloud API is unavailable (#380) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Always show an "Install Ollama" option during onboard on Linux (not just macOS), so users have a local inference fallback when `build.nvidia.com` is overloaded or down - Use the official `curl -fsSL https://ollama.com/install.sh | sh` installer on Linux (Homebrew on macOS) - Add `install-ollama` as a valid `NEMOCLAW_PROVIDER` value for non-interactive/CI mode ## Problem When Ollama is not installed and the NVIDIA API server is unavailable, `nemoclaw onboard` presents only the cloud option on Linux. Users can't get an API key and onboarding fails with no fallback path (issue #301). ## Fix The `install-ollama` option was already implemented for macOS but gated behind `process.platform === "darwin"`. This PR extends it to Linux with the appropriate installer command. The change is minimal — no restructuring of the onboard flow. Fixes #301 ## Test plan - [x] New test: verifies `Install Ollama (Linux)` option appears when Ollama is not installed on Linux - [x] New test: verifies the curl installer (not Homebrew) is invoked on Linux - [x] Existing `onboard-selection.test.js` tests still pass - [x] All onboard/credentials/inference-config tests pass - [x] `npm test` shows no new failures (pre-existing failures in `runtime-shell.test.js` and `install-preflight.test.js` are unrelated) ## Summary by CodeRabbit ## Release Notes * **New Features** * Added automatic Ollama installation option with platform-specific methods: `brew install` on macOS and curl-based installation on Linux * Enhanced provider selection with improved fallback behavior during non-interactive setup Signed-off-by: Josue Balandrano Coronel --------- Signed-off-by: Josue Gomez Signed-off-by: Julie Yaunches Co-authored-by: futhgar Co-authored-by: Test User Co-authored-by: Carlos Villela Co-authored-by: Aaron Erickson 🦞 --- src/lib/onboard.ts | 40 ++++++--- test/onboard-selection.test.ts | 157 +++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+), 11 deletions(-) diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 1bb1a5a74bc..46c1162fda7 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2217,6 +2217,7 @@ function getEffectiveProviderName(providerKey) { case "nim-local": return "nvidia-nim"; case "ollama": + case "install-ollama": return "ollama-local"; case "vllm": return "vllm-local"; @@ -2614,11 +2615,12 @@ function getNonInteractiveProvider() { "custom", "nim-local", "vllm", + "install-ollama", ]); if (!validProviders.has(normalized)) { console.error(` Unsupported NEMOCLAW_PROVIDER: ${providerKey}`); console.error( - " Valid values: build, openai, anthropic, anthropicCompatible, gemini, ollama, custom, nim-local, vllm", + " Valid values: build, openai, anthropic, anthropicCompatible, gemini, ollama, custom, nim-local, vllm, install-ollama", ); process.exit(1); } @@ -4117,9 +4119,14 @@ async function setupNim(gpu) { label: "Local vLLM [experimental] — running", }); } - // On macOS without Ollama, offer to install it - if (!hasOllama && process.platform === "darwin") { - options.push({ key: "install-ollama", label: "Install Ollama (macOS)" }); + // Without Ollama, offer to install it so users always have a local fallback + // (e.g. when the NVIDIA API server is down and cloud keys are unavailable) + if (!hasOllama && !ollamaRunning) { + if (process.platform === "darwin") { + options.push({ key: "install-ollama", label: "Install Ollama (macOS)" }); + } else if (process.platform === "linux") { + options.push({ key: "install-ollama", label: "Install Ollama (Linux)" }); + } } if (options.length > 1) { @@ -4130,10 +4137,17 @@ async function setupNim(gpu) { const providerKey = requestedProvider || "build"; selected = options.find((o) => o.key === providerKey); if (!selected) { - console.error( - ` Requested provider '${providerKey}' is not available in this environment.`, - ); - process.exit(1); + // install-ollama is valid even when Ollama is already installed — + // fall back to the existing ollama option silently + if (providerKey === "install-ollama") { + selected = options.find((o) => o.key === "ollama"); + } + if (!selected) { + console.error( + ` Requested provider '${providerKey}' is not available in this environment.`, + ); + process.exit(1); + } } note(` [non-interactive] Provider: ${selected.key}`); } else { @@ -4650,9 +4664,13 @@ async function setupNim(gpu) { } break; } else if (selected.key === "install-ollama") { - // macOS only — this option is gated by process.platform === "darwin" above - console.log(" Installing Ollama via Homebrew..."); - run(["brew", "install", "ollama"], { ignoreError: true }); + if (process.platform === "darwin") { + console.log(" Installing Ollama via Homebrew..."); + run(["brew", "install", "ollama"], { ignoreError: true }); + } else { + console.log(" Installing Ollama via official installer..."); + run("set -o pipefail; curl -fsSL https://ollama.com/install.sh | sh"); + } console.log(" Starting Ollama..."); // Shell required: backgrounding (&), env var prefix, output redirection. run(`OLLAMA_HOST=0.0.0.0:${OLLAMA_PORT} ollama serve > /dev/null 2>&1 &`, { diff --git a/test/onboard-selection.test.ts b/test/onboard-selection.test.ts index 22cf0605c05..78d0a2a4eb6 100644 --- a/test/onboard-selection.test.ts +++ b/test/onboard-selection.test.ts @@ -3130,4 +3130,161 @@ const { setupNim } = require(${onboardPath}); assert.equal(payload.result.preferredInferenceApi, "openai-completions"); assert.ok(payload.lines.some((line) => line.includes("tool-call-parser requires"))); }); + + it("offers install-ollama option on Linux when Ollama is not installed", () => { + const repoRoot = path.join(import.meta.dirname, ".."); + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-onboard-install-ollama-")); + const fakeBin = path.join(tmpDir, "bin"); + const scriptPath = path.join(tmpDir, "install-ollama-check.js"); + const onboardPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "onboard.js")); + const credentialsPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "credentials.js")); + const runnerPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "runner.js")); + const registryPath = JSON.stringify(path.join(repoRoot, "dist", "lib", "registry.js")); + + // Fake curl binary that returns a successful response — needed because + // runCurlProbe and validateOllamaModel spawn real curl via child_process. + fs.mkdirSync(fakeBin, { recursive: true }); + fs.writeFileSync( + path.join(fakeBin, "curl"), + `#!/usr/bin/env bash +body='{"id":"ok"}' +status="200" +outfile="" +while [ "$#" -gt 0 ]; do + case "$1" in + -o) outfile="$2"; shift 2 ;; + *) shift ;; + esac +done +if [ -n "$outfile" ]; then + printf '%s' "$body" > "$outfile" + printf '%s' "$status" +else + printf '%s' "$body" +fi +`, + { mode: 0o755 }, + ); + + // Simulate: no Ollama installed, no Ollama running, no vLLM — only cloud + install-ollama should appear. + // User picks install-ollama (option 7). The install command is mocked to succeed. + const script = String.raw` +const credentials = require(${credentialsPath}); +const runner = require(${runnerPath}); +const registry = require(${registryPath}); + +// Mock child_process.spawn so startOllamaAuthProxy doesn't try to spawn a real process. +const child_process = require("child_process"); +const originalSpawn = child_process.spawn; +child_process.spawn = (...args) => { + // Return a fake ChildProcess with a pid and unref() + return { pid: 99999, unref() {}, on() {} }; +}; + +// Mock spawnSync for ollama pull (real ollama is not installed) and ps checks. +const originalSpawnSync = child_process.spawnSync; +child_process.spawnSync = (cmd, args, opts) => { + const cmdStr = [cmd, ...(args || [])].join(" "); + // ollama pull — pretend it succeeds + if (cmd === "ollama" && args && args[0] === "pull") { + return { status: 0, stdout: "", stderr: "", signal: null }; + } + // ps check for isOllamaProxyProcess — pretend the proxy is running + if (cmd === "ps") { + return { status: 0, stdout: "node ollama-auth-proxy.js", stderr: "", signal: null }; + } + // Everything else (curl for probes) — use real spawnSync so fake curl binary handles it + return originalSpawnSync(cmd, args, opts); +}; + +let promptCalls = 0; +const messages = []; +const updates = []; +const runCommands = []; + +credentials.prompt = async (message) => { + promptCalls += 1; + messages.push(message); + // Select option 7 (install-ollama) on first prompt, default on model prompt + if (promptCalls === 1) return "7"; + return ""; +}; +credentials.ensureApiKey = async () => {}; +runner.runCapture = (command) => { + // Normalize: onboard.ts still sends strings, local-inference.ts sends arrays. + const cmd = Array.isArray(command) ? command.join(" ") : command; + // No ollama installed + if (cmd.includes("command -v ollama")) return ""; + // No ollama running + if (cmd.includes("127.0.0.1:11434/api/tags")) return ""; + // No vLLM running + if (cmd.includes("127.0.0.1:8000/v1/models")) return ""; + // After install, ollama list returns a model + if (cmd.includes("ollama list")) return "qwen3:8b abc 5 GB now"; + // isOllamaProxyProcess — ps check for auth proxy + if (cmd.includes("ps")) return "node ollama-auth-proxy.js"; + // validateOllamaModel probe via local-inference — return a valid JSON response + if (cmd.includes("api/generate")) return '{"response":"hello"}'; + return ""; +}; +runner.run = (command, opts) => { + runCommands.push(typeof command === "string" ? command : command.join(" ")); +}; +registry.updateSandbox = (_name, update) => updates.push(update); + +// Force platform to linux for this test +Object.defineProperty(process, 'platform', { value: 'linux' }); + +const { setupNim } = require(${onboardPath}); + +(async () => { + const originalLog = console.log; + const lines = []; + console.log = (...args) => lines.push(args.join(" ")); + try { + const result = await setupNim("install-test", null); + originalLog(JSON.stringify({ result, promptCalls, messages, updates, lines, runCommands })); + } finally { + console.log = originalLog; + } +})().catch((error) => { + console.error(error); + process.exit(1); +}); +`; + fs.writeFileSync(scriptPath, script); + + const result = spawnSync(process.execPath, [scriptPath], { + cwd: repoRoot, + encoding: "utf-8", + env: { + ...process.env, + HOME: tmpDir, + PATH: `${fakeBin}:${process.env.PATH || ""}`, + }, + }); + + assert.equal(result.status, 0, `Process failed: ${result.stderr}`); + assert.notEqual(result.stdout.trim(), "", result.stderr); + const payload = JSON.parse(result.stdout.trim()); + + // Should have shown the "Install Ollama (Linux)" option + assert.ok( + payload.lines.some((line: string) => line.includes("Install Ollama (Linux)")), + "Should show Install Ollama option on Linux" + ); + + // Should have selected ollama-local provider after install + assert.equal(payload.result.provider, "ollama-local"); + + // Should have run the curl installer (not brew) + assert.ok( + payload.runCommands.some((cmd: string) => cmd.includes("ollama.com/install.sh")), + "Should use curl installer on Linux" + ); + assert.ok( + !payload.runCommands.some((cmd: string) => cmd.includes("brew install")), + "Should NOT use brew on Linux" + ); + }); }); From 1b45c2a62e66b892d124b5d0191f75af38567400 Mon Sep 17 00:00:00 2001 From: hunglp6d <89095484+hunglp6d@users.noreply.github.com> Date: Thu, 23 Apr 2026 07:19:39 +0700 Subject: [PATCH 11/17] test(e2e): skip cleanly under VPN, cover Discord token rotation (#2257) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Extends `test/e2e/test-token-rotation.sh` to cover Discord rotation alongside Telegram (with provider-isolation checks), and makes the test resilient to environmental install failures so corporate-VPN runs SKIP cleanly instead of exiting 1 mid-Phase 0. ## Related Issue Fixes #2247 Closes #2255 ## Changes **Test script (`test/e2e/test-token-rotation.sh`)** - Phase 0 now seeds both `TELEGRAM_BOT_TOKEN` and `DISCORD_BOT_TOKEN`; Phase 1 verifies both providers and both credential hashes are stored. - Add Phase 4 (Discord rotation) and Phase 5 (re-onboard with same tokens) mirroring the existing Telegram phases. - Phase 2 (Telegram) and Phase 4 (Discord) each get a Negative provider-isolation check — the rotation message must NOT name the provider whose token didn't change. - Add `SKIP` counter and `skip()` helper. When `install.sh` fails with `(Telegram|Discord) network reachability failure` in the install log (typical of VPN/proxy blocking `api.telegram.org`), record a SKIP, mark Phases 1–5 as skipped, and still print the Summary instead of exiting 1. - Per-phase onboard failures no longer hard-exit — Summary always runs. - Add Discord prereq guards (`DISCORD_BOT_TOKEN_A`/`_B` set, A ≠ B); SKIP cleanly if either is missing. **CI (`.github/workflows/nightly-e2e.yaml`)** - Wire `DISCORD_BOT_TOKEN_A` / `DISCORD_BOT_TOKEN_B` into the `token-rotation-e2e` job so the new prereq guards do not skip the test under nightly cron. - Refresh the job comment to mention the combined Telegram + Discord coverage. ## Type of Change - [x] Code change (feature, bug fix, or refactor) ## Verification - [ ] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [ ] No secrets, API keys, or credentials committed --- Signed-off-by: Hung Le ## Summary by CodeRabbit * **Documentation** * Clarified test description to state per-provider (Telegram + Discord) fake tokens and cross-talk assertions. * **Tests** * Extended end-to-end token-rotation tests to include Discord alongside Telegram and added corresponding environment variables. * Added prereq gating, skip counting and summary reporting for missing tokens, install/network failures, or identical tokens. * Strengthened post-install verification for both provider bridges and credential registration. * Implemented provider-isolated rotation phases and reuse assertions for sandbox onboarding. --- .github/workflows/nightly-e2e.yaml | 8 +- test/e2e/test-token-rotation.sh | 380 +++++++++++++++++++++-------- 2 files changed, 281 insertions(+), 107 deletions(-) diff --git a/.github/workflows/nightly-e2e.yaml b/.github/workflows/nightly-e2e.yaml index e9fb6e303d5..b8a0289587c 100644 --- a/.github/workflows/nightly-e2e.yaml +++ b/.github/workflows/nightly-e2e.yaml @@ -10,7 +10,8 @@ # messaging-providers-e2e Validates messaging credential provider/placeholder/L7-proxy chain # for Telegram + Discord. Uses fake tokens. See PR #1081. # token-rotation-e2e Validates that rotating a messaging token and re-running onboard -# propagates the new credential to the sandbox. See issue #1903. +# propagates the new credential to the sandbox. Combined Telegram + +# Discord coverage with cross-talk assertions. See issue #1903. # sandbox-survival-e2e Sandbox survival across gateway restarts (onboard, inference, # gateway stop/start, verify sandbox + workspace + inference). # hermes-e2e Hermes Agent E2E — install → onboard --agent hermes → health @@ -208,7 +209,8 @@ jobs: # ── Token rotation (credential propagation to L7 proxy) ───── # Validates that rotating a messaging token and re-running onboard # propagates the new credential to the sandbox. Uses two fake tokens - # to prove the sandbox is rebuilt on rotation and reused when unchanged. + # per provider (Telegram + Discord) to prove the sandbox is rebuilt on + # rotation and reused when unchanged. # See: issue #1903 token-rotation-e2e: if: github.repository == 'NVIDIA/NemoClaw' @@ -227,6 +229,8 @@ jobs: GITHUB_TOKEN: ${{ github.token }} TELEGRAM_BOT_TOKEN_A: "test-fake-token-A-rotation-e2e" TELEGRAM_BOT_TOKEN_B: "test-fake-token-B-rotation-e2e" + DISCORD_BOT_TOKEN_A: "test-fake-discord-A-rotation-e2e" + DISCORD_BOT_TOKEN_B: "test-fake-discord-B-rotation-e2e" run: bash test/e2e/test-token-rotation.sh - name: Upload install log on failure diff --git a/test/e2e/test-token-rotation.sh b/test/e2e/test-token-rotation.sh index daca2ed1b7b..9f54713c9f9 100755 --- a/test/e2e/test-token-rotation.sh +++ b/test/e2e/test-token-rotation.sh @@ -6,19 +6,24 @@ # - prove that rotating a messaging token and re-running onboard propagates # the new credential to the sandbox (sandbox is rebuilt automatically) # - prove that re-running onboard with the same token reuses the sandbox +# - prove that rotating each provider in isolation only re-builds for that +# provider's bridge (no cross-talk between Telegram and Discord detection) # -# Uses two distinct fake tokens. The test validates that NemoClaw detects the -# rotation and triggers a sandbox rebuild, not the Telegram API response. +# Uses two distinct fake tokens per provider. The test validates that NemoClaw +# detects the rotation and triggers a sandbox rebuild — it does not validate +# the Telegram or Discord API responses. # # Prerequisites: # - Docker running # - NVIDIA_API_KEY set (or fake OpenAI endpoint) # - TELEGRAM_BOT_TOKEN_A and TELEGRAM_BOT_TOKEN_B set (can be fake) +# - DISCORD_BOT_TOKEN_A and DISCORD_BOT_TOKEN_B set (can be fake) # # Usage: # NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 \ # NVIDIA_API_KEY=nvapi-... \ # TELEGRAM_BOT_TOKEN_A=fake-a TELEGRAM_BOT_TOKEN_B=fake-b \ +# DISCORD_BOT_TOKEN_A=fake-c DISCORD_BOT_TOKEN_B=fake-d \ # bash test/e2e/test-token-rotation.sh set -uo pipefail @@ -31,7 +36,10 @@ fi PASS=0 FAIL=0 +SKIP=0 TOTAL=0 +INSTALL_OK=1 +PREREQS_OK=1 pass() { ((PASS++)) @@ -43,11 +51,31 @@ fail() { ((TOTAL++)) printf '\033[31m FAIL: %s\033[0m\n' "$1" } +skip() { + ((SKIP++)) + ((TOTAL++)) + printf '\033[33m SKIP: %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"; } +print_summary() { + section "Summary" + echo " Total: $TOTAL Pass: $PASS Fail: $FAIL Skip: $SKIP" + if [ "$FAIL" -gt 0 ]; then + echo "" + echo "FAILED" + exit 1 + fi + echo "" + if [ "$SKIP" -gt 0 ]; then + echo "PASSED (with $SKIP skipped)" + else + echo "ALL PASSED" + fi +} # Determine repo root if [ -d /workspace ] && [ -f /workspace/install.sh ]; then @@ -66,12 +94,28 @@ INSTALL_LOG="/tmp/nemoclaw-e2e-install.log" # ── Prerequisite checks ────────────────────────────────────────── if [ -z "${TELEGRAM_BOT_TOKEN_A:-}" ] || [ -z "${TELEGRAM_BOT_TOKEN_B:-}" ]; then - echo "SKIP: TELEGRAM_BOT_TOKEN_A and TELEGRAM_BOT_TOKEN_B must both be set" - exit 0 + skip "TELEGRAM_BOT_TOKEN_A and TELEGRAM_BOT_TOKEN_B must both be set" + PREREQS_OK=0 +fi + +if [ -z "${DISCORD_BOT_TOKEN_A:-}" ] || [ -z "${DISCORD_BOT_TOKEN_B:-}" ]; then + skip "DISCORD_BOT_TOKEN_A and DISCORD_BOT_TOKEN_B must both be set" + PREREQS_OK=0 fi -if [ "$TELEGRAM_BOT_TOKEN_A" = "$TELEGRAM_BOT_TOKEN_B" ]; then - echo "SKIP: TELEGRAM_BOT_TOKEN_A and TELEGRAM_BOT_TOKEN_B must be different" +if [ -n "${TELEGRAM_BOT_TOKEN_A:-}" ] && [ "${TELEGRAM_BOT_TOKEN_A}" = "${TELEGRAM_BOT_TOKEN_B:-}" ]; then + skip "TELEGRAM_BOT_TOKEN_A and TELEGRAM_BOT_TOKEN_B must be different" + PREREQS_OK=0 +fi + +if [ -n "${DISCORD_BOT_TOKEN_A:-}" ] && [ "${DISCORD_BOT_TOKEN_A}" = "${DISCORD_BOT_TOKEN_B:-}" ]; then + skip "DISCORD_BOT_TOKEN_A and DISCORD_BOT_TOKEN_B must be different" + PREREQS_OK=0 +fi + +# Bail to summary if any prereq failed (no phases run, but Summary still prints) +if [ "$PREREQS_OK" != "1" ]; then + print_summary exit 0 fi @@ -91,6 +135,10 @@ openshell sandbox delete "$SANDBOX_NAME" 2>/dev/null || true openshell gateway destroy -g nemoclaw 2>/dev/null || true export TELEGRAM_BOT_TOKEN="$TELEGRAM_BOT_TOKEN_A" +export DISCORD_BOT_TOKEN="$DISCORD_BOT_TOKEN_A" +# Determinism: clear ambient SLACK_* so onboard doesn't add an extra +# slack-bridge provider (messagingTokenDefs at onboard.ts). +unset SLACK_BOT_TOKEN SLACK_APP_TOKEN export NEMOCLAW_SANDBOX_NAME="$SANDBOX_NAME" export NEMOCLAW_POLICY_TIER="open" export NEMOCLAW_RECREATE_SANDBOX=1 @@ -124,119 +172,241 @@ fi if [ $install_exit -eq 0 ]; then pass "install.sh completed (exit 0)" else - fail "install.sh failed (exit $install_exit)" + INSTALL_OK=0 + if grep -qE "(Telegram|Discord) network reachability failure" "$INSTALL_LOG" 2>/dev/null; then + skip "install.sh aborted: messaging API unreachable (likely VPN / corporate proxy)" + info "Detected ' network reachability failure' in install log." + else + fail "install.sh failed (exit $install_exit)" + fi info "Last 30 lines of install log:" tail -30 "$INSTALL_LOG" 2>/dev/null || true - exit 1 fi # Verify tools are on PATH -if ! command -v openshell >/dev/null 2>&1; then - fail "openshell not found on PATH after install" - exit 1 +if [ "$INSTALL_OK" = "1" ]; then + if ! command -v openshell >/dev/null 2>&1; then + fail "openshell not found on PATH after install" + exit 1 + fi + pass "openshell installed ($(openshell --version 2>&1 || echo unknown))" + + if ! command -v nemoclaw >/dev/null 2>&1; then + fail "nemoclaw not found on PATH after install" + exit 1 + fi + pass "nemoclaw installed at $(command -v nemoclaw)" fi -pass "openshell installed ($(openshell --version 2>&1 || echo unknown))" -if ! command -v nemoclaw >/dev/null 2>&1; then - fail "nemoclaw not found on PATH after install" - exit 1 -fi -pass "nemoclaw installed at $(command -v nemoclaw)" - -# ── Phase 1: Verify first onboard with token A ────────────────── - -section "Phase 1: Verify first onboard results" - -if openshell sandbox list 2>/dev/null | grep -q "$SANDBOX_NAME"; then - pass "Sandbox $SANDBOX_NAME created and running" -else - fail "Sandbox $SANDBOX_NAME not running after first onboard" -fi - -if openshell provider get "${SANDBOX_NAME}-telegram-bridge" >/dev/null 2>&1; then - pass "Provider ${SANDBOX_NAME}-telegram-bridge exists" +if [ "$INSTALL_OK" != "1" ]; then + section "Skipping verification phases — initial install did not complete" + skip "Phase 1: Verify first onboard results" + skip "Phase 2: Re-onboard with rotated TELEGRAM_BOT_TOKEN_B" + skip "Phase 3: Re-onboard with same tokens (after Telegram rotation)" + skip "Phase 4: Re-onboard with rotated DISCORD_BOT_TOKEN_B" + skip "Phase 5: Re-onboard with same tokens (after Discord rotation)" else - fail "Provider ${SANDBOX_NAME}-telegram-bridge not found" -fi - -# Verify credential hashes are stored for this sandbox in the registry -if [ -f "$REGISTRY" ] && node -e " + # ── Phase 1: Verify first onboard with token A ────────────────── + + section "Phase 1: Verify first onboard results" + + if openshell sandbox list 2>/dev/null | grep -q "$SANDBOX_NAME"; then + pass "Sandbox $SANDBOX_NAME created and running" + else + fail "Sandbox $SANDBOX_NAME not running after first onboard" + fi + + if openshell provider get "${SANDBOX_NAME}-telegram-bridge" >/dev/null 2>&1; then + pass "Provider ${SANDBOX_NAME}-telegram-bridge exists" + else + fail "Provider ${SANDBOX_NAME}-telegram-bridge not found" + fi + + if openshell provider get "${SANDBOX_NAME}-discord-bridge" >/dev/null 2>&1; then + pass "Provider ${SANDBOX_NAME}-discord-bridge exists" + else + fail "Provider ${SANDBOX_NAME}-discord-bridge not found" + fi + + # Verify credential hashes are stored for this sandbox in the registry + if [ -f "$REGISTRY" ] && node -e " const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); const h = (r.sandboxes || {})[process.argv[2]]?.providerCredentialHashes || {}; process.exit('TELEGRAM_BOT_TOKEN' in h ? 0 : 1); " "$REGISTRY" "$SANDBOX_NAME" 2>/dev/null; then - pass "Credential hash stored for $SANDBOX_NAME" -else - fail "Credential hash not found for $SANDBOX_NAME in registry" -fi - -# ── Phase 2: Rotate token (re-onboard with token B) ────────────── - -section "Phase 2: Re-onboard with rotated TELEGRAM_BOT_TOKEN_B" + pass "Telegram credential hash stored for $SANDBOX_NAME" + else + fail "Telegram credential hash not found for $SANDBOX_NAME in registry" + fi -export TELEGRAM_BOT_TOKEN="$TELEGRAM_BOT_TOKEN_B" -unset NEMOCLAW_RECREATE_SANDBOX - -ONBOARD_OUTPUT=$(nemoclaw onboard --non-interactive 2>&1) -onboard_exit=$? - -if [ $onboard_exit -ne 0 ]; then - fail "Phase 2 onboard failed (exit $onboard_exit)" - echo "$ONBOARD_OUTPUT" | tail -30 - exit 1 -fi - -if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated"; then - pass "Credential rotation detected" -else - fail "Credential rotation not detected in onboard output" - info "Onboard output:" - echo "$ONBOARD_OUTPUT" | tail -20 -fi - -if echo "$ONBOARD_OUTPUT" | grep -q "Rebuilding sandbox"; then - pass "Sandbox rebuild triggered by rotation" -else - fail "Sandbox rebuild not triggered" - info "Onboard output:" - echo "$ONBOARD_OUTPUT" | tail -20 -fi - -if openshell sandbox list 2>/dev/null | grep -q "$SANDBOX_NAME"; then - pass "Sandbox running after rotation" -else - fail "Sandbox not running after rotation" -fi - -# ── Phase 3: Re-onboard with same token B (no change) ──────────── - -section "Phase 3: Re-onboard with same token (no rotation expected)" - -ONBOARD_OUTPUT=$(nemoclaw onboard --non-interactive 2>&1) -onboard_exit=$? - -if [ $onboard_exit -ne 0 ]; then - fail "Phase 3 onboard failed (exit $onboard_exit)" - echo "$ONBOARD_OUTPUT" | tail -30 - exit 1 -fi - -if echo "$ONBOARD_OUTPUT" | grep -q "reusing it"; then - pass "Sandbox reused when token unchanged" -else - fail "Sandbox was not reused (unexpected rebuild)" - info "Onboard output:" - echo "$ONBOARD_OUTPUT" | tail -20 + if [ -f "$REGISTRY" ] && node -e " +const r = JSON.parse(require('fs').readFileSync(process.argv[1], 'utf8')); +const h = (r.sandboxes || {})[process.argv[2]]?.providerCredentialHashes || {}; +process.exit('DISCORD_BOT_TOKEN' in h ? 0 : 1); +" "$REGISTRY" "$SANDBOX_NAME" 2>/dev/null; then + pass "Discord credential hash stored for $SANDBOX_NAME" + else + fail "Discord credential hash not found for $SANDBOX_NAME in registry" + fi + + # ── Phase 2: Rotate Telegram token only (re-onboard with token B) ─ + + section "Phase 2: Re-onboard with rotated TELEGRAM_BOT_TOKEN_B (Discord unchanged)" + + export TELEGRAM_BOT_TOKEN="$TELEGRAM_BOT_TOKEN_B" + export DISCORD_BOT_TOKEN="$DISCORD_BOT_TOKEN_A" + # Determinism: clear ambient SLACK_* so onboard doesn't add an extra + # slack-bridge provider (messagingTokenDefs at onboard.ts). + unset SLACK_BOT_TOKEN SLACK_APP_TOKEN + unset NEMOCLAW_RECREATE_SANDBOX + + ONBOARD_OUTPUT=$(nemoclaw onboard --non-interactive 2>&1) + onboard_exit=$? + + if [ $onboard_exit -ne 0 ]; then + fail "Phase 2 onboard failed (exit $onboard_exit)" + echo "$ONBOARD_OUTPUT" | tail -30 + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated"; then + pass "Credential rotation detected" + else + fail "Credential rotation not detected in onboard output" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | tail -20 + fi + + # Rotation message must name only the telegram-bridge provider — Discord + # token is unchanged, so a stray discord-bridge entry would indicate a + # false-positive in detectMessagingCredentialRotation. + if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated:.*telegram-bridge"; then + pass "Rotation message identifies telegram-bridge" + else + fail "Rotation message did not identify telegram-bridge" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | grep "credential(s) rotated" || true + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated:.*discord-bridge"; then + fail "Rotation message unexpectedly named discord-bridge (Discord token did not change)" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | grep "credential(s) rotated" || true + else + pass "Rotation message did not name discord-bridge (Discord unchanged)" + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "Rebuilding sandbox"; then + pass "Sandbox rebuild triggered by rotation" + else + fail "Sandbox rebuild not triggered" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | tail -20 + fi + + if openshell sandbox list 2>/dev/null | grep -q "$SANDBOX_NAME"; then + pass "Sandbox running after Telegram rotation" + else + fail "Sandbox not running after Telegram rotation" + fi + + # ── Phase 3: Re-onboard with same tokens (no change) ───────────── + + section "Phase 3: Re-onboard with same tokens (no rotation expected)" + + ONBOARD_OUTPUT=$(nemoclaw onboard --non-interactive 2>&1) + onboard_exit=$? + + if [ $onboard_exit -ne 0 ]; then + fail "Phase 3 onboard failed (exit $onboard_exit)" + echo "$ONBOARD_OUTPUT" | tail -30 + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "reusing it"; then + pass "Sandbox reused when tokens unchanged" + else + fail "Sandbox was not reused (unexpected rebuild)" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | tail -20 + fi + + # ── Phase 4: Rotate Discord token only (re-onboard with token B) ─ + + section "Phase 4: Re-onboard with rotated DISCORD_BOT_TOKEN_B (Telegram unchanged)" + + export TELEGRAM_BOT_TOKEN="$TELEGRAM_BOT_TOKEN_B" + export DISCORD_BOT_TOKEN="$DISCORD_BOT_TOKEN_B" + # Determinism: clear ambient SLACK_* so onboard doesn't add an extra + # slack-bridge provider (messagingTokenDefs at onboard.ts). + unset SLACK_BOT_TOKEN SLACK_APP_TOKEN + + ONBOARD_OUTPUT=$(nemoclaw onboard --non-interactive 2>&1) + onboard_exit=$? + + if [ $onboard_exit -ne 0 ]; then + fail "Phase 4 onboard failed (exit $onboard_exit)" + echo "$ONBOARD_OUTPUT" | tail -30 + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated"; then + pass "Credential rotation detected" + else + fail "Credential rotation not detected in onboard output" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | tail -20 + fi + + # Symmetric assertion to Phase 2: only the discord-bridge entry should appear. + if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated:.*discord-bridge"; then + pass "Rotation message identifies discord-bridge" + else + fail "Rotation message did not identify discord-bridge" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | grep "credential(s) rotated" || true + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "credential(s) rotated:.*telegram-bridge"; then + fail "Rotation message unexpectedly named telegram-bridge (Telegram token did not change)" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | grep "credential(s) rotated" || true + else + pass "Rotation message did not name telegram-bridge (Telegram unchanged)" + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "Rebuilding sandbox"; then + pass "Sandbox rebuild triggered by rotation" + else + fail "Sandbox rebuild not triggered" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | tail -20 + fi + + if openshell sandbox list 2>/dev/null | grep -q "$SANDBOX_NAME"; then + pass "Sandbox running after Discord rotation" + else + fail "Sandbox not running after Discord rotation" + fi + + # ── Phase 5: Re-onboard with same tokens (no change) ───────────── + + section "Phase 5: Re-onboard with same tokens (no rotation expected)" + + ONBOARD_OUTPUT=$(nemoclaw onboard --non-interactive 2>&1) + onboard_exit=$? + + if [ $onboard_exit -ne 0 ]; then + fail "Phase 5 onboard failed (exit $onboard_exit)" + echo "$ONBOARD_OUTPUT" | tail -30 + fi + + if echo "$ONBOARD_OUTPUT" | grep -q "reusing it"; then + pass "Sandbox reused when tokens unchanged" + else + fail "Sandbox was not reused (unexpected rebuild)" + info "Onboard output:" + echo "$ONBOARD_OUTPUT" | tail -20 + fi fi # ── Summary ─────────────────────────────────────────────────────── -section "Summary" -echo " Total: $TOTAL Pass: $PASS Fail: $FAIL" -if [ "$FAIL" -gt 0 ]; then - echo "" - echo "FAILED" - exit 1 -fi -echo "" -echo "ALL PASSED" +print_summary From fafbaecd69a0588c8907370a66132a2faaab9842 Mon Sep 17 00:00:00 2001 From: Prekshi Vyas <34834085+prekshivyas@users.noreply.github.com> Date: Wed, 22 Apr 2026 17:21:57 -0700 Subject: [PATCH 12/17] chore(install): bump OpenShell version to 0.0.32 (#2307) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Bumps the pinned OpenShell version range from `0.0.29` → `0.0.32` so fresh NemoClaw installs pick up sandbox hardening and TLS improvements from the last three OpenShell releases. ## Notable upstream changes **0.0.30** ([NVIDIA/OpenShell@v0.0.29...v0.0.30](https://github.com/NVIDIA/OpenShell/compare/v0.0.29...v0.0.30)) - Network policy deny rules ([OpenShell#822](https://github.com/NVIDIA/OpenShell/pull/822)) - Preserve ownership on existing `read_write` paths ([OpenShell#827](https://github.com/NVIDIA/OpenShell/pull/827)) - Disable child core dumps ([OpenShell#821](https://github.com/NVIDIA/OpenShell/pull/821)) - Escape control characters in SSE error formatting ([OpenShell#842](https://github.com/NVIDIA/OpenShell/pull/842)) - Fix silent truncation of large streaming inference responses ([OpenShell#834](https://github.com/NVIDIA/OpenShell/pull/834)) **0.0.31** ([NVIDIA/OpenShell@v0.0.30...v0.0.31](https://github.com/NVIDIA/OpenShell/compare/v0.0.30...v0.0.31)) - Inference routed-request header allowlist ([OpenShell#826](https://github.com/NVIDIA/OpenShell/pull/826)) **0.0.32** ([NVIDIA/OpenShell@v0.0.31...v0.0.32](https://github.com/NVIDIA/OpenShell/compare/v0.0.31...v0.0.32)) - **Load system CA certificates for upstream TLS connections** ([OpenShell#862](https://github.com/NVIDIA/OpenShell/pull/862)) - Publish standalone `openshell-gateway` binaries ([OpenShell#853](https://github.com/NVIDIA/OpenShell/pull/853)) ## Changes - `nemoclaw-blueprint/blueprint.yaml`: `min_openshell_version` and `max_openshell_version` → `0.0.32` - `scripts/install-openshell.sh`: `MIN_VERSION` and `MAX_VERSION` → `0.0.32` (`PIN_VERSION` follows `MAX`) - `scripts/brev-launchable-ci-cpu.sh`: default `OPENSHELL_VERSION` → `v0.0.32` - `src/lib/onboard.ts`: blueprint-fallback min version → `0.0.32` - `test/onboard.test.ts`, `test/install-openshell-version-check.test.ts`: fixtures updated; "above MAX" test case moved from `0.0.30` to `0.0.33` Historical `m-dev` comments referencing `0.0.29` left in place — they describe a self-report quirk the sidecar fallback still handles. ## Why not 0.0.33+? `0.0.34` introduced incremental sandbox policy updates and L7 request-target canonicalization — changes with larger surface area against how NemoClaw delivers policy via gRPC. Worth a follow-up PR rather than bundling here. `0.0.35` released hours before this PR was cut — too fresh. ## Type of Change - [x] Code change for a new feature, bug fix, or refactor. ## Testing - [x] `npx vitest run test/install-openshell-version-check.test.ts` — 9 passed - [x] pre-commit hooks (prek) clean: shellcheck, commitlint, gitleaks, YAML validator, CLI test suite - [ ] Nightly E2E on this branch — will be kicked off after PR opens ## Notes - No user-facing CLI behavior changes — just the pinned version range. - Two pre-existing failures in `test/onboard.test.ts` reproduce on clean `main` and are unrelated to this bump. Signed-off-by: Prekshi Vyas 🤖 Generated with [Claude Code](https://claude.com/claude-code) ## Summary by CodeRabbit * **Chores** * Updated OpenShell version constraints and default pinned version to v0.0.32 across configuration, install, and onboarding flows. * **Tests** * Updated test fixtures and expectations to match the new OpenShell version (v0.0.32). Signed-off-by: Prekshi Vyas Co-authored-by: Claude Opus 4.7 (1M context) --- nemoclaw-blueprint/blueprint.yaml | 4 ++-- scripts/brev-launchable-ci-cpu.sh | 4 ++-- scripts/install-openshell.sh | 4 ++-- src/lib/onboard.ts | 2 +- test/install-openshell-version-check.test.ts | 16 ++++++++-------- test/onboard.test.ts | 6 +++--- 6 files changed, 18 insertions(+), 18 deletions(-) diff --git a/nemoclaw-blueprint/blueprint.yaml b/nemoclaw-blueprint/blueprint.yaml index ae746e80377..d220b763026 100644 --- a/nemoclaw-blueprint/blueprint.yaml +++ b/nemoclaw-blueprint/blueprint.yaml @@ -2,8 +2,8 @@ # SPDX-License-Identifier: Apache-2.0 version: "0.1.0" -min_openshell_version: "0.0.29" -max_openshell_version: "0.0.29" +min_openshell_version: "0.0.32" +max_openshell_version: "0.0.32" min_openclaw_version: "2026.4.2" # Mirrors the components.sandbox.image manifest digest below. Lets a # downstream consumer (or release tooling) verify the blueprint declares diff --git a/scripts/brev-launchable-ci-cpu.sh b/scripts/brev-launchable-ci-cpu.sh index 63be9e15f65..7f3042b063c 100755 --- a/scripts/brev-launchable-ci-cpu.sh +++ b/scripts/brev-launchable-ci-cpu.sh @@ -28,7 +28,7 @@ # curl -fsSL https://raw.githubusercontent.com/NVIDIA/NemoClaw//scripts/brev-launchable-ci-cpu.sh | bash # # Environment overrides: -# OPENSHELL_VERSION — OpenShell CLI release tag (default: v0.0.29) +# OPENSHELL_VERSION — OpenShell CLI release tag (default: v0.0.32) # NEMOCLAW_REF — NemoClaw git ref to clone (default: main) # NEMOCLAW_CLONE_DIR — Where to clone NemoClaw (default: ~/NemoClaw) # SKIP_DOCKER_PULL — Set to 1 to skip Docker image pre-pulls @@ -40,7 +40,7 @@ set -euo pipefail # ── Configuration ──────────────────────────────────────────────────── -OPENSHELL_VERSION="${OPENSHELL_VERSION:-v0.0.29}" +OPENSHELL_VERSION="${OPENSHELL_VERSION:-v0.0.32}" NEMOCLAW_REF="${NEMOCLAW_REF:-main}" TARGET_USER="${SUDO_USER:-$(id -un)}" TARGET_HOME="$(getent passwd "$TARGET_USER" | cut -d: -f6)" diff --git a/scripts/install-openshell.sh b/scripts/install-openshell.sh index 913dc2cc234..ab10907e885 100755 --- a/scripts/install-openshell.sh +++ b/scripts/install-openshell.sh @@ -36,10 +36,10 @@ info "Detected $OS_LABEL ($ARCH_LABEL)" # Minimum version required for Landlock filesystem policy enforcement # (NVIDIA/OpenShell#810 fixes the drop_privileges/Landlock ordering bug # that caused /sandbox to remain writable on 0.0.26). -MIN_VERSION="0.0.29" +MIN_VERSION="0.0.32" # Maximum version validated for this NemoClaw release. Newer OpenShell builds # may change sandbox semantics; upgrade NemoClaw before upgrading past this. -MAX_VERSION="0.0.29" +MAX_VERSION="0.0.32" # Pin fresh installs to this version instead of pulling "latest". PIN_VERSION="$MAX_VERSION" diff --git a/src/lib/onboard.ts b/src/lib/onboard.ts index 46c1162fda7..61b5a0772cd 100644 --- a/src/lib/onboard.ts +++ b/src/lib/onboard.ts @@ -2691,7 +2691,7 @@ async function preflight() { // Source of truth: min_openshell_version in nemoclaw-blueprint/blueprint.yaml. // Fall back to the Landlock-enforcement floor (also MIN_VERSION in // scripts/install-openshell.sh) if the blueprint cannot be read. - const minOpenshellVersion = getBlueprintMinOpenshellVersion() ?? "0.0.29"; + const minOpenshellVersion = getBlueprintMinOpenshellVersion() ?? "0.0.32"; const needsUpgrade = !versionGte(currentVersion, minOpenshellVersion); if (needsUpgrade) { console.log( diff --git a/test/install-openshell-version-check.test.ts b/test/install-openshell-version-check.test.ts index 2821c06a81b..bddf131f6af 100644 --- a/test/install-openshell-version-check.test.ts +++ b/test/install-openshell-version-check.test.ts @@ -59,10 +59,10 @@ exit 1`, } describe("install-openshell.sh version check", () => { - it("exits cleanly when openshell 0.0.29 is already installed", () => { - const result = runWithInstalledVersion("0.0.29"); + it("exits cleanly when openshell 0.0.32 is already installed", () => { + const result = runWithInstalledVersion("0.0.32"); expect(result.status).toBe(0); - expect(result.stdout).toMatch(/already installed.*0\.0\.29/); + expect(result.stdout).toMatch(/already installed.*0\.0\.32/); }); it("triggers upgrade when openshell 0.0.28 is installed (below MIN_VERSION)", () => { @@ -85,7 +85,7 @@ describe("install-openshell.sh version check", () => { }); it("fails with a clear error when openshell is above MAX_VERSION", () => { - const result = runWithInstalledVersion("0.0.30"); + const result = runWithInstalledVersion("0.0.33"); expect(result.status).toBe(1); expect(result.stdout).toMatch(/above the maximum/); }); @@ -96,13 +96,13 @@ describe("install-openshell.sh version check", () => { expect(result.stdout).toMatch(/above the maximum/); }); - it("exits cleanly when openshell reports m-dev but sidecar records 0.0.29", () => { + it("exits cleanly when openshell reports m-dev but sidecar records 0.0.32", () => { const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-openshell-mdev-")); try { const fakeBin = path.join(tmp, "bin"); fs.mkdirSync(fakeBin); - // Fake openshell that reports "m-dev" (as openshell 0.0.29 does in practice) + // Fake openshell that reports "m-dev" (as some OpenShell dev builds do) writeExecutable( path.join(fakeBin, "openshell"), `#!/usr/bin/env bash @@ -111,7 +111,7 @@ exit 99`, ); // Sidecar file written by a previous install - fs.writeFileSync(path.join(fakeBin, ".openshell-installed-version"), "0.0.29\n"); + fs.writeFileSync(path.join(fakeBin, ".openshell-installed-version"), "0.0.32\n"); writeExecutable(path.join(fakeBin, "curl"), `#!/usr/bin/env bash\nexit 1`); writeExecutable(path.join(fakeBin, "gh"), `#!/usr/bin/env bash\nexit 1`); @@ -121,7 +121,7 @@ exit 99`, encoding: "utf8", }); expect(result.status).toBe(0); - expect(result.stdout).toMatch(/already installed.*0\.0\.29/); + expect(result.stdout).toMatch(/already installed.*0\.0\.32/); } finally { fs.rmSync(tmp, { recursive: true, force: true }); } diff --git a/test/onboard.test.ts b/test/onboard.test.ts index b46b18c791e..9387151bc3c 100644 --- a/test/onboard.test.ts +++ b/test/onboard.test.ts @@ -929,13 +929,13 @@ describe("onboard helpers", () => { path.join(blueprintDir, "blueprint.yaml"), [ 'version: "0.1.0"', - 'min_openshell_version: "0.0.29"', - 'max_openshell_version: "0.0.29"', + 'min_openshell_version: "0.0.32"', + 'max_openshell_version: "0.0.32"', 'min_openclaw_version: "2026.3.0"', ].join("\n"), ); try { - expect(getBlueprintMaxOpenshellVersion(tmpDir)).toBe("0.0.29"); + expect(getBlueprintMaxOpenshellVersion(tmpDir)).toBe("0.0.32"); } finally { fs.rmSync(tmpDir, { recursive: true, force: true }); } From 5fec344907249307c0b220f748e216e369750ecc Mon Sep 17 00:00:00 2001 From: Carlos Villela Date: Wed, 22 Apr 2026 17:24:43 -0700 Subject: [PATCH 13/17] feat(skills): add title tag normalization maintainer skill (#2292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Add a maintainer skill for normalizing bracketed `NemoClaw` tags in issue and PR titles. The skill provides a dry-run workflow, a reusable TypeScript helper that matches tags case-insensitively anywhere in the title, and verification guidance so maintainers can apply the cleanup safely. ## Changes - Add `.agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.md` with a dry-run-first workflow for previewing, applying, and verifying title tag cleanup - Add `.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts` to find and remove bracketed `nemoclaw` tags case-insensitively from GitHub issue and PR titles - Update `.agents/skills/nemoclaw-skills-guide/SKILL.md` to include the new maintainer skill in the catalog ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [ ] Docs updated for user-facing behavior changes - [ ] `make docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) ## AI Disclosure - [x] AI-assisted — tool: OpenAI Codex --- Signed-off-by: Carlos Villela ## Summary by CodeRabbit * **New Features** * Added a new maintainer skill to preview and bulk remove bracketed title tags containing "NemoClaw" from GitHub issues and pull requests, with dry-run preview and optional apply execution modes. * **Documentation** * Added comprehensive documentation for the new title-tag normalization skill with step-by-step usage instructions and behavioral guidelines. Signed-off-by: Carlos Villela Co-authored-by: Aaron Erickson 🦞 --- .../SKILL.md | 91 ++++++ .../scripts/normalize-title-tags.ts | 297 ++++++++++++++++++ .agents/skills/nemoclaw-skills-guide/SKILL.md | 7 +- 3 files changed, 392 insertions(+), 3 deletions(-) create mode 100644 .agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.md create mode 100644 .agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts diff --git a/.agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.md b/.agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.md new file mode 100644 index 00000000000..576240f04f9 --- /dev/null +++ b/.agents/skills/nemoclaw-maintainer-normalize-title-tags/SKILL.md @@ -0,0 +1,91 @@ +--- +name: nemoclaw-maintainer-normalize-title-tags +description: Normalizes GitHub issue and PR titles by removing any bracketed [NemoClaw] tag case-insensitively, even when the tag appears later in the title. Use when cleaning issue tags, bulk-renaming titles, or normalizing repo title hygiene. +user_invocable: true +--- + + + + +# NemoClaw Maintainer — Normalize Title Tags + +Preview and optionally apply bulk title cleanup for bracketed NemoClaw tags in GitHub issue and PR titles. + +## Examples + +- `[NemoClaw][All Platforms] local-inference policy preset missing Ollama ports` → `[All Platforms] local-inference policy preset missing Ollama ports` +- `[Bug] [Nemoclaw] [Slack] Slack configuration in Nemoclaw Onboard fails` → `[Bug] [Slack] Slack configuration in Nemoclaw Onboard fails` + +## Prerequisites + +- You must be in the NemoClaw git repository. +- The `gh` CLI must be authenticated with write access to `NVIDIA/NemoClaw`. +- Default behavior is a dry run. Do not apply changes until the user approves the preview. + +## Workflow + +Copy this checklist and track progress: + +```text +Title tag cleanup progress: +- [ ] Step 1: Verify GitHub auth +- [ ] Step 2: Preview proposed title changes +- [ ] Step 3: Confirm scope +- [ ] Step 4: Apply changes +- [ ] Step 5: Verify no matching tags remain in scope +``` + +## Step 1: Verify GitHub Auth + +```bash +gh auth status +``` + +## Step 2: Preview Proposed Changes + +```bash +node --experimental-strip-types --no-warnings \ + .agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts +``` + +The script matches bracket tags whose content is `nemoclaw`, case-insensitively, anywhere in the title. +It prints a dry-run summary by default. Review the proposed renames with the user before applying anything. + +## Step 3: Confirm Scope + +Ask the user which scope they want: + +- **Default** — all open and closed issues and PRs in `NVIDIA/NemoClaw` +- **State filter** — optionally limit to `open` or `closed` +- **Repo override** — only when the user explicitly wants a different repository + +## Step 4: Apply Changes + +Apply to all items: + +```bash +node --experimental-strip-types --no-warnings \ + .agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts \ + --apply +``` + +Apply only to open items: + +```bash +node --experimental-strip-types --no-warnings \ + .agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts \ + --state open \ + --apply +``` + +## Step 5: Verify + +The script automatically re-runs the same search after `--apply` and exits non-zero if matching tags remain. + +If verification fails, stop and show the remaining matches to the user instead of retrying blindly. + +## Notes + +- The script uses the GitHub Issues API, which covers both issues and pull requests. +- It removes only bracket tags whose content is `nemoclaw`, ignoring case. Plain-text mentions of `NemoClaw` are untouched. +- The default repository is `NVIDIA/NemoClaw`. Pass `--repo OWNER/REPO` only when the user explicitly wants a different repo. diff --git a/.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts b/.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts new file mode 100644 index 00000000000..af4ae9d9d7c --- /dev/null +++ b/.agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts @@ -0,0 +1,297 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +/** + * Preview or apply cleanup for bracketed NemoClaw tags in GitHub issue and PR titles. + * + * Removes any bracket tag whose content is `nemoclaw`, case-insensitively, + * anywhere in the title. + * + * Usage: + * node --experimental-strip-types --no-warnings \ + * .agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts \ + * [--repo OWNER/REPO] [--state all|open|closed] [--apply] + */ + +import { execFileSync } from "node:child_process"; + +type QueryState = "all" | "open" | "closed"; +type ItemState = "open" | "closed"; +type ItemType = "issue" | "pr"; + +interface GitHubIssueLike { + number: number; + title: string; + state: ItemState; + html_url: string; + pull_request?: unknown; +} + +interface TitleCleanup { + matchedTags: string[]; + newTitle: string; +} + +interface Match { + number: number; + type: ItemType; + state: ItemState; + url: string; + matchedTags: string[]; + oldTitle: string; + newTitle: string; +} + +interface Options { + repo: string; + state: QueryState; + apply: boolean; +} + +const BRACKET_TAG_REGEX = /\[[^\]]+\]/g; + +function usage(): string { + return [ + "Usage:", + " node --experimental-strip-types --no-warnings \\", + " .agents/skills/nemoclaw-maintainer-normalize-title-tags/scripts/normalize-title-tags.ts \\", + " [--repo OWNER/REPO] [--state all|open|closed] [--apply]", + "", + "Defaults:", + " --repo NVIDIA/NemoClaw", + " --state all", + " dry-run mode unless --apply is provided", + ].join("\n"); +} + +function run(cmd: string, args: string[]): string { + return execFileSync(cmd, args, { + encoding: "utf-8", + timeout: 120_000, + maxBuffer: 10 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }).trim(); +} + +function ghJson(args: string[]): unknown { + return JSON.parse(run("gh", args)); +} + +function parseArgs(argv: string[]): Options { + const options: Options = { + repo: "NVIDIA/NemoClaw", + state: "all", + apply: false, + }; + + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]; + + if (arg === "--help" || arg === "-h") { + console.log(usage()); + process.exit(0); + } + + if (arg === "--apply") { + options.apply = true; + continue; + } + + if (arg === "--repo") { + const value = argv[i + 1]; + if (!value || value.startsWith("--")) { + throw new Error("--repo requires OWNER/REPO"); + } + options.repo = value; + i += 1; + continue; + } + + if (arg === "--state") { + const value = argv[i + 1] as QueryState | undefined; + if (value !== "all" && value !== "open" && value !== "closed") { + throw new Error("--state must be one of: all, open, closed"); + } + options.state = value; + i += 1; + continue; + } + + throw new Error(`Unknown argument: ${arg}`); + } + + return options; +} + +function isNemoclawTag(tag: string): boolean { + return tag.slice(1, -1).trim().toLowerCase() === "nemoclaw"; +} + +function cleanupTitle(title: string): string { + return title.replace(/\s{2,}/g, " ").trim(); +} + +function stripNemoclawTags(title: string): TitleCleanup { + const matchedTags: string[] = []; + + const withoutTags = title.replace(BRACKET_TAG_REGEX, (tag) => { + if (!isNemoclawTag(tag)) { + return tag; + } + matchedTags.push(tag); + return ""; + }); + + return { + matchedTags, + newTitle: cleanupTitle(withoutTags), + }; +} + +function listItems(repo: string, state: QueryState): GitHubIssueLike[] { + const items: GitHubIssueLike[] = []; + + for (let page = 1; ; page += 1) { + const response = ghJson([ + "api", + `repos/${repo}/issues?state=${state}&per_page=100&page=${page}`, + ]); + + if (!Array.isArray(response) || response.length === 0) { + break; + } + + for (const item of response) { + if ( + item && + typeof item === "object" && + typeof item.number === "number" && + typeof item.title === "string" && + (item.state === "open" || item.state === "closed") && + typeof item.html_url === "string" + ) { + items.push(item as GitHubIssueLike); + } + } + } + + return items; +} + +function collectMatches(options: Options): Match[] { + const matches: Match[] = []; + + for (const item of listItems(options.repo, options.state)) { + const cleaned = stripNemoclawTags(item.title); + if (cleaned.matchedTags.length === 0 || cleaned.newTitle === item.title) { + continue; + } + + if (!cleaned.newTitle) { + console.error(`Skipping #${item.number}: cleanup would produce an empty title.`); + continue; + } + + matches.push({ + number: item.number, + type: item.pull_request ? "pr" : "issue", + state: item.state, + url: item.html_url, + matchedTags: cleaned.matchedTags, + oldTitle: item.title, + newTitle: cleaned.newTitle, + }); + } + + return matches.sort((a, b) => a.number - b.number); +} + +function printSummary(options: Options, matches: Match[]): void { + const tagCounts = new Map(); + let issueCount = 0; + let prCount = 0; + let tagTotal = 0; + + for (const match of matches) { + for (const tag of match.matchedTags) { + tagCounts.set(tag, (tagCounts.get(tag) ?? 0) + 1); + tagTotal += 1; + } + if (match.type === "issue") { + issueCount += 1; + } else { + prCount += 1; + } + } + + console.log(`Mode: ${options.apply ? "apply" : "dry-run"}`); + console.log(`Repo: ${options.repo}`); + console.log(`State: ${options.state}`); + console.log(`Title matches: ${matches.length} (${issueCount} issues, ${prCount} PRs)`); + console.log(`Tags to remove: ${tagTotal}`); + + if (tagCounts.size > 0) { + console.log("Tag counts:"); + for (const [tag, count] of tagCounts.entries()) { + console.log(` ${tag}: ${count}`); + } + } + + if (matches.length === 0) { + console.log("No matching titles found."); + return; + } + + console.log(""); + for (const match of matches) { + console.log(`#${match.number} [${match.type}] [${match.state}] ${match.url}`); + console.log(` tags: ${match.matchedTags.join(", ")}`); + console.log(` old: ${match.oldTitle}`); + console.log(` new: ${match.newTitle}`); + } +} + +function applyMatches(options: Options, matches: Match[]): void { + for (const match of matches) { + run("gh", [ + "api", + "-X", + "PATCH", + `repos/${options.repo}/issues/${match.number}`, + "-f", + `title=${match.newTitle}`, + ]); + console.log(`UPDATED #${match.number}: ${match.oldTitle} -> ${match.newTitle}`); + } +} + +function main(): void { + try { + const options = parseArgs(process.argv.slice(2)); + const matches = collectMatches(options); + printSummary(options, matches); + + if (!options.apply || matches.length === 0) { + return; + } + + console.log(""); + applyMatches(options, matches); + + console.log("\nVerifying..."); + const remaining = collectMatches({ ...options, apply: false }); + if (remaining.length > 0) { + console.error(`Verification failed: ${remaining.length} matching titles remain.`); + process.exit(1); + } + + console.log("Verification passed: 0 matching titles remain."); + } catch (error: unknown) { + const message = error instanceof Error ? error.message : String(error); + console.error(message); + console.error(usage()); + process.exit(1); + } +} + +main(); diff --git a/.agents/skills/nemoclaw-skills-guide/SKILL.md b/.agents/skills/nemoclaw-skills-guide/SKILL.md index bd116794873..0570e111b1f 100644 --- a/.agents/skills/nemoclaw-skills-guide/SKILL.md +++ b/.agents/skills/nemoclaw-skills-guide/SKILL.md @@ -21,10 +21,10 @@ The prefix in each skill name indicates who it is for. For end users operating a NemoClaw sandbox. Covers installation, inference configuration, network policy management, monitoring, remote deployment, security configuration, workspace management, and reference material. -### `nemoclaw-maintainer-*` (6 skills) +### `nemoclaw-maintainer-*` (7 skills) For project maintainers. -Covers the daily maintainer cadence (morning standup, daytime loop, evening handoff), cutting releases, finding PRs to review, and performing security code reviews. +Covers the daily maintainer cadence (morning standup, daytime loop, evening handoff), cutting releases, finding PRs to review, normalizing issue and PR title tags, and performing security code reviews. ### `nemoclaw-contributor-*` (2 skills) @@ -58,6 +58,7 @@ Covers creating pull requests that follow the project template and drafting docu | `nemoclaw-maintainer-evening` | End-of-day handoff: check version progress, bump stragglers to the next patch, generate a QA handoff summary, and cut the release tag. | | `nemoclaw-maintainer-cut-release-tag` | Cut an annotated semver tag on main, move the `latest` floating tag, and push both to origin. | | `nemoclaw-maintainer-find-review-pr` | Find open PRs labeled security + priority-high, link each to its issue, detect duplicates, and present a review summary. | +| `nemoclaw-maintainer-normalize-title-tags` | Preview and remove bracketed `NemoClaw` title tags from issues and PRs case-insensitively, even when the tag appears later in the title. | | `nemoclaw-maintainer-security-code-review` | Perform a 9-category security review of a PR or issue, producing per-category PASS/WARNING/FAIL verdicts. | ### Contributor Skills @@ -81,6 +82,6 @@ Skills are cumulative. Each role includes the skills from the roles above it: |------|----------------|-------|------------| | User | `nemoclaw-user-*` | 9 | `nemoclaw-user-get-started` | | Contributor | `nemoclaw-user-*` + `nemoclaw-contributor-*` | 11 | `nemoclaw-user-overview` | -| Maintainer | All skills | 17 | `nemoclaw-maintainer-morning` | +| Maintainer | All skills | 18 | `nemoclaw-maintainer-morning` | After identifying the role, present the applicable skills from the Skill Catalog above and recommend the starting skill. From 72056a3a77ceda858aa3390da44df3c1ec8b2df2 Mon Sep 17 00:00:00 2001 From: Miyoung Choi Date: Wed, 22 Apr 2026 17:26:56 -0700 Subject: [PATCH 14/17] docs: update commands reference for 0.0.23 release (#2312) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Catch up the user-facing docs for features and command renames merged since v0.0.22, and bump the doc-site version strings to 0.0.23. ## Related Issue ## Changes - `docs/reference/commands.md` — Documented `snapshot create --name`, the new version-aware `snapshot list` table, and version/name/timestamp selectors for `snapshot restore` (from #2184). Renamed `nemoclaw start`/`stop` sections to `nemoclaw tunnel start`/`stop` with a deprecation note on the legacy aliases, and added new `channels start`/`channels stop` per-channel subsections (from #2103). - `docs/workspace/backup-restore.md` — Added `--name` and `v` / name / timestamp selector examples for the snapshot commands. - `docs/deployment/set-up-telegram-bridge.md`, `docs/deployment/deploy-to-remote-gpu.md`, `docs/reference/troubleshooting.md` — Switched references from `nemoclaw start` to `nemoclaw tunnel start`; noted `channels stop/start` as the non-destructive way to pause a bridge. - `docs/project.json`, `docs/versions1.json` — Bumped preferred doc version to 0.0.23 and moved 0.0.22 into the version history list. - `.agents/skills/nemoclaw-user-*` — Regenerated via `scripts/docs-to-skills.py` so downstream skills match the doc sources. ## Type of Change - [ ] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [x] Doc only (includes code sample changes) ## Verification - [x] `npx prek run --all-files` passes - [ ] `npm test` passes - [ ] Tests added or updated for new or changed behavior - [x] No secrets, API keys, or credentials committed - [x] Docs updated for user-facing behavior changes - [x] `make docs` builds without warnings (doc changes only) - [x] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) Note on `npm test`: the CLI suite hit a pre-existing environmental timeout in `test/cli.test.ts:60` (`bare unknown name surfaces sandbox-not-found`) reaching a non-running local gateway. The plugin and skills YAML suites pass. This PR touches only Markdown + JSON under `docs/` and autogenerated `.agents/skills/` output, so the failing test is unrelated. ## AI Disclosure - [x] AI-assisted — tool: Claude Code --- Signed-off-by: Miyoung Choi ## Summary by CodeRabbit * **New Features** * Added `channels stop/start` commands to pause and resume individual messaging bridges without destroying sandboxes * Introduced `--name` flag for creating labeled snapshots with improved restore selection (by version, name, or timestamp) * Added `--dangerously-skip-permissions` flag to `nemoclaw onboard` * Renamed host auxiliary commands to `nemoclaw tunnel start/stop` (legacy aliases retained for backward compatibility) * **Documentation** * Expanded quickstart guide with recovery, reconfiguration, and credential reset workflows * Updated all documentation to reflect new command structure and bridge lifecycle operations Signed-off-by: Miyoung Choi Co-authored-by: Claude Opus 4.7 (1M context) --- .../nemoclaw-user-deploy-remote/SKILL.md | 14 +-- .../skills/nemoclaw-user-get-started/SKILL.md | 46 ++++++++++ .../references/commands.md | 90 ++++++++++++++++--- .../references/troubleshooting.md | 6 +- .../skills/nemoclaw-user-workspace/SKILL.md | 12 ++- docs/deployment/deploy-to-remote-gpu.md | 2 +- docs/deployment/set-up-telegram-bridge.md | 16 ++-- docs/project.json | 2 +- docs/reference/commands.md | 66 ++++++++++++-- docs/reference/troubleshooting.md | 6 +- docs/versions1.json | 4 + docs/workspace/backup-restore.md | 12 ++- 12 files changed, 234 insertions(+), 42 deletions(-) diff --git a/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md b/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md index 706a8dce0a8..9ed32a47dc6 100644 --- a/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md +++ b/.agents/skills/nemoclaw-user-deploy-remote/SKILL.md @@ -56,7 +56,7 @@ The legacy compatibility flow performs the following steps on the VM: 1. Installs Docker and the NVIDIA Container Toolkit if a GPU is present. 2. Installs the OpenShell CLI. 3. Runs `nemoclaw onboard` (the setup wizard) to create the gateway, register providers, and launch the sandbox. -4. Starts optional host auxiliary services (for example the cloudflared tunnel) when `cloudflared` is available. Channel messaging is configured during onboarding and runs through OpenShell-managed processes, not through `nemoclaw start`. +4. Starts optional host auxiliary services (for example the cloudflared tunnel) when `cloudflared` is available. Channel messaging is configured during onboarding and runs through OpenShell-managed processes, not through `nemoclaw tunnel start`. By default, the compatibility wrapper asks Brev to provision on `gcp`. Override this with `NEMOCLAW_BREV_PROVIDER` if you need a different Brev cloud provider. @@ -141,7 +141,7 @@ $ nemoclaw deploy Telegram, Discord, and Slack reach your agent through OpenShell-managed processes and gateway constructs. NemoClaw configures those channels during `nemoclaw onboard`. Tokens are registered with OpenShell providers, channel configuration is baked into the sandbox image, and runtime delivery stays under OpenShell control. -`nemoclaw start` does not start Telegram (or other chat bridges). It only starts optional host services such as the cloudflared tunnel when that binary is present. +`nemoclaw tunnel start` does not start Telegram (or other chat bridges). It only starts optional host services such as the cloudflared tunnel when that binary is present. (`nemoclaw start` is kept as a deprecated alias.) For details, refer to Commands (use the `nemoclaw-user-reference` skill). ## Step 9: Create a Telegram Bot @@ -197,15 +197,17 @@ For a full first-time flow, refer to Quickstart (use the `nemoclaw-user-get-star After the sandbox is running, send a message to your bot in Telegram. If something fails, use `openshell term` on the host, check gateway logs, and verify network policy allows the Telegram API (see Customize the Network Policy (use the `nemoclaw-user-manage-policy` skill) and the `telegram` preset). -## Step 13: `nemoclaw start` (cloudflared Only) +## Step 13: `nemoclaw tunnel start` (cloudflared Only) -`nemoclaw start` starts cloudflared when it is installed, which can expose the dashboard with a public URL. -It does not affect Telegram connectivity. +`nemoclaw tunnel start` starts cloudflared when it is installed, which can expose the dashboard with a public URL. +It does not affect Telegram connectivity. The older `nemoclaw start` still works as a deprecated alias. ```console -$ nemoclaw start +$ nemoclaw tunnel start ``` +To pause the Telegram bridge without removing its credentials or destroying the sandbox, use `nemoclaw channels stop telegram`. Re-enable it later with `nemoclaw channels start telegram`. + ## References - **Load [references/sandbox-hardening.md](references/sandbox-hardening.md)** when reviewing sandbox image security controls, auditing capability drops, or looking up the runtime resource limits. Includes the sandbox container image hardening reference, covering Docker capabilities and process limits. diff --git a/.agents/skills/nemoclaw-user-get-started/SKILL.md b/.agents/skills/nemoclaw-user-get-started/SKILL.md index 41005e53991..7fbfb1c6427 100644 --- a/.agents/skills/nemoclaw-user-get-started/SKILL.md +++ b/.agents/skills/nemoclaw-user-get-started/SKILL.md @@ -92,6 +92,52 @@ curl -fsSL https://raw.githubusercontent.com/NVIDIA/NemoClaw/refs/heads/main/uni For troubleshooting installation or onboarding issues, see the Troubleshooting guide (use the `nemoclaw-user-reference` skill). +## Step 4: Reconfigure or Recover + +Recover from a misconfigured sandbox without re-running the full onboard wizard or destroying workspace state. + +### Change inference model or API + +Change the active model or provider at runtime without rebuilding the sandbox: + +```console +$ openshell inference set -g nemoclaw -m -p +``` + +See Switch inference providers (use the `nemoclaw-user-configure-inference` skill) for provider-specific model IDs and API compatibility notes. + +### 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: + +```console +$ nemoclaw credentials list # see which keys are stored +$ nemoclaw credentials reset # clear a single key, e.g. NVIDIA_API_KEY +$ nemoclaw onboard # re-run to re-enter the cleared key +``` + +The credentials command is documented in full at Commands → `nemoclaw credentials reset` (use the `nemoclaw-user-reference` skill). + +### Rebuild a sandbox while preserving workspace state + +If you changed the underlying Dockerfile, upgraded OpenClaw, or want to pick up a new base image without losing your sandbox's workspace files, use `rebuild` instead of destroying and recreating: + +```console +$ nemoclaw rebuild +``` + +Rebuild preserves the mounted workspace and registered policies while recreating the container. See Commands → `nemoclaw rebuild` (use the `nemoclaw-user-reference` skill) for flag details. + +### Add a network preset after onboarding + +Apply an additional preset (e.g. Telegram, GitHub) to a running sandbox without re-onboarding: + +```console +$ nemoclaw policy-add +``` + +See Commands → `nemoclaw policy-add` (use the `nemoclaw-user-reference` skill) for usage details and flags. + ## References - **Load [references/windows-setup.md](references/windows-setup.md)** when installing NemoClaw on Windows, enabling WSL 2, configuring Docker Desktop for Windows, or troubleshooting a Windows-specific install error. Includes Windows-only prerequisites that must be completed before the Quickstart. diff --git a/.agents/skills/nemoclaw-user-reference/references/commands.md b/.agents/skills/nemoclaw-user-reference/references/commands.md index ad576456993..733e5e0610a 100644 --- a/.agents/skills/nemoclaw-user-reference/references/commands.md +++ b/.agents/skills/nemoclaw-user-reference/references/commands.md @@ -44,7 +44,7 @@ The wizard creates an OpenShell gateway, registers inference providers, builds t Use this command for new installs and for recreating a sandbox after changes to policy or configuration. ```console -$ nemoclaw onboard [--non-interactive] [--resume] [--recreate-sandbox] [--from ] [--agent ] [--yes-i-accept-third-party-software] +$ nemoclaw onboard [--non-interactive] [--resume] [--recreate-sandbox] [--from ] [--agent ] [--dangerously-skip-permissions] [--yes-i-accept-third-party-software] ``` > **Warning:** For NemoClaw-managed environments, use `nemoclaw onboard` when you need to create or recreate the OpenShell gateway or sandbox. @@ -152,6 +152,28 @@ $ NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_FROM_DOCKERFILE=path/to/Dockerfile nemocla If a `--resume` is attempted with a different `--from` path than the original session, onboarding exits with a conflict error rather than silently building from the wrong image. +#### `--dangerously-skip-permissions` + +> **Warning:** For development and testing only. This flag disables the sandbox's network policy and filesystem permission restrictions, so the OpenClaw agent inside the sandbox can reach any host and write anywhere in its home directory. Do not use this flag with production credentials or on hosts where other agents run. + +Replace the default balanced sandbox policy with the permissive policy bundled at `nemoclaw-blueprint/policies/openclaw-sandbox-permissive.yaml`. Concretely, this means: + +- **Network:** all known endpoints open with no HTTP method or path filtering. +- **Filesystem:** the sandbox home directory is writable (normally Landlock-restricted). +- **Messaging / inference:** unchanged — still gated by the provider credentials you supply. + +```console +$ nemoclaw onboard --dangerously-skip-permissions +``` + +Onboarding prints an explicit warning at start so the reduced security posture is visible in logs. The flag is also honored via `NEMOCLAW_DANGEROUSLY_SKIP_PERMISSIONS=1` for non-interactive runs: + +```console +$ NEMOCLAW_DANGEROUSLY_SKIP_PERMISSIONS=1 nemoclaw onboard --non-interactive --yes-i-accept-third-party-software +``` + +The flag is persisted on the sandbox registry entry, so `nemoclaw status` surfaces `Permissions: dangerously-skip-permissions (shields permanently down)` for sandboxes created this way. To tighten a sandbox after the fact, re-run `nemoclaw onboard` without the flag. + ### `nemoclaw list` List all registered sandboxes with their model, provider, and policy presets. @@ -356,6 +378,32 @@ As with `channels add`, `NEMOCLAW_NON_INTERACTIVE=1` skips the rebuild prompt an Host-side removal is the supported path because `/sandbox/.openclaw/openclaw.json` is read-only at runtime; `openclaw channels remove` cannot modify the baked config from inside the sandbox. +### `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. + +```console +$ nemoclaw my-assistant channels stop telegram +``` + +| Flag | Description | +|------|-------------| +| `--dry-run` | Report the channel that would be disabled without updating the registry or rebuilding | + +Use `channels stop` instead of `channels remove` when you want to pause a bridge temporarily. `channels remove` is destructive to credentials; `channels stop` is not. + +### `nemoclaw channels start ` + +Re-enable a channel previously paused with `channels stop`. The channel is removed from the disabled list, the sandbox is rebuilt, and the bridge registers with the gateway again using the stored credentials. + +```console +$ nemoclaw my-assistant channels start telegram +``` + +| Flag | Description | +|------|-------------| +| `--dry-run` | Report the channel that would be re-enabled without updating the registry or rebuilding | + ### `nemoclaw skill install ` Deploy a skill directory to a running sandbox. @@ -435,24 +483,42 @@ Snapshots are stored in `~/.nemoclaw/rebuild-backups//`. $ nemoclaw my-assistant snapshot create ``` +| Flag | Description | +|------|-------------| +| `--name