fix(cli): validate deploy instance names before setup prompts by Saibabu7770 · Pull Request #604 · NVIDIA/NemoClaw

Saibabu7770 · 2026-03-21T20:48:21Z

Fixes #575.

Summary

Validate nemoclaw deploy <instance-name> input before credential/setup steps.
This fails fast on malformed names and prevents API key prompt for invalid input.

Changes

Validate instanceName at start of deploy() in bin/nemoclaw.js
Exit with clear error when name is invalid
Add CLI regression tests in test/cli.test.js for:
- foo;bar
- $(id)

Testing

node --test --test-name-pattern "deploy rejects" test/cli.test.js
Manual check: invalid name is rejected immediately
Manual check: valid name proceeds past validation

Summary by CodeRabbit

Bug Fixes
- Deployment now validates instance names immediately, rejects invalid names with a clear error message, and exits early without prompting for credentials.
Tests
- Added CLI tests ensuring malformed instance names (including shell metacharacters) fail early, emit the expected error, and do not trigger credential prompts.

coderabbitai · 2026-03-21T20:48:36Z

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

The deploy(instanceName) function in bin/nemoclaw.js now performs instance-name validation inside a guarded try/catch immediately after the non-empty check and exits with code 1 on validation failure. Two CLI tests were added to test/cli.test.js asserting names with shell metacharacters are rejected before any credential prompt.

Changes

Cohort / File(s)	Summary
Deploy input validation `bin/nemoclaw.js`	Move `validateName(instanceName, "instance name")` into a `try/catch` immediately after the non-empty check; on failure log `err.message` and call `process.exit(1)`; remove unguarded validation and use the validated `name` for subsequent shell invocations.
CLI tests for validation `test/cli.test.js`	Add two CLI dispatch tests passing instance names with shell metacharacters (`"foo;bar"`, `"$(id)"`) expecting exit code `1`, an "Invalid instance name" error message, and no downstream credential-prompt output.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I sniff a name with cautious nose,
I nudge out tricks where mischief grows.
No rogue semicolon, no secret subs,
I guard the hops and tidy the hubs.
Hop—deploy is safe, no slippery snubs.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(cli): validate deploy instance names before setup prompts' accurately summarizes the main change: adding instance-name validation before credential prompts in the deploy command.
Linked Issues check	✅ Passed	The PR successfully implements the primary objective from issue `#575`: validate instance names before shell command execution to prevent command injection, adding input validation and early exit for malformed names.
Out of Scope Changes check	✅ Passed	All changes are scoped to instance-name validation: bin/nemoclaw.js adds validation logic and test/cli.test.js adds regression tests covering shell metacharacters, both directly addressing issue `#575`.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 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/cli.test.js`:
- Around line 102-104: The test "deploy rejects command-substitution instance
names" is currently using double quotes so the shell expands $(id) before it
reaches the CLI; update the test to pass a literal $(id) by changing the call to
run in that test (the run(...) invocation in test/cli.test.js) to use single
quotes around $(id) (i.e., run("deploy '$(id)'") ) so the shell does not perform
command substitution and the CLI receives the raw payload to validate.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 35943e3b-b885-4659-bc54-40ff08c07eaf

📥 Commits

Reviewing files that changed from the base of the PR and between 255e606 and d23b193.

📒 Files selected for processing (2)

bin/nemoclaw.js
test/cli.test.js

wscurran · 2026-03-23T22:24:18Z

Thanks for the proposed fix to validate deploy instance names before setup prompts, which may help prevent issues with malformed names and improve the overall user experience.

wscurran · 2026-04-14T16:13:23Z

Thanks for this — validating deploy instance names early prevents confusing failures downstream. The CLI has been refactored to TypeScript since March. Could you rebase against main and verify the validation still fits the current deploy flow? Happy to review once it's updated.

Wrap validateName for instance and sandbox in executeDeploy so invalid names exit with code 1 and a clear message instead of throwing. Add unit and CLI regression tests that ensure credentials are not read for bad deploy names. Made-with: Cursor

wscurran · 2026-04-16T20:44:35Z

Thanks for the contribution. Instance name validation for nemoclaw deploy was actually merged in before this PR was opened — validateName is already enforced on the deploy path in main. Closing as the change is already covered.

Feel free to check the current state of src/lib/deploy.ts if you have other improvements in mind.

coderabbitai Bot reviewed Mar 21, 2026

View reviewed changes

Comment thread test/cli.test.js Outdated

wscurran added bug Something fails against expected or documented behavior NemoClaw CLI labels Mar 23, 2026

prekshivyas mentioned this pull request Mar 25, 2026

refactor(cli): migrate to oclif + TypeScript #909

Closed

8 tasks

cv mentioned this pull request Mar 25, 2026

refactor(cli): shell consolidation, TypeScript migration & oclif #924

Open

wscurran mentioned this pull request Mar 30, 2026

MEDIUM: Unvalidated Instance Name in deploy() Shell Commands #575

Closed

wscurran added the status: rebase label Apr 14, 2026

Saibabu7770 force-pushed the fix/deploy-instance-name-validation-575 branch from 7eb2e25 to d12420b Compare April 16, 2026 04:14

wscurran closed this Apr 16, 2026

wscurran added area: cli Command line interface, flags, terminal UX, or output bug-fix PR fixes a bug or regression needs: rebase PR needs rebase or conflict resolution and removed NemoClaw CLI bug Something fails against expected or documented behavior labels Jun 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): validate deploy instance names before setup prompts#604

fix(cli): validate deploy instance names before setup prompts#604
Saibabu7770 wants to merge 1 commit into
NVIDIA:mainfrom
Saibabu7770:fix/deploy-instance-name-validation-575

Saibabu7770 commented Mar 21, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 21, 2026 •

edited

Loading

Reviews paused

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

wscurran commented Mar 23, 2026

Uh oh!

wscurran commented Apr 14, 2026

Uh oh!

wscurran commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Saibabu7770 commented Mar 21, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wscurran commented Mar 23, 2026

Uh oh!

wscurran commented Apr 14, 2026

Uh oh!

wscurran commented Apr 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Saibabu7770 commented Mar 21, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 21, 2026 •

edited

Loading