Skip to content

[codex] restore Tavily egress for managed Python - #6133

Closed
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/restore-tavily-python-egress
Closed

[codex] restore Tavily egress for managed Python#6133
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/restore-tavily-python-egress

Conversation

@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Summary

Restore /opt/venv/bin/python3* to the Tavily policy preset and provider-profile binary allowlists, and lock the managed interpreter into both exact allowlist tests.

Root cause

PR #5969 tightened the Tavily binary allowlist by removing Python. Deep Agents Code runs from the managed /opt/venv environment, and OpenShell attributes its Tavily requests to that Python interpreter. As a result, policy-add tavily applied successfully but the actual request remained blocked with 403 Forbidden.

Failed release-gate job: https://github.com/NVIDIA/NemoClaw/actions/runs/28533929538/job/84591431680

The fix intentionally restores only the managed interpreter path. It does not reopen system Python paths.

Changes

  • Add /opt/venv/bin/python3* to the Tavily policy preset.
  • Add /opt/venv/bin/python3* to the Tavily provider profile.
  • Document the OpenShell process-attribution reason beside both allowlists.
  • Update the preset and provider-profile contract tests.

Impact

Deep Agents Code can reach api.tavily.com after the operator explicitly applies the Tavily policy and attaches the Tavily provider, while Tavily remains denied by default and system Python interpreters remain excluded.

Validation

npx vitest run test/tavily-preset.test.ts test/validate-blueprint.test.ts --reporter=default

Test Files  2 passed (2)
Tests       67 passed (67)

A rerun of ubuntu-repo-cloud-langchain-deepagents-code is still required for live validation.

@copy-pr-bot

copy-pr-bot Bot commented Jul 1, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d9f05124-8615-4fa4-9563-cf7172901999

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/restore-tavily-python-egress

Comment @coderabbitai help to get the list of available commands.

@github-code-quality

github-code-quality Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in the codex/restore-tavily... branch is 96%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/restore-tavily... 2dec46a +/-
nemoclaw/src/se...cret-scanner.ts 100%
nemoclaw/src/commands/slash.ts 100%
nemoclaw/src/li...bprocess-env.ts 100%
nemoclaw/src/bl...eprint/state.ts 98%
nemoclaw/src/onboard/config.ts 98%
nemoclaw/src/bl...int/snapshot.ts 97%
nemoclaw/src/bl...print/runner.ts 95%
nemoclaw/src/co...ration-state.ts 94%
nemoclaw/src/bl...ate-networks.ts 94%
nemoclaw/src/index.ts 94%

TypeScript / code-coverage/cli

The overall coverage in the codex/restore-tavily... branch is 68%. Coverage data for the main branch is not yet available.

Show a code coverage summary of the most covered files.
File main codex/restore-tavily... 2dec46a +/-
src/lib/shields...nsition-lock.ts 86%
src/lib/actions...dbox/rebuild.ts 80%
src/lib/actions...all/run-plan.ts 80%
src/lib/state/o...oard-session.ts 80%
src/lib/state/sandbox.ts 72%
src/lib/onboard/preflight.ts 69%
src/lib/shields/index.ts 67%
src/lib/onboard...er-gpu-patch.ts 59%
src/lib/actions...licy-channel.ts 58%
src/lib/onboard.ts 20%

Updated July 01, 2026 17:56 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

E2E Advisor Recommendation

Required E2E: ubuntu-repo-cloud-langchain-deepagents-code
Optional E2E: None

Dispatch hint: targets=ubuntu-repo-cloud-langchain-deepagents-code

Workflow run

Full advisor summary

E2E Recommendation Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E

  • ubuntu-repo-cloud-langchain-deepagents-code (live hosted-inference E2E on ubuntu-latest; moderate runtime and requires NVIDIA_INFERENCE_API_KEY): This target runs the Deep Agents Code policy suite, including test/e2e/e2e-cloud-experimental/checks/09-deepagents-code-tavily-opt-in.sh, which applies the Tavily policy in a real sandbox and verifies the managed /opt/venv Python path can reach api.tavily.com after policy-add.

Optional E2E

  • None.

New E2E recommendations

  • provider-profile (medium): Existing live Tavily coverage validates the policy preset via policy-add, but there does not appear to be a dedicated live E2E that exercises Tavily provider-profile activation and credential/profile material end-to-end.
    • Suggested test: Add a focused Tavily provider-profile E2E target that provisions/activates the Tavily provider profile with a test credential and verifies the generated network policy includes the intended /opt/venv Python allowlist without broadening egress.

