Merge branch 'jc/safe-directory'

Follow-up on 2.45.1 regression fix.

* jc/safe-directory:
  safe.directory: setting safe.directory="." allows the "current" directory
  safe.directory: normalize the configured path
  safe.directory: normalize the checked path
  safe.directory: preliminary clean-up
This commit is contained in:
Junio C Hamano 2024-08-15 13:22:10 -07:00
commit 0ed3dde067
2 changed files with 225 additions and 11 deletions

58
setup.c
View file

@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
}
struct safe_directory_data {
const char *path;
char *path;
int is_safe;
};
@ -1235,17 +1235,45 @@ static int safe_directory_cb(const char *key, const char *value,
char *allowed = NULL;
if (!git_config_pathname(&allowed, key, value)) {
const char *check = allowed ? allowed : value;
if (ends_with(check, "/*")) {
size_t len = strlen(check);
if (!fspathncmp(check, data->path, len - 1))
char *normalized = NULL;
/*
* Setting safe.directory to a non-absolute path
* makes little sense---it won't be relative to
* the configuration file the item is defined in.
* Except for ".", which means "if we are at the top
* level of a repository, then it is OK", which is
* slightly tighter than "*" that allows discovery.
*/
if (!is_absolute_path(allowed) && strcmp(allowed, ".")) {
warning(_("safe.directory '%s' not absolute"),
allowed);
goto next;
}
/*
* A .gitconfig in $HOME may be shared across
* different machines and safe.directory entries
* may or may not exist as paths on all of these
* machines. In other words, it is not a warning
* worthy event when there is no such path on this
* machine---the entry may be useful elsewhere.
*/
normalized = real_pathdup(allowed, 0);
if (!normalized)
goto next;
if (ends_with(normalized, "/*")) {
size_t len = strlen(normalized);
if (!fspathncmp(normalized, data->path, len - 1))
data->is_safe = 1;
} else if (!fspathcmp(data->path, check)) {
} else if (!fspathcmp(data->path, normalized)) {
data->is_safe = 1;
}
}
if (allowed != value)
next:
free(normalized);
free(allowed);
}
}
return 0;
@ -1263,9 +1291,7 @@ static int ensure_valid_ownership(const char *gitfile,
const char *worktree, const char *gitdir,
struct strbuf *report)
{
struct safe_directory_data data = {
.path = worktree ? worktree : gitdir
};
struct safe_directory_data data = { 0 };
if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
(!gitfile || is_path_owned_by_current_user(gitfile, report)) &&
@ -1273,6 +1299,15 @@ static int ensure_valid_ownership(const char *gitfile,
(!gitdir || is_path_owned_by_current_user(gitdir, report)))
return 1;
/*
* normalize the data.path for comparison with normalized paths
* that come from the configuration file. The path is unsafe
* if it cannot be normalized.
*/
data.path = real_pathdup(worktree ? worktree : gitdir, 0);
if (!data.path)
return 0;
/*
* data.path is the "path" that identifies the repository and it is
* constant regardless of what failed above. data.is_safe should be
@ -1280,6 +1315,7 @@ static int ensure_valid_ownership(const char *gitfile,
*/
git_protected_config(safe_directory_cb, &data);
free(data.path);
return data.is_safe;
}

View file

