Skip to content

fix(shields): carry the live landlock stanza into the permissive policy - #8463

Merged
cv merged 7 commits into
mainfrom
fix/8461-permissive-landlock
Aug 7, 2026
Merged

fix(shields): carry the live landlock stanza into the permissive policy#8463
cv merged 7 commits into
mainfrom
fix/8461-permissive-landlock

Conversation

@Dongni-Yang

@Dongni-Yang Dongni-Yang commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

shields down --policy permissive composed a document that carried the static base's landlock stanza, 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's landlock stanza into the document, so the apply proposes no Landlock change and both shields down and the backup-all unlock window succeed.

Related Issue

Closes #8461

Changes

  • src/lib/shields/permissive-runtime.ts: in buildRuntimePermissivePolicy, copy the live policy's landlock stanza 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 preserves strict compatibility.

Why this is the right layer

OpenShell does not reject sending a landlock stanza to a live sandbox; it rejects a changed one. crates/openshell-server/src/grpc/validation.rs:571 is a comparison inside validate_static_fields_unchanged:

if baseline.landlock != new.landlock {
    return Err(Status::invalid_argument(
        "landlock policy cannot be changed on a live sandbox (applied at startup)",
    ));
}

The rejected input is authored by NemoClaw. resolvePermissivePolicyPath returns the agent's own permissive policy only when it ships one; today that is only agents/openclaw/ and agents/hermes/. Everything else falls back to nemoclaw-blueprint/policies/openclaw-sandbox-permissive.yaml:

agent startup baseline permissive document result
openclaw best_effort best_effort (own file) accepted
hermes best_effort best_effort (own file) accepted
langchain-deepagents-code strict best_effort (fallback) refused

Landlock is the only startup-sealed field that differs: include_workdir matches, process.run_as_user/run_as_group match, the filesystem lists are unioned to a superset so nothing is removed, and network_policies is 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 with policy-additions.yaml forever, and that drift is the bug class permissive-runtime.ts was 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.rs comparison 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

  • 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 page documents how the permissive document composes its landlock stanza, and none claims shields down cannot 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:1283 documents the rejection contract generically and still holds — this removes one cause, not the contract. Per docs/CONTRIBUTING.md:83-102 the dated changelog entry belongs to the pre-tag release-note PR.
  • 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: A nine-category maintainer security review passed for the current branch revision with no findings. The composer can only copy the live sandbox's Landlock value into the replacement document. It cannot select a less restrictive value, and the running sandbox keeps the startup-applied control.
  • 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: Independent review of the effective two-file diff at the current revision, changed explanatory text, the Shields command and backup-all contracts, 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 docs completed with 0 errors; its 2 Fern warnings are nonblocking.
  • 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 commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run validate:pr passed after refreshing origin/main when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — after integrating current main, 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.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — 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)

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-code E2E 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

    • Permissive runtime policies now preserve the active sandbox’s Landlock configuration, preventing unintended security-setting changes.
    • Base Landlock settings remain intact when no active configuration is available.
    • Landlock-only configurations now correctly generate a temporary permissive policy instead of using the static policy path.
  • Tests

    • Added coverage for matching, differing, omitted, and Landlock-only configurations.

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>
@Dongni-Yang Dongni-Yang self-assigned this Aug 6, 2026
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

buildRuntimePermissivePolicy now preserves the live sandbox landlock stanza and stages a temporary policy when live Landlock state exists. Tests cover live values, base fallback, and Landlock-only policies.

Changes

Landlock policy preservation

Layer / File(s) Summary
Stage policies with live Landlock state
src/lib/shields/permissive-runtime.ts
The builder stages a temporary policy when live Landlock state requires merging and copies the live landlock stanza into the generated policy.
Validate Landlock compatibility
test/permissive-runtime.test.ts
Tests verify differing and matching live stanzas, base-stanza fallback, and staging for Landlock-only policies.

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

Suggested labels: area: policy, area: security, bug-fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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: preserving the live Landlock stanza in the permissive policy.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/8461-permissive-landlock

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

@github-code-quality

github-code-quality Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage in commit f2d7f7d in the fix/8461-permissive-... branch remains at 96%, unchanged from commit d8fbb6e in the main branch.


Updated August 07, 2026 10:10 UTC

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 64c04a8 and 974cc2a.

📒 Files selected for processing (2)
  • src/lib/shields/permissive-runtime.ts
  • test/permissive-runtime.test.ts

Comment thread src/lib/shields/permissive-runtime.ts
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — No blocking findings reported

Advisor assessment: No blocking advisor findings reported
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 1 warning · 0 suggestions
  • Model comparison: normalized findings differ; normalized terminology decisions differ; normalized E2E selections differ; Nemotron reported the same number of blockers, 1 more warning, the same number of suggestions.
1 terminology difference from the second opinion

Advisory only. These are normalized differences from the primary terminology receipt.

  • startup-sealed at src/lib/shields/permissive-runtime.ts:105: primary classified it as justified; the second opinion classified it as define.
1 additional E2E selection from the second opinion

Advisory only. The primary lane did not select these E2E jobs or targets.

  • state-backup-restore: The completed second-opinion lane identified E2E coverage that the primary lane omitted.

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