Dispatch hint

  • Workflow: .github/workflows/e2e.yaml
  • jobs input: targets=ubuntu-repo-cloud-langchain-deepagents-code

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

E2E Target Recommendation

Required E2E targets: ubuntu-repo-cloud-langchain-deepagents-code
Optional E2E targets: None

Dispatch required E2E targets:

  • gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Workflow run

Full E2E target advisor summary

E2E Target Advisor

Base: origin/main
Head: HEAD
Confidence: high

Required E2E targets

  • ubuntu-repo-cloud-langchain-deepagents-code: The PR changes the Tavily opt-in policy/provider allowlist for Deep Agents Code Python egress. The live-supported LangChain Deep Agents Code target runs the deepagents-code-policy checks, including the Tavily opt-in probe that exercises policy-add tavily and managed Python reachability to api.tavily.com.
    • Dispatch: gh workflow run e2e.yaml --ref <pr-head-ref> --field targets=ubuntu-repo-cloud-langchain-deepagents-code

Optional E2E targets

  • None.

Relevant changed files

  • nemoclaw-blueprint/policies/presets/tavily.yaml
  • nemoclaw-blueprint/provider-profiles/tavily.yaml

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: Tavily opt-in egress for Deep Agents Code managed Python.
Open items: 0 required · 2 warnings · 0 suggestions · 6 test follow-ups
Top item: Add runtime coverage for managed-Python Tavily egress boundaries

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: Tavily opt-in egress for Deep Agents Code managed Python
  • PRA-2 Resolve or justify: Runtime contract for managed-Python Tavily egress is not covered in nemoclaw-blueprint/policies/presets/tavily.yaml:21
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Acceptance clause
  • PRA-T6 Add or justify test follow-up: Tavily opt-in egress for Deep Agents Code managed Python

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify security nemoclaw-blueprint/policies/presets/tavily.yaml:21 Add or identify behavior-level validation for the opt-in Tavily policy/profile: allow `/opt/venv/bin/python3` to reach `https://api.tavily.com\`, deny `/usr/bin/python3*`, `/usr/local/bin/python3*`, and `/sandbox/**/bin/python3*` to the same host, and deny `/opt/venv/bin/python3` to an unapproved host. If the runtime validation already exists outside the changed files, add a static assertion or comment reference tying this preset change to that coverage.
Review findings by urgency: 0 required fixes, 2 items to resolve/justify, 0 in-scope improvements

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: Tavily opt-in egress for Deep Agents Code managed Python

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Static allowlist equality tests exist, but the missing regression is a live opt-in policy test proving managed Python reaches Tavily while system/project Python and unapproved hosts are denied.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: `agents/langchain-deepagents-code/policy-additions.yaml` documents similar Python attribution for approved hosts; this PR adds the same kind of path to the reusable Tavily preset/profile without adding opt-in runtime validation.

PRA-2 Resolve/justify — Runtime contract for managed-Python Tavily egress is not covered

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:21
  • Category: security
  • Problem: The Tavily preset/profile now allow `/opt/venv/bin/python3*` to send credentialed GET/POST traffic to `api.tavily.com`. The changed tests prove the YAML shape, but they do not prove the live OpenShell policy boundary: that `/opt/venv/bin/python3` can reach Tavily after opt-in, that project/system Python interpreters remain denied, and that the managed interpreter cannot use this opt-in to reach other hosts. This also leaves the localized OpenShell process-attribution workaround without a durable regression proving the source boundary.
  • Impact: If OpenShell binary matching, symlink resolution, or sandbox filesystem assumptions differ from the static YAML expectation, the preset could grant Tavily API-key egress to a broader Python runtime than intended, or still fail to restore Deep Agents Code Tavily requests despite the allowlist change.
  • Recommended action: Add or identify behavior-level validation for the opt-in Tavily policy/profile: allow `/opt/venv/bin/python3` to reach `https://api.tavily.com\`, deny `/usr/bin/python3*`, `/usr/local/bin/python3*`, and `/sandbox/**/bin/python3*` to the same host, and deny `/opt/venv/bin/python3` to an unapproved host. If the runtime validation already exists outside the changed files, add a static assertion or comment reference tying this preset change to that coverage.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Read `test/e2e/e2e-cloud-experimental/checks/06-deepagents-code-python-egress.sh` and the Tavily preset/profile tests to confirm whether there is an opt-in Tavily case covering `/opt/venv/bin/python3` plus negative system/project Python and arbitrary-host probes.
  • Missing regression test: A behavior test named like `with Tavily opt-in, managed venv Python reaches api.tavily.com while system/project Python and unapproved hosts remain blocked`; existing static equality tests only prove the YAML entries.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Read `test/e2e/e2e-cloud-experimental/checks/06-deepagents-code-python-egress.sh` and the Tavily preset/profile tests to confirm whether there is an opt-in Tavily case covering `/opt/venv/bin/python3` plus negative system/project Python and arbitrary-host probes.
  • Evidence: `nemoclaw-blueprint/policies/presets/tavily.yaml` adds `/opt/venv/bin/python3*`; `test/tavily-preset.test.ts` and `test/validate-blueprint.test.ts` only assert exact allowlist arrays. Nearby Deep Agents Code egress checks currently assert arbitrary Python is blocked from Tavily by default, not that the Tavily opt-in boundary behaves correctly.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

  • None.
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — With Tavily preset/provider attached, `/opt/venv/bin/python3` can reach `https://api.tavily.com\`.. This PR changes sandbox/network policy egress for a credentialed external API. Static YAML contract tests are useful but insufficient to prove the OpenShell runtime boundary for binary matching and host denial.
  • PRA-T2 Runtime validation — With Tavily preset/provider attached, `/usr/bin/python3*`, `/usr/local/bin/python3*`, and `/sandbox/**/bin/python3*` cannot reach `https://api.tavily.com\`.. This PR changes sandbox/network policy egress for a credentialed external API. Static YAML contract tests are useful but insufficient to prove the OpenShell runtime boundary for binary matching and host denial.
  • PRA-T3 Runtime validation — With Tavily preset/provider attached, `/opt/venv/bin/python3` cannot reach an unapproved host such as `https://example.com\`.. This PR changes sandbox/network policy egress for a credentialed external API. Static YAML contract tests are useful but insufficient to prove the OpenShell runtime boundary for binary matching and host denial.
  • PRA-T4 Runtime validation — Without Tavily opt-in, Deep Agents Code managed Python remains denied from `https://api.tavily.com\`.. This PR changes sandbox/network policy egress for a credentialed external API. Static YAML contract tests are useful but insufficient to prove the OpenShell runtime boundary for binary matching and host denial.
  • PRA-T5 Acceptance clause — Deep Agents Code can reach `api.tavily.com` after the operator explicitly applies the Tavily policy and attaches the Tavily provider, while Tavily remains denied by default and system Python interpreters remain excluded. — add test evidence or identify existing coverage. Static config supports the intended opt-in and excludes `/usr/bin/python3*` and `/usr/local/bin/python3*` from exact allowlist tests, but no changed runtime or integration test proves the live OpenShell behavior for managed Python, system Python, project venv Python, or arbitrary hosts.
  • PRA-T6 Tavily opt-in egress for Deep Agents Code managed Python — Static allowlist equality tests exist, but the missing regression is a live opt-in policy test proving managed Python reaches Tavily while system/project Python and unapproved hosts are denied.. `agents/langchain-deepagents-code/policy-additions.yaml` documents similar Python attribution for approved hosts; this PR adds the same kind of path to the reusable Tavily preset/profile without adding opt-in runtime validation.

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor (Nemotron Ultra) — Changes requested

