feat: require explicit third-party software acceptance before onboarding#1290
Merged
feat: require explicit third-party software acceptance before onboarding#1290
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a third‑party usage notice and consent flow: new usage‑notice module and JSON, a CJS shim, onboarding early‑exit gate, CLI/install flags and env var for non‑interactive acceptance, installer TTY handling, and comprehensive tests exercising interactive/non‑interactive consent and persistence. Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
actor User
participant Installer as install.sh
participant CLI as nemoclaw (bin/nemoclaw.js)
participant Onboard as onboard (bin/lib/onboard.js)
participant Notice as usage-notice (src/lib/usage-notice)
participant FS as FileSystem
end
User->>Installer: run installer (interactive or non-interactive)
Installer->>Notice: invoke usage-notice CLI (--non-interactive / flag)
Notice->>FS: read notice config and state file
alt already accepted
Notice-->>Installer: success
else not accepted
alt non-interactive & flag/env present
Notice-->>FS: write acceptance
Notice-->>Installer: success
else interactive with TTY
Notice->>User: prompt "type yes"
User-->>Notice: "yes" / "no"
alt yes
Notice-->>FS: write acceptance
Notice-->>Installer: success
else no
Notice-->>Installer: failure (exit 1)
end
else failure (no TTY or not accepted)
Notice-->>Installer: failure (exit 1)
end
end
alt Notice success
Installer->>CLI: start onboard
CLI->>Onboard: run onboarding steps
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
test/usage-notice.test.js (1)
114-136: Prefertry/finallyaround stdout/stderr TTY overrides.This test currently restores TTY flags, but wrapping the override block in
try/finallywould make cleanup resilient even ifprintUsageNoticethrows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/usage-notice.test.js` around lines 114 - 136, The test modifies process.stdout.isTTY and process.stderr.isTTY but restores them outside of a try/finally, so if printUsageNotice (or helpers like loadUsageNoticeConfig) throws the original values may not be restored; update the test to wrap the call to printUsageNotice(...) in a try/finally block that restores process.stdout.isTTY and process.stderr.isTTY in the finally clause to guarantee cleanup even on exceptions.bin/lib/usage-notice.js (1)
19-21: Consider adding error handling for config file loading.
loadUsageNoticeConfig()will throw an uncaught exception ifusage-notice.jsonis missing or contains invalid JSON. While this is a bundled file that should always exist, a more defensive approach would provide a clearer error message.🛡️ Optional: Add defensive error handling
function loadUsageNoticeConfig() { - return JSON.parse(fs.readFileSync(NOTICE_CONFIG_FILE, "utf8")); + try { + return JSON.parse(fs.readFileSync(NOTICE_CONFIG_FILE, "utf8")); + } catch (err) { + throw new Error(`Failed to load usage notice config from ${NOTICE_CONFIG_FILE}: ${err.message}`); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bin/lib/usage-notice.js` around lines 19 - 21, The loadUsageNoticeConfig function currently calls fs.readFileSync and JSON.parse directly and will throw unhelpful exceptions if NOTICE_CONFIG_FILE is missing or invalid; wrap the read+parse in a try/catch inside loadUsageNoticeConfig, catch any error from fs.readFileSync/JSON.parse, and rethrow or return a clear, contextual error (e.g. throw new Error(`Failed to load ${NOTICE_CONFIG_FILE}: ${err.message}`)) so callers get a useful message rather than an uncaught JSON/FS exception; reference loadUsageNoticeConfig and NOTICE_CONFIG_FILE when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/usage-notice.test.js`:
- Around line 24-47: The tests overwrite process.env.HOME in beforeEach but
never restore it; capture the prior value (e.g. const originalHome =
process.env.HOME) at the top of beforeEach (alongside originalIsTTY) and then in
afterEach set process.env.HOME = originalHome (or delete it if originalHome is
undefined) before removing testHome; update references to testHome, beforeEach
and afterEach to use originalHome so HOME is not leaked across tests.
- Around line 10-18: Test uses CommonJS require for the notice module; switch to
ESM import to match the file's ES module usage. Replace the require(...) usage
that assigns NOTICE_ACCEPT_FLAG, ensureUsageNoticeConsent,
formatTerminalHyperlink, getUsageNoticeStateFile, hasAcceptedUsageNotice,
loadUsageNoticeConfig, printUsageNotice with an ESM dynamic import using
top-level await (e.g., const mod = await import(noticePath); const {
NOTICE_ACCEPT_FLAG, ensureUsageNoticeConsent, formatTerminalHyperlink,
getUsageNoticeStateFile, hasAcceptedUsageNotice, loadUsageNoticeConfig,
printUsageNotice } = mod;) so the test file is fully ESM-compatible and the
named exports are preserved. Ensure the test environment supports top-level
await/ESM.
---
Nitpick comments:
In `@bin/lib/usage-notice.js`:
- Around line 19-21: The loadUsageNoticeConfig function currently calls
fs.readFileSync and JSON.parse directly and will throw unhelpful exceptions if
NOTICE_CONFIG_FILE is missing or invalid; wrap the read+parse in a try/catch
inside loadUsageNoticeConfig, catch any error from fs.readFileSync/JSON.parse,
and rethrow or return a clear, contextual error (e.g. throw new Error(`Failed to
load ${NOTICE_CONFIG_FILE}: ${err.message}`)) so callers get a useful message
rather than an uncaught JSON/FS exception; reference loadUsageNoticeConfig and
NOTICE_CONFIG_FILE when making the change.
In `@test/usage-notice.test.js`:
- Around line 114-136: The test modifies process.stdout.isTTY and
process.stderr.isTTY but restores them outside of a try/finally, so if
printUsageNotice (or helpers like loadUsageNoticeConfig) throws the original
values may not be restored; update the test to wrap the call to
printUsageNotice(...) in a try/finally block that restores process.stdout.isTTY
and process.stderr.isTTY in the finally clause to guarantee cleanup even on
exceptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10ca9e8d-a89c-49e1-9a6d-e77fe3a1cc17
📒 Files selected for processing (8)
bin/lib/onboard.jsbin/lib/usage-notice.jsbin/lib/usage-notice.jsonbin/nemoclaw.jsinstall.shtest/cli.test.jstest/install-preflight.test.jstest/usage-notice.test.js
Comment on lines
+10
to
+18
| const { | ||
| NOTICE_ACCEPT_FLAG, | ||
| ensureUsageNoticeConsent, | ||
| formatTerminalHyperlink, | ||
| getUsageNoticeStateFile, | ||
| hasAcceptedUsageNotice, | ||
| loadUsageNoticeConfig, | ||
| printUsageNotice, | ||
| } = require(noticePath); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify test files under test/** are not using CommonJS require.
# Expected result: no matches.
rg -nP --glob 'test/**/*.test.js' '\brequire\s*\('Repository: NVIDIA/NemoClaw
Length of output: 14271
🏁 Script executed:
cat -n test/usage-notice.test.js | head -50Repository: NVIDIA/NemoClaw
Length of output: 1827
🏁 Script executed:
head -30 bin/lib/usage-notice.jsRepository: NVIDIA/NemoClaw
Length of output: 1035
Use ESM import instead of require(...) in this test file.
This test uses ESM syntax elsewhere (lines 4–6) but requires the module via CommonJS at line 18, violating the guideline: test/**/*.test.js must use ES modules.
💡 Proposed fix
const repoRoot = path.join(import.meta.dirname, "..");
-const noticePath = path.join(repoRoot, "bin", "lib", "usage-notice.js");
-const {
+import usageNotice from "../bin/lib/usage-notice.js";
+
+const {
NOTICE_ACCEPT_FLAG,
ensureUsageNoticeConsent,
formatTerminalHyperlink,
getUsageNoticeStateFile,
hasAcceptedUsageNotice,
loadUsageNoticeConfig,
printUsageNotice,
-} = require(noticePath);
+} = usageNotice;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/usage-notice.test.js` around lines 10 - 18, Test uses CommonJS require
for the notice module; switch to ESM import to match the file's ES module usage.
Replace the require(...) usage that assigns NOTICE_ACCEPT_FLAG,
ensureUsageNoticeConsent, formatTerminalHyperlink, getUsageNoticeStateFile,
hasAcceptedUsageNotice, loadUsageNoticeConfig, printUsageNotice with an ESM
dynamic import using top-level await (e.g., const mod = await
import(noticePath); const { NOTICE_ACCEPT_FLAG, ensureUsageNoticeConsent,
formatTerminalHyperlink, getUsageNoticeStateFile, hasAcceptedUsageNotice,
loadUsageNoticeConfig, printUsageNotice } = mod;) so the test file is fully
ESM-compatible and the named exports are preserved. Ensure the test environment
supports top-level await/ESM.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/lib/usage-notice.ts (2)
136-138: Minor message inaccuracy: acceptance could also be via env var.The message says "accepted via
${NOTICE_ACCEPT_FLAG}" butacceptedByFlagcan also betruewhenNEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1is set. Consider mentioning both.Proposed fix
writeLine( - ` [non-interactive] Third-party software notice accepted via ${NOTICE_ACCEPT_FLAG}.`, + ` [non-interactive] Third-party software notice accepted via ${NOTICE_ACCEPT_FLAG} or ${NOTICE_ACCEPT_ENV}=1.`, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/usage-notice.ts` around lines 136 - 138, Update the notice message in writeLine so it correctly reflects both acceptance mechanisms: mention the CLI flag (NOTICE_ACCEPT_FLAG) and the environment variable (NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE) instead of only the flag; locate the message construction that uses NOTICE_ACCEPT_FLAG in src/lib/usage-notice.ts (the writeLine call) and change the text to reference both the flag and env var or use a conditional that inserts the correct mechanism based on the acceptedByFlag/acceptedByEnv variables.
100-112: Confusing hyperlink label logic:link.labelis checked but not used.At line 103, the condition checks
link?.url && link?.label, but line 104 passeslink.urlas both arguments toformatTerminalHyperlink, ignoringlink.label. If the intent is to make the label text clickable (linking to the URL), this should beformatTerminalHyperlink(link.label, link.url).Currently, the
linksarray in the config is empty, so this has no immediate impact, but the logic is misleading.Proposed fix
const label = supportsTerminalHyperlinks() && link?.url && link?.label - ? formatTerminalHyperlink(link.url, link.url) + ? formatTerminalHyperlink(link.label, link.url) : link?.label || "";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/usage-notice.ts` around lines 100 - 112, The hyperlink label logic mistakenly passes link.url for both text and href to formatTerminalHyperlink; update the block iterating config.links so that when supportsTerminalHyperlinks() and link?.url and link?.label are truthy you call formatTerminalHyperlink(link.label, link.url) (using the human label as the clickable text), while preserving the existing fallbacks (use link?.label || "" for non-hyperlink display and still print link.url on its own). Adjust references in the loop around supportsTerminalHyperlinks(), formatTerminalHyperlink, link, and writeLine accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/usage-notice.ts`:
- Around line 136-138: Update the notice message in writeLine so it correctly
reflects both acceptance mechanisms: mention the CLI flag (NOTICE_ACCEPT_FLAG)
and the environment variable (NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE) instead of
only the flag; locate the message construction that uses NOTICE_ACCEPT_FLAG in
src/lib/usage-notice.ts (the writeLine call) and change the text to reference
both the flag and env var or use a conditional that inserts the correct
mechanism based on the acceptedByFlag/acceptedByEnv variables.
- Around line 100-112: The hyperlink label logic mistakenly passes link.url for
both text and href to formatTerminalHyperlink; update the block iterating
config.links so that when supportsTerminalHyperlinks() and link?.url and
link?.label are truthy you call formatTerminalHyperlink(link.label, link.url)
(using the human label as the clickable text), while preserving the existing
fallbacks (use link?.label || "" for non-hyperlink display and still print
link.url on its own). Adjust references in the loop around
supportsTerminalHyperlinks(), formatTerminalHyperlink, link, and writeLine
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d737c98b-10bf-460f-8747-dcfb0f3d3679
📒 Files selected for processing (4)
bin/lib/usage-notice.jsbin/lib/usage-notice.jsonsrc/lib/usage-notice.tstest/usage-notice.test.js
✅ Files skipped from review due to trivial changes (2)
- bin/lib/usage-notice.json
- test/usage-notice.test.js
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/install-preflight.test.js (1)
506-551: Consider extracting the duplicated npm stub into a reusable helper.The npm stubs in these two new tests (lines 515-534 and 562-581) are identical. You could use the existing
writeNpmStubhelper with a custom snippet, or extract a dedicated helper for these acceptance-gate tests.♻️ Example using writeNpmStub
+const ONBOARD_LOGGING_SNIPPET = `if [ "$1" = "pack" ]; then + tmpdir="$4" + mkdir -p "$tmpdir/package" + tar -czf "$tmpdir/openclaw-2026.3.11.tgz" -C "$tmpdir" package + exit 0 +fi +if [ "$1" = "install" ]; then exit 0; fi +if [ "$1" = "run" ] && { [ "$2" = "build" ] || [ "$2" = "build:cli" ] || [ "$2" = "--if-present" ]; }; then exit 0; fi +if [ "$1" = "link" ]; then + cat > "$NPM_PREFIX/bin/nemoclaw" <<'EOS' +#!/usr/bin/env bash +printf '%s\\\\n' "$*" >> "$NEMOCLAW_ONBOARD_LOG" +exit 0 +EOS + chmod +x "$NPM_PREFIX/bin/nemoclaw" + exit 0 +fi`; it("requires explicit terms acceptance in non-interactive install mode", () => { // ... setup ... writeNodeStub(fakeBin); - writeNpmStub( - fakeBin, - `if [ "$1" = "pack" ]; then - ... -fi`, - ); + writeNpmStub(fakeBin, ONBOARD_LOGGING_SNIPPET); // ... rest of test ... });Also applies to: 553-603
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/install-preflight.test.js` around lines 506 - 551, The test duplicates an identical npm stub; refactor by reusing the existing writeNpmStub helper instead of inlining the script in the "requires explicit terms acceptance in non-interactive install mode" test (and the similar test around lines 553-603). Replace the inline npm script block with a call to writeNpmStub(fakeBin, <scriptString>) or extract a small helper (e.g., writeAcceptanceNpmStub) and call it from both tests so the npm stub content is defined once; update references to fakeBin and NPM_PREFIX usage to match the helper signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/install-preflight.test.js`:
- Around line 506-551: The test duplicates an identical npm stub; refactor by
reusing the existing writeNpmStub helper instead of inlining the script in the
"requires explicit terms acceptance in non-interactive install mode" test (and
the similar test around lines 553-603). Replace the inline npm script block with
a call to writeNpmStub(fakeBin, <scriptString>) or extract a small helper (e.g.,
writeAcceptanceNpmStub) and call it from both tests so the npm stub content is
defined once; update references to fakeBin and NPM_PREFIX usage to match the
helper signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 701c2b77-2c3b-4acc-a142-bad6c8e469a2
📒 Files selected for processing (1)
test/install-preflight.test.js
cv
approved these changes
Apr 2, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e/test-spark-install.sh (1)
119-126: Inconsistent environment variable propagation between install paths.The
curl|bashpath (line 122) explicitly sets bothNEMOCLAW_NON_INTERACTIVE=1andNEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1inline, while the repo install path (line 125) relies on environment variable inheritance. Although the prerequisite check at lines 91-96 ensures the variable is set in the current shell, the explicit inline assignment in the curl path is more robust and self-documenting.Consider applying the same explicit pattern to the repo install path for consistency and clarity:
♻️ Suggested change for consistency
if [ "${NEMOCLAW_E2E_PUBLIC_INSTALL:-0}" = "1" ]; then url="${NEMOCLAW_INSTALL_SCRIPT_URL:-https://www.nvidia.com/nemoclaw.sh}" info "Running: curl -fsSL ... | bash (url=$url)" curl -fsSL "$url" | NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 bash >"$INSTALL_LOG" 2>&1 & else info "Running: bash install.sh --non-interactive" - bash install.sh --non-interactive >"$INSTALL_LOG" 2>&1 & + NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1 bash install.sh --non-interactive >"$INSTALL_LOG" 2>&1 & fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/test-spark-install.sh` around lines 119 - 126, The repo install branch should explicitly export the same env vars as the curl path: modify the `bash install.sh --non-interactive` invocation to prefix it with `NEMOCLAW_NON_INTERACTIVE=1 NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1` so the install script runs with identical, self-contained environment propagation as the `curl | bash` path (ensure the redirected output to `"$INSTALL_LOG"` and backgrounding `&` remain unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/e2e/e2e-cloud-experimental/test-port8080-conflict.sh`:
- Line 27: The usage example references the wrong path; update the example
invocation to point to the actual script location by replacing
"test/e2e/test-port8080-conflict.sh" with
"test/e2e/e2e-cloud-experimental/test-port8080-conflict.sh" in the comment line
inside test-port8080-conflict.sh so copy/paste users run the correct script.
---
Nitpick comments:
In `@test/e2e/test-spark-install.sh`:
- Around line 119-126: The repo install branch should explicitly export the same
env vars as the curl path: modify the `bash install.sh --non-interactive`
invocation to prefix it with `NEMOCLAW_NON_INTERACTIVE=1
NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1` so the install script runs with
identical, self-contained environment propagation as the `curl | bash` path
(ensure the redirected output to `"$INSTALL_LOG"` and backgrounding `&` remain
unchanged).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5275508-cfde-401e-bc4b-164dee7bd8fd
📒 Files selected for processing (14)
.github/workflows/nightly-e2e.yamlbin/nemoclaw.jsdocs/reference/commands.mdtest/e2e/brev-e2e.test.jstest/e2e/e2e-cloud-experimental/test-port8080-conflict.shtest/e2e/test-credential-sanitization.shtest/e2e/test-double-onboard.shtest/e2e/test-e2e-cloud-experimental.shtest/e2e/test-full-e2e.shtest/e2e/test-gpu-e2e.shtest/e2e/test-onboard-repair.shtest/e2e/test-onboard-resume.shtest/e2e/test-spark-install.shtest/e2e/test-telegram-injection.sh
✅ Files skipped from review due to trivial changes (3)
- test/e2e/test-telegram-injection.sh
- docs/reference/commands.md
- test/e2e/test-credential-sanitization.sh
This was referenced Apr 2, 2026
cjagwani
pushed a commit
to fdzdev/NemoClaw
that referenced
this pull request
Apr 3, 2026
…ing (NVIDIA#1290) ## Summary - show a third-party software notice before `install.sh` launches onboarding and before direct `nemoclaw onboard` - require explicit `yes` in interactive flows and fail immediately on any other answer - require explicit non-interactive acceptance via `--yes-i-accept-third-party-software` or `NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1` - link users to the official OpenClaw security guidance before continuing - render the security URL as a clickable terminal hyperlink where OSC 8 is supported, while still printing the raw URL - implement the new notice capability in TypeScript under `src/lib` ## Details This is aimed at issue NVIDIA#1213, but the acceptance behavior is stricter than the original issue text. The notice is specifically about third-party software liability and OpenClaw usage, not NemoClaw terms. The acceptance flag and env var are named accordingly. The current notice is intentionally formatted for standard terminal width and uses the approved wording in short, non-wrapping lines. Reference shown to users: - https://docs.openclaw.ai/gateway/security Credit: - builds on prior overlapping notice work in NVIDIA#1222 by @13ernkastel, but extends it to the current acceptance model, non-interactive enforcement, and the shared `install.sh` + `nemoclaw onboard` flow ## Validation - `npm run build:cli` - `npx vitest run test/usage-notice.test.js test/cli.test.js test/install-preflight.test.js` - `npx eslint bin/lib/usage-notice.js test/usage-notice.test.js` - `npx tsc -p jsconfig.json --noEmit` ## Note The full local git hook suite is currently blocked in this environment by an unrelated `src/lib/preflight.test.ts` localhost listen failure (`EPERM` on `127.0.0.1`). I did not change that area in this branch. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Onboarding and installer now require explicit acceptance of third‑party software terms before proceeding; onboarding exits early if consent is not given. * Interactive mode displays a bundled usage notice and prompts users to type "yes"; acceptance is persisted per user. * Non‑interactive installs allow scripted acceptance via a new explicit acceptance mechanism. * **Documentation** * CLI help, installer messages, and docs updated to document the new non‑interactive acceptance option and TTY guidance. * **Tests** * Added and updated tests and e2e scripts to cover acceptance flows and enforcement. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
lakamsani
pushed a commit
to lakamsani/NemoClaw
that referenced
this pull request
Apr 4, 2026
…ing (NVIDIA#1290) ## Summary - show a third-party software notice before `install.sh` launches onboarding and before direct `nemoclaw onboard` - require explicit `yes` in interactive flows and fail immediately on any other answer - require explicit non-interactive acceptance via `--yes-i-accept-third-party-software` or `NEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1` - link users to the official OpenClaw security guidance before continuing - render the security URL as a clickable terminal hyperlink where OSC 8 is supported, while still printing the raw URL - implement the new notice capability in TypeScript under `src/lib` ## Details This is aimed at issue NVIDIA#1213, but the acceptance behavior is stricter than the original issue text. The notice is specifically about third-party software liability and OpenClaw usage, not NemoClaw terms. The acceptance flag and env var are named accordingly. The current notice is intentionally formatted for standard terminal width and uses the approved wording in short, non-wrapping lines. Reference shown to users: - https://docs.openclaw.ai/gateway/security Credit: - builds on prior overlapping notice work in NVIDIA#1222 by @13ernkastel, but extends it to the current acceptance model, non-interactive enforcement, and the shared `install.sh` + `nemoclaw onboard` flow ## Validation - `npm run build:cli` - `npx vitest run test/usage-notice.test.js test/cli.test.js test/install-preflight.test.js` - `npx eslint bin/lib/usage-notice.js test/usage-notice.test.js` - `npx tsc -p jsconfig.json --noEmit` ## Note The full local git hook suite is currently blocked in this environment by an unrelated `src/lib/preflight.test.ts` localhost listen failure (`EPERM` on `127.0.0.1`). I did not change that area in this branch. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Onboarding and installer now require explicit acceptance of third‑party software terms before proceeding; onboarding exits early if consent is not given. * Interactive mode displays a bundled usage notice and prompts users to type "yes"; acceptance is persisted per user. * Non‑interactive installs allow scripted acceptance via a new explicit acceptance mechanism. * **Documentation** * CLI help, installer messages, and docs updated to document the new non‑interactive acceptance option and TTY guidance. * **Tests** * Added and updated tests and e2e scripts to cover acceptance flows and enforcement. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
14 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
install.shlaunches onboarding and before directnemoclaw onboardyesin interactive flows and fail immediately on any other answer--yes-i-accept-third-party-softwareorNEMOCLAW_ACCEPT_THIRD_PARTY_SOFTWARE=1src/libDetails
This is aimed at issue #1213, but the acceptance behavior is stricter than the original issue text.
The notice is specifically about third-party software liability and OpenClaw usage, not NemoClaw terms. The acceptance flag and env var are named accordingly.
The current notice is intentionally formatted for standard terminal width and uses the approved wording in short, non-wrapping lines.
Reference shown to users:
Credit:
install.sh+nemoclaw onboardflowValidation
npm run build:clinpx vitest run test/usage-notice.test.js test/cli.test.js test/install-preflight.test.jsnpx eslint bin/lib/usage-notice.js test/usage-notice.test.jsnpx tsc -p jsconfig.json --noEmitNote
The full local git hook suite is currently blocked in this environment by an unrelated
src/lib/preflight.test.tslocalhost listen failure (EPERMon127.0.0.1). I did not change that area in this branch.Summary by CodeRabbit
New Features
Documentation
Tests