feat(presets): add SidClaw agent governance preset - #1044
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughA new YAML policy preset file for Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nemoclaw-blueprint/policies/presets/sidclaw.yaml`:
- Around line 4-6: The new preset entry "sidclaw" was added (preset.name =
sidclaw), so update the preset-list test assertions in policies.test.js: add
"sidclaw" to the expected names array/list and increment the expected total
count (or update the snapshot) so the test that verifies preset names/count
passes; ensure any ordering-sensitive assertion matches the actual order used by
the preset loader.
- Around line 24-26: The binaries allowlist in the preset currently whitelists
full interpreters via the "binaries" section (entries for /usr/local/bin/node
and /usr/bin/python3), which is too broad; update the preset to remove these
generic interpreter paths and instead whitelist only the minimal, explicit
executables or wrapper scripts the preset actually needs (e.g., specific CLI
binaries, application launchers, or hashed file entries) so that merging presets
won’t unintentionally grant broad execution surface—locate the "binaries" list
in sidclaw.yaml and replace the interpreter entries with targeted binaries or
remove them entirely if not required.
- Around line 20-21: The policy currently grants write access to approval
decisions via the two entries allow: { method: POST, path:
"/api/v1/approvals/*/approve" } and allow: { method: POST, path:
"/api/v1/approvals/*/deny" } in sidclaw.yaml which is broader than intended;
remove these two POST allow rules (or replace them with a tightly scoped rule
limited to an admin/service role or additional attribute checks) so only the
intended polling GET routes remain (e.g., keep GET /api/v1/approvals/**) and
ensure any approval/deny mutation is only permitted for an explicitly authorized
role.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d6b1f957-a789-42ff-8133-242c032c56ac
📒 Files selected for processing (1)
nemoclaw-blueprint/policies/presets/sidclaw.yaml
|
✨ Thanks for submitting this proposal with a detailed summary, it addresses the addition of a network policy preset for SidClaw, an agent governance platform that could improve the security and governance of tools running inside NemoClaw sandboxes. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/policies.test.js (1)
10-13: Reduce redundant count assertion maintenance.This count check duplicates the stronger name-list assertion below and requires a second manual update whenever a preset is added/removed. Consider deriving the count from the expected names list or removing this test to avoid drift.
♻️ Optional simplification
- it("returns all 10 presets", () => { - const presets = policies.listPresets(); - expect(presets.length).toBe(10); - }); + it("returns all expected presets", () => { + const names = policies + .listPresets() + .map((p) => p.name) + .sort(); + const expected = [ + "discord", + "docker", + "huggingface", + "jira", + "npm", + "outlook", + "pypi", + "sidclaw", + "slack", + "telegram", + ]; + expect(names).toEqual(expected); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/policies.test.js` around lines 10 - 13, The test asserts a hardcoded count for policies.listPresets() which duplicates the stronger name-list assertion and will drift when presets change; update the test to derive the expected length from the expected names array (e.g., use expectedNames.length when asserting presets.length) or remove the redundant "returns all 10 presets" test entirely so maintenance only occurs in the single canonical name-list assertion that checks listPresets().
🤖 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/policies.test.js`:
- Around line 10-13: The test asserts a hardcoded count for
policies.listPresets() which duplicates the stronger name-list assertion and
will drift when presets change; update the test to derive the expected length
from the expected names array (e.g., use expectedNames.length when asserting
presets.length) or remove the redundant "returns all 10 presets" test entirely
so maintenance only occurs in the single canonical name-list assertion that
checks listPresets().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02a6f694-e4d4-4b3a-b532-df21241c9859
📒 Files selected for processing (2)
nemoclaw-blueprint/policies/presets/sidclaw.yamltest/policies.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
- nemoclaw-blueprint/policies/presets/sidclaw.yaml
|
Hi @wscurran, thanks for the positive review. All CodeRabbit feedback has been addressed. Is there anything else needed to get this across the line? |
|
I retried the automated JS→TS migration port, but this branch still needs manual follow-up after merging Please start with: git fetch origin
git merge origin/main
npx tsx scripts/ts-migration-assist.ts --base origin/main --write
npm run build:cli
npm run typecheck:cli
npm run lint
npm test |
|
Done — merged What happened:
|
Adds a network policy preset for SidClaw (https://sidclaw.com), an agent governance platform that adds policy evaluation, human-in-the-loop approval, and hash-chain audit trails to tools running inside NemoClaw sandboxes. The preset allows the sandbox to reach the SidClaw API for: - Policy evaluation (POST /api/v1/evaluate) - Approval status polling (GET /api/v1/approvals/**) - Outcome recording (POST /api/v1/traces/*/outcome) - Agent identity resolution (GET /api/v1/agents/**) Usage: Add `sidclaw` to your blueprint's preset list, then use the SidClaw SDK (`@sidclaw/sdk/nemoclaw` or `sidclaw.middleware.nemoclaw`) to wrap tool execution with governance. Docs: https://docs.sidclaw.com/docs/integrations/nemoclaw Signed-off-by: Vladimir <vladimir@vlpetrov.com>
…tests - Remove POST /approve and POST /deny routes from the sidclaw preset. Approval decisions are made by human reviewers on the SidClaw dashboard, not from inside the sandbox. The agent only needs to poll approval status (GET), not send decisions. - Add comment explaining dual-binary (node + python3) rationale: SidClaw ships both TypeScript and Python SDKs. - Update test/policies.test.js: preset count 9 → 10, add "sidclaw" to expected names array. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Vladimir <vladimir@vlpetrov.com>
…ate tests Narrow the binaries allowlist from generic node/python3 interpreters to SidClaw-specific binaries (sidclaw*, sidclaw-mcp-proxy). The approve/deny POST routes and test updates were already addressed in cf40453. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Vladimir <vladimir@vlpetrov.com>
a14d798 to
ae0cf96
Compare
Signed-off-by: Vladimir <vladimir@vlpetrov.com>
|
Thanks for contributing the SidClaw governance preset. We're currently reviewing our approach to third-party integrations in NemoClaw, including scope and support model. Tagging this for evaluation as part of that review — we'll follow up once we have clearer guidance on how we're handling new integration PRs. |
wscurran
added
area: integrations
|
Closing this revision without merge after a current-head merge-gate and 9-category security review. Credit first: @VladUZH, the least-privilege intent here is good. Your follow-up removed sandbox-side approve/deny mutations and replaced the original generic interpreter entries with proposed dedicated SidClaw paths. That narrowing materially improved the proposal and is explicitly credited even though no code is being carried forward. SidClaw is live, not obsolete. The official integration guide is available, PyPI still publishes 0.1.2, and the official npm package has advanced from the 0.1.5 cited in this PR to 0.1.11. The current package still exports
The changed files themselves add no prohibited external repository/community link; I did not use the unofficial example link in the PR body as evidence. A viable future integration would require product/design authority for the supported agent set, SDK-versus-proxy runtime, interpreter-level blast radius, provider-managed credential flow, exact routes, and data handling contract. That is not a safe mechanical salvage of this branch. Thank you again, @VladUZH, for the original least-privilege proposal and the prior narrowing. |
4 participants
Summary
Adds a network policy preset for SidClaw, an agent governance platform that adds policy evaluation, human-in-the-loop approval, and hash-chain audit trails to tools running inside NemoClaw sandboxes.
NemoClaw secures the sandbox (network, filesystem, process). SidClaw governs what happens inside it (policy, approval, audit).
What this preset enables
The
sidclawpreset allows the sandbox to reach the SidClaw API (api.sidclaw.com:443) for:POST /api/v1/evaluate— every tool call checked against governance policiesGET /api/v1/approvals/**— wait for human approval on high-risk actionsPOST /api/v1/traces/*/outcome— tamper-proof audit trailGET /api/v1/agents/**— resolve agent permissionsUsage
Add
sidclawto your blueprint's preset list, then use the SidClaw SDK:Links
Summary by CodeRabbit