Skip to content

fix(worktree-seed): harden chained symlink boundary resolution - #17857

Closed
azooz2003-bit wants to merge 15 commits into
mainfrom
parity/worktree-seed-review
Closed

azooz2003-bit wants to merge 15 commits into
mainfrom
parity/worktree-seed-review

Conversation

@azooz2003-bit

@azooz2003-bit azooz2003-bit commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

What this is

A fix-forward for #15860. A fresh security review found edge cases in the dangling-link boundary repair that needed to land separately after the original PR merged.

The defects

The resolver followed a chained symlink but dropped path components after a symlinked parent, which could classify an escaping leaf as inside the repository. The fix preserves the unresolved suffix while resolving each symlink component.

The resolver also treated a valid chain that revisited the same alias with a different suffix as a cycle. The fix keys cycle detection on the complete normalized unresolved path state, so valid chains remain allowed while genuine cycles are refused. A self-expanding symlink could otherwise grow the unresolved path indefinitely, so resolution is now bounded at 64 symlink hops and such paths are refused.

Verification

The focused command is swift test in an extracted scratch package because CMUXAgentLaunch does not build on Linux. The chained-parent and alias-revisit regressions were red before their fixes, and the self-expanding-link regression timed out before its bound. The final run passed all 85 extracted tests. python3 scripts/verify-local.py passed all 3 selected checks; native compilation and app tests remain unavailable on Linux.

Changelog

none

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Repository listings now more accurately identify symbolic links that point outside the repository, including links routed through other directories or links.
    • Links that loop or expand indefinitely are handled safely, preventing incorrect repository-boundary results.

Migrated from #15911 after correcting the PR author identity. The head branch and commit history are preserved.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes chained symlink boundary resolution in worktree seeding so escaping leaves are no longer misclassified as inside the repository. The resolver now preserves the unresolved suffix while following each symlink component, detects cycles by the full normalized unresolved path state (allowing valid alias revisits), and caps resolution at 64 hops so self-expanding links are refused rather than hanging.

Adds regression tests for chained-parent escapes, symlinked target parents, valid alias revisits, and self-expanding links. Verified with swift test on an extracted scratch package (CMUXAgentLaunch does not build on Linux) and scripts/verify-local.py (3 checks).

Written for commit 06dd0eb. Summary will update on new commits.

Review in cubic Turn on auto-fix

teamleaderleo and others added 15 commits September 30, 2026 01:07
…ink escapes

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nks out of the repository

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631).
Merged by scripts/merge-main.sh: origin/main at ecba57a.

Catch-up-previous-head: 8856418
Catch-up-base: ecba57a
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Catch-up merge by scripts/ci/catch_up_pr.py (RFC #14631).
Merged by scripts/merge-main.sh: origin/main at e948007, the newest commit with green CI fast guards (1 newer skipped).

Catch-up-previous-head: 92f98e9
Catch-up-base: e948007
Keep the catch-up branch on the bonsplit revision required by main's terminal sizing sources.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Merge-main commit by scripts/merge-main.sh.
Merged by scripts/merge-main.sh: origin/main at b3a1ca1, the newest commit with green CI fast guards (2 newer skipped).

Merge-main-previous-head: dfded72
Merge-main-base: b3a1ca1

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 46 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: e1c29ec0-db36-42dc-9c5b-3d1df95d3dd4
📥 Commits

Reviewing files that changed from the base of the PR and between a8c4861 and 06dd0eb.

📒 Files selected for processing (2)
  • Packages/macOS/CMUXAgentLaunch/Sources/CMUXAgentLaunch/WorktreeSeed/WorktreeSeedRepository.swift
  • Packages/macOS/CMUXAgentLaunch/Tests/CMUXAgentLaunchTests/WorktreeSeedRepositoryTests.swift
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Note

Pull Request opener @azooz2003-bit is not an author or co-author of any commit in this PR (commit identities: teamleaderleo, claude). The CLA check will still proceed and requires every listed identity plus @azooz2003-bit to have signed.

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

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.

2 participants