fix(shields): carry the live landlock stanza into the permissive policy - #8463
Conversation
OpenShell applies landlock at sandbox startup and rejects any later policy whose landlock stanza differs from the one the sandbox started with. An agent that ships no permissive policy of its own resolves to the OpenClaw document, whose `best_effort` contradicts a baseline such as Deep Agents Code's `strict`, so `shields down --policy permissive` was refused and `backup-all` could not open its unlock window on that sandbox. Carry the live landlock stanza into the composed document so it proposes no landlock change. The composer can only restate what the sandbox is already running, so it never relaxes the value, and landlock is startup-sealed so the running process is unaffected either way. Deep Agents Code is the only shipping agent with a landlock delta, which matches the reported scope: the failure follows the agent across macOS, Ubuntu, a GPU host, and WSL ARM, while OpenClaw is unaffected. Refs #8461 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
Capitalize Landlock in prose per the controlled word list, name OpenShell as the actor that refuses the policy, and retitle the equal-stanza test so it does not read as the absent-stanza branch it sits beside. Refs #8461 Signed-off-by: Dongni Yang <dongniy@nvidia.com>
📝 WalkthroughWalkthrough
ChangesLandlock policy preservation
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/lib/shields/permissive-runtime.ts`:
- Around line 159-161: Update the early-return condition in the permissive
runtime policy function to also require live?.landlock === undefined, ensuring a
live policy containing only landlock generates a runtime policy instead of
returning basePermissivePath. Add a regression test covering landlock without
filesystem paths, while preserving deny-by-default and fail-closed behavior.
🪄 Autofix
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: 1a4650a8-a338-4937-8a97-17fce1a936d9
📒 Files selected for processing (2)
src/lib/shields/permissive-runtime.tstest/permissive-runtime.test.ts
PR Review Advisor — No blocking findings reportedAdvisor assessment: No blocking advisor findings reported Model lanes
1 terminology difference from the second opinionAdvisory only. These are normalized differences from the primary terminology receipt.
1 additional E2E selection from the second opinionAdvisory only. The primary lane did not select these E2E jobs or targets.
Second-opinion terminology and E2E selections are advisory. Live E2E does not run automatically for pull requests. 2 semantic terminology decisionsTerminology decisions are advisory. They affect the assessment only when a separate finding identifies concrete semantic impact.
E2E guidanceAdvisory only. A maintainer can dispatch the default E2E suite against this exact revision. Recommended E2E: 2 optional E2E recommendations
This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer refresh complete for the current branch revision (
The earlier published branch had a fully green CI run. Fresh checks for the updated branch are queued; I have not retried or bypassed any outage-affected work. This PR is mergeable but still blocked on an independent human approval and completion of the new required checks. No product or unresolved architecture decision remains. |
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
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. |
|
Maintainer follow-up on the current branch revision:
The remaining gate is independent maintainer approval; no further branch change is pending. |
cv
left a comment
There was a problem hiding this comment.
Please refresh this branch onto the current main commit and rerun the required checks. The current base SHA is dd09a9ff, while main is cc4b816c; repository policy does not permit approval of stale Shields policy code. Carrying the live Landlock stanza preserves the startup-sealed value, and the tests cover different, matching, absent, and Landlock-only policy shapes; I found no additional correctness or security defect.
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
|
Maintainer follow-up after addressing the requested refresh:
The PR description continues to record that a live Deep Agents Code sandbox was unavailable on the contributor host. The code-path regression, checked-in strict-versus-best-effort policy replay, authoritative OpenShell validation, and prior required CI provide the available evidence; no required gate currently requests additional live evidence. Fresh required checks are now running. No product or unresolved architecture decision remains. The branch is waiting only for those checks and independent human re-review. |
The branch refresh addresses the stale-base request. Required checks remain separate from code-review findings.
Summary
shields down --policy permissivecomposed a document that carried the static base'slandlockstanza, so for an agent whose startup baseline differs it proposed a Landlock change on a live sandbox and OpenShell refused it. The composer now carries the live sandbox'slandlockstanza into the document, so the apply proposes no Landlock change and bothshields downand thebackup-allunlock window succeed.Related Issue
Closes #8461
Changes
src/lib/shields/permissive-runtime.ts: inbuildRuntimePermissivePolicy, copy the live policy'slandlockstanza onto the composed base when the live policy has one, alongside the existing filesystem union. Docblock updated, since it previously said only the two path lists are merged.test/permissive-runtime.test.ts: four cases cover differing, matching, absent, and Landlock-only live stanzas. The Landlock-only regression confirms that the generated runtime policy preservesstrictcompatibility.Why this is the right layer
OpenShell does not reject sending a
landlockstanza to a live sandbox; it rejects a changed one.crates/openshell-server/src/grpc/validation.rs:571is a comparison insidevalidate_static_fields_unchanged:The rejected input is authored by NemoClaw.
resolvePermissivePolicyPathreturns the agent's own permissive policy only when it ships one; today that is onlyagents/openclaw/andagents/hermes/. Everything else falls back tonemoclaw-blueprint/policies/openclaw-sandbox-permissive.yaml:best_effortbest_effort(own file)best_effortbest_effort(own file)strictbest_effort(fallback)Landlock is the only startup-sealed field that differs:
include_workdirmatches,process.run_as_user/run_as_groupmatch, the filesystem lists are unioned to a superset so nothing is removed, andnetwork_policiesis dynamically updatable. That table also matches the reported scope, where the failure follows the agent across macOS, Ubuntu, a GPU host, and WSL ARM while OpenClaw is unaffected — an agent-correlated, platform-independent fingerprint points at a checked-in file rather than a host constraint.This is non-weakening. The composer can only ever restate the value the sandbox is already running, so it cannot relax Landlock, and because Landlock is applied at startup the running process is unaffected either way.
An alternative fix is a new
agents/langchain-deepagents-code/policy-permissive.yaml. This PR does not take it: that file must be hand-synced withpolicy-additions.yamlforever, and that drift is the bug classpermissive-runtime.tswas created to end (#3168, #3942, #3957). Carrying the live stanza fixes every agent, including future ones.Note on PR #8200
#8200's scope note states that the unlock "fundamentally cannot complete because OpenShell seals policy at startup" and defers this as a design question. The
validation.rscomparison above shows OpenShell seals the change, not the value, and NemoClaw is the party proposing the change. No OpenShell change is needed.Type of Change
Quality Gates
landlockstanza, and none claimsshields downcannot work on Deep Agents Code. The two Landlock troubleshooting entries (docs/reference/troubleshooting.mdx:1548,:2274) describe sandbox startup on kernels without Landlock and stay accurate.docs/reference/commands.mdx:1283documents the rejection contract generically and still holds — this removes one cause, not the contract. Perdocs/CONTRIBUTING.md:83-102the dated changelog entry belongs to the pre-tag release-note PR.Documentation Writer Review
no-docs-neededbackup-allcontracts, runtime-control guidance, Deep Agents Landlock policy guidance, and troubleshooting guidance passed with no edits. This change restores documented behavior by preserving the live startup-applied Landlock stanza during a Shields-down policy replacement. It does not change a command, flag, output contract, configuration, default, operator workflow, policy schema, support claim, or recovery action.npm run docscompleted with 0 errors; its 2 Fern warnings are nonblocking.DGX Station Hardware Evidence
Verification
Signed-off-by:line and every commit appears asVerifiedin GitHubpre-commit,commit-msg, andpre-pushhooks passed, ornpm run validate:prpassed after refreshingorigin/mainwhen hooks were skipped or unavailablemain, permissive-runtime tests → 16/16 passed; vLLM storage tests → 27/27 passed; Deep Agents profile-plugin tests → 26/26 passed; CLI type-check and all applicable scoped repository hooks passed. A full local Shields-flow rerun passed 33/34 twice; its only miss was the unchanged 100 ms auto-restore timing assertion completing in 66–76 ms, which had passed 34/34 before the mainline refresh and is unrelated to this two-file diff.npm testfor broad runtime/test-harness changes;npm run checkfor repo-wide validation/coverage changes — command/result:npm run docsbuilds without warnings (doc changes only)Verification gap to be aware of
This was not reproduced on a live Deep Agents Code sandbox. The contributor host runs glibc 2.31 against the gateway's required 2.32, so no local OpenShell gateway is available. The evidence is the upstream comparison cited above, a replay of the composition against the checked-in YAML documents, and the per-agent table matching the reporter's platform scope. Confirmation on the
ubuntu-repo-cloud-langchain-deepagents-codeE2E lane or by NV QA would close that gap, and I am glad to hold merge for it.Signed-off-by: Dongni Yang dongniy@nvidia.com
Summary by CodeRabbit
Bug Fixes
Tests