[p-linnane]

Uninstalling or upgrading docker-desktop failed with Errno::EACCES when its install step symlink /usr/local/bin/kubectl was root-owned with mode 0700, because the uninstall step reads the link target before removing it. Install steps that already allow sudo now fall back to sudo readlink -n for that check, and a forked child reporting an error after its parent has failed no longer prints an EPIPE traceback ahead of the parent's error.

---

• 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 brew benchmark results.
• 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-6 Astra) drafted the change and tests, checked the tests fail without the change and ran brew lgtm --online; Claude Code (Opus 5.5) reviewed it, repeated that check independently and ran brew lgtm --online on this commit.

---

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 40226994-951f-4950-86d9-1e1d02ffa29c

---

📥 Commits

Reviewing files that changed from the base of the PR and between 1397358 and 840156b.

---

📒 Files selected for processing (4)

• Library/Homebrew/install_steps.rb
• Library/Homebrew/test/install_steps_spec.rb
• Library/Homebrew/test/utils/fork_spec.rb
• Library/Homebrew/utils/fork.rb

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📜 Recent review details

⏰ Context from checks skipped due to timeout. (19)

• GitHub Check: docker (x86_64 Ubuntu 26.04)
• GitHub Check: docker (x86_64 Ubuntu 24.04)
• GitHub Check: docker (arm64 Ubuntu 26.04)
• GitHub Check: docker (arm64 Ubuntu 24.04)
• GitHub Check: tests (macOS, 1/2)
• GitHub Check: tests (no Sorbet)
• GitHub Check: tests (load-only Linux)
• GitHub Check: tests (Linux, 1/2)
• GitHub Check: tests (online, 2/2)
• GitHub Check: workflow_syntax
• GitHub Check: tests (generic OS, 2/2)
• GitHub Check: tests (generic OS, 1/2)
• GitHub Check: tests (online, 1/2)
• GitHub Check: tests (load-only macOS)
• GitHub Check: tests (macOS, 2/2)
• GitHub Check: tests (Linux, 2/2)
• GitHub Check: syntax
• GitHub Check: docs
• GitHub Check: Analyze

---

🔇 Additional comments (4)

Library/Homebrew/utils/fork.rb (1)

102-104: LGTM!

---

Library/Homebrew/test/utils/fork_spec.rb (1)

32-54: LGTM!

---

Library/Homebrew/install_steps.rb (1)

1053-1053: LGTM!

Also applies to: 1310-1318, 1364-1364

---

Library/Homebrew/test/install_steps_spec.rb (1)

1694-1788: LGTM!

---

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Uninstall and removal steps can now inspect symbolic links that require elevated permissions when configured to use sudo, allowing matching links to be handled correctly while preserving nonmatching links.
  • Improved error handling when permission issues occur during background process reporting, preventing a broken communication pipe from causing an additional failure.

Walkthrough

Runner now reads symlink targets through a helper that can use elevated readlink when permitted. Child error reporting rescues Errno::EPIPE. Tests cover both changes.

Changes

Symlink inspection

Layer / File(s)	Summary
Read symlink targets with permitted elevation Library/Homebrew/install_steps.rb, Library/Homebrew/test/install_steps_spec.rb	Runner#remove and uninstall cleanup use read_symlink. The helper falls back to /usr/bin/readlink with sudo when Pathname#readlink raises Errno::EACCES and sudo is permitted. Tests cover allowed and denied elevation, matching and nonmatching targets, and failed elevated inspection.

Child error reporting

Layer / File(s)	Summary
Handle EPIPE while reporting child errors Library/Homebrew/utils/fork.rb, Library/Homebrew/test/utils/fork_spec.rb	report_forked_child_error rescues Errno::EPIPE and returns nil. A safe_fork test checks stderr output when the parent handler raises Errno::EACCES.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: mikemcquaid

---

Merge Risk | ⚪ Minimal · up to 84015

Merge Risk: ⚪ Minimal · up to 84015

The change lets uninstall and upgrade read protected symlinks when sudo is allowed, and it suppresses a spurious EPIPE traceback. No concrete merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 84015

Privileged symlink inspection is restricted to steps that already permit elevation. Existing target checks remain, and failed inspection stops removal. The added privileged operation has limited local exposure; no introduced vulnerability was established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated exposure is local to filesystem paths selected by existing install steps and privileged inspection of their symlink metadata. The changed reader does not execute the returned target or add new path sources or sudo-eligible step classes.

Trust Boundaries and Controls

• observed — Cask privileged-step dispatch remains parent-owned: the child requests a step index, and the parent checks request type, ordering, and sudo eligibility before executing the corresponding predeclared step. The fallback does not introduce a free-form privileged command request.

Resilience and Maintainability Implications

• observed — Uninstall skips missing or mismatched links, and inspection failure prevents deletion. Target validation and pathname removal remain separate, including removal-helper retries without expected-target revalidation. This concurrency limitation predates the PR; the change enables the same sequence for previously unreadable links but does not establish a new attacker capability.

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: reading protected install-step symlinks with sudo.
Description check		The description accurately explains the protected symlink failure, the sudo fallback, the child-process EPIPE handling, and the related tests.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[MikeMcQuaid]

Thanks!