Repository navigation
fix(vm): give guest-visible directories explicit modes under a strict service umask - #5147
vineethsai7 wants to merge 1 commit into
Conversation
Signed-off-by: Vineeth Sai Narajala <vnarajal@cisco.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2737ff6ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fs::create_dir_all(staging_dir.join("config")) | ||
| .map_err(|err| format!("create overlay config dir: {err}"))?; | ||
| create_disk_staging_dir(&staging_dir, 0o700)?; | ||
| create_disk_staging_dir(&staging_dir.join("upper"), 0o755)?; |
There was a problem hiding this comment.
On an upgraded packaged gateway that already has images/overlay-templates/sandbox-overlay-ext4-v1/<size>.ext4 from the old UMask=0077 code, overlay_template_image_ready and ensure_sandbox_overlay_template_image accept that file solely by its length, so this 0755 creation is never reached and every newly cloned overlay still has /upper at 0700; the sandbox user therefore still cannot traverse /newroot. Bump OVERLAY_TEMPLATE_CACHE_LAYOUT_VERSION (or validate and rebuild the cached inode mode) and cover the populated-cache upgrade path so the required sandbox e2e flow actually exercises the fix.
AGENTS.md reference: AGENTS.md:L163-L163
Useful? React with 👍 / 👎.
|
Closing this in favour of #5099, which was opened first and takes the same approach: explicit 0755 on the guest directories, whatever the service umask. |
Summary
Give VM guest root directories explicit permissions so a non-root sandbox can traverse its root filesystem when the gateway uses the packaged
UMask=0077. The capability probe's/proc/self/statuserror is the first visible failure after the guest switches to the sandbox UID.Related Issue
Closes #4014
Changes
UMask=0077, its overlay/upperwas copied into the disk as0700; the guest mounts that upper directory at/newroot. Set/upperand the image preparation/upper/srvto0755explicitly./workand/configdirectories at0700. No gateway service umask or host state directory permissions change.0755. Newly generated/etc/passwdand/etc/groupare0644;/etc/shadowand/etc/gshadoware0600. Existing/srv,/opt, and/etcdirectories from the image retain their modes.umask 0077, then check the resulting directory and account-file modes. The disk test reads inode modes from the formatted ext4 images.Release Notes
MicroVM guest disk creation now supports non-root sandboxes under the gateway's restrictive service umask.
Testing
The new disk regression test failed before the fix with
/upperat0700instead of0755, then passed after it. All of the following passed:cargo fmt --check --package openshell-driver-vm cargo clippy --quiet -p openshell-driver-vm --all-targets -- -D warnings cargo test --quiet -p openshell-driver-vmThe crate test run passed 270 library tests, 23 driver binary tests, 2 VM init tests, and 3 integration tests. A live VM startup check remains open.
Checklist