fix: optimize Linux privilege dropping with setpriv and remove redundant env exports#47
Merged
fix: optimize Linux privilege dropping with setpriv and remove redundant env exports#47
Conversation
The environment variables were being set twice: 1. Via explicit 'export' commands in the shell command string 2. Via Command::env() method on the process Since Command::env() already passes the environment to the child process, the explicit exports in the shell command were redundant. This change simplifies the code to only use Command::env(). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Replace su with runuser for cleaner privilege dropping - Eliminates the need for a shell wrapper entirely - Uses --preserve-environment flag to maintain environment variables - Simpler and more direct approach for dropping privileges 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ammario
force-pushed
the
fix-redundant-env-exports
branch
from
565269d to
da75c4c
Compare
- Replace runuser with setpriv as recommended by runuser docs for non-PAM use cases - setpriv is lighter weight - no PAM, direct execve() wrapper - Uses SUDO_UID/SUDO_GID directly instead of username lookup - Simpler and more appropriate for our use case Also adds scripts/wait-pr-checks.sh for monitoring CI status. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…N_PROGRESS state - Use 'state' field instead of 'status' - Handle IN_PROGRESS state as pending - Simplify script with jq for JSON parsing 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ammario
changed the title
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
1 participant
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.
Summary
su→runuser→setprivfor optimal privilege droppingscripts/wait-pr-checks.shfor CI monitoringBackground
Previously, environment variables were being set twice:
This led to verbose debug output like:
Changes
suwith shell wrapperrunuserwith --preserve-environmentsetprivwith --reuid/--regid (final solution)Why setpriv?
After analysis of
runuservssetpriv:Implementation uses:
--reuid=<uid>and--regid=<gid>from SUDO_UID/SUDO_GID--init-groupsto properly initialize supplementary groupsTest plan
cargo build --profile fast🤖 Generated with Claude Code