unpack_strategy/zip: move volume labels by basename

[dduugg]

The macOS ZIP strategy re-extracts volume label members with ditto, because unzip skips them, and it learns their names by scraping unzip's stderr. That scraped name was then used on both sides of FileUtils.mv. Passing a full destination path makes FileUtils call File.directory? on it, which follows a symlink, so a member unzip had already extracted under the same name could redirect the move. An archive carrying a volume label and a symlink with identical names therefore moves the label's content to wherever the symlink points, outside the unpack directory.

Handing FileUtils.mv the directory instead lets it derive the leaf from File.basename, so the only path followed is unpack_dir itself, and the final component is created by File.rename, which replaces a symlink rather than writing through it. The basename also confines a label name containing ../, which unzip does report verbatim, and the leaf check drops the names basename alone cannot confine, since Pathname#/ resolves ., .. and a bare / straight back out of the staging directory. Across filesystems FileUtils.mv copies rather than renames, and that copy opens the destination path, so a symlink unzip left on the same name is cleared before the move. For an ordinary archive the resulting destination is unchanged.

---

• 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? Reproducing needs a hand-built archive rather than brew commands, so the added spec builds one instead.
• 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.

---

Claude Code with Opus 5, with local review and testing.

---

[Copilot]

🟡 Changes recommended

Special basenames such as .. can still escape the temporary extraction directory.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Hardens macOS ZIP extraction against symlink-based destination redirection.

Changes:

• Moves volume labels using their basename and destination directory.
• Adds ZIP fixtures covering symlink collisions and normal extraction.

File summaries

File	Description
Library/Homebrew/extend/os/mac/unpack_strategy/zip.rb	Secures volume-label moves.
Library/Homebrew/test/unpack_strategy/zip_spec.rb	Adds volume-label extraction tests.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

---

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

[Copilot]

🟡 Changes recommended

The move remains vulnerable when FileUtils.mv falls back to copying across filesystems.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

Security-sensitive, macOS-specific archive extraction behavior warrants final human validation.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[MikeMcQuaid]

Thanks!