tests: access bare repositories explicitly

The safe.bareRepository configuration variable (introduced in 8d1a7448206e) allows restricting implicit bare repository discovery. Its default may well change to "explicit" in Git v3.0, at which point any test that relies on implicit discovery of a bare repository would break, even if the test subject has nothing to do with bare repositories.

This series adjusts 16 test scripts and git-p4 so that they access bare repositories explicitly. The techniques used are:

* Replace git -C <bare-repo> ... with git --git-dir=<bare-repo> ...
* Export GIT_DIR=. after cd-ing into a bare repository
* Wrap commands in (GIT_DIR=<path> && export GIT_DIR && ...)
* Add test_config_global safe.bareRepository all in the few tests where implicit discovery is genuinely part of what is being tested

Each commit is a self-contained fix to one test file (or a small related group).

This patch series is part of https://github.com/gitgitgadget/git/pull/2072.

Johannes Schindelin (17):
t0001: allow implicit bare repo discovery for aliased-command test
t0001: replace cd+git with git --git-dir in check_config
t0003: use --git-dir for bare repo attribute tests
t0056: allow implicit bare repo discovery for -C work-tree tests
t1020: use --git-dir instead of subshell for bare repo
t1900: avoid using -C <dir> for a bare repository
t2400: explicitly specify bare repo for git worktree add
t2406: use --git-dir=. for bare repository worktree repair
t5503: avoid discovering a bare repository
t5505: export GIT_DIR after git init --bare
t5509: specify bare repository path explicitly t5540/t5541: avoid accessing a bare repository via -C <dir>
t5619: wrap test_commit_bulk in GIT_DIR subshell for bare repo
t6020: use -C for worktree, --git-dir for bare repository
t9210: pass safe.bareRepository=all to scalar register
t9700: stop relying on implicit bare repo discovery
git p4 clone --bare: need to be explicit about the gitdir

 git-p4.py                                  |  1 +
 t/lib-httpd.sh                             | 12 ++--
 t/t0001-init.sh                            |  5 +-
 t/t0003-attributes.sh                      | 66 +++++++++-------------
 t/t0056-git-C.sh                           |  2 +
 t/t1020-subdirectory.sh                    |  5 +-
 t/t1900-repo-info.sh                       |  7 ++-
 t/t2400-worktree-add.sh                    | 21 +++----
 t/t2406-worktree-repair.sh                 |  2 +-
 t/t5503-tagfollow.sh                       | 13 ++---
 t/t5505-remote.sh                          |  4 +-
 t/t5509-fetch-push-namespaces.sh           | 12 ++--
 t/t5619-clone-local-ambiguous-transport.sh |  2 +-
 t/t6020-bundle-misc.sh                     |  4 +-
 t/t9210-scalar.sh                          |  2 +-
 t/t9700/test.pl                            |  9 ++-
 16 files changed, 74 insertions(+), 93 deletions(-)

base-commit: cf2139f8e1680b076e115bc0b349e369b4b0ecc4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2076%2Fdscho%2Ftests-explicit-bare-repo-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2076/dscho/tests-explicit-bare-repo-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2076

gitgitgadget

From: Johannes Schindelin <[email protected]>

8d1a7448206e (setup.c: create safe.bareRepository, 2022-07-14) introduced a setting to restrict implicit bare repository discovery, mitigating a social-engineering attack where an embedded bare repo's hooks get executed unknowingly. To allow for that default to change at some stage in the future, the tests need to be prepared.

This commit adjusts a test accordingly that runs git aliasedinit from inside a bare repo to verify that aliased commands work there. The test is about alias resolution, not bare repo discovery, so add test_config_global safe.bareRepository all to opt in explicitly.

---
 t/t0001-init.sh | 1 +
 1 file changed, 1 insertion(+)

index e4d32bb4d2..6bd0a15dac 100755
@@ -77,6 +77,7 @@ test_expect_success 'plain nested through aliased command' '
 '
 
 test_expect_success 'plain nested in bare through aliased command' '
+	test_config_global safe.bareRepository all &&
 	(
 		git init --bare bare-ancestor-aliased.git &&
 		cd bare-ancestor-aliased.git &&

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), replace cd <dir> && git config with git --git-dir=<dir> config so the helper does not rely on implicit bare repository discovery.

---
 t/t0001-init.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 6bd0a15dac..db2bf1001f 100755
@@ -20,8 +20,8 @@ check_config () {
 		return 1
 	fi
 
-	bare=$(cd "$1" && git config --bool core.bare)
-	worktree=$(cd "$1" && git config core.worktree) ||
+	bare=$(git --git-dir="$1" config --bool core.bare)
+	worktree=$(git --git-dir="$1" config core.worktree) ||
 	worktree=unset
 
 	test "$bare" = "$2" && test "$worktree" = "$3" || {

gitgitgadget

From: Johannes Schindelin <[email protected]>

The bare repo tests in t0003-attributes.sh currently cd into the bare repository inside subshells, relying on implicit discovery. Restructure these tests to pass --git-dir=bare.git to the attr_check and attr_check_source helpers instead. This makes the code much easier to read, and also makes bare repo access explicit, i.e. compatible with an eventual safe.bareRepository=explicit default.

---
 t/t0003-attributes.sh | 66 ++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

index 582e207aa1..3a34f5dbc2 100755
@@ -346,17 +346,14 @@ test_expect_success 'setup bare' '
 
 test_expect_success 'bare repository: check that .gitattribute is ignored' '
 	(
-		cd bare.git &&
-		(
-			echo "f	test=f" &&
-			echo "a/i test=a/i"
-		) >.gitattributes &&
-		attr_check f unspecified &&
-		attr_check a/f unspecified &&
-		attr_check a/c/f unspecified &&
-		attr_check a/i unspecified &&
-		attr_check subdir/a/i unspecified
-	)
+		echo "f	test=f" &&
+		echo "a/i test=a/i"
+	) >bare.git/.gitattributes &&
+	attr_check f unspecified --git-dir=bare.git &&
+	attr_check a/f unspecified --git-dir=bare.git &&
+	attr_check a/c/f unspecified --git-dir=bare.git &&
+	attr_check a/i unspecified --git-dir=bare.git &&
+	attr_check subdir/a/i unspecified --git-dir=bare.git
 '
 
 bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"

@@ -449,41 +446,32 @@ test_expect_success 'diff without repository with attr source' '
 '
 
 test_expect_success 'bare repository: with --source' '
-	(
-		cd bare.git &&
-		attr_check_source foo/bar/f f tag-1 &&
-		attr_check_source foo/bar/a/i n tag-1 &&
-		attr_check_source foo/bar/f unspecified tag-2 &&
-		attr_check_source foo/bar/a/i m tag-2 &&
-		attr_check_source foo/bar/g g tag-2 &&
-		attr_check_source foo/bar/g unspecified tag-1
-	)
+	attr_check_source foo/bar/f f tag-1 --git-dir=bare.git &&
+	attr_check_source foo/bar/a/i n tag-1 --git-dir=bare.git &&
+	attr_check_source foo/bar/f unspecified tag-2 --git-dir=bare.git &&
+	attr_check_source foo/bar/a/i m tag-2 --git-dir=bare.git &&
+	attr_check_source foo/bar/g g tag-2 --git-dir=bare.git &&
+	attr_check_source foo/bar/g unspecified tag-1 --git-dir=bare.git
 '
 
 test_expect_success 'bare repository: check that --cached honors index' '
-	(
-		cd bare.git &&
-		GIT_INDEX_FILE=../.git/index \
-		git check-attr --cached --stdin --all <../stdin-all |
-		sort >actual &&
-		test_cmp ../specified-all actual
-	)
+	GIT_INDEX_FILE=.git/index \
+	git --git-dir=bare.git check-attr --cached --stdin --all <stdin-all |
+	sort >actual &&
+	test_cmp specified-all actual
 '
 
 test_expect_success 'bare repository: test info/attributes' '
+	mkdir -p bare.git/info &&
 	(
-		cd bare.git &&
-		mkdir info &&
-		(
-			echo "f	test=f" &&
-			echo "a/i test=a/i"
-		) >info/attributes &&
-		attr_check f f &&
-		attr_check a/f f &&
-		attr_check a/c/f f &&
-		attr_check a/i a/i &&
-		attr_check subdir/a/i unspecified
-	)
+		echo "f	test=f" &&
+		echo "a/i test=a/i"
+	) >bare.git/info/attributes &&
+	attr_check f f --git-dir=bare.git &&
+	attr_check a/f f --git-dir=bare.git &&
+	attr_check a/c/f f --git-dir=bare.git &&
+	attr_check a/i a/i --git-dir=bare.git &&
+	attr_check subdir/a/i unspecified --git-dir=bare.git
 '
 
 test_expect_success 'binary macro expanded by -a' '