2 semantic terminology decisions

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

  • established — Landlock at src/lib/shields/permissive-runtime.ts:44: Keep `Landlock` as the established product name in explanatory text.
  • justified — startup-sealed at src/lib/shields/permissive-runtime.ts:105: Keep `startup-sealed` where the contrast with dynamically changeable policy state is needed.

E2E guidance

Advisory only. A maintainer can dispatch the default E2E suite against this exact revision.

Recommended E2E: cloud-inference, security-posture

2 optional E2E recommendations
  • ubuntu-repo-cloud-langchain-deepagents-code
  • shields-config

Workflow run details

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>
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Maintainer refresh complete for the current branch revision (6748f9c7f2):

  • Integrated current main with a signed, DCO-compliant merge commit through a normal push. Contributor commits and attribution are unchanged.
  • Rechecked the CodeRabbit finding. The Landlock-only early-return case is covered by the maintained branch, the thread is resolved, and CodeRabbit explicitly confirmed the fix.
  • Validation after the integration passed: permissive-runtime tests 16/16, Shields flow tests 34/34, plugin build, CLI build, CLI type-check, all applicable repository hooks, diff checks, secret scan, and repository checks.
  • All four branch commits are signed off and GitHub reports valid verification for the new maintainer merge as well as the three existing commits.
  • An independent documentation review passed with no edits. This restores already-documented Shields behavior and adds no CLI syntax, setting, default, workflow, operator choice, or support claim.
  • The nine-category sensitive-path review passed with no findings. The composer only restates the Landlock value already applied to the live sandbox; it cannot choose a less restrictive value. No credential, authentication, authorization, dependency, network destination, cryptography, or storage surface changes. Existing OpenShell rejection and Shields rollback paths remain fail-closed, with equal, differing, absent, and Landlock-only cases covered.

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.

@github-actions github-actions Bot added v0.0.105 Release target and removed v0.0.104 labels Aug 7, 2026
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Collaborator

Maintainer follow-up on the current branch revision:

  • The earlier CLI failures were unrelated timeouts in the Deep Agents profile-plugin and vLLM storage suites. Both suites passed locally (26/26 and 27/27), and the failed-job rerun passed.
  • After refreshing from current main, CLI shard 2 timed out at 5 seconds in unchanged src/lib/onboard/docker-gpu-sandbox-create-lifecycle.test.ts:383. The full file passed locally 10/10 in 560 ms. A failed-only rerun then passed shard 2, both aggregate jobs, and the complete CI run.
  • The effective diff remains limited to src/lib/shields/permissive-runtime.ts and test/permissive-runtime.test.ts. Focused validation passed: permissive runtime 16/16, Deep Agents profile plugin 26/26, vLLM storage 27/27, CLI type-check, and applicable scoped repository hooks.
  • The nine-category sensitive-path security review found no issues. The documentation writer independently concluded no-docs-needed; the docs build completed with 0 errors and 2 nonblocking Fern warnings.
  • All GitHub checks are now green. CodeRabbit’s actionable thread is resolved, and the current PR Review Advisor report recommends merging as-is with 0 blockers, 0 warnings, and 0 suggestions.

The remaining gate is independent maintainer approval; no further branch change is pending.

cv
cv previously requested changes Aug 7, 2026

@cv cv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

cv and others added 2 commits August 7, 2026 01:39
@apurvvkumaria

Copy link
Copy Markdown
Collaborator

Maintainer follow-up after addressing the requested refresh:

  • Integrated current main at d8fbb6e54 with a signed, DCO-compliant merge through a normal push. Contributor commits, authorship, and history are unchanged, and the effective PR diff remains limited to the Landlock composer and its regression tests.
  • Revalidated the combined Shields paths after the refresh. The permissive-runtime suite passed 16/16, the three affected Shields suites passed 68/68, CLI type-checking passed, and all applicable scoped repository hooks passed.
  • The only CodeRabbit finding remains resolved. It confirmed that a live policy containing only Landlock now produces a generated policy and preserves the running sandbox value. The current PR Review Advisor report is informational with no blockers, warnings, or suggestions.
  • The independent documentation writer returned no-docs-needed. This restores the documented shields down and backup-all behavior without changing commands, settings, defaults, policy schema, support claims, operator workflow, or recovery guidance. The docs build passed with 0 errors and 2 existing nonblocking Fern warnings.
  • The nine-category sensitive-path review passed with no findings. The change copies the Landlock stanza reported by the live sandbox and cannot choose a weaker value. OpenShell treats Landlock as startup-applied and rejects a live change, so this removes the contradictory replacement while retaining the existing fail-closed apply and rollback behavior. No credential, authentication, authorization, dependency, network, cryptography, logging, or storage surface changes.

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.

@cv
cv dismissed their stale review August 7, 2026 10:31

The branch refresh addresses the stale-base request. Required checks remain separate from code-review findings.

@cv
cv merged commit b1a8c13 into main Aug 7, 2026
62 of 66 checks passed
@cv
cv deleted the fix/8461-permissive-landlock branch August 7, 2026 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[All Platforms][Policy&Network] shields down and backup-all cannot relax policy on running DCode sandboxes

3 participants