Skip to content

fix: fixed uid/gid not being set during session creation - #1184

Merged
twitchyliquid64 merged 2 commits into
gominimal:mainfrom
evanspearman:fixuid
Aug 7, 2026
Merged

fix: fixed uid/gid not being set during session creation#1184
twitchyliquid64 merged 2 commits into
gominimal:mainfrom
evanspearman:fixuid

Conversation

@evanspearman

@evanspearman evanspearman commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #1181

Summary

uid and gid were not being set properly when starting the sandbox. They were defaulting to the current uid/gid (hackoniwa default). This worked on Linux in previous testing because it was always tested from users with uid/gid 1000 (the same as the sandbox user we create), but it would have blown up if a user with a different uid/gid tried to use it. Because minimald is run as root within a VM on macOS, users were being incorrectly dumped into the session as root.

This PR fixes the issue by explicitly setting uid and gid to 1000 during sandbox creation.

Testing

Started a session on macOS and verified that I was running as the correct user

Checklist

  • Docs updated if behavior changed
  • [] BREAKING CHANGE: footer present if this is a breaking change

No doc changes necessary as this is just a bug fix. While this does technically change behaviour it changes the behaviour to the correct behaviour so we'll call it a fixing change rather than a breaking change.

Note

Fix UID/GID not being set to 1000 during sandbox session creation

  • Adds explicit uidmap(1000) and gidmap(1000) calls in Sandbox::new_container in lib.rs, overriding the hakoniwa default of inheriting the current process UID/GID.
  • Adds a just initramfs-nodocker recipe to build the initramfs using cargo zigbuild and a musl target without Docker/cross.
  • Adds a macOS-only just up-nodocker aggregate recipe that sequences the full stack build using the no-Docker initramfs path.
  • Behavioral Change: containers created via Sandbox::new_container now always run as UID/GID 1000 instead of the invoking user's IDs.

Macroscope summarized 6a378f4.

Summary by CodeRabbit

  • New Features

    • Added macOS-only commands to build and run the environment without Docker.
    • Added cross-compilation support for creating the required initramfs on macOS.

  • Bug Fixes

    • Aligned sandbox container user and group permissions with the configured sandbox account.

@evanspearman
evanspearman requested a review from a team as a code owner August 6, 2026 21:56
@CLAassistant

CLAassistant commented Aug 6, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change maps sandbox container identity to UID and GID 1000. It also adds macOS-only justfile targets that build initramfs artifacts and run the startup flow without Docker compilation.

Changes

Sandbox container identity

Layer / File(s) Summary
UID and GID mapping
crates/sandbox2/src/lib.rs		 Sandbox::new_container now maps the container UID and GID to 1000.

macOS no-Docker build flow

Layer / File(s) Summary
No-Docker build recipes
justfile		 The new macOS recipes cross-compile minimald, package the initramfs without Docker, build the required artifacts, and run the existing smoke flow.

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

Suggested reviewers: twitchyliquid64

Poem

A rabbit maps the sandbox bright,
UID and GID set just right.
No Docker hops through Mac skies,
Initramfs builds, smoke tests rise.
Thump, thump—the workflow flies!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.
Title check ✅ Passed The title clearly identifies the UID/GID session-creation fix and follows the repository's lower-case Conventional Commit format.
Description check ✅ Passed The description includes the required Summary, Testing, and Checklist sections with the cause, fix, test result, and issue reference.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

coderabbitai[bot]
coderabbitai Bot reviewed Aug 6, 2026
View reviewed changes

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
justfile (1)

122-123: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Lock the dependency graph for the no-Docker build.

initramfs-nodocker omits --locked, while the adjacent minvmd-build and minimal-cli recipes use it. If Cargo resolves dependencies, this path can produce a different minimald dependency graph. Add --locked, or confirm the equivalent option for the repository’s cargo-zigbuild version.

Proposed change 
-    cargo zigbuild -p minimald --profile initramfs \
+    cargo zigbuild -p minimald --profile initramfs --locked \
       --target {{musl-target}} --features {{features}}
🤖 Prompt for 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.

In `@justfile` around lines 122 - 123, Update the initramfs-nodocker recipe’s
cargo zigbuild command to pass --locked, using the same dependency-locking
behavior as the adjacent minvmd-build and minimal-cli recipes.
🤖 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.

Nitpick comments:
In `@justfile`:
- Around line 122-123: Update the initramfs-nodocker recipe’s cargo zigbuild
command to pass --locked, using the same dependency-locking behavior as the
adjacent minvmd-build and minimal-cli recipes.
ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0ded0198-72d8-47f4-8388-a4ebf1fe1013

📥 Commits

Reviewing files that changed from the base of the PR and between f5a9a98 and b845e8a.

📒 Files selected for processing (2)
  • crates/sandbox2/src/lib.rs
  • justfile

@twitchyliquid64
twitchyliquid64 enabled auto-merge (rebase) August 7, 2026 02:03
@twitchyliquid64
twitchyliquid64 merged commit 8bfe0dd into gominimal:main Aug 7, 2026
31 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments
@twitchyliquid64 twitchyliquid64 twitchyliquid64 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Current user is root when attaching to a session

3 participants

@evanspearman @CLAassistant @twitchyliquid64