Skip to content

Commit 41c20c1

Browse files
coopernetesclaude
andcommitted
feat: add checkEmptyBranch and checkHiddenCommits built-in filters
Ports checkEmptyBranch and checkHiddenCommits from the Node.js git-proxy reference implementation as non-configurable, short-circuiting checks in both the store-and-forward (pre-receive hooks) and transparent proxy (servlet filters) pipelines. checkEmptyBranch rejects pushes where no new commits are introduced — either a new branch pointing to an existing commit, or a branch update with no resolvable commit range. checkHiddenCommits rejects pushes where the received pack contains commits that are new to the repository but fall outside the explicitly introduced commit range (oldId..newId), catching branches built on unapproved history. E2e tests cover the empty-branch rejection in both modes. The hidden-commits failure case cannot be reproduced via a standard git client (git only sends objects reachable from the pushed tip) and is documented with a comment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 63e5dfc commit 41c20c1

10 files changed

Lines changed: 493 additions & 9 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
package org.finos.gitproxy.git;
2+
3+
import static org.finos.gitproxy.git.GitClient.AnsiColor.*;
4+
import static org.finos.gitproxy.git.GitClient.SymbolCodes.*;
5+
6+
import java.util.Collection;
7+
import java.util.List;
8+
import lombok.extern.slf4j.Slf4j;
9+
import org.eclipse.jgit.lib.ObjectId;
10+
import org.eclipse.jgit.lib.Repository;
11+
import org.eclipse.jgit.transport.PreReceiveHook;
12+
import org.eclipse.jgit.transport.ReceiveCommand;
13+
import org.eclipse.jgit.transport.ReceivePack;
14+
15+
/**
16+
* Pre-receive hook that rejects pushes where no commits can be found in the pushed range. Two cases are distinguished:
17+
*
18+
* <ul>
19+
* <li><b>New branch with no new commits</b> — the branch tip resolves to an existing commit already reachable from
20+
* another ref; the developer pushed an empty branch pointer with no new work.
21+
* <li><b>Existing branch with no new commits</b> — commit data could not be resolved; this usually indicates a proxy
22+
* configuration or repository state problem.
23+
* </ul>
24+
*
25+
* <p>This hook short-circuits the chain immediately on failure (direct rejection, not via {@link ValidationContext}).
26+
*/
27+
@Slf4j
28+
public class CheckEmptyBranchHook implements PreReceiveHook {
29+
30+
@Override
31+
public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
32+
Repository repo = rp.getRepository();
33+
34+
for (ReceiveCommand cmd : commands) {
35+
if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) continue;
36+
if (cmd.getType() == ReceiveCommand.Type.DELETE) continue;
37+
38+
try {
39+
List<Commit> commits = getCommits(repo, cmd);
40+
if (!commits.isEmpty()) continue;
41+
42+
String msg;
43+
if (ObjectId.zeroId().equals(cmd.getOldId())) {
44+
msg = "Push blocked: Empty branch. Please make a commit before pushing a new branch.";
45+
} else {
46+
msg = "Push blocked: Commit data not found. Please contact an administrator for support.";
47+
}
48+
49+
rp.sendMessage(RED + "[git-proxy] " + NO_ENTRY.emoji() + " " + msg + RESET);
50+
cmd.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, msg);
51+
// Chain stops once a command is rejected; remaining commands will be skipped
52+
return;
53+
54+
} catch (Exception e) {
55+
log.error("Failed to check empty branch for {}", cmd.getRefName(), e);
56+
}
57+
}
58+
}
59+
60+
private List<Commit> getCommits(Repository repo, ReceiveCommand cmd) throws Exception {
61+
return CommitInspectionService.getCommitRange(repo, cmd.getOldId().name(), cmd.getNewId().name());
62+
}
63+
}
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
package org.finos.gitproxy.git;
2+
3+
import static org.finos.gitproxy.git.GitClient.AnsiColor.*;
4+
import static org.finos.gitproxy.git.GitClient.SymbolCodes.*;
5+
6+
import java.io.IOException;
7+
import java.util.Collection;
8+
import java.util.HashSet;
9+
import java.util.Set;
10+
import java.util.stream.Collectors;
11+
import lombok.extern.slf4j.Slf4j;
12+
import org.eclipse.jgit.lib.ObjectId;
13+
import org.eclipse.jgit.lib.Ref;
14+
import org.eclipse.jgit.lib.Repository;
15+
import org.eclipse.jgit.revwalk.RevCommit;
16+
import org.eclipse.jgit.revwalk.RevWalk;
17+
import org.eclipse.jgit.transport.PreReceiveHook;
18+
import org.eclipse.jgit.transport.ReceiveCommand;
19+
import org.eclipse.jgit.transport.ReceivePack;
20+
21+
/**
22+
* Pre-receive hook that detects "hidden" commits — objects received in the push pack that are not part of the
23+
* explicitly introduced commit range. This catches the case where a branch was created from commits that have not yet
24+
* been approved and pushed to the remote, smuggling unapproved history through as pack filler.
25+
*
26+
* <p>Algorithm:
27+
*
28+
* <ol>
29+
* <li><b>introduced</b> — commits reachable from each {@code newId} up to (not including) its {@code oldId}, across
30+
* all pushed commands; computed via {@link CommitInspectionService#getCommitRange}.
31+
* <li><b>allNew</b> — commits reachable from any pushed {@code newId} that are not reachable from any existing ref
32+
* (i.e., genuinely new to this repository); computed via {@link RevWalk}.
33+
* <li><b>hidden</b> = {@code allNew} ∖ {@code introduced} — commits in the pack but outside the pushed range.
34+
* </ol>
35+
*
36+
* <p>This hook short-circuits the chain immediately on failure (direct rejection, not via {@link ValidationContext}).
37+
*/
38+
@Slf4j
39+
public class CheckHiddenCommitsHook implements PreReceiveHook {
40+
41+
@Override
42+
public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
43+
Repository repo = rp.getRepository();
44+
45+
try {
46+
Set<String> allIntroduced = collectIntroducedCommits(repo, commands);
47+
Set<String> allNew = collectAllNewCommits(repo, commands);
48+
49+
Set<String> hidden = new HashSet<>(allNew);
50+
hidden.removeAll(allIntroduced);
51+
52+
if (hidden.isEmpty()) {
53+
log.debug("checkHiddenCommits: all {} new commit(s) are within the introduced range", allNew.size());
54+
return;
55+
}
56+
57+
String msg = "Unreferenced commits in pack (" + hidden.size() + "): "
58+
+ String.join(", ", hidden) + ".\n"
59+
+ "This usually happens when a branch was made from a commit that hasn't been approved"
60+
+ " and pushed to the remote.\n"
61+
+ "Please get approval on the commits, push them and try again.";
62+
63+
rp.sendMessage(RED + "[git-proxy] " + NO_ENTRY.emoji() + " Push blocked — hidden commits detected" + RESET);
64+
rp.sendMessage(YELLOW + "[git-proxy] " + WARNING.emoji() + " " + msg + RESET);
65+
66+
for (ReceiveCommand cmd : commands) {
67+
if (cmd.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED) {
68+
cmd.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, "Hidden commits detected");
69+
}
70+
}
71+
72+
} catch (Exception e) {
73+
log.error("Failed to check hidden commits", e);
74+
}
75+
}
76+
77+
private Set<String> collectIntroducedCommits(Repository repo, Collection<ReceiveCommand> commands) throws Exception {
78+
Set<String> introduced = new HashSet<>();
79+
for (ReceiveCommand cmd : commands) {
80+
if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) continue;
81+
if (cmd.getType() == ReceiveCommand.Type.DELETE) continue;
82+
CommitInspectionService.getCommitRange(repo, cmd.getOldId().name(), cmd.getNewId().name())
83+
.stream()
84+
.map(Commit::getSha)
85+
.forEach(introduced::add);
86+
}
87+
return introduced;
88+
}
89+
90+
/**
91+
* Collect all commits reachable from any pushed tip that are not reachable from any existing ref. These are the
92+
* commits that were genuinely new to this repository as part of the pack.
93+
*/
94+
private Set<String> collectAllNewCommits(Repository repo, Collection<ReceiveCommand> commands) throws IOException {
95+
Set<String> result = new HashSet<>();
96+
97+
try (RevWalk walk = new RevWalk(repo)) {
98+
for (ReceiveCommand cmd : commands) {
99+
if (cmd.getResult() != ReceiveCommand.Result.NOT_ATTEMPTED) continue;
100+
if (cmd.getType() == ReceiveCommand.Type.DELETE) continue;
101+
walk.markStart(walk.parseCommit(cmd.getNewId()));
102+
}
103+
104+
for (Ref ref : repo.getRefDatabase().getRefsByPrefix("refs/")) {
105+
ObjectId id = ref.getObjectId();
106+
if (id == null) continue;
107+
try {
108+
walk.markUninteresting(walk.parseCommit(id));
109+
} catch (Exception e) {
110+
// Not a commit (tag pointing to a blob/tree, etc.) — skip
111+
}
112+
}
113+
114+
for (RevCommit commit : walk) {
115+
result.add(commit.getName());
116+
}
117+
}
118+
119+
return result;
120+
}
121+
}

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -88,23 +88,27 @@ public ReceivePack create(HttpServletRequest req, Repository db)
8888
// Hook chain — order matters:
8989
//
9090
// 0. PushStorePersistenceHook.preReceive — record RECEIVED (no steps yet)
91-
// 1. AuthorEmailValidationHook — validates emails; records "checkAuthorEmails" step
92-
// 2. CommitMessageValidationHook — validates messages; records "checkCommitMessages" step
93-
// 3. ProxyPreReceiveHook — commit inspection; records "inspection" step
94-
// 4. DiffGenerationHook — generates diffs; records "diff" / "diff:default-branch" steps
95-
// 5. DiffScanningHook — scans diff added-lines for blocked content; records "scanDiff"
91+
// 1. CheckEmptyBranchHook — reject if no commits introduced (short-circuit)
92+
// 2. CheckHiddenCommitsHook — reject if pack contains commits outside push range (short-circuit)
93+
// 3. AuthorEmailValidationHook — validates emails; records "checkAuthorEmails" step
94+
// 4. CommitMessageValidationHook — validates messages; records "checkCommitMessages" step
95+
// 5. ProxyPreReceiveHook — commit inspection; records "inspection" step
96+
// 6. DiffGenerationHook — generates diffs; records "diff" / "diff:default-branch" steps
97+
// 7. DiffScanningHook — scans diff added-lines for blocked content; records "scanDiff"
9698
// step
97-
// 6. PushStorePersistenceHook.validationResult — saves APPROVED or BLOCKED record with all steps so far
98-
// 7. ApprovalPreReceiveHook — blocks until reviewer approves/rejects or timeout
99+
// 8. PushStorePersistenceHook.validationResult — saves APPROVED or BLOCKED record with all steps so far
100+
// 9. ApprovalPreReceiveHook — blocks until reviewer approves/rejects or timeout
99101
//
100102
// Post-receive (only runs when pre-receive doesn't stop the chain):
101-
// 8. ForwardingPostReceiveHook — forwards to upstream; records "forward" step
102-
// 9. PushStorePersistenceHook.postReceive — saves FORWARDED or ERROR record with forwarding step
103+
// 10. ForwardingPostReceiveHook — forwards to upstream; records "forward" step
104+
// 11. PushStorePersistenceHook.postReceive — saves FORWARDED or ERROR record with forwarding step
103105

104106
PreReceiveHook[] preHooks;
105107
if (persistenceHook != null) {
106108
preHooks = new PreReceiveHook[] {
107109
persistenceHook.preReceiveHook(),
110+
new CheckEmptyBranchHook(),
111+
new CheckHiddenCommitsHook(),
108112
new AuthorEmailValidationHook(commitConfig, validationContext, pushContext),
109113
new CommitMessageValidationHook(commitConfig, validationContext, pushContext),
110114
new ProxyPreReceiveHook(pushContext),
@@ -115,6 +119,8 @@ public ReceivePack create(HttpServletRequest req, Repository db)
115119
};
116120
} else {
117121
preHooks = new PreReceiveHook[] {
122+
new CheckEmptyBranchHook(),
123+
new CheckHiddenCommitsHook(),
118124
new AuthorEmailValidationHook(commitConfig, validationContext, pushContext),
119125
new CommitMessageValidationHook(commitConfig, validationContext, pushContext),
120126
new ProxyPreReceiveHook(pushContext),
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package org.finos.gitproxy.servlet.filter;
2+
3+
import static org.finos.gitproxy.git.GitClient.AnsiColor.*;
4+
import static org.finos.gitproxy.git.GitClient.SymbolCodes.*;
5+
import static org.finos.gitproxy.servlet.GitProxyServlet.GIT_REQUEST_ATTR;
6+
7+
import jakarta.servlet.http.HttpServletRequest;
8+
import jakarta.servlet.http.HttpServletResponse;
9+
import java.io.IOException;
10+
import java.util.Set;
11+
import lombok.extern.slf4j.Slf4j;
12+
import org.finos.gitproxy.git.GitClient;
13+
import org.finos.gitproxy.git.GitRequestDetails;
14+
import org.finos.gitproxy.git.HttpOperation;
15+
16+
/**
17+
* Filter that rejects pushes where no commits could be found in the pushed range. Two cases are distinguished:
18+
*
19+
* <ul>
20+
* <li><b>Empty branch</b> — a new branch is being created but its tip is already reachable from an existing ref; no
21+
* new commits were introduced.
22+
* <li><b>Commit data not found</b> — a non-new-branch push produced no commit data; indicates a proxy or repository
23+
* state problem.
24+
* </ul>
25+
*
26+
* <p>This filter short-circuits immediately via {@link #rejectAndSendError} without recording to {@link
27+
* org.finos.gitproxy.servlet.filter.ValidationSummaryFilter}.
28+
*
29+
* <p>Runs at order 2050, before all other content validation filters.
30+
*/
31+
@Slf4j
32+
public class CheckEmptyBranchFilter extends AbstractGitProxyFilter {
33+
34+
private static final int ORDER = 2050;
35+
36+
public CheckEmptyBranchFilter() {
37+
super(ORDER, Set.of(HttpOperation.PUSH));
38+
}
39+
40+
@Override
41+
public void doHttpFilter(HttpServletRequest request, HttpServletResponse response) throws IOException {
42+
var requestDetails = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTR);
43+
if (requestDetails == null) {
44+
log.warn("GitRequestDetails not found in request attributes");
45+
return;
46+
}
47+
48+
var commits = requestDetails.getPushedCommits();
49+
if (commits != null && !commits.isEmpty()) {
50+
return;
51+
}
52+
53+
String commitFrom = requestDetails.getCommitFrom();
54+
boolean isNewBranch = commitFrom == null || commitFrom.matches("^0+$");
55+
56+
String title;
57+
String message;
58+
if (isNewBranch) {
59+
title = NO_ENTRY.emoji() + " Push Blocked — Empty Branch";
60+
message = "Please make a commit before pushing a new branch.";
61+
} else {
62+
title = NO_ENTRY.emoji() + " Push Blocked — Commit Data Not Found";
63+
message = "Commit data not found. Please contact an administrator for support.";
64+
}
65+
66+
log.warn("checkEmptyBranch: rejecting push — {}", message);
67+
rejectAndSendError(request, response, title, GitClient.format(title, message, RED, null));
68+
}
69+
}

0 commit comments

Comments
 (0)