Skip to content

fix(sandbox): handle per-path Landlock errors instead of abandoning entire ruleset#677

Merged
johntmyers merged 2 commits intomainfrom
fix/landlock-per-path-error-handling
Mar 30, 2026
Merged

fix(sandbox): handle per-path Landlock errors instead of abandoning entire ruleset#677
johntmyers merged 2 commits intomainfrom
fix/landlock-per-path-error-handling

Conversation

@johntmyers
Copy link
Copy Markdown
Collaborator

Summary

  • Fix a security defect where a single missing filesystem path (e.g., /app) silently disables all Landlock restrictions under best_effort mode
  • Add per-path error handling that skips inaccessible paths while applying remaining rules, with correct hard_requirement enforcement
  • Pre-filter system-injected baseline paths so missing paths never reach Landlock setup

Related Issue

Closes #664

Changes

Core fix (landlock.rs)

  • Extract try_open_path() helper that returns Ok(Some(fd)) on success, Ok(None) + warning in BestEffort, or Err in HardRequirement
  • Extract classify_path_error() to produce accurate human-readable reasons for different failure types (ENOENT, EACCES, ELOOP, ENAMETOOLONG, ENOTDIR)
  • Add rules_applied counter with zero-rule safety check — refuses to call restrict_self() on an empty ruleset (which would block all filesystem access)
  • Add observability logging: info! after ruleset build showing rules_applied and skipped counts

Defense-in-depth (lib.rs)

  • Pre-filter system-injected baseline paths in enrich_proto_baseline_paths() and enrich_sandbox_baseline_paths() using Path::exists()
  • User-specified paths are NOT pre-filtered — they surface as warnings (best_effort) or errors (hard_requirement) at Landlock apply time

Tests

  • 8 new unit tests: try_open_path behavior for BestEffort/HardRequirement/valid paths, classify_path_error for 6 error types

Documentation

  • docs/reference/policy-schema.md: Expanded Landlock section with compatibility mode behavior table
  • docs/sandboxes/policies.md: New "Baseline Filesystem Paths" section explaining filtering behavior and user-path vs system-path distinction
  • architecture/security-policy.md: Per-path error handling, baseline path filtering, zero-rule safety check documentation
  • architecture/sandbox.md: Updated Landlock isolation steps, added baseline path filtering note
  • Fixed stale ABI::V1 references (code uses ABI::V2)

Testing

  • cargo test -p openshell-sandbox — 330 tests pass (including 8 new Landlock tests)
  • cargo fmt --all -- --check — clean
  • cargo clippy -p openshell-sandbox — no new warnings

Checklist

  • Follows conventional commit format
  • Tests added for new functionality
  • User-facing documentation updated
  • Architecture documentation updated
  • No secrets or credentials committed
  • Changes scoped to the issue at hand

@johntmyers johntmyers requested a review from a team as a code owner March 30, 2026 15:03
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 30, 2026

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-03-30 20:20 UTC

@johntmyers johntmyers force-pushed the fix/landlock-per-path-error-handling branch 2 times, most recently from 5595e3f to 25054ae Compare March 30, 2026 15:43
@johntmyers johntmyers requested review from drew March 30, 2026 16:15
@johntmyers johntmyers self-assigned this Mar 30, 2026
@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Mar 30, 2026
@johntmyers johntmyers force-pushed the fix/landlock-per-path-error-handling branch 2 times, most recently from f1c4317 to 25054ae Compare March 30, 2026 18:07
…ntire ruleset

A single missing path (e.g., /app in containers without that directory)
caused PathFd::new() to propagate an error out of the entire Landlock
setup closure. Under BestEffort mode, this silently disabled all
filesystem restrictions for the sandbox.

Changes:
- Extract try_open_path() and classify_path_error() helpers that handle
  PathFd failures per-path instead of per-ruleset
- BestEffort mode: skip inaccessible paths with a warning, apply
  remaining rules
- HardRequirement mode: fail immediately on any inaccessible path
- Add zero-rule safety check to prevent applying an empty ruleset that
  would block all filesystem access
- Pre-filter system-injected baseline paths (e.g., /app) in enrichment
  functions so missing paths never reach Landlock
- Add unit tests for try_open_path, classify_path_error, and error
  classification for ENOENT, EACCES, ELOOP, ENAMETOOLONG, ENOTDIR
- Update user-facing docs and architecture docs with Landlock behavior
  tables, baseline path filtering, and compatibility mode semantics
- Fix stale ABI::V1 references in docs (code uses ABI::V2)

Closes #664
@johntmyers johntmyers force-pushed the fix/landlock-per-path-error-handling branch from 25054ae to 3bbdd34 Compare March 30, 2026 18:59
NotFound errors for stale baseline paths (e.g. /app persisted in the
server-stored policy but absent in this container) are expected in
best-effort mode. Downgrade from warn! to debug! so the message does
not leak into SSH exec stdout (the pre_exec hook inherits the tracing
subscriber whose writer targets fd 1).

Genuine errors (permission denied, symlink loops, etc.) remain at warn!
for operator visibility.

Also move custom_image e2e marker from /opt to /etc (a Landlock baseline
read-only path) since the security fix now properly enforces filesystem
restrictions.
@johntmyers johntmyers merged commit 758c62d into main Mar 30, 2026
10 checks passed
@johntmyers johntmyers deleted the fix/landlock-per-path-error-handling branch March 30, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: Landlock ruleset abandoned entirely when a single path does not exist

2 participants