gitgitgadget

From: Johannes Schindelin <[email protected]>

The git -C c/a.git --work-tree=../a invocations in t0056-git-C.sh enter what is technically the .git directory of a repository to test -C combined with --work-tree. In doing so, the code relies on implicit discovery of bare repositories, which 8d1a7448206e (setup.c:
create safe.bareRepository, 2022-07-14) prepared to be prevented by default.

These tests verify the interaction between those flags, so changing them to use --git-dir would defeat their purpose. So let's just temporarily force-enable implicit discovery of bare repositories, no matter what safe.bareRepository defaults to.

---
 t/t0056-git-C.sh | 2 ++
 1 file changed, 2 insertions(+)

index 2630e756da..6b7122add5 100755
@@ -57,11 +57,13 @@ test_expect_success 'Order should not matter: "--git-dir=a.git -C c" is equivale
 test_expect_success 'Effect on --work-tree option: "-C c/a.git --work-tree=../a"  is equivalent to "--work-tree=c/a --git-dir=c/a.git"' '
 	rm c/a/a.txt &&
 	git --git-dir=c/a.git --work-tree=c/a status >expected &&
+	test_config_global safe.bareRepository all &&
 	git -C c/a.git --work-tree=../a status >actual &&
 	test_cmp expected actual
 '
 
 test_expect_success 'Order should not matter: "--work-tree=../a -C c/a.git" is equivalent to "-C c/a.git --work-tree=../a"' '
+	test_config_global safe.bareRepository all &&
 	git -C c/a.git --work-tree=../a status >expected &&
 	git --work-tree=../a -C c/a.git status >actual &&
 	test_cmp expected actual

gitgitgadget

From: Johannes Schindelin <[email protected]>

Replace an unnecessarily complex subshell pattern with a much simpler --git-dir-based one. The latter is not only simpler, it also no longer relies on implicit bare repo discovery, which would fail with safe.bareRepository=explicit.

---
 t/t1020-subdirectory.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

index 9fdbb2af80..20d2d306fe 100755
@@ -177,10 +177,7 @@ test_expect_success 'no file/rev ambiguity check inside a bare repo (explicit GI
 test_expect_success 'no file/rev ambiguity check inside a bare repo' '
 	test_when_finished "rm -fr foo.git" &&
 	git clone -s --bare .git foo.git &&
-	(
-		cd foo.git &&
-		git show -s HEAD
-	)
+	git --git-dir=foo.git show -s HEAD
 '
 
 test_expect_success SYMLINKS 'detection should not be fooled by a symlink' '

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), add an optional 6th parameter repo_flag (defaulting to -C) to the test_repo_info helper, and use it in the caller that wants to operate on a bare repository.

---
 t/t1900-repo-info.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

index 39bb77dda0..6280da1efb 100755
@@ -20,6 +20,7 @@ test_repo_info () {
 	repo_name=$3
 	key=$4
 	expected_value=$5
+	repo_flag=${6:--C}
 
 	test_expect_success "setup: $label" '
 		eval "$init_command $repo_name"

@@ -27,13 +28,13 @@ test_repo_info () {
 
 	test_expect_success "lines: $label" '
 		echo "$key=$expected_value" > expect &&
-		git -C "$repo_name" repo info "$key" >actual &&
+		git $repo_flag "$repo_name" repo info "$key" >actual &&
 		test_cmp expect actual
 	'
 
 	test_expect_success "nul: $label" '
 		printf "%s\n%s\0" "$key" "$expected_value" >expect &&
-		git -C "$repo_name" repo info --format=nul "$key" >actual &&
+		git $repo_flag "$repo_name" repo info --format=nul "$key" >actual &&
 		test_cmp_bin expect actual
 	'
 }

@@ -48,7 +49,7 @@ test_repo_info 'bare repository = false is retrieved correctly' \
 	'git init' 'nonbare' 'layout.bare' 'false'
 
 test_repo_info 'bare repository = true is retrieved correctly' \
-	'git init --bare' 'bare' 'layout.bare' 'true'
+	'git init --bare' 'bare' 'layout.bare' 'true' '--git-dir'
 
 test_repo_info 'shallow repository = false is retrieved correctly' \
 	'git init' 'nonshallow' 'layout.shallow' 'false'

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), specify the gitdir specifically in bare-repo git worktree add invocations via --git-dir=. so Git does not rely on implicit bare repository discovery.

While at it, also avoid unnecessary subshells and cding. This simplifies the logic in a rather pleasant way.

---
 t/t2400-worktree-add.sh | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

index 023e1301c8..0f8c837647 100755
@@ -171,11 +171,8 @@ test_expect_success 'not die on re-checking out current branch' '
 '
 
 test_expect_success '"add" from a bare repo' '
-	(
-		git clone --bare . bare &&
-		cd bare &&
-		git worktree add -b bare-main ../there2 main
-	)
+	git clone --bare . bare &&
+	git -C bare --git-dir=. worktree add -b bare-main ../there2 main
 '
 
 test_expect_success 'checkout from a bare repo without "add"' '

@@ -186,15 +183,11 @@ test_expect_success 'checkout from a bare repo without "add"' '
 '
 
 test_expect_success '"add" default branch of a bare repo' '
-	(
-		git clone --bare . bare2 &&
-		cd bare2 &&
-		git worktree add ../there3 main &&
-		cd ../there3 &&
-		# Simple check that a Git command does not
-		# immediately fail with the current setup
-		git status
-	) &&
+	git clone --bare . bare2 &&
+	git -C bare2 --git-dir=. worktree add ../there3 main &&
+	# Simple check that a Git command does not
+	# immediately fail with the current setup
+	git status &&
 	cat >expect <<-EOF &&
 	init.t
 	EOF

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), the test case t2406.10(repair .git file from bare.git) cannot rely on the implicit discovery of thee bare repository. Simply add a --git-dir=. to the invocation. The -C bare.git argument is still needed so that the repair command realizes works on the intended directory.

---
 t/t2406-worktree-repair.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index f5f19b3169..cac448b575 100755
@@ -84,7 +84,7 @@ test_expect_success 'repair .git file from bare.git' '
 	git -C bare.git worktree add --detach ../corrupt &&
 	git -C corrupt rev-parse --absolute-git-dir >expect &&
 	rm -f corrupt/.git &&
-	git -C bare.git worktree repair &&
+	git -C bare.git --git-dir=. worktree repair &&
 	git -C corrupt rev-parse --absolute-git-dir >actual &&
 	test_cmp expect actual
 '

gitgitgadget

From: Johannes Schindelin <[email protected]>

The test case "fetch specific OID with tag following" creates a bare repository and wants to operate on it by changing the working directory and relying on Git's implicit discovery of the bare repository.

Once the safe.bareRepository default is changed, this is no longer an option.

So let's adjust the commands to specify the bare repository explicitly, via --git-dir, and avoid changing the working directory. As a bonus, the result is arguably more readable than the original code.

---
 t/t5503-tagfollow.sh | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

index febe441041..6d178d84dd 100755
@@ -168,16 +168,13 @@ test_expect_success 'new clone fetch main and tags' '
 
 test_expect_success 'fetch specific OID with tag following' '
 	git init --bare clone3.git &&
