feat: secret scanning with gitleaks, better proxy mode output

Secret scanning (GitleaksRunner):
- fix: run gitleaks in /tmp to prevent scanning the server working directory
- fix: remove --no-git flag so gitleaks detects config from the diff context
- enrich findings with file path and file-relative line numbers parsed from diff hunk headers
- include commit hash (short) and redacted match snippet in rejection messages
- multi-line toMessage() format respects 80-char git sideband width limit

CommitInspectionService:
- fix new-branch push diff base: instead of diffing against an empty tree
  (which causes false positives from existing repo files), walk commits not
  reachable from any existing branch and diff from the oldest new commit parent

GitProxyFilter (proxy mode):
- replace GitSmartHttpTools.sendError with proper sideband output:
  CH_PROGRESS (0x02) packets per line -> printed as "remote: <line>" by git client
  CH_ERROR (0x03) packet -> triggers die() on client side
  pkt-line flush (0000) to signal end-of-stream
- rejection messages now display multi-line "remote:" output matching GitHub/GitLab UX

PushContext:
- add getStepContent(stepName) helper to look up step output by name

Test reorganization:
- split monolithic test-push-fail.sh / test-proxy-fail.sh into per-scenario
  scripts under test/ (author, diff, message, secrets for both push and proxy modes)

Author: coopernetes

AGENTS.md

@@ -0,0 +1 @@
+CLAUDE.md

CLAUDE.md

