test(e2e): prove managed exec egress denial#6513
Conversation
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe sandbox inference route probe now injects a ChangesManaged-exec launcher switch and verification
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Test as e2e-egress-check
participant Probe as python_probe
participant Sandbox as sandbox_exec
participant Tavily as Tavily endpoint
Test->>Probe: python_probe(url, DCODE_MANAGED_EXEC, /opt/venv/bin/python3)
Probe->>Sandbox: remote_cmd (managed-exec prefix + python3 -c payload)
Sandbox->>Tavily: attempt egress
Tavily-->>Sandbox: BLOCKED
Sandbox-->>Probe: BLOCKED
Probe-->>Test: expect_blocked assertion
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage remains at 96%, unchanged from the branch. TypeScript / code-coverage/cliThe overall coverage in the branch remains at 76%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
PR Review Advisor (Nemotron Ultra) — No blocking findingsMerge posture: No blocking advisor findings 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. |
PR Review Advisor — No blocking findingsMerge posture: No blocking advisor findings Action checklist
Test follow-ups to resolve or justifyIf these cover changed behavior, prefer adding them in this PR; otherwise state why existing coverage is enough or link the follow-up.
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. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
E2E Advisor RecommendationRequired E2E: Dispatch hint: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
Dispatch hint
|
E2E Target RecommendationRequired E2E targets: Dispatch required E2E targets:
Full E2E target advisor summaryE2E Target AdvisorBase: Required E2E targets
Optional E2E targets
Relevant changed files
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
E2E Target Results — ✅ All requested jobs passedRun: 28982989053
|
<!-- markdownlint-disable MD041 --> ## Summary <!-- 1-3 sentences: what this PR does and why. --> Add exact live-policy evidence that directly invoking DCode's public managed-exec helper with managed Python cannot bypass the default Tavily denial. This closes the remaining security-review evidence gap from NVIDIA#6506 without changing runtime behavior. ## Related Issue <!-- Fixes #NNN or Closes #NNN. Remove this section if none. --> Follow-up to NVIDIA#6506 and completed issue NVIDIA#6504. ## Changes <!-- Bullet list of key changes. --> - Extend the existing DCode Python-egress live check to run the managed helper directly with `/opt/venv/bin/python3` and require confirmed Tavily policy-denial evidence before opt-in. - Preserve command-shape coverage for both ordinary Python and helper-prefixed Python, with static image-contract assertions for the new boundary proof. - Rename the internal DCode launcher constant to match the `dcode-managed-exec` entrypoint it identifies. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this adds regression evidence for an existing network-policy contract and makes an internal constant rename; no user-facing command, configuration, output, schema, or runtime behavior changes. - [x] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [x] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: independent reviews confirmed that direct live-policy evidence is safer and less brittle than hard-coding the generated probe argv; command construction uses quoted argv assembly, and the existing positive `connect --probe-only` live check remains the allowed-route counterpart. - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: 97 focused CLI, integration/image-contract, and E2E-support tests passed; Bash syntax, CLI build/typecheck, repository ratchets, and scoped prek also passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not applicable; this is one additional assertion in an existing typed live check plus static/command-shape coverage. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved Python egress handling so direct execution through the managed wrapper is correctly blocked when policy requires it. * Updated probe execution so command formatting stays single-line and consistent across supported launch paths. * **Tests** * Expanded end-to-end coverage for Python reachability checks, including managed-exec invocation. * Updated policy-behavior checks to reflect the latest command execution flow. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Summary
Add exact live-policy evidence that directly invoking DCode's public managed-exec helper with managed Python cannot bypass the default Tavily denial. This closes the remaining security-review evidence gap from #6506 without changing runtime behavior.
Related Issue
Follow-up to #6506 and completed issue #6504.
Changes
/opt/venv/bin/python3and require confirmed Tavily policy-denial evidence before opt-in.dcode-managed-execentrypoint it identifies.Type of Change
Quality Gates
connect --probe-onlylive check remains the allowed-route counterpart.Verification
Verifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run check:diffpassed when hooks were skipped or unavailablenpm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result: not applicable; this is one additional assertion in an existing typed live check plus static/command-shape coverage.npm run docsbuilds without warnings (doc changes only)Signed-off-by: Apurv Kumaria akumaria@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests