fix(onboard): reject sandbox names that collide with global CLI commands#1773
Conversation
A sandbox named 'status', 'list', or 'debug' becomes inaccessible because the CLI router matches global commands before sandbox names. Add a reserved-name check in promptValidatedSandboxName that rejects names matching any global command and prompts the user to choose a different name. Closes NVIDIA#1731 Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/onboard.ts (1)
2244-2258:⚠️ Potential issue | 🟡 MinorAvoid showing RFC-1123 format errors for reserved names.
When
RESERVED_NAMES.has(sandboxName)is true, execution falls through and also prints the generic invalid-format message at Line 2252-Line 2258. That creates conflicting guidance for a syntactically valid-but-reserved name.Suggested fix
if (/^[a-z]([a-z0-9-]*[a-z0-9])?$/.test(sandboxName)) { // Reject names that collide with global CLI commands. @@ if (RESERVED_NAMES.has(sandboxName)) { console.error(` Reserved name: '${sandboxName}' is a NemoClaw CLI command.`); console.error(" Choose a different name to avoid routing conflicts."); + if (isNonInteractive()) { + process.exit(1); + } + if (attempt < MAX_ATTEMPTS - 1) { + console.error(" Please try again.\n"); + } + continue; } else { return sandboxName; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 2244 - 2258, The reserved-name branch logs that the name is a CLI command but then falls through and also prints the generic "Invalid sandbox name" format errors; update the RESERVED_NAMES handling so it exits the validation early (e.g., after the two console.error calls inside the if (RESERVED_NAMES.has(sandboxName)) block) by returning a sentinel (undefined/null) or otherwise breaking out (or continue if inside a loop) so the subsequent invalid-format checks for sandboxName are not executed; refer to RESERVED_NAMES and sandboxName in the validation block in src/lib/onboard.ts and ensure the branch stops further validation messages.
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
2239-2243: Prefer a single source of truth for reserved command names.
RESERVED_NAMESduplicates command definitions fromsrc/nemoclaw.ts(GLOBAL_COMMANDS, Line 72-Line 88 in the provided snippet). This will drift when new global commands are added.Refactor direction
- const RESERVED_NAMES = new Set([ - "onboard", "list", "deploy", "setup", "setup-spark", - "start", "stop", "status", "debug", "uninstall", - "credentials", "help", - ]); + const RESERVED_NAMES = GLOBAL_COMMANDS_FOR_SANDBOX_NAMES;Move the command list to a shared module (e.g.,
src/lib/cli-commands.ts) and consume it from both command routing and onboarding validation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 2239 - 2243, RESERVED_NAMES in onboard.ts duplicates the global command list defined as GLOBAL_COMMANDS; extract the canonical array/set of command names into a single exported constant (e.g., CLI_COMMAND_NAMES) in a new shared module and replace both RESERVED_NAMES and the source of GLOBAL_COMMANDS to import that single constant, then update onboarding validation to build a Set from that import (or export the Set directly) so both command routing (GLOBAL_COMMANDS) and onboarding use the same source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/onboard.ts`:
- Around line 2244-2258: The reserved-name branch logs that the name is a CLI
command but then falls through and also prints the generic "Invalid sandbox
name" format errors; update the RESERVED_NAMES handling so it exits the
validation early (e.g., after the two console.error calls inside the if
(RESERVED_NAMES.has(sandboxName)) block) by returning a sentinel
(undefined/null) or otherwise breaking out (or continue if inside a loop) so the
subsequent invalid-format checks for sandboxName are not executed; refer to
RESERVED_NAMES and sandboxName in the validation block in src/lib/onboard.ts and
ensure the branch stops further validation messages.
---
Nitpick comments:
In `@src/lib/onboard.ts`:
- Around line 2239-2243: RESERVED_NAMES in onboard.ts duplicates the global
command list defined as GLOBAL_COMMANDS; extract the canonical array/set of
command names into a single exported constant (e.g., CLI_COMMAND_NAMES) in a new
shared module and replace both RESERVED_NAMES and the source of GLOBAL_COMMANDS
to import that single constant, then update onboarding validation to build a Set
from that import (or export the Set directly) so both command routing
(GLOBAL_COMMANDS) and onboarding use the same source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e9e9f25c-4921-4c75-8649-b3e327abc299
📒 Files selected for processing (1)
src/lib/onboard.ts
When a reserved name like 'status' was rejected, execution fell through to the generic 'Invalid sandbox name' error, showing conflicting guidance. Now reserved names exit immediately in non-interactive mode, show 'Please try again' in interactive mode, and skip the generic error. Signed-off-by: latenighthackathon <latenighthackathon@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/onboard.ts (1)
2239-2243: Centralize the reserved command list to prevent drift.
RESERVED_NAMESduplicates the router’s global command source (src/nemoclaw.ts,GLOBAL_COMMANDS). If one list changes without the other, onboarding can regress and allow new collisions again. Prefer sharing one exported constant/module used by both paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/onboard.ts` around lines 2239 - 2243, RESERVED_NAMES duplicates GLOBAL_COMMANDS; export a single shared constant (e.g., export const GLOBAL_COMMANDS or RESERVED_COMMANDS) from a common module and import it where needed instead of redefining RESERVED_NAMES in onboard.ts; update references in onboard.ts to use the imported symbol (RESERVED_NAMES -> GLOBAL_COMMANDS or vice versa) and remove the local Set declaration so both the router and onboarding use the same source of truth.
🤖 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/onboard.ts`:
- Around line 2239-2243: RESERVED_NAMES duplicates GLOBAL_COMMANDS; export a
single shared constant (e.g., export const GLOBAL_COMMANDS or RESERVED_COMMANDS)
from a common module and import it where needed instead of redefining
RESERVED_NAMES in onboard.ts; update references in onboard.ts to use the
imported symbol (RESERVED_NAMES -> GLOBAL_COMMANDS or vice versa) and remove the
local Set declaration so both the router and onboarding use the same source of
truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e44f9a2d-c5bc-4245-8463-f4ca569dfeb7
📒 Files selected for processing (1)
src/lib/onboard.ts
|
✨ Thanks for submitting this PR, which proposes a fix for a bug where sandbox names that match global CLI commands are not rejected during onboarding . Possibly related open issues: |
## Summary - Document tier-based policy selector (Restricted/Balanced/Open) in commands, network policies, and customize-network-policy pages (from #1753) - Document configurable port overrides via environment variables (`NEMOCLAW_GATEWAY_PORT`, `NEMOCLAW_DASHBOARD_PORT`, `NEMOCLAW_VLLM_PORT`, `NEMOCLAW_OLLAMA_PORT`) (from #1645) - Document `nemoclaw <sandbox> skill install <path>` command (from #1845, #1856) - Document reserved sandbox name validation — CLI command collision check (from #1773) - Bump doc version switcher through 0.0.15 - Remove `--dangerously-skip-permissions` from onboard usage synopsis (docs-skip violation) - Regenerate agent skills from updated docs ## Test plan - [x] `make docs` builds without warnings - [x] All pre-commit hooks pass - [ ] Verify rendered pages in docs build output - [ ] Cross-references resolve correctly (`policy-tiers` anchor, `environment-variables` section) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Sandbox names that match global CLI commands (e.g. 'status', 'list', 'debug') are now rejected during onboarding with a clear error message.
Problem
If a user names their sandbox 'status', running
nemoclaw status connectroutes to the globalstatuscommand instead of the sandbox. The sandbox becomes inaccessible via normal CLI workflows since the global command always takes routing priority.Fix
Add a reserved-name check in
promptValidatedSandboxName()after RFC 1123 validation passes. Names matching any entry in the global commands set (onboard, list, deploy, setup, start, stop, status, debug, uninstall, credentials, help) are rejected with a prompt to choose a different name. In non-interactive mode, this exits with code 1 (existing behavior for invalid names).Test plan
npm run build:clipassesnpm run typecheck:clipassesnpm run lintpassesCloses #1731
Signed-off-by: latenighthackathon latenighthackathon@users.noreply.github.com
Summary by CodeRabbit