fix(copy): reject raced symlink destinations and stop hooks on failure

docs/configuration.md

@@ -331,6 +331,16 @@ symlink itself removes only the link, including when the pattern ends in `/`;
 the link target is preserved. Exclusions are skipped when the copied directory
 or one of its path components is a symlink.
 
+If directory copying fails, `git gtr new` and `git gtr pr` stop before running
+post-create hooks or automatically launching tools. The created worktree is
+left in place for inspection and cleanup.
+
+Keep copy sources, destination worktrees, and their parent directories writable
+only by trusted users. A user who can replace a worktree path or modify a copied
+tool can change what a hook executes. Hook approval covers command definitions;
+it does not pin the contents of delegated scripts or dependencies. Copy path
+checks cannot isolate execution from other users who can mutate those paths.
+
 > [!WARNING]
 > Dependency directories may contain sensitive files (credentials, tokens, cached secrets). Always use `gtr.copy.excludeDirs` to exclude sensitive subdirectories if needed.
 

lib/commands/copy.sh

@@ -95,11 +95,15 @@ cmd_copy() {
     if [ "$dry_run" -eq 1 ]; then
       log_step "[dry-run] Would copy to: $dst_branch"
       [ -n "$patterns" ] && copy_patterns "$src_path" "$dst_path" "$patterns" "$excludes" "true" "true"
-      [ -n "$resolved_dirs" ] && _copy_resolved_directories "$src_path" "$dst_path" "$resolved_dirs" "$dir_excludes" "true"
+      if [ -n "$resolved_dirs" ]; then
+        _copy_resolved_directories "$src_path" "$dst_path" "$resolved_dirs" "$dir_excludes" "true" || return 1
+      fi
     else
       log_step "Copying to: $dst_branch"
       [ -n "$patterns" ] && copy_patterns "$src_path" "$dst_path" "$patterns" "$excludes" "true"
-      [ -n "$resolved_dirs" ] && _copy_resolved_directories "$src_path" "$dst_path" "$resolved_dirs" "$dir_excludes"
+      if [ -n "$resolved_dirs" ]; then
+        _copy_resolved_directories "$src_path" "$dst_path" "$resolved_dirs" "$dir_excludes" || return 1
+      fi
     fi
     copied_any=1
   done

lib/commands/create.sh

@@ -14,7 +14,7 @@ _post_create_copy() {
 
   if [ -n "$includes" ]; then
     log_step "Copying files..."
-    copy_patterns "$repo_root" "$worktree_path" "$includes" "$excludes"
+    copy_patterns "$repo_root" "$worktree_path" "$includes" "$excludes" || return 1
   fi
 
   # Copy directories (typically git-ignored dirs like node_modules, .venv)
@@ -24,7 +24,7 @@ _post_create_copy() {
 
   if [ -n "$dir_includes" ]; then
     log_step "Copying directories..."
-    copy_directories "$repo_root" "$worktree_path" "$dir_includes" "$dir_excludes"
+    copy_directories "$repo_root" "$worktree_path" "$dir_includes" "$dir_excludes" || return 1
   fi
 }
 
@@ -261,7 +261,10 @@ _cmd_create_impl() {
 
   # Copy files based on patterns
   if [ "$skip_copy" -eq 0 ]; then
-    _post_create_copy "$repo_root" "$worktree_path"
+    if ! _post_create_copy "$repo_root" "$worktree_path"; then
+      log_error "Worktree created at $worktree_path, but copying failed; hooks and tools were not started"
+      exit 1
+    fi
   fi
 
   # Run post-create hooks (unless --no-hooks)

lib/commands/pr.sh

@@ -396,7 +396,10 @@ cmd_pr() {
   fi
 
   if [ "$skip_copy" -eq 0 ]; then
-    _post_create_copy "$repo_root" "$worktree_path"
+    if ! _post_create_copy "$repo_root" "$worktree_path"; then
+      log_error "Worktree created at $worktree_path, but copying failed; hooks and tools were not started"
+      return 1
+    fi
   fi
 
   if [ "$skip_hooks" -eq 0 ]; then

lib/copy.sh

@@ -49,6 +49,20 @@ _copy_discovery_records() {
   done
 }
 
+_copy_move_os=""
+
+# Plain mv treats a destination symlink to a directory as a container. The
+# destination can change while copying to staging, so checking -L beforehand
+# is insufficient. Use the platform's no-follow form when publishing entries.
+# This protects the final component, not ancestors writable by other users.
+_copy_move_entry() {
+  [ -n "$_copy_move_os" ] || _copy_move_os=$(detect_os)
+  case "$_copy_move_os" in
+    darwin) mv -fh -- "$1" "$2" ;;
+    *) mv -fT -- "$1" "$2" ;;
+  esac
+}
+
 # Merge without following existing destination links. Stage each leaf in its
 # destination directory, then rename it so existing hard links are not written.
 _copy_tree_entry() {
@@ -77,7 +91,7 @@ _copy_tree_entry() {
           return 1
         fi
       fi
-      if mv -f -- "$staged/leaf" "$dest"; then
+      if _copy_move_entry "$staged/leaf" "$dest"; then
         rmdir "$staged"
         return 0
       fi
@@ -192,7 +206,7 @@ _fast_copy_dir() {
     rm -rf -- "$staging"
     return 1
   fi
-  if mv -- "${staging:?}/${src##*/}" "$target"; then
+  if _copy_move_entry "${staging:?}/${src##*/}" "$target"; then
     rmdir "$staging"
   else
     rm -rf -- "$staging"
@@ -699,6 +713,7 @@ EOF
 
 # Copy already-resolved directories from a source root.
 # Usage: _copy_resolved_directories src_root dst_root resolved_dirs excludes [dry_run]
+# Returns nonzero if any directory copy fails; callers must guard the status.
 _copy_resolved_directories() {
   local src_root="$1"
   local dst_root="$2"
@@ -718,7 +733,7 @@ _copy_resolved_directories() {
   old_pwd=$(pwd)
   cd "$src_root" || return 1
 
-  local copied_count=0
+  local copied_count=0 failed=0
   local dir_path
   while IFS= read -r dir_path; do
     [ -z "$dir_path" ] && continue
@@ -741,7 +756,10 @@ _copy_resolved_directories() {
       continue
     fi
 
-    mkdir -p "$dest_parent"
+    if ! mkdir -p "$dest_parent"; then
+      failed=1
+      continue
+    fi
 
     if { if _has_subdir_excludes "$dir_path" "$excludes"; then
            _selective_copy_dir "$dir_path" "$dst_root" "$excludes"
@@ -752,6 +770,7 @@ _copy_resolved_directories() {
       copied_count=$((copied_count + 1))
     else
       log_warn "Failed to copy directory $dir_path"
+      failed=1
     fi
   done <<EOF
 $resolved_dirs
@@ -767,7 +786,7 @@ EOF
     fi
   fi
 
-  return 0
+  return "$failed"
 }
 
 # Copy directories matching patterns (typically git-ignored directories like node_modules)

tests/cmd_copy.bats

@@ -65,6 +65,18 @@ teardown() {
   [ ! -e "$TEST_WORKTREES_DIR/copy-target/cache" ]
 }
 
+@test "cmd_copy propagates a directory publication failure" {
+  mkdir -p "$TEST_REPO/deps"
+  echo value > "$TEST_REPO/deps/file"
+  git config --add gtr.copy.includeDirs deps
+  mv() { return 1; }
+
+  run cmd_copy copy-target
+
+  [ "$status" -ne 0 ]
+  [ ! -e "$TEST_WORKTREES_DIR/copy-target/deps" ]
+}
+
 @test "cmd_copy dry-run does not copy configured includeDirs" {
   mkdir -p "$TEST_REPO/.zed"
   echo "settings" > "$TEST_REPO/.zed/settings.json"

tests/cmd_create_integration.bats

@@ -129,6 +129,47 @@ _cmd_create_capture_stdout() {
   [ ! -s "$stdout_file" ]
 }
 
+@test "cmd_create stops before hooks when a directory publication races with a symlink" {
+  mkdir -p "$TEST_REPO/deps" "$TEST_REPO/outside"
+  printf 'echo expected > hook-marker\n' > "$TEST_REPO/deps/tool"
+  printf 'echo redirected > hook-marker\n' > "$TEST_REPO/outside/tool"
+  git config --add gtr.copy.includeDirs deps
+  git config --add gtr.hook.postCreate 'sh deps/tool'
+  local race_worktree="$TEST_WORKTREES_DIR/copy-race"
+  local stdout_file="$TEST_REPO/create-stdout"
+  mv() {
+    local last
+    for last in "$@"; do :; done
+    if [ "$last" = "$race_worktree/deps" ]; then
+      ln -s "$TEST_REPO/outside" "$race_worktree/deps"
+    fi
+    command mv "$@"
+  }
+
+  run _cmd_create_capture_stdout "$stdout_file" copy-race --from HEAD --no-fetch --porcelain
+
+  [ "$status" -ne 0 ]
+  [ -L "$race_worktree/deps" ]
+  [ ! -e "$race_worktree/hook-marker" ]
+  [ ! -e "$TEST_REPO/outside/deps" ]
+  [ ! -s "$stdout_file" ]
+  [[ "$output" == *"copying failed"* ]]
+}
+
+@test "cmd_create runs a copied tool once after successful directory publication" {
+  mkdir -p "$TEST_REPO/deps"
+  printf 'echo expected >> hook-marker\n' > "$TEST_REPO/deps/tool"
+  git config --add gtr.copy.includeDirs deps
+  git config --add gtr.hook.postCreate 'sh deps/tool'
+
+  run cmd_create copy-success --from HEAD --no-fetch --porcelain
+
+  [ "$status" -eq 0 ]
+  [ ! -L "$TEST_WORKTREES_DIR/copy-success/deps" ]
+  [ "$(cat "$TEST_WORKTREES_DIR/copy-success/hook-marker")" = expected ]
+  [[ "$output" == *$'hook_status\tran'* ]]
+}
+
 @test "cmd_create creates worktree with --track none" {
   cmd_create track-none --from HEAD --track none --no-fetch --yes
   [ -d "$TEST_WORKTREES_DIR/track-none" ]

tests/cmd_pr.bats

@@ -40,6 +40,21 @@ teardown() {
   [ "$(git config branch.feature/from-pr.merge)" = "refs/heads/feature/from-pr" ]
 }
 
+@test "cmd_pr does not run postCreate after a directory copy fails" {
+  mkdir -p "$TEST_REPO/deps"
+  echo value > "$TEST_REPO/deps/file"
+  git config --add gtr.copy.includeDirs deps
+  git config --add gtr.hook.postCreate 'touch hook-marker'
+  mv() { return 1; }
+
+  run cmd_pr 123 --remote origin --yes
+
+  [ "$status" -ne 0 ]
+  [ -d "$TEST_WORKTREES_DIR/feature-from-pr" ]
+  [ ! -e "$TEST_WORKTREES_DIR/feature-from-pr/hook-marker" ]
+  [[ "$output" == *"copying failed"* ]]
+}
+
 @test "cmd_pr treats an option-shaped remote as a fetch operand" {
   # Configure the name directly because `git remote add` rejects leading dashes.
   git config remote.--all.url "$TEST_REMOTE_REPO"

tests/copy_safety.bats

@@ -670,7 +670,8 @@ teardown() {
   echo private > "$outside/secret"
   echo replacement > "$src/vendor/cache/secret"
   ln -s "$outside" "$dst/vendor/cache"
-  copy_directories "$src" "$dst" vendor ''
+  run copy_directories "$src" "$dst" vendor ''
+  [ "$status" -ne 0 ]
   [ "$(cat "$outside/secret")" = private ]
 }
 
@@ -828,3 +829,63 @@ SH
   [ "$status" -ne 0 ]
   [ ! -e "$dst/cache" ]
 }
+
+@test "directory publication does not follow a symlink inserted at the move boundary" {
+  _test_tmpdir=$(mktemp -d)
+  local src="$_test_tmpdir/source space" dst="$_test_tmpdir/destination space"
+  local outside="$_test_tmpdir/outside"
+  mkdir -p "$src/cache" "$dst" "$outside"
+  echo value > "$src/cache/file"
+  echo sentinel > "$outside/keep"
+  # Deterministically schedule the other writer after all pre-copy checks.
+  # The actual system mv still performs the move with the production options.
+  mv() {
+    ln -s "$outside" "$dst/cache"
+    command mv "$@"
+  }
+
+  run _fast_copy_dir "$src/cache" "$dst/"
+
+  [ "$status" -ne 0 ]
+  [ -L "$dst/cache" ]
+  [ ! -e "$outside/cache" ]
+  [ "$(cat "$outside/keep")" = sentinel ]
+  [ "$(find "$dst" -name '.gtr-copy.*' | wc -l | tr -d ' ')" -eq 0 ]
+}
+
+@test "directory copy failures reach the caller and restore its working directory" {
+  _test_tmpdir=$(mktemp -d)
+  local src="$_test_tmpdir/src" dst="$_test_tmpdir/dst"
+  mkdir -p "$src/cache" "$dst"
+  echo value > "$src/cache/file"
+  mv() { return 1; }
+  local before result=0
+  before=$(pwd)
+
+  copy_directories "$src" "$dst" cache '' || result=$?
+
+  [ "$result" -ne 0 ]
+  [ "$(pwd)" = "$before" ]
+  [ ! -e "$dst/cache" ]
+  [ "$(find "$dst" -name '.gtr-copy.*' | wc -l | tr -d ' ')" -eq 0 ]
+}
+
+@test "staged leaf publication replaces a raced symlink without writing its referent" {
+  _test_tmpdir=$(mktemp -d)
+  local src="$_test_tmpdir/src" dst="$_test_tmpdir/dst"
+  local outside="$_test_tmpdir/outside"
+  mkdir -p "$src" "$dst" "$outside"
+  echo value > "$src/file"
+  echo sentinel > "$outside/leaf"
+  mv() {
+    ln -s "$outside" "$dst/file"
+    command mv "$@"
+  }
+
+  _copy_tree_entry "$src/file" "$dst/file"
+
+  [ ! -L "$dst/file" ]
+  [ "$(cat "$dst/file")" = value ]
+  [ "$(cat "$outside/leaf")" = sentinel ]
+  [ "$(find "$dst" -name '.gtr-copy.*' | wc -l | tr -d ' ')" -eq 0 ]
+}