-	(
-		cd clone3.git &&
-		git remote add origin .. &&
-		git fetch origin $B:refs/heads/main &&
+	git --git-dir=clone3.git remote add origin "$PWD" &&
+	git --git-dir=clone3.git fetch origin $B:refs/heads/main &&
 
-		git -C .. for-each-ref >expect &&
-		git for-each-ref >actual &&
+	git for-each-ref >expect &&
+	git --git-dir=clone3.git for-each-ref >actual &&
 
-		test_cmp expect actual
-	)
+	test_cmp expect actual
 '
 
 test_done

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), export GIT_DIR=. right after git init --bare && so subsequent commands access the bare repo explicitly.

---
 t/t5505-remote.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index e592c0bcde..6d3d8510ca 100755
@@ -561,7 +561,7 @@ test_expect_success 'add --mirror && prune' '
 	mkdir mirror &&
 	(
 		cd mirror &&
-		git init --bare &&
+		git init --bare && GIT_DIR=. && export GIT_DIR &&
 		git remote add --mirror -f origin ../one
 	) &&
 	(

@@ -583,7 +583,7 @@ test_expect_success 'add --mirror setting HEAD' '
 	mkdir headmirror &&
 	(
 		cd headmirror &&
-		git init --bare -b notmain &&
+		git init --bare -b notmain && GIT_DIR=. && export GIT_DIR &&
 		git remote add --mirror -f origin ../one &&
 		test "$(git symbolic-ref HEAD)" = "refs/heads/main"
 	)

gitgitgadget

From: Johannes Schindelin <[email protected]>

After switching from -C pushee to --git-dir=pushee as part of the safe.bareRepository preparation, ext:: URLs that used . (resolved relative to the -C target) must spell out the directory name explicitly.

---
 t/t5509-fetch-push-namespaces.sh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

index 095df1a753..7771a3b34a 100755
@@ -88,8 +88,8 @@ test_expect_success 'mirroring a repository using a ref namespace' '
 
 test_expect_success 'hide namespaced refs with transfer.hideRefs' '
 	GIT_NAMESPACE=namespace \
-		git -C pushee -c transfer.hideRefs=refs/tags \
-		ls-remote "ext::git %s ." >actual &&
+		git --git-dir=pushee -c transfer.hideRefs=refs/tags \
+		ls-remote "ext::git %s pushee" >actual &&
 	printf "$commit1\trefs/heads/main\n" >expected &&
 	test_cmp expected actual
 '

@@ -97,8 +97,8 @@ test_expect_success 'hide namespaced refs with transfer.hideRefs' '
 test_expect_success 'check that transfer.hideRefs does not match unstripped refs' '
 	git -C pushee pack-refs --all &&
 	GIT_NAMESPACE=namespace \
-		git -C pushee -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
-		ls-remote "ext::git %s ." >actual &&
+		git --git-dir=pushee -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
+		ls-remote "ext::git %s pushee" >actual &&
 	printf "$commit1\trefs/heads/main\n" >expected &&
 	printf "$commit0\trefs/tags/0\n" >>expected &&
 	printf "$commit1\trefs/tags/1\n" >>expected &&

@@ -107,8 +107,8 @@ test_expect_success 'check that transfer.hideRefs does not match unstripped refs
 
 test_expect_success 'hide full refs with transfer.hideRefs' '
 	GIT_NAMESPACE=namespace \
-		git -C pushee -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
-		ls-remote "ext::git %s ." >actual &&
+		git --git-dir=pushee -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
+		ls-remote "ext::git %s pushee" >actual &&
 	printf "$commit1\trefs/heads/main\n" >expected &&
 	test_cmp expected actual
 '

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

	(
		cd pushee &&
		git --git-dir=. -c ... ls-remote ...
	)

Hi Junio,

It would be indeed more conservative, and it even results in less changes when done in a more elegant fashion, by appending --git-dir=. after the -C pushee, as the patches "t2400: explicitly specify bare repo for git worktree add" and "t2406: use --git-dir=. for bare repository worktree repair" already do. That will not only result in vastly less changed lines (and hence less cognitive load on any reviewer), but also avoid the proposed subshell. I'll go with --git-dir=., then.

Ciao,
Johannes

Ah, yes, with or without -C shouldn't affect the decision to use "--git-dir=$path" in the first place, and the ones that need the change to explicitly say "--git-dir=$path" in this series are all the ones that wants to be at the level where HEAD and refs/ exists and access the repository contents (as if it were a bare repository even when it is a part of a repository with a worktree, or it may be going there into a real bare repository), by definition what we need to add is "--git-dir=.", no arbitrary $path adjusted for each case is needed.

Makes sense.

Assuming that we want to tighten the rule and prepare for the tightening before it happens, that is. I personally do not think it is a bad move, but I do not recall we had much discussion to gain a community consensus to go in that direction.

Hi Junio,

Well, I wanted to contribute a patch to that extent, as a discussion starter. And then the CI build broke. And then I found myself almost overwhelmed with the changes required to let the CI build pass. And then I thought I'd go through the effort of making those changes just to gauge how much of a problem changing the default would be for affected parties. And then I split up the preparatory patches into multiple sub-branches so that reviewers wouldn't get overwhelmed. And you're looking at the first sub-branch broken out from that PR.

Note that this here patch series, while it would be a necessary prerequisite for changing the safe.bareRepository default in Git 3.0, does have merit on its own, and I do not intend these patches to make such a change of default behavior more or less likely in Git 3.0.

By introducing that setting, we declared that while not necessary at the moment, it is better, really, to specify the location of bare repositories explcitly than to rely on the implicit discovery. And this patch series addresses a couple of places where Git's own test suite does not follow our own advice.

For that reason I consider this patch series strictly a spring cleaning.

Ciao,
Johannes

From: Johannes Schindelin <[email protected]>

In the test_http_push_nonff function both of these test scripts call, there were two Git invocations that assume that bare repositories will always be discovered when the current working directory is inside one. This is unlikely to be true forever because at some stage, the safe.bareRepository config is prone to be modified to be safe by default.

So let's be safe and specify the bare repository explicitly.

---
 t/lib-httpd.sh | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

index 4c76e813e3..f15158b2c5 100644
@@ -259,7 +259,7 @@ test_http_push_nonff () {
 
 	test_expect_success 'non-fast-forward push fails' '
 		cd "$REMOTE_REPO" &&
-		HEAD=$(git rev-parse --verify HEAD) &&
+		HEAD=$(git --git-dir=. rev-parse --verify HEAD) &&
 
 		cd "$LOCAL_REPO" &&
 		git checkout $BRANCH &&

@@ -270,7 +270,7 @@ test_http_push_nonff () {
 		(
 			cd "$REMOTE_REPO" &&
 			echo "$HEAD" >expect &&
-			git rev-parse --verify HEAD >actual &&
+			git --git-dir=. rev-parse --verify HEAD >actual &&
 			test_cmp expect actual
 		)
 	'

@@ -284,18 +284,16 @@ test_http_push_nonff () {
 	'
 
 	test_expect_${EXPECT_CAS_RESULT} 'force with lease aka cas' '
-		HEAD=$(	cd "$REMOTE_REPO" && git rev-parse --verify HEAD ) &&
+		HEAD=$(git --git-dir="$REMOTE_REPO" rev-parse --verify HEAD) &&
 		test_when_finished '\''
-			(cd "$REMOTE_REPO" && git update-ref HEAD "$HEAD")
+			git --git-dir="$REMOTE_REPO" update-ref HEAD "$HEAD"
 		'\'' &&
 		(
 			cd "$LOCAL_REPO" &&
 			git push -v --force-with-lease=$BRANCH:$HEAD origin
 		) &&
 		git rev-parse --verify "$BRANCH" >expect &&
-		(
-			cd "$REMOTE_REPO" && git rev-parse --verify HEAD
-		) >actual &&
+		git --git-dir="$REMOTE_REPO" rev-parse --verify HEAD >actual &&
 		test_cmp expect actual
 	'
 }

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), wrap the test_commit_bulk call in (GIT_DIR="$REPO" && export GIT_DIR && test_commit_bulk ...) because test_commit_bulk -C relies on implicit discovery which would fail once the default changes.

---
 t/t5619-clone-local-ambiguous-transport.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index cce62bf78d..3e9aac9015 100755
@@ -21,7 +21,7 @@ test_expect_success 'setup' '
 	echo "secret" >sensitive/secret &&
 
 	git init --bare "$REPO" &&
-	test_commit_bulk -C "$REPO" --ref=main 1 &&
+	(GIT_DIR="$REPO" && export GIT_DIR && test_commit_bulk --ref=main 1) &&
 
 	git -C "$REPO" update-ref HEAD main &&
 	git -C "$REPO" update-server-info &&

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), adjust a loop that iterated over both a bare (cloned) and a non-bare (unbundled) repository using the same -C flag: the bare repo needs --git-dir to avoid implicit discovery, while the non-bare one keeps -C.

---
 t/t6020-bundle-misc.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 500c81b8a1..82df105b47 100755
@@ -594,9 +594,9 @@ do
 		reflist=$(git for-each-ref --format="%(objectname)") &&
 		git rev-list --objects --filter=$filter --missing=allow-any \
 			$reflist >expect &&
-		for repo in cloned unbundled
+		for opt in "--git-dir cloned" "-C unbundled"
 		do
-			git -C $repo rev-list --objects --missing=allow-any \
+			git $opt rev-list --objects --missing=allow-any \
 				$reflist >actual &&
 			test_cmp expect actual || return 1
 		done

gitgitgadget

From: Johannes Schindelin <[email protected]>

This test expects scalar register to discover a bare repo and reject it. Since scalar does not support --git-dir (that option would not make sense in the context of that command), pass -c safe.bareRepository=all to opt into implicit discovery of bare repositories, so the test keeps working once the default changes to explicit.

---
 t/t9210-scalar.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 009437a5f3..54513c220b 100755
@@ -88,7 +88,7 @@ test_expect_success 'scalar enlistments need a worktree' '
 	test_when_finished rm -rf bare test &&
 
 	git init --bare bare/src &&
-	! scalar register bare/src 2>err &&
+	! scalar -c safe.bareRepository=all register bare/src 2>err &&
 	grep "Scalar enlistments require a worktree" err &&
 
 	git init test/src &&

gitgitgadget

From: Johannes Schindelin <[email protected]>

Currently, the "alternate bare repo" test case relies on Git discovering non-bare and bare repositories alike. However, the automatic discovery of bare repository represents a weakness that leaves Git users vulnerable. To that end, the safe.bareRepository config was introduced, but out of backwards-compatibility concerns, the default is not yet secure.

To prepare for that default to switch to the secure one, where bare repositories are never discovered automatically but instead must be specified explicitly, let's do exactly that in this test case: specify it explicitly, via setting the environment variable GIT_DIR.

---
 t/t9700/test.pl | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index f83e6169e2..99b712b626 100755
@@ -153,9 +153,12 @@ unlink $tmpfile3;
 chdir($abs_repo_dir);
 
 # open alternate bare repo
-my $r4 = Git->repository(Directory => "$abs_repo_dir/bare.git");
-is($r4->command_oneline(qw(log --format=%s)), "bare commit",
-	"log of bare repo works");
+{
+	local $ENV{GIT_DIR} = "$abs_repo_dir/bare.git";
+	my $r4 = Git->repository(Directory => "$abs_repo_dir/bare.git");
+	is($r4->command_oneline(qw(log --format=%s)), "bare commit",
+		"log of bare repo works");
+}
 
 # unquoting paths
 is(Git::unquote_path('abc'), 'abc', 'unquote unquoted path');

gitgitgadget

From: Johannes Schindelin <[email protected]>

When safe.bareRepository will change to be safe by default, bare repositories won't be discovered by default anymore. To prepare for this, git p4 must be explicit about the gitdir when cloning into a bare repository, and no longer rely on that implicit discovery.

---
 git-p4.py | 1 +
 1 file changed, 1 insertion(+)

index c0ca7becaf..dd38dbca22 100755
@@ -4360,6 +4360,7 @@ class P4Clone(P4Sync):
         init_cmd = ["git", "init"]
         if self.cloneBare:
             init_cmd.append("--bare")
+            os.environ["GIT_DIR"] = os.getcwd()
         retcode = subprocess.call(init_cmd)
         if retcode:
             raise subprocess.CalledProcessError(retcode, init_cmd)

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

I do not recall such a change for safe.bareRepository discussed in the recent past, and I do not have a strong opinion yet because of that, but if no such change to require "explicit" comes, we would be losing test coverage with these patches, because these rewrite the ones that rely on a working code in implicit cases?

Shoudln't there be a patch [01/18] before everything else that updates Documentation/BreakingChanges.adoc to propose the default change?

I've scanned the patches and assuming that there is no implicit access to bare repository allowed, the strategies taken in them ...

... which are summarized nicely above, all make sense.

Thanks.

Hi Junio,

Note that the commit message says "may well" not "will". Precisely because such a discussion has not been had in the recent past.

But yes, that discussion has not happened, and I was quite surprised about that when I recently looked (I prepared some of my code for such a change, assuming that Git 3.0 as a natural inflection point would be the time for 8d1a7448206e's long journey to come to a conclusion).

Well, yes and no. There should be an update to BreakingChanges to propose that default change, but obviously it should not only be a documentation
change: It should also adjust Documentation/config/safe.adoc to mention the intended change of behavior, and it should introduce WITH_BREAKING_HANGES-specific conditional code in setup.c.

And, crucially, it should pass the test suite under WITH_BREAKING_CHANGES.

To do that, a lot more needs to happen than just the 17 patches in this here patch series. Most of the additional changes are quite mechanical, and can be validated relatively easily despite their sheer number. And only at the very end of those changes can that safe.bareRepository change even be proposed. The eventual patch that does what you ask (but not as 1/18, for the reasons I outlined above), and more, is:
https://github.com/gitgitgadget/git/pull/2072/changes/435e3505e7997a25f45777a6ba436899a793c59c

I plan on proposing that patch in due time, to start the discussion whether or not it is a good idea to change the default of safe.bareRepository in Git 3.0.

Ciao,
Johannes

Ah, very true.

And of course we'd need some tests conditional to breaking-changes prerequisite in the meantime, to make sure that the default is to allow "all" before breaking changes, and with the prereq the test checks the new behaviour is to deny by default.

As to the changes to existing tests (i.e., this patch series started doing), they have a bit of balancing act.

* Most importantly, they need to make sure their indivial operations, when working on a bare repository is allowed in any unspecified means, continue to behave sensibly. "git log -p" in a bare repository should still produce patches, "git bisect" in a bare repository should work and assume --no-checkout, etc.

* Then all of them would need to be somehow told that working on a bare repository is allowed in suitable way. For most of then it should be done by setting safe.bareRepository in the global scope, just like many tests specify the default branch name upfront just once (this is a tangent, but we should find a way to get rid of GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME variable support in our binary and instead set it up in the test environment's ~/.gitconfig), some may add --git-dir=., etc. But the primary focus with this change is to preserve what individual operation each test wants to validate (see above).

* There may need to be some tests behind the WITH_BREAKING_CHANGES prerequisite that makes sure that the access to the repository is blocked without the configuration loosening the condition, but they hopefully would be minimum---unlike "how this particular command should operate in a bare repository?" that vary per command, "no command should work in a bare repository unless ..." that is enforced in the setup phase do not have to validate each and every command.

I agree with all the flow you propose here. And for that process, I think the early parts of the effort (i.e., there ~20 patches) that prepare existing tests that make sure the operations of individual commands in bare repositories should be kept to bare minimum by freezing the world to allow bare repositories, just like initial branch names are frozen to either 'master' or 'main' in many test scripts. Adding "--git-dir=." (regardless of the "-C there") you proposed in the other thread is a valid way (but it may be tedious to do and verify); doing "git config --global safe.bareRepository all" upfront, if it works, might be simpler. Any approach that would achieve what we want is fine.

Yup, I do not think it is a bad thing in the longer term, even though I suspect it might be a bit more disruptive than others we have in the breaking changes document.

Thanks for working on this.

The safe.bareRepository configuration variable (introduced in 8d1a7448206e) allows restricting implicit bare repository discovery. Its default may well change to "explicit" in Git v3.0, at which point any test that relies on implicit discovery of a bare repository would break, even if the test subject has nothing to do with bare repositories.

This series adjusts 16 test scripts and git-p4 so that they access bare repositories explicitly. The techniques used are:

* Replace git -C <bare-repo> ... with git --git-dir=<bare-repo> ...
* Export GIT_DIR=. after cd-ing into a bare repository
* Wrap commands in (GIT_DIR=<path> && export GIT_DIR && ...)
* Add test_config_global safe.bareRepository all in the few tests where implicit discovery is genuinely part of what is being tested

Each commit is a self-contained fix to one test file (or a small related group).

This patch series is part of https://github.com/gitgitgadget/git/pull/2072.

Changes since v1:

* Dramatically simplified the patch "t5509: specify bare repository path explicitly"

Johannes Schindelin (17):
t0001: allow implicit bare repo discovery for aliased-command test
t0001: replace cd+git with git --git-dir in check_config
t0003: use --git-dir for bare repo attribute tests
t0056: allow implicit bare repo discovery for -C work-tree tests
t1020: use --git-dir instead of subshell for bare repo
t1900: avoid using -C <dir> for a bare repository
t2400: explicitly specify bare repo for git worktree add
t2406: use --git-dir=. for bare repository worktree repair
t5503: avoid discovering a bare repository
t5505: export GIT_DIR after git init --bare
t5509: specify bare repository path explicitly t5540/t5541: avoid accessing a bare repository via -C <dir>
t5619: wrap test_commit_bulk in GIT_DIR subshell for bare repo
t6020: use -C for worktree, --git-dir for bare repository
t9210: pass safe.bareRepository=all to scalar register
t9700: stop relying on implicit bare repo discovery
git p4 clone --bare: need to be explicit about the gitdir

 git-p4.py                                  |  1 +
 t/lib-httpd.sh                             | 12 ++--
 t/t0001-init.sh                            |  5 +-
 t/t0003-attributes.sh                      | 66 +++++++++-------------
 t/t0056-git-C.sh                           |  2 +
 t/t1020-subdirectory.sh                    |  5 +-
 t/t1900-repo-info.sh                       |  7 ++-
 t/t2400-worktree-add.sh                    | 21 +++----
 t/t2406-worktree-repair.sh                 |  2 +-
 t/t5503-tagfollow.sh                       | 13 ++---
 t/t5505-remote.sh                          |  4 +-
 t/t5509-fetch-push-namespaces.sh           |  6 +-
 t/t5619-clone-local-ambiguous-transport.sh |  2 +-
 t/t6020-bundle-misc.sh                     |  4 +-
 t/t9210-scalar.sh                          |  2 +-
 t/t9700/test.pl                            |  9 ++-
 16 files changed, 71 insertions(+), 90 deletions(-)

base-commit: cf2139f8e1680b076e115bc0b349e369b4b0ecc4
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2076%2Fdscho%2Ftests-explicit-bare-repo-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2076/dscho/tests-explicit-bare-repo-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2076

Range-diff vs v1:

  1:  a1cdbd58f0 =  1:  a1cdbd58f0 t0001: allow implicit bare repo discovery for aliased-command test
  2:  78744602fb =  2:  78744602fb t0001: replace `cd`+`git` with `git --git-dir` in `check_config`
  3:  a4f7a6df51 =  3:  a4f7a6df51 t0003: use `--git-dir` for bare repo attribute tests
  4:  5b6bb18632 =  4:  5b6bb18632 t0056: allow implicit bare repo discovery for `-C` work-tree tests
  5:  c38f0a68f1 =  5:  c38f0a68f1 t1020: use `--git-dir` instead of subshell for bare repo
  6:  a084c39273 =  6:  a084c39273 t1900: avoid using `-C <dir>` for a bare repository
  7:  6a7730cf57 =  7:  6a7730cf57 t2400: explicitly specify bare repo for `git worktree add`
  8:  2905e000c5 =  8:  2905e000c5 t2406: use `--git-dir=.` for bare repository worktree repair
  9:  9001883e15 =  9:  9001883e15 t5503: avoid discovering a bare repository
 10:  6932658411 = 10:  6932658411 t5505: export `GIT_DIR` after `git init --bare`
 11:  f6fc807af6 ! 11:  2f1e745b55 t5509: specify bare repository path explicitly

@@ Metadata
## Commit message ##
t5509: specify bare repository path explicitly

- After switching from -C pushee to --git-dir=pushee as part of
- the safe.bareRepository preparation, ext:: URLs that used .
- (resolved relative to the -C target) must spell out the directory
- name explicitly.
+ When ls-remote is told to switch the current working directory to the
+ bare repository pushee via -C pushee, as part of the
+ safe.bareRepository preparation let's append --git-dir=. to spell
+ out that this is a bare repository that does not need to be discovered
+ implictly.

@@ t/t5509-fetch-push-namespaces.sh: test_expect_success 'mirroring a repository us
test_expect_success 'hide namespaced refs with transfer.hideRefs' '
GIT_NAMESPACE=namespace \
- git -C pushee -c transfer.hideRefs=refs/tags \
-- ls-remote "ext::git %s ." >actual &&
-+ git --git-dir=pushee -c transfer.hideRefs=refs/tags \
-+ ls-remote "ext::git %s pushee" >actual &&
++ git -C pushee --git-dir=. -c transfer.hideRefs=refs/tags \
+ ls-remote "ext::git %s ." >actual &&
printf "$commit1\trefs/heads/main\n" >expected &&
test_cmp expected actual
- '
@@ t/t5509-fetch-push-namespaces.sh: test_expect_success 'hide namespaced refs with transfer.hideRefs' '
test_expect_success 'check that transfer.hideRefs does not match unstripped refs' '
git -C pushee pack-refs --all &&
GIT_NAMESPACE=namespace \
- git -C pushee -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
-- ls-remote "ext::git %s ." >actual &&
-+ git --git-dir=pushee -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
-+ ls-remote "ext::git %s pushee" >actual &&
++ git -C pushee --git-dir=. -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
+ ls-remote "ext::git %s ." >actual &&
printf "$commit1\trefs/heads/main\n" >expected &&
printf "$commit0\trefs/tags/0\n" >>expected &&
- printf "$commit1\trefs/tags/1\n" >>expected &&
@@ t/t5509-fetch-push-namespaces.sh: test_expect_success 'check that transfer.hideRefs does not match unstripped refs

test_expect_success 'hide full refs with transfer.hideRefs' '
GIT_NAMESPACE=namespace \
- git -C pushee -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
-- ls-remote "ext::git %s ." >actual &&
-+ git --git-dir=pushee -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
-+ ls-remote "ext::git %s pushee" >actual &&
++ git -C pushee --git-dir=. -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
+ ls-remote "ext::git %s ." >actual &&
printf "$commit1\trefs/heads/main\n" >expected && test_cmp expected actual
- '

 12:  5aa3f2a225 = 12:  c8789bd542 t5540/t5541: avoid accessing a bare repository via `-C <dir>`
 13:  97f22f9e87 = 13:  f09a96e55d t5619: wrap `test_commit_bulk` in `GIT_DIR` subshell for bare repo
 14:  1ae4caf155 = 14:  01ec77c908 t6020: use `-C` for worktree, `--git-dir` for bare repository
 15:  861a8e0940 = 15:  00eaefbf62 t9210: pass `safe.bareRepository=all` to `scalar register`
 16:  1f1668a6f4 = 16:  890dfd024d t9700: stop relying on implicit bare repo discovery
 17:  c8e5bef27b = 17:  139b9da946 git p4 clone --bare: need to be explicit about the gitdir

gitgitgadget

From: Johannes Schindelin <[email protected]>

8d1a7448206e (setup.c: create safe.bareRepository, 2022-07-14) introduced a setting to restrict implicit bare repository discovery, mitigating a social-engineering attack where an embedded bare repo's hooks get executed unknowingly. To allow for that default to change at some stage in the future, the tests need to be prepared.

This commit adjusts a test accordingly that runs git aliasedinit from inside a bare repo to verify that aliased commands work there. The test is about alias resolution, not bare repo discovery, so add test_config_global safe.bareRepository all to opt in explicitly.

---
 t/t0001-init.sh | 1 +
 1 file changed, 1 insertion(+)

index e4d32bb4d2..6bd0a15dac 100755
@@ -77,6 +77,7 @@ test_expect_success 'plain nested through aliased command' '
 '
 
 test_expect_success 'plain nested in bare through aliased command' '
+	test_config_global safe.bareRepository all &&
 	(
 		git init --bare bare-ancestor-aliased.git &&
 		cd bare-ancestor-aliased.git &&

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Yes, I think most of the tests we run things in a bare repository is not about bare repository discovery but how individual commands behave in a bare repository, and these commands must be kept working even after a future version of Git starts to tighten the rules.

 t/test-lib.sh | 5 +++++
 1 file changed, 5 insertions(+)

index 70fd3e9baf..fe85a0bf89 100644
@@ -1563,6 +1563,11 @@ HOME="$TRASH_DIRECTORY"
 GNUPGHOME="$HOME/gnupg-home-not-used"
 export HOME GNUPGHOME USER_HOME
 
+if test -n "$WITH_BREAKING_CHANGES"
+then
+	git config --global safe.bareRepository all
+fi
+
 # "rm -rf" existing trash directory, even if a previous run left it
 # with bad permissions.
 remove_trash_directory () {

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), replace cd <dir> && git config with git --git-dir=<dir> config so the helper does not rely on implicit bare repository discovery.

---
 t/t0001-init.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 6bd0a15dac..db2bf1001f 100755
@@ -20,8 +20,8 @@ check_config () {
 		return 1
 	fi
 
-	bare=$(cd "$1" && git config --bool core.bare)
-	worktree=$(cd "$1" && git config core.worktree) ||
+	bare=$(git --git-dir="$1" config --bool core.bare)
+	worktree=$(git --git-dir="$1" config core.worktree) ||
 	worktree=unset
 
 	test "$bare" = "$2" && test "$worktree" = "$3" || {

gitgitgadget

From: Johannes Schindelin <[email protected]>

The bare repo tests in t0003-attributes.sh currently cd into the bare repository inside subshells, relying on implicit discovery. Restructure these tests to pass --git-dir=bare.git to the attr_check and attr_check_source helpers instead. This makes the code much easier to read, and also makes bare repo access explicit, i.e. compatible with an eventual safe.bareRepository=explicit default.

---
 t/t0003-attributes.sh | 66 ++++++++++++++++++-------------------------
 1 file changed, 27 insertions(+), 39 deletions(-)

index 582e207aa1..3a34f5dbc2 100755
@@ -346,17 +346,14 @@ test_expect_success 'setup bare' '
 
 test_expect_success 'bare repository: check that .gitattribute is ignored' '
 	(
-		cd bare.git &&
-		(
-			echo "f	test=f" &&
-			echo "a/i test=a/i"
-		) >.gitattributes &&
-		attr_check f unspecified &&
-		attr_check a/f unspecified &&
-		attr_check a/c/f unspecified &&
-		attr_check a/i unspecified &&
-		attr_check subdir/a/i unspecified
-	)
+		echo "f	test=f" &&
+		echo "a/i test=a/i"
+	) >bare.git/.gitattributes &&
+	attr_check f unspecified --git-dir=bare.git &&
+	attr_check a/f unspecified --git-dir=bare.git &&
+	attr_check a/c/f unspecified --git-dir=bare.git &&
+	attr_check a/i unspecified --git-dir=bare.git &&
+	attr_check subdir/a/i unspecified --git-dir=bare.git
 '
 
 bad_attr_source_err="fatal: bad --attr-source or GIT_ATTR_SOURCE"

@@ -449,41 +446,32 @@ test_expect_success 'diff without repository with attr source' '
 '
 
 test_expect_success 'bare repository: with --source' '
-	(
-		cd bare.git &&
-		attr_check_source foo/bar/f f tag-1 &&
-		attr_check_source foo/bar/a/i n tag-1 &&
-		attr_check_source foo/bar/f unspecified tag-2 &&
-		attr_check_source foo/bar/a/i m tag-2 &&
-		attr_check_source foo/bar/g g tag-2 &&
-		attr_check_source foo/bar/g unspecified tag-1
-	)
+	attr_check_source foo/bar/f f tag-1 --git-dir=bare.git &&
+	attr_check_source foo/bar/a/i n tag-1 --git-dir=bare.git &&
+	attr_check_source foo/bar/f unspecified tag-2 --git-dir=bare.git &&
+	attr_check_source foo/bar/a/i m tag-2 --git-dir=bare.git &&
+	attr_check_source foo/bar/g g tag-2 --git-dir=bare.git &&
+	attr_check_source foo/bar/g unspecified tag-1 --git-dir=bare.git
 '
 
 test_expect_success 'bare repository: check that --cached honors index' '
-	(
-		cd bare.git &&
-		GIT_INDEX_FILE=../.git/index \
-		git check-attr --cached --stdin --all <../stdin-all |
-		sort >actual &&
-		test_cmp ../specified-all actual
-	)
+	GIT_INDEX_FILE=.git/index \
+	git --git-dir=bare.git check-attr --cached --stdin --all <stdin-all |
+	sort >actual &&
+	test_cmp specified-all actual
 '
 
 test_expect_success 'bare repository: test info/attributes' '
+	mkdir -p bare.git/info &&
 	(
-		cd bare.git &&
-		mkdir info &&
-		(
-			echo "f	test=f" &&
-			echo "a/i test=a/i"
-		) >info/attributes &&
-		attr_check f f &&
-		attr_check a/f f &&
-		attr_check a/c/f f &&
-		attr_check a/i a/i &&
-		attr_check subdir/a/i unspecified
-	)
+		echo "f	test=f" &&
+		echo "a/i test=a/i"
+	) >bare.git/info/attributes &&
+	attr_check f f --git-dir=bare.git &&
+	attr_check a/f f --git-dir=bare.git &&
+	attr_check a/c/f f --git-dir=bare.git &&
+	attr_check a/i a/i --git-dir=bare.git &&
+	attr_check subdir/a/i unspecified --git-dir=bare.git
 '
 
 test_expect_success 'binary macro expanded by -a' '

gitgitgadget

From: Johannes Schindelin <[email protected]>

The git -C c/a.git --work-tree=../a invocations in t0056-git-C.sh enter what is technically the .git directory of a repository to test -C combined with --work-tree. In doing so, the code relies on implicit discovery of bare repositories, which 8d1a7448206e (setup.c:
create safe.bareRepository, 2022-07-14) prepared to be prevented by default.

These tests verify the interaction between those flags, so changing them to use --git-dir would defeat their purpose. So let's just temporarily force-enable implicit discovery of bare repositories, no matter what safe.bareRepository defaults to.

---
 t/t0056-git-C.sh | 2 ++
 1 file changed, 2 insertions(+)

index 2630e756da..6b7122add5 100755
@@ -57,11 +57,13 @@ test_expect_success 'Order should not matter: "--git-dir=a.git -C c" is equivale
 test_expect_success 'Effect on --work-tree option: "-C c/a.git --work-tree=../a"  is equivalent to "--work-tree=c/a --git-dir=c/a.git"' '
 	rm c/a/a.txt &&
 	git --git-dir=c/a.git --work-tree=c/a status >expected &&
+	test_config_global safe.bareRepository all &&
 	git -C c/a.git --work-tree=../a status >actual &&
 	test_cmp expected actual
 '
 
 test_expect_success 'Order should not matter: "--work-tree=../a -C c/a.git" is equivalent to "-C c/a.git --work-tree=../a"' '
+	test_config_global safe.bareRepository all &&
 	git -C c/a.git --work-tree=../a status >expected &&
 	git --work-tree=../a -C c/a.git status >actual &&
 	test_cmp expected actual

gitgitgadget

From: Johannes Schindelin <[email protected]>

Replace an unnecessarily complex subshell pattern with a much simpler --git-dir-based one. The latter is not only simpler, it also no longer relies on implicit bare repo discovery, which would fail with safe.bareRepository=explicit.

---
 t/t1020-subdirectory.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

index 9fdbb2af80..20d2d306fe 100755
@@ -177,10 +177,7 @@ test_expect_success 'no file/rev ambiguity check inside a bare repo (explicit GI
 test_expect_success 'no file/rev ambiguity check inside a bare repo' '
 	test_when_finished "rm -fr foo.git" &&
 	git clone -s --bare .git foo.git &&
-	(
-		cd foo.git &&
-		git show -s HEAD
-	)
+	git --git-dir=foo.git show -s HEAD
 '
 
 test_expect_success SYMLINKS 'detection should not be fooled by a symlink' '

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), add an optional 6th parameter repo_flag (defaulting to -C) to the test_repo_info helper, and use it in the caller that wants to operate on a bare repository.

---
 t/t1900-repo-info.sh | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

index 39bb77dda0..6280da1efb 100755
@@ -20,6 +20,7 @@ test_repo_info () {
 	repo_name=$3
 	key=$4
 	expected_value=$5
+	repo_flag=${6:--C}
 
 	test_expect_success "setup: $label" '
 		eval "$init_command $repo_name"

@@ -27,13 +28,13 @@ test_repo_info () {
 
 	test_expect_success "lines: $label" '
 		echo "$key=$expected_value" > expect &&
-		git -C "$repo_name" repo info "$key" >actual &&
+		git $repo_flag "$repo_name" repo info "$key" >actual &&
 		test_cmp expect actual
 	'
 
 	test_expect_success "nul: $label" '
 		printf "%s\n%s\0" "$key" "$expected_value" >expect &&
-		git -C "$repo_name" repo info --format=nul "$key" >actual &&
+		git $repo_flag "$repo_name" repo info --format=nul "$key" >actual &&
 		test_cmp_bin expect actual
 	'
 }

@@ -48,7 +49,7 @@ test_repo_info 'bare repository = false is retrieved correctly' \
 	'git init' 'nonbare' 'layout.bare' 'false'
 
 test_repo_info 'bare repository = true is retrieved correctly' \
-	'git init --bare' 'bare' 'layout.bare' 'true'
+	'git init --bare' 'bare' 'layout.bare' 'true' '--git-dir'
 
 test_repo_info 'shallow repository = false is retrieved correctly' \
 	'git init' 'nonshallow' 'layout.shallow' 'false'

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), specify the gitdir specifically in bare-repo git worktree add invocations via --git-dir=. so Git does not rely on implicit bare repository discovery.

While at it, also avoid unnecessary subshells and cding. This simplifies the logic in a rather pleasant way.

---
 t/t2400-worktree-add.sh | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

index 023e1301c8..0f8c837647 100755
@@ -171,11 +171,8 @@ test_expect_success 'not die on re-checking out current branch' '
 '
 
 test_expect_success '"add" from a bare repo' '
-	(
-		git clone --bare . bare &&
-		cd bare &&
-		git worktree add -b bare-main ../there2 main
-	)
+	git clone --bare . bare &&
+	git -C bare --git-dir=. worktree add -b bare-main ../there2 main
 '
 
 test_expect_success 'checkout from a bare repo without "add"' '

@@ -186,15 +183,11 @@ test_expect_success 'checkout from a bare repo without "add"' '
 '
 
 test_expect_success '"add" default branch of a bare repo' '
-	(
-		git clone --bare . bare2 &&
-		cd bare2 &&
-		git worktree add ../there3 main &&
-		cd ../there3 &&
-		# Simple check that a Git command does not
-		# immediately fail with the current setup
-		git status
-	) &&
+	git clone --bare . bare2 &&
+	git -C bare2 --git-dir=. worktree add ../there3 main &&
+	# Simple check that a Git command does not
+	# immediately fail with the current setup
+	git status &&
 	cat >expect <<-EOF &&
 	init.t
 	EOF

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), the test case t2406.10(repair .git file from bare.git) cannot rely on the implicit discovery of thee bare repository. Simply add a --git-dir=. to the invocation. The -C bare.git argument is still needed so that the repair command realizes works on the intended directory.

---
 t/t2406-worktree-repair.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index f5f19b3169..cac448b575 100755
@@ -84,7 +84,7 @@ test_expect_success 'repair .git file from bare.git' '
 	git -C bare.git worktree add --detach ../corrupt &&
 	git -C corrupt rev-parse --absolute-git-dir >expect &&
 	rm -f corrupt/.git &&
-	git -C bare.git worktree repair &&
+	git -C bare.git --git-dir=. worktree repair &&
 	git -C corrupt rev-parse --absolute-git-dir >actual &&
 	test_cmp expect actual
 '

gitgitgadget

From: Johannes Schindelin <[email protected]>

The test case "fetch specific OID with tag following" creates a bare repository and wants to operate on it by changing the working directory and relying on Git's implicit discovery of the bare repository.

Once the safe.bareRepository default is changed, this is no longer an option.

So let's adjust the commands to specify the bare repository explicitly, via --git-dir, and avoid changing the working directory. As a bonus, the result is arguably more readable than the original code.

---
 t/t5503-tagfollow.sh | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

index febe441041..6d178d84dd 100755
@@ -168,16 +168,13 @@ test_expect_success 'new clone fetch main and tags' '
 
 test_expect_success 'fetch specific OID with tag following' '
 	git init --bare clone3.git &&
-	(
-		cd clone3.git &&
-		git remote add origin .. &&
-		git fetch origin $B:refs/heads/main &&
+	git --git-dir=clone3.git remote add origin "$PWD" &&
+	git --git-dir=clone3.git fetch origin $B:refs/heads/main &&
 
-		git -C .. for-each-ref >expect &&
-		git for-each-ref >actual &&
+	git for-each-ref >expect &&
+	git --git-dir=clone3.git for-each-ref >actual &&
 
-		test_cmp expect actual
-	)
+	test_cmp expect actual
 '
 
 test_done

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), export GIT_DIR=. right after git init --bare && so subsequent commands access the bare repo explicitly.

---
 t/t5505-remote.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index e592c0bcde..6d3d8510ca 100755
@@ -561,7 +561,7 @@ test_expect_success 'add --mirror && prune' '
 	mkdir mirror &&
 	(
 		cd mirror &&
-		git init --bare &&
+		git init --bare && GIT_DIR=. && export GIT_DIR &&
 		git remote add --mirror -f origin ../one
 	) &&
 	(

@@ -583,7 +583,7 @@ test_expect_success 'add --mirror setting HEAD' '
 	mkdir headmirror &&
 	(
 		cd headmirror &&
-		git init --bare -b notmain &&
+		git init --bare -b notmain && GIT_DIR=. && export GIT_DIR &&
 		git remote add --mirror -f origin ../one &&
 		test "$(git symbolic-ref HEAD)" = "refs/heads/main"
 	)

gitgitgadget

From: Johannes Schindelin <[email protected]>

When ls-remote is told to switch the current working directory to the bare repository pushee via -C pushee, as part of the safe.bareRepository preparation let's append --git-dir=. to spell out that this is a bare repository that does not need to be discovered implictly.

---
 t/t5509-fetch-push-namespaces.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

index 095df1a753..5167c16c1f 100755
@@ -88,7 +88,7 @@ test_expect_success 'mirroring a repository using a ref namespace' '
 
 test_expect_success 'hide namespaced refs with transfer.hideRefs' '
 	GIT_NAMESPACE=namespace \
-		git -C pushee -c transfer.hideRefs=refs/tags \
+		git -C pushee --git-dir=. -c transfer.hideRefs=refs/tags \
 		ls-remote "ext::git %s ." >actual &&
 	printf "$commit1\trefs/heads/main\n" >expected &&
 	test_cmp expected actual

@@ -97,7 +97,7 @@ test_expect_success 'hide namespaced refs with transfer.hideRefs' '
 test_expect_success 'check that transfer.hideRefs does not match unstripped refs' '
 	git -C pushee pack-refs --all &&
 	GIT_NAMESPACE=namespace \
-		git -C pushee -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
+		git -C pushee --git-dir=. -c transfer.hideRefs=refs/namespaces/namespace/refs/tags \
 		ls-remote "ext::git %s ." >actual &&
 	printf "$commit1\trefs/heads/main\n" >expected &&
 	printf "$commit0\trefs/tags/0\n" >>expected &&

@@ -107,7 +107,7 @@ test_expect_success 'check that transfer.hideRefs does not match unstripped refs
 
 test_expect_success 'hide full refs with transfer.hideRefs' '
 	GIT_NAMESPACE=namespace \
-		git -C pushee -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
+		git -C pushee --git-dir=. -c transfer.hideRefs="^refs/namespaces/namespace/refs/tags" \
 		ls-remote "ext::git %s ." >actual &&
 	printf "$commit1\trefs/heads/main\n" >expected &&
 	test_cmp expected actual

gitgitgadget

From: Johannes Schindelin <[email protected]>

In the test_http_push_nonff function both of these test scripts call, there were two Git invocations that assume that bare repositories will always be discovered when the current working directory is inside one. This is unlikely to be true forever because at some stage, the safe.bareRepository config is prone to be modified to be safe by default.

So let's be safe and specify the bare repository explicitly.

---
 t/lib-httpd.sh | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

index 4c76e813e3..f15158b2c5 100644
@@ -259,7 +259,7 @@ test_http_push_nonff () {
 
 	test_expect_success 'non-fast-forward push fails' '
 		cd "$REMOTE_REPO" &&
-		HEAD=$(git rev-parse --verify HEAD) &&
+		HEAD=$(git --git-dir=. rev-parse --verify HEAD) &&
 
 		cd "$LOCAL_REPO" &&
 		git checkout $BRANCH &&

@@ -270,7 +270,7 @@ test_http_push_nonff () {
 		(
 			cd "$REMOTE_REPO" &&
 			echo "$HEAD" >expect &&
-			git rev-parse --verify HEAD >actual &&
+			git --git-dir=. rev-parse --verify HEAD >actual &&
 			test_cmp expect actual
 		)
 	'

@@ -284,18 +284,16 @@ test_http_push_nonff () {
 	'
 
 	test_expect_${EXPECT_CAS_RESULT} 'force with lease aka cas' '
-		HEAD=$(	cd "$REMOTE_REPO" && git rev-parse --verify HEAD ) &&
+		HEAD=$(git --git-dir="$REMOTE_REPO" rev-parse --verify HEAD) &&
 		test_when_finished '\''
-			(cd "$REMOTE_REPO" && git update-ref HEAD "$HEAD")
+			git --git-dir="$REMOTE_REPO" update-ref HEAD "$HEAD"
 		'\'' &&
 		(
 			cd "$LOCAL_REPO" &&
 			git push -v --force-with-lease=$BRANCH:$HEAD origin
 		) &&
 		git rev-parse --verify "$BRANCH" >expect &&
-		(
-			cd "$REMOTE_REPO" && git rev-parse --verify HEAD
-		) >actual &&
+		git --git-dir="$REMOTE_REPO" rev-parse --verify HEAD >actual &&
 		test_cmp expect actual
 	'
 }

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), wrap the test_commit_bulk call in (GIT_DIR="$REPO" && export GIT_DIR && test_commit_bulk ...) because test_commit_bulk -C relies on implicit discovery which would fail once the default changes.

---
 t/t5619-clone-local-ambiguous-transport.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index cce62bf78d..3e9aac9015 100755
@@ -21,7 +21,7 @@ test_expect_success 'setup' '
 	echo "secret" >sensitive/secret &&
 
 	git init --bare "$REPO" &&
-	test_commit_bulk -C "$REPO" --ref=main 1 &&
+	(GIT_DIR="$REPO" && export GIT_DIR && test_commit_bulk --ref=main 1) &&
 
 	git -C "$REPO" update-ref HEAD main &&
 	git -C "$REPO" update-server-info &&

gitgitgadget

From: Johannes Schindelin <[email protected]>

To prepare for safe.bareRepository defaulting to explicit (see 8d1a7448206e), adjust a loop that iterated over both a bare (cloned) and a non-bare (unbundled) repository using the same -C flag: the bare repo needs --git-dir to avoid implicit discovery, while the non-bare one keeps -C.

---
 t/t6020-bundle-misc.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 500c81b8a1..82df105b47 100755
@@ -594,9 +594,9 @@ do
 		reflist=$(git for-each-ref --format="%(objectname)") &&
 		git rev-list --objects --filter=$filter --missing=allow-any \
 			$reflist >expect &&
-		for repo in cloned unbundled
+		for opt in "--git-dir cloned" "-C unbundled"
 		do
-			git -C $repo rev-list --objects --missing=allow-any \
+			git $opt rev-list --objects --missing=allow-any \
 				$reflist >actual &&
 			test_cmp expect actual || return 1
 		done

gitgitgadget

From: Johannes Schindelin <[email protected]>

This test expects scalar register to discover a bare repo and reject it. Since scalar does not support --git-dir (that option would not make sense in the context of that command), pass -c safe.bareRepository=all to opt into implicit discovery of bare repositories, so the test keeps working once the default changes to explicit.

---
 t/t9210-scalar.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 009437a5f3..54513c220b 100755
@@ -88,7 +88,7 @@ test_expect_success 'scalar enlistments need a worktree' '
 	test_when_finished rm -rf bare test &&
 
 	git init --bare bare/src &&
-	! scalar register bare/src 2>err &&
+	! scalar -c safe.bareRepository=all register bare/src 2>err &&
 	grep "Scalar enlistments require a worktree" err &&
 
 	git init test/src &&

gitgitgadget

From: Johannes Schindelin <[email protected]>

Currently, the "alternate bare repo" test case relies on Git discovering non-bare and bare repositories alike. However, the automatic discovery of bare repository represents a weakness that leaves Git users vulnerable. To that end, the safe.bareRepository config was introduced, but out of backwards-compatibility concerns, the default is not yet secure.

To prepare for that default to switch to the secure one, where bare repositories are never discovered automatically but instead must be specified explicitly, let's do exactly that in this test case: specify it explicitly, via setting the environment variable GIT_DIR.

---
 t/t9700/test.pl | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index f83e6169e2..99b712b626 100755
@@ -153,9 +153,12 @@ unlink $tmpfile3;
 chdir($abs_repo_dir);
 
 # open alternate bare repo
-my $r4 = Git->repository(Directory => "$abs_repo_dir/bare.git");
-is($r4->command_oneline(qw(log --format=%s)), "bare commit",
-	"log of bare repo works");
+{
+	local $ENV{GIT_DIR} = "$abs_repo_dir/bare.git";
+	my $r4 = Git->repository(Directory => "$abs_repo_dir/bare.git");
+	is($r4->command_oneline(qw(log --format=%s)), "bare commit",
+		"log of bare repo works");
+}
 
 # unquoting paths
 is(Git::unquote_path('abc'), 'abc', 'unquote unquoted path');

gitgitgadget

From: Johannes Schindelin <[email protected]>

When safe.bareRepository will change to be safe by default, bare repositories won't be discovered by default anymore. To prepare for this, git p4 must be explicit about the gitdir when cloning into a bare repository, and no longer rely on that implicit discovery.

---
 git-p4.py | 1 +
 1 file changed, 1 insertion(+)

index c0ca7becaf..dd38dbca22 100755
@@ -4360,6 +4360,7 @@ class P4Clone(P4Sync):
         init_cmd = ["git", "init"]
         if self.cloneBare:
             init_cmd.append("--bare")
+            os.environ["GIT_DIR"] = os.getcwd()
         retcode = subprocess.call(init_cmd)
         if retcode:
             raise subprocess.CalledProcessError(retcode, init_cmd)

gitgitgadget