Merge posture: Do not merge yet
Primary next action: Resolve or justify PRA-1: Source-of-truth review needed: nemoclaw-blueprint/policies/presets/tavily.yaml:20 and nemoclaw-blueprint/provider-profiles/tavily.yaml:23.
Open items: 0 required · 6 warnings · 1 suggestion · 8 test follow-ups
Since last review: 0 prior items resolved · 0 still apply · 5 new items found

Action checklist

  • PRA-1 Resolve or justify: Source-of-truth review needed: nemoclaw-blueprint/policies/presets/tavily.yaml:20 and nemoclaw-blueprint/provider-profiles/tavily.yaml:23
  • PRA-2 Resolve or justify: Binary allowlist wildcard /opt/venv/bin/python3* broader than necessary in nemoclaw-blueprint/policies/presets/tavily.yaml:20
  • PRA-3 Resolve or justify: Provider profile binary allowlist expanded with same wildcard in nemoclaw-blueprint/provider-profiles/tavily.yaml:23
  • PRA-4 Resolve or justify: Missing negative tests for binary allowlist enforcement in test/validate-blueprint.test.ts:506
  • PRA-5 Resolve or justify: No cross-consistency test between policy preset and provider profile in test/tavily-preset.test.ts:25
  • PRA-6 Resolve or justify: OpenShell attribution workaround not tracked as temporary in nemoclaw-blueprint/policies/presets/tavily.yaml:20
  • PRA-T1 Add or justify test follow-up: Runtime validation
  • PRA-T2 Add or justify test follow-up: Runtime validation
  • PRA-T3 Add or justify test follow-up: Runtime validation
  • PRA-T4 Add or justify test follow-up: Runtime validation
  • PRA-T5 Add or justify test follow-up: Runtime validation
  • PRA-T6 Add or justify test follow-up: Missing negative tests for binary allowlist enforcement
  • PRA-T7 Add or justify test follow-up: Acceptance clause
  • PRA-T8 Add or justify test follow-up: nemoclaw-blueprint/policies/presets/tavily.yaml:20 and nemoclaw-blueprint/provider-profiles/tavily.yaml:23
  • PRA-7 In-scope improvement: PR feat(mcp): add OpenShell-managed MCP servers #5876 modifies same files — coordinate to avoid policy regression in nemoclaw-blueprint/policies/presets/tavily.yaml:1

