safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
cover safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
This supersedes my earlier series [*1*] which took the approach of adjusting individual tests to access bare repositories explicitly.
As Junio suggested [*2*], this series instead takes the approach of setting safe.bareRepository=all in the test environment's global config whenever WITH_BREAKING_CHANGES is in effect, so that existing tests continue to work without individual modifications.
Implementing this turned out to require a number of follow-up adjustments, because writing to $HOME/.gitconfig has side effects beyond the intended
setting: $HOME is the trash directory, which doubles as the test repository's working tree, so the file shows up in ls-files and status output, and tests that manipulate $HOME/.gitconfig for their own purposes can clobber or remove the setting. Patches 2 through 7 address these interactions in the affected test scripts.
The final patch flips the safe.bareRepository default to "explicit" under WITH_BREAKING_CHANGES.
Footnote [*1*]:
https://lore.kernel.org/git/[email protected]/
Footnote [*2*]: https://lore.kernel.org/git/[email protected]/
Johannes Schindelin (8):
test-lib: allow bare repository access when breaking changes are
enabled
t7900: do not let $HOME/.gitconfig interfere with XDG tests
t1300: remove global config settings injected by test-lib.sh
t1305: use --git-dir=. for bare repo in include cycle test
t5601: restore .gitconfig after includeIf test
ls-files tests: filter .gitconfig from --others output status tests: filter .gitconfig from status output safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
Documentation/BreakingChanges.adoc | 24 +++++++++++++++++ Documentation/config/safe.adoc | 10 +++++-- setup.c | 4 +++ t/t0035-safe-bare-repository.sh | 10 +++++-- t/t1300-config.sh | 7 +++++ t/t1305-config-include.sh | 4 +-- t/t3000-ls-files-others.sh | 4 +++ t/t3001-ls-files-others-exclude.sh | 3 +++ t/t3002-ls-files-dashpath.sh | 2 ++ t/t3009-ls-files-others-nonsubmodule.sh | 1 + ...common-prefixes-and-directory-traversal.sh | 3 ++- t/t5601-clone.sh | 4 ++- t/t7060-wtstatus.sh | 3 +-- t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++ t/t7064-wtstatus-pv2.sh | 1 + t/t7104-reset-hard.sh | 2 +- t/t7508-status.sh | 4 +++ t/t7521-ignored-mode.sh | 1 + t/t7900-maintenance.sh | 12 +++++++-- t/test-lib-functions.sh | 8 ++++++ t/test-lib.sh | 6 +++++ 21 files changed, 127 insertions(+), 13 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2098%2Fdscho%2Fsafe-bare-repo-default-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2098/dscho/safe-bare-repo-default-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2098
Signature · 1 line
gitgitgadget
PATCH 1/8 test-lib: allow bare repository access when breaking changes are enabled
From: Johannes Schindelin <[email protected]>
A future patch will change the safe.bareRepository default from all to explicit under WITH_BREAKING_CHANGES. At that point, every test that operates on a bare repository through implicit discovery would fail, regardless of whether the test is actually about discovery or about how a specific command behaves once inside a bare repository.
The maintainer suggested [1] setting safe.bareRepository=all in the test environment's global config whenever WITH_BREAKING_CHANGES is in effect, rather than adjusting each affected test to access bare repositories explicitly (via --git-dir, GIT_DIR, or similar). This means the test suite continues to exercise only the historical default behavior even after the user-facing default changes, relying on a small number of dedicated tests in t0035 to validate the new, stricter default.
Since $HOME points at the trash directory (which doubles as the test repository's working tree), writing to $HOME/.gitconfig also creates a file inside the working tree. Exclude it via .git/info/exclude to limit the fallout, though this does not help tests that use git ls-files --others without --exclude-standard or git status --ignored; those are addressed by subsequent commits.
[1] https://lore.kernel.org/git/[email protected]/
--- t/test-lib.sh | 6 ++++++ 1 file changed, 6 insertions(+)
t/test-lib.sh +6 -0
@@ -1597,6 +1597,12 @@ cd -P "$TRASH_DIRECTORY" || BAIL_OUT "cannot cd -P to \"$TRASH_DIRECTORY\"" TRASH_DIRECTORY=$(pwd) HOME="$TRASH_DIRECTORY" +if test -n "$WITH_BREAKING_CHANGES" +then + git config --global safe.bareRepository all && + echo "/.gitconfig" >>.git/info/exclude +fi + start_test_output "$0" # Convenience
Signature · 1 line
gitgitgadget
PATCH 2/8 t7900: do not let `$HOME/.gitconfig` interfere with XDG tests
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The XDG config tests for git maintenance register/unregister create a fresh $XDG_CONFIG_HOME/git/config and expect git to use that location. However, if $HOME/.gitconfig exists (which may happen when test-lib.sh writes global config, e.g. to set safe.bareRepository), git prefers $HOME/.gitconfig over the XDG location, and the maintenance.repo entry ends up in the wrong file.
This is an inherent consequence of setting global config in test-lib.sh rather than adjusting individual tests: writing any entry to $HOME/.gitconfig has side effects beyond the intended setting, because the mere existence of that file changes which global config location git prefers for all subsequent writes. Individual per-test adjustments would not have this interaction.
Fix this by overriding HOME to a non-existent directory inside the subshells that test XDG behavior. Since these subshells already override XDG_CONFIG_HOME, they do not need $HOME/.gitconfig at all, and the subshell scoping ensures the original HOME is restored automatically.
--- t/t7900-maintenance.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
t/t7900-maintenance.sh +10 -2
@@ -101,8 +101,12 @@ test_expect_success "maintenance.autoDetach overrides gc.autoDetach" ' test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' ' test_when_finished rm -r .config/git/config && ( + # Override HOME so that .gitconfig (which test-lib.sh may + # have created, e.g. to set safe.bareRepository) does not + # take precedence over the XDG location. + HOME=$PWD/must-not-exist && XDG_CONFIG_HOME=.config && - export XDG_CONFIG_HOME && + export HOME XDG_CONFIG_HOME && mkdir -p $XDG_CONFIG_HOME/git && >$XDG_CONFIG_HOME/git/config && git maintenance register &&
@@ -124,8 +128,12 @@ test_expect_success 'register does not need XDG_CONFIG_HOME config to exist' ' test_expect_success 'unregister uses XDG_CONFIG_HOME config if it exists' ' test_when_finished rm -r .config/git/config && ( + # Override HOME so that .gitconfig (which test-lib.sh may + # have created, e.g. to set safe.bareRepository) does not + # take precedence over the XDG location. + HOME=$PWD/must-not-exist && XDG_CONFIG_HOME=.config && - export XDG_CONFIG_HOME && + export HOME XDG_CONFIG_HOME && mkdir -p $XDG_CONFIG_HOME/git && >$XDG_CONFIG_HOME/git/config && git maintenance register &&
Signature · 1 line
gitgitgadget
PATCH 3/8 t1300: remove global config settings injected by test-lib.sh
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Since test-lib.sh now writes safe.bareRepository=all to the global config when WITH_BREAKING_CHANGES is in effect, that entry shows up in git config --list output. Tests in t1300 that expect exact config contents then fail because of this unexpected extra line.
Unlike the working-tree contamination fixed in the preceding commits, this is not about the file's existence but about its content leaking into test expectations. Since t1300 does not use bare repositories, simply remove the injected setting in a preparatory step.
--- t/t1300-config.sh | 7 +++++++ 1 file changed, 7 insertions(+)
t/t1300-config.sh +7 -0
@@ -11,6 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh +# test-lib.sh may have added global config (e.g. safe.bareRepository) +# that would appear in "git config --list" output and break tests +# that expect exact config contents. +test_expect_success 'remove global config from test-lib.sh' ' + test_might_fail git config --global --unset-all safe.bareRepository +' + for mode in legacy subcommands do
Signature · 1 line
gitgitgadget
PATCH 4/8 t1305: use `--git-dir=.` for bare repo in include cycle test
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Earlier tests in t1305 overwrite $HOME/.gitconfig with their own content as part of testing config includes. This clobbers the safe.bareRepository=all entry that test-lib.sh writes when WITH_BREAKING_CHANGES is in effect, causing git -C cycle config to fail with "not in a git directory" when it tries to access the bare repository created by git init --bare cycle.
Use --git-dir=. to access the bare repo explicitly, avoiding the dependency on global config for repository discovery.
--- t/t1305-config-include.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
t/t1305-config-include.sh +2 -2
@@ -350,9 +350,9 @@ test_expect_success 'conditional include, onbranch, implicit /** for /' ' test_expect_success 'include cycles are detected' ' git init --bare cycle && - git -C cycle config include.path cycle && + git -C cycle --git-dir=. config include.path cycle && git config -f cycle/cycle include.path config && - test_must_fail git -C cycle config --get-all test.value 2>stderr && + test_must_fail git -C cycle --git-dir=. config --get-all test.value 2>stderr && grep "exceeded maximum include depth" stderr '
Signature · 1 line
gitgitgadget
PATCH 5/8 t5601: restore `.gitconfig` after includeIf test
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
One test in t5601 overwrites $HOME/.gitconfig with an includeIf configuration snippet and removes the file in its cleanup. This destroys the safe.bareRepository=all entry that test-lib.sh writes when WITH_BREAKING_CHANGES is in effect, causing later tests that use git -C <bare-repo> config to fail with "not in a git directory".
Back up .gitconfig before overwriting and restore it in the cleanup, so the global config survives into subsequent tests.
--- t/t5601-clone.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
t/t5601-clone.sh +3 -1
@@ -813,7 +813,9 @@ test_expect_success 'clone with includeIf' ' test_when_finished "rm -rf repo \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\"" && git clone --bare --no-local src "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && - test_when_finished "rm \"$HOME\"/.gitconfig" && + test_when_finished "cp \"$HOME\"/.gitconfig.bak \ + \"$HOME\"/.gitconfig 2>/dev/null || rm -f \"$HOME\"/.gitconfig" && + cp "$HOME"/.gitconfig "$HOME"/.gitconfig.bak 2>/dev/null && cat >"$HOME"/.gitconfig <<-EOF && [includeIf "onbranch:something"] path = /does/not/exist.inc
Signature · 1 line
gitgitgadget
PATCH 6/8 ls-files tests: filter `.gitconfig` from `--others` output
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The global safe.bareRepository=all setting in test-lib.sh is written to $HOME/.gitconfig, which unfortunately lives inside the test repository's working tree. The .git/info/exclude entry added alongside it handles most commands, but git ls-files --others without --exclude-standard does not consult info/exclude at all, so the file appears in the output.
Ideally, each test that accesses a bare repository would simply specify --git-dir or GIT_DIR explicitly, which would require no global config and produce no side effects in the working tree. As that approach was not taken, filter .gitconfig from the output before comparing against expected results. In t7104, the test already uses --exclude-standard, so it suffices to switch from the bare git ls-files -o to git ls-files -o --exclude-standard which respects the info/exclude entry; the other tests deliberately omit --exclude-standard because their purpose is to verify unfiltered --others output.
--- t/t3000-ls-files-others.sh | 4 ++++ t/t3001-ls-files-others-exclude.sh | 3 +++ t/t3002-ls-files-dashpath.sh | 2 ++ t/t3009-ls-files-others-nonsubmodule.sh | 1 + t/t3011-common-prefixes-and-directory-traversal.sh | 3 ++- t/t7104-reset-hard.sh | 2 +- t/test-lib-functions.sh | 8 ++++++++ 7 files changed, 21 insertions(+), 2 deletions(-)
t/t3000-ls-files-others.sh +4 -0
@@ -53,16 +53,19 @@ test_expect_success 'setup: expected output' ' test_expect_success 'ls-files --others' ' git ls-files --others >output && + test_filter_gitconfig output && test_cmp expected1 output ' test_expect_success 'ls-files --others --directory' ' git ls-files --others --directory >output && + test_filter_gitconfig output && test_cmp expected2 output ' test_expect_success '--no-empty-directory hides empty directory' ' git ls-files --others --directory --no-empty-directory >output && + test_filter_gitconfig output && test_cmp expected3 output '
@@ -70,6 +73,7 @@ test_expect_success 'ls-files --others handles non-submodule .git' ' mkdir not-a-submodule && echo foo >not-a-submodule/.git && git ls-files -o >output && + test_filter_gitconfig output && test_cmp expected1 output '
t/t3001-ls-files-others-exclude.sh +3 -0
@@ -72,6 +72,7 @@ test_expect_success 'git ls-files --others with various exclude options.' ' --exclude-per-directory=.gitignore \ --exclude-from=.git/ignore \ >output && + test_filter_gitconfig output && test_cmp expect output '
@@ -84,6 +85,7 @@ test_expect_success 'git ls-files --others with \r\n line endings.' ' --exclude-per-directory=.gitignore \ --exclude-from=.git/ignore \ >output && + test_filter_gitconfig output && test_cmp expect output '
@@ -99,6 +101,7 @@ test_expect_success 'git ls-files --others with various exclude options.' ' --exclude-per-directory=.gitignore \ --exclude-from=.git/ignore \ >output && + test_filter_gitconfig output && test_cmp expect output '
t/t3002-ls-files-dashpath.sh +2 -0
@@ -24,6 +24,7 @@ test_expect_success 'setup' ' test_expect_success 'git ls-files without path restriction.' ' test_when_finished "rm -f expect" && git ls-files --others >output && + test_filter_gitconfig output && cat >expect <<-\EOF && -- -foo
@@ -63,6 +64,7 @@ test_expect_success 'git ls-files with path restriction with -- --.' ' test_expect_success 'git ls-files with no path restriction.' ' test_when_finished "rm -f expect" && git ls-files --others -- >output && + test_filter_gitconfig output && cat >expect <<-\EOF && -- -foo
t/t3009-ls-files-others-nonsubmodule.sh +1 -0
@@ -36,6 +36,7 @@ test_expect_success 'setup: directories' ' test_expect_success 'ls-files --others handles untracked git repositories' ' git ls-files -o >output && + test_filter_gitconfig output && cat >expect <<-EOF && nonrepo-untracked-file/untracked output
t/t3011-common-prefixes-and-directory-traversal.sh +2 -1
@@ -26,7 +26,7 @@ test_expect_success 'setup' ' ' test_expect_success 'git ls-files -o shows the right entries' ' - cat <<-EOF >expect && + cat >expect <<-EOF && .gitignore actual an_ignored_dir/ignored
@@ -39,6 +39,7 @@ test_expect_success 'git ls-files -o shows the right entries' ' untracked_repo/ EOF git ls-files -o >actual && + test_filter_gitconfig actual && test_cmp expect actual '
t/t7104-reset-hard.sh +1 -1
@@ -21,7 +21,7 @@ test_expect_success setup ' rm -f hello && mkdir -p hello && >hello/world && - test "$(git ls-files -o)" = hello/world + test "$(git ls-files -o --exclude-standard)" = hello/world '
t/test-lib-functions.sh +8 -0
@@ -2069,3 +2069,11 @@ test_trailing_hash () { test_redact_non_printables () { tr -d "\n\r" | tr "[\001-\040][\177-\377]" "." } + +# Remove .gitconfig entries from a file in place. test-lib.sh may +# create $HOME/.gitconfig (e.g. to set safe.bareRepository) which +# can appear in ls-files or status output. +test_filter_gitconfig () { + sed "/\\.gitconfig/d" "$1" >"$1.filtered" && + mv "$1.filtered" "$1" +}
Signature · 1 line
gitgitgadget
Re: [PATCH 6/8] ls-files tests: filter `.gitconfig` from `--others` output
"Johannes Schindelin via GitGitGadget" <[email protected]>
t/test-lib-functions.sh test_trailing_hash () { · line 2079
Quoted 7 lines · writes:
t/test-lib-functions.sh +7
-0+# Remove .gitconfig entries from a file in place. test-lib.sh may +# create $HOME/.gitconfig (e.g. to set safe.bareRepository) which +# can appear in ls-files or status output. +test_filter_gitconfig () { + sed "/\\.gitconfig/d" "$1" >"$1.filtered" && + mv "$1.filtered" "$1" +}
Thanks.
When I suggested the "just use the usual configuration mechanism to keep the semantics of existing bare repository tests, and that would be very unintrusive", I didn't think of this fallout from an extra file getting reported by ls-files, and this helper function does make sense. If we were starting from scratch, we probably would have created $HOME that is separate from the test repository in the test framework. The simplest layout would be to emulate a developer who has repositories under their $HOME, i.e., $TRASH_DIRECTORY stays to be the $HOME, but the initial test repository would be created as a directory inside $TRASH_DIRECTORY instead of using the trash itself, or something like that. It is all water under the bridge now, unless somebody wants an opportunity to work on a non-trivial clean-up.
By the way, when merged to 'seen', with the 'ps/test-set-e-clean' topic already in 'next', many tests (especially the ones with early test_done, like p4 tests in my environment where p4 does not exist) seem to fail with WITH_BREAKING_CHANGES turned on. I don't have enough time to be sitting in front of the keyboard to isolate the cause, but because this series is one of the topics with biggest impact to the t/ directory that was replaced after the last successful run, I thought I should mention it.
PATCH 7/8 status tests: filter `.gitconfig` from status output
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Since test-lib.sh creates $HOME/.gitconfig when WITH_BREAKING_CHANGES is in effect, the file appears in git status output as either untracked (?? .gitconfig) or ignored (!! .gitconfig / ! .gitconfig, depending on porcelain version), because the .git/info/exclude entry causes git to treat it as an ignored file rather than hiding it entirely.
In t7061 and t7521, which are pervasively affected, introduce a filter_gitconfig helper that strips all status-prefix variants of .gitconfig from the output before comparison. In the remaining scripts (t7060, t7064, t7508), apply targeted adjustments.
--- t/t7060-wtstatus.sh | 3 +-- t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++++++++++ t/t7064-wtstatus-pv2.sh | 1 + t/t7508-status.sh | 4 ++++ t/t7521-ignored-mode.sh | 1 + 5 files changed, 34 insertions(+), 2 deletions(-)
t/t7060-wtstatus.sh +1 -2
@@ -9,6 +9,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME test_expect_success setup ' git config --global advice.statusuoption false && + echo "/.gitconfig" >>.git/info/exclude && test_commit A && test_commit B oneside added && git checkout A^0 &&
@@ -221,7 +222,6 @@ test_expect_success 'status --branch with detached HEAD' ' git status --branch --porcelain >actual && cat >expected <<-EOF && ## HEAD (no branch) - ?? .gitconfig ?? actual ?? expect ?? expected
@@ -237,7 +237,6 @@ test_expect_success 'status --porcelain=v1 --branch with detached HEAD' ' git status --branch --porcelain=v1 >actual && cat >expected <<-EOF && ## HEAD (no branch) - ?? .gitconfig ?? actual ?? expect ?? expected
t/t7061-wtstatus-ignore.sh +27 -0
@@ -18,6 +18,7 @@ test_expect_success 'status untracked directory with --ignored' ' : >untracked/ignored && : >untracked/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -27,6 +28,7 @@ test_expect_success 'same with gitignore starting with BOM' ' : >untracked/ignored && : >untracked/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -40,18 +42,22 @@ test_expect_success 'status untracked files --ignored with pathspec (no match)' test_expect_success 'status untracked files --ignored with pathspec (literal match)' ' git status --porcelain --ignored -- untracked/ignored >actual && echo "!! untracked/ignored" >expected && + test_filter_gitconfig actual && test_cmp expected actual && git status --porcelain --ignored -- untracked/uncommitted >actual && echo "?? untracked/uncommitted" >expected && + test_filter_gitconfig actual && test_cmp expected actual ' test_expect_success 'status untracked files --ignored with pathspec (glob match)' ' git status --porcelain --ignored -- untracked/i\* >actual && echo "!! untracked/ignored" >expected && + test_filter_gitconfig actual && test_cmp expected actual && git status --porcelain --ignored -- untracked/u\* >actual && echo "?? untracked/uncommitted" >expected && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -65,6 +71,7 @@ EOF test_expect_success 'status untracked directory with --ignored -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual ' cat >expected <<\EOF
@@ -76,9 +83,11 @@ test_expect_success 'status of untracked directory with --ignored works with or git status --porcelain --ignored >tmp && grep untracked/ tmp >actual && rm tmp && + test_filter_gitconfig actual && test_cmp expected actual && git status --porcelain --ignored untracked/ >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -89,6 +98,7 @@ EOF test_expect_success 'status prefixed untracked sub-directory with --ignored -u' ' git status --porcelain --ignored -u untracked/ >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -104,6 +114,7 @@ test_expect_success 'status ignored directory with --ignore' ' mkdir ignored && : >ignored/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -116,6 +127,7 @@ EOF test_expect_success 'status ignored directory with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -130,6 +142,7 @@ test_expect_success 'status empty untracked directory with --ignore' ' mkdir untracked-ignored && mkdir untracked-ignored/test && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -141,6 +154,7 @@ EOF test_expect_success 'status empty untracked directory with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -155,6 +169,7 @@ test_expect_success 'status untracked directory with ignored files with --ignore : >untracked-ignored/ignored && : >untracked-ignored/test/ignored && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -168,6 +183,7 @@ EOF test_expect_success 'status untracked directory with ignored files with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -185,6 +201,7 @@ test_expect_success 'status ignored tracked directory with --ignore' ' git commit -m. && echo "tracked" >.gitignore && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -196,6 +213,7 @@ EOF test_expect_success 'status ignored tracked directory with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -208,6 +226,7 @@ EOF test_expect_success 'status ignored tracked directory and ignored file with --ignore' ' echo "committed" >>.gitignore && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -219,6 +238,7 @@ EOF test_expect_success 'status ignored tracked directory and ignored file with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -233,6 +253,7 @@ test_expect_success 'status ignored tracked directory and uncommitted file with echo "tracked" >.gitignore && : >tracked/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -245,6 +266,7 @@ EOF test_expect_success 'status ignored tracked directory and uncommitted file with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -260,6 +282,7 @@ test_expect_success 'status ignored tracked directory with uncommitted file in u mkdir tracked/ignored && : >tracked/ignored/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -272,6 +295,7 @@ EOF test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -287,6 +311,7 @@ test_expect_success 'status ignored tracked directory with uncommitted file in t git add -f tracked/ignored/committed && git commit -m. && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -299,6 +324,7 @@ EOF test_expect_success 'status ignored tracked directory with uncommitted file in tracked subdir with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -310,6 +336,7 @@ test_expect_success 'status ignores submodule in excluded directory' ' git init tracked/submodule && test_commit -C tracked/submodule initial && git status --porcelain --ignored -u tracked/submodule >actual && + test_filter_gitconfig actual && test_cmp expected actual '
t/t7064-wtstatus-pv2.sh +1 -0
@@ -231,6 +231,7 @@ test_expect_success 'ignored files are printed with --ignored' ' EOF git status --porcelain=v2 --ignored --untracked-files=all >actual && + test_filter_gitconfig actual && test_cmp expect actual '
t/t7508-status.sh +4 -0
@@ -263,6 +263,7 @@ test_expect_success 'status with gitignore' ' !! untracked EOF git status -s --ignored >output && + test_filter_gitconfig output && test_cmp expect output && cat >expect <<\EOF &&
@@ -296,6 +297,7 @@ Ignored files: EOF git status --ignored >output && + test_filter_gitconfig output && test_cmp expect output '
@@ -328,6 +330,7 @@ test_expect_success 'status with gitignore (nothing untracked)' ' !! untracked EOF git status -s --ignored >output && + test_filter_gitconfig output && test_cmp expect output && cat >expect <<\EOF &&
@@ -358,6 +361,7 @@ Ignored files: EOF git status --ignored >output && + test_filter_gitconfig output && test_cmp expect output '
t/t7521-ignored-mode.sh +1 -0
@@ -30,6 +30,7 @@ test_expect_success 'Verify behavior of status on directories with ignored files dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign && git status --porcelain=v2 --ignored=matching --untracked-files=all >output && + test_filter_gitconfig output && test_cmp expect output '
Signature · 1 line
gitgitgadget
PATCH 8/8 safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When an attacker can convince a user to clone a crafted repository that contains an embedded bare repository with malicious hooks, any Git command the user runs after entering that subdirectory will discover the bare repository and execute the hooks. The user does not even need to run a Git command explicitly: many shell prompts run git status in the background to display branch and dirty state information, and git status in turn may invoke the fsmonitor hook if so configured, making the user vulnerable the moment they cd into the directory. The safe.bareRepository configuration variable (introduced in 8959555cee7e (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02)) already provides protection against this attack vector by allowing users to set it to "explicit", but the default remained "all" for backwards compatibility.
Since Git 3.0 is the natural point to change defaults to safer values, flip the default from "all" to "explicit" when built with WITH_BREAKING_CHANGES. This means Git will refuse to work with bare repositories that are discovered implicitly by walking up the directory tree. Bare repositories specified via --git-dir or GIT_DIR continue to work, and directories that look like .git, worktrees, or submodule directories are unaffected (the existing is_implicit_bare_repo() whitelist handles those cases).
Users who rely on implicit bare repository discovery can restore the previous behavior by setting safe.bareRepository=all in their global or system configuration.
The test for the "safe.bareRepository in the repository" scenario needed a more involved fix: it writes a safe.bareRepository=all entry into the bare repository's own config to verify that repo-local config does not override the protected (global) setting. Previously, test_config -C was used to write that entry, but its cleanup runs git -C <bare-repo> config --unset, which itself fails when the default is "explicit" and the global config has already been cleaned up. Switching to direct git config --file access avoids going through repository discovery entirely.
--- Documentation/BreakingChanges.adoc | 24 ++++++++++++++++++++++++ Documentation/config/safe.adoc | 10 ++++++++-- setup.c | 4 ++++ t/t0035-safe-bare-repository.sh | 10 ++++++++-- 4 files changed, 44 insertions(+), 4 deletions(-)
Documentation/BreakingChanges.adoc +24 -0
@@ -216,6 +216,30 @@ would be significant, we may decide to defer this change to a subsequent minor release. This evaluation will also take into account our own experience with how painful it is to keep Rust an optional component. +* The default value of `safe.bareRepository` will change from `all` to + `explicit`. It is all too easy for an attacker to trick a user into cloning a + repository that contains an embedded bare repository with malicious hooks + configured. If the user enters that subdirectory and runs any Git command, Git + discovers the bare repository and the hooks fire. The user does not even need + to run a Git command explicitly: many shell prompts run `git status` in the + background to display branch and dirty state information, and `git status` in + turn may invoke the fsmonitor hook if so configured, making the user + vulnerable the moment they `cd` into the directory. The `safe.bareRepository` + configuration variable was introduced in 8959555cee (setup_git_directory(): + add an owner check for the top-level directory, 2022-03-02) with a default of + `all` to preserve backwards compatibility. ++ +Changing the default to `explicit` means that Git will refuse to work with bare +repositories that are discovered implicitly by walking up the directory tree. +Bare repositories specified explicitly via the `--git-dir` command-line option +or the `GIT_DIR` environment variable continue to work regardless of this +setting. Repositories that look like a `.git` directory, a worktree, or a +submodule directory are also unaffected. ++ +Users who rely on implicit discovery of bare repositories can restore the +previous behavior by setting `safe.bareRepository=all` in their global or +system configuration. + === Removals * Support for grafting commits has long been superseded by git-replace(1).
Documentation/config/safe.adoc +8 -2
@@ -2,10 +2,12 @@ safe.bareRepository:: Specifies which bare repositories Git will work with. The currently supported values are: + -* `all`: Git works with all bare repositories. This is the default. +* `all`: Git works with all bare repositories. This is the default in + Git 2.x. * `explicit`: Git only works with bare repositories specified via the top-level `--git-dir` command-line option, or the `GIT_DIR` - environment variable (see linkgit:git[1]). + environment variable (see linkgit:git[1]). This will be the default + in Git 3.0. + If you do not use bare repositories in your workflow, then it may be beneficial to set `safe.bareRepository` to `explicit` in your global
@@ -13,6 +15,10 @@ config. This will protect you from attacks that involve cloning a repository that contains a bare repository and running a Git command within that directory. + +If you use bare repositories regularly and want to preserve the current +behavior after upgrading to Git 3.0, set `safe.bareRepository` to `all` +in your global or system config. ++ This config setting is only respected in protected configuration (see <<SCOPES>>). This prevents untrusted repositories from tampering with this value.
setup.c +4 -0
@@ -1485,7 +1485,11 @@ static int allowed_bare_repo_cb(const char *key, const char *value,static enum allowed_bare_repo get_allowed_bare_repo (void ) { +#ifdef WITH_BREAKING_CHANGES +enum allowed_bare_repo result =ALLOWED_BARE_REPO_EXPLICIT ; +#else enum allowed_bare_repo result =ALLOWED_BARE_REPO_ALL ; +#endif git_protected_config (allowed_bare_repo_cb, &result);return result; }
t/t0035-safe-bare-repository.sh +8 -2
@@ -44,11 +44,16 @@ test_expect_success 'setup an embedded bare repo, secondary worktree and submodu test_path_is_dir outer-repo/.git/modules/subn ' -test_expect_success 'safe.bareRepository unset' ' +test_expect_success !WITH_BREAKING_CHANGES 'safe.bareRepository unset' ' test_unconfig --global safe.bareRepository && expect_accepted_implicit -C outer-repo/bare-repo ' +test_expect_success WITH_BREAKING_CHANGES 'safe.bareRepository unset (defaults to explicit)' ' + test_unconfig --global safe.bareRepository && + expect_rejected -C outer-repo/bare-repo +' + test_expect_success 'safe.bareRepository=all' ' test_config_global safe.bareRepository all && expect_accepted_implicit -C outer-repo/bare-repo
@@ -63,7 +68,8 @@ test_expect_success 'safe.bareRepository in the repository' ' # safe.bareRepository must not be "explicit", otherwise # git config fails with "fatal: not in a git directory" (like # safe.directory) - test_config -C outer-repo/bare-repo safe.bareRepository all && + test_when_finished "git config --file outer-repo/bare-repo/config --unset safe.bareRepository" && + git config --file outer-repo/bare-repo/config safe.bareRepository all && test_config_global safe.bareRepository explicit && expect_rejected -C outer-repo/bare-repo '
Signature · 1 line
gitgitgadget
v2 cover safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
In reply to Johannes Schindelin via GitGitGadgetThis supersedes my earlier series [*1*] which took the approach of adjusting individual tests to access bare repositories explicitly.
As Junio suggested [*2*], this series instead takes the approach of setting safe.bareRepository=all in the test environment's global config whenever WITH_BREAKING_CHANGES is in effect, so that existing tests continue to work without individual modifications.
Implementing this turned out to require a number of follow-up adjustments, because writing to $HOME/.gitconfig has side effects beyond the intended
setting: $HOME is the trash directory, which doubles as the test repository's working tree, so the file shows up in ls-files and status output, and tests that manipulate $HOME/.gitconfig for their own purposes can clobber or remove the setting. Patches 2 through 7 address these interactions in the affected test scripts.
The final patch flips the safe.bareRepository default to "explicit" under WITH_BREAKING_CHANGES.
Footnote [*1*]:
https://lore.kernel.org/git/[email protected]/
Footnote [*2*]: https://lore.kernel.org/git/[email protected]/
Changes since v1:
* Made it compatible with Patrick Steinhardt's set -e work.
Johannes Schindelin (8):
test-lib: allow bare repository access when breaking changes are
enabled
t7900: do not let $HOME/.gitconfig interfere with XDG tests
t1300: remove global config settings injected by test-lib.sh
t1305: use --git-dir=. for bare repo in include cycle test
t5601: restore .gitconfig after includeIf test
ls-files tests: filter .gitconfig from --others output status tests: filter .gitconfig from status output safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
Documentation/BreakingChanges.adoc | 24 +++++++++++++++++ Documentation/config/safe.adoc | 10 +++++-- setup.c | 4 +++ t/t0035-safe-bare-repository.sh | 10 +++++-- t/t1300-config.sh | 7 +++++ t/t1305-config-include.sh | 4 +-- t/t3000-ls-files-others.sh | 4 +++ t/t3001-ls-files-others-exclude.sh | 3 +++ t/t3002-ls-files-dashpath.sh | 2 ++ t/t3009-ls-files-others-nonsubmodule.sh | 1 + ...common-prefixes-and-directory-traversal.sh | 3 ++- t/t5601-clone.sh | 4 ++- t/t7060-wtstatus.sh | 3 +-- t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++ t/t7064-wtstatus-pv2.sh | 1 + t/t7104-reset-hard.sh | 2 +- t/t7508-status.sh | 4 +++ t/t7521-ignored-mode.sh | 1 + t/t7900-maintenance.sh | 12 +++++++-- t/test-lib-functions.sh | 8 ++++++ t/test-lib.sh | 13 +++++++++ 21 files changed, 134 insertions(+), 13 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2098%2Fdscho%2Fsafe-bare-repo-default-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2098/dscho/safe-bare-repo-default-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2098
Range-diff vs v1:
1: 62707b4109 ! 1: 179fcf5369 test-lib: allow bare repository access when breaking changes are enabled
@@ Commit message
--exclude-standard or git status --ignored; those are addressed
by subsequent commits.
+ The .git/info/exclude write is guarded by test -d .git/info
+ rather than using mkdir -p, because some tests (e.g. t0008)
+ expect to create .git/info/ themselves and would fail with
+ Patrick Steinhardt's set -e preparation (ps/test-set-e-clean) if
+ the directory already existed. For tests using TEST_NO_CREATE_REPO
+ (where no .git/ exists at all), the guard also handles that case.
+
[1] https://lore.kernel.org/git/[email protected]/
@@ t/test-lib.sh: cd -P "$TRASH_DIRECTORY" || BAIL_OUT "cannot cd -P to \"$TRASH_DI
+if test -n "$WITH_BREAKING_CHANGES"
+then
+ git config --global safe.bareRepository all &&
-+ echo "/.gitconfig" >>.git/info/exclude
++ # Only write to .git/info/exclude when the directory exists
++ # (i.e. when git init created the repo). If we mkdir -p it
++ # ourselves, tests that expect to create .git/info/ themselves
++ # (e.g. t0008) would fail.
++ if test -d .git/info
++ then
++ echo "/.gitconfig" >>.git/info/exclude
++ fi
+fi
+
start_test_output "$0"
2: d9a2e76f3c = 2: 4dc5151e59 t7900: do not let `$HOME/.gitconfig` interfere with XDG tests 3: 9c10e72eed = 3: 7d68155805 t1300: remove global config settings injected by test-lib.sh 4: 092ec11621 = 4: 5ff48e0892 t1305: use `--git-dir=.` for bare repo in include cycle test 5: 3aca302275 = 5: ed7294ace3 t5601: restore `.gitconfig` after includeIf test 6: ef57244778 = 6: 556db0eabe ls-files tests: filter `.gitconfig` from `--others` output 7: 56fe902644 = 7: ac4da79eac status tests: filter `.gitconfig` from status output 8: 64db45e385 = 8: 73bb1aa171 safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
Signature · 1 line
gitgitgadget
PATCH v2 1/8 test-lib: allow bare repository access when breaking changes are enabled
From: Johannes Schindelin <[email protected]>
A future patch will change the safe.bareRepository default from all to explicit under WITH_BREAKING_CHANGES. At that point, every test that operates on a bare repository through implicit discovery would fail, regardless of whether the test is actually about discovery or about how a specific command behaves once inside a bare repository.
The maintainer suggested [1] setting safe.bareRepository=all in the test environment's global config whenever WITH_BREAKING_CHANGES is in effect, rather than adjusting each affected test to access bare repositories explicitly (via --git-dir, GIT_DIR, or similar). This means the test suite continues to exercise only the historical default behavior even after the user-facing default changes, relying on a small number of dedicated tests in t0035 to validate the new, stricter default.
Since $HOME points at the trash directory (which doubles as the test repository's working tree), writing to $HOME/.gitconfig also creates a file inside the working tree. Exclude it via .git/info/exclude to limit the fallout, though this does not help tests that use git ls-files --others without --exclude-standard or git status --ignored; those are addressed by subsequent commits.
The .git/info/exclude write is guarded by test -d .git/info rather than using mkdir -p, because some tests (e.g. t0008) expect to create .git/info/ themselves and would fail with Patrick Steinhardt's set -e preparation (ps/test-set-e-clean) if the directory already existed. For tests using TEST_NO_CREATE_REPO (where no .git/ exists at all), the guard also handles that case.
[1] https://lore.kernel.org/git/[email protected]/
--- t/test-lib.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+)
t/test-lib.sh +13 -0
@@ -1597,6 +1597,19 @@ cd -P "$TRASH_DIRECTORY" || BAIL_OUT "cannot cd -P to \"$TRASH_DIRECTORY\"" TRASH_DIRECTORY=$(pwd) HOME="$TRASH_DIRECTORY" +if test -n "$WITH_BREAKING_CHANGES" +then + git config --global safe.bareRepository all && + # Only write to .git/info/exclude when the directory exists + # (i.e. when git init created the repo). If we mkdir -p it + # ourselves, tests that expect to create .git/info/ themselves + # (e.g. t0008) would fail. + if test -d .git/info + then + echo "/.gitconfig" >>.git/info/exclude + fi +fi + start_test_output "$0" # Convenience
Signature · 1 line
gitgitgadget
PATCH v2 2/8 t7900: do not let `$HOME/.gitconfig` interfere with XDG tests
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The XDG config tests for git maintenance register/unregister create a fresh $XDG_CONFIG_HOME/git/config and expect git to use that location. However, if $HOME/.gitconfig exists (which may happen when test-lib.sh writes global config, e.g. to set safe.bareRepository), git prefers $HOME/.gitconfig over the XDG location, and the maintenance.repo entry ends up in the wrong file.
This is an inherent consequence of setting global config in test-lib.sh rather than adjusting individual tests: writing any entry to $HOME/.gitconfig has side effects beyond the intended setting, because the mere existence of that file changes which global config location git prefers for all subsequent writes. Individual per-test adjustments would not have this interaction.
Fix this by overriding HOME to a non-existent directory inside the subshells that test XDG behavior. Since these subshells already override XDG_CONFIG_HOME, they do not need $HOME/.gitconfig at all, and the subshell scoping ensures the original HOME is restored automatically.
--- t/t7900-maintenance.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
t/t7900-maintenance.sh +10 -2
@@ -101,8 +101,12 @@ test_expect_success "maintenance.autoDetach overrides gc.autoDetach" ' test_expect_success 'register uses XDG_CONFIG_HOME config if it exists' ' test_when_finished rm -r .config/git/config && ( + # Override HOME so that .gitconfig (which test-lib.sh may + # have created, e.g. to set safe.bareRepository) does not + # take precedence over the XDG location. + HOME=$PWD/must-not-exist && XDG_CONFIG_HOME=.config && - export XDG_CONFIG_HOME && + export HOME XDG_CONFIG_HOME && mkdir -p $XDG_CONFIG_HOME/git && >$XDG_CONFIG_HOME/git/config && git maintenance register &&
@@ -124,8 +128,12 @@ test_expect_success 'register does not need XDG_CONFIG_HOME config to exist' ' test_expect_success 'unregister uses XDG_CONFIG_HOME config if it exists' ' test_when_finished rm -r .config/git/config && ( + # Override HOME so that .gitconfig (which test-lib.sh may + # have created, e.g. to set safe.bareRepository) does not + # take precedence over the XDG location. + HOME=$PWD/must-not-exist && XDG_CONFIG_HOME=.config && - export XDG_CONFIG_HOME && + export HOME XDG_CONFIG_HOME && mkdir -p $XDG_CONFIG_HOME/git && >$XDG_CONFIG_HOME/git/config && git maintenance register &&
Signature · 1 line
gitgitgadget
PATCH v2 3/8 t1300: remove global config settings injected by test-lib.sh
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Since test-lib.sh now writes safe.bareRepository=all to the global config when WITH_BREAKING_CHANGES is in effect, that entry shows up in git config --list output. Tests in t1300 that expect exact config contents then fail because of this unexpected extra line.
Unlike the working-tree contamination fixed in the preceding commits, this is not about the file's existence but about its content leaking into test expectations. Since t1300 does not use bare repositories, simply remove the injected setting in a preparatory step.
--- t/t1300-config.sh | 7 +++++++ 1 file changed, 7 insertions(+)
t/t1300-config.sh +7 -0
@@ -11,6 +11,13 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh +# test-lib.sh may have added global config (e.g. safe.bareRepository) +# that would appear in "git config --list" output and break tests +# that expect exact config contents. +test_expect_success 'remove global config from test-lib.sh' ' + test_might_fail git config --global --unset-all safe.bareRepository +' + for mode in legacy subcommands do
Signature · 1 line
gitgitgadget
PATCH v2 4/8 t1305: use `--git-dir=.` for bare repo in include cycle test
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Earlier tests in t1305 overwrite $HOME/.gitconfig with their own content as part of testing config includes. This clobbers the safe.bareRepository=all entry that test-lib.sh writes when WITH_BREAKING_CHANGES is in effect, causing git -C cycle config to fail with "not in a git directory" when it tries to access the bare repository created by git init --bare cycle.
Use --git-dir=. to access the bare repo explicitly, avoiding the dependency on global config for repository discovery.
--- t/t1305-config-include.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
t/t1305-config-include.sh +2 -2
@@ -350,9 +350,9 @@ test_expect_success 'conditional include, onbranch, implicit /** for /' ' test_expect_success 'include cycles are detected' ' git init --bare cycle && - git -C cycle config include.path cycle && + git -C cycle --git-dir=. config include.path cycle && git config -f cycle/cycle include.path config && - test_must_fail git -C cycle config --get-all test.value 2>stderr && + test_must_fail git -C cycle --git-dir=. config --get-all test.value 2>stderr && grep "exceeded maximum include depth" stderr '
Signature · 1 line
gitgitgadget
PATCH v2 5/8 t5601: restore `.gitconfig` after includeIf test
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
One test in t5601 overwrites $HOME/.gitconfig with an includeIf configuration snippet and removes the file in its cleanup. This destroys the safe.bareRepository=all entry that test-lib.sh writes when WITH_BREAKING_CHANGES is in effect, causing later tests that use git -C <bare-repo> config to fail with "not in a git directory".
Back up .gitconfig before overwriting and restore it in the cleanup, so the global config survives into subsequent tests.
--- t/t5601-clone.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
t/t5601-clone.sh +3 -1
@@ -813,7 +813,9 @@ test_expect_success 'clone with includeIf' ' test_when_finished "rm -rf repo \"$HTTPD_DOCUMENT_ROOT_PATH/repo.git\"" && git clone --bare --no-local src "$HTTPD_DOCUMENT_ROOT_PATH/repo.git" && - test_when_finished "rm \"$HOME\"/.gitconfig" && + test_when_finished "cp \"$HOME\"/.gitconfig.bak \ + \"$HOME\"/.gitconfig 2>/dev/null || rm -f \"$HOME\"/.gitconfig" && + cp "$HOME"/.gitconfig "$HOME"/.gitconfig.bak 2>/dev/null && cat >"$HOME"/.gitconfig <<-EOF && [includeIf "onbranch:something"] path = /does/not/exist.inc
Signature · 1 line
gitgitgadget
PATCH v2 6/8 ls-files tests: filter `.gitconfig` from `--others` output
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
The global safe.bareRepository=all setting in test-lib.sh is written to $HOME/.gitconfig, which unfortunately lives inside the test repository's working tree. The .git/info/exclude entry added alongside it handles most commands, but git ls-files --others without --exclude-standard does not consult info/exclude at all, so the file appears in the output.
Ideally, each test that accesses a bare repository would simply specify --git-dir or GIT_DIR explicitly, which would require no global config and produce no side effects in the working tree. As that approach was not taken, filter .gitconfig from the output before comparing against expected results. In t7104, the test already uses --exclude-standard, so it suffices to switch from the bare git ls-files -o to git ls-files -o --exclude-standard which respects the info/exclude entry; the other tests deliberately omit --exclude-standard because their purpose is to verify unfiltered --others output.
--- t/t3000-ls-files-others.sh | 4 ++++ t/t3001-ls-files-others-exclude.sh | 3 +++ t/t3002-ls-files-dashpath.sh | 2 ++ t/t3009-ls-files-others-nonsubmodule.sh | 1 + t/t3011-common-prefixes-and-directory-traversal.sh | 3 ++- t/t7104-reset-hard.sh | 2 +- t/test-lib-functions.sh | 8 ++++++++ 7 files changed, 21 insertions(+), 2 deletions(-)
t/t3000-ls-files-others.sh +4 -0
@@ -53,16 +53,19 @@ test_expect_success 'setup: expected output' ' test_expect_success 'ls-files --others' ' git ls-files --others >output && + test_filter_gitconfig output && test_cmp expected1 output ' test_expect_success 'ls-files --others --directory' ' git ls-files --others --directory >output && + test_filter_gitconfig output && test_cmp expected2 output ' test_expect_success '--no-empty-directory hides empty directory' ' git ls-files --others --directory --no-empty-directory >output && + test_filter_gitconfig output && test_cmp expected3 output '
@@ -70,6 +73,7 @@ test_expect_success 'ls-files --others handles non-submodule .git' ' mkdir not-a-submodule && echo foo >not-a-submodule/.git && git ls-files -o >output && + test_filter_gitconfig output && test_cmp expected1 output '
t/t3001-ls-files-others-exclude.sh +3 -0
@@ -72,6 +72,7 @@ test_expect_success 'git ls-files --others with various exclude options.' ' --exclude-per-directory=.gitignore \ --exclude-from=.git/ignore \ >output && + test_filter_gitconfig output && test_cmp expect output '
@@ -84,6 +85,7 @@ test_expect_success 'git ls-files --others with \r\n line endings.' ' --exclude-per-directory=.gitignore \ --exclude-from=.git/ignore \ >output && + test_filter_gitconfig output && test_cmp expect output '
@@ -99,6 +101,7 @@ test_expect_success 'git ls-files --others with various exclude options.' ' --exclude-per-directory=.gitignore \ --exclude-from=.git/ignore \ >output && + test_filter_gitconfig output && test_cmp expect output '
t/t3002-ls-files-dashpath.sh +2 -0
@@ -24,6 +24,7 @@ test_expect_success 'setup' ' test_expect_success 'git ls-files without path restriction.' ' test_when_finished "rm -f expect" && git ls-files --others >output && + test_filter_gitconfig output && cat >expect <<-\EOF && -- -foo
@@ -63,6 +64,7 @@ test_expect_success 'git ls-files with path restriction with -- --.' ' test_expect_success 'git ls-files with no path restriction.' ' test_when_finished "rm -f expect" && git ls-files --others -- >output && + test_filter_gitconfig output && cat >expect <<-\EOF && -- -foo
t/t3009-ls-files-others-nonsubmodule.sh +1 -0
@@ -36,6 +36,7 @@ test_expect_success 'setup: directories' ' test_expect_success 'ls-files --others handles untracked git repositories' ' git ls-files -o >output && + test_filter_gitconfig output && cat >expect <<-EOF && nonrepo-untracked-file/untracked output
t/t3011-common-prefixes-and-directory-traversal.sh +2 -1
@@ -26,7 +26,7 @@ test_expect_success 'setup' ' ' test_expect_success 'git ls-files -o shows the right entries' ' - cat <<-EOF >expect && + cat >expect <<-EOF && .gitignore actual an_ignored_dir/ignored
@@ -39,6 +39,7 @@ test_expect_success 'git ls-files -o shows the right entries' ' untracked_repo/ EOF git ls-files -o >actual && + test_filter_gitconfig actual && test_cmp expect actual '
t/t7104-reset-hard.sh +1 -1
@@ -21,7 +21,7 @@ test_expect_success setup ' rm -f hello && mkdir -p hello && >hello/world && - test "$(git ls-files -o)" = hello/world + test "$(git ls-files -o --exclude-standard)" = hello/world '
t/test-lib-functions.sh +8 -0
@@ -2069,3 +2069,11 @@ test_trailing_hash () { test_redact_non_printables () { tr -d "\n\r" | tr "[\001-\040][\177-\377]" "." } + +# Remove .gitconfig entries from a file in place. test-lib.sh may +# create $HOME/.gitconfig (e.g. to set safe.bareRepository) which +# can appear in ls-files or status output. +test_filter_gitconfig () { + sed "/\\.gitconfig/d" "$1" >"$1.filtered" && + mv "$1.filtered" "$1" +}
Signature · 1 line
gitgitgadget
PATCH v2 7/8 status tests: filter `.gitconfig` from status output
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
Since test-lib.sh creates $HOME/.gitconfig when WITH_BREAKING_CHANGES is in effect, the file appears in git status output as either untracked (?? .gitconfig) or ignored (!! .gitconfig / ! .gitconfig, depending on porcelain version), because the .git/info/exclude entry causes git to treat it as an ignored file rather than hiding it entirely.
In t7061 and t7521, which are pervasively affected, introduce a filter_gitconfig helper that strips all status-prefix variants of .gitconfig from the output before comparison. In the remaining scripts (t7060, t7064, t7508), apply targeted adjustments.
--- t/t7060-wtstatus.sh | 3 +-- t/t7061-wtstatus-ignore.sh | 27 +++++++++++++++++++++++++++ t/t7064-wtstatus-pv2.sh | 1 + t/t7508-status.sh | 4 ++++ t/t7521-ignored-mode.sh | 1 + 5 files changed, 34 insertions(+), 2 deletions(-)
t/t7060-wtstatus.sh +1 -2
@@ -9,6 +9,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME test_expect_success setup ' git config --global advice.statusuoption false && + echo "/.gitconfig" >>.git/info/exclude && test_commit A && test_commit B oneside added && git checkout A^0 &&
@@ -221,7 +222,6 @@ test_expect_success 'status --branch with detached HEAD' ' git status --branch --porcelain >actual && cat >expected <<-EOF && ## HEAD (no branch) - ?? .gitconfig ?? actual ?? expect ?? expected
@@ -237,7 +237,6 @@ test_expect_success 'status --porcelain=v1 --branch with detached HEAD' ' git status --branch --porcelain=v1 >actual && cat >expected <<-EOF && ## HEAD (no branch) - ?? .gitconfig ?? actual ?? expect ?? expected
t/t7061-wtstatus-ignore.sh +27 -0
@@ -18,6 +18,7 @@ test_expect_success 'status untracked directory with --ignored' ' : >untracked/ignored && : >untracked/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -27,6 +28,7 @@ test_expect_success 'same with gitignore starting with BOM' ' : >untracked/ignored && : >untracked/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -40,18 +42,22 @@ test_expect_success 'status untracked files --ignored with pathspec (no match)' test_expect_success 'status untracked files --ignored with pathspec (literal match)' ' git status --porcelain --ignored -- untracked/ignored >actual && echo "!! untracked/ignored" >expected && + test_filter_gitconfig actual && test_cmp expected actual && git status --porcelain --ignored -- untracked/uncommitted >actual && echo "?? untracked/uncommitted" >expected && + test_filter_gitconfig actual && test_cmp expected actual ' test_expect_success 'status untracked files --ignored with pathspec (glob match)' ' git status --porcelain --ignored -- untracked/i\* >actual && echo "!! untracked/ignored" >expected && + test_filter_gitconfig actual && test_cmp expected actual && git status --porcelain --ignored -- untracked/u\* >actual && echo "?? untracked/uncommitted" >expected && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -65,6 +71,7 @@ EOF test_expect_success 'status untracked directory with --ignored -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual ' cat >expected <<\EOF
@@ -76,9 +83,11 @@ test_expect_success 'status of untracked directory with --ignored works with or git status --porcelain --ignored >tmp && grep untracked/ tmp >actual && rm tmp && + test_filter_gitconfig actual && test_cmp expected actual && git status --porcelain --ignored untracked/ >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -89,6 +98,7 @@ EOF test_expect_success 'status prefixed untracked sub-directory with --ignored -u' ' git status --porcelain --ignored -u untracked/ >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -104,6 +114,7 @@ test_expect_success 'status ignored directory with --ignore' ' mkdir ignored && : >ignored/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -116,6 +127,7 @@ EOF test_expect_success 'status ignored directory with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -130,6 +142,7 @@ test_expect_success 'status empty untracked directory with --ignore' ' mkdir untracked-ignored && mkdir untracked-ignored/test && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -141,6 +154,7 @@ EOF test_expect_success 'status empty untracked directory with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -155,6 +169,7 @@ test_expect_success 'status untracked directory with ignored files with --ignore : >untracked-ignored/ignored && : >untracked-ignored/test/ignored && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -168,6 +183,7 @@ EOF test_expect_success 'status untracked directory with ignored files with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -185,6 +201,7 @@ test_expect_success 'status ignored tracked directory with --ignore' ' git commit -m. && echo "tracked" >.gitignore && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -196,6 +213,7 @@ EOF test_expect_success 'status ignored tracked directory with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -208,6 +226,7 @@ EOF test_expect_success 'status ignored tracked directory and ignored file with --ignore' ' echo "committed" >>.gitignore && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -219,6 +238,7 @@ EOF test_expect_success 'status ignored tracked directory and ignored file with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -233,6 +253,7 @@ test_expect_success 'status ignored tracked directory and uncommitted file with echo "tracked" >.gitignore && : >tracked/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -245,6 +266,7 @@ EOF test_expect_success 'status ignored tracked directory and uncommitted file with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -260,6 +282,7 @@ test_expect_success 'status ignored tracked directory with uncommitted file in u mkdir tracked/ignored && : >tracked/ignored/uncommitted && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -272,6 +295,7 @@ EOF test_expect_success 'status ignored tracked directory with uncommitted file in untracked subdir with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -287,6 +311,7 @@ test_expect_success 'status ignored tracked directory with uncommitted file in t git add -f tracked/ignored/committed && git commit -m. && git status --porcelain --ignored >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -299,6 +324,7 @@ EOF test_expect_success 'status ignored tracked directory with uncommitted file in tracked subdir with --ignore -u' ' git status --porcelain --ignored -u >actual && + test_filter_gitconfig actual && test_cmp expected actual '
@@ -310,6 +336,7 @@ test_expect_success 'status ignores submodule in excluded directory' ' git init tracked/submodule && test_commit -C tracked/submodule initial && git status --porcelain --ignored -u tracked/submodule >actual && + test_filter_gitconfig actual && test_cmp expected actual '
t/t7064-wtstatus-pv2.sh +1 -0
@@ -231,6 +231,7 @@ test_expect_success 'ignored files are printed with --ignored' ' EOF git status --porcelain=v2 --ignored --untracked-files=all >actual && + test_filter_gitconfig actual && test_cmp expect actual '
t/t7508-status.sh +4 -0
@@ -263,6 +263,7 @@ test_expect_success 'status with gitignore' ' !! untracked EOF git status -s --ignored >output && + test_filter_gitconfig output && test_cmp expect output && cat >expect <<\EOF &&
@@ -296,6 +297,7 @@ Ignored files: EOF git status --ignored >output && + test_filter_gitconfig output && test_cmp expect output '
@@ -328,6 +330,7 @@ test_expect_success 'status with gitignore (nothing untracked)' ' !! untracked EOF git status -s --ignored >output && + test_filter_gitconfig output && test_cmp expect output && cat >expect <<\EOF &&
@@ -358,6 +361,7 @@ Ignored files: EOF git status --ignored >output && + test_filter_gitconfig output && test_cmp expect output '
t/t7521-ignored-mode.sh +1 -0
@@ -30,6 +30,7 @@ test_expect_success 'Verify behavior of status on directories with ignored files dir/ignored/ignored_1.ign dir/ignored/ignored_2.ign && git status --porcelain=v2 --ignored=matching --untracked-files=all >output && + test_filter_gitconfig output && test_cmp expect output '
Signature · 1 line
gitgitgadget
PATCH v2 8/8 safe.bareRepository: default to "explicit" with WITH_BREAKING_CHANGES
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When an attacker can convince a user to clone a crafted repository that contains an embedded bare repository with malicious hooks, any Git command the user runs after entering that subdirectory will discover the bare repository and execute the hooks. The user does not even need to run a Git command explicitly: many shell prompts run git status in the background to display branch and dirty state information, and git status in turn may invoke the fsmonitor hook if so configured, making the user vulnerable the moment they cd into the directory. The safe.bareRepository configuration variable (introduced in 8959555cee7e (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02)) already provides protection against this attack vector by allowing users to set it to "explicit", but the default remained "all" for backwards compatibility.
Since Git 3.0 is the natural point to change defaults to safer values, flip the default from "all" to "explicit" when built with WITH_BREAKING_CHANGES. This means Git will refuse to work with bare repositories that are discovered implicitly by walking up the directory tree. Bare repositories specified via --git-dir or GIT_DIR continue to work, and directories that look like .git, worktrees, or submodule directories are unaffected (the existing is_implicit_bare_repo() whitelist handles those cases).
Users who rely on implicit bare repository discovery can restore the previous behavior by setting safe.bareRepository=all in their global or system configuration.
The test for the "safe.bareRepository in the repository" scenario needed a more involved fix: it writes a safe.bareRepository=all entry into the bare repository's own config to verify that repo-local config does not override the protected (global) setting. Previously, test_config -C was used to write that entry, but its cleanup runs git -C <bare-repo> config --unset, which itself fails when the default is "explicit" and the global config has already been cleaned up. Switching to direct git config --file access avoids going through repository discovery entirely.
--- Documentation/BreakingChanges.adoc | 24 ++++++++++++++++++++++++ Documentation/config/safe.adoc | 10 ++++++++-- setup.c | 4 ++++ t/t0035-safe-bare-repository.sh | 10 ++++++++-- 4 files changed, 44 insertions(+), 4 deletions(-)
Documentation/BreakingChanges.adoc +24 -0
@@ -216,6 +216,30 @@ would be significant, we may decide to defer this change to a subsequent minor release. This evaluation will also take into account our own experience with how painful it is to keep Rust an optional component. +* The default value of `safe.bareRepository` will change from `all` to + `explicit`. It is all too easy for an attacker to trick a user into cloning a + repository that contains an embedded bare repository with malicious hooks + configured. If the user enters that subdirectory and runs any Git command, Git + discovers the bare repository and the hooks fire. The user does not even need + to run a Git command explicitly: many shell prompts run `git status` in the + background to display branch and dirty state information, and `git status` in + turn may invoke the fsmonitor hook if so configured, making the user + vulnerable the moment they `cd` into the directory. The `safe.bareRepository` + configuration variable was introduced in 8959555cee (setup_git_directory(): + add an owner check for the top-level directory, 2022-03-02) with a default of + `all` to preserve backwards compatibility. ++ +Changing the default to `explicit` means that Git will refuse to work with bare +repositories that are discovered implicitly by walking up the directory tree. +Bare repositories specified explicitly via the `--git-dir` command-line option +or the `GIT_DIR` environment variable continue to work regardless of this +setting. Repositories that look like a `.git` directory, a worktree, or a +submodule directory are also unaffected. ++ +Users who rely on implicit discovery of bare repositories can restore the +previous behavior by setting `safe.bareRepository=all` in their global or +system configuration. + === Removals * Support for grafting commits has long been superseded by git-replace(1).
Documentation/config/safe.adoc +8 -2
@@ -2,10 +2,12 @@ safe.bareRepository:: Specifies which bare repositories Git will work with. The currently supported values are: + -* `all`: Git works with all bare repositories. This is the default. +* `all`: Git works with all bare repositories. This is the default in + Git 2.x. * `explicit`: Git only works with bare repositories specified via the top-level `--git-dir` command-line option, or the `GIT_DIR` - environment variable (see linkgit:git[1]). + environment variable (see linkgit:git[1]). This will be the default + in Git 3.0. + If you do not use bare repositories in your workflow, then it may be beneficial to set `safe.bareRepository` to `explicit` in your global
@@ -13,6 +15,10 @@ config. This will protect you from attacks that involve cloning a repository that contains a bare repository and running a Git command within that directory. + +If you use bare repositories regularly and want to preserve the current +behavior after upgrading to Git 3.0, set `safe.bareRepository` to `all` +in your global or system config. ++ This config setting is only respected in protected configuration (see <<SCOPES>>). This prevents untrusted repositories from tampering with this value.
setup.c +4 -0
@@ -1485,7 +1485,11 @@ static int allowed_bare_repo_cb(const char *key, const char *value,static enum allowed_bare_repo get_allowed_bare_repo (void ) { +#ifdef WITH_BREAKING_CHANGES +enum allowed_bare_repo result =ALLOWED_BARE_REPO_EXPLICIT ; +#else enum allowed_bare_repo result =ALLOWED_BARE_REPO_ALL ; +#endif git_protected_config (allowed_bare_repo_cb, &result);return result; }
t/t0035-safe-bare-repository.sh +8 -2
@@ -44,11 +44,16 @@ test_expect_success 'setup an embedded bare repo, secondary worktree and submodu test_path_is_dir outer-repo/.git/modules/subn ' -test_expect_success 'safe.bareRepository unset' ' +test_expect_success !WITH_BREAKING_CHANGES 'safe.bareRepository unset' ' test_unconfig --global safe.bareRepository && expect_accepted_implicit -C outer-repo/bare-repo ' +test_expect_success WITH_BREAKING_CHANGES 'safe.bareRepository unset (defaults to explicit)' ' + test_unconfig --global safe.bareRepository && + expect_rejected -C outer-repo/bare-repo +' + test_expect_success 'safe.bareRepository=all' ' test_config_global safe.bareRepository all && expect_accepted_implicit -C outer-repo/bare-repo
@@ -63,7 +68,8 @@ test_expect_success 'safe.bareRepository in the repository' ' # safe.bareRepository must not be "explicit", otherwise # git config fails with "fatal: not in a git directory" (like # safe.directory) - test_config -C outer-repo/bare-repo safe.bareRepository all && + test_when_finished "git config --file outer-repo/bare-repo/config --unset safe.bareRepository" && + git config --file outer-repo/bare-repo/config safe.bareRepository all && test_config_global safe.bareRepository explicit && expect_rejected -C outer-repo/bare-repo '
Signature · 1 line
gitgitgadget