Skip to content

Commit 2881669

Browse files
coopernetesclaude
coopernetes
and
claude
committed
feat: add diff scanning, aggregate validation, and approval flow for transparent proxy
Proxy-mode pushes now generate and persist diffs (via ScanDiffFilter) so the dashboard can display them, run all validation filters before reporting failures in aggregate (ValidationSummaryFilter), and block valid pushes pending review (PushFinalizerFilter). Approved pushes are detected on re-push via AllowApprovedPushFilter and forwarded to upstream. Key changes: - ScanDiffFilter always records a "diff" PushStep for dashboard display - Content filters use recordIssue() instead of blockAndSendError() so all failures are collected; ValidationSummaryFilter sends a combined rejection - PushFinalizerFilter (order 5000) sets BLOCKED for unapproved valid pushes and ALLOWED for pre-approved re-pushes; overrides doFilter() to bypass the preApproved short-circuit - AllowApprovedPushFilter moved after EnrichPushCommitsFilter so GitRequestDetails is populated when the approval lookup runs - FetchFinalizerFilter marks passing fetches as ALLOWED - GitRequestDetails defaults to PENDING; isRefDeletion() + skipForRefDeletion() centralize branch-deletion handling across the filter chain - Fix PID file deleteOnExit() removing the file before stop task can read it - E2e tests exercise the full push→block→approve→re-push flow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f8d9fd6 commit 2881669

46 files changed

