Conversation
Signed-off-by: Tinson Lai <tinsonl@nvidia.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)
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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 docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/brew.yaml`:
- Around line 14-54: Change the brew preset entries that currently use "tls:
terminate" and path-based REST rules to use CONNECT-style full access: for each
host block (github.com, ghcr.io, pkg-containers.githubusercontent.com,
objects.githubusercontent.com, raw.githubusercontent.com) replace the "tls:
terminate" key with "access: full" and remove the method/path rules under
"rules" (path filtering won't work over CONNECT tunneling); keep
protocol/enforcement as needed but ensure the host blocks use "access: full" to
allow CONNECT-tunneled package/registry traffic.
In `@test/policies.test.js`:
- Line 28: Update the tests that assert the list of preset names and
package-manager invariants to account for the newly added "brew" preset: add
"brew" to the expected names array used in the preset-name test (the array that
currently includes "brew" on line shown), increment the related total preset
count assertion (preset-count) to reflect one more preset, and include "brew" in
the package-manager schema guardrail checks (the package-manager invariant tests
that assert access: "full" / CONNECT model). Ensure the same package-manager
validation logic used for other managers is applied to "brew" so it follows the
standardized CONNECT/access: full expectations.
🪄 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: 29ef03ca-0a24-4019-a811-a0744e3a565f
📒 Files selected for processing (2)
nemoclaw-blueprint/policies/presets/brew.yamltest/policies.test.js
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
✨ Thanks for submitting this pull request, which proposes a way to support Homebrew package access in sandboxes by adding a new network policy preset. |
Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
ericksoa
left a comment
There was a problem hiding this comment.
Reviewed: purely additive — new brew preset file and test count bump. No existing behavior modified. access: full is consistent with npm/pypi presets. Low regression risk. LGTM.
<!-- markdownlint-disable MD041 --> ## Summary - Add a `brew` network policy preset that allows scoped HTTPS access to the Homebrew/Linuxbrew package registry domains (formulae.brew.sh, github.com/Homebrew, ghcr.io, and associated GitHub CDN hosts). - This preset only provides default network endpoints — it does not install Homebrew into the sandbox image. The sandbox image is immutable by design; embedding a full Linuxbrew installation would add significant bloat for a tool that not every sandbox needs. - For workflows that require installing system packages, the recommended approach is to customise the OpenShell sandbox policy to allow the necessary network access, rather than baking package managers into the base image. - Since the sandbox runs as a non-root user, the standard Homebrew installer will refuse to run. There is an undocumented workaround for non-root installations discussed in Homebrew/discussions#3386. ## Related Issue Fixes NVIDIA#491 ## Changes - nemoclaw-blueprint/policies/presets/brew.yaml - test/policies.test.js ## Type of Change <!-- Check the one that applies. --> - [x] Code change for a new feature, bug fix, or refactor. - [ ] Code change with doc updates. - [ ] Doc only. Prose changes without code sample modifications. - [ ] Doc only. Includes code sample changes. ## Testing <!-- What testing was done? --> - [x] `npx prek run --all-files` passes (or equivalently `make check`). - [x] `npm test` passes. - [x] `make docs` builds without warnings. (for doc-only changes) ## Checklist ### General - [x] I have read and followed the [contributing guide](https://github.com/NVIDIA/NemoClaw/blob/main/CONTRIBUTING.md). - [x] I have read and followed the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md). (for doc-only changes) ### Code Changes <!-- Skip if this is a doc-only PR. --> - [x] Formatters applied — `npx prek run --all-files` auto-fixes formatting (or `make format` for targeted runs). - [ ] Tests added or updated for new or changed behavior. - [x] No secrets, API keys, or credentials committed. - [ ] Doc pages updated for any user-facing behavior changes (new commands, changed defaults, new features, bug fixes that contradict existing docs). ### Doc Changes <!-- Skip if this PR has no doc changes. --> - [ ] Follows the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md). Try running the `update-docs` agent skill to draft changes while complying with the style guide. For example, prompt your agent with "`/update-docs` catch up the docs for the new changes I made in this PR." - [ ] New pages include SPDX license header and frontmatter, if creating a new page. - [ ] Cross-references and links verified. --- <!-- DCO sign-off (required by CI). Replace with your real name and email. --> Signed-off-by: Tinson Lai <tinsonl@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a Homebrew (Linuxbrew) policy preset enabling package manager access to required external hosts with enforced network rules and an allowlist of Homebrew binaries. * **Tests** * Updated tests to include and validate the newly added brew policy preset. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Tinson Lai <tinsonl@nvidia.com> Co-authored-by: Aaron Erickson 🦞 <aerickson@nvidia.com>
Summary
brewnetwork policy preset that allows scoped HTTPS access to theHomebrew/Linuxbrew package registry domains (formulae.brew.sh, github.com/Homebrew,
ghcr.io, and associated GitHub CDN hosts).
Homebrew into the sandbox image. The sandbox image is immutable by design;
embedding a full Linuxbrew installation would add significant bloat for a
tool that not every sandbox needs.
approach is to customise the OpenShell sandbox policy to allow the
necessary network access, rather than baking package managers into the
base image.
will refuse to run. There is an undocumented workaround for non-root
installations discussed in Homebrew on linux without root access Homebrew/discussions#3386.
Related Issue
Fixes #491
Changes
Type of Change
Testing
npx prek run --all-filespasses (or equivalentlymake check).npm testpasses.make docsbuilds without warnings. (for doc-only changes)Checklist
General
Code Changes
npx prek run --all-filesauto-fixes formatting (ormake formatfor targeted runs).Doc Changes
update-docsagent skill to draft changes while complying with the style guide. For example, prompt your agent with "/update-docscatch up the docs for the new changes I made in this PR."Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
New Features
Tests