working_copy: check reserved names even if existing parent entry is symlink

Although the path would be skipped anyway, it seemed odd that ".git" was
rejected as a reserved path whereas ".git/pwned" wasn't. Let's make both fail.
This commit is contained in:
Yuya Nishihara 2024-10-22 19:20:54 +09:00
parent 12b277bd79
commit 361d8d6a21
2 changed files with 57 additions and 6 deletions

View file

@ -682,13 +682,10 @@ fn create_parent_dirs(
// A directory named ".git" or ".jj" can be temporarily created. It
// might trick workspace path discovery, but is harmless so long as the
// directory is empty.
let new_dir_created = match fs::create_dir(&dir_path) {
Ok(()) => true, // New directory
let (new_dir_created, is_dir) = match fs::create_dir(&dir_path) {
Ok(()) => (true, true), // New directory
Err(err) => match dir_path.symlink_metadata() {
Ok(m) if m.is_dir() => false, // Existing directory
Ok(_) => {
return Ok(None); // Skip existing file or symlink
}
Ok(m) => (false, m.is_dir()), // Existing file or directory
Err(_) => {
return Err(CheckoutError::Other {
message: format!(
@ -707,6 +704,9 @@ fn create_parent_dirs(
fs::remove_dir(&dir_path).ok();
}
})?;
if !is_dir {
return Ok(None); // Skip existing file or symlink
}
}
let mut file_path = dir_path;

View file

@ -2001,6 +2001,57 @@ fn test_check_out_reserved_file_path_vfat(vfat_path_str: &str, file_path_strs: &
}
}
#[test_case(".git"; "root .git file")]
#[test_case(".git/pwned"; "root .git dir")]
fn test_check_out_reserved_file_path_dot_git_symlink(file_path_str: &str) {
if !check_symlink_support().unwrap() {
eprintln!("Skipping test because symlink isn't supported");
return;
}
let mut test_workspace = TestWorkspace::init();
let repo = &test_workspace.repo;
let workspace_root = test_workspace.workspace.workspace_root().to_owned();
// Create symlink .git -> ../git-repo
let git_repo_dir = test_workspace.env.root().join("git-repo");
let dot_git_path = workspace_root.join(".git");
std::fs::create_dir(&git_repo_dir).unwrap();
symlink_dir(&git_repo_dir, &dot_git_path);
assert!(dot_git_path.exists());
let file_path = repo_path(file_path_str);
let disk_path = file_path.to_fs_path_unchecked(&workspace_root);
let tree1 = create_tree(repo, &[(file_path, "contents")]);
let tree2 = create_tree(repo, &[]);
let commit1 = commit_with_tree(repo.store(), tree1);
let commit2 = commit_with_tree(repo.store(), tree2);
// Checkout should fail.
let ws = &mut test_workspace.workspace;
let result = ws.check_out(repo.op_id().clone(), None, &commit1);
assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. }));
// Therefore, "pwned" file shouldn't be created.
assert!(!git_repo_dir.join("pwned").exists());
assert!(!dot_git_path.join("pwned").exists());
// Pretend that the checkout somehow succeeded.
let mut locked_ws = ws.start_working_copy_mutation().unwrap();
locked_ws.locked_wc().reset(&commit1).block_on().unwrap();
locked_ws.finish(repo.op_id().clone()).unwrap();
if file_path_str != ".git" {
std::fs::write(&disk_path, "").unwrap();
}
// Check out empty tree, which tries to remove the file.
let result = ws.check_out(repo.op_id().clone(), None, &commit2);
assert_matches!(result, Err(CheckoutError::ReservedPathComponent { .. }));
// The existing file shouldn't be removed.
assert!(disk_path.exists());
}
#[test]
fn test_fsmonitor() {
let test_repo = TestRepo::init();