Findings index

ID Severity Category Location Required action
PRA-1 Resolve/justify architecture Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
PRA-2 Resolve/justify security nemoclaw-blueprint/policies/presets/tavily.yaml:20 Change /opt/venv/bin/python3* to /opt/venv/bin/python3 in both the policy preset and provider profile. This matches the actual venv interpreter symlink and follows least privilege.
PRA-3 Resolve/justify security nemoclaw-blueprint/provider-profiles/tavily.yaml:23 Apply same narrowing to /opt/venv/bin/python3 in provider profile. Ensure both files stay in sync.
PRA-4 Resolve/justify tests test/validate-blueprint.test.ts:506 Add negative test cases in validate-blueprint.test.ts or tavily-preset.test.ts asserting denial for: /usr/bin/python3, /opt/venv/bin/python3.12, /opt/venv/bin/pip, /opt/venv/bin/dcode.
PRA-5 Resolve/justify correctness test/tavily-preset.test.ts:25 Add a test asserting policy preset tavily binaries === provider profile tavily binaries.
PRA-6 Resolve/justify security nemoclaw-blueprint/policies/presets/tavily.yaml:20 Add TODO comment with link to OpenShell issue or NemoClaw tracking issue: 'TODO: Remove when OpenShell fixes process attribution for managed venvs (tracking: <issue-url>)'.
PRA-7 Improvement scope nemoclaw-blueprint/policies/presets/tavily.yaml:1 Fetch PR #5876 diff for these files and verify no conflict. If both merge, ensure combined binary allowlist is minimal and correct.
Review findings by urgency: 0 required fixes, 6 items to resolve/justify, 1 in-scope improvement

⚠️ Resolve or justify before merge

Investigate these in the current review; either fix them, explain why they are not applicable, or document the accepted risk.

PRA-1 Resolve/justify — Source-of-truth review needed: nemoclaw-blueprint/policies/presets/tavily.yaml:20 and nemoclaw-blueprint/provider-profiles/tavily.yaml:23

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Current tests only lock allowlist content (positive). No test proves attribution behavior stability. Integration test of Deep Agents Code sandbox Tavily egress would validate workaround works.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: YAML comment: 'OpenShell attributes Deep Agents Code Tavily requests to its managed Python venv.' No TODO, no issue link, no removal timeline.

PRA-2 Resolve/justify — Binary allowlist wildcard /opt/venv/bin/python3* broader than necessary

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:20
  • Category: security
  • Problem: The wildcard matches python3, python3.13, python3.13t, and any future python3.x binary added to the venv. Dockerfile.base shows venv created with 'python3 -m venv' producing only a 'python3' symlink. The wildcard expands the credential injection and egress surface beyond the single interpreter entry point.
  • Impact: Any binary matching python3* in /opt/venv/bin/ can now egress to api.tavily.com and receive the injected TAVILY_API_KEY via gateway credential injection. If the venv gains additional entry points (pip, pytest, user scripts), they gain unintended Tavily access.
  • Recommended action: Change /opt/venv/bin/python3* to /opt/venv/bin/python3 in both the policy preset and provider profile. This matches the actual venv interpreter symlink and follows least privilege.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check agents/langchain-deepagents-code/Dockerfile.base: RUN python3 -m venv --copies "$VIRTUAL_ENV" creates python3 symlink. The wildcard is broader than the single entry point needed.
  • Missing regression test: Add negative test verifying /opt/venv/bin/python3.12 (or other version) is denied egress to api.tavily.com under the tavily preset, confirming least privilege enforcement.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check agents/langchain-deepagents-code/Dockerfile.base: RUN python3 -m venv --copies "$VIRTUAL_ENV" creates python3 symlink. The wildcard is broader than the single entry point needed.
  • Evidence: Diff shows '+ - { path: /opt/venv/bin/python3* }' added to binaries array. Dockerfile.base confirms python3 -m venv creates python3 symlink only.

