From 6f054f9fb3a501c35b55c65e547a244f14c38d56 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Thu, 28 Jul 2022 17:35:17 -0400 Subject: builtin/clone.c: disallow `--local` clones with symlinks When cloning a repository with `--local`, Git relies on either making a hardlink or copy to every file in the "objects" directory of the source repository. This is done through the callpath `cmd_clone()` -> `clone_local()` -> `copy_or_link_directory()`. The way this optimization works is by enumerating every file and directory recursively in the source repository's `$GIT_DIR/objects` directory, and then either making a copy or hardlink of each file. The only exception to this rule is when copying the "alternates" file, in which case paths are rewritten to be absolute before writing a new "alternates" file in the destination repo. One quirk of this implementation is that it dereferences symlinks when cloning. This behavior was most recently modified in 36596fd2df (clone: better handle symlinked files at .git/objects/, 2019-07-10), which attempted to support `--local` clones of repositories with symlinks in their objects directory in a platform-independent way. Unfortunately, this behavior of dereferencing symlinks (that is, creating a hardlink or copy of the source's link target in the destination repository) can be used as a component in attacking a victim by inadvertently exposing the contents of file stored outside of the repository. Take, for example, a repository that stores a Dockerfile and is used to build Docker images. When building an image, Docker copies the directory contents into the VM, and then instructs the VM to execute the Dockerfile at the root of the copied directory. This protects against directory traversal attacks by copying symbolic links as-is without dereferencing them. That is, if a user has a symlink pointing at their private key material (where the symlink is present in the same directory as the Dockerfile, but the key itself is present outside of that directory), the key is unreadable to a Docker image, since the link will appear broken from the container's point of view. This behavior enables an attack whereby a victim is convinced to clone a repository containing an embedded submodule (with a URL like "file:///proc/self/cwd/path/to/submodule") which has a symlink pointing at a path containing sensitive information on the victim's machine. If a user is tricked into doing this, the contents at the destination of those symbolic links are exposed to the Docker image at runtime. One approach to preventing this behavior is to recreate symlinks in the destination repository. But this is problematic, since symlinking the objects directory are not well-supported. (One potential problem is that when sharing, e.g. a "pack" directory via symlinks, different writers performing garbage collection may consider different sets of objects to be reachable, enabling a situation whereby garbage collecting one repository may remove reachable objects in another repository). Instead, prohibit the local clone optimization when any symlinks are present in the `$GIT_DIR/objects` directory of the source repository. Users may clone the repository again by prepending the "file://" scheme to their clone URL, or by adding the `--no-local` option to their `git clone` invocation. The directory iterator used by `copy_or_link_directory()` must no longer dereference symlinks (i.e., it *must* call `lstat()` instead of `stat()` in order to discover whether or not there are symlinks present). This has no bearing on the overall behavior, since we will immediately `die()` on encounter a symlink. Note that t5604.33 suggests that we do support local clones with symbolic links in the source repository's objects directory, but this was likely unintentional, or at least did not take into consideration the problem with sharing parts of the objects directory with symbolic links at the time. Update this test to reflect which options are and aren't supported. Helped-by: Johannes Schindelin Signed-off-by: Taylor Blau --- t/t5604-clone-reference.sh | 50 +++++++++++++++++----------------------------- 1 file changed, 18 insertions(+), 32 deletions(-) (limited to 't') diff --git a/t/t5604-clone-reference.sh b/t/t5604-clone-reference.sh index 2f7be23044..9d32f1c4a4 100755 --- a/t/t5604-clone-reference.sh +++ b/t/t5604-clone-reference.sh @@ -300,8 +300,6 @@ test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown file ln -s ../an-object $obj && cd ../ && - find . -type f | sort >../../../T.objects-files.raw && - find . -type l | sort >../../../T.objects-symlinks.raw && echo unknown_content >unknown_file ) && git -C T fsck && @@ -310,19 +308,27 @@ test_expect_success SYMLINKS 'setup repo with manually symlinked or unknown file test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at objects/' ' - for option in --local --no-hardlinks --shared --dissociate + # None of these options work when cloning locally, since T has + # symlinks in its `$GIT_DIR/objects` directory + for option in --local --no-hardlinks --dissociate do - git clone $option T T$option || return 1 && - git -C T$option fsck || return 1 && - git -C T$option rev-list --all --objects >T$option.objects && - test_cmp T.objects T$option.objects && - ( - cd T$option/.git/objects && - find . -type f | sort >../../../T$option.objects-files.raw && - find . -type l | sort >../../../T$option.objects-symlinks.raw - ) + test_must_fail git clone $option T T$option 2>err || return 1 && + test_i18ngrep "symlink.*exists" err || return 1 done && + # But `--shared` clones should still work, even when specifying + # a local path *and* that repository has symlinks present in its + # `$GIT_DIR/objects` directory. + git clone --shared T T--shared && + git -C T--shared fsck && + git -C T--shared rev-list --all --objects >T--shared.objects && + test_cmp T.objects T--shared.objects && + ( + cd T--shared/.git/objects && + find . -type f | sort >../../../T--shared.objects-files.raw && + find . -type l | sort >../../../T--shared.objects-symlinks.raw + ) && + for raw in $(ls T*.raw) do sed -e "s!/../!/Y/!; s![0-9a-f]\{38,\}!Z!" -e "/commit-graph/d" \ @@ -330,26 +336,6 @@ test_expect_success SYMLINKS 'clone repo with symlinked or unknown files at obje sort $raw.de-sha-1 >$raw.de-sha || return 1 done && - cat >expected-files <<-EOF && - ./Y/Z - ./Y/Z - ./Y/Z - ./a-loose-dir/Z - ./an-object - ./info/packs - ./pack/pack-Z.idx - ./pack/pack-Z.pack - ./packs/pack-Z.idx - ./packs/pack-Z.pack - ./unknown_file - EOF - - for option in --local --no-hardlinks --dissociate - do - test_cmp expected-files T$option.objects-files.raw.de-sha || return 1 && - test_must_be_empty T$option.objects-symlinks.raw.de-sha || return 1 - done && - echo ./info/alternates >expected-files && test_cmp expected-files T--shared.objects-files.raw && test_must_be_empty T--shared.objects-symlinks.raw -- cgit v1.2.3 From 7de0c306f7b758d3fb537c18c2751f6250cea7a0 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:13:58 -0400 Subject: t/lib-submodule-update.sh: allow local submodules To prepare for changing the default value of `protocol.file.allow` to "user", update the `prolog()` function in lib-submodule-update to allow submodules to be cloned over the file protocol. This is used by a handful of submodule-related test scripts, which themselves will have to tweak the value of `protocol.file.allow` in certain locations. Those will be done in subsequent commits. Signed-off-by: Taylor Blau --- t/lib-submodule-update.sh | 1 + 1 file changed, 1 insertion(+) (limited to 't') diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh index 4b714e9308..cc5b58bdde 100644 --- a/t/lib-submodule-update.sh +++ b/t/lib-submodule-update.sh @@ -196,6 +196,7 @@ test_git_directory_exists () { # the submodule repo if it doesn't exist and configures the most problematic # settings for diff.ignoreSubmodules. prolog () { + test_config_global protocol.file.allow always && (test -d submodule_update_repo || create_lib_submodule_repo) && test_config_global diff.ignoreSubmodules all && test_config diff.ignoreSubmodules all -- cgit v1.2.3 From 8a96dbcb339d25ba1813632319ea4052bc586ddf Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:16:10 -0400 Subject: t/t1NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Tests that interact with submodules a handful of times use `test_config_global` instead. Signed-off-by: Taylor Blau --- t/t1091-sparse-checkout-builtin.sh | 3 ++- t/t1500-rev-parse.sh | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 84acfc48b6..749c8f1c0a 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -449,7 +449,8 @@ test_expect_success 'interaction with submodules' ' ( cd super && mkdir modules && - git submodule add ../repo modules/child && + git -c protocol.file.allow=always \ + submodule add ../repo modules/child && git add . && git commit -m "add submodule" && git sparse-checkout init --cone && diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh index 408b97d5af..acef9fda0c 100755 --- a/t/t1500-rev-parse.sh +++ b/t/t1500-rev-parse.sh @@ -163,7 +163,8 @@ test_expect_success 'showing the superproject correctly' ' test_commit -C super test_commit && test_create_repo sub && test_commit -C sub test_commit && - git -C super submodule add ../sub dir/sub && + git -c protocol.file.allow=always \ + -C super submodule add ../sub dir/sub && echo $(pwd)/super >expect && git -C super/dir/sub rev-parse --show-superproject-working-tree >out && test_cmp expect out && -- cgit v1.2.3 From 99f4abb8dae4c9c604e5d5cf255958bbe537b928 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:20:03 -0400 Subject: t/2NNNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Tests that interact with submodules a handful of times use `test_config_global` instead. Test scripts that rely on submodules throughout use a `git config --global` during a setup test towards the beginning of the script. Signed-off-by: Taylor Blau --- t/t2400-worktree-add.sh | 2 ++ t/t2403-worktree-move.sh | 7 +++++-- t/t2405-worktree-submodule.sh | 1 + 3 files changed, 8 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh index 5a7495474a..cd02f7854d 100755 --- a/t/t2400-worktree-add.sh +++ b/t/t2400-worktree-add.sh @@ -597,6 +597,7 @@ test_expect_success '"add" should not fail because of another bad worktree' ' ' test_expect_success '"add" with uninitialized submodule, with submodule.recurse unset' ' + test_config_global protocol.file.allow always && test_create_repo submodule && test_commit -C submodule first && test_create_repo project && @@ -612,6 +613,7 @@ test_expect_success '"add" with uninitialized submodule, with submodule.recurse ' test_expect_success '"add" with initialized submodule, with submodule.recurse unset' ' + test_config_global protocol.file.allow always && git -C project-clone submodule update --init && git -C project-clone worktree add ../project-4 ' diff --git a/t/t2403-worktree-move.sh b/t/t2403-worktree-move.sh index a4e1a178e0..e8246eed9c 100755 --- a/t/t2403-worktree-move.sh +++ b/t/t2403-worktree-move.sh @@ -138,7 +138,8 @@ test_expect_success 'move a repo with uninitialized submodule' ' ( cd withsub && test_commit initial && - git submodule add "$PWD"/.git sub && + git -c protocol.file.allow=always \ + submodule add "$PWD"/.git sub && git commit -m withsub && git worktree add second HEAD && git worktree move second third @@ -148,7 +149,7 @@ test_expect_success 'move a repo with uninitialized submodule' ' test_expect_success 'not move a repo with initialized submodule' ' ( cd withsub && - git -C third submodule update && + git -c protocol.file.allow=always -C third submodule update && test_must_fail git worktree move third forth ) ' @@ -227,6 +228,7 @@ test_expect_success 'remove cleans up .git/worktrees when empty' ' ' test_expect_success 'remove a repo with uninitialized submodule' ' + test_config_global protocol.file.allow always && ( cd withsub && git worktree add to-remove HEAD && @@ -235,6 +237,7 @@ test_expect_success 'remove a repo with uninitialized submodule' ' ' test_expect_success 'not remove a repo with initialized submodule' ' + test_config_global protocol.file.allow always && ( cd withsub && git worktree add to-remove HEAD && diff --git a/t/t2405-worktree-submodule.sh b/t/t2405-worktree-submodule.sh index e1b2bfd87e..51120d5deb 100755 --- a/t/t2405-worktree-submodule.sh +++ b/t/t2405-worktree-submodule.sh @@ -7,6 +7,7 @@ test_description='Combination of submodules and multiple worktrees' base_path=$(pwd -P) test_expect_success 'setup: create origin repos' ' + git config --global protocol.file.allow always && git init origin/sub && test_commit -C origin/sub file1 && git init origin/main && -- cgit v1.2.3 From f8d510ed0b357787c8d035d64f240bd82b424dc4 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:20:28 -0400 Subject: t/t3NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Tests that interact with submodules a handful of times use `test_config_global` instead. Test scripts that rely on submodules throughout use a `git config --global` during a setup test towards the beginning of the script. Signed-off-by: Taylor Blau --- t/t3200-branch.sh | 1 + t/t3420-rebase-autostash.sh | 2 +- t/t3426-rebase-submodule.sh | 3 ++- t/t3512-cherry-pick-submodule.sh | 2 ++ t/t3600-rm.sh | 3 ++- t/t3906-stash-submodule.sh | 2 +- 6 files changed, 9 insertions(+), 4 deletions(-) (limited to 't') diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 3ec3e1d730..631a0b506a 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -279,6 +279,7 @@ test_expect_success 'deleting checked-out branch from repo that is a submodule' git init repo1 && git init repo1/sub && test_commit -C repo1/sub x && + test_config_global protocol.file.allow always && git -C repo1 submodule add ./sub && git -C repo1 commit -m "adding sub" && diff --git a/t/t3420-rebase-autostash.sh b/t/t3420-rebase-autostash.sh index ca331733fb..80df13a9a9 100755 --- a/t/t3420-rebase-autostash.sh +++ b/t/t3420-rebase-autostash.sh @@ -307,7 +307,7 @@ test_expect_success 'autostash is saved on editor failure with conflict' ' test_expect_success 'autostash with dirty submodules' ' test_when_finished "git reset --hard && git checkout master" && git checkout -b with-submodule && - git submodule add ./ sub && + git -c protocol.file.allow=always submodule add ./ sub && test_tick && git commit -m add-submodule && echo changed >sub/file0 && diff --git a/t/t3426-rebase-submodule.sh b/t/t3426-rebase-submodule.sh index 0ad3a07bf4..fb21f675bb 100755 --- a/t/t3426-rebase-submodule.sh +++ b/t/t3426-rebase-submodule.sh @@ -47,7 +47,8 @@ test_expect_success 'rebase interactive ignores modified submodules' ' git init sub && git -C sub commit --allow-empty -m "Initial commit" && git init super && - git -C super submodule add ../sub && + git -c protocol.file.allow=always \ + -C super submodule add ../sub && git -C super config submodule.sub.ignore dirty && >super/foo && git -C super add foo && diff --git a/t/t3512-cherry-pick-submodule.sh b/t/t3512-cherry-pick-submodule.sh index 6ece1d8573..697bc68958 100755 --- a/t/t3512-cherry-pick-submodule.sh +++ b/t/t3512-cherry-pick-submodule.sh @@ -10,6 +10,8 @@ KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 test_submodule_switch "cherry-pick" test_expect_success 'unrelated submodule/file conflict is ignored' ' + test_config_global protocol.file.allow always && + test_create_repo sub && touch sub/file && diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index efec8d13b6..99dab76332 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -321,7 +321,7 @@ test_expect_success 'rm removes empty submodules from work tree' ' test_expect_success 'rm removes removed submodule from index and .gitmodules' ' git reset --hard && - git submodule update && + git -c protocol.file.allow=always submodule update && rm -rf submod && git rm submod && git status -s -uno --ignore-submodules=none >actual && @@ -627,6 +627,7 @@ cat >expect.deepmodified < Date: Fri, 29 Jul 2022 15:20:43 -0400 Subject: t/t4NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Tests that interact with submodules a handful of times use `test_config_global` instead. Test scripts that rely on submodules throughout use a `git config --global` during a setup test towards the beginning of the script. Signed-off-by: Taylor Blau --- t/t4059-diff-submodule-not-initialized.sh | 2 +- t/t4060-diff-submodule-option-diff-format.sh | 4 ++-- t/t4067-diff-partial-clone.sh | 1 + t/t4208-log-magic-pathspec.sh | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) (limited to 't') diff --git a/t/t4059-diff-submodule-not-initialized.sh b/t/t4059-diff-submodule-not-initialized.sh index 49bca7b48d..d489230df8 100755 --- a/t/t4059-diff-submodule-not-initialized.sh +++ b/t/t4059-diff-submodule-not-initialized.sh @@ -49,7 +49,7 @@ test_expect_success 'setup - submodules' ' ' test_expect_success 'setup - git submodule add' ' - git submodule add ./sm2 sm1 && + git -c protocol.file.allow=always submodule add ./sm2 sm1 && commit_file sm1 .gitmodules && git diff-tree -p --no-commit-id --submodule=log HEAD -- sm1 >actual && cat >expected <<-EOF && diff --git a/t/t4060-diff-submodule-option-diff-format.sh b/t/t4060-diff-submodule-option-diff-format.sh index fc8229c726..57b19125c0 100755 --- a/t/t4060-diff-submodule-option-diff-format.sh +++ b/t/t4060-diff-submodule-option-diff-format.sh @@ -759,9 +759,9 @@ test_expect_success 'diff --submodule=diff with .git file' ' ' test_expect_success 'setup nested submodule' ' - git submodule add -f ./sm2 && + git -c protocol.file.allow=always submodule add -f ./sm2 && git commit -a -m "add sm2" && - git -C sm2 submodule add ../sm2 nested && + git -c protocol.file.allow=always -C sm2 submodule add ../sm2 nested && git -C sm2 commit -a -m "nested sub" && head10=$(git -C sm2 rev-parse --short --verify HEAD) ' diff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh index 804f2a82e8..28f42a4046 100755 --- a/t/t4067-diff-partial-clone.sh +++ b/t/t4067-diff-partial-clone.sh @@ -77,6 +77,7 @@ test_expect_success 'diff skips same-OID blobs' ' test_expect_success 'when fetching missing objects, diff skips GITLINKs' ' test_when_finished "rm -rf sub server client trace" && + test_config_global protocol.file.allow always && test_create_repo sub && test_commit -C sub first && diff --git a/t/t4208-log-magic-pathspec.sh b/t/t4208-log-magic-pathspec.sh index 6cdbe4747a..aeaf0d5ba3 100755 --- a/t/t4208-log-magic-pathspec.sh +++ b/t/t4208-log-magic-pathspec.sh @@ -126,6 +126,7 @@ test_expect_success 'command line pathspec parsing for "git log"' ' test_expect_success 'tree_entry_interesting does not match past submodule boundaries' ' test_when_finished "rm -rf repo submodule" && + test_config_global protocol.file.allow always && git init submodule && test_commit -C submodule initial && git init repo && -- cgit v1.2.3 From 225d2d50ccef4baae410a96b9dc9e3978d164826 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:21:06 -0400 Subject: t/t5NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Tests that interact with submodules a handful of times use `test_config_global` instead. Test scripts that rely on submodules throughout use a `git config --global` during a setup test towards the beginning of the script. Signed-off-by: Taylor Blau --- t/t5510-fetch.sh | 1 + t/t5526-fetch-submodules.sh | 1 + t/t5545-push-options.sh | 1 + t/t5572-pull-submodule.sh | 4 ++++ t/t5601-clone.sh | 1 + t/t5614-clone-submodules-shallow.sh | 8 ++++++++ t/t5616-partial-clone.sh | 2 ++ t/t5617-clone-submodules-remote.sh | 1 + 8 files changed, 19 insertions(+) (limited to 't') diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh index 2013051a64..b60ba0f788 100755 --- a/t/t5510-fetch.sh +++ b/t/t5510-fetch.sh @@ -627,6 +627,7 @@ test_expect_success 'fetch.writeCommitGraph' ' ' test_expect_success 'fetch.writeCommitGraph with submodules' ' + test_config_global protocol.file.allow always && git clone dups super && ( cd super && diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 53d7b8ed75..aa04eacb65 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -35,6 +35,7 @@ add_upstream_commit() { } test_expect_success setup ' + git config --global protocol.file.allow always && mkdir deepsubmodule && ( cd deepsubmodule && diff --git a/t/t5545-push-options.sh b/t/t5545-push-options.sh index 38e6f7340e..77ce917cdd 100755 --- a/t/t5545-push-options.sh +++ b/t/t5545-push-options.sh @@ -113,6 +113,7 @@ test_expect_success 'push options and submodules' ' test_commit -C parent one && git -C parent push --mirror up && + test_config_global protocol.file.allow always && git -C parent submodule add ../upstream workbench && git -C parent/workbench remote add up ../../upstream && git -C parent commit -m "add submodule" && diff --git a/t/t5572-pull-submodule.sh b/t/t5572-pull-submodule.sh index 37fd06b0be..9b3b4af610 100755 --- a/t/t5572-pull-submodule.sh +++ b/t/t5572-pull-submodule.sh @@ -46,6 +46,10 @@ KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR=1 KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES=1 test_submodule_switch_func "git_pull_noff" +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'pull --recurse-submodule setup' ' test_create_repo child && test_commit -C child bar && diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index 7df3c5373a..50d482114d 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -738,6 +738,7 @@ test_expect_success 'batch missing blob request does not inadvertently try to fe echo aa >server/a && echo bb >server/b && # Also add a gitlink pointing to an arbitrary repository + test_config_global protocol.file.allow always && git -C server submodule add "$(pwd)/repo_for_submodule" c && git -C server add a b c && git -C server commit -m x && diff --git a/t/t5614-clone-submodules-shallow.sh b/t/t5614-clone-submodules-shallow.sh index e4e6ea4d52..d361738b51 100755 --- a/t/t5614-clone-submodules-shallow.sh +++ b/t/t5614-clone-submodules-shallow.sh @@ -24,6 +24,7 @@ test_expect_success 'setup' ' test_expect_success 'nonshallow clone implies nonshallow submodule' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git clone --recurse-submodules "file://$pwd/." super_clone && git -C super_clone log --oneline >lines && test_line_count = 3 lines && @@ -33,6 +34,7 @@ test_expect_success 'nonshallow clone implies nonshallow submodule' ' test_expect_success 'shallow clone with shallow submodule' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git clone --recurse-submodules --depth 2 --shallow-submodules "file://$pwd/." super_clone && git -C super_clone log --oneline >lines && test_line_count = 2 lines && @@ -42,6 +44,7 @@ test_expect_success 'shallow clone with shallow submodule' ' test_expect_success 'shallow clone does not imply shallow submodule' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git clone --recurse-submodules --depth 2 "file://$pwd/." super_clone && git -C super_clone log --oneline >lines && test_line_count = 2 lines && @@ -51,6 +54,7 @@ test_expect_success 'shallow clone does not imply shallow submodule' ' test_expect_success 'shallow clone with non shallow submodule' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git clone --recurse-submodules --depth 2 --no-shallow-submodules "file://$pwd/." super_clone && git -C super_clone log --oneline >lines && test_line_count = 2 lines && @@ -60,6 +64,7 @@ test_expect_success 'shallow clone with non shallow submodule' ' test_expect_success 'non shallow clone with shallow submodule' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git clone --recurse-submodules --no-local --shallow-submodules "file://$pwd/." super_clone && git -C super_clone log --oneline >lines && test_line_count = 3 lines && @@ -69,6 +74,7 @@ test_expect_success 'non shallow clone with shallow submodule' ' test_expect_success 'clone follows shallow recommendation' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git config -f .gitmodules submodule.sub.shallow true && git add .gitmodules && git commit -m "recommend shallow for sub" && @@ -87,6 +93,7 @@ test_expect_success 'clone follows shallow recommendation' ' test_expect_success 'get unshallow recommended shallow submodule' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git clone --no-local "file://$pwd/." super_clone && ( cd super_clone && @@ -103,6 +110,7 @@ test_expect_success 'get unshallow recommended shallow submodule' ' test_expect_success 'clone follows non shallow recommendation' ' test_when_finished "rm -rf super_clone" && + test_config_global protocol.file.allow always && git config -f .gitmodules submodule.sub.shallow false && git add .gitmodules && git commit -m "recommend non shallow for sub" && diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh index d98c550267..a6138e8cfc 100755 --- a/t/t5616-partial-clone.sh +++ b/t/t5616-partial-clone.sh @@ -171,6 +171,8 @@ test_expect_success 'partial clone with transfer.fsckobjects=1 works with submod test_config -C src_with_sub uploadpack.allowfilter 1 && test_config -C src_with_sub uploadpack.allowanysha1inwant 1 && + test_config_global protocol.file.allow always && + git -C src_with_sub submodule add "file://$(pwd)/submodule" mysub && git -C src_with_sub commit -m "commit with submodule" && diff --git a/t/t5617-clone-submodules-remote.sh b/t/t5617-clone-submodules-remote.sh index 1a041df10b..0152dc0440 100755 --- a/t/t5617-clone-submodules-remote.sh +++ b/t/t5617-clone-submodules-remote.sh @@ -7,6 +7,7 @@ test_description='Test cloning repos with submodules using remote-tracking branc pwd=$(pwd) test_expect_success 'setup' ' + git config --global protocol.file.allow always && git checkout -b master && test_commit commit1 && mkdir sub && -- cgit v1.2.3 From 0f21b8f468566b991eea60bb7bdf2fce9265e367 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:21:18 -0400 Subject: t/t6NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Signed-off-by: Taylor Blau --- t/t6008-rev-list-submodule.sh | 2 +- t/t6134-pathspec-in-submodule.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 't') diff --git a/t/t6008-rev-list-submodule.sh b/t/t6008-rev-list-submodule.sh index c4af9ca0a7..a65e5f283c 100755 --- a/t/t6008-rev-list-submodule.sh +++ b/t/t6008-rev-list-submodule.sh @@ -23,7 +23,7 @@ test_expect_success 'setup' ' : > super-file && git add super-file && - git submodule add "$(pwd)" sub && + git -c protocol.file.allow=always submodule add "$(pwd)" sub && git symbolic-ref HEAD refs/heads/super && test_tick && git commit -m super-initial && diff --git a/t/t6134-pathspec-in-submodule.sh b/t/t6134-pathspec-in-submodule.sh index c670668409..2fde65b431 100755 --- a/t/t6134-pathspec-in-submodule.sh +++ b/t/t6134-pathspec-in-submodule.sh @@ -9,7 +9,7 @@ test_expect_success 'setup a submodule' ' : >pretzel/a && git -C pretzel add a && git -C pretzel commit -m "add a file" -- a && - git submodule add ./pretzel sub && + git -c protocol.file.allow=always submodule add ./pretzel sub && git commit -a -m "add submodule" && git submodule deinit --all ' -- cgit v1.2.3 From 0d3beb71dad7906f576b0de9cea32164549163fe Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:21:40 -0400 Subject: t/t7NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that only need to interact with submodules in a limited capacity have individual Git commands annotated with the appropriate configuration via `-c`. Tests that interact with submodules a handful of times use `test_config_global` instead. Test scripts that rely on submodules throughout use a `git config --global` during a setup test towards the beginning of the script. Signed-off-by: Taylor Blau --- t/t7001-mv.sh | 2 ++ t/t7064-wtstatus-pv2.sh | 1 + t/t7300-clean.sh | 1 + t/t7400-submodule-basic.sh | 4 ++++ t/t7403-submodule-sync.sh | 2 ++ t/t7406-submodule-update.sh | 1 + t/t7407-submodule-foreach.sh | 1 + t/t7408-submodule-reference.sh | 4 ++++ t/t7409-submodule-detached-work-tree.sh | 4 ++++ t/t7411-submodule-config.sh | 3 +++ t/t7413-submodule-is-active.sh | 1 + t/t7414-submodule-mistakes.sh | 3 ++- t/t7415-submodule-names.sh | 4 ++++ t/t7416-submodule-dash-url.sh | 4 ++++ t/t7417-submodule-path-url.sh | 4 ++++ t/t7418-submodule-sparse-gitmodules.sh | 4 ++++ t/t7419-submodule-set-branch.sh | 4 ++++ t/t7420-submodule-set-url.sh | 4 ++++ t/t7421-submodule-summary-add.sh | 4 ++++ t/t7506-status-submodule.sh | 2 ++ t/t7507-commit-verbose.sh | 1 + t/t7800-difftool.sh | 1 + t/t7814-grep-recurse-submodules.sh | 4 ++++ 23 files changed, 62 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index 63d5f41a12..6156eeb9ad 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -307,6 +307,7 @@ test_expect_success SYMLINKS 'check moved symlink' ' rm -f moved symlink test_expect_success 'setup submodule' ' + test_config_global protocol.file.allow always && git commit -m initial && git reset --hard && git submodule add ./. sub && @@ -513,6 +514,7 @@ test_expect_success 'moving a submodule in nested directories' ' ' test_expect_success 'moving nested submodules' ' + test_config_global protocol.file.allow always && git commit -am "cleanup commit" && mkdir sub_nested_nested && (cd sub_nested_nested && diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh index 601b47830b..2fdb6ecc17 100755 --- a/t/t7064-wtstatus-pv2.sh +++ b/t/t7064-wtstatus-pv2.sh @@ -462,6 +462,7 @@ test_expect_success 'create and add submodule, submodule appears clean (A. S...) git checkout initial-branch && git clone . sub_repo && git clone . super_repo && + test_config_global protocol.file.allow always && ( cd super_repo && git submodule add ../sub_repo sub1 && diff --git a/t/t7300-clean.sh b/t/t7300-clean.sh index cb5e34d94c..713f113a4a 100755 --- a/t/t7300-clean.sh +++ b/t/t7300-clean.sh @@ -480,6 +480,7 @@ test_expect_success 'should not clean submodules' ' git init && test_commit msg hello.world ) && + test_config_global protocol.file.allow always && git submodule add ./repo/.git sub1 && git commit -m "sub1" && git branch before_sub2 && diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index fec7e0299d..bf1a4dfadb 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -11,6 +11,10 @@ subcommands of git submodule. . ./test-lib.sh +test_expect_success 'setup - enable local submodules' ' + git config --global protocol.file.allow always +' + test_expect_success 'submodule deinit works on empty repository' ' git submodule deinit --all ' diff --git a/t/t7403-submodule-sync.sh b/t/t7403-submodule-sync.sh index 0726799e74..3bc904bacb 100755 --- a/t/t7403-submodule-sync.sh +++ b/t/t7403-submodule-sync.sh @@ -11,6 +11,8 @@ These tests exercise the "git submodule sync" subcommand. . ./test-lib.sh test_expect_success setup ' + git config --global protocol.file.allow always && + echo file >file && git add file && test_tick && diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index acb8766ac2..328b9b7012 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -22,6 +22,7 @@ compare_head() test_expect_success 'setup a submodule tree' ' + git config --global protocol.file.allow always && echo file > file && git add file && test_tick && diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 6b2aa917e1..a3404ac330 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -13,6 +13,7 @@ that are currently checked out. test_expect_success 'setup a submodule tree' ' + git config --global protocol.file.allow always && echo file > file && git add file && test_tick && diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index a3892f494b..02feb85779 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -17,6 +17,10 @@ test_alternate_is_used () { test_cmp expect actual } +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'preparing first repository' ' test_create_repo A && ( diff --git a/t/t7409-submodule-detached-work-tree.sh b/t/t7409-submodule-detached-work-tree.sh index fc018e3638..e12eed5a30 100755 --- a/t/t7409-submodule-detached-work-tree.sh +++ b/t/t7409-submodule-detached-work-tree.sh @@ -12,6 +12,10 @@ on detached working trees TEST_NO_CREATE_REPO=1 . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'submodule on detached working tree' ' git init --bare remote && test_create_repo bundle1 && diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh index ad28e93880..c583c4e373 100755 --- a/t/t7411-submodule-config.sh +++ b/t/t7411-submodule-config.sh @@ -12,6 +12,9 @@ from the database and from the worktree works. TEST_NO_CREATE_REPO=1 . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' test_expect_success 'submodule config cache setup' ' mkdir submodule && (cd submodule && diff --git a/t/t7413-submodule-is-active.sh b/t/t7413-submodule-is-active.sh index c8e7e98331..c8b5ac2928 100755 --- a/t/t7413-submodule-is-active.sh +++ b/t/t7413-submodule-is-active.sh @@ -9,6 +9,7 @@ submodules which are "active" and interesting to the user. . ./test-lib.sh test_expect_success 'setup' ' + git config --global protocol.file.allow always && git init sub && test_commit -C sub initial && git init super && diff --git a/t/t7414-submodule-mistakes.sh b/t/t7414-submodule-mistakes.sh index f2e7df59cf..cf95603d7d 100755 --- a/t/t7414-submodule-mistakes.sh +++ b/t/t7414-submodule-mistakes.sh @@ -30,7 +30,8 @@ test_expect_success 'no warning when updating entry' ' test_expect_success 'submodule add does not warn' ' test_when_finished "git rm -rf submodule .gitmodules" && - git submodule add ./embed submodule 2>stderr && + git -c protocol.file.allow=always \ + submodule add ./embed submodule 2>stderr && test_i18ngrep ! warning stderr ' diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index f70368bc2e..f37456f15b 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -8,6 +8,10 @@ real-world setup that confirms we catch this in practice. . ./test-lib.sh . "$TEST_DIRECTORY"/lib-pack.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'check names' ' cat >expect <<-\EOF && valid diff --git a/t/t7416-submodule-dash-url.sh b/t/t7416-submodule-dash-url.sh index d21dc8b009..3ebd985981 100755 --- a/t/t7416-submodule-dash-url.sh +++ b/t/t7416-submodule-dash-url.sh @@ -3,6 +3,10 @@ test_description='check handling of disallowed .gitmodule urls' . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'create submodule with protected dash in url' ' git init upstream && git -C upstream commit --allow-empty -m base && diff --git a/t/t7417-submodule-path-url.sh b/t/t7417-submodule-path-url.sh index f7e7e94d7b..b17d18034a 100755 --- a/t/t7417-submodule-path-url.sh +++ b/t/t7417-submodule-path-url.sh @@ -3,6 +3,10 @@ test_description='check handling of .gitmodule path with dash' . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'create submodule with dash in path' ' git init upstream && git -C upstream commit --allow-empty -m base && diff --git a/t/t7418-submodule-sparse-gitmodules.sh b/t/t7418-submodule-sparse-gitmodules.sh index 3f7f271883..16331c34c4 100755 --- a/t/t7418-submodule-sparse-gitmodules.sh +++ b/t/t7418-submodule-sparse-gitmodules.sh @@ -14,6 +14,10 @@ also by committing .gitmodules and then just removing it from the filesystem. . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'sparse checkout setup which hides .gitmodules' ' git init upstream && git init submodule && diff --git a/t/t7419-submodule-set-branch.sh b/t/t7419-submodule-set-branch.sh index 3b925c302f..5357093e98 100755 --- a/t/t7419-submodule-set-branch.sh +++ b/t/t7419-submodule-set-branch.sh @@ -12,6 +12,10 @@ as expected. TEST_NO_CREATE_REPO=1 . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'submodule config cache setup' ' mkdir submodule && (cd submodule && diff --git a/t/t7420-submodule-set-url.sh b/t/t7420-submodule-set-url.sh index ef0cb6e8e1..d6bf62b3ac 100755 --- a/t/t7420-submodule-set-url.sh +++ b/t/t7420-submodule-set-url.sh @@ -12,6 +12,10 @@ as expected. TEST_NO_CREATE_REPO=1 . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'submodule config cache setup' ' mkdir submodule && ( diff --git a/t/t7421-submodule-summary-add.sh b/t/t7421-submodule-summary-add.sh index b070f13714..ce64d8b137 100755 --- a/t/t7421-submodule-summary-add.sh +++ b/t/t7421-submodule-summary-add.sh @@ -12,6 +12,10 @@ while making sure to add submodules using `git submodule add` instead of . ./test-lib.sh +test_expect_success 'setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'summary test environment setup' ' git init sm && test_commit -C sm "add file" file file-content file-tag && diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh index 3fcb44767f..459300c40b 100755 --- a/t/t7506-status-submodule.sh +++ b/t/t7506-status-submodule.sh @@ -251,6 +251,7 @@ test_expect_success 'status with merge conflict in .gitmodules' ' test_create_repo_with_commit sub1 && test_tick && test_create_repo_with_commit sub2 && + test_config_global protocol.file.allow always && ( cd super && prev=$(git rev-parse HEAD) && @@ -326,6 +327,7 @@ test_expect_success 'diff --submodule with merge conflict in .gitmodules' ' # sub2 will have an untracked file # sub3 will have an untracked repository test_expect_success 'setup superproject with untracked file in nested submodule' ' + test_config_global protocol.file.allow always && ( cd super && git clean -dfx && diff --git a/t/t7507-commit-verbose.sh b/t/t7507-commit-verbose.sh index ed2653d46f..bd0ae4b1ea 100755 --- a/t/t7507-commit-verbose.sh +++ b/t/t7507-commit-verbose.sh @@ -74,6 +74,7 @@ test_expect_success 'diff in message is retained with -v' ' test_expect_success 'submodule log is stripped out too with -v' ' git config diff.submodule log && + test_config_global protocol.file.allow always && git submodule add ./. sub && git commit -m "sub added" && ( diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index a578b35761..cca50697b5 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -626,6 +626,7 @@ test_expect_success 'difftool --no-symlinks detects conflict ' ' test_expect_success 'difftool properly honors gitlink and core.worktree' ' test_when_finished rm -rf submod/ule && + test_config_global protocol.file.allow always && git submodule add ./. submod/ule && test_config -C submod/ule diff.tool checktrees && test_config -C submod/ule difftool.checktrees.cmd '\'' diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh index 828cb3ba58..f465c0d140 100755 --- a/t/t7814-grep-recurse-submodules.sh +++ b/t/t7814-grep-recurse-submodules.sh @@ -193,6 +193,7 @@ test_expect_success !MINGW 'grep recurse submodule colon in name' ' git -C "su:b" commit -m "add fi:le" && test_tick && + test_config_global protocol.file.allow always && git -C parent submodule add "../su:b" "su:b" && git -C parent commit -m "add submodule" && test_tick && @@ -227,6 +228,7 @@ test_expect_success 'grep history with moved submoules' ' git -C sub commit -m "add file" && test_tick && + test_config_global protocol.file.allow always && git -C parent submodule add ../sub dir/sub && git -C parent commit -m "add submodule" && test_tick && @@ -271,6 +273,7 @@ test_expect_success 'grep using relative path' ' mkdir parent/src && echo "(1|2)d(3|4)" >parent/src/file2 && git -C parent add src/file2 && + test_config_global protocol.file.allow always && git -C parent submodule add ../sub && git -C parent commit -m "add files and submodule" && test_tick && @@ -313,6 +316,7 @@ test_expect_success 'grep from a subdir' ' mkdir parent/src && echo "(1|2)d(3|4)" >parent/src/file && git -C parent add src/file && + test_config_global protocol.file.allow always && git -C parent submodule add ../sub src/sub && git -C parent submodule add ../sub sub && git -C parent commit -m "add files and submodules" && -- cgit v1.2.3 From f4a32a550f9d40471fb42ed1e5c8612dfe4a83b1 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 29 Jul 2022 15:21:53 -0400 Subject: t/t9NNN: allow local submodules To prepare for the default value of `protocol.file.allow` to change to "user", ensure tests that rely on local submodules can initialize them over the file protocol. Tests that interact with submodules a handful of times use `test_config_global`. Signed-off-by: Taylor Blau --- t/t9304-fast-import-marks.sh | 1 + t/t9350-fast-export.sh | 2 ++ 2 files changed, 3 insertions(+) (limited to 't') diff --git a/t/t9304-fast-import-marks.sh b/t/t9304-fast-import-marks.sh index d4359dba21..73f3ca2b11 100755 --- a/t/t9304-fast-import-marks.sh +++ b/t/t9304-fast-import-marks.sh @@ -25,6 +25,7 @@ test_expect_success 'import with large marks file' ' ' test_expect_success 'setup dump with submodule' ' + test_config_global protocol.file.allow always && git submodule add "$PWD" sub && git commit -m "add submodule" && git fast-export HEAD >dump diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh index 1372842559..703428fd0a 100755 --- a/t/t9350-fast-export.sh +++ b/t/t9350-fast-export.sh @@ -265,6 +265,7 @@ test_expect_success 'signed-tags=warn-strip' ' test_expect_success 'setup submodule' ' + test_config_global protocol.file.allow always && git checkout -f master && mkdir sub && ( @@ -290,6 +291,7 @@ test_expect_success 'setup submodule' ' test_expect_success 'submodule fast-export | fast-import' ' + test_config_global protocol.file.allow always && SUBENT1=$(git ls-tree master^ sub) && SUBENT2=$(git ls-tree master sub) && rm -rf new && -- cgit v1.2.3 From 32696a4cbe90929ae79ea442f5102c513ce3dfaa Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 28 Sep 2022 18:50:36 -0400 Subject: shell: add basic tests We have no tests of even basic functionality of git-shell. Let's add a couple of obvious ones. This will serve as a framework for adding tests for new things we fix, as well as making sure we don't screw anything up too badly while doing so. Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- t/t9850-shell.sh | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100755 t/t9850-shell.sh (limited to 't') diff --git a/t/t9850-shell.sh b/t/t9850-shell.sh new file mode 100755 index 0000000000..2af476c3af --- /dev/null +++ b/t/t9850-shell.sh @@ -0,0 +1,31 @@ +#!/bin/sh + +test_description='git shell tests' +. ./test-lib.sh + +test_expect_success 'shell allows upload-pack' ' + printf 0000 >input && + git upload-pack . expect && + git shell -c "git-upload-pack $SQ.$SQ" actual && + test_cmp expect actual +' + +test_expect_success 'shell forbids other commands' ' + test_must_fail git shell -c "git config foo.bar baz" +' + +test_expect_success 'shell forbids interactive use by default' ' + test_must_fail git shell +' + +test_expect_success 'shell allows interactive command' ' + mkdir git-shell-commands && + write_script git-shell-commands/ping <<-\EOF && + echo pong + EOF + echo pong >expect && + echo ping | git shell >actual && + test_cmp expect actual +' + +test_done -- cgit v1.2.3 From 71ad7fe1bcec2a115bd0ab187240348358aa7f21 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 28 Sep 2022 18:52:48 -0400 Subject: shell: limit size of interactive commands When git-shell is run in interactive mode (which must be enabled by creating $HOME/git-shell-commands), it reads commands from stdin, one per line, and executes them. We read the commands with git_read_line_interactively(), which uses a strbuf under the hood. That means we'll accept an input of arbitrary size (limited only by how much heap we can allocate). That creates two problems: - the rest of the code is not prepared to handle large inputs. The most serious issue here is that split_cmdline() uses "int" for most of its types, which can lead to integer overflow and out-of-bounds array reads and writes. But even with that fixed, we assume that we can feed the command name to snprintf() (via xstrfmt()), which is stuck for historical reasons using "int", and causes it to fail (and even trigger a BUG() call). - since the point of git-shell is to take input from untrusted or semi-trusted clients, it's a mild denial-of-service. We'll allocate as many bytes as the client sends us (actually twice as many, since we immediately duplicate the buffer). We can fix both by just limiting the amount of per-command input we're willing to receive. We should also fix split_cmdline(), of course, which is an accident waiting to happen, but that can come on top. Most calls to split_cmdline(), including the other one in git-shell, are OK because they are reading from an OS-provided argv, which is limited in practice. This patch should eliminate the immediate vulnerabilities. I picked 4MB as an arbitrary limit. It's big enough that nobody should ever run into it in practice (since the point is to run the commands via exec, we're subject to OS limits which are typically much lower). But it's small enough that allocating it isn't that big a deal. The code is mostly just swapping out fgets() for the strbuf call, but we have to add a few niceties like flushing and trimming line endings. We could simplify things further by putting the buffer on the stack, but 4MB is probably a bit much there. Note that we'll _always_ allocate 4MB, which for normal, non-malicious requests is more than we would before this patch. But on the other hand, other git programs are happy to use 96MB for a delta cache. And since we'd never touch most of those pages, on a lazy-allocating OS like Linux they won't even get allocated to actual RAM. The ideal would be a version of strbuf_getline() that accepted a maximum value. But for a minimal vulnerability fix, let's keep things localized and simple. We can always refactor further on top. The included test fails in an obvious way with ASan or UBSan (which notice the integer overflow and out-of-bounds reads). Without them, it fails in a less obvious way: we may segfault, or we may try to xstrfmt() a long string, leading to a BUG(). Either way, it fails reliably before this patch, and passes with it. Note that we don't need an EXPENSIVE prereq on it. It does take 10-15s to fail before this patch, but with the new limit, we fail almost immediately (and the perl process generating 2GB of data exits via SIGPIPE). Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- shell.c | 34 ++++++++++++++++++++++++++++++---- t/t9850-shell.sh | 6 ++++++ 2 files changed, 36 insertions(+), 4 deletions(-) (limited to 't') diff --git a/shell.c b/shell.c index cef7ffdc9e..02cfd9627f 100644 --- a/shell.c +++ b/shell.c @@ -47,6 +47,8 @@ static void cd_to_homedir(void) die("could not chdir to user's home directory"); } +#define MAX_INTERACTIVE_COMMAND (4*1024*1024) + static void run_shell(void) { int done = 0; @@ -67,22 +69,46 @@ static void run_shell(void) run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE); do { - struct strbuf line = STRBUF_INIT; const char *prog; char *full_cmd; char *rawargs; + size_t len; char *split_args; const char **argv; int code; int count; fprintf(stderr, "git> "); - if (git_read_line_interactively(&line) == EOF) { + + /* + * Avoid using a strbuf or git_read_line_interactively() here. + * We don't want to allocate arbitrary amounts of memory on + * behalf of a possibly untrusted client, and we're subject to + * OS limits on command length anyway. + */ + fflush(stdout); + rawargs = xmalloc(MAX_INTERACTIVE_COMMAND); + if (!fgets(rawargs, MAX_INTERACTIVE_COMMAND, stdin)) { fprintf(stderr, "\n"); - strbuf_release(&line); + free(rawargs); break; } - rawargs = strbuf_detach(&line, NULL); + len = strlen(rawargs); + + /* + * If we truncated due to our input buffer size, reject the + * command. That's better than running bogus input, and + * there's a good chance it's just malicious garbage anyway. + */ + if (len >= MAX_INTERACTIVE_COMMAND - 1) + die("invalid command format: input too long"); + + if (len > 0 && rawargs[len - 1] == '\n') { + if (--len > 0 && rawargs[len - 1] == '\r') + --len; + rawargs[len] = '\0'; + } + split_args = xstrdup(rawargs); count = split_cmdline(split_args, &argv); if (count < 0) { diff --git a/t/t9850-shell.sh b/t/t9850-shell.sh index 2af476c3af..cfc71c3bd4 100755 --- a/t/t9850-shell.sh +++ b/t/t9850-shell.sh @@ -28,4 +28,10 @@ test_expect_success 'shell allows interactive command' ' test_cmp expect actual ' +test_expect_success 'shell complains of overlong commands' ' + perl -e "print \"a\" x 2**12 for (0..2**19)" | + test_must_fail git shell 2>err && + grep "too long" err +' + test_done -- cgit v1.2.3 From 4a7dab5ce429c4a2badbfd49e1f794c1229b20e4 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t1092: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t1092 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t1092-sparse-checkout-compatibility.sh | 2 ++ 1 file changed, 2 insertions(+) (limited to 't') diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index e9a815ca7a..141a7bc65b 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -390,6 +390,8 @@ test_expect_success 'submodule handling' ' test_all_match git add modules && test_all_match git commit -m "add modules directory" && + test_config_global protocol.file.allow always && + run_on_all git submodule add "$(pwd)/initial-repo" modules/sub && test_all_match git commit -m "add submodule" && -- cgit v1.2.3 From 067aa8fb41d09814d26f82515573b067feba87ba Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:48:56 -0400 Subject: t2080: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t1092 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t2080-parallel-checkout-basics.sh | 3 +++ 1 file changed, 3 insertions(+) (limited to 't') diff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh index 3e0f8c675f..393ba3545e 100755 --- a/t/t2080-parallel-checkout-basics.sh +++ b/t/t2080-parallel-checkout-basics.sh @@ -41,6 +41,8 @@ TEST_NO_CREATE_REPO=1 # - m/m (file) # test_expect_success 'setup repo for checkout with various types of changes' ' + test_config_global protocol.file.allow always && + git init sub && ( cd sub && @@ -140,6 +142,7 @@ do esac test_expect_success "$mode checkout on clone" ' + test_config_global protocol.file.allow always && repo=various_${mode}_clone && set_checkout_config $workers $threshold && test_checkout_workers $expected_workers \ -- cgit v1.2.3 From 092d3a2bf97abd6153313686bb2580e6421a1d34 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t1092: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t1092 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t1092-sparse-checkout-compatibility.sh | 2 ++ 1 file changed, 2 insertions(+) (limited to 't') diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 4ba1617752..522c760ca8 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -793,6 +793,8 @@ test_expect_success 'submodule handling' ' test_all_match git add modules && test_all_match git commit -m "add modules directory" && + test_config_global protocol.file.allow always && + run_on_all git submodule add "$(pwd)/initial-repo" modules/sub && test_all_match git commit -m "add submodule" && -- cgit v1.2.3 From ef374dd9b8eecaf84458677865732b89948c19be Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:48:56 -0400 Subject: t2080: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t1092 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t2080-parallel-checkout-basics.sh | 3 +++ 1 file changed, 3 insertions(+) (limited to 't') diff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh index 3e0f8c675f..393ba3545e 100755 --- a/t/t2080-parallel-checkout-basics.sh +++ b/t/t2080-parallel-checkout-basics.sh @@ -41,6 +41,8 @@ TEST_NO_CREATE_REPO=1 # - m/m (file) # test_expect_success 'setup repo for checkout with various types of changes' ' + test_config_global protocol.file.allow always && + git init sub && ( cd sub && @@ -140,6 +142,7 @@ do esac test_expect_success "$mode checkout on clone" ' + test_config_global protocol.file.allow always && repo=various_${mode}_clone && set_checkout_config $workers $threshold && test_checkout_workers $expected_workers \ -- cgit v1.2.3 From e175fb5767f90291d7156960f4a33d726175fff2 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t3207: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t3207 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t3207-branch-submodule.sh | 1 + 1 file changed, 1 insertion(+) (limited to 't') diff --git a/t/t3207-branch-submodule.sh b/t/t3207-branch-submodule.sh index cfde6b237f..fe72b24716 100755 --- a/t/t3207-branch-submodule.sh +++ b/t/t3207-branch-submodule.sh @@ -28,6 +28,7 @@ test_no_branch () { } test_expect_success 'setup superproject and submodule' ' + git config --global protocol.file.allow always && mkdir test_dirs && ( cd test_dirs && -- cgit v1.2.3 From c193e6bbee370a2fc4caae81bf4a2c7be122bc3f Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t5516: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t5516 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t5516-fetch-push.sh | 1 + 1 file changed, 1 insertion(+) (limited to 't') diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index 4dfb080433..6d5702fd0e 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -216,6 +216,7 @@ test_expect_success 'push with negotiation proceeds anyway even if negotiation f test_expect_success 'push with negotiation does not attempt to fetch submodules' ' mk_empty submodule_upstream && test_commit -C submodule_upstream submodule_commit && + test_config_global protocol.file.allow always && git submodule add ./submodule_upstream submodule && mk_empty testrepo && git push testrepo $the_first_commit:refs/remotes/origin/first_commit && -- cgit v1.2.3 From 59f2f80280860e9796661876b4a2cd673448898d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t5537: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t5537 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t5537-fetch-shallow.sh | 2 ++ 1 file changed, 2 insertions(+) (limited to 't') diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 92948de7a0..9573a2655e 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -162,6 +162,8 @@ test_expect_success 'fetch --update-shallow' ' ' test_expect_success 'fetch --update-shallow into a repo with submodules' ' + test_config_global protocol.file.allow always && + git init a-submodule && test_commit -C a-submodule foo && git init repo-with-sub && -- cgit v1.2.3 From 8a7bfa0fd3af2d39be1a808774005c78575908d1 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t7814: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t7814 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t7814-grep-recurse-submodules.sh | 1 + 1 file changed, 1 insertion(+) (limited to 't') diff --git a/t/t7814-grep-recurse-submodules.sh b/t/t7814-grep-recurse-submodules.sh index a4476dc492..9a23a52ac1 100755 --- a/t/t7814-grep-recurse-submodules.sh +++ b/t/t7814-grep-recurse-submodules.sh @@ -546,6 +546,7 @@ test_expect_failure 'grep saves textconv cache in the appropriate repository' ' test_expect_success 'grep partially-cloned submodule' ' # Set up clean superproject and submodule for partial cloning. + test_config_global protocol.file.allow always && git init super && git init super/sub && ( -- cgit v1.2.3 From 541607d9348307ac4bc4245a913672eacc4f6e61 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t3206: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t3206 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t3206-range-diff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't') diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh index d12e4e4cc6..03d4df2359 100755 --- a/t/t3206-range-diff.sh +++ b/t/t3206-range-diff.sh @@ -782,7 +782,7 @@ test_expect_success 'submodule changes are shown irrespective of diff.submodule' sub_oid3=$(git -C sub-repo rev-parse HEAD) && git checkout -b main-sub topic && - git submodule add ./sub-repo sub && + git -c protocol.file.allow=always submodule add ./sub-repo sub && git -C sub checkout --detach sub-first && git commit -m "add sub" sub && sup_oid1=$(git rev-parse --short HEAD) && -- cgit v1.2.3 From d9fcaeece2ff946dc3648a22252a92c5400fabe0 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t5537: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t5537 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t5537-fetch-shallow.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 't') diff --git a/t/t5537-fetch-shallow.sh b/t/t5537-fetch-shallow.sh index 10e9a7ff26..dc7a824254 100755 --- a/t/t5537-fetch-shallow.sh +++ b/t/t5537-fetch-shallow.sh @@ -175,7 +175,8 @@ test_expect_success 'fetch --update-shallow into a repo with submodules' ' test_expect_success 'fetch --update-shallow a commit that is also a shallow point into a repo with submodules' ' test_when_finished "rm -rf repo-with-sub" && git init repo-with-sub && - git -C repo-with-sub submodule add ../a-submodule a-submodule && + git -c protocol.file.allow=always -C repo-with-sub \ + submodule add ../a-submodule a-submodule && git -C repo-with-sub commit -m "added submodule" && SHALLOW=$(cat shallow/.git/shallow) && -- cgit v1.2.3 From 9a167cb7860d7ae7cd5685434d0194c934800128 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Fri, 30 Sep 2022 16:47:00 -0400 Subject: t7527: prepare for changing protocol.file.allow Explicitly cloning over the "file://" protocol in t7527 in preparation for merging a security release which will change the default value of this configuration to be "user". Signed-off-by: Taylor Blau --- t/t7527-builtin-fsmonitor.sh | 4 ++++ 1 file changed, 4 insertions(+) (limited to 't') diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh index 56c0dfffea..d419085379 100755 --- a/t/t7527-builtin-fsmonitor.sh +++ b/t/t7527-builtin-fsmonitor.sh @@ -813,6 +813,10 @@ my_match_and_clean () { git -C super/dir_1/dir_2/sub clean -d -f } +test_expect_success 'submodule setup' ' + git config --global protocol.file.allow always +' + test_expect_success 'submodule always visited' ' test_when_finished "git -C super fsmonitor--daemon stop; \ rm -rf super; \ -- cgit v1.2.3