Repository navigation
fix: require chroot write grants for open side effects - #277
Open
jamesboyzj-design wants to merge 1 commit into
Open
jamesboyzj-design wants to merge 1 commit into
jamesboyzj-design wants to merge 1 commit into
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The virtual chroot checked the requested descriptor mode but missed open side effects. A read-only grant could therefore allow
O_RDONLY | O_TRUNCto erase an existing file orO_RDONLY | O_CREATto create a file. This change requires write authority for creation/truncation before any COW or host-open work, while preserving read-only descriptor access and read-only mount protection.This is exactly commit
70e258bfrom #274, cherry-picked onto maina0eb434as9d32abc, as requested in #274 (comment). It contains only the open-side-effect implementation, helper and regressions; no access-query or credential-checker changes. It does not close #150. #274 still contains the original shared commit; its effective diff will exclude this fix once this standalone change lands.Validation on Linux aarch64, kernel
7.0.14-orbstack-00380-ga7e0a2dc9535, Landlock ABI 8, Rust 1.96.1, Docker--init --security-opt seccomp=unconfinedwithout privileged mode:a0eb434with identical test/helper sources; both pass here.HOME=/root; the exact case passes separately as root. The profile HOME case was explicitly run separately as root and passed. This is not a fully green unprivileged workspace claim.git diff --checkpassed. aarch64 uses the raw *at fallback for legacy open; distinct native x86_64 legacy coverage remains for this PR's CI. Checkpoint restore is unverified.Build:
cargo test --offline --locked --workspace --no-run --message-format=json; execute Cargo-reported test binaries with--test-threads=1as uid/gid 65534 with supplementary groups cleared. C smoke runs as root. Baseline regression filter:readonly_open_cannot_mutatein the core integration executable. Tests use native Linux/tmpfor process output.