PRA-3 Resolve/justify — Provider profile binary allowlist expanded with same wildcard

  • Location: nemoclaw-blueprint/provider-profiles/tavily.yaml:23
  • Category: security
  • Problem: Provider profile binaries array controls which processes receive the injected TAVILY_API_KEY. The same wildcard expansion here widens the credential injection surface identically to the policy preset.
  • Impact: Any python3*-matching binary in the venv can now obtain the Tavily API key via gateway credential injection, not just egress through the network policy.
  • Recommended action: Apply same narrowing to /opt/venv/bin/python3 in provider profile. Ensure both files stay in sync.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare both YAML files — they must have identical binary allowlists for consistency. Current diff shows identical wildcard added to both.
  • Missing regression test: Test that provider profile binaries exactly match policy preset binaries for tavily, and that neither contains the wildcard after fix.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare both YAML files — they must have identical binary allowlists for consistency. Current diff shows identical wildcard added to both.
  • Evidence: Diff shows '+ - /opt/venv/bin/python3*' added to provider profile binaries array with same comment.

PRA-4 Resolve/justify — Missing negative tests for binary allowlist enforcement

  • Location: test/validate-blueprint.test.ts:506
  • Category: tests
  • Problem: Tests only assert positive equality of the allowlist array. No test verifies that binaries outside the allowlist (system python3, venv pip, venv dcode, different python version) are denied Tavily egress.
  • Impact: Regression could widen the allowlist further without test failure. No confidence that enforcement actually denies non-allowlisted binaries at runtime.
  • Recommended action: Add negative test cases in validate-blueprint.test.ts or tavily-preset.test.ts asserting denial for: /usr/bin/python3, /opt/venv/bin/python3.12, /opt/venv/bin/pip, /opt/venv/bin/dcode.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/validate-blueprint.test.ts around line 506 — only positive expect(profile.binaries).toEqual([...]) exists.
  • Missing regression test: Negative test: create sandbox with tavily preset, attempt egress from non-allowlisted binary, verify 403/denial.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/validate-blueprint.test.ts around line 506 — only positive expect(profile.binaries).toEqual([...]) exists.
  • Evidence: Test file diff shows only positive assertion updated to include the new wildcard entry.

PRA-5 Resolve/justify — No cross-consistency test between policy preset and provider profile

  • Location: test/tavily-preset.test.ts:25
  • Category: correctness
  • Problem: Policy preset binaries and provider profile binaries must stay in sync for correct credential injection + egress behavior. Tests verify each independently but not their equality.
  • Impact: Future edits could diverge the two allowlists, causing either credential injection without egress (broken functionality) or egress without credential injection (silent failures).
  • Recommended action: Add a test asserting policy preset tavily binaries === provider profile tavily binaries.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare test/tavily-preset.test.ts (loads preset) and test/validate-blueprint.test.ts (loads provider profile) — no cross-reference exists.
  • Missing regression test: Unit test loading both YAML files and asserting binary arrays are identical.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare test/tavily-preset.test.ts (loads preset) and test/validate-blueprint.test.ts (loads provider profile) — no cross-reference exists.
  • Evidence: Two separate test files each validate their own YAML structure with no shared assertion.

PRA-6 Resolve/justify — OpenShell attribution workaround not tracked as temporary

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:20
  • Category: security
  • Problem: YAML comment documents workaround for OpenShell's process-attribution behavior but lacks TODO/link to upstream fix tracking. Workaround expands attack surface and should be removed when root cause is fixed.
  • Impact: Workaround may persist indefinitely, keeping unnecessarily broad binary allowlist. No visibility on when/if OpenShell will fix attribution.
  • Recommended action: Add TODO comment with link to OpenShell issue or NemoClaw tracking issue: 'TODO: Remove when OpenShell fixes process attribution for managed venvs (tracking: <issue-url>)'.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for OpenShell issue tracker or NemoClaw issue referencing network attribution for managed Python venvs. Current comment has no tracking reference.
  • Missing regression test: Integration test confirming Deep Agents Code sandbox can successfully call Tavily API with this policy (validates workaround works end-to-end).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for OpenShell issue tracker or NemoClaw issue referencing network attribution for managed Python venvs. Current comment has no tracking reference.
  • Evidence: Comment: '# OpenShell attributes Deep Agents Code Tavily requests to its managed Python venv.' No TODO, no issue link, no removal condition.

💡 In-scope improvements

These are lower-risk, not throwaway. Prefer fixing them in this PR when they are local to changed code; defer only with rationale or a linked follow-up.

