ref:81b6f84d5c73fe2f46b44fb2bae5f28d96a22e3e

fix(upload-pack): don't taint reachability with gitlink (mode 160000) SHAs

A v2 fetch on fangorn/hephaestus failed with `did not receive expected object 36e9a223...`. Trace: 1. Tree d85a381c has a gitlink entry `{mode: "160000", name: "ecad-drc", sha: 36e9a223...}`. By coincidence (or because the submodule URL loops back at this repo), 36e9a223 is *also* a real commit in this repo's history, reachable from every wanted branch tip. 2. collect_single_tree_entry dispatched the gitlink to the same head as regular files (the function head matched on `%{sha: sha}` without checking mode). That head did `MapSet.put(vis, sha)` before calling read_blob_entry. The read returned %Commit{}, not %Blob{}, so the entry produced no pack output — but the SHA was now in `vis`. 3. Walking parent_chain from a tip eventually reached abc92a55, whose parent is 36e9a223. collect_reachable saw 36e9a223 in `visited` and short-circuited (`{[], visited}`), never reading it, never recursing into its parents, never emitting it into the pack. 4. Result: 1043 reachable objects omitted (121 commits + 562 trees + 360 blobs). Client's index-pack rejected the resulting pack. Fix: give mode 160000 its own function head that returns `{acc, vis}` unchanged. Submodule pointers never emit anything and never touch the visited set. Regression test builds a 3-commit chain where the middle commit's tree has a gitlink whose SHA equals the root commit, and asserts all three commits land in the pack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SHA: 81b6f84d5c73fe2f46b44fb2bae5f28d96a22e3e
Author: CI <ci@anvil.test>
Date: 2026-05-14 03:32
Parents: 44a1729
2 files changed +207 -0
Type
lib/ex_git_objectstore/protocol/upload_pack_v2.ex +12 −0
@@ -850,6 +850,18 @@
{objs ++ acc, vis}
end
# Mode 160000 = gitlink (submodule pointer). The SHA names a commit
# in a different repo, so it must not enter `vis` — if the same SHA
# also appears as a real commit in this repo's ancestry, the existing
# `MapSet.put(vis, sha)` in the blob-entry head below would mark it
# visited, and the commit-graph walk would later skip over it. That
# silently prunes everything reachable through that commit. (Seen
# against fangorn/hephaestus, where a gitlink and a real commit
# happened to share SHA 36e9a223...; 1043 objects went missing.)
defp collect_single_tree_entry(_repo, %{mode: "160000"}, acc, vis, _depth, _opts) do
{acc, vis}
end
defp collect_single_tree_entry(repo, %{sha: sha}, acc, vis, _depth, opts) do
cond do
MapSet.member?(vis, sha) ->
test/ex_git_objectstore/protocol/upload_pack_v2_test.exs +195 −0
@@ -899,6 +899,201 @@
pack = extract_sideband_pack(recomposed)
assert {:ok, _entries} = Reader.parse(pack)
end
test "multi-frame fetch response is byte-identical to feed/2 (Anvil prod regression)" do
# Smaller tests fit in one sideband frame. Cloning a real repo
# from prod (~200 MB) failed with `invalid index-pack output`
# client-side. Pin behavior across many sideband frames by
# building a repo whose pack body spans dozens of frames.
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
# 30 blobs × 80 KiB random bytes ≈ 2.4 MB raw pack ≈ ~37 sideband
# frames. Random bytes so zlib can't compress them away.
blob_shas =
for i <- 1..30 do
content = "header-#{i}\n" <> :crypto.strong_rand_bytes(80 * 1024)
blob = Blob.from_content(content)
{:ok, blob_sha} = Object.write(repo, blob)
{i, content, blob_sha}
end
tree_entries =
Enum.map(blob_shas, fn {i, _content, blob_sha} ->
%{mode: "100644", name: "blob#{i}", sha: blob_sha}
end)
tree = Tree.new(tree_entries)
{:ok, tree_sha} = Object.write(repo, tree)
commit = %Commit{
tree: tree_sha,
parents: [],
author: "T <t@t.com> 1000000000 +0000",
committer: "T <t@t.com> 1000000000 +0000",
message: "multi-frame\n"
}
{:ok, commit_sha} = Object.write(repo, commit)
:ok = Ref.put(repo, "refs/heads/main", commit_sha, nil)
client_data =
PktLine.encode("command=fetch") <>
PktLine.delim() <>
PktLine.encode("want #{commit_sha}") <>
PktLine.encode("done") <>
PktLine.flush()
{_advert, s1} = UploadPackV2.init(repo)
{_advert, s2} = UploadPackV2.init(repo)
{expected, _} = UploadPackV2.feed(s1, client_data)
{streamed, new_s2} = collect_stream(s2, client_data)
# Sanity check that we actually built a multi-frame pack.
assert byte_size(expected) > 2 * 1024 * 1024
assert byte_size(streamed) == byte_size(expected),
"expected #{byte_size(expected)} bytes, got #{byte_size(streamed)}"
assert streamed == expected
assert new_s2.phase == :done
end
test "submodule (mode 160000) gitlink does NOT poison reachability of a same-SHA commit" do
# Anvil prod regression (fangorn/hephaestus): the walker treated a
# tree entry with mode "160000" as a blob, marked its target SHA as
# visited, and then skipped the commit with that SHA when the
# parent-chain traversal arrived at it. Result: 1043 reachable
# objects (121 commits + 562 trees + 360 blobs) silently pruned
# from the pack, causing the client to fail with
# `did not receive expected object <sha>`.
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
# Build: root_commit (a)
# ^
# | parent
# middle_commit (b) — has a tree with a gitlink to a
# ^ commit-SHA that *equals* root_commit.sha
# | parent
# tip_commit (c) — wanted by the client
{a_sha, _, _} = create_commit(repo, "root\n", "root\n")
f_blob = Blob.from_content("f.txt body\n")
{:ok, f_blob_sha} = Object.write(repo, f_blob)
gitlink_tree =
Tree.new([
%{mode: "100644", name: "f.txt", sha: f_blob_sha},
%{mode: "160000", name: "ecad-drc", sha: a_sha}
])
{:ok, gitlink_tree_sha} = Object.write(repo, gitlink_tree)
b = %Commit{
tree: gitlink_tree_sha,
parents: [a_sha],
author: "T <t@t.com> 1000000000 +0000",
committer: "T <t@t.com> 1000000000 +0000",
message: "middle (gitlink)\n"
}
{:ok, b_sha} = Object.write(repo, b)
{c_sha, _, _} = create_commit(repo, "tip\n", "tip\n", [b_sha])
:ok = Ref.put(repo, "refs/heads/main", c_sha, nil)
client_data =
PktLine.encode("command=fetch") <>
PktLine.delim() <>
PktLine.encode("want #{c_sha}") <>
PktLine.encode("done") <>
PktLine.flush()
{_advert, state} = UploadPackV2.init(repo)
{streamed, _new_state} = collect_stream(state, client_data)
pack = extract_sideband_pack(streamed)
assert {:ok, entries} = Reader.parse(pack)
shas_in_pack = entries |> Enum.map(& &1.sha) |> MapSet.new()
assert MapSet.member?(shas_in_pack, a_sha),
"root commit #{a_sha} (referenced by a gitlink and a parent) must be in the pack"
# All three commits must be present
for s <- [a_sha, b_sha, c_sha] do
assert MapSet.member?(shas_in_pack, s), "missing commit #{s} from pack"
end
end
test "streamed pack passes `git index-pack --stdin` (Anvil prod regression)" do
# The strongest acceptance check: feed the streamed pack bytes
# through real git's pack-indexer. If real git rejects them
# ("invalid index-pack output" was the prod symptom), this fails.
repo = RepoHelper.memory_repo()
ExGitObjectstore.init(repo)
blob_shas =
for i <- 1..30 do
content = "blob #{i}\n" <> :crypto.strong_rand_bytes(80 * 1024)
blob = Blob.from_content(content)
{:ok, blob_sha} = Object.write(repo, blob)
{i, blob_sha}
end
tree_entries =
Enum.map(blob_shas, fn {i, blob_sha} ->
%{mode: "100644", name: "blob#{i}", sha: blob_sha}
end)
tree = Tree.new(tree_entries)
{:ok, tree_sha} = Object.write(repo, tree)
commit = %Commit{
tree: tree_sha,
parents: [],
author: "T <t@t.com> 1000000000 +0000",
committer: "T <t@t.com> 1000000000 +0000",
message: "ipack\n"
}
{:ok, commit_sha} = Object.write(repo, commit)
:ok = Ref.put(repo, "refs/heads/main", commit_sha, nil)
client_data =
PktLine.encode("command=fetch") <>
PktLine.delim() <>
PktLine.encode("want #{commit_sha}") <>
PktLine.encode("done") <>
PktLine.flush()
{_advert, state} = UploadPackV2.init(repo)
{streamed, _} = collect_stream(state, client_data)
pack_body = extract_sideband_pack(streamed)
assert is_binary(pack_body) and byte_size(pack_body) > 0
tmp_dir = Path.join(System.tmp_dir!(), "anvil_ipack_#{System.unique_integer([:positive])}")
File.mkdir_p!(tmp_dir)
pack_path = Path.join(tmp_dir, "stream.pack")
File.write!(pack_path, pack_body)
try do
# `git index-pack <packfile>` parses the pack and writes a .idx
# alongside; exits non-zero if the pack is malformed. This is
# exactly the step that fails on the client during clone, so
# asserting it passes here pins the regression to a real-git
# behavior, not just our own parser.
{output, exit_code} =
System.cmd("git", ["index-pack", "-v", pack_path], stderr_to_stdout: true)
assert exit_code == 0,
"git index-pack rejected streamed pack: #{output}\npack size #{byte_size(pack_body)} bytes"
after
File.rm_rf!(tmp_dir)
end
end
end
# --- Helpers ---