feat: tag push support in S&F hook chain and proxy filters

Closes #19

Lightweight and annotated tags now flow through both push modes:

Store-and-forward hooks:
- CheckEmptyBranchHook, CheckHiddenCommitsHook: skip tag refs (no commit
  chain to validate)
- CommitInspectionService: guard pushedCommits null/empty before walking

Transparent proxy filters:
- ParseGitRequestFilter: sets branch from packet-line ref for tag pushes;
  GitReceivePackParser only handles OBJ_COMMIT so commit is left null for
  tags — downstream filters must tolerate this
- CheckUserPushPermissionFilter, CheckEmptyBranchFilter: skip refs/tags/*
  (no author email or commit list available)
- CheckHiddenCommitsFilter: dereference commitTo via ^{commit} to peel
  annotated tag objects before walking; zero-object packs resolve to null
  and are safely skipped
- AllowApprovedPushFilter: remove null-commit guard so approved tag pushes
  are recognised as pre-approved on re-push

Tests: new CheckHiddenCommitsFilterTest (lightweight + annotated tag cases
using real JGit in-memory repo); AllowApprovedPushFilterTest extended with
tagPush_approvedRecord_setsPreApproved; CheckUserPushPermissionFilterTest
and CheckEmptyBranchFilterTest extended with tag-skip cases.

Integration scripts: test/push-pass-tag.sh (S&F mode) and
test/proxy-pass-tag.sh (proxy mode, approval flow).

Docs: jgit-proxy-core/GIT_INTERNALS.md explains lightweight vs annotated
tag wire format, ^{commit} dereference, and per-filter rationale.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: coopernetes

jgit-proxy-core/GIT_INTERNALS.md

@@ -0,0 +1,124 @@
+# Git internals reference
+
+Notes on git/JGit behaviour that inform how filters and hooks are written.
+Add a section here when you hit a non-obvious edge case so the next person doesn't have to rediscover it.
+
+---
+
+## Tag objects
+
+### Lightweight vs annotated tags
+
+Git has two kinds of tags, and they behave very differently at the object level.
+
+**Lightweight tag** — just a named pointer, stored as a ref file.
+The ref value is the SHA of the commit it points to directly.
+There is no tag object in the object store.
+
+```
+refs/tags/v1.0 → a3f9c1... (commit)
+```
+
+**Annotated tag** — a first-class git object of type `tag`.
+The ref points to the tag object SHA, not the commit SHA.
+The tag object contains metadata (tagger, date, message) and a pointer to the tagged commit.
+
+```
+refs/tags/v1.0 → b7d2e4... (tag object)
+                     └─→ a3f9c1... (commit)
+```
+
+The key consequence: **`cmd.getNewId()` for an annotated tag push returns the tag object SHA, not a commit SHA.**
+Any code that calls `RevWalk.parseCommit(cmd.getNewId())` directly will throw `IncorrectObjectTypeException` for annotated tags.
+
+### The `^{commit}` dereference
+
+Both git and JGit support a peeling suffix to follow any chain of tag objects to the final commit:
+
+```java
+// Safe for both lightweight and annotated tags:
+ObjectId commitId = repository.resolve(sha + "^{commit}");
+```
+
+For a lightweight tag, `sha` is already a commit SHA — `^{commit}` is a no-op.
+For an annotated tag, JGit follows the tag → commit chain.
+For a chain of tags (a tag of a tag), it follows all the way down.
+
+`^{tree}` works the same way but stops at a tree object instead of a commit.
+
+`resolve()` returns `null` when the peel fails — for example when a tag points to a blob or tree
+rather than a commit (legal but extremely rare). Always null-check the result.
+
+### What git sends over the wire for a tag push
+
+When you run `git push origin refs/tags/v1.0`:
+
+- The packet line header is `<oldOid> <newOid> refs/tags/v1.0` (same format as a branch push).
+- For a **lightweight tag** to an already-upstream commit, git sends a thin pack with **zero objects**
+  because the commit already exists at the remote. Trying to read a pack entry from an empty pack
+  produces garbage or a `DataFormatException` — this is normal, not a corruption.
+- For an **annotated tag**, git sends a pack containing the tag object (type 4, `OBJ_TAG`).
+  The tagged commit is not included if it already exists upstream.
+- `commitFrom` (`oldOid`) is all-zeros for a new tag — the same value used for a new branch.
+  Code that uses `commitFrom == zeros` as a signal for "new branch" must also account for new tags.
+
+### How each hook/filter handles tags
+
+#### S&F hooks (`CheckEmptyBranchHook`, `CheckHiddenCommitsHook`)
+
+Tags push commits that already exist upstream.
+`CommitInspectionService.getCommitRange()` returns an empty list — the commit at the tag tip is
+already reachable from existing heads, so it is not "new".
+
+**`CheckEmptyBranchHook`** — an empty commit range on a zero-oldId ref would normally mean the branch
+has no new commits (a reject condition). For tags this is always the case and is legitimate, so the
+hook skips any ref whose name starts with `refs/tags/`.
+
+**`CheckHiddenCommitsHook`** — calls `walk.parseCommit()` on `cmd.getNewId()`.
+For an annotated tag this throws. Fix: resolve through `^{commit}` first.
+
+```java
+ObjectId commitId = repo.resolve(cmd.getNewId().name() + "^{commit}");
+if (commitId == null) continue;
+walk.markStart(walk.parseCommit(commitId));
+```
+
+All other S&F hooks (`AuthorEmailValidationHook`, `CommitMessageValidationHook`, etc.) delegate to
+`CommitInspectionService.getCommitDetails()` or `getCommitRange()`, both of which use `^{commit}`.
+They are safe transitively.
+
+#### Proxy-mode filters
+
+The proxy pipeline sees the same two objects as the S&F hooks — the packet line SHAs and the pack data —
+but runs as servlet filters without JGit's `ReceivePack` infrastructure.
+
+**`ParseGitRequestFilter`** — extracts `branch`, `commitFrom`, `commitTo` from the packet line,
+then tries to parse the first pack object as a commit.
+For a tag push this fails (the pack contains a tag object or no new objects).
+The parse exception is caught; `requestDetails.commit` is left null.
+`requestDetails.branch` is set to the full ref name (e.g. `refs/tags/v1.0`),
+so `GitRequestDetails.isTagPush()` works correctly downstream.
+
+**`CheckUserPushPermissionFilter`** — uses the commit author email to identify the pushing user.
+Null commit → null email → rejects with "Unknown User".
+Fix: skip the email check for tag pushes; the user is already verified by HTTP basic auth.
+
+**`CheckEmptyBranchFilter`** — empty `pushedCommits` + zero `commitFrom` looks like an empty branch push.
+Fix: skip for tag refs, same reasoning as the S&F hook.
+
+**`CheckHiddenCommitsFilter`** — calls `walk.parseCommit(repo.resolve(toCommit))` where `toCommit`
+is the tag object SHA.
+Fix: use `repo.resolve(toCommit + "^{commit}")`, consistent with all other `CommitInspectionService` callers.
+
+**`EnrichPushCommitsFilter`** — unpacks the pack objects into the local repo clone using JGit's
+`PackParser`, which handles tag objects fine. Then calls `CommitInspectionService.getCommitRange()`
+(fixed via `^{commit}`), which returns empty for a tag on an existing commit. Normal behaviour.
+
+**`ScanDiffFilter`** — calls `getDiff(repo, fromCommit, toCommit)`.
+`toCommit + "^{tree}"` peels through the tag chain to the tree; this works correctly.
+For a new tag (`fromCommit == zeros`) the diff base falls through to `findNewBranchBase()`,
+which also uses `^{commit}` and returns null (no new commits), so the diff is against the empty tree.
+This produces a full-snapshot diff of the tagged commit, which is harmless for typical content checks.
+
+**`SecretScanningFilter`** — passes `commitFrom`/`commitTo` to `gitleaks git`.
+Gitleaks calls native `git log`, which peels tags natively. No special handling needed.

jgit-proxy-core/src/main/java/org/finos/gitproxy/git/CheckEmptyBranchHook.java

@@ -43,6 +43,8 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
         for (ReceiveCommand cmd : commands) {
             if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) continue;
             if (cmd.getType() == ReceiveCommand.Type.DELETE) continue;
+            // Tags legitimately point to existing commits — skip the "empty branch" check
+            if (cmd.getRefName().startsWith("refs/tags/")) continue;
 
             try {
                 List<Commit> commits = getCommits(repo, cmd);

jgit-proxy-core/src/main/java/org/finos/gitproxy/git/CheckHiddenCommitsHook.java

@@ -125,7 +125,10 @@ private Set<String> collectAllNewCommits(Repository repo, Collection<ReceiveComm
             for (ReceiveCommand cmd : commands) {
                 if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) continue;
                 if (cmd.getType() == ReceiveCommand.Type.DELETE) continue;
-                walk.markStart(walk.parseCommit(cmd.getNewId()));
+                // Dereference annotated tags to their target commit before walking
+                ObjectId commitId = repo.resolve(cmd.getNewId().name() + "^{commit}");
+                if (commitId == null) continue; // non-commit object (blob/tree tag) — skip
+                walk.markStart(walk.parseCommit(commitId));
             }
 
             for (Ref ref : repo.getRefDatabase().getRefsByPrefix("refs/")) {

jgit-proxy-core/src/main/java/org/finos/gitproxy/git/CommitInspectionService.java

@@ -37,7 +37,8 @@ public class CommitInspectionService {
      */
     public static Commit getCommitDetails(Repository repository, String commitId) throws IOException {
         try (RevWalk revWalk = new RevWalk(repository)) {
-            ObjectId objectId = repository.resolve(commitId);
+            // Use "^{commit}" to dereference annotated tags to their target commit
+            ObjectId objectId = repository.resolve(commitId + "^{commit}");
             if (objectId == null) {
                 throw new IOException("Commit not found: " + commitId);
             }
@@ -63,7 +64,8 @@ public static List<Commit> getCommitRange(Repository repository, String fromComm
 
         try (Git git = new Git(repository)) {
             ObjectId fromId = repository.resolve(fromCommit);
-            ObjectId toId = repository.resolve(toCommit);
+            // Use "^{commit}" to dereference annotated tags to their target commit
+            ObjectId toId = repository.resolve(toCommit + "^{commit}");
 
             if (toId == null) {
                 throw new IOException("Commit not found: " + toCommit);
@@ -181,7 +183,8 @@ private static boolean isNullCommit(String commitId) {
      * {@code null} if the oldest new commit is a root commit (base is the empty tree).
      */
     private static ObjectId findNewBranchBase(Repository repository, String toCommit) throws IOException {
-        ObjectId toId = repository.resolve(toCommit);
+        // Use "^{commit}" to dereference annotated tags to their target commit
+        ObjectId toId = repository.resolve(toCommit + "^{commit}");
         if (toId == null) return null;
 
         try (Git git = new Git(repository)) {

jgit-proxy-core/src/main/java/org/finos/gitproxy/git/GitRequestDetails.java

@@ -40,6 +40,11 @@ public boolean isRefDeletion() {
         return commitTo != null && commitTo.matches("^0+$");
     }
 
+    /** Returns true when this push targets a tag ref ({@code refs/tags/} prefix). */
+    public boolean isTagPush() {
+        return branch != null && branch.startsWith("refs/tags/");
+    }
+
     @Builder
     @Getter
     public static class RepoRef {

jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/filter/AllowApprovedPushFilter.java

@@ -53,7 +53,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
         }
 
         var details = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTR);
-        if (details == null || details.getCommit() == null) {
+        if (details == null) {
             return;
         }
 

jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckEmptyBranchFilter.java

@@ -51,6 +51,12 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
             return;
         }
 
+        // Tags legitimately point to existing commits — pushedCommits will be empty but that is expected.
+        if (requestDetails.isTagPush()) {
+            log.debug("Tag push detected (ref={}), skipping empty branch check", requestDetails.getBranch());
+            return;
+        }
+
         var commits = requestDetails.getPushedCommits();
         if (commits != null && !commits.isEmpty()) {
             return;

jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckHiddenCommitsFilter.java

@@ -119,7 +119,8 @@ private Set<String> collectAllNewCommits(Repository repo, String toCommit) throw
         Set<String> result = new HashSet<>();
 
         try (RevWalk walk = new RevWalk(repo)) {
-            ObjectId tip = repo.resolve(toCommit);
+            // Dereference annotated tags to their target commit before walking
+            ObjectId tip = repo.resolve(toCommit + "^{commit}");
             if (tip == null) {
                 log.warn("Could not resolve commitTo {} in cached repository", toCommit);
                 return result;

jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckUserPushPermissionFilter.java

@@ -50,14 +50,17 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
             return;
         }
 
-        // Branch deletions send commitTo = 0000...0 (zero SHA) with no new commits.
-        // There is no author to extract identity from; the user is already authenticated
-        // by HTTP basic auth at the transport layer, so let deletions pass through.
+        // Branch deletions and tag pushes carry no commit author to identify the user;
+        // identity is already verified by HTTP basic auth at the transport layer.
         String commitTo = requestDetails.getCommitTo();
         if (commitTo != null && commitTo.matches("^0+$")) {
             log.debug("Branch deletion detected (commitTo={}), skipping user permission check", commitTo);
             return;
         }
+        if (requestDetails.isTagPush()) {
+            log.debug("Tag push detected (ref={}), skipping user email check", requestDetails.getBranch());
+            return;
+        }
 
         // Extract user email from commit author
         String userEmail = null;

jgit-proxy-core/src/test/java/org/finos/gitproxy/git/CheckEmptyBranchHookTest.java

@@ -142,6 +142,48 @@ void alreadyRejectedCommand_skipped() throws Exception {
         assertEquals("pre-rejected", cmd.getMessage());
     }
 
+    @Test
+    void lightweightTag_skipped() throws Exception {
+        // Lightweight tags point directly to a commit — the empty-branch check must not fire
+        ObjectId c1 = createCommit("Tagged commit");
+        ReceivePack rp = new ReceivePack(repo);
+        ReceiveCommand cmd = new ReceiveCommand(ObjectId.zeroId(), c1, "refs/tags/v1.0");
+
+        CheckEmptyBranchHook hook = new CheckEmptyBranchHook(new PushContext());
+        hook.onPreReceive(rp, List.of(cmd));
+
+        assertEquals(
+                ReceiveCommand.Result.NOT_ATTEMPTED,
+                cmd.getResult(),
+                "Lightweight tag push must not be rejected by CheckEmptyBranchHook");
+    }
+
+    @Test
+    void annotatedTag_skipped() throws Exception {
+        // Annotated tags point to a tag object, not a commit directly
+        ObjectId c1 = createCommit("Tagged commit");
+        // Create an annotated tag via JGit
+        org.eclipse.jgit.lib.ObjectInserter inserter = repo.newObjectInserter();
+        org.eclipse.jgit.lib.TagBuilder tb = new org.eclipse.jgit.lib.TagBuilder();
+        tb.setTag("v2.0");
+        tb.setObjectId(repo.parseCommit(c1));
+        tb.setTagger(new PersonIdent("Dev", "dev@example.com"));
+        tb.setMessage("Release 2.0");
+        ObjectId tagId = inserter.insert(tb);
+        inserter.flush();
+
+        ReceivePack rp = new ReceivePack(repo);
+        ReceiveCommand cmd = new ReceiveCommand(ObjectId.zeroId(), tagId, "refs/tags/v2.0");
+
+        CheckEmptyBranchHook hook = new CheckEmptyBranchHook(new PushContext());
+        hook.onPreReceive(rp, List.of(cmd));
+
+        assertEquals(
+                ReceiveCommand.Result.NOT_ATTEMPTED,
+                cmd.getResult(),
+                "Annotated tag push must not be rejected by CheckEmptyBranchHook");
+    }
+
     @Test
     void step_recordedOnPass() throws Exception {
         ObjectId c1 = createCommit("First");