Lines changed: 1404 additions & 303 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/config/CommitConfig.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ public class CommitConfig {
1919
@Builder.Default
2020
private MessageConfig message = MessageConfig.builder().build();
2121

22+
/** Configuration for diff content scanning. */
23+
@Builder.Default
24+
private DiffConfig diff = DiffConfig.builder().build();
25+
2226
/** Configuration for author validation. */
2327
@Data
2428
@Builder
@@ -94,6 +98,16 @@ public static class BlockConfig {
9498
private List<Pattern> patterns = new ArrayList<>();
9599
}
96100

101+
/** Configuration for diff content scanning. */
102+
@Data
103+
@Builder
104+
public static class DiffConfig {
105+
106+
/** Configuration for blocking specific patterns in diff content. */
107+
@Builder.Default
108+
private BlockConfig block = BlockConfig.builder().build();
109+
}
110+
97111
/**
98112
* Create a default configuration with no restrictions.
99113
*

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/db/PushRecordMapper.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public static PushRecord fromRequestDetails(GitRequestDetails details) {
2727
PushStatus status = mapResult(details.getResult());
2828
if (status == PushStatus.ERROR) {
2929
builder.errorMessage(details.getReason());
30-
} else if (status == PushStatus.BLOCKED) {
30+
} else if (status == PushStatus.BLOCKED || status == PushStatus.REJECTED) {
3131
builder.blockedMessage(details.getReason());
3232
}
3333
}
@@ -117,6 +117,7 @@ public static PushStatus mapResult(GitRequestDetails.GitResult result) {
117117
case PENDING -> PushStatus.PROCESSING;
118118
case ALLOWED -> PushStatus.APPROVED;
119119
case BLOCKED -> PushStatus.BLOCKED;
120+
case REJECTED -> PushStatus.REJECTED;
120121
case ACCEPTED -> PushStatus.RECEIVED;
121122
case ERROR -> PushStatus.ERROR;
122123
};

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import java.nio.charset.StandardCharsets;
55
import java.time.Instant;
66
import java.util.ArrayList;
7+
import java.util.Collection;
78
import java.util.List;
89
import lombok.extern.slf4j.Slf4j;
910
import org.eclipse.jgit.api.Git;
@@ -13,6 +14,7 @@
1314
import org.eclipse.jgit.lib.ObjectId;
1415
import org.eclipse.jgit.lib.ObjectReader;
1516
import org.eclipse.jgit.lib.PersonIdent;
17+
import org.eclipse.jgit.lib.Ref;
1618
import org.eclipse.jgit.lib.Repository;
1719
import org.eclipse.jgit.revwalk.RevCommit;
1820
import org.eclipse.jgit.revwalk.RevWalk;
@@ -72,8 +74,17 @@ public static List<Commit> getCommitRange(Repository repository, String fromComm
7274
if (fromId != null && !isNullCommit(fromCommit)) {
7375
revCommits = git.log().addRange(fromId, toId).call();
7476
} else {
75-
// New branch - get all commits up to toCommit
76-
revCommits = git.log().add(toId).call();
77+
// New branch — exclude commits reachable from any existing ref so we only
78+
// validate commits that are genuinely new in this push. The local cache is a
79+
// bare clone, so existing branch tips live under refs/heads/ (not refs/remotes/).
80+
var logCmd = git.log().add(toId);
81+
Collection<Ref> existingRefs = repository.getRefDatabase().getRefsByPrefix("refs/heads/");
82+
for (Ref ref : existingRefs) {
83+
if (ref.getObjectId() != null) {
84+
logCmd.not(ref.getObjectId());
85+
}
86+
}
87+
revCommits = logCmd.call();
7788
}
7889

7990
for (RevCommit revCommit : revCommits) {
@@ -99,7 +110,10 @@ public static List<DiffEntry> getDiff(Repository repository, String fromCommit,
99110
CanonicalTreeParser oldTreeParser = new CanonicalTreeParser();
100111
CanonicalTreeParser newTreeParser = new CanonicalTreeParser();
101112

102-
ObjectId oldId = repository.resolve(fromCommit + "^{tree}");
113+
// For new-branch pushes fromCommit is the all-zeros null object, which does not
114+
// exist in the repository. Guard before calling resolve() — JGit throws
115+
// MissingObjectException rather than returning null for the zero SHA.
116+
ObjectId oldId = isNullCommit(fromCommit) ? null : repository.resolve(fromCommit + "^{tree}");
103117
ObjectId newId = repository.resolve(toCommit + "^{tree}");
104118

105119
if (oldId != null) {

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

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
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.ArrayList;
7+
import java.util.Collection;
8+
import java.util.LinkedHashSet;
9+
import java.util.List;
10+
import java.util.Set;
11+
import java.util.regex.Pattern;
12+
import lombok.RequiredArgsConstructor;
13+
import lombok.extern.slf4j.Slf4j;
14+
import org.eclipse.jgit.lib.Repository;
15+
import org.eclipse.jgit.transport.PreReceiveHook;
16+
import org.eclipse.jgit.transport.ReceiveCommand;
17+
import org.eclipse.jgit.transport.ReceivePack;
18+
import org.finos.gitproxy.config.CommitConfig;
19+
import org.finos.gitproxy.db.model.PushStep;
20+
import org.finos.gitproxy.db.model.StepStatus;
21+
22+
/**
23+
* Pre-receive hook that scans the diff content of incoming pushes for blocked literals and patterns. Runs after
24+
* {@link DiffGenerationHook} so the diff is already available for review, but independently generates its own diff for
25+
* scanning.
26+
*
27+
* <p>Only added lines (those prefixed with {@code +} in the unified diff) are scanned — deletions and context lines are
28+
* ignored. Violations are reported via sideband and recorded in the shared {@link ValidationContext}.
29+
*/
30+
@Slf4j
31+
@RequiredArgsConstructor
32+
public class DiffScanningHook implements PreReceiveHook {
33+
34+
private final CommitConfig commitConfig;
35+
private final ValidationContext validationContext;
36+
private final PushContext pushContext;
37+
38+
@Override
39+
public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
40+
rp.sendMessage(CYAN + "[git-proxy] " + KEY.emoji() + " Scanning diff content..." + RESET);
41+
42+
Repository repo = rp.getRepository();
43+
List<String> logs = new ArrayList<>();
44+
boolean anyFailed = false;
45+
46+
for (ReceiveCommand cmd : commands) {
47+
if (cmd.getType() == ReceiveCommand.Type.DELETE) {
48+
continue;
49+
}
50+
51+
try {
52+
String diff = CommitInspectionService.getFormattedDiff(
53+
repo, cmd.getOldId().name(), cmd.getNewId().name());
54+
55+
if (diff.isEmpty()) {
56+
rp.sendMessage(GREEN + "[git-proxy] " + HEAVY_CHECK_MARK.emoji() + " " + cmd.getRefName()
57+
+ " — empty diff, nothing to scan" + RESET);
58+
logs.add("SKIP: " + cmd.getRefName() + " — empty diff");
59+
continue;
60+
}
61+
62+
List<String> violations = scanDiff(diff);
63+
64+
if (!violations.isEmpty()) {
65+
for (String violation : violations) {
66+
validationContext.addIssue("DiffContent", violation, "ref: " + cmd.getRefName());
67+
rp.sendMessage(RED + "[git-proxy] " + CROSS_MARK.emoji() + " " + violation + RESET);
68+
logs.add("FAIL: " + violation);
69+
}
70+
anyFailed = true;
71+
} else {
72+
rp.sendMessage(GREEN + "[git-proxy] " + HEAVY_CHECK_MARK.emoji() + " " + cmd.getRefName()
73+
+ " — clean" + RESET);
74+
logs.add("PASS: " + cmd.getRefName());
75+
}
76+
77+
} catch (Exception e) {
78+
log.error("Failed to scan diff for {}", cmd.getRefName(), e);
79+
rp.sendMessage(YELLOW + "[git-proxy] " + WARNING.emoji() + " Could not scan diff: " + e.getMessage()
80+
+ RESET);
81+
logs.add("ERROR: " + cmd.getRefName() + " — " + e.getMessage());
82+
}
83+
}
84+
85+
if (!anyFailed) {
86+
pushContext.addStep(PushStep.builder()
87+
.stepName("scanDiff")
88+
.status(StepStatus.PASS)
89+
.logs(logs)
90+
.build());
91+
}
92+
}
93+
94+
/**
95+
* Scans the unified diff for blocked content, returning a deduplicated list of violation descriptions. Only added
96+
* lines (prefixed with {@code +}, excluding the {@code +++} header) are checked.
97+
*/
98+
List<String> scanDiff(String diff) {
99+
Set<String> violations = new LinkedHashSet<>();
100+
CommitConfig.BlockConfig block = commitConfig.getDiff().getBlock();
101+
102+
if (block.getLiterals().isEmpty() && block.getPatterns().isEmpty()) {
103+
return List.of();
104+
}
105+
106+
String currentFile = null;
107+
for (String line : diff.lines().toList()) {
108+
if (line.startsWith("diff --git ")) {
109+
currentFile = extractFileName(line);
110+
}
111+
112+
// Only scan added lines; skip the +++ header line
113+
if (!line.startsWith("+") || line.startsWith("+++")) {
114+
continue;
115+
}
116+
String content = line.substring(1);
117+
118+
for (String literal : block.getLiterals()) {
119+
if (content.toLowerCase().contains(literal.toLowerCase())) {
120+
String location = currentFile != null ? " in " + currentFile : "";
121+
violations.add("diff contains blocked term: \"" + literal + "\"" + location);
122+
}
123+
}
124+
125+
for (Pattern pattern : block.getPatterns()) {
126+
if (pattern.matcher(content).find()) {
127+
String location = currentFile != null ? " in " + currentFile : "";
128+
violations.add("diff matches blocked pattern: " + pattern.pattern() + location);
129+
}
130+
}
131+
}
132+
133+
return new ArrayList<>(violations);
134+
}
135+
136+
/** Extracts the {@code b/} path from a {@code diff --git a/... b/...} header line. */
137+
private static String extractFileName(String diffHeader) {
138+
// "diff --git a/path/to/file b/path/to/file"
139+
String[] parts = diffHeader.split(" ");
140+
if (parts.length >= 4) {
141+
String bPath = parts[3];
142+
return bPath.startsWith("b/") ? bPath.substring(2) : bPath;
143+
}
144+
return diffHeader;
145+
}
146+
}

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,14 @@ public class GitRequestDetails {
2525
private GitProxyProvider provider;
2626
private List<GitProxyFilter> filters = new ArrayList<>();
2727
private List<PushStep> steps = new ArrayList<>(); // Filter/hook results for audit trail
28-
private GitResult result = GitResult.ALLOWED;
28+
private GitResult result = GitResult.PENDING;
2929
private String reason;
3030

31+
/** Returns true when this push is a ref deletion (commitTo is the all-zeros null SHA). */
32+
public boolean isRefDeletion() {
33+
return commitTo != null && commitTo.matches("^0+$");
34+
}
35+
3136
@Builder
3237
@Getter
3338
public static class Repository {
@@ -45,6 +50,7 @@ public enum GitResult {
4550
PENDING,
4651
ALLOWED,
4752
BLOCKED,
53+
REJECTED, // hard reject with no review queue (transparent proxy mode)
4854
ACCEPTED, // for async where a push or fetch is accepted but not yet complete
4955
ERROR;
5056
}

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,14 @@ public ReceivePack create(HttpServletRequest req, Repository db)
9292
// 2. CommitMessageValidationHook — validates messages; records "checkCommitMessages" step
9393
// 3. ProxyPreReceiveHook — commit inspection; records "inspection" step
9494
// 4. DiffGenerationHook — generates diffs; records "diff" / "diff:default-branch" steps
95-
// 5. PushStorePersistenceHook.validationResult — saves APPROVED or BLOCKED record with all steps so far
96-
// 6. ApprovalPreReceiveHook — blocks until reviewer approves/rejects or timeout
95+
// 5. DiffScanningHook — scans diff added-lines for blocked content; records "scanDiff"
96+
// 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
9799
//
98100
// Post-receive (only runs when pre-receive doesn't stop the chain):
99-
// 7. ForwardingPostReceiveHook — forwards to upstream; records "forward" step
100-
// 8. PushStorePersistenceHook.postReceive — saves FORWARDED or ERROR record with forwarding step
101+
// 8. ForwardingPostReceiveHook — forwards to upstream; records "forward" step
102+
// 9. PushStorePersistenceHook.postReceive — saves FORWARDED or ERROR record with forwarding step
101103

102104
PreReceiveHook[] preHooks;
103105
if (persistenceHook != null) {
@@ -107,6 +109,7 @@ public ReceivePack create(HttpServletRequest req, Repository db)
107109
new CommitMessageValidationHook(commitConfig, validationContext, pushContext),
108110
new ProxyPreReceiveHook(pushContext),
109111
new DiffGenerationHook(pushContext),
112+
new DiffScanningHook(commitConfig, validationContext, pushContext),
110113
persistenceHook.validationResultHook(validationContext),
111114
new ApprovalPreReceiveHook(pushStore, approvalGateway, serviceUrl)
112115
};
@@ -115,7 +118,8 @@ public ReceivePack create(HttpServletRequest req, Repository db)
115118
new AuthorEmailValidationHook(commitConfig, validationContext, pushContext),
116119
new CommitMessageValidationHook(commitConfig, validationContext, pushContext),
117120
new ProxyPreReceiveHook(pushContext),
118-
new DiffGenerationHook(pushContext)
121+
new DiffGenerationHook(pushContext),
122+
new DiffScanningHook(commitConfig, validationContext, pushContext)
119123
};
120124
}
121125
rp.setPreReceiveHook(chainPreReceiveHooks(preHooks));

