feat(build): add --build-isolation for sandboxed builds#1073
feat(build): add --build-isolation for sandboxed builds#1073pavank63 wants to merge 2 commits intopython-wheel-build:mainfrom
Conversation
Add build isolation using ephemeral Unix users and Linux namespaces to prevent untrusted build backends from reading credentials or interfering with the host system. The isolation mechanism: - Creates an ephemeral Unix user (useradd/userdel ~10ms each) that cannot read credential files like .netrc (root:root 600) - Uses Linux namespaces via unshare for network (no routing), PID (process visibility), IPC (shared memory), and UTS (hostname) - setpriv drops privileges before entering namespaces, avoiding the setgroups() denial that affects runuser in user namespaces - Works in unprivileged containers (Podman/Docker) without --privileged or --cap-add SYS_ADMIN New CLI flag --build-isolation/--no-build-isolation supersedes --network-isolation for build steps. The flag threads through WorkContext, BuildEnvironment, and the build backend hook caller. Build dependency installation (uv pip install) explicitly opts out since it needs access to the local PyPI mirror. Signed-off-by: Pavan Kalyan Reddy Cherupally <pcherupa@redhat.com> Co-Authored-By: Claude <claude@anthropic.com>
When build isolation is enabled, remove environment variables listed in FROMAGER_SCRUB_ENV_VARS (comma-separated) from the build subprocess environment. This allows downstream build systems to strip sensitive variables (e.g. internal registry tokens, CI credentials) without hardcoding any variable names in fromager itself. Signed-off-by: Pavan Kalyan Reddy Cherupally <pcherupa@redhat.com> Co-Authored-By: Claude <claude@anthropic.com>
📝 WalkthroughWalkthroughThis pull request adds build isolation support to Fromager, extending the existing network isolation infrastructure. The changes introduce a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/fromager/__main__.py`:
- Around line 33-40: The current module-level call to
external_commands.detect_build_isolation() runs on import and creates/deletes a
system user for every CLI invocation; move this probe to be lazy by removing the
import-time call and instead implement a helper (e.g.,
_ensure_build_isolation_supported()) that calls
external_commands.detect_build_isolation() only when build isolation is actually
requested (the branch that checks a --build-isolation flag), and preserve the
exported flags SUPPORTS_BUILD_ISOLATION and BUILD_ISOLATION_ERROR by setting
them inside that helper (or returning results) so they are initialized only
on-demand; alternatively make the probe a safe no-op that simply verifies the
presence of the isolation script and that unshare --user --map-root-user -- true
succeeds (without creating users) before enabling isolation.
In `@src/fromager/run_build_isolation.sh`:
- Around line 56-62: The command existence checks using "command -v ip" and
"command -v hostname" use the wrong redirection order ("2>&1 >/dev/null") which
leaves stderr attached; change both checks to redirect stdout then stderr (e.g.,
">/dev/null 2>&1") so both streams are discarded when checking presence of ip
and hostname in the run_build_isolation.sh script.
- Around line 37-64: The EXIT trap that runs userdel for BUILD_USER never
executes because the script uses exec to replace the shell with setpriv; change
the invocation so the script does not exec (i.e., invoke setpriv/unshare in a
child process and wait for it) or explicitly perform cleanup after the child
returns: remove the exec before setpriv --reuid/--regid/... -- unshare ... and
instead spawn the command (preserving BUILD_USER, BUILD_UID, BUILD_GID usage),
capture its exit status, then run the existing trap/cleanup (userdel
"$BUILD_USER" 2>/dev/null || true) before exiting so ephemeral fmr_* accounts
are always removed.
- Around line 40-42: The current chmod command makes the build dir
world-accessible; instead change ownership to the ephemeral build user/group so
only that user (or its primary group) can access it: locate the block that
checks FROMAGER_BUILD_DIR and replace the chmod -R o+rwX "$FROMAGER_BUILD_DIR"
call with a chown/chgrp approach that sets ownership to "$BUILD_USER" (and/or
its primary group) and applies permissions for owner/group (not "other"), e.g.
chown -R "$BUILD_USER": "$FROMAGER_BUILD_DIR" and adjust mode to u+rwX,g+rwX
(with safe stderr redirection and || true to preserve existing error handling).
Ensure references to FROMAGER_BUILD_DIR and BUILD_USER (and any fmr_* ephemeral
user naming convention) are used so the correct directory/user is targeted.
- Around line 52-53: The unshare invocation in run_build_isolation.sh currently
omits the mount namespace flag; update the exec line that calls setpriv/unshare
(the command including "setpriv --reuid=... --regid=... --clear-groups -- \
unshare --uts --net --pid --ipc --fork --map-root-user -- \") to include --mount
(i.e., add --mount after --ipc) so the script actually creates a mount namespace
and enables making system dirs read-only and isolating /tmp as promised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f7e0a88e-542a-4c84-8bba-81cd8f699e26
📒 Files selected for processing (6)
src/fromager/__main__.pysrc/fromager/build_environment.pysrc/fromager/context.pysrc/fromager/dependencies.pysrc/fromager/external_commands.pysrc/fromager/run_build_isolation.sh
| try: | ||
| external_commands.detect_build_isolation() | ||
| except Exception as e: | ||
| SUPPORTS_BUILD_ISOLATION: bool = False | ||
| BUILD_ISOLATION_ERROR: str | None = str(e) | ||
| else: | ||
| SUPPORTS_BUILD_ISOLATION = True | ||
| BUILD_ISOLATION_ERROR = None |
There was a problem hiding this comment.
Probe runs on every CLI invocation — creates and deletes a real system user for fromager --help.
detect_build_isolation() executes at module import, which means every invocation of the CLI (including --help, --version, shell-completion calls, and subcommands that never use isolation) forks useradd/userdel under root. Combined with the EXIT-trap-vs-exec leak flagged in run_build_isolation.sh, this will steadily pollute /etc/passwd on any host that runs Fromager frequently.
Consider detecting lazily only when --build-isolation is requested, or make the probe a no-op (e.g., just check that the script exists and unshare --user --map-root-user -- true works) instead of actually creating a user:
🛠 Lazy-detect sketch
-try:
- external_commands.detect_build_isolation()
-except Exception as e:
- SUPPORTS_BUILD_ISOLATION: bool = False
- BUILD_ISOLATION_ERROR: str | None = str(e)
-else:
- SUPPORTS_BUILD_ISOLATION = True
- BUILD_ISOLATION_ERROR = None
+SUPPORTS_BUILD_ISOLATION: bool | None = None
+BUILD_ISOLATION_ERROR: str | None = None
+
+def _ensure_build_isolation_supported() -> None:
+ global SUPPORTS_BUILD_ISOLATION, BUILD_ISOLATION_ERROR
+ if SUPPORTS_BUILD_ISOLATION is not None:
+ return
+ try:
+ external_commands.detect_build_isolation()
+ except Exception as e:
+ SUPPORTS_BUILD_ISOLATION = False
+ BUILD_ISOLATION_ERROR = str(e)
+ else:
+ SUPPORTS_BUILD_ISOLATION = True…then call _ensure_build_isolation_supported() inside the if build_isolation: branch at line 252.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fromager/__main__.py` around lines 33 - 40, The current module-level call
to external_commands.detect_build_isolation() runs on import and creates/deletes
a system user for every CLI invocation; move this probe to be lazy by removing
the import-time call and instead implement a helper (e.g.,
_ensure_build_isolation_supported()) that calls
external_commands.detect_build_isolation() only when build isolation is actually
requested (the branch that checks a --build-isolation flag), and preserve the
exported flags SUPPORTS_BUILD_ISOLATION and BUILD_ISOLATION_ERROR by setting
them inside that helper (or returning results) so they are initialized only
on-demand; alternatively make the probe a safe no-op that simply verifies the
presence of the isolation script and that unshare --user --map-root-user -- true
succeeds (without creating users) before enabling isolation.
| trap 'userdel "$BUILD_USER" 2>/dev/null || true' EXIT | ||
|
|
||
| # Make build dir writable by ephemeral user if set | ||
| if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then | ||
| chmod -R o+rwX "$FROMAGER_BUILD_DIR" 2>/dev/null || true | ||
| fi | ||
|
|
||
| # --- Enter namespaces as ephemeral user --- | ||
| # setpriv drops to the ephemeral user, then unshare creates namespaces. | ||
| # --map-root-user maps the ephemeral user to UID 0 inside the namespace | ||
| # so it can run ip/hostname. | ||
|
|
||
| BUILD_UID=$(id -u "$BUILD_USER") | ||
| BUILD_GID=$(id -g "$BUILD_USER") | ||
|
|
||
| exec setpriv --reuid="$BUILD_UID" --regid="$BUILD_GID" --clear-groups -- \ | ||
| unshare --uts --net --pid --ipc --fork --map-root-user -- \ | ||
| /bin/bash -c ' | ||
| # bring loopback up | ||
| if command -v ip 2>&1 >/dev/null; then | ||
| ip link set lo up | ||
| fi | ||
| # set hostname | ||
| if command -v hostname 2>&1 >/dev/null; then | ||
| hostname localhost | ||
| fi | ||
| exec "$@" | ||
| ' -- "$@" |
There was a problem hiding this comment.
Ephemeral user leaks: EXIT trap never fires after exec.
exec at line 52 replaces the current shell, so Bash never reaches its exit path and the trap ... EXIT set on line 37 is discarded. Every successful build (the common case) leaves a fmr_* entry behind in /etc/passwd//etc/shadow, and they accumulate indefinitely. Only the failure-before-exec path currently cleans up.
Run the child without exec and let the trap do the cleanup, or clean up explicitly:
🛠 Suggested fix
-exec setpriv --reuid="$BUILD_UID" --regid="$BUILD_GID" --clear-groups -- \
- unshare --uts --net --pid --ipc --fork --map-root-user -- \
- /bin/bash -c '
+setpriv --reuid="$BUILD_UID" --regid="$BUILD_GID" --clear-groups -- \
+ unshare --uts --net --pid --ipc --fork --map-root-user -- \
+ /bin/bash -c '
# bring loopback up
if command -v ip 2>&1 >/dev/null; then
ip link set lo up
fi
# set hostname
if command -v hostname 2>&1 >/dev/null; then
hostname localhost
fi
exec "$@"
' -- "$@"
+status=$?
+exit "$status"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| trap 'userdel "$BUILD_USER" 2>/dev/null || true' EXIT | |
| # Make build dir writable by ephemeral user if set | |
| if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then | |
| chmod -R o+rwX "$FROMAGER_BUILD_DIR" 2>/dev/null || true | |
| fi | |
| # --- Enter namespaces as ephemeral user --- | |
| # setpriv drops to the ephemeral user, then unshare creates namespaces. | |
| # --map-root-user maps the ephemeral user to UID 0 inside the namespace | |
| # so it can run ip/hostname. | |
| BUILD_UID=$(id -u "$BUILD_USER") | |
| BUILD_GID=$(id -g "$BUILD_USER") | |
| exec setpriv --reuid="$BUILD_UID" --regid="$BUILD_GID" --clear-groups -- \ | |
| unshare --uts --net --pid --ipc --fork --map-root-user -- \ | |
| /bin/bash -c ' | |
| # bring loopback up | |
| if command -v ip 2>&1 >/dev/null; then | |
| ip link set lo up | |
| fi | |
| # set hostname | |
| if command -v hostname 2>&1 >/dev/null; then | |
| hostname localhost | |
| fi | |
| exec "$@" | |
| ' -- "$@" | |
| trap 'userdel "$BUILD_USER" 2>/dev/null || true' EXIT | |
| # Make build dir writable by ephemeral user if set | |
| if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then | |
| chmod -R o+rwX "$FROMAGER_BUILD_DIR" 2>/dev/null || true | |
| fi | |
| # --- Enter namespaces as ephemeral user --- | |
| # setpriv drops to the ephemeral user, then unshare creates namespaces. | |
| # --map-root-user maps the ephemeral user to UID 0 inside the namespace | |
| # so it can run ip/hostname. | |
| BUILD_UID=$(id -u "$BUILD_USER") | |
| BUILD_GID=$(id -g "$BUILD_USER") | |
| setpriv --reuid="$BUILD_UID" --regid="$BUILD_GID" --clear-groups -- \ | |
| unshare --uts --net --pid --ipc --fork --map-root-user -- \ | |
| /bin/bash -c ' | |
| # bring loopback up | |
| if command -v ip 2>&1 >/dev/null; then | |
| ip link set lo up | |
| fi | |
| # set hostname | |
| if command -v hostname 2>&1 >/dev/null; then | |
| hostname localhost | |
| fi | |
| exec "$@" | |
| ' -- "$@" | |
| status=$? | |
| exit "$status" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fromager/run_build_isolation.sh` around lines 37 - 64, The EXIT trap that
runs userdel for BUILD_USER never executes because the script uses exec to
replace the shell with setpriv; change the invocation so the script does not
exec (i.e., invoke setpriv/unshare in a child process and wait for it) or
explicitly perform cleanup after the child returns: remove the exec before
setpriv --reuid/--regid/... -- unshare ... and instead spawn the command
(preserving BUILD_USER, BUILD_UID, BUILD_GID usage), capture its exit status,
then run the existing trap/cleanup (userdel "$BUILD_USER" 2>/dev/null || true)
before exiting so ephemeral fmr_* accounts are always removed.
| if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then | ||
| chmod -R o+rwX "$FROMAGER_BUILD_DIR" 2>/dev/null || true | ||
| fi |
There was a problem hiding this comment.
chmod -R o+rwX makes the build dir world-accessible, not ephemeral-user-accessible.
o+rwX sets the "other" bits, so every user on the host/container — not just $BUILD_USER — gains read/write. With parallel builds each running under a different fmr_* user, one build's ephemeral user can read and tamper with another's build tree, defeating the isolation this PR advertises. Prefer chown/chgrp to the ephemeral user (or its primary group) instead:
🛠 Suggested fix
-if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then
- chmod -R o+rwX "$FROMAGER_BUILD_DIR" 2>/dev/null || true
-fi
+if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then
+ chown -R "$BUILD_USER":"$BUILD_USER" "$FROMAGER_BUILD_DIR" 2>/dev/null || true
+fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then | |
| chmod -R o+rwX "$FROMAGER_BUILD_DIR" 2>/dev/null || true | |
| fi | |
| if [ -n "${FROMAGER_BUILD_DIR:-}" ] && [ -d "$FROMAGER_BUILD_DIR" ]; then | |
| chown -R "$BUILD_USER":"$BUILD_USER" "$FROMAGER_BUILD_DIR" 2>/dev/null || true | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fromager/run_build_isolation.sh` around lines 40 - 42, The current chmod
command makes the build dir world-accessible; instead change ownership to the
ephemeral build user/group so only that user (or its primary group) can access
it: locate the block that checks FROMAGER_BUILD_DIR and replace the chmod -R
o+rwX "$FROMAGER_BUILD_DIR" call with a chown/chgrp approach that sets ownership
to "$BUILD_USER" (and/or its primary group) and applies permissions for
owner/group (not "other"), e.g. chown -R "$BUILD_USER": "$FROMAGER_BUILD_DIR"
and adjust mode to u+rwX,g+rwX (with safe stderr redirection and || true to
preserve existing error handling). Ensure references to FROMAGER_BUILD_DIR and
BUILD_USER (and any fmr_* ephemeral user naming convention) are used so the
correct directory/user is targeted.
| exec setpriv --reuid="$BUILD_UID" --regid="$BUILD_GID" --clear-groups -- \ | ||
| unshare --uts --net --pid --ipc --fork --map-root-user -- \ |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
unshare --mount --map-root-user unprivileged user namespace requirements
💡 Result:
To run unshare --mount --map-root-user as an unprivileged (non-root) user, the following requirements must be met: Kernel configuration: - CONFIG_USER_NS=y (user namespaces enabled at compile time). - CONFIG_NAMESPACES=y (general namespace support). Runtime kernel settings (check with cat /proc/sys/kernel/unprivileged_userns_clone; some distros like older Debian disable it): - kernel.unprivileged_userns_clone=1 (allows unprivileged creation of user namespaces). Set with sudo sysctl kernel.unprivileged_userns_clone=1. - Sufficient user.max_user_namespaces limit (default usually allows it; check /proc/sys/user/max_user_namespaces). User setup: - Entries in /etc/subuid and /etc/subgid for the user (e.g., username:100000:65536), as --map-root-user uses subordinate IDs to map the caller's UID/GID to root (0:0) inside the namespace. The newuidmap/newgidmap setuid helpers (provided by util-linux or shadow) use these. util-linux version: - Recent enough for --map-root-user (available since ~2.26; works in modern versions). Process state: - Not in a chroot (EPERM if caller's root dir differs from mount namespace root). - Caller must have write access to /proc/self/{uid_map,gid_map,setgroups} post-unshare (handled by unshare). How it works: 1. unshare creates a user namespace (CLONE_NEWUSER, unprivileged since Linux 3.8). 2. --map-root-user writes to uid_map/gid_map (e.g., 0 1) and setgroups=deny, gaining CAP_SYS_ADMIN in the new user namespace. 3. This CAP_SYS_ADMIN (in user ns owning the mount ns) allows unsharing/using mount namespace (CLONE_NEWNS) for mounts like bind mounts, proc, tmpfs (but not block devices). Examples from man pages work unprivileged: unshare --user --map-root-user sh -c whoami # outputs root unshare --map-root-user --mount mount --bind foo bar If it fails with "Operation not permitted": - Check/enable kernel.unprivileged_userns_clone. - Verify subuid/subgid (use id -u and check files). - Ensure kernel >=3.8, CONFIG_USER_NS=y (grep CONFIG_USER_NS /boot/config-$(uname -r)). - No seccomp/container restrictions (e.g., Docker blocks by default). As of 2026, most distros (Ubuntu 22+, Fedora, etc.) enable by default; older/secure setups may need tweaks.
Citations:
- 1: https://man7.org/linux/man-pages/man1/unshare.1.html
- 2: https://man7.org/linux/man-pages/man7/user_namespaces.7.html
- 3: https://manpages.ubuntu.com/manpages/jammy/man7/user_namespaces.7.html
- 4: https://man7.org/linux/man-pages/man7/user%5Fnamespaces.7.html
- 5: https://www.man7.org/linux/man-pages/man1/unshare.1.html
- 6: https://manpages.org/unshare
- 7: https://www.baeldung.com/linux/kernel-enable-user-namespaces
- 8: https://unix.stackexchange.com/questions/303213/how-to-enable-user-namespaces-in-the-kernel-for-unprivileged-unshare/303214
- 9: https://blog.mister-muffin.de/2015/10/25/unshare-without-superuser-privileges/
- 10: https://unix.stackexchange.com/questions/800126/config-namespaces-config-user-ns-and-user-max-namespaces
🏁 Script executed:
cat -n src/fromager/run_build_isolation.sh | head -60Repository: python-wheel-build/fromager
Length of output: 2580
🏁 Script executed:
# Search for Issue `#1019` references in the codebase
rg "1019" --type-list | head -20
rg "mount" src/fromager/run_build_isolation.shRepository: python-wheel-build/fromager
Length of output: 460
🏁 Script executed:
# Check the full context around lines 52-53 and look for any mount setup
sed -n '40,70p' src/fromager/run_build_isolation.shRepository: python-wheel-build/fromager
Length of output: 895
🏁 Script executed:
# Search for /tmp isolation or mount setup elsewhere
rg "/tmp" src/fromager/ -A 2 -B 2Repository: python-wheel-build/fromager
Length of output: 880
🏁 Script executed:
# Search for references to Issue 1019 or mount-related isolation setup
rg "1019\|mount" src/fromager/ -iRepository: python-wheel-build/fromager
Length of output: 53
🏁 Script executed:
# Check if there's any mount setup or planned isolation features in the codebase
rg "FROMAGER.*MOUNT\|ISOLATION.*MOUNT" -iRepository: python-wheel-build/fromager
Length of output: 53
🏁 Script executed:
# Look for any TODOs or FIXMEs related to mount isolation
rg "TODO\|FIXME" src/fromager/run_build_isolation.shRepository: python-wheel-build/fromager
Length of output: 53
🏁 Script executed:
# Check the __main__.py file more thoroughly for sandbox-isolation context
rg -A 10 -B 5 "mount.*PID.*IPC" src/fromager/__main__.pyRepository: python-wheel-build/fromager
Length of output: 541
Add --mount to unshare invocation to fulfill promised isolation.
The help text (--build-isolation) explicitly promises "mount, PID, IPC, and network namespace isolation" and "makes system directories read-only, and isolates /tmp," but the unshare command at lines 52–53 omits --mount. Without it:
- World-readable files in
$HOME(.config/*, SSH known_hosts, etc.) remain accessible /tmpis only isolated via sticky bit + ephemeral user, not per-build mount namespace- Cannot enforce read-only
/usror tmpfs overlays as promised
Add --mount after --ipc:
unshare --uts --net --pid --ipc --mount --fork --map-root-user -- \
The --map-root-user flag already grants sufficient CAP_SYS_ADMIN privilege for mount operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fromager/run_build_isolation.sh` around lines 52 - 53, The unshare
invocation in run_build_isolation.sh currently omits the mount namespace flag;
update the exec line that calls setpriv/unshare (the command including "setpriv
--reuid=... --regid=... --clear-groups -- \ unshare --uts --net --pid --ipc
--fork --map-root-user -- \") to include --mount (i.e., add --mount after --ipc)
so the script actually creates a mount namespace and enables making system dirs
read-only and isolating /tmp as promised.
| if command -v ip 2>&1 >/dev/null; then | ||
| ip link set lo up | ||
| fi | ||
| # set hostname | ||
| if command -v hostname 2>&1 >/dev/null; then | ||
| hostname localhost | ||
| fi |
There was a problem hiding this comment.
Redirection order is reversed.
2>&1 >/dev/null first points stderr at the current stdout (the terminal), then redirects stdout to /dev/null, so stderr still leaks to the caller. For command -v this is usually harmless because it only writes to stdout, but the idiom is a footgun — flip the order:
🛠 Suggested fix
- if command -v ip 2>&1 >/dev/null; then
+ if command -v ip >/dev/null 2>&1; then
ip link set lo up
fi
# set hostname
- if command -v hostname 2>&1 >/dev/null; then
+ if command -v hostname >/dev/null 2>&1; then
hostname localhost
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if command -v ip 2>&1 >/dev/null; then | |
| ip link set lo up | |
| fi | |
| # set hostname | |
| if command -v hostname 2>&1 >/dev/null; then | |
| hostname localhost | |
| fi | |
| if command -v ip >/dev/null 2>&1; then | |
| ip link set lo up | |
| fi | |
| # set hostname | |
| if command -v hostname >/dev/null 2>&1; then | |
| hostname localhost | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/fromager/run_build_isolation.sh` around lines 56 - 62, The command
existence checks using "command -v ip" and "command -v hostname" use the wrong
redirection order ("2>&1 >/dev/null") which leaves stderr attached; change both
checks to redirect stdout then stderr (e.g., ">/dev/null 2>&1") so both streams
are discarded when checking presence of ip and hostname in the
run_build_isolation.sh script.
tiran
left a comment
tiran
left a comment
There was a problem hiding this comment.
I would prefer to start with a design proposal first and come to a mutual agreement how we want to address build isolation.
2 participants
Summary
--build-isolationflag that sandboxes build backend subprocesses using ephemeral Unix users and Linux namespaces (network, PID, IPC, UTS).netrc(root:root 600), network namespace blocks exfiltration, PID/IPC namespaces prevent cross-processinterference
--privilegedor--cap-add SYS_ADMINFROMAGER_SCRUB_ENV_VARSenvironment variable for downstream build systems to strip sensitive env vars from build subprocessesTest plan
.netrcis unreadable by build subprocess (Permission denied)useradd/userdeloverhead is ~10ms eachuv pip installfor build deps correctly opts out of isolationCloses: #1019
🤖 Generated with Claude Code