@@ -8,7 +8,7 @@ Java rewrite of [FINOS git-proxy](https://github.com/finos/git-proxy) (Node.js).
 |--------|---------|
 | `jgit-proxy-core` | Shared library: filter chain, JGit hooks, push store, provider model, approval abstraction |
 | `jgit-proxy-server` | Standalone proxy-only server (`GitProxyJettyApplication`) — no dashboard, no Spring |
-| `jgit-proxy-api` | Dashboard + REST API (`GitProxyWithDashboardApplication`) — Spring MVC, approval UI, depends on `jgit-proxy-server` |
+| `jgit-proxy-dashboard` | Dashboard + REST API (`GitProxyWithDashboardApplication`) — Spring MVC, approval UI, depends on `jgit-proxy-server` |
 
 ## Architecture
 
@@ -40,8 +40,8 @@ Unit tests live under each module's `src/test/`. E2e tests are in `jgit-proxy-se
 ./gradlew :jgit-proxy-server:stop
 
 # Proxy + dashboard + REST API (http://localhost:8080/):
-./gradlew :jgit-proxy-api:run &
-./gradlew :jgit-proxy-api:stop
+./gradlew :jgit-proxy-dashboard:run &
+./gradlew :jgit-proxy-dashboard:stop
 
 # Logs: jgit-proxy-server/logs/application.log  (DEBUG for org.finos.gitproxy)
 # Default DB: h2-file — persisted to jgit-proxy-server/.data/gitproxy.mv.db

gitleaks

@@ -110,10 +110,13 @@ public static List<DiffEntry> getDiff(Repository repository, String fromCommit,
             CanonicalTreeParser oldTreeParser = new CanonicalTreeParser();
             CanonicalTreeParser newTreeParser = new CanonicalTreeParser();
 
-            // For new-branch pushes fromCommit is the all-zeros null object, which does not
-            // exist in the repository.  Guard before calling resolve() — JGit throws
-            // MissingObjectException rather than returning null for the zero SHA.
-            ObjectId oldId = isNullCommit(fromCommit) ? null : repository.resolve(fromCommit + "^{tree}");
+            // For new-branch pushes fromCommit is the all-zeros null object.  Rather than
+            // diffing against an empty tree (which would show the entire repo snapshot and
+            // trigger false-positive secret findings from existing files like package-lock.json),
+            // find the parent of the oldest new commit so only the genuinely new content is scanned.
+            ObjectId oldId = isNullCommit(fromCommit)
+                    ? findNewBranchBase(repository, toCommit)
+                    : repository.resolve(fromCommit + "^{tree}");
             ObjectId newId = repository.resolve(toCommit + "^{tree}");
 
             if (oldId != null) {
@@ -171,6 +174,41 @@ private static boolean isNullCommit(String commitId) {
         return commitId == null || commitId.matches("^0+$");
     }
 
+    /**
+     * For a new-branch push, find the tree that should be used as the diff base so that only the genuinely new content
+     * is scanned. Walks commits reachable from {@code toCommit} that are NOT reachable from any existing
+     * {@code refs/heads/*} branch tip, then returns the tree of the oldest such commit's first parent. Returns
+     * {@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);
+        if (toId == null) return null;
+
+        try (Git git = new Git(repository)) {
+            var logCmd = git.log().add(toId);
+            Collection<Ref> existingRefs = repository.getRefDatabase().getRefsByPrefix("refs/heads/");
+            for (Ref ref : existingRefs) {
+                if (ref.getObjectId() != null) logCmd.not(ref.getObjectId());
+            }
+
+            List<RevCommit> newCommits = new ArrayList<>();
+            for (RevCommit c : logCmd.call()) {
+                newCommits.add(c);
+            }
+
+            if (newCommits.isEmpty()) return null;
+
+            // logCmd returns newest-first; last entry is the oldest new commit
+            RevCommit oldest = newCommits.get(newCommits.size() - 1);
+            if (oldest.getParentCount() == 0) return null; // root commit — empty tree is correct
+
+            String parentSha = oldest.getParent(0).getName();
+            return repository.resolve(parentSha + "^{tree}");
+        } catch (GitAPIException e) {
+            throw new IOException("Failed to find new-branch base commit", e);
+        }
+    }
+
     /**
      * Convert a JGit RevCommit to our Commit model.
      *

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

@@ -11,7 +11,6 @@
 import java.util.regex.Pattern;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.PreReceiveHook;
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.transport.ReceivePack;
@@ -21,8 +20,7 @@
 
 /**
  * Pre-receive hook that scans the diff content of incoming pushes for blocked literals and patterns. Runs after
- * {@link DiffGenerationHook} so the diff is already available for review, but independently generates its own diff for
- * scanning.
+ * {@link DiffGenerationHook} and re-uses the diff already stored in {@link PushContext} rather than regenerating it.
  *
  * <p>Only added lines (those prefixed with {@code +} in the unified diff) are scanned — deletions and context lines are
  * ignored. Violations are reported via sideband and recorded in the shared {@link ValidationContext}.
@@ -39,7 +37,17 @@ public class DiffScanningHook implements PreReceiveHook {
     public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
         rp.sendMessage(CYAN + "[git-proxy] " + KEY.emoji() + "  Scanning diff content..." + RESET);
 
-        Repository repo = rp.getRepository();
+        // Re-use the diff already generated by DiffGenerationHook
+        String diff = pushContext
+                .getStepContent(DiffGenerationHook.STEP_NAME_PUSH_DIFF)
+                .orElse(null);
+
+        if (diff == null || diff.isBlank()) {
+            rp.sendMessage(YELLOW + "[git-proxy]   " + WARNING.emoji() + "  No diff available for scanning — skipping"
+                    + RESET);
+            return;
+        }
+
         List<String> logs = new ArrayList<>();
         boolean anyFailed = false;
 
@@ -49,9 +57,6 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
             }
 
             try {
-                String diff = CommitInspectionService.getFormattedDiff(
-                        repo, cmd.getOldId().name(), cmd.getNewId().name());
-
                 if (diff.isEmpty()) {
                     rp.sendMessage(GREEN + "[git-proxy]   " + HEAVY_CHECK_MARK.emoji() + "  " + cmd.getRefName()
                             + " — empty diff, nothing to scan" + RESET);

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

@@ -187,13 +187,11 @@ public static String formatForOperation(
     }
 
     private static String formatTitle(String content) {
-        return "\n\n\t" + content + "\n";
+        return "\n\n" + content + "\n";
     }
 
     private static String formatMessage(String content) {
-        // Indent each line with a tab to align with the title
-        String indented = content.lines().map(line -> "\t" + line).collect(java.util.stream.Collectors.joining("\n"));
-        return "\n" + indented + "\n";
+        return "\n" + content + "\n";
     }
 
     private static String convertSymbolsToPlain(String content) {
@@ -253,9 +251,10 @@ public static String buildValidationSummary(List<PushStep> steps) {
                                 "  " + sym(SymbolCodes.HEAVY_CHECK_MARK) + "  " + step.getStepName() + " — passed"))
                         .append("\n");
             } else if (step.getStatus() == StepStatus.FAIL) {
+                String detail = step.getErrorMessage() != null ? step.getErrorMessage() : "failed";
                 sb.append(color(
                                 AnsiColor.RED,
-                                "  " + sym(SymbolCodes.CROSS_MARK) + "  " + step.getStepName() + " — failed"))
+                                "  " + sym(SymbolCodes.CROSS_MARK) + "  " + step.getStepName() + " — " + detail))
                         .append("\n");
             }
         }

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

@@ -97,6 +97,8 @@ public Optional<List<Finding>> scan(String diff, CommitConfig.SecretScanningConf
 
             ProcessBuilder pb = new ProcessBuilder(cmd);
             pb.redirectErrorStream(false);
+            // Run in /tmp so gitleaks doesn't walk the server's working directory
+            pb.directory(reportFile.getParent().toFile());
             Process process = pb.start();
 
             // Write diff to stdin in a daemon thread; closing stdin signals EOF to gitleaks
@@ -127,6 +129,7 @@ public Optional<List<Finding>> scan(String diff, CommitConfig.SecretScanningConf
                 return Optional.of(Collections.emptyList());
             } else if (exitCode == 1) {
                 List<Finding> findings = readFindings(reportFile);
+                enrichFindings(findings, diff);
                 log.debug("gitleaks: {} finding(s)", findings.size());
                 return Optional.of(findings);
             } else {
@@ -340,7 +343,6 @@ private static List<String> buildCommand(
         List<String> cmd = new ArrayList<>();
         cmd.add(binaryPath.toString());
         cmd.add("detect");
-        cmd.add("--no-git");
         cmd.add("--pipe");
         cmd.add("--report-format");
         cmd.add("json");
@@ -375,6 +377,66 @@ private static List<Finding> readFindings(Path reportFile) {
         }
     }
 
+    // -------------------------------------------------------------------------
+    // Diff-line enrichment
+    // -------------------------------------------------------------------------
+
+    /**
+     * Gitleaks {@code --pipe} mode reports {@code StartLine} as the 0-indexed line number within the raw diff text and
+     * leaves {@code File} empty. This method parses the diff to map each finding back to the actual file path and the
+     * file-relative (1-indexed) line number.
+     */
+    private static void enrichFindings(List<Finding> findings, String diff) {
+        if (findings.isEmpty()) return;
+
+        String[] lines = diff.split("\n", -1);
+        String[] fileAtLine = new String[lines.length];
+        int[] fileLineAtLine = new int[lines.length];
+
+        String currentFile = null;
+        int nextFileLine = 0;
+
+        for (int i = 0; i < lines.length; i++) {
+            String line = lines[i];
+            if (line.startsWith("+++ b/")) {
+                currentFile = line.substring("+++ b/".length());
+            } else if (line.startsWith("+++ /dev/null")) {
+                currentFile = null;
+            } else if (line.startsWith("@@")) {
+                nextFileLine = parseHunkNewStart(line);
+            } else if (line.startsWith("+")) {
+                fileAtLine[i] = currentFile;
+                fileLineAtLine[i] = nextFileLine;
+                nextFileLine++;
+            } else if (line.startsWith(" ")) {
+                nextFileLine++;
+            }
+        }
+
+        for (Finding f : findings) {
+            int idx = f.getStartLine(); // 0-indexed line in diff
+            if (idx >= 0 && idx < lines.length && fileAtLine[idx] != null) {
+                f.setFile(fileAtLine[idx]);
+                f.setStartLine(fileLineAtLine[idx]);
+            }
+        }
+    }
+
+    private static int parseHunkNewStart(String hunkHeader) {
+        // "@@ -old_start[,old_count] +new_start[,new_count] @@"
+        int plusIdx = hunkHeader.indexOf(" +");
+        if (plusIdx < 0) return 1;
+        int start = plusIdx + 2;
+        int end = hunkHeader.indexOf(',', start);
+        if (end < 0) end = hunkHeader.indexOf(' ', start);
+        if (end < 0) return 1;
+        try {
+            return Integer.parseInt(hunkHeader.substring(start, end));
+        } catch (NumberFormatException e) {
+            return 1;
+        }
+    }
+
     // -------------------------------------------------------------------------
     // Finding model
     // -------------------------------------------------------------------------
@@ -396,28 +458,42 @@ public static class Finding {
         @JsonProperty("StartLine")
         private int startLine;
 
+        @JsonProperty("Commit")
+        private String commit;
+
         /**
          * The matched text with the actual secret value partially redacted by gitleaks (the {@code Match} field) rather
          * than the raw {@code Secret} field, to avoid logging plaintext secrets.
          */
         @JsonProperty("Match")
         private String match;
 
-        /** Human-readable summary suitable for a push error message. */
+        /**
+         * Multi-line summary suitable for a push error message. Each line is kept short to fit the git sideband 80-char
+         * width limit (git prefixes each newline with "remote: ").
+         */
         public String toMessage() {
-            StringBuilder sb = new StringBuilder("Secret detected");
-            if (ruleId != null && !ruleId.isBlank()) {
-                sb.append(" [").append(ruleId).append("]");
-            }
-            if (description != null && !description.isBlank()) {
-                sb.append(": ").append(description);
-            }
+            StringBuilder sb = new StringBuilder();
+
+            // Line 1: rule ID + location
+            sb.append("[").append(ruleId != null ? ruleId : "unknown").append("]");
             if (file != null && !file.isBlank()) {
-                sb.append(" in ").append(file);
+                sb.append("  ").append(file);
                 if (startLine > 0) {
                     sb.append(":").append(startLine);
                 }
             }
+
+            // Line 2: commit hash (short)
+            if (commit != null && commit.length() >= 7) {
+                sb.append("\n  commit: ").append(commit, 0, 7);
+            }
+
+            // Line 3: redacted match snippet
+            if (match != null && !match.isBlank()) {
+                sb.append("\n  match:  ").append(match);
+            }
+
             return sb.toString();
         }
     }

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

@@ -3,6 +3,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import org.finos.gitproxy.db.model.PushStep;
 
 /**
@@ -22,4 +23,12 @@ public void addStep(PushStep step) {
     public List<PushStep> getSteps() {
         return Collections.unmodifiableList(steps);
     }
+
+    /** Returns the content of the first step matching {@code stepName}, if present. */
+    public Optional<String> getStepContent(String stepName) {
+        return steps.stream()
+                .filter(s -> stepName.equals(s.getStepName()))
+                .findFirst()
+                .map(PushStep::getContent);
+    }
 }

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

@@ -11,7 +11,6 @@
 import java.util.Optional;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
-import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.PreReceiveHook;
 import org.eclipse.jgit.transport.ReceiveCommand;
 import org.eclipse.jgit.transport.ReceivePack;
@@ -21,10 +20,9 @@
 
 /**
  * Pre-receive hook that scans the diff of incoming pushes for secrets using gitleaks. Runs after
- * {@link DiffGenerationHook} in the store-and-forward pipeline.
+ * {@link DiffGenerationHook} and re-uses the diff already stored in {@link PushContext} rather than regenerating it.
  *
- * <p>Each ref's diff is generated independently using {@link CommitInspectionService#getFormattedDiff} and piped to
- * gitleaks. Findings are reported via the sideband channel and recorded in the shared {@link ValidationContext}.
+ * <p>Findings are reported via the sideband channel and recorded in the shared {@link ValidationContext}.
  *
  * <p>If the gitleaks binary is unavailable or execution fails, the hook logs a warning and continues (fail-open).
  * Pushes are never blocked because the scanner is misconfigured.
@@ -55,7 +53,17 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
 
         rp.sendMessage(color(CYAN, "[git-proxy] " + sym(KEY) + "  Scanning for secrets..."));
 
-        Repository repo = rp.getRepository();
+        // Re-use the diff already generated by DiffGenerationHook
+        String diff = pushContext
+                .getStepContent(DiffGenerationHook.STEP_NAME_PUSH_DIFF)
+                .orElse(null);
+
+        if (diff == null || diff.isBlank()) {
+            rp.sendMessage(color(
+                    YELLOW, "[git-proxy]   " + sym(WARNING) + "  No diff available for secret scanning — skipping"));
+            return;
+        }
+
         List<String> logs = new ArrayList<>();
         boolean anyFailed = false;
 
@@ -65,18 +73,6 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
             }
 
             try {
-                String diff = CommitInspectionService.getFormattedDiff(
-                        repo, cmd.getOldId().name(), cmd.getNewId().name());
-
-                if (diff.isBlank()) {
-                    rp.sendMessage(color(
-                            GREEN,
-                            "[git-proxy]   " + sym(HEAVY_CHECK_MARK) + "  " + cmd.getRefName()
-                                    + " — empty diff, nothing to scan"));
-                    logs.add("SKIP: " + cmd.getRefName() + " — empty diff");
-                    continue;
-                }
-
                 Optional<List<GitleaksRunner.Finding>> result = runner.scan(diff, config);
 
                 if (result.isEmpty()) {
@@ -112,12 +108,10 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
             }
         }
 
-        if (!anyFailed) {
-            pushContext.addStep(PushStep.builder()
-                    .stepName(STEP_NAME)
-                    .status(StepStatus.PASS)
-                    .logs(logs)
-                    .build());
-        }
+        pushContext.addStep(PushStep.builder()
+                .stepName(STEP_NAME)
+                .status(anyFailed ? StepStatus.FAIL : StepStatus.PASS)
+                .logs(logs)
+                .build());
     }
 }