‎jgit-proxy-core/src/main/java/org/finos/gitproxy/servlet/GitProxyServlet.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,15 @@
1212

1313
@Slf4j
1414
public class GitProxyServlet extends AsyncProxyServlet.Transparent {
15-
public static final String GIT_REQUEST_ATTRIBUTE = "org.finos.gitproxy.gitproxy.gitRequest";
15+
public static final String GIT_REQUEST_ATTR = "gitproxy.gitRequest";
16+
public static final String ERROR_ATTR = "gitproxy.error";
17+
public static final String PRE_APPROVED_ATTR = "gitproxy.preApproved";
18+
public static final String SERVICE_URL_ATTR = "gitproxy.serviceUrl";
1619

1720
@Override
1821
protected void service(HttpServletRequest clientRequest, HttpServletResponse proxyResponse)
1922
throws ServletException, IOException {
20-
var details = (GitRequestDetails) clientRequest.getAttribute(GIT_REQUEST_ATTRIBUTE);
23+
var details = (GitRequestDetails) clientRequest.getAttribute(GIT_REQUEST_ATTR);
2124
var canProxy = details != null && details.getResult() == GitRequestDetails.GitResult.ALLOWED;
2225
if (canProxy) {
2326
super.service(clientRequest, proxyResponse);

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package org.finos.gitproxy.servlet.filter;
22

3-
import static org.finos.gitproxy.servlet.GitProxyProviderServlet.GIT_REQUEST_ATTRIBUTE;
3+
import static org.finos.gitproxy.servlet.GitProxyProviderServlet.*;
4+
import static org.finos.gitproxy.servlet.GitProxyServlet.*;
45

56
import jakarta.servlet.http.HttpServletRequest;
67
import jakarta.servlet.http.HttpServletResponse;
@@ -31,9 +32,6 @@
3132
@Slf4j
3233
public class AllowApprovedPushFilter extends AbstractGitProxyFilter {
3334

34-
private static final String PRE_APPROVED_ATTR = "gitproxy.preApproved";
35-
private static final String SERVICE_URL_ATTR = "gitproxy.serviceUrl";
36-
3735
private final PushStore pushStore;
3836
private final String serviceUrl;
3937

@@ -53,7 +51,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
5351
return;
5452
}
5553

56-
var details = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTRIBUTE);
54+
var details = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTR);
5755
if (details == null || details.getCommit() == null) {
5856
return;
5957
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package org.finos.gitproxy.servlet.filter;
22

3-
import static org.finos.gitproxy.servlet.GitProxyProviderServlet.GIT_REQUEST_ATTRIBUTE;
3+
import static org.finos.gitproxy.servlet.GitProxyServlet.GIT_REQUEST_ATTR;
44

55
import jakarta.servlet.ServletException;
66
import jakarta.servlet.http.HttpServletRequest;
@@ -15,7 +15,7 @@ public interface AuditFilter extends GitProxyFilter {
1515
@Override
1616
default void doHttpFilter(HttpServletRequest request, HttpServletResponse response)
1717
throws IOException, ServletException {
18-
var requestDetails = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTRIBUTE);
18+
var requestDetails = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTR);
1919
audit(requestDetails);
2020
}
2121
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import static org.finos.gitproxy.git.GitClient.AnsiColor.*;
44
import static org.finos.gitproxy.git.GitClient.SymbolCodes.*;
5-
import static org.finos.gitproxy.servlet.GitProxyProviderServlet.GIT_REQUEST_ATTRIBUTE;
5+
import static org.finos.gitproxy.servlet.GitProxyServlet.GIT_REQUEST_ATTR;
66

77
import jakarta.servlet.http.HttpServletRequest;
88
import jakarta.servlet.http.HttpServletResponse;
@@ -38,7 +38,7 @@ public CheckAuthorEmailsFilter(CommitConfig commitConfig) {
3838

3939
@Override
4040
public void doHttpFilter(HttpServletRequest request, HttpServletResponse response) throws IOException {
41-
var requestDetails = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTRIBUTE);
41+
var requestDetails = (GitRequestDetails) request.getAttribute(GIT_REQUEST_ATTR);
4242
if (requestDetails == null) {
4343
log.warn("GitRequestDetails not found in request attributes");
4444
return;
@@ -71,7 +71,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
7171
+ "\n"
7272
+ "Verify your Git email address is valid:\n"
7373
+ " git config user.email \"you@example.com\"";
74-
blockAndSendError(request, response, "Illegal author emails", GitClient.format(title, message, RED, null));
74+
recordIssue(request, "Illegal author emails", GitClient.format(title, message, RED, null));
7575
return;
7676
}
7777

0 commit comments

Comments
 (0)