[shiqui]

Description

When a destination symlink appears after directory-copy checks, plain mv can move the staged directory into the link's referent, leave the link in the worktree, and report success. An approved postCreate hook can then resolve its tool through that link.

Use mv -h on macOS and mv -T on GNU platforms when publishing staged directories and leaves. Propagate directory-copy failures through copy, new, and pr; creation stops before hooks and automatic tool launches, leaving the worktree available for inspection. Add seven regression tests and document the writable-directory trust boundary.

Motivation

Related: VULN-600.

A benign, deterministic fixture against 2d4852d inserted a symlink immediately before the native move. Before this change, an outside copy occurred, a redirected tool wrote a marker, and creation returned success. After this change, creation returns failure, no outside copy occurs, and the hook marker is absent. A normal-copy control still runs its copied tool exactly once.

This overlaps the copy-confinement work in #208/#210. It does not establish a repository-identity or hook-approval bypass from #207.

Type of Change

• Bug fix
• Documentation update
• Behavior change: directory-copy errors now produce failure and prevent creation hooks from running.

Testing

After rebasing onto 801f33c (including #215 and #216), all 130 tests in the four affected BATS files passed under Bash 5. All 14 selected publication and trailing-slash regression checks passed under Bash 3.2. ShellCheck, completion checks, and diff checks also passed. The rebase retained both independently added tests at the conflict location.

Initial full-suite validation on macOS with Bash 3.2.57 and Bash 5.3.20, before rebasing:

• All seven added tests pass on both shells.
• Bash 5 full BATS run: 610/612 passed; the two fish-output checks passed on targeted rerun with writable temporary fish data/cache directories.
• Bash 3.2 full BATS run: 599/611 passed. All 12 failures also reproduce against unchanged baseline source (PR remote matching, debug traps, generated fish integration, and terminal quoting). The final additional cmd_copy regression was covered by a subsequent 14/14 command-suite run.
• ShellCheck 0.11.0, completion generation check, Bash syntax checks, and git diff --check pass.
• Disposable CLI smoke test passed creation, configured file/directory copying, one post-create hook execution, navigation, listing, running a command, and removal.

Native Linux and Windows execution has not been tested locally; CI should validate the GNU path.

Scope and limitations

The fixture uses one UID and deliberately schedules the mutation. The reported cross-UID/setgid conditions and 7/60 success rate remain unverified; no severity or duplicate determination is implied.

This is focused hardening of destination publication and error handling. Sources, worktrees, and their ancestors must still be writable only by trusted users. Mutable ancestors, later changes to tools, and arbitrary concurrent writers remain outside an execution-isolation guarantee. Hook approval continues to cover command definitions rather than pinning delegated code.

No new dependency, command, flag, or configuration key is introduced. Failed creation leaves its worktree in place for inspection and cleanup.

License Acknowledgment

This contribution is submitted under the repository's Apache License 2.0.

Summary by CodeRabbit

• Bug Fixes

  • Directory-copy failures now stop git gtr new and git gtr pr before post-create hooks or automatic tool launches. The worktree remains in place, and the command reports failure.
  • Copy operations now handle destination symlinks more safely and report failures instead of continuing as if copying succeeded.

• Documentation

  • Added guidance on copy-path permissions and the limits of hook approval and path checks.

[linear]

VULN-600

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

Staged file and directory publication now rejects destination symlinks that appear after copy checks. Copy failures propagate through copy, new, and pr; failed creation leaves the worktree in place and stops before hooks or automatic tool launches. The configuration documentation describes the writable-path trust boundary.

Changes

Worktree copying

Layer / File(s)	Summary
Safe copy publication and failure reporting lib/copy.sh, tests/copy_safety.bats	Staged files and directories use platform-specific move options that do not publish through a destination symlink. Directory-copy failures are returned to callers. Tests cover publication races, move failures, and staging cleanup.
Stop creation after copy failure lib/commands/copy.sh, lib/commands/create.sh, lib/commands/pr.sh, tests/cmd_copy.bats, tests/cmd_create_integration.bats, tests/cmd_pr.bats, docs/configuration.md	Copy commands return failure when directory copying fails. Creation and pull-request commands stop before hooks and tools if post-create copying fails. Tests cover failure and successful publication; documentation describes the trust boundary.

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

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant cmd_create
  participant cmd_pr
  participant _post_create_copy
  participant _copy_resolved_directories
  participant hooks_and_tools
  cmd_create->>_post_create_copy: copy configured files and directories
  cmd_pr->>_post_create_copy: copy configured files and directories
  _post_create_copy->>_copy_resolved_directories: copy resolved directories
  _copy_resolved_directories-->>_post_create_copy: return copy status
  _post_create_copy-->>cmd_create: return failure when copying fails
  _post_create_copy-->>cmd_pr: return failure when copying fails
  Note over cmd_create,hooks_and_tools: Commands stop before hooks and tools on copy failure

Loading

Possibly related PRs

• coderabbitai/git-worktree-runner#210: Introduced staged directory publication and safe leaf replacement, which this change updates to reject raced destination symlinks.

Suggested reviewers: natoboram

Merge Risk: Minimal · up to 37249

Directory-copy failures stop creation before hooks. No change-specific merge blocker remains.

Security architecture risk: Low · up to 37249

The change narrows a symlink-race attack path and stops creation workflows after reported directory-copy failures. No newly introduced security concern was established. Protection remains dependent on trusted writable directories, and recovery does not provide transactional rollback of all copied content.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The relevant attack surface is filesystem publication into worktrees followed by execution of copied tools through configured hooks. Exploiting the described race requires the ability to mutate the relevant destination path during copying; it is not evidence of a new remote entrypoint or an approval bypass. The routed mv declarations inspected in the tests are deliberate failure or race injectors.

Security Findings and Attack Paths

• inferred — The base permitted a destination symlink introduced after checks to redirect a staged directory move while allowing later hook execution. The head addresses that sequence through final-component move semantics and a failure gate before execution. The regression fixture models this attack path; this review inspected the fixture and implementation rather than independently executing the exploit.

Trust Boundaries and Controls

• observed — The control protects the final destination component, not concurrently mutable ancestors. Documentation explicitly requires trusted writers for sources, worktrees, and parent directories, and distinguishes approval of hook command definitions from pinning delegated script or dependency contents. This trust assumption is not newly introduced by the PR.

Resilience and Maintainability Implications

• observed — The new gate is not a universal fail-closed contract for every configured copy. Individual configured-file publication failures are still omitted from the success count while copy_patterns returns success, so those failures need not suppress hooks. This aggregation behavior exists in the base and was not established as newly exposed or worsened by the PR; it is therefore recorded as a limitation rather than an active PR concern.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the two main changes: rejecting raced symlink destinations during copy, and stopping hooks when copying fails. The raw summary and objectives both describe these changes, including the…
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.

Full details: Title check

Explanation

The title names the two main changes: rejecting raced symlink destinations during copy, and stopping hooks when copying fails. The raw summary and objectives both describe these changes, including the mv -h/-T publication fix, failure propagation through copy/new/pr, and the regression tests. The title is specific and accurate. It is long at 70 characters, but it stays within a single conventional-commit style sentence.

Full details: Docstring Coverage

Explanation

Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 8 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

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

_{A rabbit checks each path with care,
No link may send the copies elsewhere.
If copying fails, the hooks wait still,
The worktree stays upon the hill.
Then tools run once when paths are clear.}

Recent review info

Run configuration

• Source mode: Auto
• Repository: coderabbitai/git-worktree-runner
• Branch: main
• File: .coderabbit.yaml
• Commit: 6d9c0d1
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: c1e47bdd-f32c-4c14-accc-b94c4845d1bb

Commits

Reviewing files that changed from the base of the PR and between 6d9c0d1 and 3724972.

Files selected for processing: 9

• docs/configuration.md
• lib/commands/copy.sh
• lib/commands/create.sh
• lib/commands/pr.sh
• lib/copy.sh
• tests/cmd_copy.bats
• tests/cmd_create_integration.bats
• tests/cmd_pr.bats
• tests/copy_safety.bats

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