ref:93e9e00a58579c11e5c9b2dcacb60193a0b3baee

Fix 6 interop bugs found via git binary testing (18 failing tests)

1. Commit extra header ordering: preserve gpgsig position in extra_headers during parsing so encode_content emits headers in original order 2. Commit empty header value: don't add trailing space for valueless headers 3. Tag multiline extra headers: use continuation line encoding (space prefix) 4. Tag empty header value: same empty-value fix as commits 5. Filesystem ref listing: filter out .lock and .tmp files 6. Ref validation: add missing git-check-ref-format rules (control chars, space, ~, ^, :, ?, *, [, @{, dot-prefixed components, trailing dot) Closes #5 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
SHA: 93e9e00a58579c11e5c9b2dcacb60193a0b3baee
Author: Cole Christensen <cole.christensen@macmillan.com>
Date: 2026-02-11 06:29
Parents: 543ccf6
5 files changed +500 -4
Type
lib/ex_git_objectstore/object/commit.ex +11 −2
@@ -58,13 +58,17 @@
lines = lines ++ ["author #{commit.author}\n", "committer #{commit.committer}\n"]
gpgsig_in_extra = Enum.any?(commit.extra_headers, fn {key, _} -> key == "gpgsig" end)
# If gpgsig is set but NOT in extra_headers, emit it separately (programmatic creation)
lines =
if commit.gpgsig && !gpgsig_in_extra do
if commit.gpgsig do
lines ++ [encode_multiline_header("gpgsig", commit.gpgsig)]
else
lines
end
# Emit all extra_headers in order (includes gpgsig at its original position if parsed)
lines =
lines ++
Enum.map(commit.extra_headers, fn {key, value} ->
@@ -104,6 +108,8 @@
# Encode a header value that may contain newlines as continuation lines.
# Per the git spec, continuation lines are prefixed with a space character.
defp encode_multiline_header(key, ""), do: "#{key}\n"
defp encode_multiline_header(key, value) do
if String.contains?(value, "\n") do
# Split on newlines and prefix continuation lines with space
@@ -151,7 +157,10 @@
Map.put(acc, :committer, value)
{"gpgsig", value}, acc ->
# Store in both gpgsig field (API compat) and extra_headers (order preservation)
Map.put(acc, :gpgsig, value)
acc
|> Map.put(:gpgsig, value)
|> Map.update(:extra_headers, [{"gpgsig", value}], &(&1 ++ [{"gpgsig", value}]))
{key, value}, acc ->
Map.update(acc, :extra_headers, [{key, value}], &(&1 ++ [{key, value}]))
lib/ex_git_objectstore/object/tag.ex +17 −1
@@ -59,7 +59,7 @@
lines =
lines ++
Enum.map(tag.extra_headers || [], fn {key, value} ->
"#{key} #{value}\n"
encode_multiline_header(key, value)
end)
lines = lines ++ ["\n", tag.message]
@@ -115,5 +115,21 @@
_ ->
{:error, :invalid_tag_format}
end
end
defp encode_multiline_header(key, ""), do: "#{key}\n"
defp encode_multiline_header(key, value) do
if String.contains?(value, "\n") do
[first | rest] = String.split(value, "\n")
lines =
["#{key} #{first}\n"] ++
Enum.map(rest, fn line -> " #{line}\n" end)
IO.iodata_to_binary(lines)
else
"#{key} #{value}\n"
end
end
lib/ex_git_objectstore/ref.ex +31 −0
@@ -173,9 +173,40 @@
String.starts_with?(ref_path, "/") ->
{:error, {:invalid_ref_name, ref_path, :leading_slash}}
has_control_chars?(ref_path) ->
{:error, {:invalid_ref_name, ref_path, :control_char}}
has_forbidden_chars?(ref_path) ->
{:error, {:invalid_ref_name, ref_path, :forbidden_char}}
String.contains?(ref_path, "@{") ->
{:error, {:invalid_ref_name, ref_path, :at_brace}}
has_dot_component?(ref_path) ->
{:error, {:invalid_ref_name, ref_path, :dot_component}}
String.ends_with?(ref_path, ".") ->
{:error, {:invalid_ref_name, ref_path, :trailing_dot}}
true ->
:ok
end
end
defp has_control_chars?(str) do
str
|> :binary.bin_to_list()
|> Enum.any?(fn byte -> byte < 0x20 or byte == 0x7F end)
end
defp has_forbidden_chars?(str) do
String.match?(str, ~r/[ ~^:?*\[]/)
end
defp has_dot_component?(ref_path) do
ref_path
|> String.split("/")
|> Enum.any?(&String.starts_with?(&1, "."))
end
@doc """
lib/ex_git_objectstore/storage/filesystem.ex +6 −1
@@ -253,7 +253,12 @@
loose_refs =
case list_files_recursive(base_dir) do
{:ok, files} ->
Enum.flat_map(files, fn file ->
files
|> Enum.reject(fn file ->
basename = Path.basename(file)
String.ends_with?(basename, ".lock") or String.contains?(basename, ".tmp")
end)
|> Enum.flat_map(fn file ->
ref_name = ref_prefix <> Path.relative_to(file, base_dir)
case File.read(file) do
test/ex_git_objectstore/interop_edge_cases_test.exs +435 −0
@@ -1,0 +1,435 @@
# Copyright 2026 Cole Christensen
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
defmodule ExGitObjectstore.InteropEdgeCasesTest do
@moduledoc """
Interoperability edge case tests that verify ExGitObjectstore correctly
handles data created by the real `git` binary, and vice versa.
Every test in this file demonstrates a REAL FAILURE where the library
does not correctly interoperate with git. All tests are expected to FAIL.
When the underlying bug is fixed, the test should start passing.
"""
use ExUnit.Case, async: false
alias ExGitObjectstore.{Repo, Ref}
alias ExGitObjectstore.Object.{Commit, Tag}
alias ExGitObjectstore.Storage.Filesystem
@moduletag :interop_edge_cases
# ============================================================================
# Test helpers
# ============================================================================
defp make_tmp_dir(label) do
dir =
Path.join(
System.tmp_dir!(),
"interop_edge_#{label}_#{:erlang.unique_integer([:positive])}"
)
File.mkdir_p!(dir)
dir
end
defp git!(dir, args) do
{output, status} = System.cmd("git", args, cd: dir, stderr_to_stdout: true)
if status != 0, do: raise("git #{Enum.join(args, " ")} failed: #{output}")
String.trim(output)
end
defp make_bare_repo(label) do
dir = make_tmp_dir(label)
git!(dir, ["init", "--bare"])
dir
end
defp make_filesystem_repo(bare_dir) do
repo_id = "test_#{:erlang.unique_integer([:positive])}"
parent = Path.dirname(bare_dir)
link_base = Path.join([parent, "repos", repo_id])
File.mkdir_p!(Path.dirname(link_base))
File.rm(link_base)
File.ln_s!(bare_dir, link_base)
Repo.new(repo_id, storage: {Filesystem, %{root: parent}})
end
# ============================================================================
# BUG 1: Commit extra header ordering is not preserved on round-trip
#
# Root cause: Commit.encode_content/1 always emits gpgsig before all
# extra_headers, regardless of their original order. In real git commits,
# extra headers like "encoding" or "mergetag" can appear BEFORE gpgsig.
#
# Impact: Round-tripping a commit with encoding/mergetag before gpgsig
# produces a different byte sequence and therefore a different SHA-1,
# corrupting the commit hash.
#
# Location: lib/ex_git_objectstore/object/commit.ex, encode_content/1
# ============================================================================
describe "commit extra header ordering (BUG: gpgsig always emitted first)" do
test "encoding header before gpgsig must preserve original order" do
# Real git commits with i18n.commitEncoding produce "encoding" header
# before gpgsig. The library reorders them, breaking the hash.
raw_content =
"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" <>
"author Test <test@example.com> 1000000000 +0000\n" <>
"committer Test <test@example.com> 1000000000 +0000\n" <>
"encoding UTF-8\n" <>
"gpgsig -----BEGIN PGP SIGNATURE-----\n" <>
" iQEzBAABCAAdFiEEtest\n" <>
" =abcd\n" <>
" -----END PGP SIGNATURE-----\n" <>
"\nSigned commit with encoding\n"
{:ok, commit} = Commit.parse_content(raw_content)
re_encoded = Commit.encode_content(commit)
assert re_encoded == raw_content,
"Commit header order not preserved on round-trip.\n" <>
"The library always puts gpgsig before extra_headers,\n" <>
"but the original has encoding BEFORE gpgsig.\n\n" <>
"Original:\n#{inspect(raw_content)}\n\n" <>
"Re-encoded:\n#{inspect(re_encoded)}"
end
test "mergetag before gpgsig must preserve original order" do
# Merge commits that merge a signed tag have mergetag before gpgsig.
# This is the most common real-world case of this bug.
raw_content =
"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" <>
"parent aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" <>
"author Test <test@example.com> 1000000000 +0000\n" <>
"committer Test <test@example.com> 1000000000 +0000\n" <>
"mergetag object bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb\n" <>
" type commit\n" <>
" tag v1.0\n" <>
" tagger Test <test@example.com> 1000000000 +0000\n" <>
" \n" <>
" Release v1.0\n" <>
"gpgsig -----BEGIN PGP SIGNATURE-----\n" <>
" \n" <>
" iQEzBAABCAAdFiEEtest\n" <>
" =abcd\n" <>
" -----END PGP SIGNATURE-----\n" <>
"\nMerge tag 'v1.0'\n"
{:ok, commit} = Commit.parse_content(raw_content)
re_encoded = Commit.encode_content(commit)
assert re_encoded == raw_content,
"Merge commit with mergetag before gpgsig: order not preserved.\n" <>
"The library always emits gpgsig first, then extra_headers,\n" <>
"but this merge commit has mergetag BEFORE gpgsig.\n\n" <>
"Original:\n#{inspect(raw_content)}\n\n" <>
"Re-encoded:\n#{inspect(re_encoded)}"
end
end
# ============================================================================
# BUG 2: Commit empty header value adds trailing space on encode
#
# Root cause: encode_multiline_header/2 uses "#{key} #{value}\n" which
# produces "myheader \n" when value is "". Should produce "myheader\n".
#
# Impact: Commits with value-less headers (rare but valid) get a trailing
# space added, changing the byte sequence and SHA-1 hash.
#
# Location: lib/ex_git_objectstore/object/commit.ex, encode_multiline_header/2
# ============================================================================
describe "commit empty header value encoding (BUG: trailing space added)" do
test "header with no value must not get trailing space" do
# A header line like "myheader\n" (key only, no space, no value) is
# valid in git. The parser produces {"myheader", ""} which is correct,
# but the encoder writes "myheader \n" (with trailing space).
raw_content =
"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" <>
"author Test <test@test.com> 1000000000 +0000\n" <>
"committer Test <test@test.com> 1000000000 +0000\n" <>
"myheader\n" <>
"\nTest commit\n"
{:ok, commit} = Commit.parse_content(raw_content)
re_encoded = Commit.encode_content(commit)
assert re_encoded == raw_content,
"Empty header value round-trip failed.\n" <>
"The encoder adds a trailing space to value-less headers.\n" <>
"\"myheader\\n\" becomes \"myheader \\n\"\n\n" <>
"Original:\n#{inspect(raw_content)}\n\n" <>
"Re-encoded:\n#{inspect(re_encoded)}"
end
end
# ============================================================================
# BUG 3: Tag multiline extra header values lose continuation line format
#
# Root cause: Tag.encode_content/1 uses simple "#{key} #{value}\n" for
# extra headers, which does NOT handle multiline values. Multiline values
# need continuation lines prefixed with a space character.
#
# Impact: Tags with multiline extra headers (e.g., embedded signatures)
# get corrupted on round-trip because continuation lines are flattened.
#
# Location: lib/ex_git_objectstore/object/tag.ex, encode_content/1
# ============================================================================
describe "tag multiline extra header encoding (BUG: no continuation lines)" do
test "multiline extra header must use continuation line format" do
# Tag.parse_content correctly folds continuation lines into a single
# value with embedded newlines. But encode_content doesn't unfold
# them back into continuation lines prefixed with a space.
raw_content =
"object aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" <>
"type commit\n" <>
"tag v1.0\n" <>
"tagger Test <test@test.com> 1000000000 +0000\n" <>
"extra-header first line\n" <>
" continuation line\n" <>
"\nTag message\n"
{:ok, tag} = Tag.parse_content(raw_content)
# Verify parse worked correctly
assert tag.extra_headers == [{"extra-header", "first line\ncontinuation line"}]
re_encoded = Tag.encode_content(tag)
assert re_encoded == raw_content,
"Tag multiline extra header round-trip failed.\n" <>
"The encoder uses simple string interpolation instead of\n" <>
"proper continuation line encoding with space prefix.\n\n" <>
"Original:\n#{inspect(raw_content)}\n\n" <>
"Re-encoded:\n#{inspect(re_encoded)}"
end
end
# ============================================================================
# BUG 4: Tag empty header value adds trailing space on encode
#
# Root cause: Same as Bug 2 but in Tag.encode_content/1. Uses
# "#{key} #{value}\n" which produces "myheader \n" for empty values.
#
# Impact: Tags with value-less extra headers get corrupted on round-trip.
#
# Location: lib/ex_git_objectstore/object/tag.ex, encode_content/1
# ============================================================================
describe "tag empty header value encoding (BUG: trailing space added)" do
test "tag header with no value must not get trailing space" do
raw_content =
"object aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n" <>
"type commit\n" <>
"tag v1.0\n" <>
"tagger Test <test@test.com> 1000000000 +0000\n" <>
"myheader\n" <>
"\nTag with empty header\n"
{:ok, tag} = Tag.parse_content(raw_content)
# Parser correctly produces empty string value
assert tag.extra_headers == [{"myheader", ""}]
re_encoded = Tag.encode_content(tag)
assert re_encoded == raw_content,
"Tag empty header value round-trip failed.\n" <>
"The encoder adds a trailing space: \"myheader \\n\"\n" <>
"instead of \"myheader\\n\"\n\n" <>
"Original:\n#{inspect(raw_content)}\n\n" <>
"Re-encoded:\n#{inspect(re_encoded)}"
end
end
# ============================================================================
# BUG 5: Lock and tmp files included in ref listing
#
# Root cause: list_files_recursive in Filesystem storage returns ALL
# regular files, including .lock files (used for atomic ref updates)
# and .tmp files (used for atomic writes). Git's show-ref and branch
# commands filter these out.
#
# Impact: ExGitObjectstore.branches/1 and tags/1 return spurious entries
# like "refs/heads/main.lock" during concurrent ref updates.
#
# Location: lib/ex_git_objectstore/storage/filesystem.ex,
# list_files_recursive/2 and list_refs/3
# ============================================================================
describe "lock and tmp files in ref listing (BUG: not filtered)" do
test "branches listing must not include .lock files" do
dir = make_bare_repo("lock_refs")
try do
# Create a valid branch
commit_sha = String.duplicate("a", 40)
ref_dir = Path.join([dir, "refs", "heads"])
File.mkdir_p!(ref_dir)
File.write!(Path.join(ref_dir, "main"), commit_sha <> "\n")
# Simulate an in-progress ref update with a .lock file
File.write!(Path.join(ref_dir, "main.lock"), commit_sha <> "\n")
repo = make_filesystem_repo(dir)
{:ok, branches} = ExGitObjectstore.branches(repo)
branch_names = Enum.map(branches, &elem(&1, 0))
refute Enum.any?(branch_names, &String.contains?(&1, ".lock")),
"branches/1 returned .lock file entries: #{inspect(branch_names)}\n" <>
"Git's show-ref and branch commands filter out .lock files,\n" <>
"but the library includes them."
after
File.rm_rf!(dir)
File.rm_rf!(Path.join(Path.dirname(dir), "repos"))
end
end
test "branches listing must not include .tmp files" do
dir = make_bare_repo("tmp_refs")
try do
commit_sha = String.duplicate("a", 40)
ref_dir = Path.join([dir, "refs", "heads"])
File.mkdir_p!(ref_dir)
File.write!(Path.join(ref_dir, "main"), commit_sha <> "\n")
# Simulate an atomic write temp file
File.write!(Path.join(ref_dir, "main.tmp.12345"), commit_sha <> "\n")
repo = make_filesystem_repo(dir)
{:ok, branches} = ExGitObjectstore.branches(repo)
branch_names = Enum.map(branches, &elem(&1, 0))
refute Enum.any?(branch_names, &String.contains?(&1, ".tmp")),
"branches/1 returned .tmp file entries: #{inspect(branch_names)}\n" <>
"Temporary files from atomic writes should not appear in ref listings."
after
File.rm_rf!(dir)
File.rm_rf!(Path.join(Path.dirname(dir), "repos"))
end
end
end
# ============================================================================
# BUG 6: Ref validation is too permissive compared to git
#
# Root cause: validate_ref_name/1 only checks for a few patterns
# (.., .lock suffix, //, null byte, trailing /, leading /, empty) but
# misses many rules from git-check-ref-format:
# - No ASCII control characters (< 0x20) or DEL (0x7F)
# - No space, ~, ^, :, ?, *, [
# - No @{ sequence
# - Components cannot start with "."
# - Cannot end with "."
#
# Impact: The library accepts ref names that git rejects, which means
# refs created by the library may cause errors when accessed by git.
#
# Location: lib/ex_git_objectstore/ref.ex, validate_ref_name/1
# ============================================================================
describe "ref validation too permissive (BUG: missing git-check-ref-format rules)" do
test "ref names with spaces must be rejected" do
result = Ref.validate_ref_name("refs/heads/my branch")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/my branch' but " <>
"git check-ref-format rejects ref names containing spaces"
end
test "ref names with tilde must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature~1")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature~1' but " <>
"git check-ref-format rejects ref names containing ~"
end
test "ref names with caret must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature^2")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature^2' but " <>
"git check-ref-format rejects ref names containing ^"
end
test "ref names with colon must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature:name")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature:name' but " <>
"git check-ref-format rejects ref names containing :"
end
test "ref names with question mark must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature?")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature?' but " <>
"git check-ref-format rejects ref names containing ?"
end
test "ref names with asterisk must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature*")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature*' but " <>
"git check-ref-format rejects ref names containing *"
end
test "ref names with open bracket must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature[1]")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature[1]' but " <>
"git check-ref-format rejects ref names containing ["
end
test "ref names with control characters must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature\tname")
assert {:error, _} = result,
"validate_ref_name accepted ref with tab character but " <>
"git check-ref-format rejects ref names containing control chars"
end
test "ref names with @{ sequence must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature@{0}")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature@{0}' but " <>
"git check-ref-format rejects ref names containing the @{ sequence"
end
test "ref path components starting with dot must be rejected" do
result = Ref.validate_ref_name("refs/heads/.hidden")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/.hidden' but " <>
"git check-ref-format rejects ref names with components starting with ."
end
test "ref names ending with dot must be rejected" do
result = Ref.validate_ref_name("refs/heads/feature.")
assert {:error, _} = result,
"validate_ref_name accepted 'refs/heads/feature.' but " <>
"git check-ref-format rejects ref names ending with ."
end
end
end