Skip to content

fix(security): add --dry-run flag to policy-add for egress disclosure #1276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
brandonpelfrey merged 3 commits into from
Apr 8, 2026
Merged

fix(security): add --dry-run flag to policy-add for egress disclosure #1276

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9893f04
fix(security): add --dry-run flag to policy-add for egress disclosure
fdzdev Apr 1, 2026
8a7ce81
Merge branch 'main' into fix/policy-apply-disclosure
brandonpelfrey Apr 8, 2026
fe7981b
fix(cli): honor --dry-run for policy-add
brandonpelfrey Apr 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 17 additions & 3 deletions bin/nemoclaw.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1077,14 +1077,28 @@ function sandboxLogs(sandboxName, follow) {
exitWithSpawnResult(result);
}

async function sandboxPolicyAdd(sandboxName) {
async function sandboxPolicyAdd(sandboxName, args = []) {
const dryRun = args.includes("--dry-run");
const allPresets = policies.listPresets();
const applied = policies.getAppliedPresets(sandboxName);

const { prompt: askPrompt } = require("./lib/credentials");
const answer = await policies.selectFromList(allPresets, { applied });
if (!answer) return;

const presetContent = policies.loadPreset(answer);
if (!presetContent) return;

const endpoints = policies.getPresetEndpoints(presetContent);
if (endpoints.length > 0) {
console.log(` Endpoints that would be opened: ${endpoints.join(", ")}`);
}

if (dryRun) {
console.log(" --dry-run: no changes applied.");
return;
}
Comment on lines +1089 to +1100

@coderabbitai coderabbitai Bot Apr 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Silent failure when preset name is invalid provides no user feedback.

When loadPreset returns null (nonexistent or invalid preset name), the function silently returns without informing the user. This is confusing UX—the user won't know why nothing happened.

Proposed fix to add error feedback 
   const presetContent = policies.loadPreset(answer);
-  if (!presetContent) return;
+  if (!presetContent) {
+    console.error(`  Unknown preset: ${answer}`);
+    return;
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const presetContent = policies.loadPreset(answer);
if (!presetContent) return;
const endpoints = policies.getPresetEndpoints(presetContent);
if (endpoints.length > 0) {
console.log(` Endpoints that would be opened: ${endpoints.join(", ")}`);
}
if (dryRun) {
console.log(" --dry-run: no changes applied.");
return;
}
const presetContent = policies.loadPreset(answer);
if (!presetContent) {
console.error(` Unknown preset: ${answer}`);
return;
}
const endpoints = policies.getPresetEndpoints(presetContent);
if (endpoints.length > 0) {
console.log(` Endpoints that would be opened: ${endpoints.join(", ")}`);
}
if (dryRun) {
console.log(" --dry-run: no changes applied.");
return;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@bin/nemoclaw.js` around lines 689 - 700, When policies.loadPreset(answer)
returns null/undefined (presetContent falsy), the script currently returns
silently; change the behavior in the block using loadPreset/presetContent so
that if presetContent is falsy you print a clear error (e.g., "Preset '{answer}'
not found" or "Invalid preset: {answer}") to stderr or console.error and exit
with a non-zero code (or return an error status) instead of silently returning;
ensure this check happens before calling policies.getPresetEndpoints and
preserves the existing dryRun handling and messaging flow.

@brandonpelfrey brandonpelfrey Apr 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fdzdev Let's add a warning that no preset was found by that name

@coderabbitai coderabbitai Bot Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


const confirm = await askPrompt(` Apply '${answer}' to sandbox '${sandboxName}'? [Y/n]: `);
if (confirm.toLowerCase() === "n") return;

Expand Down Expand Up @@ -1179,7 +1193,7 @@ function help() {
nemoclaw <name> destroy Stop NIM + delete sandbox ${D}(--yes to skip prompt)${R}

${G}Policy Presets:${R}
nemoclaw <name> policy-add Add a network or filesystem policy preset
nemoclaw <name> policy-add Add a network or filesystem policy preset ${D}(--dry-run to preview)${R}
nemoclaw <name> policy-list List presets ${D}(● = applied)${R}

${G}Compatibility Commands:${R}
Expand Down Expand Up @@ -1285,7 +1299,7 @@ const [cmd, ...args] = process.argv.slice(2);
sandboxLogs(cmd, actionArgs.includes("--follow"));
break;
case "policy-add":
await sandboxPolicyAdd(cmd);
await sandboxPolicyAdd(cmd, actionArgs);
break;
case "policy-list":
sandboxPolicyList(cmd);
Expand Down
23 changes: 18 additions & 5 deletions test/policies.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const SELECT_FROM_LIST_ITEMS = [
{ name: "pypi", description: "Python Package Index (PyPI) access" },
];

function runPolicyAdd(confirmAnswer) {
function runPolicyAdd(confirmAnswer, extraArgs = []) {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-policy-add-"));
const scriptPath = path.join(tmpDir, "policy-add-check.js");
const script = String.raw`
Expand All @@ -28,6 +28,8 @@ const policies = require(${POLICIES_PATH});
const credentials = require(${CREDENTIALS_PATH});
const calls = [];
policies.selectFromList = async () => "pypi";
policies.loadPreset = () => "network_policies:\n pypi:\n host: pypi.org\n";
policies.getPresetEndpoints = () => ["pypi.org"];
credentials.prompt = async (message) => {
calls.push({ type: "prompt", message });
return ${JSON.stringify(confirmAnswer)};
Expand All @@ -42,10 +44,10 @@ policies.getAppliedPresets = () => [];
policies.applyPreset = (sandboxName, presetName) => {
calls.push({ type: "apply", sandboxName, presetName });
};
process.argv = ["node", "nemoclaw.js", "test-sandbox", "policy-add"];
process.argv = ["node", "nemoclaw.js", "test-sandbox", "policy-add", ...${JSON.stringify(extraArgs)}];
require(${CLI_PATH});
setImmediate(() => {
process.stdout.write(JSON.stringify(calls));
process.stdout.write("\n__CALLS__" + JSON.stringify(calls));
});
`;

Expand Down Expand Up @@ -658,7 +660,7 @@ describe("policies", () => {
const result = runPolicyAdd("y");

expect(result.status).toBe(0);
const calls = JSON.parse(result.stdout.trim());
const calls = JSON.parse(result.stdout.split("__CALLS__")[1].trim());
expect(calls).toContainEqual({
type: "prompt",
message: " Apply 'pypi' to sandbox 'test-sandbox'? [Y/n]: ",
Expand All @@ -674,12 +676,23 @@ describe("policies", () => {
const result = runPolicyAdd("n");

expect(result.status).toBe(0);
const calls = JSON.parse(result.stdout.trim());
const calls = JSON.parse(result.stdout.split("__CALLS__")[1].trim());
expect(calls).toContainEqual({
type: "prompt",
message: " Apply 'pypi' to sandbox 'test-sandbox'? [Y/n]: ",
});
expect(calls.some((call) => call.type === "apply")).toBeFalsy();
});

it("does not prompt or apply when --dry-run is passed", () => {
const result = runPolicyAdd("y", ["--dry-run"]);

expect(result.status).toBe(0);
const calls = JSON.parse(result.stdout.split("__CALLS__")[1].trim());
expect(calls.some((call) => call.type === "prompt")).toBeFalsy();
expect(calls.some((call) => call.type === "apply")).toBeFalsy();
expect(result.stdout).toMatch(/Endpoints that would be opened: pypi\.org/);
expect(result.stdout).toMatch(/--dry-run: no changes applied\./);
});
});
});
Loading