Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch

Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch

[Paul Tarjan]

Junio C Hamano <gitster@pobox.com> writes:

> So, is this topic still viable?

Yes, still viable from my end. The patch fixed the production
problem I ran into.

I thought v3 had addressed the outstanding feedback (HTTP
behavioral test, dropped the commit-graph comparison, packet
trace and pack-reuse analysis sent on Mar 13 in response to
Peff's questions), but I may have missed something. Is there a
specific concern I should address?

Paul

Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch

[Jeff King]

On Thu, Mar 12, 2026 at 08:27:05AM +0100, Patrick Steinhardt wrote:

> > > Note that I'm not arguing that we shouldn't have protection on the
> > > client, too. But I'd first like to understand whether there is a bug
> > > lurking somewhere that causes us to send invalid packfiles.
> > 
> > Agreed, there may well be a server-side bug here. Regardless, the
> > client should fail fast rather than consume unbounded resources.
> 
> Probably, yes. What I'm trying to figure out is whether there are edge
> cases here where it's _valid_ for the server to send a thin pack with a
> REF_DELTA. Because if so, unconditionally disabling the lazy fetches
> would break such edge cases.
> 
> I don't think there are such cases, but I wouldn't consider myself an
> expert with partial clones.
> 
> Cc'd Peff, as he's implemented a couple fixes in this area a couple
> years ago.

Hmm, I'm not sure I have much wisdom. Here's the most plausible scenario
I could come up with.

A backfill fetch like this is going to have a noop negotiation
algorithm. So the server does not have any idea what the client has, and
therefore shouldn't be sending any thin deltas against it.

But it _can_ send deltas against objects which are part of the backfill
itself. Normally we'd send those as OFS_DELTA, because they're both in
the same output pack. But there are cases where we might not:

  - if the client did not tell us it understands ofs-deltas; this would
    not be true for any version of Git in the last 15+ years, but maybe
    there is a bug in sending or parsing the capability? Or an alternate
    Git implementation on the client side which forgets to send it?

  - the verbatim pack-reuse code will sometimes rewrite ofs-delta into
    ref-delta. I don't remember all of the cases where this might
    happen. Certainly if the client hasn't claimed to support
    ofs-deltas, but I think maybe some other cases? I'd have to dig into
    it.

Now there's a catch: the pack is not really thin, and so index-pack
should not need to do an extra backfill request in order to get the base
object. But depending how index-pack is written, it might try to do so
anyway. If X is a delta against base Y, but Y is itself a delta, we
might not have resolved it yet. And so when we try to resolve X, we
think "aha, let us see if we have Y", and then eagerly attempt a
backfill fetch (probably triggered from odb_has_object() or similar).
When in fact the right thing to do is to queue X, resolve everything we
can, and see if we ended up with Y (actually index-pack works from the
bases up in the final resolution phase, but the effect is the same).

If that's what is happening, then I _think_ Paul's patch will do the
right thing. We'd say "no, we don't have that object" without doing the
backfill, and then eventually find it as part of the final resolution.

It would be nice to confirm that's what's going on, though (and it isn't
really a thin pack). If the problem can be reproduced, I don't suppose
we have a GIT_TRACE_PACKET output from a failing instance? That would
confirm that we're correctly using the noop negotiation.

-Peff

Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch

[Paul Tarjan]

Jeff King <peff@peff.net> writes:

> It would be nice to confirm that's what's going on, though (and it isn't
> really a thin pack). If the problem can be reproduced, I don't suppose
> we have a GIT_TRACE_PACKET output from a failing instance? That would
> confirm that we're correctly using the noop negotiation.

Yeah. I couldn't capture one live during the incident, but I did a
clean-room reproduction afterward: empty bare repo, git remote add
origin pointing at the same GitHub monorepo, then the exact argv
from fetch_objects() with a tree SHA pulled from one of the
.promisor sidecars written during the incident:

  $ git -c fetch.negotiationAlgorithm=noop fetch origin \
        --no-tags --no-write-fetch-head --recurse-submodules=no \
        --filter=blob:none --stdin <<< "$TREE_OID"

  packet:        fetch< version 2
  packet:        fetch< agent=git/github-94ec3a8682aa-Linux
  packet:        fetch< ls-refs=unborn
  packet:        fetch< fetch=shallow wait-for-done filter
  packet:        fetch< server-option
  packet:        fetch< object-format=sha1
  packet:        fetch< 0000
  packet:        fetch> command=fetch
  packet:        fetch> agent=git/2.51.2-Linux
  packet:        fetch> object-format=sha1
  packet:        fetch> 0001
  packet:        fetch> thin-pack
  packet:        fetch> no-progress
  packet:        fetch> ofs-delta
  packet:        fetch> filter blob:none
  packet:        fetch> want 086faa42a038f2e4a1b5e395cde544f8f3c6a9a5
  packet:        fetch> done
  packet:        fetch> 0000
  packet:        fetch< packfile
  packet:     sideband< PACK ...
  packet:     sideband< 0000

Zero have lines, noop negotiation doing its thing.

Two interesting bits:

1. The client sends thin-pack unconditionally (fetch-pack.c doesn't
   check what negotiation algorithm is in use). So the server is
   allowed to send a thin pack. With zero haves it has nothing to
   thin against, but nothing on the wire actually forbids it.

2. filter blob:none is on the wire -- hardcoded by fetch_objects().
   Combined with thin-pack, this gives the server a way to end up
   omitting an object that happens to be a delta base.

For this particular tree GitHub responded cleanly (5 trees, no
deltas, 2701 bytes), so whatever triggers the REF_DELTA depends on
the specific objects being fetched and how they're stored
server-side.

> But it _can_ send deltas against objects which are part of the backfill
> itself. Normally we'd send those as OFS_DELTA, because they're both in
> the same output pack. But there are cases where we might not:
>
>   - the verbatim pack-reuse code will sometimes rewrite ofs-delta into
>     ref-delta. I don't remember all of the cases where this might
>     happen. Certainly if the client hasn't claimed to support
>     ofs-deltas, but I think maybe some other cases? I'd have to dig into
>     it.

I think it's (b). What's interesting is that Jonathan Tan flagged
exactly this risk when he introduced the prefetch call.
8a30a1efd1 ("index-pack: prefetch missing REF_DELTA bases",
2019-05-14):

    Support for lazy fetching should still generally be turned off
    in index-pack because it is used as part of the lazy fetching
    process itself (if not, infinite loops may occur), but we do
    need to fetch the REF_DELTA bases. (When fetching REF_DELTA
    bases, it is unlikely that those are REF_DELTA themselves,
    because we do not send "have" when making such fetches.)

So the "infinite loops may occur" door was left open deliberately,
on the argument that it's unlikely. My patch just closes it.

Here's what I think went wrong in practice: fetch_objects()
hardcodes --filter=blob:none. When the want is a tree, the server
walks tree reachability and assembles potentially millions of tree
objects with no blobs. If pack-reuse passes through an object
stored server-side as OFS_DELTA against a blob, it comes out as
REF_DELTA and the blob base gets filtered out by blob:none.

I have a live ps snapshot from the incident showing the chain:

  3719679  git -c fetch.negotiationAlgorithm=noop fetch origin \
               --no-tags --no-write-fetch-head \
               --recurse-submodules=no --filter=blob:none --stdin
  3725911  git index-pack --stdin --fix-thin --keep=fetch-pack \
               3719679 on <host> --promisor --pack_header=2,7535361
  3726936  git -c fetch.negotiationAlgorithm=noop fetch origin \
               --no-tags --no-write-fetch-head \
               --recurse-submodules=no --filter=blob:none --stdin

--keep embeds the parent PID, so the chain is clear: lazy-fetch
(3719679) -> index-pack (3725911, 7,535,361 objects) -> depth-2
lazy-fetch (3726936). 7.5M objects is about right for wanting a
single tree with no haves and filter=blob:none -- you get the
entire commit+tree closure minus blobs. One REF_DELTA against a
filtered blob in 7.5M objects and you're recursing.

> Now there's a catch: the pack is not really thin, and so index-pack
> should not need to do an extra backfill request in order to get the base
> object.

Actually, the pack IS thin in this case -- the REF_DELTA's base is
a blob that the server filtered out because of blob:none. The base
isn't in the pack at all. fix_unresolved_deltas() correctly sees it
as unresolved and calls promisor_remote_get_direct().

> If that's what is happening, then I _think_ Paul's patch will do the
> right thing. We'd say "no, we don't have that object" without doing the
> backfill, and then eventually find it as part of the final resolution.

Right, that's what happens. The child hits the guard at the top of
fetch_objects() and bails with "lazy fetching disabled". The
depth-1 index-pack can't resolve the delta, which is the correct
outcome -- retrying the same noop fetch would just get the same
response.

I also wrote a deterministic local reproducer (no network needed)
that shows the recursion directly. Two scripts, needs python3 + git,
works on Linux and macOS. Ran it just now:

repro-depth1.sh sets up a local bare server with two similar blobs,
a blob:none partial-clone client, and a hand-crafted 68-byte thin
pack with one REF_DELTA against a promised-but-absent blob:

  === without GIT_NO_LAZY_FETCH (current git) ===
  index-pack invocations: 2
  lazy-fetch spawns:      1

  === with GIT_NO_LAZY_FETCH=1 (what the patch sets) ===
  index-pack invocations: 1
  lazy-fetch spawns:      0

repro-unbounded.sh goes further: uploadpack.packObjectsHook always
returns the same thin pack, so every lazy-fetch spawns another.
Watchdog kills it at depth 50:

  === without patch ===
  recursion depth reached: 55
  index-pack invocations:  55
  tmp_pack_* on disk:      55

  === with GIT_NO_LAZY_FETCH=1 (the patch) ===
  recursion depth:         0
  index-pack invocations:  1
  tmp_pack_* on disk:      0

Those tmp_pack_* files are the disk growth -- each index-pack
streams to a tmpfile, blocks in fix_unresolved_deltas() to
lazy-fetch the base, and never gets to rename it. In the real
incident each was ~1.4 GB.

Scripts below.

--- >8 --- repro-depth1.sh --- >8 ---

#!/bin/bash
# Deterministic depth-1 recursion proof for promisor-remote lazy-fetch bug.
# No network. Runs in ~1 second.
#
# Mechanism:
#   - Local bare server with two similar blobs; client is a blob:none
#     partial clone (both blobs promised, neither local).
#   - Craft a 68-byte thin pack by hand: one REF_DELTA object whose base
#     SHA is one of the promised-but-absent blobs.
#   - Feed to `git -C client index-pack --stdin --fix-thin`.
#   - fix_unresolved_deltas() sees the unresolved REF_DELTA, calls
#     promisor_remote_get_direct() for the base, which spawns
#     `git -c fetch.negotiationAlgorithm=noop fetch ... --stdin`.
#   - That child fetch spawns its own index-pack --fix-thin.
#   - GIT_TRACE shows the nested spawn.

set -euo pipefail

W="${TMPDIR:-/tmp}/promisor-recursion-demo"
rm -rf "$W"; mkdir -p "$W"; cd "$W"

git -c init.defaultBranch=main init -q --bare server.git
git -C server.git config uploadpack.allowFilter true
git -C server.git config uploadpack.allowAnySHA1InWant true
git -c init.defaultBranch=main init -q work
(
    cd work
    python3 -c "print('\n'.join('shared line %d ' * 4 % (i,i,i,i) for i in range(200)))" > f
    git add f; git commit -qm base
    python3 -c "print('\n'.join('shared line %d ' * 4 % (i,i,i,i) for i in range(200))); print('extra')" > f
    git add f; git commit -qm delta
    git push -q ../server.git main
)

printf '%s\n%s\n' "$(git -C work rev-parse HEAD~1:f)" "$(git -C work rev-parse HEAD:f)" | \
    git -C server.git pack-objects --stdout --delta-base-offset > both.pack

python3 << 'PYEOF'
import struct, zlib, hashlib

with open('both.pack', 'rb') as f:
    data = f.read()
assert data[:4] == b'PACK' and struct.unpack('>I', data[8:12])[0] == 2

def parse_hdr(data, pos):
    b = data[pos]; otype = (b >> 4) & 7; sz = b & 0xf; sh = 4; p = pos
    while b & 0x80:
        p += 1; b = data[p]; sz |= (b & 0x7f) << sh; sh += 7
    return otype, sz, p + 1

def zlib_end(data, pos):
    d = zlib.decompressobj(); p = pos
    while p < len(data):
        chunk = data[p:p+512]; d.decompress(chunk)
        if d.unused_data: return p + len(chunk) - len(d.unused_data)
        if d.eof: return p + len(chunk)
        p += len(chunk)
    return p

t1, s1, d1 = parse_hdr(data, 12)
if t1 == 6:
    p = d1; b = data[p]; p += 1
    while b & 0x80: b = data[p]; p += 1
    e1 = zlib_end(data, p)
else:
    e1 = zlib_end(data, d1)
t2, s2, d2 = parse_hdr(data, e1)

if t1 == 3 and t2 == 6:
    base_raw = zlib.decompress(data[d1:e1])
    delta_sz, delta_hdr_end = s2, d2
elif t1 == 6 and t2 == 3:
    base_raw = zlib.decompress(data[d2:zlib_end(data, d2)])
    delta_sz, delta_hdr_end = s1, d1
else:
    raise SystemExit(f"expected one blob + one OFS_DELTA, got types {t1},{t2}")

base_sha = hashlib.sha1(f"blob {len(base_raw)}\0".encode() + base_raw).digest()

p = delta_hdr_end; b = data[p]; p += 1
while b & 0x80: b = data[p]; p += 1
delta_zlib = data[p:zlib_end(data, p)]

sz = delta_sz
hb = [(7 << 4) | (sz & 0xf)]; sz >>= 4
while sz: hb[-1] |= 0x80; hb.append(sz & 0x7f); sz >>= 7
thin = b'PACK' + struct.pack('>II', 2, 1) + bytes(hb) + base_sha + delta_zlib
thin += hashlib.sha1(thin).digest()
with open('thin.pack', 'wb') as f: f.write(thin)
print(f"thin.pack: {len(thin)} bytes, REF_DELTA base={base_sha.hex()}")
PYEOF

git clone -q --no-local --filter=blob:none --no-checkout "file://$W/server.git" client

echo "=== without GIT_NO_LAZY_FETCH (current git) ==="
GIT_TRACE="$W/trace.txt" \
    git -C client index-pack --stdin --fix-thin < thin.pack >/dev/null 2>&1 || true
grep -E 'built-in: git (index-pack|fetch)|run_command:.*negotiationAlgorithm' trace.txt
echo "index-pack invocations: $(grep -c 'built-in: git index-pack' trace.txt)"
echo "lazy-fetch spawns:      $(grep -c 'run_command:.*negotiationAlgorithm=noop' trace.txt)"

echo ""
echo "=== with GIT_NO_LAZY_FETCH=1 (what the patch sets) ==="
rm -rf client
git clone -q --no-local --filter=blob:none --no-checkout "file://$W/server.git" client
GIT_NO_LAZY_FETCH=1 GIT_TRACE="$W/trace2.txt" \
    git -C client index-pack --stdin --fix-thin < thin.pack 2>&1 || true
echo "index-pack invocations: $(grep -c 'built-in: git index-pack' trace2.txt)"
echo "lazy-fetch spawns:      $(grep -c 'run_command:.*negotiationAlgorithm=noop' trace2.txt || echo 0)"

--- >8 --- repro-unbounded.sh --- >8 ---

#!/bin/bash
# Unbounded recursion proof. Run repro-depth1.sh first.
# uploadpack.packObjectsHook always returns the same thin pack,
# so every lazy-fetch spawns another. Watchdog kills at depth 50.

set -uo pipefail

W="${TMPDIR:-/tmp}/promisor-recursion-demo"
cd "$W"
test -f thin.pack || { echo "run repro-depth1.sh first"; exit 1; }

cat > evil-pack-objects.sh << EOF
#!/bin/sh
cat > /dev/null
cat "$W/thin.pack"
EOF
chmod +x evil-pack-objects.sh

cat > fake-global.gitconfig << EOF
[uploadpack]
    packObjectsHook = $W/evil-pack-objects.sh
EOF

rm -rf client
GIT_CONFIG_GLOBAL= git clone -q --no-local --filter=blob:none --no-checkout \
    "file://$W/server.git" client

TRACE="$W/unbounded.trace"
rm -f "$TRACE"

echo "=== spawning (watchdog kills at depth 50) ==="
GIT_TRACE="$TRACE" GIT_CONFIG_GLOBAL="$W/fake-global.gitconfig" \
    git -C client index-pack --stdin --fix-thin < thin.pack >/dev/null 2>&1 &
ROOT=$!

LIMIT=50
while kill -0 "$ROOT" 2>/dev/null; do
    N=$(grep -c 'run_command:.*negotiationAlgorithm=noop' "$TRACE" 2>/dev/null || true)
    N=${N:-0}
    if (( N >= LIMIT )); then
        echo ">>> depth $N reached, killing process tree $ROOT <<<"
        pkill -KILL -f "promisor-recursion-demo/client" 2>/dev/null || true
        kill -KILL "$ROOT" 2>/dev/null || true
        sleep 0.2
        pkill -KILL -f "promisor-recursion-demo/client" 2>/dev/null || true
        break
    fi
    sleep 0.02
done
wait "$ROOT" 2>/dev/null
EC=$?

DEPTH=$(grep -c 'run_command:.*negotiationAlgorithm=noop' "$TRACE" 2>/dev/null || true)
DEPTH=${DEPTH:-0}
IPACKS=$(grep -c 'built-in: git index-pack' "$TRACE" 2>/dev/null || true)
IPACKS=${IPACKS:-0}
TMPS=$(ls client/.git/objects/pack/tmp_pack_* 2>/dev/null | wc -l | tr -d ' ')

echo ""
echo "=== without patch (root exit $EC) ==="
echo "recursion depth reached: $DEPTH"
echo "index-pack invocations:  $IPACKS"
echo "tmp_pack_* on disk:      $TMPS"
echo ""
echo "first 4 + last 2 spawns (identical line, no termination condition):"
grep -E 'built-in: git index-pack|run_command:.*negotiationAlgorithm=noop' "$TRACE" | head -4
echo "..."
grep -E 'built-in: git index-pack|run_command:.*negotiationAlgorithm=noop' "$TRACE" | tail -2

echo ""
echo "=== with GIT_NO_LAZY_FETCH=1 (the patch) ==="
rm -rf client; rm -f "$TRACE"
GIT_CONFIG_GLOBAL= git clone -q --no-local --filter=blob:none --no-checkout \
    "file://$W/server.git" client
GIT_NO_LAZY_FETCH=1 GIT_TRACE="$TRACE" GIT_CONFIG_GLOBAL="$W/fake-global.gitconfig" \
    git -C client index-pack --stdin --fix-thin < thin.pack 2>&1 || true
CTRL_DEPTH=$(grep -c 'run_command:.*negotiationAlgorithm=noop' "$TRACE" 2>/dev/null || true)
CTRL_IPACKS=$(grep -c 'built-in: git index-pack' "$TRACE" 2>/dev/null || true)
CTRL_TMPS=$(ls client/.git/objects/pack/tmp_pack_* 2>/dev/null | wc -l | tr -d ' ')
echo "recursion depth:         ${CTRL_DEPTH:-0}"
echo "index-pack invocations:  ${CTRL_IPACKS:-0}"
echo "tmp_pack_* on disk:      $CTRL_TMPS"

rm -rf "$W/client"

Paul

Re: [PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch

[Patrick Steinhardt]

On Wed, Mar 11, 2026 at 08:18:46AM -0600, Paul Tarjan wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Is this a theoretical concern or a practical one? I would expect that
> > backfill fetches never cause the server side to send a pack with
> > REF_DELTA objects to nonexistent objects. And if they did they are
> > broken.
> 
> Practical. We hit this at Anthropic: 276 GB of promisor packs written
> by `git maintenance --task=prefetch` in 90 minutes against a ~10 GB
> monorepo with ~61K stale prefetch refs pointing at GC'd commits.

I must be misunderstanding something here, but how is it that a commit
can be garbage collected if a ref points to it? That shouldn't ever
happen, as reachable commits should not be pruned.

Or do you mean to say that the commits don't exist on the server side
anymore?

> > Hm. Can we craft a test that shows us the resulting failure in practice?
> > Testing for the environment variable feels like a bad proxy to me, as
> > I'd rather want to learn how Git would fail now.
> 
> Good point. Reworked the test in v3. It now injects a thin pack
> containing a REF_DELTA against a missing base via HTTP (using the
> replace_packfile pattern from t5616). This triggers the actual
> recursion path: index-pack encounters the missing base, calls
> promisor_remote_get_direct(), which hits the GIT_NO_LAZY_FETCH=1
> guard and fails with "lazy fetching disabled". Without the fix,
> the depth-2 fetch would proceed and potentially recurse.

Great, thanks.

> > Okay, so this seems to be an issue that can be hit in the wild. But I
> > have to wonder whether this really is a bug on the client-side, or
> > whether this is a bug that actually sits on your server. So ultimately:
> > why does the server send REF_DELTA objects in the first place? Is it
> > using git-upload-pack(1), or is it using a different implementation of
> > Git to serve data?
> 
> The server is GitHub. I did a blob:none partial clone and after some
> further git operations ended up in this state. I don't have
> server-side data on why it sent REF_DELTAs against missing bases.

That's certainly curious. Do you maybe have multiple remotes attached to
the repository, or are you dropping/modifying the object filter at some
point?

All subsequent fetches need to use the same object filter as you've used
during the initial clone, otherwise you may run into a situation as you
have described. But in theory, Git knows to continue using the filter.

> > Note that I'm not arguing that we shouldn't have protection on the
> > client, too. But I'd first like to understand whether there is a bug
> > lurking somewhere that causes us to send invalid packfiles.
> 
> Agreed, there may well be a server-side bug here. Regardless, the
> client should fail fast rather than consume unbounded resources.

Probably, yes. What I'm trying to figure out is whether there are edge
cases here where it's _valid_ for the server to send a thin pack with a
REF_DELTA. Because if so, unconditionally disabling the lazy fetches
would break such edge cases.

I don't think there are such cases, but I wouldn't consider myself an
expert with partial clones.

Cc'd Peff, as he's implemented a couple fixes in this area a couple
years ago.

Patrick

Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch

[Paul Tarjan]

Patrick Steinhardt <ps@pks.im> writes:

> I must be misunderstanding something here, but how is it that a commit
> can be garbage collected if a ref points to it? That shouldn't ever
> happen, as reachable commits should not be pruned.
>
> Or do you mean to say that the commits don't exist on the server side
> anymore?

Sloppy wording on my part — "GC'd" is wrong. These refs pointed at
commits that were promised but never materialized on the partial
clone. The ~77K broken refs looked like:

  error: refs/prefetch/remotes/origin/claude/add-azure-dependencies-EaaDn \
         does not point to a valid object!

They were created by git maintenance --task=prefetch, which runs
git fetch --prefetch --prune and writes refs/prefetch/remotes/origin/<branch>
pointing at the remote tip. On a blob:none partial clone it fetches
commit/tree metadata, but some referenced commits were never
actually downloaded before the upstream branches (ephemeral
CI/automation branches, force-pushed and deleted within days)
disappeared.

This is a red herring for the patch though. The stale prefetch refs
explain why the outer fetch got a thin pack — the client advertised
haves from promised-but-absent commits. But the recursion (depth-1
to depth-2+) is entirely inside fetch_objects() with noop
negotiation, independent of any refs.

I'll fix the commit message wording in a v4 if you'd like.

> That's certainly curious. Do you maybe have multiple remotes attached to
> the repository, or are you dropping/modifying the object filter at some
> point?
>
> All subsequent fetches need to use the same object filter as you've used
> during the initial clone, otherwise you may run into a situation as you
> have described. But in theory, Git knows to continue using the filter.

Nobody intentionally changed the filter. What happened is the
lazy-fetch child kept re-writing it. fetch_objects() hardcodes
--filter=blob:none on the child argv, and the child's
builtin/fetch.c writes the active filter to config.

23547c40 ("fetch: do not override partial clone filter", 2020)
guards this write behind a check for an already-set filter. But I
was unsetting remote.origin.partialclonefilter manually trying to
stop the storm, so the guard passed and the next lazy-fetch child
wrote it right back:

  21:45  (unset)     manual git config --unset
  22:05  blob:none   re-written by a lazy-fetch child
  22:11  (unset)     manual unset again
  23:11  blob:none   re-written again
  23:13  (unset)     manual unset
  23:28  blob:none   re-written (caught live by a config-mtime trap)

The actual mitigation was unsetting remote.origin.promisor too —
with no promisor remote, fix_unresolved_deltas() skips the prefetch
entirely.

This is arguably a separate bug: fetch_objects() should probably
pass -c remote.<name>.partialclonefilter=blob:none to override for
the single invocation, rather than --filter=blob:none which
persists to config. Not in scope for this patch, but I could follow
up separately if there's interest.

Paul

Re: [PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch

[Junio C Hamano]

Paul Tarjan <paul@paultarjan.com> writes:

> Patrick Steinhardt <ps@pks.im> writes:
>
>> I must be misunderstanding something here, but how is it that a commit
>> can be garbage collected if a ref points to it? That shouldn't ever
>> happen, as reachable commits should not be pruned.
>>
>> Or do you mean to say that the commits don't exist on the server side
>> anymore?
>
> Sloppy wording on my part — "GC'd" is wrong. These refs pointed at
> commits that were promised but never materialized on the partial
> clone. The ~77K broken refs looked like:
> ...
> This is arguably a separate bug: fetch_objects() should probably
> pass -c remote.<name>.partialclonefilter=blob:none to override for
> the single invocation, rather than --filter=blob:none which
> persists to config. Not in scope for this patch, but I could follow
> up separately if there's interest.

So, is this topic still viable?

At least I see that v3 was not satisfactory enough from the
discussion thread, but do we know what needs updating, how much more
work is needed, and where we want to go?

For now I'll drop the copy I have (from more than a month ago) from
my tree.

[PATCH v2] promisor-remote: prevent lazy-fetch recursion in child fetch

[Paul Tarjan via GitGitGadget]

From: Paul Tarjan <github@paulisageek.com>

fetch_objects() spawns a child `git fetch` to lazily fill in missing
objects. That child's index-pack, when it receives a thin pack
containing a REF_DELTA against a still-missing base, explicitly
calls promisor_remote_get_direct() — which is fetch_objects() again.
If the base is truly unavailable (e.g. because many refs in the
local store point at objects that have been garbage-collected on the
server), each recursive lazy-fetch can trigger another, leading to
unbounded recursion with runaway disk and process consumption.

The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add
--no-lazy-fetch option, 2021-08-31)) already exists at the top of
fetch_objects(); the missing piece is propagating it into the child
fetch's environment. Add that propagation so the child's
index-pack, if it encounters a REF_DELTA against a missing base,
hits the guard and fails fast instead of recursing.

Depth-1 lazy fetch (the whole point of fetch_objects()) is
unaffected: only the child and its descendants see the variable.
With negotiationAlgorithm=noop the client advertises no "have"
lines, so a well-behaved server sends requested objects
un-deltified or deltified only against objects in the same pack;
the child's index-pack should never need a depth-2 fetch. If it
does, the server response was broken or the local store is already
corrupt, and further fetching would not help.

This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy
fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a
different entry point.

Add a test that verifies the child fetch environment contains
GIT_NO_LAZY_FETCH=1 via a reference-transaction hook.

Signed-off-by: Paul Tarjan <github@paulisageek.com>
---
    promisor-remote: prevent recursive lazy-fetch during index-pack
    
    Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by
    fetch_objects() so that index-pack cannot recurse back into lazy-fetch
    when resolving REF_DELTA bases.
    
    We hit this in production: 276 GB of promisor packs written in 90
    minutes against a 100 GB monorepo with ~61K stale prefetch refs pointing
    at GC'd commits.
    
    Changes since v1:
    
     * Dropped CC: trailers from commit message (moved here for
       GitGitGadget)
     * Moved test into t0411-clone-from-partial.sh instead of a new file
     * Removed duplicate commit-message summary from PR description

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v2
Pull-Request: https://github.com/git/git/pull/2224

Range-diff vs v1:

 1:  7b723441f4 ! 1:  907ca7a0ac promisor-remote: prevent lazy-fetch recursion in child fetch
     @@ Commit message
          different entry point.
      
          Add a test that verifies the child fetch environment contains
     -    GIT_NO_LAZY_FETCH=1 via a reference-transaction hook, and that
     -    only one fetch subprocess is spawned.
     +    GIT_NO_LAZY_FETCH=1 via a reference-transaction hook.
      
     -    Cc: Jonathan Tan <jonathantanmy@google.com>
     -    Cc: Han Xin <hanxin.hx@bytedance.com>
     -    Cc: Jeff Hostetler <jeffhostetler@github.com>
     -    Cc: Christian Couder <christian.couder@gmail.com>
          Signed-off-by: Paul Tarjan <github@paulisageek.com>
      
       ## promisor-remote.c ##
     @@ promisor-remote.c: static int fetch_objects(struct repository *repo,
       		     "fetch", remote_name, "--no-tags",
       		     "--no-write-fetch-head", "--recurse-submodules=no",
      
     - ## t/meson.build ##
     -@@ t/meson.build: integration_tests = [
     -   't0303-credential-external.sh',
     -   't0410-partial-clone.sh',
     -   't0411-clone-from-partial.sh',
     -+  't0412-promisor-no-lazy-fetch-recursion.sh',
     -   't0450-txt-doc-vs-help.sh',
     -   't0500-progress-display.sh',
     -   't0600-reffiles-backend.sh',
     -
     - ## t/t0412-promisor-no-lazy-fetch-recursion.sh (new) ##
     -@@
     -+#!/bin/sh
     -+
     -+test_description='promisor-remote: no recursive lazy-fetch
     -+
     -+Verify that fetch_objects() sets GIT_NO_LAZY_FETCH=1 in the child
     -+fetch environment, so that index-pack cannot recurse back into
     -+fetch_objects() when resolving REF_DELTA bases.
     -+'
     -+
     -+. ./test-lib.sh
     -+
     -+test_expect_success 'setup' '
     -+	test_create_repo server &&
     -+	test_commit -C server foo &&
     -+	git -C server repack -a -d --write-bitmap-index &&
     + ## t/t0411-clone-from-partial.sh ##
     +@@ t/t0411-clone-from-partial.sh: test_expect_success 'promisor lazy-fetching can be re-enabled' '
     + 	test_path_is_file script-executed
     + '
     + 
     ++test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' '
     ++	test_create_repo nolazy-server &&
     ++	test_commit -C nolazy-server foo &&
     ++	git -C nolazy-server repack -a -d --write-bitmap-index &&
      +
     -+	git clone "file://$(pwd)/server" client &&
     -+	HASH=$(git -C client rev-parse foo) &&
     -+	rm -rf client/.git/objects/* &&
     -+
     -+	git -C client config core.repositoryformatversion 1 &&
     -+	git -C client config extensions.partialclone "origin"
     -+'
     -+
     -+test_expect_success 'lazy-fetch spawns only one fetch subprocess' '
     -+	GIT_TRACE="$(pwd)/trace" git -C client cat-file -p "$HASH" &&
     -+
     -+	grep "git fetch" trace >fetches &&
     -+	test_line_count = 1 fetches
     -+'
     ++	git clone "file://$(pwd)/nolazy-server" nolazy-client &&
     ++	HASH=$(git -C nolazy-client rev-parse foo) &&
     ++	rm -rf nolazy-client/.git/objects/* &&
      +
     -+test_expect_success 'child of lazy-fetch has GIT_NO_LAZY_FETCH=1' '
     -+	rm -rf client/.git/objects/* &&
     ++	git -C nolazy-client config core.repositoryformatversion 1 &&
     ++	git -C nolazy-client config extensions.partialclone "origin" &&
      +
      +	# Install a reference-transaction hook to record the env var
      +	# as seen by processes inside the child fetch.
     -+	test_hook -C client reference-transaction <<-\EOF &&
     ++	test_hook -C nolazy-client reference-transaction <<-\EOF &&
      +	echo "$GIT_NO_LAZY_FETCH" >>../env-in-child
      +	EOF
      +
      +	rm -f env-in-child &&
     -+	git -C client cat-file -p "$HASH" &&
     ++	git -C nolazy-client cat-file -p "$HASH" &&
      +
      +	# The hook runs inside the child fetch, which should have
      +	# GIT_NO_LAZY_FETCH=1 in its environment.
      +	grep "^1$" env-in-child
      +'
      +
     -+test_done
     + test_done


 promisor-remote.c             |  7 +++++++
 t/t0411-clone-from-partial.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/promisor-remote.c b/promisor-remote.c
index 96fa215b06..35c7aab93d 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,
 	child.in = -1;
 	if (repo != the_repository)
 		prepare_other_repo_env(&child.env, repo->gitdir);
+	/*
+	 * Prevent the child's index-pack from recursing back into
+	 * fetch_objects() when resolving REF_DELTA bases it does not
+	 * have.  With noop negotiation the server should never need
+	 * to send such deltas, so a depth-2 fetch would not help.
+	 */
+	strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT);
 	strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop",
 		     "fetch", remote_name, "--no-tags",
 		     "--no-write-fetch-head", "--recurse-submodules=no",
diff --git a/t/t0411-clone-from-partial.sh b/t/t0411-clone-from-partial.sh
index 9e6bca5625..10a829fb80 100755
--- a/t/t0411-clone-from-partial.sh
+++ b/t/t0411-clone-from-partial.sh
@@ -78,4 +78,30 @@ test_expect_success 'promisor lazy-fetching can be re-enabled' '
 	test_path_is_file script-executed
 '
 
+test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' '
+	test_create_repo nolazy-server &&
+	test_commit -C nolazy-server foo &&
+	git -C nolazy-server repack -a -d --write-bitmap-index &&
+
+	git clone "file://$(pwd)/nolazy-server" nolazy-client &&
+	HASH=$(git -C nolazy-client rev-parse foo) &&
+	rm -rf nolazy-client/.git/objects/* &&
+
+	git -C nolazy-client config core.repositoryformatversion 1 &&
+	git -C nolazy-client config extensions.partialclone "origin" &&
+
+	# Install a reference-transaction hook to record the env var
+	# as seen by processes inside the child fetch.
+	test_hook -C nolazy-client reference-transaction <<-\EOF &&
+	echo "$GIT_NO_LAZY_FETCH" >>../env-in-child
+	EOF
+
+	rm -f env-in-child &&
+	git -C nolazy-client cat-file -p "$HASH" &&
+
+	# The hook runs inside the child fetch, which should have
+	# GIT_NO_LAZY_FETCH=1 in its environment.
+	grep "^1$" env-in-child
+'
+
 test_done

base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5
-- 
gitgitgadget

[PATCH v3] promisor-remote: prevent lazy-fetch recursion in child fetch

[Paul Tarjan via GitGitGadget]

From: Paul Tarjan <github@paulisageek.com>

fetch_objects() spawns a child `git fetch` to lazily fill in missing
objects. That child's index-pack, when it receives a thin pack
containing a REF_DELTA against a still-missing base, calls
promisor_remote_get_direct() -- which is fetch_objects() again.

With negotiationAlgorithm=noop the client advertises no "have"
lines, so a well-behaved server sends requested objects
un-deltified or deltified only against objects in the same pack.
A server that nevertheless sends REF_DELTA against a base the
client does not have is misbehaving; however the client should
not recurse unboundedly in response.

Propagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment
so that if the child's index-pack encounters such a REF_DELTA, it
hits the existing guard at the top of fetch_objects() and fails
fast instead of recursing.  Depth-1 lazy fetch (the whole point
of fetch_objects()) is unaffected: only the child and its
descendants see the variable.

Add a test that injects a thin pack containing a REF_DELTA against
a missing base via HTTP, triggering the recursion path through
index-pack's promisor_remote_get_direct() call.  With the fix, the
child's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the
depth-2 fetch with a "lazy fetching disabled" warning.

Signed-off-by: Paul Tarjan <github@paulisageek.com>
---
    promisor-remote: prevent recursive lazy-fetch during index-pack
    
    Propagate GIT_NO_LAZY_FETCH=1 into the child fetch spawned by
    fetch_objects() so that index-pack cannot recurse back into lazy-fetch
    when resolving REF_DELTA bases.
    
    We hit this in production: 276 GB of promisor packs written in 90
    minutes against a ~10 GB monorepo with ~61K stale prefetch refs pointing
    at GC'd commits.
    
    Changes since v2:
    
     * Replaced env-var-proxy test with behavioral test that injects a thin
       pack containing a REF_DELTA against a missing base via HTTP,
       triggering the actual recursion path through index-pack's
       promisor_remote_get_direct() call
     * Moved test from t0411-clone-from-partial.sh to t5616-partial-clone.sh
       (requires HTTP infrastructure)
     * Dropped commit-graph comparison from commit message per review
       feedback
    
    Changes since v1:
    
     * Dropped CC: trailers from commit message (moved here for
       GitGitGadget)
     * Moved test into t0411-clone-from-partial.sh instead of a new file
     * Removed duplicate commit-message summary from PR description

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2224%2Fptarjan%2Fclaude%2Ffix-lazy-fetch-recursion-KP9Hl-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2224/ptarjan/claude/fix-lazy-fetch-recursion-KP9Hl-v3
Pull-Request: https://github.com/git/git/pull/2224

Range-diff vs v2:

 1:  907ca7a0ac ! 1:  d58ccb858f promisor-remote: prevent lazy-fetch recursion in child fetch
     @@ Commit message
      
          fetch_objects() spawns a child `git fetch` to lazily fill in missing
          objects. That child's index-pack, when it receives a thin pack
     -    containing a REF_DELTA against a still-missing base, explicitly
     -    calls promisor_remote_get_direct() — which is fetch_objects() again.
     -    If the base is truly unavailable (e.g. because many refs in the
     -    local store point at objects that have been garbage-collected on the
     -    server), each recursive lazy-fetch can trigger another, leading to
     -    unbounded recursion with runaway disk and process consumption.
     +    containing a REF_DELTA against a still-missing base, calls
     +    promisor_remote_get_direct() -- which is fetch_objects() again.
      
     -    The GIT_NO_LAZY_FETCH guard (introduced by e6d5479e7a (git: add
     -    --no-lazy-fetch option, 2021-08-31)) already exists at the top of
     -    fetch_objects(); the missing piece is propagating it into the child
     -    fetch's environment. Add that propagation so the child's
     -    index-pack, if it encounters a REF_DELTA against a missing base,
     -    hits the guard and fails fast instead of recursing.
     -
     -    Depth-1 lazy fetch (the whole point of fetch_objects()) is
     -    unaffected: only the child and its descendants see the variable.
          With negotiationAlgorithm=noop the client advertises no "have"
          lines, so a well-behaved server sends requested objects
     -    un-deltified or deltified only against objects in the same pack;
     -    the child's index-pack should never need a depth-2 fetch. If it
     -    does, the server response was broken or the local store is already
     -    corrupt, and further fetching would not help.
     +    un-deltified or deltified only against objects in the same pack.
     +    A server that nevertheless sends REF_DELTA against a base the
     +    client does not have is misbehaving; however the client should
     +    not recurse unboundedly in response.
      
     -    This is the same bug shape that 3a1ea94a49 (commit-graph.c: no lazy
     -    fetch in lookup_commit_in_graph(), 2022-07-01) addressed at a
     -    different entry point.
     +    Propagate GIT_NO_LAZY_FETCH=1 into the child fetch's environment
     +    so that if the child's index-pack encounters such a REF_DELTA, it
     +    hits the existing guard at the top of fetch_objects() and fails
     +    fast instead of recursing.  Depth-1 lazy fetch (the whole point
     +    of fetch_objects()) is unaffected: only the child and its
     +    descendants see the variable.
      
     -    Add a test that verifies the child fetch environment contains
     -    GIT_NO_LAZY_FETCH=1 via a reference-transaction hook.
     +    Add a test that injects a thin pack containing a REF_DELTA against
     +    a missing base via HTTP, triggering the recursion path through
     +    index-pack's promisor_remote_get_direct() call.  With the fix, the
     +    child's fetch_objects() sees GIT_NO_LAZY_FETCH=1 and blocks the
     +    depth-2 fetch with a "lazy fetching disabled" warning.
      
          Signed-off-by: Paul Tarjan <github@paulisageek.com>
      
     @@ promisor-remote.c: static int fetch_objects(struct repository *repo,
       		     "fetch", remote_name, "--no-tags",
       		     "--no-write-fetch-head", "--recurse-submodules=no",
      
     - ## t/t0411-clone-from-partial.sh ##
     -@@ t/t0411-clone-from-partial.sh: test_expect_success 'promisor lazy-fetching can be re-enabled' '
     - 	test_path_is_file script-executed
     + ## t/t5616-partial-clone.sh ##
     +@@ t/t5616-partial-clone.sh: test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against
     + 	! test -e "$HTTPD_ROOT_PATH/one-time-script"
       '
       
     -+test_expect_success 'lazy-fetch child has GIT_NO_LAZY_FETCH=1' '
     -+	test_create_repo nolazy-server &&
     -+	test_commit -C nolazy-server foo &&
     -+	git -C nolazy-server repack -a -d --write-bitmap-index &&
     ++test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' '
     ++	SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
     ++	rm -rf "$SERVER" repo &&
     ++	test_create_repo "$SERVER" &&
     ++	test_config -C "$SERVER" uploadpack.allowfilter 1 &&
     ++	test_config -C "$SERVER" uploadpack.allowanysha1inwant 1 &&
     ++
     ++	# Create a commit with 2 blobs to be used as delta base and content.
     ++	for i in $(test_seq 10)
     ++	do
     ++		echo "this is a line" >>"$SERVER/foo.txt" &&
     ++		echo "this is another line" >>"$SERVER/bar.txt" || return 1
     ++	done &&
     ++	git -C "$SERVER" add foo.txt bar.txt &&
     ++	git -C "$SERVER" commit -m initial &&
     ++	BLOB_FOO=$(git -C "$SERVER" rev-parse HEAD:foo.txt) &&
     ++	BLOB_BAR=$(git -C "$SERVER" rev-parse HEAD:bar.txt) &&
      +
     -+	git clone "file://$(pwd)/nolazy-server" nolazy-client &&
     -+	HASH=$(git -C nolazy-client rev-parse foo) &&
     -+	rm -rf nolazy-client/.git/objects/* &&
     ++	# Partial clone with blob:none. The client has commits and
     ++	# trees but no blobs.
     ++	test_config -C "$SERVER" protocol.version 2 &&
     ++	git -c protocol.version=2 clone --no-checkout \
     ++		--filter=blob:none $HTTPD_URL/one_time_script/server repo &&
      +
     -+	git -C nolazy-client config core.repositoryformatversion 1 &&
     -+	git -C nolazy-client config extensions.partialclone "origin" &&
     ++	# Sanity check: client does not have either blob locally.
     ++	git -C repo rev-list --objects --ignore-missing \
     ++		-- $BLOB_FOO >objlist &&
     ++	test_line_count = 0 objlist &&
      +
     -+	# Install a reference-transaction hook to record the env var
     -+	# as seen by processes inside the child fetch.
     -+	test_hook -C nolazy-client reference-transaction <<-\EOF &&
     -+	echo "$GIT_NO_LAZY_FETCH" >>../env-in-child
     ++	# Craft a thin pack where BLOB_FOO is a REF_DELTA against
     ++	# BLOB_BAR. Since the client has neither blob (blob:none
     ++	# filter), the delta base will be missing. This simulates a
     ++	# misbehaving server that sends REF_DELTA against an object
     ++	# the client does not have.
     ++	test-tool -C "$SERVER" pack-deltas --num-objects=1 >thin.pack <<-EOF &&
     ++	REF_DELTA $BLOB_FOO $BLOB_BAR
      +	EOF
      +
     -+	rm -f env-in-child &&
     -+	git -C nolazy-client cat-file -p "$HASH" &&
     ++	replace_packfile thin.pack &&
      +
     -+	# The hook runs inside the child fetch, which should have
     -+	# GIT_NO_LAZY_FETCH=1 in its environment.
     -+	grep "^1$" env-in-child
     ++	# Trigger a lazy fetch for BLOB_FOO. The child fetch spawned
     ++	# by fetch_objects() receives our crafted thin pack. Its
     ++	# index-pack encounters the missing delta base (BLOB_BAR) and
     ++	# tries to lazy-fetch it via promisor_remote_get_direct().
     ++	#
     ++	# With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1
     ++	# to the child, so the depth-2 fetch is blocked and we see the
     ++	# "lazy fetching disabled" warning. The object cannot be
     ++	# resolved, so cat-file fails.
     ++	#
     ++	# Without the fix: the depth-2 fetch would proceed, potentially
     ++	# recursing unboundedly with a persistently misbehaving server.
     ++	test_must_fail git -C repo -c protocol.version=2 \
     ++		cat-file -p $BLOB_FOO 2>err &&
     ++	test_grep "lazy fetching disabled" err &&
     ++
     ++	# Ensure that the one-time-script was used.
     ++	! test -e "$HTTPD_ROOT_PATH/one-time-script"
      +'
      +
     - test_done
     + # DO NOT add non-httpd-specific tests here, because the last part of this
     + # test script is only executed when httpd is available and enabled.
     + 


 promisor-remote.c        |  7 +++++
 t/t5616-partial-clone.sh | 60 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/promisor-remote.c b/promisor-remote.c
index 96fa215b06..35c7aab93d 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -42,6 +42,13 @@ static int fetch_objects(struct repository *repo,
 	child.in = -1;
 	if (repo != the_repository)
 		prepare_other_repo_env(&child.env, repo->gitdir);
+	/*
+	 * Prevent the child's index-pack from recursing back into
+	 * fetch_objects() when resolving REF_DELTA bases it does not
+	 * have.  With noop negotiation the server should never need
+	 * to send such deltas, so a depth-2 fetch would not help.
+	 */
+	strvec_pushf(&child.env, "%s=1", NO_LAZY_FETCH_ENVIRONMENT);
 	strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop",
 		     "fetch", remote_name, "--no-tags",
 		     "--no-write-fetch-head", "--recurse-submodules=no",
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 1e354e057f..27f131c8d9 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -907,6 +907,66 @@ test_expect_success PERL_TEST_HELPERS 'tolerate server sending REF_DELTA against
 	! test -e "$HTTPD_ROOT_PATH/one-time-script"
 '
 
+test_expect_success PERL_TEST_HELPERS 'lazy-fetch of REF_DELTA with missing base does not recurse' '
+	SERVER="$HTTPD_DOCUMENT_ROOT_PATH/server" &&
+	rm -rf "$SERVER" repo &&
+	test_create_repo "$SERVER" &&
+	test_config -C "$SERVER" uploadpack.allowfilter 1 &&
+	test_config -C "$SERVER" uploadpack.allowanysha1inwant 1 &&
+
+	# Create a commit with 2 blobs to be used as delta base and content.
+	for i in $(test_seq 10)
+	do
+		echo "this is a line" >>"$SERVER/foo.txt" &&
+		echo "this is another line" >>"$SERVER/bar.txt" || return 1
+	done &&
+	git -C "$SERVER" add foo.txt bar.txt &&
+	git -C "$SERVER" commit -m initial &&
+	BLOB_FOO=$(git -C "$SERVER" rev-parse HEAD:foo.txt) &&
+	BLOB_BAR=$(git -C "$SERVER" rev-parse HEAD:bar.txt) &&
+
+	# Partial clone with blob:none. The client has commits and
+	# trees but no blobs.
+	test_config -C "$SERVER" protocol.version 2 &&
+	git -c protocol.version=2 clone --no-checkout \
+		--filter=blob:none $HTTPD_URL/one_time_script/server repo &&
+
+	# Sanity check: client does not have either blob locally.
+	git -C repo rev-list --objects --ignore-missing \
+		-- $BLOB_FOO >objlist &&
+	test_line_count = 0 objlist &&
+
+	# Craft a thin pack where BLOB_FOO is a REF_DELTA against
+	# BLOB_BAR. Since the client has neither blob (blob:none
+	# filter), the delta base will be missing. This simulates a
+	# misbehaving server that sends REF_DELTA against an object
+	# the client does not have.
+	test-tool -C "$SERVER" pack-deltas --num-objects=1 >thin.pack <<-EOF &&
+	REF_DELTA $BLOB_FOO $BLOB_BAR
+	EOF
+
+	replace_packfile thin.pack &&
+
+	# Trigger a lazy fetch for BLOB_FOO. The child fetch spawned
+	# by fetch_objects() receives our crafted thin pack. Its
+	# index-pack encounters the missing delta base (BLOB_BAR) and
+	# tries to lazy-fetch it via promisor_remote_get_direct().
+	#
+	# With the fix: fetch_objects() propagates GIT_NO_LAZY_FETCH=1
+	# to the child, so the depth-2 fetch is blocked and we see the
+	# "lazy fetching disabled" warning. The object cannot be
+	# resolved, so cat-file fails.
+	#
+	# Without the fix: the depth-2 fetch would proceed, potentially
+	# recursing unboundedly with a persistently misbehaving server.
+	test_must_fail git -C repo -c protocol.version=2 \
+		cat-file -p $BLOB_FOO 2>err &&
+	test_grep "lazy fetching disabled" err &&
+
+	# Ensure that the one-time-script was used.
+	! test -e "$HTTPD_ROOT_PATH/one-time-script"
+'
+
 # DO NOT add non-httpd-specific tests here, because the last part of this
 # test script is only executed when httpd is available and enabled.
 

base-commit: 7b2bccb0d58d4f24705bf985de1f4612e4cf06e5
-- 
gitgitgadget