PRA-7 Improvement — PR #5876 modifies same files — coordinate to avoid policy regression

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:1
  • Category: scope
  • Problem: Open PR feat(mcp): add OpenShell-managed MCP servers #5876 (feat(mcp): add OpenShell-managed MCP servers) also modifies nemoclaw-blueprint/policies/presets/tavily.yaml and nemoclaw-blueprint/provider-profiles/tavily.yaml. Uncoordinated changes to sandbox network policy are a systemic risk.
  • Impact: Combined effect of both PRs could further widen binary allowlists or create conflicting policy state.
  • Suggested action: Fetch PR feat(mcp): add OpenShell-managed MCP servers #5876 diff for these files and verify no conflict. If both merge, ensure combined binary allowlist is minimal and correct.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check GitHub PR feat(mcp): add OpenShell-managed MCP servers #5876 diff for nemoclaw-blueprint/policies/presets/tavily.yaml and nemoclaw-blueprint/provider-profiles/tavily.yaml.
  • Missing regression test: Combined policy validation test after both PRs merge.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Drift context shows PR feat(mcp): add OpenShell-managed MCP servers #5876 has sameFiles: ["nemoclaw-blueprint/policies/presets/tavily.yaml", "nemoclaw-blueprint/provider-profiles/tavily.yaml"]
Simplification opportunities: 1 possible cut

These are safe simplification checks only. Do not remove validation, security controls, data-loss prevention, or required tests.

  • PRA-6 shrink (nemoclaw-blueprint/policies/presets/tavily.yaml:20): /opt/venv/bin/python3*
    • Replacement: /opt/venv/bin/python3
    • Net: 0 lines
    • Safety boundary: Must preserve Deep Agents Code Tavily egress functionality; verify with integration test before narrowing
Test follow-ups to resolve or justify

If these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.

  • PRA-T1 Runtime validation — denies Tavily egress from non-allowlisted binary (system python3) under tavily preset. Sandbox/network policy changes need behavioral runtime validation. Unit tests verify YAML structure only. PR body acknowledges live validation (ubuntu-repo-cloud-langchain-deepagents-code) still required.
  • PRA-T2 Runtime validation — denies Tavily egress from venv pip/dcode under tavily preset. Sandbox/network policy changes need behavioral runtime validation. Unit tests verify YAML structure only. PR body acknowledges live validation (ubuntu-repo-cloud-langchain-deepagents-code) still required.
  • PRA-T3 Runtime validation — denies Tavily egress from /opt/venv/bin/python3.12 under tavily preset. Sandbox/network policy changes need behavioral runtime validation. Unit tests verify YAML structure only. PR body acknowledges live validation (ubuntu-repo-cloud-langchain-deepagents-code) still required.
  • PRA-T4 Runtime validation — Deep Agents Code sandbox with tavily preset + provider can successfully call Tavily API (integration). Sandbox/network policy changes need behavioral runtime validation. Unit tests verify YAML structure only. PR body acknowledges live validation (ubuntu-repo-cloud-langchain-deepagents-code) still required.
  • PRA-T5 Runtime validation — policy preset tavily binaries === provider profile tavily binaries (cross-consistency). Sandbox/network policy changes need behavioral runtime validation. Unit tests verify YAML structure only. PR body acknowledges live validation (ubuntu-repo-cloud-langchain-deepagents-code) still required.
  • PRA-T6 Missing negative tests for binary allowlist enforcement — Add negative test cases in validate-blueprint.test.ts or tavily-preset.test.ts asserting denial for: /usr/bin/python3, /opt/venv/bin/python3.12, /opt/venv/bin/pip, /opt/venv/bin/dcode.
  • PRA-T7 Acceptance clause — Deep Agents Code can reach api.tavily.com after operator applies Tavily policy + attaches Tavily provider; Tavily denied by default; system Python excluded — add test evidence or identify existing coverage. Unit tests pass (67/67) verifying YAML structure. PR body acknowledges: 'A rerun of ubuntu-repo-cloud-langchain-deepagents-code is still required for live validation.' Runtime egress behavior not validated in CI.
  • PRA-T8 nemoclaw-blueprint/policies/presets/tavily.yaml:20 and nemoclaw-blueprint/provider-profiles/tavily.yaml:23 — Current tests only lock allowlist content (positive). No test proves attribution behavior stability. Integration test of Deep Agents Code sandbox Tavily egress would validate workaround works.. YAML comment: 'OpenShell attributes Deep Agents Code Tavily requests to its managed Python venv.' No TODO, no issue link, no removal timeline.
Since last review details

