fix(sandbox): avoid nested login shells in Deep Agents Code smoke

[Dongni-Yang]

Summary

Deep Agents Code terminal smoke commands previously added two nested login shells inside the OpenShell transport. This change keeps both command shells but removes their login mode, routes the smoke through the image-baked managed launcher, and clears BASH_ENV and ENV for that process.

This is defense in depth, not the root fix for #8624. OpenShell still starts the transport through a sandbox-user login shell before any NemoClaw-controlled process runs, so startup files can still execute first. NVIDIA/OpenShell#2668 tracks that upstream behavior.

Related Issue

Refs #8624

Changes

• Route Deep Agents Code smoke commands through dcode-managed-exec and non-login sh -c layers.
• Preserve existing login-shell behavior for other terminal agents whose smoke commands depend on profile-provided PATH entries.
• Clear BASH_ENV and ENV at the managed Deep Agents Code boundary.
• Read the smoke command from the final test-stub argument instead of a fixed position.
• State explicitly that the existing smoke marker is diagnostic evidence only; it is not bound below the upstream transport shell.

Root Cause and Remaining Boundary

OpenShell flattens the requested arguments into a command string and starts that string with /bin/bash -lc in the supervisor. The user's profile therefore runs before dcode-managed-exec, BASH_ENV=, or ENV= can take effect. A per-invocation output marker cannot be hidden from or bound below that earlier shell.

This PR removes the two additional login modes that NemoClaw controlled. The issue remains open because a non-login transport and runner-bound execution evidence require the upstream change.

Type of Change

• 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

• Tests added or updated for changed behavior
• Existing tests cover changed behavior — justification:
• Tests not applicable — justification:
• Docs updated for user-facing behavior changes
• Docs not applicable — justification: No supported command, option, default, configuration, or documented result changes. Existing Deep Agents Code setup documentation remains accurate.
• Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
• Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: Codex Desktop reviewed the complete five-file diff against the nine-category security rubric. The PR removes two profile-sourcing layers, preserves argv boundaries and failure handling, introduces no credential or authorization change, and restores the pre-existing main-branch diagnostic parser rather than claiming an unattainable local trust boundary. The remaining upstream login-shell limitation is explicit and is not introduced or worsened here.
• Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Documentation Writer Review

• Documentation writer subagent reviewed the completed changes
• Result: no-docs-needed
• Evidence: The effective diff changes only internal Deep Agents Code smoke invocation and tests. docs/get-started/quickstart-langchain-deepagents-code.mdx already describes the dcode --version smoke check and launcher environment normalization accurately; no docs/, fern/, public error, or CLI reference surface changes.
• Agent: Codex Desktop

DGX Station Hardware Evidence

• Tested on DGX Station
• Tested commit:
• Station profile/scenario:
• Result:
• Supporting evidence:

Verification

• PR description includes a Signed-off-by: line and every new commit is signed for GitHub verification
• Normal pre-commit, commit-message, and pre-push hooks passed
• Targeted behavior tests pass: 44 focused CLI tests and 1 compiled-CLI dispatch test passed; CLI type checking passed
• Applicable broad gate passed — command/result:
• Quality Gates section completed with required justifications or waivers
• No secrets, API keys, or credentials committed
• npm run docs builds without warnings (doc changes only)
• Doc pages follow the style guide (doc changes only)
• New doc pages include SPDX header and frontmatter (new pages only)

---

Signed-off-by: Dongni Yang dongniy@nvidia.com
Signed-off-by: Apurv Kumaria akumaria@nvidia.com

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Terminal smoke execution now selects shell arguments by agent. Deep Agents Code uses the managed launcher with cleared startup-environment variables and a non-login shell. Other agents retain login-shell execution. Tests cover argument construction, command invocation, and exit-marker validation.

Changes

Terminal smoke execution

Layer / File(s)	Summary
Agent-specific smoke command construction and validation src/lib/agent/terminal-smoke.ts, src/lib/agent/terminal-smoke.test.ts, test/cli/connect-terminal-agent.test.ts	buildAgentSmokeArgs routes Deep Agents Code through the managed launcher with /bin/sh -c, cleared BASH_ENV and ENV, and no login shell. Other agents continue using sandbox exec with sh -lc. Exit parsing now requires exactly one marker.
Proxy shell boundary documentation src/lib/actions/sandbox/connect-inference-route-probe.ts, src/lib/actions/sandbox/connect-inference-route-probe.test.ts	Comments and test descriptions document OpenShell login-shell behavior, managed launcher handling, and parser protections for contaminated startup output.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing nested login shells in Deep Agents Code smoke execution.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/8624-dcode-probe-login-profile

---

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

[github-code-quality]

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit 82bd827 in the fix/8624-dcode-probe... branch remains at 96%, unchanged from commit ba0fbfc in the main branch.

---

_{Updated August 12, 2026 09:07 UTC}

[github-actions]

PR Review Advisor — Blocking findings reported

Advisor assessment: Blockers require maintainer review
Next action: Review the blockers below.
Findings: 1 blocker · 0 warnings · 0 suggestions

Model lanes

• GPT-5.6 Terra (primary): Completed · medium confidence · 1 blocker · 0 warnings · 0 suggestions
• Nemotron 3 Ultra (second opinion): Failed after a partial review · low confidence · 0 blockers · 0 warnings · 0 suggestions

Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests.

4 semantic terminology decisions

Terminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.

• define — transport-level login shell at src/lib/actions/sandbox/connect-inference-route-probe.ts:103: Define the term in the changed comment before using it as the remaining boundary.
• established — managed command at src/lib/agent/terminal-smoke.ts:37: Keep the established term with its existing repository meaning.
• established — diagnostic evidence at src/lib/agent/terminal-smoke.ts:36: Keep the established term and state that the smoke marker cannot establish managed-command execution.
• justified — startup-file read at src/lib/actions/sandbox/connect-inference-route-probe.ts:102: Keep the modifier because it identifies the profile-sourcing event relevant to the nested-shell risk.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite for the commit under review.

Recommended E2E: inference-routing

Manual-only E2E: full-e2e, hermes-e2e, network-policy, onboard-repair, onboard-resume
The manual PR workflow does not run these selectors for the commit under review. Run them from reviewed code on main.

Blockers

PRA-1 Blocker — Reject forged smoke exit markers

• Location: src/lib/agent/terminal-smoke.ts:19
• Category: correctness
• Problem: getSmokeExitCode accepts the first matching smoke exit marker anywhere in captured output. The OpenShell transport shell can emit this marker before the managed smoke command starts, so a prefixed success marker can make a failed or unstarted smoke command report success.
• Impact: A sandbox-user startup profile can forge a success marker before the managed command runs, allowing connect to report passed terminal smoke checks after a failed or skipped command.
• Fix: Accept only a final, unambiguous smoke-runner exit marker, and reject output that contains an earlier marker or lacks the required final marker.
• Verification: Inspect getSmokeExitCode and run its focused source test with a prefixed EXIT:0 marker followed by a failed smoke-command result.
• Test coverage: Add a terminal-smoke test where captured output contains a forged prefixed NEMOCLAW_AGENT_SMOKE_EXIT:0 marker and the managed smoke command fails; assert runAgentSmokeCommands returns ok: false.
• Evidence: src/lib/agent/terminal-smoke.ts:19 parses the first matching NEMOCLAW_AGENT_SMOKE_EXIT marker. src/lib/agent/terminal-smoke.ts:36-37 states that the upstream transport shell can emit the marker before the managed command starts. src/lib/agent/terminal-smoke.ts:84-87 treats exit code 0 as successful smoke evidence.

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[apurvvkumaria]

Security review — BLOCKED\n\nThe updated wording now describes the local hardening accurately: NemoClaw removes two nested login-shell layers, while the OpenShell transport still runs a sandbox-user startup profile before the managed launcher.\n\nOne unresolved trust-boundary blocker remains:\n\n- src/lib/agent/terminal-smoke.ts accepts the first NEMOCLAW_AGENT_SMOKE_EXIT marker found anywhere in captured output.\n- NVIDIA/OpenShell#2668 means a sandbox-user startup profile runs before the requested launcher.\n- That profile can print NEMOCLAW_AGENT_SMOKE_EXIT:0 and exit successfully before the real smoke command starts. The current parser can then report a successful smoke check without evidence that the repository-defined command ran.\n- Removing NemoClaw's nested shells reduces repeated profile execution, but it does not remove this earlier output source or the profile's side effects.\n\nA local fail-closed change could stop Deep Agents Code connect from falling back to smoke when the managed route result is indeterminate. That would change compatibility for older images and other unavailable-probe cases. The alternative is to wait for an upstream non-login exec contract and validate the managed boundary against it. This choice needs an explicit architecture and compatibility decision.\n\nI did not find a secret disclosure, dependency, cryptography, or privilege-escalation defect in the changed argv construction. The focused source tests pass 44/44, the compiled-CLI dispatch test passes, and CLI build, type checking, pre-commit, and pre-push checks pass. PR #8670 must remain unmerged until the trust-evidence decision above is resolved.

[prekshivyas]

Blocking security finding on latest PR commit a19233fb5.

getSmokeExitCode accepts the first NEMOCLAW_AGENT_SMOKE_EXIT marker in combined OpenShell output. The transport login shell runs before the managed launcher, so sandbox-user startup output can forge an earlier :0 marker and make a later failed smoke command report success.

Bind the accepted evidence to a marker only the smoke runner can produce, or reject any pre-runner marker before parsing the runner result. Add a regression with a preamble NEMOCLAW_AGENT_SMOKE_EXIT:0 followed by the runner's nonzero marker and assert runAgentSmokeCommands returns failure.

Addressed by d5039af: duplicate/forged smoke markers are rejected; focused terminal/onboard smoke tests and the exact-head pre-push typecheck passed.

[prekshivyas]

Blocking smoke-evidence integrity finding on latest PR commit 3656529d6.

src/lib/agent/terminal-smoke.ts:18-24 accepts exactly one static NEMOCLAW_AGENT_SMOKE_EXIT:0 marker. That rejects an extra forged marker, but the OpenShell transport still starts its login shell before the managed runner. A hostile startup file can print exactly one success marker and exit before the requested runner executes; the parser then reports smoke success even though no smoke command ran.

Please bind success to evidence produced by the managed runner/transport rather than a static line that startup output can replace, and add a regression where startup output emits one success marker and terminates before the runner. If a per-invocation identifier is used, its trust boundary must prevent startup code from learning and replaying it before execution.

Security rubric: 1 PASS (no credential flow); 2 FAIL (untrusted transport output can satisfy the parser); 3 PASS (no auth change); 4 PASS (no dependency change); 5 FAIL (early startup failure can become success); 6 PASS (no cryptography change); 7 PASS (no config weakening); 8 FAIL (the exactly-one test misses replacement rather than duplication); 9 FAIL (the evidence is not bound to runner execution). Files reviewed: the complete 5-file diff.

[Dongni-Yang]

Thanks — the finding is correct. matches.length === 1 rejects duplication but not replacement, so one forged marker followed by an early exit still parses as success. Before pushing a third attempt I want to check the approach with you, because I think the per-invocation identifier route is a dead end and I'd rather not spend another round on it.

A per-invocation identifier cannot meet the bar you set

Your constraint — "its trust boundary must prevent startup code from learning and replaying it before execution" — is not satisfiable here, on any delivery channel:

• argv — the startup file can read the whole command line: tr '\0' ' ' </proc/self/cmdline, or $BASH_EXECUTION_STRING.
• env — printenv; the environment is established before startup files run.
• stdin — a single IFS= read -r n consumes the payload before the runner execs. Per openshell-cli run.rs, the entire stdin payload is delivered into the sandbox before the login shell starts, so challenge-response is impossible in principle rather than merely leaky. It is also not plumbed: captureOpenshell does not forward opts.input (runtime.ts:68-79) and captureOpenshellCommand hardcodes stdio: ["ignore","pipe","pipe"] (client.ts:235).

A nonce would raise forgery from "hardcode a constant" to "parse a variable" without moving the boundary — the same class of fix already dismissed once.

Proposed instead: remove the pre-runner window, then order the evidence

1. No sandbox-writable startup file on the Deep Agents Code path. Add --env HOME=/usr/local/lib/nemoclaw to the dcode smoke argv. That directory is install -d -o root -g root -m 0755 (agents/langchain-deepagents-code/Dockerfile:36), so uid 999 cannot plant .profile, .bash_profile, or .bashrc there. With the existing BASH_ENV=/ENV= scrub, the transport login shell has no sandbox-writable hook left. This is byte-identical to what buildSandboxInferenceRouteProbeArgs already ships in this PR (connect-inference-route-probe.ts:95), and dcode-launcher.sh:19 re-exports HOME=/sandbox before the managed command runs, so dcode --version still sees the real home.

2. Ordered, runner-produced evidence. The runner emits NEMOCLAW_AGENT_SMOKE_BEGIN before the command and NEMOCLAW_AGENT_SMOKE_EXIT:<rc> after it. The parser accepts only: exactly one BEGIN, exactly one EXIT, and BEGIN before EXIT. Your exact scenario — one success marker, then exit before the runner — emits zero BEGIN and fails closed, with no identifier to learn or replay.

Against your rubric: 2 a result marker with no preceding begin marker is rejected, and on the dcode path the untrusted producer no longer executes; 5 startup output that prints one marker and exits fails closed, as does a runner killed mid-command (BEGIN, no EXIT); 8 the rule now covers replacement and ordering, not only duplication, on both markers; 9 BEGIN can only be printed after the transport finished sourcing startup files.

I reproduced the attack locally (hostile $HOME/.bash_profile printing one marker and exiting: today's parser returns 0) and confirmed both layers close it.

One thing I want your call on before I push

The other terminal agents keep sh -lc because their smoke commands rely on profile-provided PATH, and their HOME is sandbox-writable. For those, this gives real protection against accidental output, truncation, early exit, replayed markers, and any single forged marker — but not against sandbox-user startup code written specifically to emit both markers in order. I do not think that is closable while the transport starts a login shell (NVIDIA/OpenShell#2668 is the removal condition).

So I would add a fourth item to the Deep Agents Code entry under Documented Risk Acceptances in SECURITY.md stating exactly that. Since that is a security claim in the project's name, does that scoping work for you, or would you rather the non-dcode agents also move off the login shell in this PR? Happy to do the latter, but it changes their PATH resolution and I did not want to fold that in unannounced.

The exact-commit audit found that this blocker requires a trusted OpenShell transport guarantee that NemoClaw cannot provide in this PR. The accepted change removes NemoClaw-added login shells and records the remaining boundary. Dismissing this review before a full current-commit review.