fix: add missing binaries to all policy presets and default policies#677
fix: add missing binaries to all policy presets and default policies#677ross-shulyha wants to merge 2 commits intoNVIDIA:mainfrom
Conversation
…discord policies All 9 presets and the default telegram/discord entries in openclaw-sandbox.yaml lacked binaries sections, causing OpenShell's OPA rego to deny every request with 403 regardless of endpoint match. - Add binaries to all presets (discord, docker, huggingface, jira, npm, outlook, pypi, slack, telegram) - Add binaries to default telegram and discord policies - Switch pypi/npm from tls:terminate to access:full for CONNECT tunneling compatibility - Add test ensuring every preset includes a binaries section Closes NVIDIA#676 Related: NVIDIA#19, NVIDIA#356, NVIDIA#585 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded missing Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
nemoclaw-blueprint/policies/presets/docker.yaml (1)
44-52: Binaries section correctly addresses the missingbinary_allowedrequirement.The list appropriately covers Docker CLI binaries (
docker*) plus standard agent binaries (openclaw, node, python3, curl) per the PR objectives.One operational note: Based on learnings, container registry presets that use CONNECT tunneling typically require
access: fullinstead oftls: terminate. The pre-existing endpoint config (lines 12-43) usestls: terminate, which may still cause issues fordocker pulloperations that rely on CONNECT. Consider verifying this works end-to-end or addressing the TLS config in a follow-up if pulls fail.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nemoclaw-blueprint/policies/presets/docker.yaml` around lines 44 - 52, The endpoint for the docker/container registry preset currently uses "tls: terminate", which can break Docker CLI operations that rely on CONNECT tunneling; update the endpoint TLS configuration to use "access: full" instead of "tls: terminate" (or add an environment-specific override) so docker pull/push via CONNECT works end-to-end — look for the preset's endpoint block that pairs with the "binaries" entries (docker* paths) and replace "tls: terminate" with "access: full" (or document/flag it as a follow-up if you prefer validation before changing).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@nemoclaw-blueprint/policies/presets/docker.yaml`:
- Around line 44-52: The endpoint for the docker/container registry preset
currently uses "tls: terminate", which can break Docker CLI operations that rely
on CONNECT tunneling; update the endpoint TLS configuration to use "access:
full" instead of "tls: terminate" (or add an environment-specific override) so
docker pull/push via CONNECT works end-to-end — look for the preset's endpoint
block that pairs with the "binaries" entries (docker* paths) and replace "tls:
terminate" with "access: full" (or document/flag it as a follow-up if you prefer
validation before changing).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2714c174-43dc-43dc-8a5e-b9ed62335518
📒 Files selected for processing (11)
nemoclaw-blueprint/policies/openclaw-sandbox.yamlnemoclaw-blueprint/policies/presets/discord.yamlnemoclaw-blueprint/policies/presets/docker.yamlnemoclaw-blueprint/policies/presets/huggingface.yamlnemoclaw-blueprint/policies/presets/jira.yamlnemoclaw-blueprint/policies/presets/npm.yamlnemoclaw-blueprint/policies/presets/outlook.yamlnemoclaw-blueprint/policies/presets/pypi.yamlnemoclaw-blueprint/policies/presets/slack.yamlnemoclaw-blueprint/policies/presets/telegram.yamltest/policies.test.js
Docker pull/push operations use CONNECT tunneling through the proxy, which breaks under tls: terminate. Switch to access: full, matching the approach used for pypi and npm presets. Addresses CodeRabbit review feedback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Closes #676
Related: #19, #356, #585
Summary
binariessections to all 9 presets (discord, docker, huggingface, jira, npm, outlook, pypi, slack, telegram) and the default telegram/discord policies inopenclaw-sandbox.yamltls: terminatetoaccess: fullfor CONNECT tunneling compatibility (same approach as the workinggithubpolicy in the default config)binariessection to prevent regressionsProblem
OpenShell's OPA rego (
sandbox-policy.rego) requires bothendpoint_allowedANDbinary_allowedto match. Whenbinariesis absent,binary_allowediterates over zero candidates and always returns false — so the proxy returns 403 Forbidden for every request, even when the endpoint matches the policy.PR #356 identified this for pypi/npm, but the same bug affects all 9 presets and the default telegram/discord entries. This PR provides a comprehensive fix.
Binary selection rationale
/usr/binand/usr/local/bin, plus venv paths for pypihuggingface-cli,git, andgit-remote-http*(for LFS)Test plan
binariesvalidation test)pip installworks inside sandbox after applying pypi preset with this fix (tested on macOS M4 Max with openshell 0.0.11, gateway 0.0.12-dev.6)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests