Skip to content

fix(bootstrap): preserve symlinks when extracting dmgs - #10949

Merged
jdx merged 2 commits into
mainfrom
agent/fix-bootstrap-dmg-symlink-loop
Jul 12, 2026
Merged

jdx merged 2 commits into
mainfrom
agent/fix-bootstrap-dmg-symlink-loop

Conversation

@jdx

@jdx jdx commented Jul 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • preserve symlinks while copying mounted DMG contents instead of following them
  • always attempt to detach a mounted DMG when copying fails
  • add a regression test covering a directory symlink cycle

Root cause

DMG images commonly contain an Applications symlink. The generic recursive copy followed that link into the host /Applications directory, eventually encountering a symlink loop in the installed Xcode SDK. Returning from that copy error also skipped hdiutil detach, leaving the image mounted.

Impact

mise bootstrap packages apply can extract DMG-backed brew casks without traversing the host filesystem or leaking mounts after copy failures.

Validation

  • cargo fmt --all -- --check
  • cargo test --all-features file::tests::test_copy_dir_all_preserve_symlinks_does_not_follow_loops
  • real brew-cask:kitty bootstrap reached successful DMG copy and ejection; it then stopped at the separate existing on_system_conditional limitation

Note

Medium Risk
Changes filesystem behavior on macOS for all un_dmg callers (aqua DMG installs and brew cask bootstrap); scope is localized but mistakes could still corrupt installs or leave mounts attached.

Overview
DMG extraction now copies mounted volume contents with copy_dir_all_preserve_symlinks instead of the generic recursive copy, so entries like an Applications symlink are recreated rather than followed into the host filesystem (avoiding symlink loops and accidental traversal of /Applications).

un_dmg always runs hdiutil detach after the copy attempt, surfacing copy failures first and attaching detach errors when both fail.

Adds a Unix regression test for a self-referential directory symlink alongside normal file copy.

Reviewed by Cursor Bugbot for commit 057e15a. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

  • Bug Fixes
    • Improved DMG extraction to preserve symbolic links and avoid following symlink loops.
    • Ensured the mounted disk image is detached even if copying its contents fails.
    • Improved reliability when copying directories that contain symbolic links.

@coderabbitai

coderabbitai Bot commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

DMG extraction now copies mounted contents without following symlinks, recreates symlink targets, always attempts detachment, and tests loop-like symlink handling on Unix.

Changes

DMG symlink-preserving copy

Layer / File(s) Summary
Symlink-aware directory copy
src/file.rs
Adds a helper that recreates directories, copies regular files, and preserves symlinks without following them; a Unix test validates loop-like symlink handling.
DMG extraction integration
src/file.rs
Updates un_dmg to use the new helper, always run hdiutil detach, and select copy or detach errors based on their results.

Estimated code review effort: 2 (Simple) | ~10 minutes

Poem

Through folders I hop,
A link points back—still I stop,
Files land safe and bright.
DMG bids farewell,
Symlinks keep their little spell,
Then moonrise crowns the night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: preserving symlinks during DMG extraction in bootstrap.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@jdx
jdx marked this pull request as ready for review July 12, 2026 00:24
@greptile-apps

greptile-apps Bot commented Jul 12, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a symlink-loop crash in DMG extraction by replacing the generic recursive copy (which followed symlinks) with a new copy_dir_all_preserve_symlinks helper that walks with follow_links(false) and recreates symlinks verbatim. It also ensures hdiutil detach always runs after copy, chaining both errors when they both fail.

  • copy_dir_all_preserve_symlinks uses WalkDir with follow_links(false), yielding directory symlinks as symlink entries (not traversed), file symlinks preserved via make_symlink, and regular files copied with fs::copy. This correctly avoids following Applications -> /Applications (or self-referential) links in DMGs.
  • un_dmg now captures the copy result, unconditionally calls hdiutil detach, and reports both errors via wrap_err_with when both fail — a clean improvement over the original that leaked mounts on copy failure.
  • A Unix regression test validates that a source/Applications -> . cycle is preserved as a symlink rather than traversed.

Confidence Score: 5/5

Safe to merge; the scope is narrow (macOS DMG extraction only), the logic is straightforward, and a targeted regression test covers the loop-avoidance behavior.

The new helper correctly handles directory symlinks, file symlinks, and regular files without following loops. The un_dmg error-handling rewrite is clean and covers all four outcome combinations. No edge cases appear to be missed.

No files require special attention.

Important Files Changed

Filename Overview
src/file.rs Adds copy_dir_all_preserve_symlinks (preserves symlinks, avoids loops) and updates un_dmg to use it; also fixes detach-always-runs and chains both errors when copy+detach both fail. Regression test included.

Reviews (2): Last reviewed commit: "fix(bootstrap): report dmg detach failur..." | Re-trigger Greptile

Comment thread src/file.rs Resolved
Entire-Checkpoint: ac5ac64cabc5
@github-actions

Copy link
Copy Markdown
Contributor

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.5 x -- echo 20.3 ± 0.9 18.4 24.0 1.00
mise x -- echo 20.9 ± 1.2 18.9 34.0 1.03 ± 0.08

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.5 env 19.8 ± 0.9 17.9 25.8 1.00
mise env 20.3 ± 1.0 18.4 24.9 1.03 ± 0.07

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.5 hook-env 20.5 ± 0.9 18.7 25.5 1.00
mise hook-env 20.9 ± 0.9 19.1 24.4 1.02 ± 0.06

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.7.5 ls 17.7 ± 0.8 16.2 21.9 1.00
mise ls 18.3 ± 0.9 16.9 22.9 1.04 ± 0.07

xtasks/test/perf

Command mise-2026.7.5 mise Variance
install (cached) 166ms 165ms +0%
ls (cached) 61ms 60ms +1%
bin-paths (cached) 63ms 64ms -1%
task-ls (cached) 126ms 128ms -1%

@jdx
jdx merged commit 5f39514 into main Jul 12, 2026
34 checks passed
@jdx
jdx deleted the agent/fix-bootstrap-dmg-symlink-loop branch July 12, 2026 05:42
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.

1 participant