Current findings, using the urgency labels above:

PRA-1 Resolve/justify — Source-of-truth review needed: nemoclaw-blueprint/policies/presets/tavily.yaml:20 and nemoclaw-blueprint/provider-profiles/tavily.yaml:23

  • Location: not file-specific
  • Category: architecture
  • Problem: The advisor marked localized patch analysis as needs_followup.
  • Impact: A localized workaround can preserve or hide an invalid state when the source boundary is unclear.
  • Recommended action: Identify the invalid state, source boundary, source-fix constraint, regression test, and removal condition before merging the localized behavior.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Missing regression test: Current tests only lock allowlist content (positive). No test proves attribution behavior stability. Integration test of Deep Agents Code sandbox Tavily egress would validate workaround works.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Inspect the localized patch and source-of-truth review fields for a concrete invalid state, source boundary, source-fix constraint, regression test, and removal condition.
  • Evidence: YAML comment: 'OpenShell attributes Deep Agents Code Tavily requests to its managed Python venv.' No TODO, no issue link, no removal timeline.

PRA-2 Resolve/justify — Binary allowlist wildcard /opt/venv/bin/python3* broader than necessary

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:20
  • Category: security
  • Problem: The wildcard matches python3, python3.13, python3.13t, and any future python3.x binary added to the venv. Dockerfile.base shows venv created with 'python3 -m venv' producing only a 'python3' symlink. The wildcard expands the credential injection and egress surface beyond the single interpreter entry point.
  • Impact: Any binary matching python3* in /opt/venv/bin/ can now egress to api.tavily.com and receive the injected TAVILY_API_KEY via gateway credential injection. If the venv gains additional entry points (pip, pytest, user scripts), they gain unintended Tavily access.
  • Recommended action: Change /opt/venv/bin/python3* to /opt/venv/bin/python3 in both the policy preset and provider profile. This matches the actual venv interpreter symlink and follows least privilege.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check agents/langchain-deepagents-code/Dockerfile.base: RUN python3 -m venv --copies "$VIRTUAL_ENV" creates python3 symlink. The wildcard is broader than the single entry point needed.
  • Missing regression test: Add negative test verifying /opt/venv/bin/python3.12 (or other version) is denied egress to api.tavily.com under the tavily preset, confirming least privilege enforcement.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check agents/langchain-deepagents-code/Dockerfile.base: RUN python3 -m venv --copies "$VIRTUAL_ENV" creates python3 symlink. The wildcard is broader than the single entry point needed.
  • Evidence: Diff shows '+ - { path: /opt/venv/bin/python3* }' added to binaries array. Dockerfile.base confirms python3 -m venv creates python3 symlink only.

PRA-3 Resolve/justify — Provider profile binary allowlist expanded with same wildcard

  • Location: nemoclaw-blueprint/provider-profiles/tavily.yaml:23
  • Category: security
  • Problem: Provider profile binaries array controls which processes receive the injected TAVILY_API_KEY. The same wildcard expansion here widens the credential injection surface identically to the policy preset.
  • Impact: Any python3*-matching binary in the venv can now obtain the Tavily API key via gateway credential injection, not just egress through the network policy.
  • Recommended action: Apply same narrowing to /opt/venv/bin/python3 in provider profile. Ensure both files stay in sync.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare both YAML files — they must have identical binary allowlists for consistency. Current diff shows identical wildcard added to both.
  • Missing regression test: Test that provider profile binaries exactly match policy preset binaries for tavily, and that neither contains the wildcard after fix.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare both YAML files — they must have identical binary allowlists for consistency. Current diff shows identical wildcard added to both.
  • Evidence: Diff shows '+ - /opt/venv/bin/python3*' added to provider profile binaries array with same comment.

PRA-4 Resolve/justify — Missing negative tests for binary allowlist enforcement

  • Location: test/validate-blueprint.test.ts:506
  • Category: tests
  • Problem: Tests only assert positive equality of the allowlist array. No test verifies that binaries outside the allowlist (system python3, venv pip, venv dcode, different python version) are denied Tavily egress.
  • Impact: Regression could widen the allowlist further without test failure. No confidence that enforcement actually denies non-allowlisted binaries at runtime.
  • Recommended action: Add negative test cases in validate-blueprint.test.ts or tavily-preset.test.ts asserting denial for: /usr/bin/python3, /opt/venv/bin/python3.12, /opt/venv/bin/pip, /opt/venv/bin/dcode.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Check test/validate-blueprint.test.ts around line 506 — only positive expect(profile.binaries).toEqual([...]) exists.
  • Missing regression test: Negative test: create sandbox with tavily preset, attempt egress from non-allowlisted binary, verify 403/denial.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Check test/validate-blueprint.test.ts around line 506 — only positive expect(profile.binaries).toEqual([...]) exists.
  • Evidence: Test file diff shows only positive assertion updated to include the new wildcard entry.

