From 2177537349d8730c85b151d31d3a5fc914681e52 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 7 Jan 2026 00:08:30 +0100 Subject: [PATCH 1/3] tests: use only valid file:// URLs on Windows On Windows, file:// URLs need to have Windows paths. In Git's test suite, however, by virtue of being implemented in shell and using the Unix paths that are so conveniently converted to Windows behind the scenes by the POSIX emulation layer required to run the shell, quite often file:// URLs are used that use _Unix_ paths. Now, `git upload-pack` is a Windows program in Git for Windows, and has not the first idea what do do about Unix paths. The only reason why those incorrect file:// URLs still "work" in the test suite is that Git insists on calling `git upload-pack` through a shell, even if the complexity of the command-line `git-upload-pack ` does not warrant the penalty. And that shell, again by virtue of using that POSIX emulation layer, automagically converts that incorrect Unix path to a Windows one. However, it is incorrect to use file:// URLs with incorrect paths. Therefore, let's just fix those URLs. Signed-off-by: Johannes Schindelin --- t/lib-bundle-uri-protocol.sh | 2 +- t/t1900-repo.sh | 2 +- t/t3421-rebase-topology-linear.sh | 2 +- t/t5150-request-pull.sh | 2 +- t/t5318-commit-graph.sh | 2 +- t/t5510-fetch.sh | 4 ++-- t/t5527-fetch-odd-refs.sh | 2 +- t/t5537-fetch-shallow.sh | 2 +- t/t5553-set-upstream.sh | 6 +++--- t/t5601-clone.sh | 6 +++--- t/t5610-clone-detached.sh | 8 ++++---- t/t5810-proto-disable-local.sh | 2 +- t/t7406-submodule-update.sh | 2 +- t/test-lib.sh | 2 ++ 14 files changed, 23 insertions(+), 21 deletions(-) diff --git a/t/lib-bundle-uri-protocol.sh b/t/lib-bundle-uri-protocol.sh index de09b6b02e2485..d92eb345e80b92 100644 --- a/t/lib-bundle-uri-protocol.sh +++ b/t/lib-bundle-uri-protocol.sh @@ -9,7 +9,7 @@ BUNDLE_URI_TEST_BUNDLE_URI= case "$BUNDLE_URI_PROTOCOL" in file) BUNDLE_URI_PARENT=file_parent - BUNDLE_URI_REPO_URI="file://$PWD/file_parent" + BUNDLE_URI_REPO_URI="file://$(pwd)/file_parent" BUNDLE_URI_BUNDLE_URI="$BUNDLE_URI_REPO_URI/fake.bdl" test_set_prereq BUNDLE_URI_FILE ;; diff --git a/t/t1900-repo.sh b/t/t1900-repo.sh index 51d55f11a5ed66..0e132ffb3e1b0e 100755 --- a/t/t1900-repo.sh +++ b/t/t1900-repo.sh @@ -70,7 +70,7 @@ test_expect_success 'setup remote' ' ' test_repo_info 'shallow repository = true is retrieved correctly' \ - 'git clone --depth 1 "file://$PWD/remote"' 'shallow' 'layout.shallow' 'true' + 'git clone --depth 1 "file://$(pwd)/remote"' 'shallow' 'layout.shallow' 'true' test_repo_info 'object.format = sha1 is retrieved correctly' \ 'git init --object-format=sha1' 'sha1' 'object.format' 'sha1' diff --git a/t/t3421-rebase-topology-linear.sh b/t/t3421-rebase-topology-linear.sh index f5b7807abd089d..4c2ed7ef628d7e 100755 --- a/t/t3421-rebase-topology-linear.sh +++ b/t/t3421-rebase-topology-linear.sh @@ -37,7 +37,7 @@ test_expect_success 'setup branches and remote tracking' ' do git branch branch-$tag $tag || return 1 done && - git remote add origin "file://$PWD" && + git remote add origin "file://$(pwd)" && git fetch origin ' diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh index 270ce6ea48796a..f986d6c1e0f289 100755 --- a/t/t5150-request-pull.sh +++ b/t/t5150-request-pull.sh @@ -14,7 +14,7 @@ test_expect_success 'setup' ' git clone upstream.git upstream-private && git clone downstream.git local && - trash_url="file://$TRASH_DIRECTORY" && + trash_url="$TRASH_DIRECTORY_URL" && downstream_url="$trash_url/downstream.git/" && upstream_url="$trash_url/upstream.git/" && diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 98c69109632c2d..524d7eaa991c75 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -355,7 +355,7 @@ test_expect_success 'commit grafts invalidate commit-graph' ' test_expect_success 'replace-objects invalidates commit-graph' ' test_when_finished rm -rf shallow && - git clone --depth 2 "file://$TRASH_DIRECTORY/full" shallow && + git clone --depth 2 "$TRASH_DIRECTORY_URL/full" shallow && ( cd shallow && git commit-graph write --reachable && diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index ce1c23684ece38..eaa34dc37d97a5 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -981,7 +981,7 @@ test_expect_success 'fetch.writeCommitGraph with submodules' ' git clone dups super && ( cd super && - git submodule add "file://$TRASH_DIRECTORY/three" && + git submodule add "$TRASH_DIRECTORY_URL/three" && git commit -m "add submodule" ) && git clone "super" super-clone && @@ -1314,7 +1314,7 @@ test_expect_success 'fetching with auto-gc does not lock up' ' echo "$*" && false EOF - git clone "file://$PWD" auto-gc && + git clone "file://$(pwd)" auto-gc && test_commit test2 && ( cd auto-gc && diff --git a/t/t5527-fetch-odd-refs.sh b/t/t5527-fetch-odd-refs.sh index 0de8eb5b6f8855..7b4da983c93fa3 100755 --- a/t/t5527-fetch-odd-refs.sh +++ b/t/t5527-fetch-odd-refs.sh @@ -23,7 +23,7 @@ test_expect_success 'setup repo with odd suffix ref' ' ' test_expect_success 'suffix ref is ignored during fetch' ' - git clone --bare file://"$PWD" suffix && + git clone --bare file://"$(pwd)" suffix && echo three >expect && git --git-dir=suffix log -1 --format=%s refs/heads/main >actual && test_cmp expect actual diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 6588ce62264331..448b383e4fbf8b 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -236,7 +236,7 @@ test_expect_success '.git/shallow is edited by repack' ' git -C shallow-server checkout main && git clone --depth=1 --no-tags --no-single-branch \ - "file://$PWD/shallow-server" shallow-client && + "file://$(pwd)/shallow-server" shallow-client && : now remove the branch and fetch with prune && git -C shallow-server branch -D branch && diff --git a/t/t5553-set-upstream.sh b/t/t5553-set-upstream.sh index 70e3376d31b431..3f7ad9378cc705 100755 --- a/t/t5553-set-upstream.sh +++ b/t/t5553-set-upstream.sh @@ -84,7 +84,7 @@ test_expect_success 'fetch --set-upstream ./does-not-exist fails with invalid ur test_expect_success 'fetch --set-upstream with valid URL sets upstream to URL' ' clear_config other other2 && - url="file://$PWD" && + url="file://$(pwd)" && git fetch --set-upstream "$url" && check_config main "$url" HEAD && check_config_missing other && @@ -172,7 +172,7 @@ test_expect_success 'pull --set-upstream upstream with more than one branch does test_expect_success 'pull --set-upstream with valid URL sets upstream to URL' ' clear_config main other other2 && git checkout main && - url="file://$PWD" && + url="file://$(pwd)" && git pull --set-upstream "$url" && check_config main "$url" HEAD && check_config_missing other && @@ -182,7 +182,7 @@ test_expect_success 'pull --set-upstream with valid URL sets upstream to URL' ' test_expect_success 'pull --set-upstream with valid URL and branch sets branch' ' clear_config main other other2 && git checkout main && - url="file://$PWD" && + url="file://$(pwd)" && git pull --set-upstream "$url" main && check_config main "$url" refs/heads/main && check_config_missing other && diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index d743d986c401a0..fb0842b8715730 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -249,14 +249,14 @@ test_expect_success 'clone respects global branch.autosetuprebase' ' test_expect_success 'respect url-encoding of file://' ' git init x+y && - git clone "file://$PWD/x+y" xy-url-1 && - git clone "file://$PWD/x%2By" xy-url-2 + git clone "file://$(pwd)/x+y" xy-url-1 && + git clone "file://$(pwd)/x%2By" xy-url-2 ' test_expect_success 'do not query-string-decode + in URLs' ' rm -rf x+y && git init "x y" && - test_must_fail git clone "file://$PWD/x+y" xy-no-plus + test_must_fail git clone "file://$(pwd)/x+y" xy-no-plus ' test_expect_success 'do not respect url-encoding of non-url path' ' diff --git a/t/t5610-clone-detached.sh b/t/t5610-clone-detached.sh index a7ec21eda5aabf..0f51f06f7c45d9 100755 --- a/t/t5610-clone-detached.sh +++ b/t/t5610-clone-detached.sh @@ -24,7 +24,7 @@ test_expect_success 'setup' ' test_expect_success 'clone repo (detached HEAD points to branch)' ' git checkout main^0 && - git clone "file://$PWD" detached-branch + git clone "file://$(pwd)" detached-branch ' test_expect_success 'cloned HEAD matches' ' echo three >expect && @@ -37,7 +37,7 @@ test_expect_failure 'cloned HEAD is detached' ' test_expect_success 'clone repo (detached HEAD points to tag)' ' git checkout two^0 && - git clone "file://$PWD" detached-tag + git clone "file://$(pwd)" detached-tag ' test_expect_success 'cloned HEAD matches' ' echo two >expect && @@ -50,7 +50,7 @@ test_expect_success 'cloned HEAD is detached' ' test_expect_success 'clone repo (detached HEAD points to history)' ' git checkout two^ && - git clone "file://$PWD" detached-history + git clone "file://$(pwd)" detached-history ' test_expect_success 'cloned HEAD matches' ' echo one >expect && @@ -65,7 +65,7 @@ test_expect_success 'clone repo (orphan detached HEAD)' ' git checkout main^0 && echo four >file && git commit -a -m four && - git clone "file://$PWD" detached-orphan + git clone "file://$(pwd)" detached-orphan ' test_expect_success 'cloned HEAD matches' ' echo four >expect && diff --git a/t/t5810-proto-disable-local.sh b/t/t5810-proto-disable-local.sh index 96a2c46e7a6bd5..624fa951e12360 100755 --- a/t/t5810-proto-disable-local.sh +++ b/t/t5810-proto-disable-local.sh @@ -9,7 +9,7 @@ test_expect_success 'setup repository to clone' ' test_commit one ' -test_proto "file://" file "file://$PWD" +test_proto "file://" file "file://$(pwd)" test_proto "path" file . test_expect_success 'setup repo with dash' ' diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 3adab12091a5f0..c51d692cfae614 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -1085,7 +1085,7 @@ test_expect_success 'submodule update --quiet passes quietness to fetch with a s test_when_finished "rm -rf super4 super5 super6" && git clone . super4 && (cd super4 && - git submodule add --quiet file://"$TRASH_DIRECTORY"/submodule submodule3 && + git submodule add --quiet "$TRASH_DIRECTORY_URL"/submodule submodule3 && git commit -am "setup submodule3" ) && (cd submodule && diff --git a/t/test-lib.sh b/t/test-lib.sh index 0fb76f7d11e840..94f7c4aaa46b4d 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -348,6 +348,7 @@ case "$TRASH_DIRECTORY" in /*) ;; # absolute path is good *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;; esac +TRASH_DIRECTORY_URL=file://"$TRASH_DIRECTORY" # If --stress was passed, run this test repeatedly in several parallel loops. if test "$GIT_TEST_STRESS_STARTED" = "done" @@ -1682,6 +1683,7 @@ Darwin) test_set_prereq GREP_STRIPS_CR test_set_prereq WINDOWS GIT_TEST_CMP="GIT_DIR=/dev/null git diff --no-index --ignore-cr-at-eol --" + TRASH_DIRECTORY_URL=file://"$(cygpath -am "$TRASH_DIRECTORY")" ;; *CYGWIN*) test_set_prereq POSIXPERM From 6ba2ebc9b9aed88b5a107fcb81d0f918f5e80de0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 5 Jan 2026 15:15:04 +0100 Subject: [PATCH 2/3] run-command: refactor out logic to determine whether shell is needed There is logic hidden deep inside `run-command.c` that determines whether a given command _actually_ needs to be run through shell (for example, if `git-upload-pack` is the program name, it does not need the shell at all). Extract this useful functionality into its own function. Signed-off-by: Johannes Schindelin --- run-command.c | 2 +- run-command.h | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/run-command.c b/run-command.c index e3e02475ccec50..75eb19936bd646 100644 --- a/run-command.c +++ b/run-command.c @@ -291,7 +291,7 @@ static const char **prepare_shell_cmd(struct strvec *out, const char **argv) if (!argv[0]) BUG("shell command is empty"); - if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) { + if (does_cmd_require_shell(argv[0])) { strvec_push_nodup(out, git_shell_path()); strvec_push(out, "-c"); diff --git a/run-command.h b/run-command.h index 0df25e445f001c..29966fc8f18de0 100644 --- a/run-command.h +++ b/run-command.h @@ -200,6 +200,15 @@ int exists_in_PATH(const char *command); */ char *git_shell_path(void); +/** + * Determines whether the command needs to run through a shell (for example, + * `git-upload-pack` does not need a shell, while `less -R` does). + */ +static inline int does_cmd_require_shell(const char *cmd) +{ + return strcspn(cmd, "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(cmd); +} + /** * Start a sub-process. Takes a pointer to a `struct child_process` * that specifies the details and returns pipe FDs (if requested). From 91c9b959bbaa72cf418ae47ace77d70904adef14 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 5 Jan 2026 16:34:00 +0100 Subject: [PATCH 3/3] git_connect: avoid full shell when executing `git-upload-pack` When the `git-upload-pack` program needs to be called, we do not actually need to run this through a shell... This reduces the security surface of running Git on servers a bit. Signed-off-by: Johannes Schindelin --- connect.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/connect.c b/connect.c index c6f76e30829ff2..66247868e84a82 100644 --- a/connect.c +++ b/connect.c @@ -1512,7 +1512,14 @@ struct child_process *git_connect(int fd[2], const char *url, version); } } - strvec_push(&conn->args, cmd.buf); + if (!conn->use_shell || conn->args.nr > 0 || + does_cmd_require_shell(prog)) + strvec_push(&conn->args, cmd.buf); + else { + /* avoid running through shell; it's unnecessary */ + strvec_pushl(&conn->args, prog, path, NULL); + conn->use_shell = 0; + } if (start_command(conn)) die(_("unable to fork"));