Skip to content

fix: add missing binaries to all policy presets and default policies - #677

Closed
ross-shulyha wants to merge 2 commits into
NVIDIA:mainfrom
ross-shulyha:fix/default-policy-missing-binaries
Closed

fix: add missing binaries to all policy presets and default policies#677
ross-shulyha wants to merge 2 commits into
NVIDIA:mainfrom
ross-shulyha:fix/default-policy-missing-binaries

Conversation

@ross-shulyha

@ross-shulyha ross-shulyha commented Mar 23, 2026

Copy link
Copy Markdown

Closes #676
Related: #19, #356, #585

Summary

  • Add binaries sections to all 9 presets (discord, docker, huggingface, jira, npm, outlook, pypi, slack, telegram) and the default telegram/discord policies in openclaw-sandbox.yaml
  • Switch pypi/npm presets from tls: terminate to access: full for CONNECT tunneling compatibility (same approach as the working github policy in the default config)
  • Add a test that ensures every preset includes a binaries section to prevent regressions

Problem

OpenShell's OPA rego (sandbox-policy.rego) requires both endpoint_allowed AND binary_allowed to match. When binaries is absent, binary_allowed iterates 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

  • Messaging/API presets (telegram, discord, slack, jira, outlook): openclaw, node, python3, curl — these are the binaries that make HTTP requests from agent scripts
  • Package manager presets (pypi, npm): pip/python and npm/node/yarn/npx at both /usr/bin and /usr/local/bin, plus venv paths for pypi
  • Docker preset: docker CLI binaries plus the standard agent set
  • Hugging Face preset: standard agent set plus huggingface-cli, git, and git-remote-http* (for LFS)

Test plan

  • 223/223 tests pass (including new preset binaries validation test)
  • Pre-commit hooks pass (lint-staged, vitest)
  • Pre-push hooks pass (TypeScript + Python type checks)
  • Verified pip install works 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

    • Network policies extended with executable allowlists for common runtimes and tools (OpenClaw, Node.js, Python 3, curl, Docker CLI, npm, npx, yarn, git, huggingface-cli).
    • Several registry/preset endpoints simplified to use full access for registry/auth hosts.
  • Tests

    • Added a preset-schema validation test to ensure all policy presets include a binaries section.

…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>
@coderabbitai

coderabbitai Bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5614ba3e-8430-41fc-b347-5f9b64bb6094

📥 Commits

Reviewing files that changed from the base of the PR and between 9d0b659 and 58fe8a9.

📒 Files selected for processing (1)
  • nemoclaw-blueprint/policies/presets/docker.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
  • nemoclaw-blueprint/policies/presets/docker.yaml

📝 Walkthrough

Walkthrough

Added missing binaries allowlists to default sandbox policies and all policy presets, and simplified several registry/pypi/npm endpoint configs to access: full. A test was added to ensure every preset YAML includes a top-level binaries: section.

Changes

Cohort / File(s) Summary
Sandbox Default Policies
nemoclaw-blueprint/policies/openclaw-sandbox.yaml
Added network_policies.telegram and network_policies.discord binaries allowlists (openclaw, node*, python3*, curl).
Chat/API Presets
nemoclaw-blueprint/policies/presets/discord.yaml, nemoclaw-blueprint/policies/presets/slack.yaml, nemoclaw-blueprint/policies/presets/telegram.yaml, nemoclaw-blueprint/policies/presets/outlook.yaml, nemoclaw-blueprint/policies/presets/jira.yaml
Added binaries allowlists with common tool patterns (/usr/local/bin/openclaw, /usr/local/bin/node*, /usr/bin/node*, /usr/local/bin/python3*, /usr/bin/python3*, /usr/bin/curl).
Registry & Tool Presets
nemoclaw-blueprint/policies/presets/docker.yaml, nemoclaw-blueprint/policies/presets/huggingface.yaml
Added binaries lists (docker*, openclaw, node*, python3*, curl, plus git/huggingface-cli/git-remote-http* where applicable); replaced per-endpoint HTTP rule blocks with access: full for Docker registry endpoints.
Package Manager Presets
nemoclaw-blueprint/policies/presets/npm.yaml, nemoclaw-blueprint/policies/presets/pypi.yaml
Replaced explicit protocol/enforcement/tls/rules with access: full for registries; added binaries entries for npm*/npx*/node*/yarn* and python*/pip* across /usr/bin and /usr/local/bin.
Tests
test/policies.test.js
Added test asserting every preset YAML loaded via policies.loadPreset(p.name) contains a top-level binaries: section.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibble at YAML, neat and small,

I add the binaries so proxies won't stall.
No more 403s, the endpoints sing—
Hoppity hops, let the network spring! 🥕✨

🚥 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 accurately describes the main change: adding missing binaries sections to all policy presets and default policies, addressing the 403 Forbidden issue.
Linked Issues check ✅ Passed The PR fully addresses all coding objectives from issue #676: adds binaries sections to all 9 presets and default policies, switches npm/pypi/docker to access: full for CONNECT tunneling, and adds automated regression test.
Out of Scope Changes check ✅ Passed All changes are in-scope: policy preset updates to fix 403 errors, schema migration from explicit rules to access: full for tunneling compatibility, and a regression test. No unrelated modifications present.
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.

Tip

You can disable sequence diagrams in the walkthrough.

Disable the reviews.sequence_diagrams setting to disable sequence diagrams in the walkthrough.

@coderabbitai coderabbitai Bot left a comment

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.

🧹 Nitpick comments (1)
nemoclaw-blueprint/policies/presets/docker.yaml (1)

44-52: Binaries section correctly addresses the missing binary_allowed requirement.

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: full instead of tls: terminate. The pre-existing endpoint config (lines 12-43) uses tls: terminate, which may still cause issues for docker pull operations 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

📥 Commits

Reviewing files that changed from the base of the PR and between ffa1283 and 9d0b659.

📒 Files selected for processing (11)
  • nemoclaw-blueprint/policies/openclaw-sandbox.yaml
  • nemoclaw-blueprint/policies/presets/discord.yaml
  • nemoclaw-blueprint/policies/presets/docker.yaml
  • nemoclaw-blueprint/policies/presets/huggingface.yaml
  • nemoclaw-blueprint/policies/presets/jira.yaml
  • nemoclaw-blueprint/policies/presets/npm.yaml
  • nemoclaw-blueprint/policies/presets/outlook.yaml
  • nemoclaw-blueprint/policies/presets/pypi.yaml
  • nemoclaw-blueprint/policies/presets/slack.yaml
  • nemoclaw-blueprint/policies/presets/telegram.yaml
  • test/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>
@wscurran

Copy link
Copy Markdown
Contributor

Thanks for submitting this PR, adding missing binaries to all policy presets and default policies is a great way to improve security and prevent unauthorized access.

@wscurran
wscurran requested a review from drobison00 March 23, 2026 20:11
jestyr27 added a commit to uhstray-io/NemoClaw that referenced this pull request Mar 24, 2026
The upstream discord preset uses tls: terminate with method/path rules,
which causes the OpenShell proxy to return 403 on HTTPS CONNECT tunnels.
Switch to access: full following the pattern from PR NVIDIA#677.

Fixes the "fetch failed" error when OpenClaw tries to reach Discord API.
See: NVIDIA#685
@drobison00

Copy link
Copy Markdown

Duplicates #317

@drobison00 drobison00 closed this Mar 24, 2026
@wscurran wscurran added the bug-fix PR fixes a bug or regression label Jun 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All policy presets and default telegram/discord policies return 403 due to missing binaries section

3 participants