refs: validate the name passed to git_reference_remove - #7385

Summary

git_reference_remove() never validated its name argument, so it could delete
files from the repository control directory. This validates the name the same way
git_reference_delete() already does and rejects HEAD.

Fixes #7376

The problem

git_reference_remove() passed name straight through:

int git_reference_remove(git_repository *repo, const char *name)
{
	...
	return git_refdb_delete(db, name, NULL, NULL);
}

The filesystem backend's loose_lock() (src/libgit2/refdb_fs.c) only checks
that the name is a valid filesystem path:

if (!git_path_is_valid(backend->repo, name, 0, GIT_FS_PATH_REJECT_FILESYSTEM_DEFAULTS)) {

so the name is joined onto the repository directory and loose_delete() unlinks
whatever is there. git_reference_delete() never had this problem because it
operates on an already-looked-up git_reference and explicitly refuses HEAD;
the by-name path had neither check.

Reproduced before the change

Against main at 0551dfd, on a fresh git_repository_init() repo (macOS,
clang, files backend):

  config         exists-before=1  remove()=0  exists-after=0
  description    exists-before=1  remove()=0  exists-after=0
  HEAD           exists-before=1  remove()=0  exists-after=0

Each call returned success and the file was gone. After the change:

  config         exists-before=1  remove()=-12  exists-after=1   (GIT_EINVALIDSPEC)
  description    exists-before=1  remove()=-12  exists-after=1   (GIT_EINVALIDSPEC)
  HEAD           exists-before=1  remove()=-1   exists-after=1   ("cannot delete HEAD")

The change

src/libgit2/refs.c — validate, then reject HEAD, before touching the refdb:

if ((error = git_reference__normalize_name(NULL, name,
		GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL)) < 0)
	return error;

if (!strcmp(name, GIT_HEAD_FILE)) {
	git_error_set(GIT_ERROR_REFERENCE, "cannot delete HEAD");
	return GIT_ERROR;
}

GIT_REFERENCE_FORMAT_ALLOW_ONELEVEL is the flag the create and lookup paths
already use (reference_normalize_for_repo), so this does not make remove
stricter than the rest of the reference API. is_valid_normalized_name()
constrains one-level names to [A-Z_]+ (not starting or ending with _), which
is what excludes config, description and index; HEAD passes that test,
hence the explicit guard, worded and returning the same way
git_reference_delete() does.

Passing NULL as the output buffer is the supported "validate only" mode of
git_reference__normalize_name (normalize = (buf != NULL)), so nothing is
allocated on this path.

Edge cases a reviewer will probably ask about

• Legitimate one-level refs still work. ORIG_HEAD, FETCH_HEAD,
  MERGE_HEAD, CHERRY_PICK_HEAD all satisfy [A-Z_]+. There is a new test
  that creates and removes ORIG_HEAD.
• @ still works. is_valid_normalized_name() special-cases a single @
  as shorthand for HEAD; that is unchanged, and it is not caught by the HEAD
  guard (strcmp against the literal name, as in git_reference_delete).
• Packed and loose. The check is in refs.c, above git_refdb_delete, so
  it applies to packed refs, loose refs and the reftable backend identically. No
  backend was touched.
• NULL name now returns an error instead of reaching the backend:
  git_reference__normalize_name starts with GIT_ASSERT_ARG(name).
• Behavior change for callers. A caller that previously removed a non-reference
  path through this API now gets GIT_EINVALIDSPEC instead of 0. That is the
  point of the fix, but it is an observable change, so the public doc comment in
  include/git2/refs.h now states that name must be a well-formed reference
  name, that HEAD cannot be removed, and that GIT_EINVALIDSPEC is possible.
  No error code that was previously returned has changed meaning.

What this does NOT fix — please read

Refname syntax is not the same thing as "lives under refs/". A name with more
than one segment that is outside refs/ is a perfectly well-formed reference
name, so it still passes normalization and still reaches the loose backend.
Verified with the patch applied:

  logs/HEAD              exists-before=1  remove()=0  exists-after=0
  hooks/pre-commit       exists-before=1  remove()=0  exists-after=0
  info/exclude           exists-before=1  remove()=0  exists-after=0
  objects/info/packs     exists-before=1  remove()=0  exists-after=0

So this PR closes the one-level cases from the issue (config, description,
index, HEAD) and leaves the multi-level cases (including logs/HEAD, which
the issue also lists) open. I did not want to imply otherwise.

Closing that gap means deciding, at the backend or at refs.c, which names
outside refs/ count as references — core git uses an explicit root-ref
allowlist (HEAD, ORIG_HEAD, FETCH_HEAD, MERGE_HEAD, CHERRY_PICK_HEAD,
REVERT_HEAD, REBASE_HEAD, AUTO_MERGE, BISECT_EXPECT_REV). libgit2 has
something adjacent in git_reference__is_pseudoref(), but it lists only
MERGE_HEAD and FETCH_HEAD and is used for lookup. Applying a policy like
that would also have to apply to git_reference_create(),
git_reference_lookup() and git_reference_rename() to stay coherent, which is
a larger API decision than a bug fix should make unilaterally. Happy to do it in
this PR or a follow-up if you tell me which shape you want.

Validation

Built on macOS 15 / arm64, clang, Ninja, -DREGEX_BACKEND=regcomp_l,
-DUSE_HTTPS=SecureTransport.

Before the change, the two new negative tests failed as expected:

1) Failure:
refs::delete::remove_head
  Function call succeeded: git_reference_remove(g_repo, "HEAD")

2) Failure:
refs::delete::remove_invalid_name
  Function call failed: (git_reference_remove(g_repo, invalid[i]))
  error 0 (expected -12)

Not run locally, so CI will be the first to cover them: the Linux/Windows
matrix, the sanitizer builds (ASan/MSan/UBSan/TSan), the leak checkers
(valgrind/leaks/win32), the -DENABLE_WERROR=ON -DDEPRECATE_HARD=ON build,
the online/network suites, and the invasive suite. The change adds no
allocations and no new branches in a hot path, so I do not expect sanitizer
movement, but I have not proven that.

No docs/changelog.md entry — that file looks like it is updated per release
rather than per PR, tell me if you want one.

Note on disclosure

#7376 is already a public issue, so I am following up on it publicly. If you
consider this a security issue that should have gone to security@libgit2.com per
SECURITY.md instead, say so and I will close this and resend privately.

Checklist

• Tests cover the functional change, and fail without it
• Public API documentation updated (include/git2/refs.h)
• Full offline + util suites pass, files and reftable backends
• Behavior change called out above
• Known remaining gap called out above rather than glossed over

Generated with Claude Code on behalf of @ethanstoner.