PRA-5 Resolve/justify — No cross-consistency test between policy preset and provider profile

  • Location: test/tavily-preset.test.ts:25
  • Category: correctness
  • Problem: Policy preset binaries and provider profile binaries must stay in sync for correct credential injection + egress behavior. Tests verify each independently but not their equality.
  • Impact: Future edits could diverge the two allowlists, causing either credential injection without egress (broken functionality) or egress without credential injection (silent failures).
  • Recommended action: Add a test asserting policy preset tavily binaries === provider profile tavily binaries.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Compare test/tavily-preset.test.ts (loads preset) and test/validate-blueprint.test.ts (loads provider profile) — no cross-reference exists.
  • Missing regression test: Unit test loading both YAML files and asserting binary arrays are identical.
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Compare test/tavily-preset.test.ts (loads preset) and test/validate-blueprint.test.ts (loads provider profile) — no cross-reference exists.
  • Evidence: Two separate test files each validate their own YAML structure with no shared assertion.

PRA-6 Resolve/justify — OpenShell attribution workaround not tracked as temporary

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:20
  • Category: security
  • Problem: YAML comment documents workaround for OpenShell's process-attribution behavior but lacks TODO/link to upstream fix tracking. Workaround expands attack surface and should be removed when root cause is fixed.
  • Impact: Workaround may persist indefinitely, keeping unnecessarily broad binary allowlist. No visibility on when/if OpenShell will fix attribution.
  • Recommended action: Add TODO comment with link to OpenShell issue or NemoClaw tracking issue: 'TODO: Remove when OpenShell fixes process attribution for managed venvs (tracking: <issue-url>)'.
  • Expected follow-up: Resolve in this PR or explain why the risk is acceptable.
  • Verification: Search for OpenShell issue tracker or NemoClaw issue referencing network attribution for managed Python venvs. Current comment has no tracking reference.
  • Missing regression test: Integration test confirming Deep Agents Code sandbox can successfully call Tavily API with this policy (validates workaround works end-to-end).
  • Done when: The risk is fixed or explicitly justified in the PR. Verification: Search for OpenShell issue tracker or NemoClaw issue referencing network attribution for managed Python venvs. Current comment has no tracking reference.
  • Evidence: Comment: '# OpenShell attributes Deep Agents Code Tavily requests to its managed Python venv.' No TODO, no issue link, no removal condition.

PRA-7 Improvement — PR #5876 modifies same files — coordinate to avoid policy regression

  • Location: nemoclaw-blueprint/policies/presets/tavily.yaml:1
  • Category: scope
  • Problem: Open PR feat(mcp): add OpenShell-managed MCP servers #5876 (feat(mcp): add OpenShell-managed MCP servers) also modifies nemoclaw-blueprint/policies/presets/tavily.yaml and nemoclaw-blueprint/provider-profiles/tavily.yaml. Uncoordinated changes to sandbox network policy are a systemic risk.
  • Impact: Combined effect of both PRs could further widen binary allowlists or create conflicting policy state.
  • Suggested action: Fetch PR feat(mcp): add OpenShell-managed MCP servers #5876 diff for these files and verify no conflict. If both merge, ensure combined binary allowlist is minimal and correct.
  • Expected follow-up: Prefer a current-PR fix when local to changed code; defer only with rationale or linked follow-up.
  • Verification: Check GitHub PR feat(mcp): add OpenShell-managed MCP servers #5876 diff for nemoclaw-blueprint/policies/presets/tavily.yaml and nemoclaw-blueprint/provider-profiles/tavily.yaml.
  • Missing regression test: Combined policy validation test after both PRs merge.
  • Done when: The local improvement is applied, or the PR notes why it should be deferred.
  • Evidence: Drift context shows PR feat(mcp): add OpenShell-managed MCP servers #5876 has sameFiles: ["nemoclaw-blueprint/policies/presets/tavily.yaml", "nemoclaw-blueprint/provider-profiles/tavily.yaml"]

Workflow run details

This is an automated, non-binding review; it still expects maintainers and agents to respond to each required or warning item. Treat suggestions as current-PR improvements when they touch changed code; defer only with maintainer rationale or a linked follow-up. A human maintainer must make the final merge decision.

@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Superseded by #6134, which uses a fresh GitHub-verified signed commit and follows the NemoClaw PR template and DCO requirements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants