fix(security): remove npm registry from base sandbox policy (#1458)

[ColinM-sys]

Summary

• Remove the hardcoded npm_registry entry from the base sandbox policy. npm registry access was being granted to every sandbox regardless of opt-in.
• Add a regression test asserting the base policy never re-declares npm_registry or references registry.npmjs.org.

Fixes #1458.

Why

The reporter onboarded a sandbox with zero policy presets and observed that npm install still succeeded inside the sandbox, while pip install was correctly rejected. The root cause: network_policies.npm_registry was hardcoded into the base policy with GET /** on registry.npmjs.org plus the npm/node binaries, while PyPI was only declared in presets/pypi.yaml. Two equivalent package-manager paths, one silently allowed and one correctly gated — exactly the same shape as the GitHub leak fixed in #1583/#1660.

What changed

• nemoclaw-blueprint/policies/openclaw-sandbox.yaml — remove the npm_registry block, leave a comment pointing at the existing presets/npm.yaml and [All platforms][Regression] None policy added but NPM install is able to execute #1458.
• test/validate-blueprint.test.ts — new regression test under #1458 asserting the base policy declares no npm_registry entry and references no registry.npmjs.org host.

Test plan

• npx vitest run test/validate-blueprint.test.ts → 28/28 passing.
• Negative-case verification: stash the YAML edit and re-run — the new regression test fails as expected, then passes again after pop.
• Maintainer should sanity-check that no runtime-time openclaw plugins install flow depends on npm reachability from inside the sandbox. The build-time openclaw plugins install /opt/nemoclaw step in the Dockerfile is a local-path install, not an npm fetch, so it is unaffected by this change.

Summary by CodeRabbit

• Bug Fixes

  • Removed npm registry access from the base sandbox policy to prevent unintended npm installs in sandboxes that haven't explicitly selected the npm preset. npm functionality must now be enabled through the dedicated npm preset.

• Tests

  • Added regression test to ensure npm registry access is not present in base sandbox policy.

[coderabbitai]

📝 Walkthrough

Walkthrough

This change removes npm registry network access from the base sandbox policy to fix a regression where npm operations could execute without explicit policy selection. A regression test validates that the npm_registry entry is absent from the base policy configuration.

Changes

Cohort / File(s)	Summary
Policy Configuration nemoclaw-blueprint/policies/openclaw-sandbox.yaml	Removed the npm_registry network policy block that granted access to registry.npmjs.org and npm binaries (/usr/local/bin/npm, /usr/local/bin/node). Replaced with explanatory comment indicating npm access should come from presets/npm.yaml or later via openshell policy set, addressing issue #1458.
Policy Validation Tests test/validate-blueprint.test.ts	Added regression test asserting that network_policies does not contain an npm_registry entry and that no endpoints expose registry.npmjs.org access, ensuring npm registry isolation in base sandbox policy.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A sandbox so clean, with policies tight,
Npm must now earn its invite!
No sneaky registry calls in the base,
Just presets and permissions—everything in place. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: removing npm registry from the base sandbox policy for security reasons, with a direct reference to issue #1458.
Linked Issues check	✅ Passed	The PR removes npm_registry from base policy and adds a regression test ensuring npm registry access is blocked, directly addressing #1458's requirement to block npm registry when no policies are applied.
Out of Scope Changes check	✅ Passed	All changes are scoped to removing npm_registry from base policy and adding a corresponding regression test, with no unrelated modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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]

🧹 Nitpick comments (1)

test/validate-blueprint.test.ts (1)

205-206: Optional hardening: include Yarn registry in the host regression check.

Since npm policy preset covers both npm and Yarn registries, this test can guard both with one matcher to prevent a parallel bypass.

♻️ Suggested diff

-    const npmHosts = findEndpoints((h) => h === "registry.npmjs.org");
-    expect(npmHosts).toEqual([]);
+    const npmOrYarnHosts = findEndpoints(
+      (h) => h === "registry.npmjs.org" || h === "registry.yarnpkg.com",
+    );
+    expect(npmOrYarnHosts).toEqual([]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/validate-blueprint.test.ts` around lines 205 - 206, The test currently
queries findEndpoints with a predicate matching only "registry.npmjs.org"
(stored in npmHosts) which misses Yarn; update the predicate passed to
findEndpoints in validate-blueprint.test.ts to match both registries (e.g.,
check h === "registry.npmjs.org" || h === "registry.yarnpkg.com"), optionally
rename npmHosts to registryHosts for clarity, and keep the
expect(registryHosts).toEqual([]) assertion so the test fails if either npm or
Yarn registries are present.

🤖 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/validate-blueprint.test.ts`:
- Around line 205-206: The test currently queries findEndpoints with a predicate
matching only "registry.npmjs.org" (stored in npmHosts) which misses Yarn;
update the predicate passed to findEndpoints in validate-blueprint.test.ts to
match both registries (e.g., check h === "registry.npmjs.org" || h ===
"registry.yarnpkg.com"), optionally rename npmHosts to registryHosts for
clarity, and keep the expect(registryHosts).toEqual([]) assertion so the test
fails if either npm or Yarn registries are present.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b0d8d15a-1593-444e-b313-11e6bb892087

📥 Commits

Reviewing files that changed from the base of the PR and between e8b30a2 and 6325243.

📒 Files selected for processing (2)

• nemoclaw-blueprint/policies/openclaw-sandbox.yaml
• test/validate-blueprint.test.ts

[wscurran]

✨ Thanks for submitting this PR, which proposes a way to fix the security issue related to npm registry access and may help improve the overall security of the NemoClaw platform.

---

Possibly related open PRs:

• #1660 fix(security): gate GitHub access behind an opt-in policy preset (#1583)

---

Possibly related open issues:

• #1458 [NemoClaw][All platforms][Regression] None policy added but NPM install is able to execute
• #1583 [NemoClaw][All Platform] GitHub policy + binary is injected into sandbox runtime policy without user opt-in

[ColinM-sys]

Closing — the npm_registry entry was addressed in #1672 (merged). My approach removed it from base entirely; theirs restricted it to GET-only. Either way the regression in #1458 is resolved.