Message ID | 20240521195659.870714-13-gitster@pobox.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Fix various overly aggressive protections in 2.45.1 and friends | expand |
Junio C Hamano <gitster@pobox.com> writes: > This partially reverts f4aa8c8b (fetch/clone: detect dubious > ownership of local repositories, 2024-04-10) that broke typical > read-only use cases (e.g. by git-daemon serving fetches and clones) > where "nobody" who has no write permission serves repositories owned > by others. The function to die upon seeing dubious ownership is > still kept, as there are other users of it, but calls to it from the > generic repository discovery code path, which triggered in cases far > wider than originally intended (i.e. to stop local clones), have > been removed. > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > --- I am inclined to propose dropping this step, actually. cf. https://lore.kernel.org/git/xmqq7cfmaofl.fsf@gitster.g/
Hi Junio, On Tue, 21 May 2024, Junio C Hamano wrote: > Junio C Hamano <gitster@pobox.com> writes: > > > This partially reverts f4aa8c8b (fetch/clone: detect dubious > > ownership of local repositories, 2024-04-10) that broke typical > > read-only use cases (e.g. by git-daemon serving fetches and clones) > > where "nobody" who has no write permission serves repositories owned > > by others. The function to die upon seeing dubious ownership is > > still kept, as there are other users of it, but calls to it from the > > generic repository discovery code path, which triggered in cases far > > wider than originally intended (i.e. to stop local clones), have > > been removed. > > > > Signed-off-by: Junio C Hamano <gitster@pobox.com> > > --- > > I am inclined to propose dropping this step, actually. > > cf. https://lore.kernel.org/git/xmqq7cfmaofl.fsf@gitster.g/ To https://github.com/dscho/git + c6da96aa5f0...f71d7009814 maint-2.39 -> tentative/maint-2.39 (forced update) + fff57b200d1...21dc6c4d521 maint-2.40 -> tentative/maint-2.40 (forced update) + 616450032a0...0d21b3451cd maint-2.41 -> tentative/maint-2.41 (forced update) + b1ea89bc2d6...e9bd0c8f8c4 maint-2.42 -> tentative/maint-2.42 (forced update) + 093c42a6c6b...9926037ce8c maint-2.43 -> tentative/maint-2.43 (forced update) + 3c7a7b923b3...aec5a9bf52c maint-2.44 -> tentative/maint-2.44 (forced update) + aeddcb02756...d3c56966d13 maint-2.45 -> tentative/maint-2.45 (forced update) This command-line comes up with no differences (meaning: you resolved the merge conflicts, even the ones without conflict markers, in the same way as I did, which is good): for i in $(seq 39 45) do case $i in 39) gitster=jc/fix-2.45.1-and-friends-for-2.39;; 45) gitster=jc/fix-2.45.1-and-friends-for-maint;; *) gitster=fixes/2.45.1/2.$i;; esac && git diff gitster/$gitster..dscho/tentative/maint-2.$i -- \ ':(exclude)Documentation/RelNotes/' \ ':(exclude)GIT-VERSION-GEN' \ ':(exclude)RelNotes' done Ciao, Johannes
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > To https://github.com/dscho/git > + c6da96aa5f0...f71d7009814 maint-2.39 -> tentative/maint-2.39 (forced update) > + fff57b200d1...21dc6c4d521 maint-2.40 -> tentative/maint-2.40 (forced update) > + 616450032a0...0d21b3451cd maint-2.41 -> tentative/maint-2.41 (forced update) > + b1ea89bc2d6...e9bd0c8f8c4 maint-2.42 -> tentative/maint-2.42 (forced update) > + 093c42a6c6b...9926037ce8c maint-2.43 -> tentative/maint-2.43 (forced update) > + 3c7a7b923b3...aec5a9bf52c maint-2.44 -> tentative/maint-2.44 (forced update) > + aeddcb02756...d3c56966d13 maint-2.45 -> tentative/maint-2.45 (forced update) > > This command-line comes up with no differences (meaning: you resolved the > merge conflicts, even the ones without conflict markers, in the same way > as I did, which is good): Yeah, but I am afraid that it is a bit too premature to worry about the integration to merge them up. What's your take on the symlink stuff Joey raised recently? Thanks.
diff --git a/path.c b/path.c index d61f70e87d..492e17ad12 100644 --- a/path.c +++ b/path.c @@ -840,7 +840,6 @@ const char *enter_repo(const char *path, int strict) if (!suffix[i]) return NULL; gitfile = read_gitfile(used_path.buf); - die_upon_dubious_ownership(gitfile, NULL, used_path.buf); if (gitfile) { strbuf_reset(&used_path); strbuf_addstr(&used_path, gitfile); @@ -851,7 +850,6 @@ const char *enter_repo(const char *path, int strict) } else { const char *gitfile = read_gitfile(path); - die_upon_dubious_ownership(gitfile, NULL, path); if (gitfile) path = gitfile; if (chdir(path)) diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh index b3d6ddc4bc..a481ed97b7 100755 --- a/t/t0411-clone-from-partial.sh +++ b/t/t0411-clone-from-partial.sh @@ -23,7 +23,7 @@ test_expect_success 'create evil repo' ' >evil/.git/shallow ' -test_expect_success 'local clone must not fetch from promisor remote and execute script' ' +test_expect_failure 'local clone must not fetch from promisor remote and execute script' ' rm -f script-executed && test_must_fail git clone \ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ @@ -33,7 +33,7 @@ test_expect_success 'local clone must not fetch from promisor remote and execute test_path_is_missing script-executed ' -test_expect_success 'clone from file://... must not fetch from promisor remote and execute script' ' +test_expect_failure 'clone from file://... must not fetch from promisor remote and execute script' ' rm -f script-executed && test_must_fail git clone \ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \ @@ -43,7 +43,7 @@ test_expect_success 'clone from file://... must not fetch from promisor remote a test_path_is_missing script-executed ' -test_expect_success 'fetch from file://... must not fetch from promisor remote and execute script' ' +test_expect_failure 'fetch from file://... must not fetch from promisor remote and execute script' ' rm -f script-executed && test_must_fail git fetch \ --upload-pack="GIT_TEST_ASSUME_DIFFERENT_OWNER=true git-upload-pack" \
This partially reverts f4aa8c8b (fetch/clone: detect dubious ownership of local repositories, 2024-04-10) that broke typical read-only use cases (e.g. by git-daemon serving fetches and clones) where "nobody" who has no write permission serves repositories owned by others. The function to die upon seeing dubious ownership is still kept, as there are other users of it, but calls to it from the generic repository discovery code path, which triggered in cases far wider than originally intended (i.e. to stop local clones), have been removed. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- path.c | 2 -- t/t0411-clone-from-partial.sh | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-)