@ -119,4 +119,182 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' '
test_path_is_dir target
'
test_expect_success SYMLINKS 'checked paths are normalized' '
test_when_finished "rm -rf repository; rm -f repo" &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global --unset-all safe.directory
) &&
git init repository &&
ln -s repository repo &&
(
cd repository &&
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
test_commit sample
) &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global safe.directory "$(pwd)/repository"
) &&
git -C repository for-each-ref &&
git -C repository/ for-each-ref &&
git -C repo for-each-ref &&
git -C repo/ for-each-ref &&
test_must_fail git -C repository/.git for-each-ref &&
test_must_fail git -C repository/.git/ for-each-ref &&
test_must_fail git -C repo/.git for-each-ref &&
test_must_fail git -C repo/.git/ for-each-ref
'
test_expect_success SYMLINKS 'checked leading paths are normalized' '
test_when_finished "rm -rf repository; rm -f repo" &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global --unset-all safe.directory
) &&
mkdir -p repository &&
git init repository/s &&
ln -s repository repo &&
(
cd repository/s &&
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
test_commit sample
) &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global safe.directory "$(pwd)/repository/*"
) &&
git -C repository/s for-each-ref &&
git -C repository/s/ for-each-ref &&
git -C repo/s for-each-ref &&
git -C repo/s/ for-each-ref &&
git -C repository/s/.git for-each-ref &&
git -C repository/s/.git/ for-each-ref &&
git -C repo/s/.git for-each-ref &&
git -C repo/s/.git/ for-each-ref
'
test_expect_success SYMLINKS 'configured paths are normalized' '
test_when_finished "rm -rf repository; rm -f repo" &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global --unset-all safe.directory
) &&
git init repository &&
ln -s repository repo &&
(
cd repository &&
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
test_commit sample
) &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global safe.directory "$(pwd)/repo"
) &&
git -C repository for-each-ref &&
git -C repository/ for-each-ref &&
git -C repo for-each-ref &&
git -C repo/ for-each-ref &&
test_must_fail git -C repository/.git for-each-ref &&
test_must_fail git -C repository/.git/ for-each-ref &&
test_must_fail git -C repo/.git for-each-ref &&
test_must_fail git -C repo/.git/ for-each-ref
'
test_expect_success SYMLINKS 'configured leading paths are normalized' '
test_when_finished "rm -rf repository; rm -f repo" &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global --unset-all safe.directory
) &&
mkdir -p repository &&
git init repository/s &&
ln -s repository repo &&
(
cd repository/s &&
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
test_commit sample
) &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global safe.directory "$(pwd)/repo/*"
) &&
git -C repository/s for-each-ref &&
git -C repository/s/ for-each-ref &&
git -C repository/s/.git for-each-ref &&
git -C repository/s/.git/ for-each-ref &&
git -C repo/s for-each-ref &&
git -C repo/s/ for-each-ref &&
git -C repo/s/.git for-each-ref &&
git -C repo/s/.git/ for-each-ref
'
test_expect_success 'safe.directory set to a dot' '
test_when_finished "rm -rf repository" &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global --unset-all safe.directory
) &&
mkdir -p repository/subdir &&
git init repository &&
(
cd repository &&
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
test_commit sample
) &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global safe.directory "."
) &&
git -C repository for-each-ref &&
git -C repository/ for-each-ref &&
git -C repository/.git for-each-ref &&
git -C repository/.git/ for-each-ref &&
# What is allowed is repository/subdir but the repository
# path is repository.
test_must_fail git -C repository/subdir for-each-ref &&
# Likewise, repository .git/refs is allowed with "." but
# repository/.git that is accessed is not allowed.
test_must_fail git -C repository/.git/refs for-each-ref
'
test_expect_success 'safe.directory set to asterisk' '
test_when_finished "rm -rf repository" &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global --unset-all safe.directory
) &&
mkdir -p repository/subdir &&
git init repository &&
(
cd repository &&
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
test_commit sample
) &&
(
sane_unset GIT_TEST_ASSUME_DIFFERENT_OWNER &&
git config --global safe.directory "*"
) &&
# these are trivial
git -C repository for-each-ref &&
git -C repository/ for-each-ref &&
git -C repository/.git for-each-ref &&
git -C repository/.git/ for-each-ref &&
# With "*", everything is allowed, and the repository is
# discovered, which is different behaviour from "." above.
git -C repository/subdir for-each-ref &&
# Likewise.
git -C repository/.git/refs for-each-ref
'
test_done