Repository navigation
bug: the root allow-list is implemented twice and the audited helper has no caller #5190
Description
User Story
I run OpenShell sandboxes with the Podman and Kubernetes drivers, and I read through the isolation core (openshell-sandbox, openshell-binary-identity, openshell-isolation-interface) at 00528d24a. This item comes from reading the code. I have not hit it in production. It is one line of the umbrella issue #5179.
Problem Statement
crates/openshell-sandbox/src/linux/landlock.rs:76 defines open_root_allowlist and RootEntryHandle, documented as the audited primitive in linux/mod.rs: it opens only named root entries and checks identity before and after openat. It has no production caller (only its own tests use it). The mandatory baseline instead uses capability_free_baseline_entries in sandbox/linux/landlock.rs:176, which lists / by path with std::fs::read_dir and keeps every entry except the private name. Its comment says the private root is pinned and validated, but _private_root (line 191) is a local binding that is dropped when the function returns, before the lazy iterator opens any entry.
Impact / Why This Matters
Two implementations of the same mechanism will drift. Reviewers who audit crate::linux are not auditing the code that runs, and one comment overstates what the code holds.
Acceptance Criteria
- One implementation of the root enumeration lives in
crate::linux::landlockand the baseline uses it, or the unused helper and its contract text are removed. - The private-root comment matches what the code does (keep the fd alive inside the iterator or reword).
- Existing baseline tests still pass.
Reproduction Steps
Found by reading the code; not reproduced live. rg open_root_allowlist crates/ and confirm there is no non-test caller.
Environment
- OpenShell:
mainat00528d24a - OS: Fedora 44, Linux 7.2.14
- Deployment: Podman driver (local gateway); code read for the Kubernetes driver too
Activity
mrunalp commentedon Nov 3, 2026
Accepted for implementation in #5221.
Source inspection at
32dcc4a67: The audited open_root_allowlist helper has no production caller; the baseline uses a separate enumeration path and drops the private-root validation descriptor before lazy entry opening. Removing the unused helper and correcting the lifetime comment is a suitable scoped cleanup.This assessment is based on the source and the linked report/PR; I have not independently run deployment reproductions or the PR test suite. CI and code review still need to validate the fix.
refactor(sandbox): remove the unused Landlock root allow-list helper (N…
EmilienM commentedon Nov 11, 2026
Confirmed against
mainat00528d24a, with notes on both ACs to help triage.On AC1 (remove vs unify): only
abi_versionfromcrate::linux::landlockruns in production today, viaprobe_availabilityinsandbox/linux/landlock.rs. Unifying the baseline ontoopen_root_allowlistdoesn't fit the baseline's contract:open_root_allowlistcollects every handle up front in aVec<RootEntryHandle>. The baseline has to stay lazy (add the rule, drop the fd, open the next entry) because of the FD limit;capability_free_baseline_prepares_under_low_descriptor_limitpins that behavior atRLIMIT_NOFILE=64./bin, and those roots still have to work.So the removal branch seems right: delete the helper,
RootEntryHandle,validate_component, and their tests; keepabi_version; rewrite the module doc, which currently describes only the removed API. The statat-before/fstat-after race check stays in history if a real caller shows up later.On AC2: the validation half of the comment is accurate. The private root is opened with
O_DIRECTORY|O_NOFOLLOW, so a symlinked.openshellis rejected on the spot, andcapability_free_baseline_rejects_private_root_redirectcovers that. "Pin" is the wrong word: the fd is dropped when the function returns, before the lazy iterator opens anything, and holding it wouldn't enforce anything the name check plusO_NOFOLLOWdon't already do. Rewording the comment should be enough.