feat(dcode): add bounded headless validation profiles - #7822
Conversation
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
jyaunches
added
area: docs
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a digest-bound managed DCode validation profile that can be supplied during rebuild, baked into the image, enforced for headless command execution, persisted in sandbox state, and exposed through status and documentation. ChangesManaged DCode validation
Estimated code review effort: 5 (Critical) | ~90 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
🌿 Preview your docs: https://nvidia-preview-pr-7822.docs.buildwithfern.com/nemoclaw |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall coverage in commit 9f48519 in the TypeScript / code-coverage/cliThe overall coverage in commit 9f48519 in the Show a code coverage summary of the most impacted files.
Updated |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/domain/lifecycle/options.ts (1)
150-168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winTyped-object path doesn't validate
dcodeValidationProfile, unlike the sibling options — and it's the path the real CLI uses.
dcodeAutoApprovalModeandtoolDisclosureare both re-validated in this branch even when passed as a typed object (lines 150-163), butdcodeValidationProfileis only guarded in the array/argv branch (lines 93-106).src/commands/sandbox/rebuild.tsalways builds a typedRebuildSandboxOptionsobject, so the "requires an absolute JSON path or 'disabled'" guard tested inoptions.test.tsnever actually protects the shippedsandbox rebuildcommand — an empty string (e.g. from--dcode-validation-profile=) silently falls through to "inherit existing profile" instead of raising the intended validation error.
[major]Please also add a test to `options.test.ts` exercising `normalizeRebuildSandboxOptions({ dcodeValidationProfile: "" })` throwing, to close the coverage gap that let this slip.🛠️ Proposed fix
rawToolDisclosure = options.toolDisclosure; const toolDisclosure = normalizeToolDisclosure(rawToolDisclosure); if (rawToolDisclosure !== undefined && !toolDisclosure) { throw new Error(`toolDisclosure must be one of: ${TOOL_DISCLOSURE_VALUES.join(", ")}.`); } + if (options.dcodeValidationProfile !== undefined && !options.dcodeValidationProfile) { + throw new Error("dcodeValidationProfile requires an absolute JSON path or 'disabled'."); + } return { ...options, ...(dcodeAutoApprovalMode ? { dcodeAutoApprovalMode } : {}), ...(toolDisclosure ? { toolDisclosure } : {}), }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/domain/lifecycle/options.ts` around lines 150 - 168, Update the typed-object normalization path in normalizeRebuildSandboxOptions to validate dcodeValidationProfile with the same absolute-JSON-path-or-"disabled" rules used by the array/argv path, rejecting an empty string instead of treating it as inheritance. Preserve valid values and add an options.test.ts case asserting normalizeRebuildSandboxOptions({ dcodeValidationProfile: "" }) throws.
🧹 Nitpick comments (2)
test/langchain-deepagents-code-auto-approval-image.test.ts (1)
68-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider dropping the exact indented source snippet.
Matching
if request.tool_call["name"] != "execute":\n return handler(request)...locks whitespace and statement layout, so any harmless reformat of the patch breaks the test without a behavior change. The interception behavior is already exercised intest/langchain-deepagents-code-validation-profile.test.ts; a narrower marker (e.g. the class name plusexecute_managed_validation_command) keeps the image contract without the formatting coupling.As per path instructions: "Prefer observable outcomes through the public boundary over source-text, private-shape, or mock-call assertions."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/langchain-deepagents-code-auto-approval-image.test.ts` around lines 68 - 71, Remove the exact indented source-snippet assertion from the patcher test. In the test containing the _NemoClawValidationProfileMiddleware assertion, retain the class marker and add a narrower stable marker such as execute_managed_validation_command, relying on the existing validation-profile test for interception behavior without coupling to formatting.Source: Path instructions
src/lib/actions/sandbox/rebuild-pipeline.ts (1)
67-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
DCODE_VALIDATION_PROFILE_ENVinstead of duplicating its value.The literal
"NEMOCLAW_DCODE_VALIDATION_PROFILE_B64"duplicates the constantDCODE_VALIDATION_PROFILE_ENVexported bysrc/lib/onboard/dcode/validation-profile.ts(already imported and used for this exact purpose inrebuild-preflight-phase.ts). If that constant's value is ever renamed, this scoped-env save/restore list would silently drift out of sync with no compile-time signal.♻️ Proposed fix
+import { DCODE_VALIDATION_PROFILE_ENV } from "../../onboard/dcode/validation-profile"; + const scopedEnvKeys = [ BRAVE_API_KEY_ENV, TAVILY_API_KEY_ENV, MESSAGING_SETUP_APPLIER_ENV_KEY, "OPENSHELL_GATEWAY", DOCKER_GPU_PATCH_NETWORK_ENV, - "NEMOCLAW_DCODE_VALIDATION_PROFILE_B64", + DCODE_VALIDATION_PROFILE_ENV, ...REBUILD_HERMES_DASHBOARD_ENV_KEYS, ...MESSAGING_CHANNEL_CONFIG_ENV_KEYS, ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/actions/sandbox/rebuild-pipeline.ts` at line 67, Replace the duplicated "NEMOCLAW_DCODE_VALIDATION_PROFILE_B64" literal in the scoped environment save/restore list with the imported DCODE_VALIDATION_PROFILE_ENV constant from validation-profile.ts, reusing the existing symbol rather than adding another import or value.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/langchain-deepagents-code/Dockerfile`:
- Around line 135-138: Update the DCode validation profile handling around
NEMOCLAW_DCODE_VALIDATION_PROFILE_B64 to fail closed when base64 decoding fails,
preserving the decoder’s nonzero status instead of allowing cleanup to make the
branch succeed. Strengthen Dockerfile-side base64 and decoded-size validation to
match patchDcodeValidationProfileDockerArg, and abort before later validation
whenever encoding or size checks fail.
In `@agents/langchain-deepagents-code/managed-dcode-runtime.py`:
- Around line 1668-1678: The invocation counter in the validation flow must not
be consumed before execution succeeds. Move the _VALIDATION_INVOCATIONS update
for the (profile["contentDigest"], command_id) key from before the
executable/working-directory checks to immediately after the child is
successfully spawned via Popen, while retaining the existing maxInvocations
check before execution.
- Around line 1795-1806: Update the post-selector process wait around
return_code and _terminate_validation_process so it uses the remaining profile
timeout derived from timeoutSeconds and the command’s start/deadline time,
rather than a fixed one-second wait. Only terminate and mark the command failed
after that remaining deadline is exceeded; otherwise preserve the child’s actual
exit status and existing terminal_status handling.
- Around line 1569-1576: Align the validation-profile digest test with the UTF-8
contract by changing its JSON serialization to use ensure_ascii=False and adding
a non-ASCII argv/path fixture. Update
test/langchain-deepagents-code-validation-profile.test.ts lines 73-75;
agents/langchain-deepagents-code/managed-dcode-runtime.py lines 1569-1576
requires no direct change because its digest logic already uses
ensure_ascii=False.
In `@agents/langchain-deepagents-code/patch-managed-deepagents-code.py`:
- Around line 691-701: The validation_profile_active branch must only enable and
auto-approve shell execution when the patched create_deep_agent middleware is
available. Update the condition to require _nemoclaw_original_create_deep_agent
is not None, preserving the existing fail-closed defaults when middleware
installation is unavailable.
- Around line 668-671: Update the subagent handling loop around
_NemoClawValidationProfileMiddleware to explicitly handle pre-built non-dict
subagent objects, ensuring validation middleware is injected or the shape is
rejected before forwarding to create_deep_agent. Preserve existing dict-subagent
behavior and prevent unsupported objects from bypassing the validation profile
and auto-approved shell path.
In `@src/lib/onboard.ts`:
- Around line 2262-2269: Update the profile resolution in the
onboarding/recreate flow around managedDcodeValidationProfile and the related
registration logic to fall back to the existing registry entry when the
environment request is unset. Preserve an explicit disabled value as a clear
operation, and route both onboarding and recreates through the same
authoritative lifecycle path so persisted managed DCode capabilities remain
registered.
In `@src/lib/onboard/dcode/validation-profile.test.ts`:
- Around line 132-143: Update the test containing mkdtempSync in
validation-profile.test.ts to remove the created temporary directory during
cleanup, using a suite or test teardown hook and the existing directory-removal
utility. Clean up filesystem resources only; do not add redundant mock,
environment, or global restoration.
In `@src/lib/state/registry.ts`:
- Line 35: Move the pure DcodeValidationProfile contract and clone/parse helper
from onboarding code into a neutral state or domain module, then update
src/lib/state/registry.ts lines 35 and 141 to import and use the neutral helper,
and update src/lib/state/registry/types.ts line 7 to import the contract there.
Ensure registry state and persistence no longer depend on onboarding-owned
modules.
In `@test/langchain-deepagents-code-auto-approval-image.test.ts`:
- Around line 63-64: Update the assertion around envBlock to capture the ENV
HOME= index, assert that the anchor exists before slicing, then verify the
sliced block does not contain NEMOCLAW_DCODE_VALIDATION_PROFILE_B64; preserve
the existing negative-content check while preventing a missing anchor from
passing vacuously.
In `@test/langchain-deepagents-code-validation-profile.test.ts`:
- Around line 30-34: Update the executable harness setup around the executables
mapping to resolve each command path through the runtime before profiling and
validation, rather than hardcoding /bin paths. Ensure the profiled paths match
executable.resolve(strict=True) on usrmerge and non-usrmerge hosts, while
preserving the existing echo, sleep, and yes command expectations.
---
Outside diff comments:
In `@src/lib/domain/lifecycle/options.ts`:
- Around line 150-168: Update the typed-object normalization path in
normalizeRebuildSandboxOptions to validate dcodeValidationProfile with the same
absolute-JSON-path-or-"disabled" rules used by the array/argv path, rejecting an
empty string instead of treating it as inheritance. Preserve valid values and
add an options.test.ts case asserting normalizeRebuildSandboxOptions({
dcodeValidationProfile: "" }) throws.
---
Nitpick comments:
In `@src/lib/actions/sandbox/rebuild-pipeline.ts`:
- Line 67: Replace the duplicated "NEMOCLAW_DCODE_VALIDATION_PROFILE_B64"
literal in the scoped environment save/restore list with the imported
DCODE_VALIDATION_PROFILE_ENV constant from validation-profile.ts, reusing the
existing symbol rather than adding another import or value.
In `@test/langchain-deepagents-code-auto-approval-image.test.ts`:
- Around line 68-71: Remove the exact indented source-snippet assertion from the
patcher test. In the test containing the _NemoClawValidationProfileMiddleware
assertion, retain the class marker and add a narrower stable marker such as
execute_managed_validation_command, relying on the existing validation-profile
test for interception behavior without coupling to formatting.
🪄 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: Enterprise
Run ID: 69c1fdc2-a845-46d0-b1ce-0b087de812db
📒 Files selected for processing (28)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pydocs/get-started/quickstart-langchain-deepagents-code.mdxpackage.jsonschemas/dcode-validation-profile.schema.jsonsrc/commands/sandbox/rebuild.tssrc/lib/actions/sandbox/rebuild-pipeline.tssrc/lib/actions/sandbox/rebuild-preflight-confirmation.tssrc/lib/actions/sandbox/rebuild-preflight-phase.tssrc/lib/actions/sandbox/rebuild/validation-profile.test.tssrc/lib/actions/sandbox/rebuild/validation-profile.tssrc/lib/actions/sandbox/status-snapshot.tssrc/lib/actions/sandbox/status-text.tssrc/lib/actions/sandbox/status.test.tssrc/lib/domain/lifecycle/options.test.tssrc/lib/domain/lifecycle/options.tssrc/lib/onboard.tssrc/lib/onboard/dcode/validation-profile.test.tssrc/lib/onboard/dcode/validation-profile.tssrc/lib/onboard/dockerfile-patch.tssrc/lib/onboard/sandbox-dockerfile-patch-flow.test.tssrc/lib/onboard/sandbox-dockerfile-patch-flow.tssrc/lib/onboard/sandbox-registration.tssrc/lib/state/registry.tssrc/lib/state/registry/types.tstest/langchain-deepagents-code-auto-approval-image.test.tstest/langchain-deepagents-code-validation-profile.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
Nemotron output stays in workflow artifacts and does not change the assessment above. E2E guidanceAdvisory only. E2E / PR Gate selects and runs jobs independently. Recommended E2E: This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/langchain-deepagents-code-progressive-tool-disclosure.test.ts (1)
475-484: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAssert validation behavior instead of private middleware layout.
This reaches the private
_NemoClawValidationProfileMiddlewaretype and requires a specific per-graph instance layout. A valid wiring refactor could break this while managedexecutebehavior remains correct. Exercisecreate_cli_agentthrough the execute boundary and assert the resulting validation receipt or rejection instead.As per path instructions, tests should “prefer observable outcomes through the public boundary over … private-shape … assertions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/langchain-deepagents-code-progressive-tool-disclosure.test.ts` around lines 475 - 484, The validation_counts helper currently asserts private middleware types and per-graph instance counts. Replace this layout inspection with tests that invoke create_cli_agent through the public execute boundary, asserting the expected validation receipt for valid input and rejection for invalid input; retain only the observable backend behavior needed by the test.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/langchain-deepagents-code-progressive-tool-disclosure.test.ts`:
- Around line 516-542: Preserve the original
managed.managed_validation_profile_enabled function before replacing it with the
always-true lambda, then wrap both validation cases in an outer try/finally that
restores this function. Keep the existing _nemoclaw_original_create_deep_agent
restoration in the same cleanup scope, ensuring both stubs are restored even
when either assertion fails.
---
Nitpick comments:
In `@test/langchain-deepagents-code-progressive-tool-disclosure.test.ts`:
- Around line 475-484: The validation_counts helper currently asserts private
middleware types and per-graph instance counts. Replace this layout inspection
with tests that invoke create_cli_agent through the public execute boundary,
asserting the expected validation receipt for valid input and rejection for
invalid input; retain only the observable backend behavior needed by the test.
🪄 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: Enterprise
Run ID: a3f39aa6-0a05-4f72-8489-9b49aa474e3b
📒 Files selected for processing (18)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/patch-managed-deepagents-code.pysrc/lib/actions/sandbox/rebuild-preflight-phase.tssrc/lib/actions/sandbox/rebuild/validation-profile.test.tssrc/lib/actions/sandbox/rebuild/validation-profile.tssrc/lib/domain/dcode-validation-profile.tssrc/lib/domain/lifecycle/options.test.tssrc/lib/domain/lifecycle/options.tssrc/lib/onboard/dcode/validation-profile.test.tssrc/lib/onboard/sandbox-dockerfile-patch-flow.tssrc/lib/onboard/sandbox-registration.test.tssrc/lib/onboard/sandbox-registration.tssrc/lib/state/registry.tssrc/lib/state/registry/types.tstest/langchain-deepagents-code-auto-approval-image.test.tstest/langchain-deepagents-code-progressive-tool-disclosure.test.tstest/langchain-deepagents-code-validation-profile.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- src/lib/actions/sandbox/rebuild/validation-profile.test.ts
- src/lib/state/registry.ts
- src/lib/domain/lifecycle/options.test.ts
- src/lib/actions/sandbox/rebuild/validation-profile.ts
- src/lib/onboard/sandbox-registration.test.ts
- src/lib/state/registry/types.ts
- agents/langchain-deepagents-code/patch-managed-deepagents-code.py
- test/langchain-deepagents-code-validation-profile.test.ts
- src/lib/domain/lifecycle/options.ts
- src/lib/onboard/sandbox-registration.ts
- agents/langchain-deepagents-code/managed-dcode-runtime.py
- src/lib/onboard/dcode/validation-profile.test.ts
- src/lib/onboard/sandbox-dockerfile-patch-flow.ts
- test/langchain-deepagents-code-auto-approval-image.test.ts
- agents/langchain-deepagents-code/Dockerfile
- src/lib/actions/sandbox/rebuild-preflight-phase.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
agents/langchain-deepagents-code/managed-dcode-runtime.py (1)
1642-1671: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winProbe relies on inode ownership under the sticky claims directory — worth an explicit comment.
The unlink denial for
sandbox_probeholds only because the hard link resolves to the root-owned anchor inode in a0o1733sticky directory; the sandbox user creating the link does not grant deletion rights. That subtlety is the whole security argument for write-once slots, so state it inline so a future refactor does not swap the anchor link for a fresh sandbox-owned file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@agents/langchain-deepagents-code/managed-dcode-runtime.py` around lines 1642 - 1671, In validate_managed_validation_invocation_budget_unprivileged, add an inline comment at the os.link(anchor, sandbox_probe, ...) operation documenting that the hard link preserves the root-owned anchor inode, so the sticky 0o1733 claims directory denies sandbox deletion. Clarify that replacing this anchor link with a newly created sandbox-owned file would invalidate the write-once security guarantee.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agents/langchain-deepagents-code/managed-dcode-runtime.py`:
- Around line 2136-2143: Anchor the execution deadline to the command spawn time
rather than function entry: update the flow around
_verified_validation_source_identity and the child-process launch to capture a
spawn timestamp and calculate deadline from it. Retain the existing started
timestamp solely for durationMs reporting, and ensure the timeout receipt is
based on the newly anchored deadline.
- Around line 1867-1869: Update the raw_format and raw_oid decoding in
execute_managed_validation_command to use defensive, non-throwing ASCII decoding
so malformed bytes become values the existing status and identity checks can
reject. Preserve the bounded rejected/source_identity_mismatch receipt behavior
and avoid allowing UnicodeDecodeError to escape before those checks.
In `@test/langchain-deepagents-code-image.test.ts`:
- Around line 195-211: Replace the runtime source-text assertions in the test
with a behavioral test for initialize_managed_validation_invocation_budget and
validate_managed_validation_invocation_budget_unprivileged using a temporary
budget root. Assert that a second or rollback attempt to reuse an invocation
claim is rejected, preserving the write-once guarantee through the public
behavior rather than variable names or formatting; keep the Dockerfile wiring
assertions and relevant policy/path checks.
---
Nitpick comments:
In `@agents/langchain-deepagents-code/managed-dcode-runtime.py`:
- Around line 1642-1671: In
validate_managed_validation_invocation_budget_unprivileged, add an inline
comment at the os.link(anchor, sandbox_probe, ...) operation documenting that
the hard link preserves the root-owned anchor inode, so the sticky 0o1733 claims
directory denies sandbox deletion. Clarify that replacing this anchor link with
a newly created sandbox-owned file would invalidate the write-once security
guarantee.
🪄 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: Enterprise
Run ID: b066480b-e795-4adc-a2e8-f32f5f95faa3
📒 Files selected for processing (12)
agents/langchain-deepagents-code/Dockerfileagents/langchain-deepagents-code/managed-dcode-runtime.pyagents/langchain-deepagents-code/policy-additions.yamldocs/get-started/quickstart-langchain-deepagents-code.mdxschemas/dcode-validation-profile.schema.jsonscripts/check-dcode-profile-import-gate.shtest/langchain-deepagents-code-image.test.tstest/langchain-deepagents-code-profile-build-gate.test.tstest/langchain-deepagents-code-progressive-tool-disclosure.test.tstest/langchain-deepagents-code-provider-label.test.tstest/langchain-deepagents-code-validation-profile.test.tstest/onboard-terminal-dashboard.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- schemas/dcode-validation-profile.schema.json
- docs/get-started/quickstart-langchain-deepagents-code.mdx
- test/langchain-deepagents-code-validation-profile.test.ts
- agents/langchain-deepagents-code/Dockerfile
- test/langchain-deepagents-code-progressive-tool-disclosure.test.ts
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
Signed-off-by: Julie Yaunches <jyaunches@nvidia.com>
cv
left a comment
cv
left a comment
There was a problem hiding this comment.
Requesting changes at exact head 913becb.
Blocking findings:
- Product scope is not approved. #7774 still carries needs: design, and its only design proposal explicitly requests maintainer approval. It also differs materially from this implementation: runtime versus rebuild-only updates, and fixed environment values versus inherited values.
- taskIdentity is not enforced. The runtime validates and echoes it but never compares it with a trusted current task or run identity; only source identity is reverified.
- Status does not prove the effective profile. sandbox status --json returns registry state, and the new test expects the profile even when reconciliation reports the sandbox missing.
- The profile digest does not bind environment values. Execution reads allowlisted names from ambient os.environ, so identical profile digests can produce different behavior and unblocked build-control variables can influence descendants.
- The acceptance-level E2E is missing. No live test launches the built managed image and exercises dcode -n with an untrusted repository and escape-seeking prompt; the current harness calls the Python executor directly.
Security warning: timeout cleanup kills only the original process group. A descendant that calls setsid(), detaches stdio, and delays its work can outlive the receipt and mutation guard. Add command-scoped descendant cleanup and a regression test.
Please record the product decision, address these runtime contracts, add the trusted prompt-level E2E, update onto current main, and rerun exact-head security, documentation-writer, CI, and E2E gates.
|
Babysitting status for exact head 913becb (plain comment; no Changes Requested review from me): the existing product-scope and runtime-identity/status/digest findings remain current because no commit followed that review. The branch is also based on 6f3afab rather than current main da1b103, with maintainer edits disabled. I am tracking this PR and will re-review a refreshed, one-hour-quiet revision after the accepted scope and concrete findings are addressed. |
|
Correction to my prior handoff: conflict-free base refreshes are explicitly waived. Please do not merge main solely for base currency; preserving exact-head evidence is preferred unless GitHub reports a real conflict or reviewed behavior requires a change. The substantive blocker or missing evidence described in the earlier handoff remains, but base age by itself is not a blocker. This is a plain coordination comment, not Changes Requested. |
|
Babysitting review for exact head
I verified that |
|
Exact-head babysitting update for
The latest automated conflict resolution also dropped main’s I am not approving ordinary or privileged workflows while the product decision and correctness/security blockers remain. I will keep monitoring this PR and will re-review a future head only after it has been quiet for one hour. |
|
Heads-up for overlap coordination: #8018 tracks a managed MCP snapshot failure in the same DCode runtime and policy files changed by this draft. The current patch does not modify |
5 participants
Summary
Managed headless
dcode -npreviously had no command-execution path.This change adds immutable, sandbox-bound validation profiles that admit only exact validation commands through a bounded direct executor while arbitrary shell execution remains disabled.
Related Issue
Fixes #7774
Changes
nemoclaw.dcode.validation-profile.v1schema, canonical digest validation, secure host-file loading, transactional rebuild flag, registry persistence, and machine-readable status.executetool with exact argv, working-directory, environment-name, timeout, output, invocation, and secret-shape checks.HOMEandPATHfixed.dcode -nautomation. Directly enabling the upstream shell is insufficient because it cannot bind exact commands to sandbox, task, and source identities or enforce per-command budgets;validation-profile.test.tsandlangchain-deepagents-code-validation-profile.test.tsprotect that contract.Type of Change
Quality Gates
Documentation Writer Review
docs-updated913becb95. The main-sync delta contains only three accepted test-timeout and stability changes; product behavior, issue [dcode] Add a bounded managed headless validation-command profile #7774 documentation, and writing guidance are unchanged. All three focused files passed (3 files, 101 tests),check:diffpassed, andgit diff --checkpassed.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin 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:npm run test:changed -- --maxWorkers=4passed 2875/2876 tests; the sole local failure is unrelated and caused by hostpython3lacking PyYAML. Required GitHub CI is authoritative.npm run docsbuilds without warnings (doc changes only) — passed with zero errors; Fern reported the existing unauthenticated redirect-check and light-mode accent-contrast advisories.Signed-off-by: Julie Yaunches jyaunches@nvidia.com