Repository navigation
fix(security): add binaries restrictions to all messaging and preset policies #645
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.
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
Changes from 3 commits
d3cd6df
77c03c3
ecb2da2
cad8cd0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,3 +23,7 @@ network_policies: | |||||||||||
| tls: terminate | ||||||||||||
| rules: | ||||||||||||
| - allow: { method: GET, path: "/**" } | ||||||||||||
| binaries: | ||||||||||||
| - { path: /usr/local/bin/pip } | ||||||||||||
| - { path: /usr/local/bin/pip3 } | ||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same wrapper-script concern as npm —
Suggested change
|
||||||||||||
| - { path: /usr/local/bin/python3 } | ||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,64 @@ | ||||||
| // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. | ||||||
| // SPDX-License-Identifier: Apache-2.0 | ||||||
|
|
||||||
| import { describe, it, expect } from "vitest"; | ||||||
| import fs from "node:fs"; | ||||||
| import path from "node:path"; | ||||||
|
|
||||||
| const BASELINE = path.join(import.meta.dirname, "..", "nemoclaw-blueprint", "policies", "openclaw-sandbox.yaml"); | ||||||
| const PRESETS_DIR = path.join(import.meta.dirname, "..", "nemoclaw-blueprint", "policies", "presets"); | ||||||
|
|
||||||
| describe("binaries restriction: baseline policy", () => { | ||||||
| it("every network_policies entry has a binaries section", () => { | ||||||
| // Parse YAML manually (no yaml dependency) — find all top-level keys under network_policies | ||||||
| // and verify each has a "binaries:" line within its block | ||||||
| const yaml = fs.readFileSync(BASELINE, "utf-8"); | ||||||
| const lines = yaml.split("\n"); | ||||||
| let inNetworkPolicies = false; | ||||||
| let currentBlock = null; | ||||||
| const blocks = []; | ||||||
|
|
||||||
| for (let i = 0; i < lines.length; i++) { | ||||||
| const line = lines[i]; | ||||||
| if (/^network_policies:/.test(line)) { inNetworkPolicies = true; continue; } | ||||||
| if (inNetworkPolicies && /^\S/.test(line) && line.trim() !== "") { | ||||||
| if (currentBlock) blocks.push(currentBlock); | ||||||
| currentBlock = null; | ||||||
| inNetworkPolicies = false; | ||||||
| continue; | ||||||
| } | ||||||
| if (!inNetworkPolicies) continue; | ||||||
| // Top-level entry under network_policies (2-space indent, not a comment) | ||||||
| if (/^ (?!#)\S.*:\s*$/.test(line)) { | ||||||
| if (currentBlock) blocks.push(currentBlock); | ||||||
| currentBlock = { name: line.trim().replace(/:$/, ""), startLine: i + 1, lines: [line] }; | ||||||
| continue; | ||||||
| } | ||||||
| if (currentBlock) currentBlock.lines.push(line); | ||||||
| } | ||||||
| if (currentBlock) blocks.push(currentBlock); | ||||||
|
|
||||||
| expect(blocks.length).toBeGreaterThan(0); | ||||||
|
|
||||||
| const violators = blocks.filter(b => !b.lines.some(l => /^\s+binaries:/.test(l))); | ||||||
|
|
||||||
| expect(violators.map(b => b.name)).toEqual([]); | ||||||
| }); | ||||||
| }); | ||||||
|
|
||||||
| describe("binaries restriction: policy presets", () => { | ||||||
| it("every preset YAML has a binaries section", () => { | ||||||
| const presets = fs.readdirSync(PRESETS_DIR).filter(f => f.endsWith(".yaml")); | ||||||
| expect(presets.length).toBeGreaterThan(0); | ||||||
|
|
||||||
| const missing = []; | ||||||
| for (const file of presets) { | ||||||
| const content = fs.readFileSync(path.join(PRESETS_DIR, file), "utf-8"); | ||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This substring check would pass if
Suggested change
|
||||||
| if (!/^\s+binaries:\s*$/m.test(content)) { | ||||||
| missing.push(file); | ||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| expect(missing).toEqual([]); | ||||||
| }); | ||||||
| }); | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm,yarn, andnpxare shell scripts / symlinks that spawnnodeunder the hood. If the sandbox checks the actual process making the syscall, these entries won't match —nodeis the real caller. Same concern applies topip/pip3in the pypi preset (they're Python wrappers).Can you verify how the sandbox engine resolves the binary? If it checks the top-level process, these are fine. If it checks the process making the network call, we need
nodeandpython3here instead (or in addition).Also,
npxuses the same registries and should be included: