mirrors/git
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge branch 'jc/safe-directory' into maint-2.46

Browse Source 
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-26 11:10:23 -07:00
parent b452be06ff ee0be850b0
commit c93649f98a
2 changed files with 225 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

58
setup.c
View File

@ -1215,7 +1215,7 @@ static int canonicalize_ceiling_entry(struct string_list_item *item,
} }
struct safe_directory_data { struct safe_directory_data {
const char *path; char *path;
int is_safe; int is_safe;
}; };
@ -1235,17 +1235,45 @@ static int safe_directory_cb(const char *key, const char *value,
char *allowed = NULL; char *allowed = NULL;
if (!git_config_pathname(&allowed, key, value)) { if (!git_config_pathname(&allowed, key, value)) {
const char *check = allowed ? allowed : value; char *normalized = NULL;
if (ends_with(check, "/*")) {
size_t len = strlen(check); /*
if (!fspathncmp(check, data->path, len - 1)) * 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; data->is_safe = 1;
} else if (!fspathcmp(data->path, check)) { } else if (!fspathcmp(data->path, normalized)) {
data->is_safe = 1; data->is_safe = 1;
} }
} next:
if (allowed != value) free(normalized);
free(allowed); free(allowed);
}
} }
return 0; return 0;
@ -1263,9 +1291,7 @@ static int ensure_valid_ownership(const char *gitfile,
const char *worktree, const char *gitdir, const char *worktree, const char *gitdir,
struct strbuf *report) struct strbuf *report)
{ {
struct safe_directory_data data = { struct safe_directory_data data = { 0 };
.path = worktree ? worktree : gitdir
};
if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) && if (!git_env_bool("GIT_TEST_ASSUME_DIFFERENT_OWNER", 0) &&
(!gitfile || is_path_owned_by_current_user(gitfile, report)) && (!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))) (!gitdir || is_path_owned_by_current_user(gitdir, report)))
return 1; 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 * data.path is the "path" that identifies the repository and it is
* constant regardless of what failed above. data.is_safe should be * 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); git_protected_config(safe_directory_cb, &data);
free(data.path);
return data.is_safe; return data.is_safe;
} }

178
t/t0033-safe-directory.sh
View File

@ -119,4 +119,182 @@ test_expect_success 'local clone of unowned repo accepted in safe directory' '
test_path_is_dir target 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 test_done