{"thread":{"id":"65134","subject":"[PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch","startedAt":"2026-03-04T16:57:51Z","lastAt":"2026-04-15T18:05:45Z","messageCount":12,"participants":["Paul Tarjan via GitGitGadget","Junio C Hamano","Paul Tarjan","Patrick Steinhardt","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"537791","messageId":"pull.2224.git.git.1772643468305.gitgitgadget@gmail.com","threadId":"65134","inReplyTo":null,"subject":"[PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T16:57:48Z","receivedAt":"2026-03-04T16:57:51Z","isPatch":true,"body":"From: Paul Tarjan <github@paulisageek.com>\n\nfetch_objects() spawns a child `git fetch` to lazily fill in missing\nobjects. That child's index-pack, when it receives a thin pack\ncontaining a REF_DELTA against a still-missing base, explicitly\ncalls promisor_remote_get_direct() — which is fetch_objects() again.\nIf the base is truly unavailable (e.g. because many refs in the\nlocal store point at objects that have been garbage-collected on the\nserver), each recursive lazy-fetch can trigger another, leading to\nunbounded recursion with runaway disk and process consumption.\n\nThe GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add\n--no-lazy-fetch option, 2021-08-31)) already exists at the top of\nfetch_objects(); the missing piece is propagating it into the child\nfetch's environment. Add that propagation so the child's\nindex-pack, if it encounters a REF_DELTA against a missing base,\nhits the guard and fails fast instead of recursing.\n\nDepth-1 lazy fetch (the whole point of fetch_objects()) is\nunaffected: only the child and its descendants see the variable.\nWith negotiationAlgorithm=noop the client advertises no \"have\"\nlines, so a well-behaved server sends requested objects\nun-deltified or deltified only against objects in the same pack;\nthe child's index-pack should never need a depth-2 fetch. If it\ndoes, the server response was broken or the local store is already\ncorrupt, and further fetching would not help.\n\nThis is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy\nfetch in lookup_commit_in_graph(), 2022-07-01) addressed at a\ndifferent entry point.\n\nAdd a test that verifies the child fetch environment contains\nGIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that\nonly one fetch subprocess is spawned.\n\nCc: Jonathan Tan <jonathantanmy@google.com>\nCc: Han Xin <hanxin.hx@bytedance.com>\nCc: Jeff Hostetler <jeffhostetler@github.com>\nCc: Christian Couder <christian.couder@gmail.com>\nSigned-off-by: Paul Tarjan <github@paulisageek.com>\n---\n    promisor-remote: prevent recursive lazy-fetch during index-pack\n    \n    fetch_objects() in promisor-remote.c spawns a child git fetch to lazily\n    fill missing objects. That child's index-pack --fix-thin, when it hits a\n    REF_DELTA against a still-missing base, calls\n    promisor_remote_get_direct() — which is fetch_objects() again. Unbounded\n    recursion.\n    \n    We hit this in production: 276 GB of promisor packs written in 90\n    minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing\n    at GC'd commits. Every thin pack picked a bad delta base, and the\n    recursion fanned out until the mount filled.\n    \n    The fix is one line: propagate GIT_NO_LAZY_FETCH=1 into the child\n    fetch's environment. The guard already exists at the top of\n    fetch_objects() (added by e6d5479e7a, 2021); nothing was setting it in\n    the child. This is the same bug shape that Han Xin's 3a1ea94a49 (2022)\n    closed at lookup_commit_in_graph().\n    \n    Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected —\n    only the child and its descendants see the variable. With\n    negotiationAlgorithm=noop the client advertises no \"have\" lines, so a\n    well-behaved server sends objects un-deltified or deltified only against\n    objects in the same pack. A depth-2 fetch would not help; if the server\n    sends broken thin packs, recursing just makes it worse.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v1\nPull-Request: https://github.com/git/git/pull/2224\n\n promisor-remote.c                           |  7 +++\n t/meson.build                               |  1 +\n t/t0412-promisor-no-lazy-fetch-recursion.sh | 49 +++++++++++++++++++++\n 3 files changed, 57 insertions(+)\n create mode 100755 t/t0412-promisor-no-lazy-fetch-recursion.sh\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 96fa215b06..35c7aab93d 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,\n \tchild.in = -1;\n \tif (repo != the_repository)\n \t\tprepare_other_repo_env(&child.env, repo->gitdir);\n+\t/*\n+\t * Prevent the child's index-pack from recursing back into\n+\t * fetch_objects() when resolving REF_DELTA bases it does not\n+\t * have.  With noop negotiation the server should never need\n+\t * to send such deltas, so a depth-2 fetch would not help.\n+\t */\n+\tstrvec_pushf(&child.env, \"%s=1\", NO_LAZY_FETCH_ENVIRONMENT);\n \tstrvec_pushl(&child.args, \"-c\", \"fetch.negotiationAlgorithm=noop\",\n \t\t     \"fetch\", remote_name, \"--no-tags\",\n \t\t     \"--no-write-fetch-head\", \"--recurse-submodules=no\",\ndiff --git a/t/meson.build b/t/meson.build\nindex e5174ee575..0499533dff 100644\n--- a/t/meson.build\n+++ b/t/meson.build\n@@ -141,6 +141,7 @@ integration_tests = [\n   't0303-credential-external.sh',\n   't0410-partial-clone.sh',\n   't0411-clone-from-partial.sh',\n+  't0412-promisor-no-lazy-fetch-recursion.sh',\n   't0450-txt-doc-vs-help.sh',\n   't0500-progress-display.sh',\n   't0600-reffiles-backend.sh',\ndiff --git a/t/t0412-promisor-no-lazy-fetch-recursion.sh b/t/t0412-promisor-no-lazy-fetch-recursion.sh\nnew file mode 100755\nindex 0000000000..ec203543d4\n--- /dev/null\n+++ b/t/t0412-promisor-no-lazy-fetch-recursion.sh\n@@ -0,0 +1,49 @@\n+#!/bin/sh\n+\n+test_description='promisor-remote: no recursive lazy-fetch\n+\n+Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child\n+fetch environment, so that index-pack cannot recurse back into\n+fetch_objects() when resolving REF_DELTA bases.\n+'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\ttest_create_repo server &&\n+\ttest_commit -C server foo &&\n+\tgit -C server repack -a -d --write-bitmap-index &&\n+\n+\tgit clone \"file://$(pwd)/server\" client &&\n+\tHASH=$(git -C client rev-parse foo) &&\n+\trm -rf client/.git/objects/* &&\n+\n+\tgit -C client config core.repositoryformatversion 1 &&\n+\tgit -C client config extensions.partialclone \"origin\"\n+'\n+\n+test_expect_success 'lazy-fetch spawns only one fetch subprocess' '\n+\tGIT_TRACE=\"$(pwd)/trace\" git -C client cat-file -p \"$HASH\" &&\n+\n+\tgrep \"git fetch\" trace >fetches &&\n+\ttest_line_count = 1 fetches\n+'\n+\n+test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' '\n+\trm -rf client/.git/objects/* &&\n+\n+\t# Install a reference-transaction hook to record the env var\n+\t# as seen by processes inside the child fetch.\n+\ttest_hook -C client reference-transaction <<-\\EOF &&\n+\techo \"$GIT_NO_LAZY_FETCH\" >>../env-in-child\n+\tEOF\n+\n+\trm -f env-in-child &&\n+\tgit -C client cat-file -p \"$HASH\" &&\n+\n+\t# The hook runs inside the child fetch, which should have\n+\t# GIT_NO_LAZY_FETCH=1 in its environment.\n+\tgrep \"^1$\" env-in-child\n+'\n+\n+test_done\n\nbase-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5\n-- \ngitgitgadget\n"},{"id":"537797","messageId":"xmqqikbb8pbd.fsf@gitster.g","threadId":"65134","inReplyTo":"pull.2224.git.git.1772643468305.gitgitgadget@gmail.com","subject":"Re: [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-04T17:41:42Z","receivedAt":"2026-03-04T17:41:45Z","isPatch":true,"body":"\"Paul Tarjan via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Paul Tarjan <github@paulisageek.com>\n>\n> fetch_objects() spawns a child `git fetch` to lazily fill in missing\n> objects. That child's index-pack, when it receives a thin pack\n> containing a REF_DELTA against a still-missing base, explicitly\n> calls promisor_remote_get_direct() — which is fetch_objects() again.\n> If the base is truly unavailable (e.g. because many refs in the\n> local store point at objects that have been garbage-collected on the\n> server), each recursive lazy-fetch can trigger another, leading to\n> unbounded recursion with runaway disk and process consumption.\n>\n> The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add\n> --no-lazy-fetch option, 2021-08-31)) already exists at the top of\n> fetch_objects(); the missing piece is propagating it into the child\n> fetch's environment. Add that propagation so the child's\n> index-pack, if it encounters a REF_DELTA against a missing base,\n> hits the guard and fails fast instead of recursing.\n>\n> Depth-1 lazy fetch (the whole point of fetch_objects()) is\n> unaffected: only the child and its descendants see the variable.\n> With negotiationAlgorithm=noop the client advertises no \"have\"\n> lines, so a well-behaved server sends requested objects\n> un-deltified or deltified only against objects in the same pack;\n> the child's index-pack should never need a depth-2 fetch. If it\n> does, the server response was broken or the local store is already\n> corrupt, and further fetching would not help.\n>\n> This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy\n> fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a\n> different entry point.\n>\n> Add a test that verifies the child fetch environment contains\n> GIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that\n> only one fetch subprocess is spawned.\n>\n> Cc: Jonathan Tan <jonathantanmy@google.com>\n> Cc: Han Xin <hanxin.hx@bytedance.com>\n> Cc: Jeff Hostetler <jeffhostetler@github.com>\n> Cc: Christian Couder <christian.couder@gmail.com>\n\nI would suggest dropping these CC: lines from the proposed log\nmessage.  As far as I can see, they do not have their intended\neffect; [*1*] does not show any of these folks listed on Cc:\n\n*1* https://lore.kernel.org/git/pull.2224.git.git.1772643468305.gitgitgadget@gmail.com/\n\nI am not a GitGitGadget user, but I think ...\n\n> Signed-off-by: Paul Tarjan <github@paulisageek.com>\n> ---\n>     promisor-remote: prevent recursive lazy-fetch during index-pack\n>     \n>     fetch_objects() in promisor-remote.c spawns a child git fetch to lazily\n>     fill missing objects. That child's index-pack --fix-thin, when it hits a\n>     REF_DELTA against a still-missing base, calls\n>     promisor_remote_get_direct() — which is fetch_objects() again. Unbounded\n>     recursion.\n>     \n>     We hit this in production: 276 GB of promisor packs written in 90\n>     minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing\n>     at GC'd commits. Every thin pack picked a bad delta base, and the\n>     recursion fanned out until the mount filled.\n>     \n>     The fix is one line: propagate GIT_NO_LAZY_FETCH=1 into the child\n>     fetch's environment. The guard already exists at the top of\n>     fetch_objects() (added by e6d5479e7a, 2021); nothing was setting it in\n>     the child. This is the same bug shape that Han Xin's 3a1ea94a49 (2022)\n>     closed at lookup_commit_in_graph().\n>     \n>     Depth-1 lazy fetch (the whole point of fetch_objects()) is unaffected —\n>     only the child and its descendants see the variable. With\n>     negotiationAlgorithm=noop the client advertises no \"have\" lines, so a\n>     well-behaved server sends objects un-deltified or deltified only against\n>     objects in the same pack. A depth-2 fetch would not help; if the server\n>     sends broken thin packs, recursing just makes it worse.\n\n... once I heard that the tool expects list of folks to CC: on this\nside, i.e., not in the proposed commit log message, but in the pull\nrequest description.  I also do not see much point in duplicating\nmost of what appears in the proposed log message here after the\nthree dash line, but that is a separate story.\n\nThis is totally an unrelated tangent, but perhaps we'd need a\nbest-practice document/guide for GitGitGadget users, that covers at\nleast the following two things?\n\n * The pull-request message appear under three-dash in the e-mailed\n   patch, where additional information that are not meant to become\n   part of the log message goes.  You do not want to duplicate your\n   commit log message there.\n\n * Do not write Cc: trailers in your commit log message, as\n   GitGitGadget does not pay attention to them.  If you want to\n   specify whom to Cc: your patches, write these in your\n   pull-request message instead, which GitGitGadget does pay\n   attention to.\n\n> diff --git a/promisor-remote.c b/promisor-remote.c\n> index 96fa215b06..35c7aab93d 100644\n> --- a/promisor-remote.c\n> +++ b/promisor-remote.c\n> @@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,\n>  \tchild.in = -1;\n>  \tif (repo != the_repository)\n>  \t\tprepare_other_repo_env(&child.env, repo->gitdir);\n> +\t/*\n> +\t * Prevent the child's index-pack from recursing back into\n> +\t * fetch_objects() when resolving REF_DELTA bases it does not\n> +\t * have.  With noop negotiation the server should never need\n> +\t * to send such deltas, so a depth-2 fetch would not help.\n> +\t */\n> +\tstrvec_pushf(&child.env, \"%s=1\", NO_LAZY_FETCH_ENVIRONMENT);\n>  \tstrvec_pushl(&child.args, \"-c\", \"fetch.negotiationAlgorithm=noop\",\n>  \t\t     \"fetch\", remote_name, \"--no-tags\",\n>  \t\t     \"--no-write-fetch-head\", \"--recurse-submodules=no\",\n\nLooks good.\n\n> diff --git a/t/meson.build b/t/meson.build\n> index e5174ee575..0499533dff 100644\n> --- a/t/meson.build\n> +++ b/t/meson.build\n> @@ -141,6 +141,7 @@ integration_tests = [\n>    't0303-credential-external.sh',\n>    't0410-partial-clone.sh',\n>    't0411-clone-from-partial.sh',\n> +  't0412-promisor-no-lazy-fetch-recursion.sh',\n\nHmph, do we really need an entirely new test script file dedicated\nfor this single liner change, instead of adding to some existing\ntest script that already covers related topics (like promisors and\nlazy fetches from them)?\n\n>    't0450-txt-doc-vs-help.sh',\n>    't0500-progress-display.sh',\n>    't0600-reffiles-backend.sh',\n> diff --git a/t/t0412-promisor-no-lazy-fetch-recursion.sh b/t/t0412-promisor-no-lazy-fetch-recursion.sh\n> new file mode 100755\n> index 0000000000..ec203543d4\n> --- /dev/null\n> +++ b/t/t0412-promisor-no-lazy-fetch-recursion.sh\n> @@ -0,0 +1,49 @@\n> +#!/bin/sh\n> +\n> +test_description='promisor-remote: no recursive lazy-fetch\n> +\n> +Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child\n> +fetch environment, so that index-pack cannot recurse back into\n> +fetch_objects() when resolving REF_DELTA bases.\n> +'\n> +\n> +. ./test-lib.sh\n> +\n> +test_expect_success 'setup' '\n> +\ttest_create_repo server &&\n> +\ttest_commit -C server foo &&\n> +\tgit -C server repack -a -d --write-bitmap-index &&\n> +\n> +\tgit clone \"file://$(pwd)/server\" client &&\n> +\tHASH=$(git -C client rev-parse foo) &&\n> +\trm -rf client/.git/objects/* &&\n> +\n> +\tgit -C client config core.repositoryformatversion 1 &&\n> +\tgit -C client config extensions.partialclone \"origin\"\n> +'\n> +\n> +test_expect_success 'lazy-fetch spawns only one fetch subprocess' '\n> +\tGIT_TRACE=\"$(pwd)/trace\" git -C client cat-file -p \"$HASH\" &&\n> +\n> +\tgrep \"git fetch\" trace >fetches &&\n> +\ttest_line_count = 1 fetches\n> +'\n> +\n> +test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' '\n> +\trm -rf client/.git/objects/* &&\n> +\n> +\t# Install a reference-transaction hook to record the env var\n> +\t# as seen by processes inside the child fetch.\n> +\ttest_hook -C client reference-transaction <<-\\EOF &&\n> +\techo \"$GIT_NO_LAZY_FETCH\" >>../env-in-child\n> +\tEOF\n> +\n> +\trm -f env-in-child &&\n> +\tgit -C client cat-file -p \"$HASH\" &&\n> +\n> +\t# The hook runs inside the child fetch, which should have\n> +\t# GIT_NO_LAZY_FETCH=1 in its environment.\n> +\tgrep \"^1$\" env-in-child\n> +'\n> +\n> +test_done\n>\n> base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5\n"},{"id":"537822","messageId":"20260304182057.26463-1-github@paulisageek.com","threadId":"65134","inReplyTo":"xmqqikbb8pbd.fsf@gitster.g","subject":"Re: [PATCH] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan","fromEmail":"paul@paultarjan.com","sentAt":"2026-03-04T18:20:57Z","receivedAt":"2026-03-04T18:20:59Z","isPatch":true,"body":"On Tue, Mar 4, 2026, Junio C Hamano wrote:\n> I would suggest dropping these CC: lines from the proposed log\n> message.  As far as I can see, they do not have their intended\n> effect; [*1*] does not show any of these folks listed on Cc:\n\nDone, moved them to the PR description for GitGitGadget to pick up.\n\n> I also do not see much point in duplicating\n> most of what appears in the proposed log message here after the\n> three dash line, but that is a separate story.\n\nCleaned up the PR description to avoid the duplication.\n\n> Hmph, do we really need an entirely new test script file dedicated\n> for this single liner change, instead of adding to some existing\n> test script that already covers related topics (like promisors and\n> lazy fetches from them)?\n\nMoved the test into t0411-clone-from-partial.sh, which already has\nthe other lazy-fetch tests. Dropped the separate t0412 file and the\nmeson.build entry.\n"},{"id":"537823","messageId":"pull.2224.v2.git.git.1772648846009.gitgitgadget@gmail.com","threadId":"65134","inReplyTo":"pull.2224.git.git.1772643468305.gitgitgadget@gmail.com","subject":"[PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-04T18:27:25Z","receivedAt":"2026-03-04T18:27:28Z","isPatch":true,"body":"From: Paul Tarjan <github@paulisageek.com>\n\nfetch_objects() spawns a child `git fetch` to lazily fill in missing\nobjects. That child's index-pack, when it receives a thin pack\ncontaining a REF_DELTA against a still-missing base, explicitly\ncalls promisor_remote_get_direct() — which is fetch_objects() again.\nIf the base is truly unavailable (e.g. because many refs in the\nlocal store point at objects that have been garbage-collected on the\nserver), each recursive lazy-fetch can trigger another, leading to\nunbounded recursion with runaway disk and process consumption.\n\nThe GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add\n--no-lazy-fetch option, 2021-08-31)) already exists at the top of\nfetch_objects(); the missing piece is propagating it into the child\nfetch's environment. Add that propagation so the child's\nindex-pack, if it encounters a REF_DELTA against a missing base,\nhits the guard and fails fast instead of recursing.\n\nDepth-1 lazy fetch (the whole point of fetch_objects()) is\nunaffected: only the child and its descendants see the variable.\nWith negotiationAlgorithm=noop the client advertises no \"have\"\nlines, so a well-behaved server sends requested objects\nun-deltified or deltified only against objects in the same pack;\nthe child's index-pack should never need a depth-2 fetch. If it\ndoes, the server response was broken or the local store is already\ncorrupt, and further fetching would not help.\n\nThis is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy\nfetch in lookup_commit_in_graph(), 2022-07-01) addressed at a\ndifferent entry point.\n\nAdd a test that verifies the child fetch environment contains\nGIT_NO_LAZY_FETCH=1 via a reference-transaction hook.\n\nSigned-off-by: Paul Tarjan <github@paulisageek.com>\n---\n    promisor-remote: prevent recursive lazy-fetch during index-pack\n    \n    Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by\n    fetch_objects() so that index-pack cannot recurse back into lazy-fetch\n    when resolving REF_DELTA bases.\n    \n    We hit this in production: 276 GB of promisor packs written in 90\n    minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing\n    at GC'd commits.\n    \n    Changes since v1:\n    \n     * Dropped CC: trailers from commit message (moved here for\n       GitGitGadget)\n     * Moved test into t0411-clone-from-partial.sh instead of a new file\n     * Removed duplicate commit-message summary from PR description\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v2\nPull-Request: https://github.com/git/git/pull/2224\n\nRange-diff vs v1:\n\n 1:  7b723441f4 ! 1:  907ca7a0ac promisor-remote: prevent lazy-fetch recursion in child fetch\n     @@ Commit message\n          different entry point.\n      \n          Add a test that verifies the child fetch environment contains\n     -    GIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that\n     -    only one fetch subprocess is spawned.\n     +    GIT_NO_LAZY_FETCH=1 via a reference-transaction hook.\n      \n     -    Cc: Jonathan Tan <jonathantanmy@google.com>\n     -    Cc: Han Xin <hanxin.hx@bytedance.com>\n     -    Cc: Jeff Hostetler <jeffhostetler@github.com>\n     -    Cc: Christian Couder <christian.couder@gmail.com>\n          Signed-off-by: Paul Tarjan <github@paulisageek.com>\n      \n       ## promisor-remote.c ##\n     @@ promisor-remote.c: static int fetch_objects(struct repository *repo,\n       \t\t     \"fetch\", remote_name, \"--no-tags\",\n       \t\t     \"--no-write-fetch-head\", \"--recurse-submodules=no\",\n      \n     - ## t/meson.build ##\n     -@@ t/meson.build: integration_tests = [\n     -   't0303-credential-external.sh',\n     -   't0410-partial-clone.sh',\n     -   't0411-clone-from-partial.sh',\n     -+  't0412-promisor-no-lazy-fetch-recursion.sh',\n     -   't0450-txt-doc-vs-help.sh',\n     -   't0500-progress-display.sh',\n     -   't0600-reffiles-backend.sh',\n     -\n     - ## t/t0412-promisor-no-lazy-fetch-recursion.sh (new) ##\n     -@@\n     -+#!/bin/sh\n     -+\n     -+test_description='promisor-remote: no recursive lazy-fetch\n     -+\n     -+Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child\n     -+fetch environment, so that index-pack cannot recurse back into\n     -+fetch_objects() when resolving REF_DELTA bases.\n     -+'\n     -+\n     -+. ./test-lib.sh\n     -+\n     -+test_expect_success 'setup' '\n     -+\ttest_create_repo server &&\n     -+\ttest_commit -C server foo &&\n     -+\tgit -C server repack -a -d --write-bitmap-index &&\n     + ## t/t0411-clone-from-partial.sh ##\n     +@@ t/t0411-clone-from-partial.sh: test_expect_success 'promisor lazy-fetching can be re-enabled' '\n     + \ttest_path_is_file script-executed\n     + '\n     + \n     ++test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' '\n     ++\ttest_create_repo nolazy-server &&\n     ++\ttest_commit -C nolazy-server foo &&\n     ++\tgit -C nolazy-server repack -a -d --write-bitmap-index &&\n      +\n     -+\tgit clone \"file://$(pwd)/server\" client &&\n     -+\tHASH=$(git -C client rev-parse foo) &&\n     -+\trm -rf client/.git/objects/* &&\n     -+\n     -+\tgit -C client config core.repositoryformatversion 1 &&\n     -+\tgit -C client config extensions.partialclone \"origin\"\n     -+'\n     -+\n     -+test_expect_success 'lazy-fetch spawns only one fetch subprocess' '\n     -+\tGIT_TRACE=\"$(pwd)/trace\" git -C client cat-file -p \"$HASH\" &&\n     -+\n     -+\tgrep \"git fetch\" trace >fetches &&\n     -+\ttest_line_count = 1 fetches\n     -+'\n     ++\tgit clone \"file://$(pwd)/nolazy-server\" nolazy-client &&\n     ++\tHASH=$(git -C nolazy-client rev-parse foo) &&\n     ++\trm -rf nolazy-client/.git/objects/* &&\n      +\n     -+test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' '\n     -+\trm -rf client/.git/objects/* &&\n     ++\tgit -C nolazy-client config core.repositoryformatversion 1 &&\n     ++\tgit -C nolazy-client config extensions.partialclone \"origin\" &&\n      +\n      +\t# Install a reference-transaction hook to record the env var\n      +\t# as seen by processes inside the child fetch.\n     -+\ttest_hook -C client reference-transaction <<-\\EOF &&\n     ++\ttest_hook -C nolazy-client reference-transaction <<-\\EOF &&\n      +\techo \"$GIT_NO_LAZY_FETCH\" >>../env-in-child\n      +\tEOF\n      +\n      +\trm -f env-in-child &&\n     -+\tgit -C client cat-file -p \"$HASH\" &&\n     ++\tgit -C nolazy-client cat-file -p \"$HASH\" &&\n      +\n      +\t# The hook runs inside the child fetch, which should have\n      +\t# GIT_NO_LAZY_FETCH=1 in its environment.\n      +\tgrep \"^1$\" env-in-child\n      +'\n      +\n     -+test_done\n     + test_done\n\n\n promisor-remote.c             |  7 +++++++\n t/t0411-clone-from-partial.sh | 26 ++++++++++++++++++++++++++\n 2 files changed, 33 insertions(+)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 96fa215b06..35c7aab93d 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,\n \tchild.in = -1;\n \tif (repo != the_repository)\n \t\tprepare_other_repo_env(&child.env, repo->gitdir);\n+\t/*\n+\t * Prevent the child's index-pack from recursing back into\n+\t * fetch_objects() when resolving REF_DELTA bases it does not\n+\t * have.  With noop negotiation the server should never need\n+\t * to send such deltas, so a depth-2 fetch would not help.\n+\t */\n+\tstrvec_pushf(&child.env, \"%s=1\", NO_LAZY_FETCH_ENVIRONMENT);\n \tstrvec_pushl(&child.args, \"-c\", \"fetch.negotiationAlgorithm=noop\",\n \t\t     \"fetch\", remote_name, \"--no-tags\",\n \t\t     \"--no-write-fetch-head\", \"--recurse-submodules=no\",\ndiff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh\nindex 9e6bca5625..10a829fb80 100755\n--- a/t/t0411-clone-from-partial.sh\n+++ b/t/t0411-clone-from-partial.sh\n@@ -78,4 +78,30 @@ test_expect_success 'promisor lazy-fetching can be re-enabled' '\n \ttest_path_is_file script-executed\n '\n \n+test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' '\n+\ttest_create_repo nolazy-server &&\n+\ttest_commit -C nolazy-server foo &&\n+\tgit -C nolazy-server repack -a -d --write-bitmap-index &&\n+\n+\tgit clone \"file://$(pwd)/nolazy-server\" nolazy-client &&\n+\tHASH=$(git -C nolazy-client rev-parse foo) &&\n+\trm -rf nolazy-client/.git/objects/* &&\n+\n+\tgit -C nolazy-client config core.repositoryformatversion 1 &&\n+\tgit -C nolazy-client config extensions.partialclone \"origin\" &&\n+\n+\t# Install a reference-transaction hook to record the env var\n+\t# as seen by processes inside the child fetch.\n+\ttest_hook -C nolazy-client reference-transaction <<-\\EOF &&\n+\techo \"$GIT_NO_LAZY_FETCH\" >>../env-in-child\n+\tEOF\n+\n+\trm -f env-in-child &&\n+\tgit -C nolazy-client cat-file -p \"$HASH\" &&\n+\n+\t# The hook runs inside the child fetch, which should have\n+\t# GIT_NO_LAZY_FETCH=1 in its environment.\n+\tgrep \"^1$\" env-in-child\n+'\n+\n test_done\n\nbase-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5\n-- \ngitgitgadget\n"},{"id":"538578","messageId":"abFJhFhHLhS4qdrM@pks.im","threadId":"65134","inReplyTo":"pull.2224.v2.git.git.1772648846009.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-11T10:52:52Z","receivedAt":"2026-03-11T10:52:59Z","isPatch":true,"body":"On Wed, Mar 04, 2026 at 06:27:25PM +0000, Paul Tarjan via GitGitGadget wrote:\n> From: Paul Tarjan <github@paulisageek.com>\n> \n> fetch_objects() spawns a child `git fetch` to lazily fill in missing\n> objects. That child's index-pack, when it receives a thin pack\n> containing a REF_DELTA against a still-missing base, explicitly\n> calls promisor_remote_get_direct() — which is fetch_objects() again.\n> If the base is truly unavailable (e.g. because many refs in the\n> local store point at objects that have been garbage-collected on the\n> server), each recursive lazy-fetch can trigger another, leading to\n> unbounded recursion with runaway disk and process consumption.\n\nIs this a theoretical concern or a practical one? I would expect that\nbackfill fetches never cause the server side to send a pack with\nREF_DELTA objects to nonexistent objects. And if they did they are\nbroken.\n\n> The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add\n> --no-lazy-fetch option, 2021-08-31)) already exists at the top of\n> fetch_objects(); the missing piece is propagating it into the child\n> fetch's environment. Add that propagation so the child's\n> index-pack, if it encounters a REF_DELTA against a missing base,\n> hits the guard and fails fast instead of recursing.\n> \n> Depth-1 lazy fetch (the whole point of fetch_objects()) is\n> unaffected: only the child and its descendants see the variable.\n> With negotiationAlgorithm=noop the client advertises no \"have\"\n> lines, so a well-behaved server sends requested objects\n> un-deltified or deltified only against objects in the same pack;\n> the child's index-pack should never need a depth-2 fetch. If it\n> does, the server response was broken or the local store is already\n> corrupt, and further fetching would not help.\n\nExactly, this here matches my understanding. The backfill fetches don't\nperform negotiation, so we shouldn't ever see a thin pack in the first\nplace. What I don't yet understand is your comment about the depth-2\nfetch -- when would we ever do that?\n\n> This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy\n> fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a\n> different entry point.\n\nI dunno, I think it's quite different overall. In the mentioned commit\nwe protect against a stale commit-graph, which is something that is\nquite plausible to happen on the client side. But here we protect us\nagainst a remote side that sends a packfile that violates specs, as far\nas I understand.\n\n> Add a test that verifies the child fetch environment contains\n> GIT_NO_LAZY_FETCH=1 via a reference-transaction hook.\n\nHm. Can we craft a test that shows us the resulting failure in practice?\nTesting for the environment variable feels like a bad proxy to me, as\nI'd rather want to learn how Git would fail now.\n\n> Signed-off-by: Paul Tarjan <github@paulisageek.com>\n> ---\n>     promisor-remote: prevent recursive lazy-fetch during index-pack\n>     \n>     Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by\n>     fetch_objects() so that index-pack cannot recurse back into lazy-fetch\n>     when resolving REF_DELTA bases.\n>     \n>     We hit this in production: 276 GB of promisor packs written in 90\n>     minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing\n>     at GC'd commits.\n\nOkay, so this seems to be an issue that can be hit in the wild. But I\nhave to wonder whether this really is a bug on the client-side, or\nwhether this is a bug that actually sits on your server. So ultimately:\nwhy does the server send REF_DELTA objects in the first place? Is it\nusing git-upload-pack(1), or is it using a different implementation of\nGit to serve data?\n\nNote that I'm not arguing that we shouldn't have protection on the\nclient, too. But I'd first like to understand whether there is a bug\nlurking somewhere that causes us to send invalid packfiles.\n\nPatrick\n"},{"id":"538607","messageId":"20260311141846.12315-1-github@paulisageek.com","threadId":"65134","inReplyTo":"abFJhFhHLhS4qdrM@pks.im","subject":"Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan","fromEmail":"paul@paultarjan.com","sentAt":"2026-03-11T14:18:46Z","receivedAt":"2026-03-11T14:18:50Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Is this a theoretical concern or a practical one? I would expect that\n> backfill fetches never cause the server side to send a pack with\n> REF_DELTA objects to nonexistent objects. And if they did they are\n> broken.\n\nPractical. We hit this at Anthropic: 276 GB of promisor packs written\nby `git maintenance --task=prefetch` in 90 minutes against a ~10 GB\nmonorepo with ~61K stale prefetch refs pointing at GC'd commits.\n\n> Exactly, this here matches my understanding. The backfill fetches don't\n> perform negotiation, so we shouldn't ever see a thin pack in the first\n> place. What I don't yet understand is your comment about the depth-2\n> fetch -- when would we ever do that?\n\nThe code path already exists and is tested: t5616 line 832 (\"tolerate\nserver sending REF_DELTA against missing promisor objects\") creates\nexactly this scenario. index-pack's fix_unresolved_deltas() calls\npromisor_remote_get_direct() when it encounters a REF_DELTA against a\nmissing base (builtin/index-pack.c:1508). That's the depth-2 fetch.\n\nWith noop negotiation a well-behaved server shouldn't send REF_DELTA\nagainst objects the client doesn't have. But partial clones with\nblob:none mean the client is missing most blobs, and if the server\nsends a thin pack deltified against one of those filtered-out blobs,\nindex-pack will try to fetch the base.\n\n> I dunno, I think it's quite different overall. In the mentioned commit\n> we protect against a stale commit-graph, which is something that is\n> quite plausible to happen on the client side. But here we protect us\n> against a remote side that sends a packfile that violates specs, as far\n> as I understand.\n\nFair point. The commit-graph case is purely client-side corruption,\nwhile this requires a misbehaving server. The bug shape is the same\n(unbounded recursion through fetch_objects()) but the trigger is\ndifferent. I'll drop the comparison in the next version.\n\n> Hm. Can we craft a test that shows us the resulting failure in practice?\n> Testing for the environment variable feels like a bad proxy to me, as\n> I'd rather want to learn how Git would fail now.\n\nGood point. Reworked the test in v3. It now injects a thin pack\ncontaining a REF_DELTA against a missing base via HTTP (using the\nreplace_packfile pattern from t5616). This triggers the actual\nrecursion path: index-pack encounters the missing base, calls\npromisor_remote_get_direct(), which hits the GIT_NO_LAZY_FETCH=1\nguard and fails with \"lazy fetching disabled\". Without the fix,\nthe depth-2 fetch would proceed and potentially recurse.\n\n> Okay, so this seems to be an issue that can be hit in the wild. But I\n> have to wonder whether this really is a bug on the client-side, or\n> whether this is a bug that actually sits on your server. So ultimately:\n> why does the server send REF_DELTA objects in the first place? Is it\n> using git-upload-pack(1), or is it using a different implementation of\n> Git to serve data?\n\nThe server is GitHub. I did a blob:none partial clone and after some\nfurther git operations ended up in this state. I don't have\nserver-side data on why it sent REF_DELTAs against missing bases.\n\n> Note that I'm not arguing that we shouldn't have protection on the\n> client, too. But I'd first like to understand whether there is a bug\n> lurking somewhere that causes us to send invalid packfiles.\n\nAgreed, there may well be a server-side bug here. Regardless, the\nclient should fail fast rather than consume unbounded resources.\n"},{"id":"538608","messageId":"pull.2224.v3.git.git.1773238778894.gitgitgadget@gmail.com","threadId":"65134","inReplyTo":"pull.2224.v2.git.git.1772648846009.gitgitgadget@gmail.com","subject":"[PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-11T14:19:38Z","receivedAt":"2026-03-11T14:19:41Z","isPatch":true,"body":"From: Paul Tarjan <github@paulisageek.com>\n\nfetch_objects() spawns a child `git fetch` to lazily fill in missing\nobjects. That child's index-pack, when it receives a thin pack\ncontaining a REF_DELTA against a still-missing base, calls\npromisor_remote_get_direct() -- which is fetch_objects() again.\n\nWith negotiationAlgorithm=noop the client advertises no \"have\"\nlines, so a well-behaved server sends requested objects\nun-deltified or deltified only against objects in the same pack.\nA server that nevertheless sends REF_DELTA against a base the\nclient does not have is misbehaving; however the client should\nnot recurse unboundedly in response.\n\nPropagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment\nso that if the child's index-pack encounters such a REF_DELTA, it\nhits the existing guard at the top of fetch_objects() and fails\nfast instead of recursing.  Depth-1 lazy fetch (the whole point\nof fetch_objects()) is unaffected: only the child and its\ndescendants see the variable.\n\nAdd a test that injects a thin pack containing a REF_DELTA against\na missing base via HTTP, triggering the recursion path through\nindex-pack's promisor_remote_get_direct() call.  With the fix, the\nchild's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the\ndepth-2 fetch with a \"lazy fetching disabled\" warning.\n\nSigned-off-by: Paul Tarjan <github@paulisageek.com>\n---\n    promisor-remote: prevent recursive lazy-fetch during index-pack\n    \n    Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by\n    fetch_objects() so that index-pack cannot recurse back into lazy-fetch\n    when resolving REF_DELTA bases.\n    \n    We hit this in production: 276 GB of promisor packs written in 90\n    minutes against a ~10 GB monorepo with ~61K stale prefetch refs pointing\n    at GC'd commits.\n    \n    Changes since v2:\n    \n     * Replaced env-var-proxy test with behavioral test that injects a thin\n       pack containing a REF_DELTA against a missing base via HTTP,\n       triggering the actual recursion path through index-pack's\n       promisor_remote_get_direct() call\n     * Moved test from t0411-clone-from-partial.sh to t5616-partial-clone.sh\n       (requires HTTP infrastructure)\n     * Dropped commit-graph comparison from commit message per review\n       feedback\n    \n    Changes since v1:\n    \n     * Dropped CC: trailers from commit message (moved here for\n       GitGitGadget)\n     * Moved test into t0411-clone-from-partial.sh instead of a new file\n     * Removed duplicate commit-message summary from PR description\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v3\nPull-Request: https://github.com/git/git/pull/2224\n\nRange-diff vs v2:\n\n 1:  907ca7a0ac ! 1:  d58ccb858f promisor-remote: prevent lazy-fetch recursion in child fetch\n     @@ Commit message\n      \n          fetch_objects() spawns a child `git fetch` to lazily fill in missing\n          objects. That child's index-pack, when it receives a thin pack\n     -    containing a REF_DELTA against a still-missing base, explicitly\n     -    calls promisor_remote_get_direct() — which is fetch_objects() again.\n     -    If the base is truly unavailable (e.g. because many refs in the\n     -    local store point at objects that have been garbage-collected on the\n     -    server), each recursive lazy-fetch can trigger another, leading to\n     -    unbounded recursion with runaway disk and process consumption.\n     +    containing a REF_DELTA against a still-missing base, calls\n     +    promisor_remote_get_direct() -- which is fetch_objects() again.\n      \n     -    The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add\n     -    --no-lazy-fetch option, 2021-08-31)) already exists at the top of\n     -    fetch_objects(); the missing piece is propagating it into the child\n     -    fetch's environment. Add that propagation so the child's\n     -    index-pack, if it encounters a REF_DELTA against a missing base,\n     -    hits the guard and fails fast instead of recursing.\n     -\n     -    Depth-1 lazy fetch (the whole point of fetch_objects()) is\n     -    unaffected: only the child and its descendants see the variable.\n          With negotiationAlgorithm=noop the client advertises no \"have\"\n          lines, so a well-behaved server sends requested objects\n     -    un-deltified or deltified only against objects in the same pack;\n     -    the child's index-pack should never need a depth-2 fetch. If it\n     -    does, the server response was broken or the local store is already\n     -    corrupt, and further fetching would not help.\n     +    un-deltified or deltified only against objects in the same pack.\n     +    A server that nevertheless sends REF_DELTA against a base the\n     +    client does not have is misbehaving; however the client should\n     +    not recurse unboundedly in response.\n      \n     -    This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy\n     -    fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a\n     -    different entry point.\n     +    Propagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment\n     +    so that if the child's index-pack encounters such a REF_DELTA, it\n     +    hits the existing guard at the top of fetch_objects() and fails\n     +    fast instead of recursing.  Depth-1 lazy fetch (the whole point\n     +    of fetch_objects()) is unaffected: only the child and its\n     +    descendants see the variable.\n      \n     -    Add a test that verifies the child fetch environment contains\n     -    GIT_NO_LAZY_FETCH=1 via a reference-transaction hook.\n     +    Add a test that injects a thin pack containing a REF_DELTA against\n     +    a missing base via HTTP, triggering the recursion path through\n     +    index-pack's promisor_remote_get_direct() call.  With the fix, the\n     +    child's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the\n     +    depth-2 fetch with a \"lazy fetching disabled\" warning.\n      \n          Signed-off-by: Paul Tarjan <github@paulisageek.com>\n      \n     @@ promisor-remote.c: static int fetch_objects(struct repository *repo,\n       \t\t     \"fetch\", remote_name, \"--no-tags\",\n       \t\t     \"--no-write-fetch-head\", \"--recurse-submodules=no\",\n      \n     - ## t/t0411-clone-from-partial.sh ##\n     -@@ t/t0411-clone-from-partial.sh: test_expect_success 'promisor lazy-fetching can be re-enabled' '\n     - \ttest_path_is_file script-executed\n     + ## t/t5616-partial-clone.sh ##\n     +@@ t/t5616-partial-clone.sh: test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against\n     + \t! test -e \"$HTTPD_ROOT_PATH/one-time-script\"\n       '\n       \n     -+test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' '\n     -+\ttest_create_repo nolazy-server &&\n     -+\ttest_commit -C nolazy-server foo &&\n     -+\tgit -C nolazy-server repack -a -d --write-bitmap-index &&\n     ++test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' '\n     ++\tSERVER=\"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n     ++\trm -rf \"$SERVER\" repo &&\n     ++\ttest_create_repo \"$SERVER\" &&\n     ++\ttest_config -C \"$SERVER\" uploadpack.allowfilter 1 &&\n     ++\ttest_config -C \"$SERVER\" uploadpack.allowanysha1inwant 1 &&\n     ++\n     ++\t# Create a commit with 2 blobs to be used as delta base and content.\n     ++\tfor i in $(test_seq 10)\n     ++\tdo\n     ++\t\techo \"this is a line\" >>\"$SERVER/foo.txt\" &&\n     ++\t\techo \"this is another line\" >>\"$SERVER/bar.txt\" || return 1\n     ++\tdone &&\n     ++\tgit -C \"$SERVER\" add foo.txt bar.txt &&\n     ++\tgit -C \"$SERVER\" commit -m initial &&\n     ++\tBLOB_FOO=$(git -C \"$SERVER\" rev-parse HEAD:foo.txt) &&\n     ++\tBLOB_BAR=$(git -C \"$SERVER\" rev-parse HEAD:bar.txt) &&\n      +\n     -+\tgit clone \"file://$(pwd)/nolazy-server\" nolazy-client &&\n     -+\tHASH=$(git -C nolazy-client rev-parse foo) &&\n     -+\trm -rf nolazy-client/.git/objects/* &&\n     ++\t# Partial clone with blob:none. The client has commits and\n     ++\t# trees but no blobs.\n     ++\ttest_config -C \"$SERVER\" protocol.version 2 &&\n     ++\tgit -c protocol.version=2 clone --no-checkout \\\n     ++\t\t--filter=blob:none $HTTPD_URL/one_time_script/server repo &&\n      +\n     -+\tgit -C nolazy-client config core.repositoryformatversion 1 &&\n     -+\tgit -C nolazy-client config extensions.partialclone \"origin\" &&\n     ++\t# Sanity check: client does not have either blob locally.\n     ++\tgit -C repo rev-list --objects --ignore-missing \\\n     ++\t\t-- $BLOB_FOO >objlist &&\n     ++\ttest_line_count = 0 objlist &&\n      +\n     -+\t# Install a reference-transaction hook to record the env var\n     -+\t# as seen by processes inside the child fetch.\n     -+\ttest_hook -C nolazy-client reference-transaction <<-\\EOF &&\n     -+\techo \"$GIT_NO_LAZY_FETCH\" >>../env-in-child\n     ++\t# Craft a thin pack where BLOB_FOO is a REF_DELTA against\n     ++\t# BLOB_BAR. Since the client has neither blob (blob:none\n     ++\t# filter), the delta base will be missing. This simulates a\n     ++\t# misbehaving server that sends REF_DELTA against an object\n     ++\t# the client does not have.\n     ++\ttest-tool -C \"$SERVER\" pack-deltas --num-objects=1 >thin.pack <<-EOF &&\n     ++\tREF_DELTA $BLOB_FOO $BLOB_BAR\n      +\tEOF\n      +\n     -+\trm -f env-in-child &&\n     -+\tgit -C nolazy-client cat-file -p \"$HASH\" &&\n     ++\treplace_packfile thin.pack &&\n      +\n     -+\t# The hook runs inside the child fetch, which should have\n     -+\t# GIT_NO_LAZY_FETCH=1 in its environment.\n     -+\tgrep \"^1$\" env-in-child\n     ++\t# Trigger a lazy fetch for BLOB_FOO. The child fetch spawned\n     ++\t# by fetch_objects() receives our crafted thin pack. Its\n     ++\t# index-pack encounters the missing delta base (BLOB_BAR) and\n     ++\t# tries to lazy-fetch it via promisor_remote_get_direct().\n     ++\t#\n     ++\t# With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1\n     ++\t# to the child, so the depth-2 fetch is blocked and we see the\n     ++\t# \"lazy fetching disabled\" warning. The object cannot be\n     ++\t# resolved, so cat-file fails.\n     ++\t#\n     ++\t# Without the fix: the depth-2 fetch would proceed, potentially\n     ++\t# recursing unboundedly with a persistently misbehaving server.\n     ++\ttest_must_fail git -C repo -c protocol.version=2 \\\n     ++\t\tcat-file -p $BLOB_FOO 2>err &&\n     ++\ttest_grep \"lazy fetching disabled\" err &&\n     ++\n     ++\t# Ensure that the one-time-script was used.\n     ++\t! test -e \"$HTTPD_ROOT_PATH/one-time-script\"\n      +'\n      +\n     - test_done\n     + # DO NOT add non-httpd-specific tests here, because the last part of this\n     + # test script is only executed when httpd is available and enabled.\n     + \n\n\n promisor-remote.c        |  7 +++++\n t/t5616-partial-clone.sh | 60 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 67 insertions(+)\n\ndiff --git a/promisor-remote.c b/promisor-remote.c\nindex 96fa215b06..35c7aab93d 100644\n--- a/promisor-remote.c\n+++ b/promisor-remote.c\n@@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,\n \tchild.in = -1;\n \tif (repo != the_repository)\n \t\tprepare_other_repo_env(&child.env, repo->gitdir);\n+\t/*\n+\t * Prevent the child's index-pack from recursing back into\n+\t * fetch_objects() when resolving REF_DELTA bases it does not\n+\t * have.  With noop negotiation the server should never need\n+\t * to send such deltas, so a depth-2 fetch would not help.\n+\t */\n+\tstrvec_pushf(&child.env, \"%s=1\", NO_LAZY_FETCH_ENVIRONMENT);\n \tstrvec_pushl(&child.args, \"-c\", \"fetch.negotiationAlgorithm=noop\",\n \t\t     \"fetch\", remote_name, \"--no-tags\",\n \t\t     \"--no-write-fetch-head\", \"--recurse-submodules=no\",\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 1e354e057f..27f131c8d9 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -907,6 +907,66 @@ test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against\n \t! test -e \"$HTTPD_ROOT_PATH/one-time-script\"\n '\n \n+test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' '\n+\tSERVER=\"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\trm -rf \"$SERVER\" repo &&\n+\ttest_create_repo \"$SERVER\" &&\n+\ttest_config -C \"$SERVER\" uploadpack.allowfilter 1 &&\n+\ttest_config -C \"$SERVER\" uploadpack.allowanysha1inwant 1 &&\n+\n+\t# Create a commit with 2 blobs to be used as delta base and content.\n+\tfor i in $(test_seq 10)\n+\tdo\n+\t\techo \"this is a line\" >>\"$SERVER/foo.txt\" &&\n+\t\techo \"this is another line\" >>\"$SERVER/bar.txt\" || return 1\n+\tdone &&\n+\tgit -C \"$SERVER\" add foo.txt bar.txt &&\n+\tgit -C \"$SERVER\" commit -m initial &&\n+\tBLOB_FOO=$(git -C \"$SERVER\" rev-parse HEAD:foo.txt) &&\n+\tBLOB_BAR=$(git -C \"$SERVER\" rev-parse HEAD:bar.txt) &&\n+\n+\t# Partial clone with blob:none. The client has commits and\n+\t# trees but no blobs.\n+\ttest_config -C \"$SERVER\" protocol.version 2 &&\n+\tgit -c protocol.version=2 clone --no-checkout \\\n+\t\t--filter=blob:none $HTTPD_URL/one_time_script/server repo &&\n+\n+\t# Sanity check: client does not have either blob locally.\n+\tgit -C repo rev-list --objects --ignore-missing \\\n+\t\t-- $BLOB_FOO >objlist &&\n+\ttest_line_count = 0 objlist &&\n+\n+\t# Craft a thin pack where BLOB_FOO is a REF_DELTA against\n+\t# BLOB_BAR. Since the client has neither blob (blob:none\n+\t# filter), the delta base will be missing. This simulates a\n+\t# misbehaving server that sends REF_DELTA against an object\n+\t# the client does not have.\n+\ttest-tool -C \"$SERVER\" pack-deltas --num-objects=1 >thin.pack <<-EOF &&\n+\tREF_DELTA $BLOB_FOO $BLOB_BAR\n+\tEOF\n+\n+\treplace_packfile thin.pack &&\n+\n+\t# Trigger a lazy fetch for BLOB_FOO. The child fetch spawned\n+\t# by fetch_objects() receives our crafted thin pack. Its\n+\t# index-pack encounters the missing delta base (BLOB_BAR) and\n+\t# tries to lazy-fetch it via promisor_remote_get_direct().\n+\t#\n+\t# With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1\n+\t# to the child, so the depth-2 fetch is blocked and we see the\n+\t# \"lazy fetching disabled\" warning. The object cannot be\n+\t# resolved, so cat-file fails.\n+\t#\n+\t# Without the fix: the depth-2 fetch would proceed, potentially\n+\t# recursing unboundedly with a persistently misbehaving server.\n+\ttest_must_fail git -C repo -c protocol.version=2 \\\n+\t\tcat-file -p $BLOB_FOO 2>err &&\n+\ttest_grep \"lazy fetching disabled\" err &&\n+\n+\t# Ensure that the one-time-script was used.\n+\t! test -e \"$HTTPD_ROOT_PATH/one-time-script\"\n+'\n+\n # DO NOT add non-httpd-specific tests here, because the last part of this\n # test script is only executed when httpd is available and enabled.\n \n\nbase-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5\n-- \ngitgitgadget\n"},{"id":"538724","messageId":"abJqySqfdFoY8cEu@pks.im","threadId":"65134","inReplyTo":"20260311141846.12315-1-github@paulisageek.com","subject":"Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-12T07:27:05Z","receivedAt":"2026-03-12T07:27:12Z","isPatch":true,"body":"On Wed, Mar 11, 2026 at 08:18:46AM -0600, Paul Tarjan wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > Is this a theoretical concern or a practical one? I would expect that\n> > backfill fetches never cause the server side to send a pack with\n> > REF_DELTA objects to nonexistent objects. And if they did they are\n> > broken.\n> \n> Practical. We hit this at Anthropic: 276 GB of promisor packs written\n> by `git maintenance --task=prefetch` in 90 minutes against a ~10 GB\n> monorepo with ~61K stale prefetch refs pointing at GC'd commits.\n\nI must be misunderstanding something here, but how is it that a commit\ncan be garbage collected if a ref points to it? That shouldn't ever\nhappen, as reachable commits should not be pruned.\n\nOr do you mean to say that the commits don't exist on the server side\nanymore?\n\n> > Hm. Can we craft a test that shows us the resulting failure in practice?\n> > Testing for the environment variable feels like a bad proxy to me, as\n> > I'd rather want to learn how Git would fail now.\n> \n> Good point. Reworked the test in v3. It now injects a thin pack\n> containing a REF_DELTA against a missing base via HTTP (using the\n> replace_packfile pattern from t5616). This triggers the actual\n> recursion path: index-pack encounters the missing base, calls\n> promisor_remote_get_direct(), which hits the GIT_NO_LAZY_FETCH=1\n> guard and fails with \"lazy fetching disabled\". Without the fix,\n> the depth-2 fetch would proceed and potentially recurse.\n\nGreat, thanks.\n\n> > Okay, so this seems to be an issue that can be hit in the wild. But I\n> > have to wonder whether this really is a bug on the client-side, or\n> > whether this is a bug that actually sits on your server. So ultimately:\n> > why does the server send REF_DELTA objects in the first place? Is it\n> > using git-upload-pack(1), or is it using a different implementation of\n> > Git to serve data?\n> \n> The server is GitHub. I did a blob:none partial clone and after some\n> further git operations ended up in this state. I don't have\n> server-side data on why it sent REF_DELTAs against missing bases.\n\nThat's certainly curious. Do you maybe have multiple remotes attached to\nthe repository, or are you dropping/modifying the object filter at some\npoint?\n\nAll subsequent fetches need to use the same object filter as you've used\nduring the initial clone, otherwise you may run into a situation as you\nhave described. But in theory, Git knows to continue using the filter.\n\n> > Note that I'm not arguing that we shouldn't have protection on the\n> > client, too. But I'd first like to understand whether there is a bug\n> > lurking somewhere that causes us to send invalid packfiles.\n> \n> Agreed, there may well be a server-side bug here. Regardless, the\n> client should fail fast rather than consume unbounded resources.\n\nProbably, yes. What I'm trying to figure out is whether there are edge\ncases here where it's _valid_ for the server to send a thin pack with a\nREF_DELTA. Because if so, unconditionally disabling the lazy fetches\nwould break such edge cases.\n\nI don't think there are such cases, but I wouldn't consider myself an\nexpert with partial clones.\n\nCc'd Peff, as he's implemented a couple fixes in this area a couple\nyears ago.\n\nPatrick\n"},{"id":"538834","messageId":"20260313014315.GA3201544@coredump.intra.peff.net","threadId":"65134","inReplyTo":"abJqySqfdFoY8cEu@pks.im","subject":"Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2026-03-13T01:43:15Z","receivedAt":"2026-03-13T01:43:17Z","isPatch":true,"body":"On Thu, Mar 12, 2026 at 08:27:05AM +0100, Patrick Steinhardt wrote:\n\n> > > Note that I'm not arguing that we shouldn't have protection on the\n> > > client, too. But I'd first like to understand whether there is a bug\n> > > lurking somewhere that causes us to send invalid packfiles.\n> > \n> > Agreed, there may well be a server-side bug here. Regardless, the\n> > client should fail fast rather than consume unbounded resources.\n> \n> Probably, yes. What I'm trying to figure out is whether there are edge\n> cases here where it's _valid_ for the server to send a thin pack with a\n> REF_DELTA. Because if so, unconditionally disabling the lazy fetches\n> would break such edge cases.\n> \n> I don't think there are such cases, but I wouldn't consider myself an\n> expert with partial clones.\n> \n> Cc'd Peff, as he's implemented a couple fixes in this area a couple\n> years ago.\n\nHmm, I'm not sure I have much wisdom. Here's the most plausible scenario\nI could come up with.\n\nA backfill fetch like this is going to have a noop negotiation\nalgorithm. So the server does not have any idea what the client has, and\ntherefore shouldn't be sending any thin deltas against it.\n\nBut it _can_ send deltas against objects which are part of the backfill\nitself. Normally we'd send those as OFS_DELTA, because they're both in\nthe same output pack. But there are cases where we might not:\n\n  - if the client did not tell us it understands ofs-deltas; this would\n    not be true for any version of Git in the last 15+ years, but maybe\n    there is a bug in sending or parsing the capability? Or an alternate\n    Git implementation on the client side which forgets to send it?\n\n  - the verbatim pack-reuse code will sometimes rewrite ofs-delta into\n    ref-delta. I don't remember all of the cases where this might\n    happen. Certainly if the client hasn't claimed to support\n    ofs-deltas, but I think maybe some other cases? I'd have to dig into\n    it.\n\nNow there's a catch: the pack is not really thin, and so index-pack\nshould not need to do an extra backfill request in order to get the base\nobject. But depending how index-pack is written, it might try to do so\nanyway. If X is a delta against base Y, but Y is itself a delta, we\nmight not have resolved it yet. And so when we try to resolve X, we\nthink \"aha, let us see if we have Y\", and then eagerly attempt a\nbackfill fetch (probably triggered from odb_has_object() or similar).\nWhen in fact the right thing to do is to queue X, resolve everything we\ncan, and see if we ended up with Y (actually index-pack works from the\nbases up in the final resolution phase, but the effect is the same).\n\nIf that's what is happening, then I _think_ Paul's patch will do the\nright thing. We'd say \"no, we don't have that object\" without doing the\nbackfill, and then eventually find it as part of the final resolution.\n\nIt would be nice to confirm that's what's going on, though (and it isn't\nreally a thin pack). If the problem can be reproduced, I don't suppose\nwe have a GIT_TRACE_PACKET output from a failing instance? That would\nconfirm that we're correctly using the noop negotiation.\n\n-Peff\n"},{"id":"538881","messageId":"20260313124326.75586-1-github@paulisageek.com","threadId":"65134","inReplyTo":"20260313014315.GA3201544@coredump.intra.peff.net","subject":"Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan","fromEmail":"paul@paultarjan.com","sentAt":"2026-03-13T12:43:25Z","receivedAt":"2026-03-13T12:43:30Z","isPatch":true,"body":"Jeff King <peff@peff.net> writes:\n\n> It would be nice to confirm that's what's going on, though (and it isn't\n> really a thin pack). If the problem can be reproduced, I don't suppose\n> we have a GIT_TRACE_PACKET output from a failing instance? That would\n> confirm that we're correctly using the noop negotiation.\n\nYeah. I couldn't capture one live during the incident, but I did a\nclean-room reproduction afterward: empty bare repo, git remote add\norigin pointing at the same GitHub monorepo, then the exact argv\nfrom fetch_objects() with a tree SHA pulled from one of the\n.promisor sidecars written during the incident:\n\n  $ git -c fetch.negotiationAlgorithm=noop fetch origin \\\n        --no-tags --no-write-fetch-head --recurse-submodules=no \\\n        --filter=blob:none --stdin <<< \"$TREE_OID\"\n\n  packet:        fetch< version 2\n  packet:        fetch< agent=git/github-94ec3a8682aa-Linux\n  packet:        fetch< ls-refs=unborn\n  packet:        fetch< fetch=shallow wait-for-done filter\n  packet:        fetch< server-option\n  packet:        fetch< object-format=sha1\n  packet:        fetch< 0000\n  packet:        fetch> command=fetch\n  packet:        fetch> agent=git/2.51.2-Linux\n  packet:        fetch> object-format=sha1\n  packet:        fetch> 0001\n  packet:        fetch> thin-pack\n  packet:        fetch> no-progress\n  packet:        fetch> ofs-delta\n  packet:        fetch> filter blob:none\n  packet:        fetch> want 086faa42a038f2e4a1b5e395cde544f8f3c6a9a5\n  packet:        fetch> done\n  packet:        fetch> 0000\n  packet:        fetch< packfile\n  packet:     sideband< PACK ...\n  packet:     sideband< 0000\n\nZero have lines, noop negotiation doing its thing.\n\nTwo interesting bits:\n\n1. The client sends thin-pack unconditionally (fetch-pack.c doesn't\n   check what negotiation algorithm is in use). So the server is\n   allowed to send a thin pack. With zero haves it has nothing to\n   thin against, but nothing on the wire actually forbids it.\n\n2. filter blob:none is on the wire -- hardcoded by fetch_objects().\n   Combined with thin-pack, this gives the server a way to end up\n   omitting an object that happens to be a delta base.\n\nFor this particular tree GitHub responded cleanly (5 trees, no\ndeltas, 2701 bytes), so whatever triggers the REF_DELTA depends on\nthe specific objects being fetched and how they're stored\nserver-side.\n\n> But it _can_ send deltas against objects which are part of the backfill\n> itself. Normally we'd send those as OFS_DELTA, because they're both in\n> the same output pack. But there are cases where we might not:\n>\n>   - the verbatim pack-reuse code will sometimes rewrite ofs-delta into\n>     ref-delta. I don't remember all of the cases where this might\n>     happen. Certainly if the client hasn't claimed to support\n>     ofs-deltas, but I think maybe some other cases? I'd have to dig into\n>     it.\n\nI think it's (b). What's interesting is that Jonathan Tan flagged\nexactly this risk when he introduced the prefetch call.\n8a30a1efd1 (\"index-pack: prefetch missing REF_DELTA bases\",\n2019-05-14):\n\n    Support for lazy fetching should still generally be turned off\n    in index-pack because it is used as part of the lazy fetching\n    process itself (if not, infinite loops may occur), but we do\n    need to fetch the REF_DELTA bases. (When fetching REF_DELTA\n    bases, it is unlikely that those are REF_DELTA themselves,\n    because we do not send \"have\" when making such fetches.)\n\nSo the \"infinite loops may occur\" door was left open deliberately,\non the argument that it's unlikely. My patch just closes it.\n\nHere's what I think went wrong in practice: fetch_objects()\nhardcodes --filter=blob:none. When the want is a tree, the server\nwalks tree reachability and assembles potentially millions of tree\nobjects with no blobs. If pack-reuse passes through an object\nstored server-side as OFS_DELTA against a blob, it comes out as\nREF_DELTA and the blob base gets filtered out by blob:none.\n\nI have a live ps snapshot from the incident showing the chain:\n\n  3719679  git -c fetch.negotiationAlgorithm=noop fetch origin \\\n               --no-tags --no-write-fetch-head \\\n               --recurse-submodules=no --filter=blob:none --stdin\n  3725911  git index-pack --stdin --fix-thin --keep=fetch-pack \\\n               3719679 on <host> --promisor --pack_header=2,7535361\n  3726936  git -c fetch.negotiationAlgorithm=noop fetch origin \\\n               --no-tags --no-write-fetch-head \\\n               --recurse-submodules=no --filter=blob:none --stdin\n\n--keep embeds the parent PID, so the chain is clear: lazy-fetch\n(3719679) -> index-pack (3725911, 7,535,361 objects) -> depth-2\nlazy-fetch (3726936). 7.5M objects is about right for wanting a\nsingle tree with no haves and filter=blob:none -- you get the\nentire commit+tree closure minus blobs. One REF_DELTA against a\nfiltered blob in 7.5M objects and you're recursing.\n\n> Now there's a catch: the pack is not really thin, and so index-pack\n> should not need to do an extra backfill request in order to get the base\n> object.\n\nActually, the pack IS thin in this case -- the REF_DELTA's base is\na blob that the server filtered out because of blob:none. The base\nisn't in the pack at all. fix_unresolved_deltas() correctly sees it\nas unresolved and calls promisor_remote_get_direct().\n\n> If that's what is happening, then I _think_ Paul's patch will do the\n> right thing. We'd say \"no, we don't have that object\" without doing the\n> backfill, and then eventually find it as part of the final resolution.\n\nRight, that's what happens. The child hits the guard at the top of\nfetch_objects() and bails with \"lazy fetching disabled\". The\ndepth-1 index-pack can't resolve the delta, which is the correct\noutcome -- retrying the same noop fetch would just get the same\nresponse.\n\nI also wrote a deterministic local reproducer (no network needed)\nthat shows the recursion directly. Two scripts, needs python3 + git,\nworks on Linux and macOS. Ran it just now:\n\nrepro-depth1.sh sets up a local bare server with two similar blobs,\na blob:none partial-clone client, and a hand-crafted 68-byte thin\npack with one REF_DELTA against a promised-but-absent blob:\n\n  === without GIT_NO_LAZY_FETCH (current git) ===\n  index-pack invocations: 2\n  lazy-fetch spawns:      1\n\n  === with GIT_NO_LAZY_FETCH=1 (what the patch sets) ===\n  index-pack invocations: 1\n  lazy-fetch spawns:      0\n\nrepro-unbounded.sh goes further: uploadpack.packObjectsHook always\nreturns the same thin pack, so every lazy-fetch spawns another.\nWatchdog kills it at depth 50:\n\n  === without patch ===\n  recursion depth reached: 55\n  index-pack invocations:  55\n  tmp_pack_* on disk:      55\n\n  === with GIT_NO_LAZY_FETCH=1 (the patch) ===\n  recursion depth:         0\n  index-pack invocations:  1\n  tmp_pack_* on disk:      0\n\nThose tmp_pack_* files are the disk growth -- each index-pack\nstreams to a tmpfile, blocks in fix_unresolved_deltas() to\nlazy-fetch the base, and never gets to rename it. In the real\nincident each was ~1.4 GB.\n\nScripts below.\n\n--- >8 --- repro-depth1.sh --- >8 ---\n\n#!/bin/bash\n# Deterministic depth-1 recursion proof for promisor-remote lazy-fetch bug.\n# No network. Runs in ~1 second.\n#\n# Mechanism:\n#   - Local bare server with two similar blobs; client is a blob:none\n#     partial clone (both blobs promised, neither local).\n#   - Craft a 68-byte thin pack by hand: one REF_DELTA object whose base\n#     SHA is one of the promised-but-absent blobs.\n#   - Feed to `git -C client index-pack --stdin --fix-thin`.\n#   - fix_unresolved_deltas() sees the unresolved REF_DELTA, calls\n#     promisor_remote_get_direct() for the base, which spawns\n#     `git -c fetch.negotiationAlgorithm=noop fetch ... --stdin`.\n#   - That child fetch spawns its own index-pack --fix-thin.\n#   - GIT_TRACE shows the nested spawn.\n\nset -euo pipefail\n\nW=\"${TMPDIR:-/tmp}/promisor-recursion-demo\"\nrm -rf \"$W\"; mkdir -p \"$W\"; cd \"$W\"\n\ngit -c init.defaultBranch=main init -q --bare server.git\ngit -C server.git config uploadpack.allowFilter true\ngit -C server.git config uploadpack.allowAnySHA1InWant true\ngit -c init.defaultBranch=main init -q work\n(\n    cd work\n    python3 -c \"print('\\n'.join('shared line %d ' * 4 % (i,i,i,i) for i in range(200)))\" > f\n    git add f; git commit -qm base\n    python3 -c \"print('\\n'.join('shared line %d ' * 4 % (i,i,i,i) for i in range(200))); print('extra')\" > f\n    git add f; git commit -qm delta\n    git push -q ../server.git main\n)\n\nprintf '%s\\n%s\\n' \"$(git -C work rev-parse HEAD~1:f)\" \"$(git -C work rev-parse HEAD:f)\" | \\\n    git -C server.git pack-objects --stdout --delta-base-offset > both.pack\n\npython3 << 'PYEOF'\nimport struct, zlib, hashlib\n\nwith open('both.pack', 'rb') as f:\n    data = f.read()\nassert data[:4] == b'PACK' and struct.unpack('>I', data[8:12])[0] == 2\n\ndef parse_hdr(data, pos):\n    b = data[pos]; otype = (b >> 4) & 7; sz = b & 0xf; sh = 4; p = pos\n    while b & 0x80:\n        p += 1; b = data[p]; sz |= (b & 0x7f) << sh; sh += 7\n    return otype, sz, p + 1\n\ndef zlib_end(data, pos):\n    d = zlib.decompressobj(); p = pos\n    while p < len(data):\n        chunk = data[p:p+512]; d.decompress(chunk)\n        if d.unused_data: return p + len(chunk) - len(d.unused_data)\n        if d.eof: return p + len(chunk)\n        p += len(chunk)\n    return p\n\nt1, s1, d1 = parse_hdr(data, 12)\nif t1 == 6:\n    p = d1; b = data[p]; p += 1\n    while b & 0x80: b = data[p]; p += 1\n    e1 = zlib_end(data, p)\nelse:\n    e1 = zlib_end(data, d1)\nt2, s2, d2 = parse_hdr(data, e1)\n\nif t1 == 3 and t2 == 6:\n    base_raw = zlib.decompress(data[d1:e1])\n    delta_sz, delta_hdr_end = s2, d2\nelif t1 == 6 and t2 == 3:\n    base_raw = zlib.decompress(data[d2:zlib_end(data, d2)])\n    delta_sz, delta_hdr_end = s1, d1\nelse:\n    raise SystemExit(f\"expected one blob + one OFS_DELTA, got types {t1},{t2}\")\n\nbase_sha = hashlib.sha1(f\"blob {len(base_raw)}\\0\".encode() + base_raw).digest()\n\np = delta_hdr_end; b = data[p]; p += 1\nwhile b & 0x80: b = data[p]; p += 1\ndelta_zlib = data[p:zlib_end(data, p)]\n\nsz = delta_sz\nhb = [(7 << 4) | (sz & 0xf)]; sz >>= 4\nwhile sz: hb[-1] |= 0x80; hb.append(sz & 0x7f); sz >>= 7\nthin = b'PACK' + struct.pack('>II', 2, 1) + bytes(hb) + base_sha + delta_zlib\nthin += hashlib.sha1(thin).digest()\nwith open('thin.pack', 'wb') as f: f.write(thin)\nprint(f\"thin.pack: {len(thin)} bytes, REF_DELTA base={base_sha.hex()}\")\nPYEOF\n\ngit clone -q --no-local --filter=blob:none --no-checkout \"file://$W/server.git\" client\n\necho \"=== without GIT_NO_LAZY_FETCH (current git) ===\"\nGIT_TRACE=\"$W/trace.txt\" \\\n    git -C client index-pack --stdin --fix-thin < thin.pack >/dev/null 2>&1 || true\ngrep -E 'built-in: git (index-pack|fetch)|run_command:.*negotiationAlgorithm' trace.txt\necho \"index-pack invocations: $(grep -c 'built-in: git index-pack' trace.txt)\"\necho \"lazy-fetch spawns:      $(grep -c 'run_command:.*negotiationAlgorithm=noop' trace.txt)\"\n\necho \"\"\necho \"=== with GIT_NO_LAZY_FETCH=1 (what the patch sets) ===\"\nrm -rf client\ngit clone -q --no-local --filter=blob:none --no-checkout \"file://$W/server.git\" client\nGIT_NO_LAZY_FETCH=1 GIT_TRACE=\"$W/trace2.txt\" \\\n    git -C client index-pack --stdin --fix-thin < thin.pack 2>&1 || true\necho \"index-pack invocations: $(grep -c 'built-in: git index-pack' trace2.txt)\"\necho \"lazy-fetch spawns:      $(grep -c 'run_command:.*negotiationAlgorithm=noop' trace2.txt || echo 0)\"\n\n--- >8 --- repro-unbounded.sh --- >8 ---\n\n#!/bin/bash\n# Unbounded recursion proof. Run repro-depth1.sh first.\n# uploadpack.packObjectsHook always returns the same thin pack,\n# so every lazy-fetch spawns another. Watchdog kills at depth 50.\n\nset -uo pipefail\n\nW=\"${TMPDIR:-/tmp}/promisor-recursion-demo\"\ncd \"$W\"\ntest -f thin.pack || { echo \"run repro-depth1.sh first\"; exit 1; }\n\ncat > evil-pack-objects.sh << EOF\n#!/bin/sh\ncat > /dev/null\ncat \"$W/thin.pack\"\nEOF\nchmod +x evil-pack-objects.sh\n\ncat > fake-global.gitconfig << EOF\n[uploadpack]\n    packObjectsHook = $W/evil-pack-objects.sh\nEOF\n\nrm -rf client\nGIT_CONFIG_GLOBAL= git clone -q --no-local --filter=blob:none --no-checkout \\\n    \"file://$W/server.git\" client\n\nTRACE=\"$W/unbounded.trace\"\nrm -f \"$TRACE\"\n\necho \"=== spawning (watchdog kills at depth 50) ===\"\nGIT_TRACE=\"$TRACE\" GIT_CONFIG_GLOBAL=\"$W/fake-global.gitconfig\" \\\n    git -C client index-pack --stdin --fix-thin < thin.pack >/dev/null 2>&1 &\nROOT=$!\n\nLIMIT=50\nwhile kill -0 \"$ROOT\" 2>/dev/null; do\n    N=$(grep -c 'run_command:.*negotiationAlgorithm=noop' \"$TRACE\" 2>/dev/null || true)\n    N=${N:-0}\n    if (( N >= LIMIT )); then\n        echo \">>> depth $N reached, killing process tree $ROOT <<<\"\n        pkill -KILL -f \"promisor-recursion-demo/client\" 2>/dev/null || true\n        kill -KILL \"$ROOT\" 2>/dev/null || true\n        sleep 0.2\n        pkill -KILL -f \"promisor-recursion-demo/client\" 2>/dev/null || true\n        break\n    fi\n    sleep 0.02\ndone\nwait \"$ROOT\" 2>/dev/null\nEC=$?\n\nDEPTH=$(grep -c 'run_command:.*negotiationAlgorithm=noop' \"$TRACE\" 2>/dev/null || true)\nDEPTH=${DEPTH:-0}\nIPACKS=$(grep -c 'built-in: git index-pack' \"$TRACE\" 2>/dev/null || true)\nIPACKS=${IPACKS:-0}\nTMPS=$(ls client/.git/objects/pack/tmp_pack_* 2>/dev/null | wc -l | tr -d ' ')\n\necho \"\"\necho \"=== without patch (root exit $EC) ===\"\necho \"recursion depth reached: $DEPTH\"\necho \"index-pack invocations:  $IPACKS\"\necho \"tmp_pack_* on disk:      $TMPS\"\necho \"\"\necho \"first 4 + last 2 spawns (identical line, no termination condition):\"\ngrep -E 'built-in: git index-pack|run_command:.*negotiationAlgorithm=noop' \"$TRACE\" | head -4\necho \"...\"\ngrep -E 'built-in: git index-pack|run_command:.*negotiationAlgorithm=noop' \"$TRACE\" | tail -2\n\necho \"\"\necho \"=== with GIT_NO_LAZY_FETCH=1 (the patch) ===\"\nrm -rf client; rm -f \"$TRACE\"\nGIT_CONFIG_GLOBAL= git clone -q --no-local --filter=blob:none --no-checkout \\\n    \"file://$W/server.git\" client\nGIT_NO_LAZY_FETCH=1 GIT_TRACE=\"$TRACE\" GIT_CONFIG_GLOBAL=\"$W/fake-global.gitconfig\" \\\n    git -C client index-pack --stdin --fix-thin < thin.pack 2>&1 || true\nCTRL_DEPTH=$(grep -c 'run_command:.*negotiationAlgorithm=noop' \"$TRACE\" 2>/dev/null || true)\nCTRL_IPACKS=$(grep -c 'built-in: git index-pack' \"$TRACE\" 2>/dev/null || true)\nCTRL_TMPS=$(ls client/.git/objects/pack/tmp_pack_* 2>/dev/null | wc -l | tr -d ' ')\necho \"recursion depth:         ${CTRL_DEPTH:-0}\"\necho \"index-pack invocations:  ${CTRL_IPACKS:-0}\"\necho \"tmp_pack_* on disk:      $CTRL_TMPS\"\n\nrm -rf \"$W/client\"\n\nPaul\n"},{"id":"538882","messageId":"20260313124329.75626-1-github@paulisageek.com","threadId":"65134","inReplyTo":"abJqySqfdFoY8cEu@pks.im","subject":"Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Paul Tarjan","fromEmail":"paul@paultarjan.com","sentAt":"2026-03-13T12:43:29Z","receivedAt":"2026-03-13T12:43:34Z","isPatch":true,"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> I must be misunderstanding something here, but how is it that a commit\n> can be garbage collected if a ref points to it? That shouldn't ever\n> happen, as reachable commits should not be pruned.\n>\n> Or do you mean to say that the commits don't exist on the server side\n> anymore?\n\nSloppy wording on my part — \"GC'd\" is wrong. These refs pointed at\ncommits that were promised but never materialized on the partial\nclone. The ~77K broken refs looked like:\n\n  error: refs/prefetch/remotes/origin/claude/add-azure-dependencies-EaaDn \\\n         does not point to a valid object!\n\nThey were created by git maintenance --task=prefetch, which runs\ngit fetch --prefetch --prune and writes refs/prefetch/remotes/origin/<branch>\npointing at the remote tip. On a blob:none partial clone it fetches\ncommit/tree metadata, but some referenced commits were never\nactually downloaded before the upstream branches (ephemeral\nCI/automation branches, force-pushed and deleted within days)\ndisappeared.\n\nThis is a red herring for the patch though. The stale prefetch refs\nexplain why the outer fetch got a thin pack — the client advertised\nhaves from promised-but-absent commits. But the recursion (depth-1\nto depth-2+) is entirely inside fetch_objects() with noop\nnegotiation, independent of any refs.\n\nI'll fix the commit message wording in a v4 if you'd like.\n\n> That's certainly curious. Do you maybe have multiple remotes attached to\n> the repository, or are you dropping/modifying the object filter at some\n> point?\n>\n> All subsequent fetches need to use the same object filter as you've used\n> during the initial clone, otherwise you may run into a situation as you\n> have described. But in theory, Git knows to continue using the filter.\n\nNobody intentionally changed the filter. What happened is the\nlazy-fetch child kept re-writing it. fetch_objects() hardcodes\n--filter=blob:none on the child argv, and the child's\nbuiltin/fetch.c writes the active filter to config.\n\n23547c40 (\"fetch: do not override partial clone filter\", 2020)\nguards this write behind a check for an already-set filter. But I\nwas unsetting remote.origin.partialclonefilter manually trying to\nstop the storm, so the guard passed and the next lazy-fetch child\nwrote it right back:\n\n  21:45  (unset)     manual git config --unset\n  22:05  blob:none   re-written by a lazy-fetch child\n  22:11  (unset)     manual unset again\n  23:11  blob:none   re-written again\n  23:13  (unset)     manual unset\n  23:28  blob:none   re-written (caught live by a config-mtime trap)\n\nThe actual mitigation was unsetting remote.origin.promisor too —\nwith no promisor remote, fix_unresolved_deltas() skips the prefetch\nentirely.\n\nThis is arguably a separate bug: fetch_objects() should probably\npass -c remote.<name>.partialclonefilter=blob:none to override for\nthe single invocation, rather than --filter=blob:none which\npersists to config. Not in scope for this patch, but I could follow\nup separately if there's interest.\n\nPaul\n"},{"id":"541686","messageId":"xmqqik9s6qvd.fsf@gitster.g","threadId":"65134","inReplyTo":"20260313124329.75626-1-github@paulisageek.com","subject":"Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-15T18:05:42Z","receivedAt":"2026-04-15T18:05:45Z","isPatch":true,"body":"Paul Tarjan <paul@paultarjan.com> writes:\n\n> Patrick Steinhardt <ps@pks.im> writes:\n>\n>> I must be misunderstanding something here, but how is it that a commit\n>> can be garbage collected if a ref points to it? That shouldn't ever\n>> happen, as reachable commits should not be pruned.\n>>\n>> Or do you mean to say that the commits don't exist on the server side\n>> anymore?\n>\n> Sloppy wording on my part — \"GC'd\" is wrong. These refs pointed at\n> commits that were promised but never materialized on the partial\n> clone. The ~77K broken refs looked like:\n> ...\n> This is arguably a separate bug: fetch_objects() should probably\n> pass -c remote.<name>.partialclonefilter=blob:none to override for\n> the single invocation, rather than --filter=blob:none which\n> persists to config. Not in scope for this patch, but I could follow\n> up separately if there's interest.\n\nSo, is this topic still viable?\n\nAt least I see that v3 was not satisfactory enough from the\ndiscussion thread, but do we know what needs updating, how much more\nwork is needed, and where we want to go?\n\nFor now I'll drop the copy I have (from more than a month ago) from\nmy tree.\n"}]}