Skip to content

Allow non-admin user installs on macOS if prefix is user-writable #948

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

Allow non-admin user installs on macOS if prefix is user-writable #948

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
15084c1
Allow non-admin user installs on macOS if prefix is user-writable
willmcginnis Mar 3, 2025
a8b3dcb
Fixed improper formatting
willmcginnis Mar 3, 2025
98e98cc
Merge branch 'Homebrew:master' into patch-1
willmcginnis Mar 3, 2025
de18eb8
Adding sudoless mode incorporating suggestions
willmcginnis Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 48 additions & 6 deletions install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,18 @@ REQUIRED_GIT_VERSION=2.7.0 # HOMEBREW_MINIMUM_GIT_VERSION in brew.sh in Homebr
export HOMEBREW_NO_ANALYTICS_THIS_RUN=1
export HOMEBREW_NO_ANALYTICS_MESSAGE_OUTPUT=1

macos_sudoless_mode() {
if [[ -n "${MACOS_SUDOLESS_SATISFIED-}" ]]
then
return "${MACOS_SUDOLESS_SATISFIED}"
fi

[[ -d "${HOMEBREW_PREFIX}" ]] && "${CHMOD[@]}" -R "ug+w" "${HOMEBREW_PREFIX}"
Copy link
Contributor

Choose a reason for hiding this comment

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

You should test (read) the file mode rather than change it.

Copy link
Author

Choose a reason for hiding this comment

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

Testing would be a somewhat convoluted find and if there is an issue and you can fix it with a chmod, it does not make sense to do a find then a chmod if just the initial chmod would work and handle both cases.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the user does have sudo access and there are many many files under the prefix?

Copy link
Author

@willmcginnis willmcginnis Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Fair point. Doing a check then bailing out at the first sign of not writable is a way to make it less bad.

If there are a lot of files, and the user does have sudo, then yes, as it is now and also with find it would do a large check over all of them.

There is a chicken and egg of what comes first of running without sudo and making sure it can run without sudo.

My thoughts go to a begin/rescue or try/except pattern fitting the issue well but that’s a larger change.

MACOS_SUDOLESS_SATISFIED=$?

return "${MACOS_SUDOLESS_SATISFIED}"
}

unset HAVE_SUDO_ACCESS # unset this from the environment

have_sudo_access() {
Expand All @@ -225,6 +237,11 @@ have_sudo_access() {
return 1
fi

if [[ -n "${HOMEBREW_ON_MACOS-}" ]] && macos_sudoless_mode
then
return 1
fi

local -a SUDO=("/usr/bin/sudo")
if [[ -n "${SUDO_ASKPASS-}" ]]
then
Expand All @@ -247,7 +264,19 @@ have_sudo_access() {

if [[ -n "${HOMEBREW_ON_MACOS-}" ]] && [[ "${HAVE_SUDO_ACCESS}" -ne 0 ]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if [[ -n "${HOMEBREW_ON_MACOS-}" ]] && [[ "${HAVE_SUDO_ACCESS}" -ne 0 ]]
if [[ -n "${HOMEBREW_ON_MACOS-}" ]] && [[ "${HAVE_SUDO_ACCESS}" -ne 0 && ! -w "${HOMEBREW_PREFIX}" ]]

Copy link
Author

Choose a reason for hiding this comment

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

I figured that if it's not writable then that's handled by the earlier part, but this extra check would be okay if you think it is good to have.

then
abort "Need sudo access on macOS (e.g. the user ${USER} needs to be an Administrator)!"
abort "$(
cat <<EOABORT
Need sudo access on macOS (e.g. the user ${USER} needs to be an Administrator)!

Or for a standard (non-admin) user, run:

sudo mkdir -p "${HOMEBREW_PREFIX}"
sudo chown -R "${USER}:${GROUP}" "${HOMEBREW_PREFIX}"
sudo chmod -R "ug+w" "${HOMEBREW_PREFIX}"
Copy link
Contributor

@XuehaiPan XuehaiPan Mar 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It is not a good idea to add write permission to the group for all files/dirs. For example, you should remove the write access after installation:

chmod -R go-w "${HOMEBREW_PREFIX}/share/zsh"

Copy link
Author

Choose a reason for hiding this comment

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

An earlier objection was "What if the directories under it are not writable?"
so then if we only want some to be writable then
What is a clean way to determine what should have write and what shouldn't?

Copy link
Contributor

Choose a reason for hiding this comment

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

Adding the permission for the user is enough, right?

Copy link
Author

Choose a reason for hiding this comment

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

I was just matching other chmods in the script. But yes just the user is enough, agreed.


Then run this installer script again.
EOABORT
)"
fi

return "${HAVE_SUDO_ACCESS}"
Expand Down Expand Up @@ -861,11 +890,24 @@ fi
# Headless install may have failed, so fallback to original 'xcode-select' method
if should_install_command_line_tools && test -t 0
then
ohai "Installing the Command Line Tools (expect a GUI popup):"
execute "/usr/bin/xcode-select" "--install"
echo "Press any key when the installation has completed."
getc
execute_sudo "/usr/bin/xcode-select" "--switch" "/Library/Developer/CommandLineTools"
if macos_sudoless_mode
then
abort "$(
cat <<EOABORT
Installing without sudo on macOS requires manual Command Line Tools installation. Run:

sudo xcode-select --install

Then run this installer script again.
EOABORT
)"
else
ohai "Installing the Command Line Tools (expect a GUI popup):"
execute "/usr/bin/xcode-select" "--install"
echo "Press any key when the installation has completed."
getc
execute_sudo "/usr/bin/xcode-select" "--switch" "/Library/Developer/CommandLineTools"
fi
fi

if [[ -n "${HOMEBREW_ON_MACOS-}" ]] && ! output="$(/usr/bin/xcrun clang 2>&1)" && [[ "${output}" == *"license"* ]]
Expand Down
Loading