fix(presets): add /usr/bin/node to communication preset binaries (#481)#1652
fix(presets): add /usr/bin/node to communication preset binaries (#481)#1652latenighthackathon wants to merge 3 commits intoNVIDIA:mainfrom
Conversation
…DIA#481) The telegram, discord, and slack presets only allowlist /usr/local/bin/node in the binaries field. OPA does exact-path matching, so on sandbox base images that install Node.js to /usr/bin/node (Debian/Ubuntu apt default instead of the manual / nvm path), the L7 proxy silently 403s every outbound HTTPS request from the bot — even though the endpoint policies are correctly applied and showing in 'openshell policy get'. Multiple users in NVIDIA#481 confirmed this on: - WSL2 Ubuntu (Bilguunzayaa, dknos) - DGX Spark ARM/Grace Blackwell (eddie-schick) - Pop!_OS x86_64 (cortexj) - Ubuntu Server 25.10 (b1skit) The original fix attempt NVIDIA#1084 by @cortexj was correct in spirit but also added /usr/bin/curl and /usr/bin/bash without a clear need, plus an unrelated package-lock.json rewrite. Maintainer (kjw3) asked for a narrowed re-spin in a 2026-04-01 review and the PR has been stalled since. This change implements exactly the narrow fix kjw3 directionally approved: just /usr/bin/node, no curl/bash, no lockfile churn. Adds a regression test asserting both /usr/local/bin/node and /usr/bin/node are present in all three communication presets. Closes NVIDIA#481 Signed-off-by: latenighthackathon <[email protected]>
|
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 (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded additional exact-path binary allowlist entries ( 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 unit tests (beta)
Comment |
cv
left a comment
There was a problem hiding this comment.
LGTM — security review PASS.
- Same binary (Node.js), alternate installation path — no new attack surface
- Pattern already established in
brave.yaml - Only communication presets modified (discord, slack, telegram)
- Regression test included
Note: outlook.yaml, jira.yaml, huggingface.yaml likely need the same fix in a follow-up.
…VIDIA#481) cv flagged in review of the original communication-preset fix that outlook, jira, and huggingface presets have the same /usr/bin/node gap. Apply the same narrow fix to those three presets. The huggingface preset additionally gates on python3 for the inference SDK and has the same Debian/Ubuntu vs manual-install path split, so add /usr/bin/python3 alongside /usr/local/bin/python3 there too. Extends the existing regression test to cover all six node-gated presets (telegram, discord, slack, outlook, jira, huggingface) and adds a separate assertion for huggingface python3 paths. Signed-off-by: latenighthackathon <[email protected]>
|
@cv Thanks for the security review and the follow-up flag — pushed a second commit applying the same fix to outlook, jira, and huggingface. The huggingface preset also gates on python3 so I added |
|
Closing in favor of #1653 — that PR includes the same |
Summary
/usr/bin/nodealongside the existing/usr/local/bin/nodein thebinariesallowlist for the telegram, discord, and slack presetsProblem
The
telegram,discord, andslackpresets only allowlist/usr/local/bin/nodein thebinariesfield. OPA does exact-path matching on this list, so on sandbox base images that install Node.js to/usr/bin/node(Debian/Ubuntu apt default, instead of the manual install / nvm path), the L7 proxy silently 403s every outbound HTTPS request from the bot — even though the endpoint policies are correctly applied and showing inopenshell policy get.The user-visible symptom is the bot failing to talk to its API with
CONNECT tunnel failed, response 403, whileopenshell policy getcheerfully reports the preset as applied. The disconnect is hard to diagnose because the binary allowlist is invisible to the user-facing policy view. Multiple users in #481 confirmed this across WSL2 Ubuntu, DGX Spark ARM, Pop!_OS x86_64, and Ubuntu Server.This PR augments #1084 with the maintainer review feedback applied: just
/usr/bin/node, nocurl/bashadditions, nopackage-lock.jsonchurn.Test plan
test/policies.test.jsasserts both/usr/local/bin/nodeand/usr/bin/nodeare present in the binaries section of all three communication presetsnpx vitest run test/policies.test.js) — including the existingextractPresetEntries"works on every real preset file" test that catches preset YAML structural breakageCloses #481
Signed-off-by: latenighthackathon [email protected]
Summary by CodeRabbit
Configuration Updates
Tests