Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support bash in privileged mode #16948

Merged
merged 1 commit into from Mar 27, 2024
Merged

Support bash in privileged mode #16948

merged 1 commit into from Mar 27, 2024

Conversation

Bo98
Copy link
Member

@Bo98 Bo98 commented Mar 25, 2024

This enables "privileged" mode in bash (which despite the name doesn't necessary elevate anything). This will filter out even more environment variables. It also enables the ability for EUID to be different to UID. This is a powerful tool that can be used in certain environments to better handle cask sudo privileges and I would like to give users the option to use it.

We defer the safety of such scenarios to the user calling brew. Homebrew will check if EUID or UID is root and abort, which should prevent the most common misuse via setuid root executables. No further checks than that are made. The caller should assume that either the EUID or UID can be escalated to at any time.

For compatibility reasons, we will only use the EUID in formula build scripts and set UID to equal that.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

TIL about bash -p. From reading about it: this seems to make a look more sense for bin/brew and friends here so 👍🏻 from me.

@@ -44,9 +44,10 @@ _create_lock() {
[[ -x "${ruby}" ]] || ruby="$(type -P ruby)"
[[ -x "${python}" ]] || python="$(type -P python)"

if [[ -x "${ruby}" ]] && "${ruby}" -e "exit(RUBY_VERSION >= '1.8.7')"
# Use /dev/stdin, otherwise Ruby can error if uid != euid.
if [[ -x "${ruby}" ]] && "${ruby}" /dev/stdin <<<"exit(RUBY_VERSION >= '1.8.7')"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if [[ -x "${ruby}" ]] && "${ruby}" /dev/stdin <<<"exit(RUBY_VERSION >= '1.8.7')"
if [[ -x "${ruby}" ]] && "${ruby}" - <<<"exit(RUBY_VERSION >= '1.8.7')"

This is equivalent, no? Though I suppose /dev/stdin is more explicit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I've adjusted the comment to mention that

@Bo98 Bo98 merged commit a1cb45f into master Mar 27, 2024
25 checks passed
@Bo98 Bo98 deleted the privileged-bash branch March 27, 2024 04:55
@MikeMcQuaid
Copy link
Member

Looks good, thanks @Bo98!

@github-actions github-actions bot added the outdated PR was locked due to age label Apr 27, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants