Skip to content

fix: quote sandbox name in openshell policy commands - #49

Merged
ericksoa merged 3 commits into
mainfrom
fix/policy-set-sandbox-name
Mar 17, 2026
Merged

fix: quote sandbox name in openshell policy commands#49
ericksoa merged 3 commits into
mainfrom
fix/policy-set-sandbox-name

Conversation

@ericksoa

Copy link
Copy Markdown
Contributor

Summary

Fixes #46 — onboarding fails at step [7/7] with unexpected argument 'mkdir' when applying policy presets.

The sandbox name was interpolated unquoted into shell commands:

// Before (broken)
run(`openshell policy set --policy "${tmpFile}" --wait ${sandboxName}`);
run(`openshell policy get --full ${sandboxName} 2>/dev/null`);

// After (fixed)
run(`openshell policy set --policy "${tmpFile}" --wait "${sandboxName}"`);
run(`openshell policy get --full "${sandboxName}" 2>/dev/null`);

On DGX Spark, prior sudo usage pollutes openshell's shell context, causing stray tokens like mkdir to leak into the argument list.

Changes

  • Extract buildPolicySetCommand() and buildPolicyGetCommand() helpers that properly quote all arguments
  • Add 4 tests verifying quoting behavior
  • All 42 tests pass

Test plan

  • Run npm test — all 42 tests pass
  • Run full onboard on DGX Spark and verify step [7/7] completes
  • Verify nemoclaw <name> policy-add works with preset application

🤖 Generated with Claude Code

The sandbox name was interpolated unquoted into shell commands passed
to openshell policy set/get. This caused argument parsing failures
when the sandbox name or prior shell state introduced unexpected
tokens (e.g. 'mkdir' from DGX Spark sudo context).

Extracts buildPolicySetCommand/buildPolicyGetCommand helpers that
properly quote all arguments, and adds tests to verify.

Fixes #46

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@dnandakumar-nv

dnandakumar-nv commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Nice fix.

One pre-existing bug worth fixing here while we're touching this file: currentPolicy is declared const on line 94 but reassigned on line 136:

  const currentPolicy = parseCurrentPolicy(rawPolicy);            
  // ...                                                                                                                                               
  if (!currentPolicy.includes("version:")) {                      
    currentPolicy = "version: 1\n" + currentPolicy; // TypeError: Assignment to constant variable                                                      
  }                                                                                                                                                    

This will throw at runtime if a sandbox has an existing policy with no version: field. Simple fix is changing const to let on line 94.

@dnandakumar-nv
dnandakumar-nv self-requested a review March 17, 2026 18:14
Resolve merge conflict in bin/lib/policies.js — keep the extracted
buildPolicyGetCommand/buildPolicySetCommand helpers (PR's approach)
over main's inline quoting, since the helpers are tested and exported.
@ericksoa
ericksoa merged commit b9f428d into main Mar 17, 2026
ericksoa added a commit that referenced this pull request Mar 17, 2026
currentPolicy was declared const but reassigned when prepending a
version field to policies missing one. This throws TypeError at
runtime for sandboxes with existing policies that lack a version field.

Spotted by @dnandakumar-nv in #49.
dnandakumar-nv pushed a commit that referenced this pull request Mar 17, 2026
…or (#232)

currentPolicy was declared const but reassigned when prepending a
version field to policies missing one. This throws TypeError at
runtime for sandboxes with existing policies that lack a version field.

Spotted by @dnandakumar-nv in #49.
Ryuketsukami pushed a commit to Ryuketsukami/NemoClaw that referenced this pull request Mar 24, 2026
The sandbox name was interpolated unquoted into shell commands passed
to openshell policy set/get. This caused argument parsing failures
when the sandbox name or prior shell state introduced unexpected
tokens (e.g. 'mkdir' from DGX Spark sudo context).

Extracts buildPolicySetCommand/buildPolicyGetCommand helpers that
properly quote all arguments, and adds tests to verify.

Fixes NVIDIA#46

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ryuketsukami pushed a commit to Ryuketsukami/NemoClaw that referenced this pull request Mar 24, 2026
…or (NVIDIA#232)

currentPolicy was declared const but reassigned when prepending a
version field to policies missing one. This throws TypeError at
runtime for sandboxes with existing policies that lack a version field.

Spotted by @dnandakumar-nv in NVIDIA#49.
jessesanford pushed a commit to jessesanford/NemoClaw that referenced this pull request Mar 24, 2026
The sandbox name was interpolated unquoted into shell commands passed
to openshell policy set/get. This caused argument parsing failures
when the sandbox name or prior shell state introduced unexpected
tokens (e.g. 'mkdir' from DGX Spark sudo context).

Extracts buildPolicySetCommand/buildPolicyGetCommand helpers that
properly quote all arguments, and adds tests to verify.

Fixes NVIDIA#46

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jessesanford pushed a commit to jessesanford/NemoClaw that referenced this pull request Mar 24, 2026
…or (NVIDIA#232)

currentPolicy was declared const but reassigned when prepending a
version field to policies missing one. This throws TypeError at
runtime for sandboxes with existing policies that lack a version field.

Spotted by @dnandakumar-nv in NVIDIA#49.
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
@cv
cv deleted the fix/policy-set-sandbox-name branch June 28, 2026 00:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NemoClaw onboarding fails at step [7/7] — openshell policy set receives unexpected mkdir argument

3 participants