Skip to content

Harden cask uninstall path checks#21989

Merged
MikeMcQuaid merged 1 commit intomainfrom
fix-cask-uninstall-path-validation
Apr 11, 2026
Merged

Harden cask uninstall path checks#21989
MikeMcQuaid merged 1 commit intomainfrom
fix-cask-uninstall-path-validation

Conversation

@MikeMcQuaid
Copy link
Copy Markdown
Member

  • stop each_resolved_path from accepting . or .. segments that can escape intended delete targets
  • re-check glob matches after expansion so undeletable paths are filtered before rm -rf runs
  • add regressions in abstract_uninstall_spec.rb so traversal and glob bypasses stay covered

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests (excluding integration tests) for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

  • AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

Used OpenAI Codex xhigh and manually reviewed all changes.


Copilot AI review requested due to automatic review settings April 10, 2026 19:40
@MikeMcQuaid MikeMcQuaid enabled auto-merge April 10, 2026 19:41
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens Homebrew Cask uninstall/zap deletion path handling to reduce path traversal and unsafe deletions, and adds regression tests to keep these protections in place.

Changes:

  • Tighten each_resolved_path validation to reject paths containing ./.. segments.
  • Filter out undeletable paths after glob expansion (instead of checking only the pre-glob path).
  • Add RSpec coverage for traversal rejection and post-glob undeletable filtering for both uninstall and zap.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
Library/Homebrew/cask/artifact/abstract_uninstall.rb Strengthens each_resolved_path path validation and filters undeletable glob results before deletion.
Library/Homebrew/test/cask/artifact/abstract_uninstall_spec.rb Adds regression tests for traversal-segment rejection and undeletable glob-match filtering for uninstall/zap.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- validate cask delete paths before `~` normalization so `.` and
  `..` cannot be hidden inside user-relative directives
- keep warnings distinct for relative paths versus absolute paths
  with relative segments so rejected directives are easier to audit
- cover `./`, `~/../`, post-glob undeletable matches, and
  `Errno::EPERM` guidance in `abstract_uninstall_spec.rb`
@MikeMcQuaid MikeMcQuaid force-pushed the fix-cask-uninstall-path-validation branch from 0b98e8f to 4275592 Compare April 11, 2026 06:47
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Apr 11, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 11, 2026
@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Apr 11, 2026
Merged via the queue into main with commit ffbc592 Apr 11, 2026
38 checks passed
@MikeMcQuaid MikeMcQuaid deleted the fix-cask-uninstall-path-validation branch April 11, 2026 07:33
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