Skip to content

Allow sudo in sandboxed Cask install steps - #23476

Merged
MikeMcQuaid merged 2 commits into
Homebrew:mainfrom
SSakutaro:fix-sandboxed-cask-sudo
Aug 9, 2026
Merged

Allow sudo in sandboxed Cask install steps#23476
MikeMcQuaid merged 2 commits into
Homebrew:mainfrom
SSakutaro:fix-sandboxed-cask-sudo

Conversation

@SSakutaro

Copy link
Copy Markdown
Contributor

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Codex (GPT-5.6) was used to investigate the regression, help implement the change, and draft the tests and PR description. I reviewed the diff, manually verified


While investigating Homebrew/homebrew-cask#279650, I installed the parallels Cask to reproduce its reported uninstall failure. The installation itself failed with the same Operation not permitted - /usr/bin/sudo error on multiple Macs.

Tracing the failure showed that structured Cask install steps have been executed inside a sandbox since #23461. Steps that explicitly specify sudo: true cannot launch /usr/bin/sudo from that sandbox on macOS and fail with EPERM.

For example, the parallels Cask contains:

postflight_steps do
  run "Parallels Desktop.app/Contents/MacOS/inittool",
      args: ["init"],
      base: :appdir,
      sudo: true
end

I think this regression occurred because #23461 moved structured Cask step blocks into a sandboxed subprocess without accounting for sudo: true. The tests and automated review appear to have covered only unprivileged commands, so the interaction between the macOS sandbox and /usr/bin/sudo may not have been exercised before merge.

Reproduction

$ brew install --cask parallels
==> Installing Cask parallels
==> Moving App 'Parallels Desktop.app' to '/Applications/Parallels Desktop.app'
Operation not permitted - /usr/bin/sudo
Error: Failure while executing; `/usr/bin/sudo -E -- /Applications/Parallels\ Desktop.app/Contents/MacOS/inittool
init` exited with 127.

Change

This change adds a sandbox rule that allows /usr/bin/sudo to execute without inheriting the sandbox when a structured step explicitly sets sudo: true.

All other structured step processing remains sandboxed. The exception is not added to step blocks that do not explicitly request sudo.

This preserves the structured install-step authoring enforced by #23475 while restoring support for steps that explicitly require elevated privileges.

Unit tests cover:

  • Generation of the process-exec rule for /usr/bin/sudo with the no-sandbox modifier.
  • Addition of that rule when a structured Cask step specifies sudo: true.

The unit tests verify that the correct sandbox rule is added without invoking real sudo. I separately verified the actual privileged execution by manually installing the parallels Cask.

Verification

After applying this change, I manually verified that the original installation failure was resolved:

$ brew install --cask parallels
🍺  parallels was successfully installed!

I also verified that another Cask could be removed successfully:

$ brew remove --cask lg-onscreen-control

brew remove --cask parallels still fails due to a separate issue: must_succeed: false is lost while serialising Cask artifact data, causing the pkill command in its uninstall stanza to be treated as mandatory.

That issue is unrelated to the sandboxed sudo regression addressed by this PR and will be submitted separately.

Copilot AI lite review requested due to automatic review settings August 9, 2026 04:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request restores support for structured Cask install steps that explicitly require elevation (sudo: true) when those steps are executed inside Homebrew’s sandboxed subprocesses. It does so by adding an explicit sandbox rule permitting /usr/bin/sudo to exec with the no-sandbox modifier when a step requests sudo, while keeping all other structured step execution sandboxed.

Changes:

  • Add Sandbox#allow_process_exec to emit process-exec rules with an optional no-sandbox modifier.
  • Allow /usr/bin/sudo to run outside the sandbox for sandboxed structured install steps when any step has sudo: true.
  • Add unit tests covering both the sandbox rule generation and the Cask install-steps behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
Library/Homebrew/sandbox.rb Adds a helper to allow process-exec rules with an optional no-sandbox modifier.
Library/Homebrew/cask/artifact/install_steps.rb Conditionally adds a sandbox exception for /usr/bin/sudo when structured steps request sudo: true.
Library/Homebrew/test/sandbox_shared_spec.rb Tests that allow_process_exec produces a process-exec rule with no-sandbox and the expected path filter.
Library/Homebrew/test/cask/artifact/install_steps_spec.rb Tests that sandboxed structured steps requesting sudo: true add the /usr/bin/sudo (no-sandbox) exec allowance.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@SSakutaro

Copy link
Copy Markdown
Contributor Author

It might be better to add a sudo check and run it outside the sandbox.
(@bevanjkay cited this PR as a reference in the following two PRs, but with just the current changes, it probably won't be fixed ?)
Homebrew/homebrew-cask#279917
Homebrew/homebrew-cask#279931
)
I'm going to take a shower now and will be right back as soon as I'm done.

@SSakutaro
SSakutaro requested a balanced review from Copilot August 9, 2026 06:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Library/Homebrew/test/cask/artifact/install_steps_spec.rb:150

  • The new tests cover only cases that grant the no-sandbox exception. Because this exception removes the main security boundary, also assert that an ordinary structured step does not call allow_process_exec; otherwise the PR's stated negative guarantee can regress unnoticed.

        Cask::Installer.new(cask, command: NeverSudoSystemCommand).install_artifacts
      end
    end
  end

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@MikeMcQuaid
MikeMcQuaid added this pull request to the merge queue Aug 9, 2026
Merged via the queue into Homebrew:main with commit c84082d Aug 9, 2026
42 checks passed
@SSakutaro
SSakutaro deleted the fix-sandboxed-cask-sudo branch August 9, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants