Skip to content

Commit eec328c

Browse files
fix: preserve directory permissions during copy-ignored step (#1590)
## Problem `copy_dir_recursive_fallback()` uses `fs::create_dir_all()` to create destination directories, which always uses default permissions (0755). This means source directory permissions are lost during the copy-ignored step. For example, a directory with mode `0700` (used by tools like Postgres for data directories) becomes `0755` in the new worktree, breaking applications that require specific directory permissions. ## Solution After creating the destination directory, copy the source directory's permissions using `fs::set_permissions()` on Unix. This preserves the original permission mode for all directories copied recursively. ## Testing - Added `test_copy_ignored_preserves_directory_permissions` — creates a directory with mode 0700, copies it via copy-ignored, and verifies the destination has the same mode - Updated `test_copy_ignored_error_includes_path_directory` — the source directory now also has restricted permissions so that the permission-preservation code maintains the error scenario - All 37 copy-ignored tests pass --- Closes #1589 — automated triage --------- Co-authored-by: worktrunk-bot <254187624+worktrunk-bot@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 50903e3 commit eec328c

2 files changed

Lines changed: 92 additions & 0 deletions

File tree

src/commands/step_commands.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,18 @@ fn copy_dir_recursive_fallback(src: &Path, dest: &Path, force: bool) -> anyhow::
907907
}
908908
}
909909

910+
// Preserve source directory permissions AFTER copying contents.
911+
// Must be done after the loop — if the source lacks write permission (e.g., 0o555),
912+
// setting it before copying would make the destination read-only and fail the copies.
913+
#[cfg(unix)]
914+
{
915+
let src_perms = fs::metadata(src)
916+
.with_context(|| format!("reading permissions for {}", src.display()))?
917+
.permissions();
918+
fs::set_permissions(dest, src_perms)
919+
.with_context(|| format!("setting permissions on {}", dest.display()))?;
920+
}
921+
910922
Ok(())
911923
}
912924

tests/integration_tests/step_copy_ignored.rs

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,86 @@ fn test_copy_ignored_broken_symlink_in_dir_at_dest(mut repo: TestRepo) {
10821082
);
10831083
}
10841084

1085+
/// Test that directory permissions are preserved during copy (GitHub issue #1589)
1086+
///
1087+
/// When copying gitignored directories, the destination directories should have the
1088+
/// same permissions as the source directories. For example, a directory with mode 0700
1089+
/// should not become 0755 in the destination.
1090+
#[cfg(unix)]
1091+
#[rstest]
1092+
fn test_copy_ignored_preserves_directory_permissions(mut repo: TestRepo) {
1093+
use std::os::unix::fs::PermissionsExt;
1094+
1095+
let feature_path = repo.add_worktree("feature");
1096+
1097+
// Create a directory with restrictive permissions (0700)
1098+
let test_dir = repo.root_path().join("test");
1099+
fs::create_dir_all(&test_dir).unwrap();
1100+
fs::write(test_dir.join("file"), "content").unwrap();
1101+
fs::set_permissions(&test_dir, fs::Permissions::from_mode(0o700)).unwrap();
1102+
1103+
// Add to .gitignore
1104+
fs::write(repo.root_path().join(".gitignore"), "test\n").unwrap();
1105+
1106+
// Run copy-ignored
1107+
let output = repo
1108+
.wt_command()
1109+
.args(["step", "copy-ignored"])
1110+
.current_dir(&feature_path)
1111+
.output()
1112+
.unwrap();
1113+
assert!(
1114+
output.status.success(),
1115+
"copy-ignored should succeed: {}",
1116+
String::from_utf8_lossy(&output.stderr)
1117+
);
1118+
1119+
// Verify directory was copied
1120+
let dest_dir = feature_path.join("test");
1121+
assert!(dest_dir.exists(), "test directory should be copied");
1122+
assert!(dest_dir.join("file").exists(), "test/file should be copied");
1123+
1124+
// Verify directory permissions are preserved (0700, not default 0755)
1125+
let dest_mode = fs::metadata(&dest_dir).unwrap().permissions().mode() & 0o777;
1126+
assert_eq!(
1127+
dest_mode, 0o700,
1128+
"Directory permissions should be preserved (expected 0700, got {dest_mode:04o})"
1129+
);
1130+
1131+
// Also verify read-only directories (0o555) are handled correctly.
1132+
// Permissions must be set AFTER copying contents, otherwise the destination
1133+
// becomes read-only before files are copied into it.
1134+
let readonly_dir = repo.root_path().join("readonly");
1135+
fs::create_dir_all(&readonly_dir).unwrap();
1136+
fs::write(readonly_dir.join("data"), "content").unwrap();
1137+
fs::set_permissions(&readonly_dir, fs::Permissions::from_mode(0o555)).unwrap();
1138+
fs::write(repo.root_path().join(".gitignore"), "test\nreadonly\n").unwrap();
1139+
1140+
let output = repo
1141+
.wt_command()
1142+
.args(["step", "copy-ignored"])
1143+
.current_dir(&feature_path)
1144+
.output()
1145+
.unwrap();
1146+
let dest_readonly = feature_path.join("readonly");
1147+
assert!(
1148+
output.status.success(),
1149+
"copy-ignored should handle read-only source dirs: {}",
1150+
String::from_utf8_lossy(&output.stderr)
1151+
);
1152+
// Check permissions BEFORE restoring for cleanup
1153+
let dest_readonly_mode = fs::metadata(&dest_readonly).unwrap().permissions().mode() & 0o777;
1154+
// Restore for cleanup
1155+
fs::set_permissions(&readonly_dir, fs::Permissions::from_mode(0o755)).unwrap();
1156+
if dest_readonly.exists() {
1157+
fs::set_permissions(&dest_readonly, fs::Permissions::from_mode(0o755)).unwrap();
1158+
}
1159+
assert_eq!(
1160+
dest_readonly_mode, 0o555,
1161+
"Read-only directory permissions should be preserved (expected 0555, got {dest_readonly_mode:04o})"
1162+
);
1163+
}
1164+
10851165
/// Test that VCS metadata directories are excluded from copy-ignored (GitHub issue #1249)
10861166
///
10871167
/// VCS metadata directories like `.jj` (Jujutsu), `.hg` (Mercurial) contain internal

0 commit comments

Comments
 (0)