fix: secret scanning broken in Docker — missing git binary + exit code ambiguity (#202)

## Summary

- **Root cause of #189:** The Docker runtime image was missing the
\`git\` binary. \`gitleaks git\` mode shells out to \`git\` to traverse
commit history — without it, gitleaks exits 1 with an empty report. The
proxy misread exit 1 as "findings present, read report", got an empty
list, and silently passed the push.
- **Secondary fix:** Use \`--exit-code 2\` so gitleaks findings produce
exit 2, making runtime errors (exit 1) unambiguous from clean scans
(exit 0) and findings (exit 2).
- **Fail-closed:** When \`secret-scan: enabled: true\` and the scanner
errors or is unavailable, the push is now blocked with a clear message
instead of silently passing.
- **Smoke tests upgraded:** Fake truncated PEM keys replaced with real
keys from \`openssl\`/\`ssh-keygen\` (with fallback detection). Added
PKCS#8 (\`BEGIN PRIVATE KEY\`) alongside PKCS#1 (\`BEGIN RSA PRIVATE
KEY\`).
- **CI:** New \`docker-smoke-test\` job builds the image, seeds Gitea,
and runs \`push-fail-secrets.sh\` + \`proxy-fail-secrets.sh\` end-to-end
against the real Docker distribution.
- **Descriptive error on blocked \`/info/refs\`:** Previously returned a
bare 403 with no body.

## Test plan

- [ ] CI passes — \`docker-smoke-test\` job blocks all secret-containing
pushes
- [ ] \`GitleaksRunnerTest\` — 4 tests covering \`scanGit\` and \`scan\`
modes
- [ ] \`SecretScanningFilterTest#scannerUnavailable_failClosed\` —
scanner error produces FAIL step
- [ ] \`UrlRuleFilterTest\` — descriptive \`sendError\` +
\`recordFetch\` on blocked \`/info/refs\`
- [ ] Local: \`source test/gitea/env.sh && bash
test/push-fail-secrets.sh && bash test/proxy-fail-secrets.sh\`

closes #189
closes #200

Author: coopernetes

.github/workflows/ci.yml

@@ -53,6 +53,69 @@ jobs:
           path: "**/build/reports/jacoco/"
           retention-days: 14
 
+  docker-smoke-test:
+    name: CI / Docker Smoke Test (${{ matrix.db }})
+    runs-on: ubuntu-latest
+    if: >
+      github.ref == 'refs/heads/main' ||
+      github.event_name == 'pull_request'
+    strategy:
+      matrix:
+        db: [default, postgres, mongo]
+        # auth: [ldap, oidc]  # Uncomment when auth smoke tests are added
+      fail-fast: false
+
+    steps:
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # ratchet:actions/checkout@v6
+
+      - name: Build and start stack
+        env:
+          DB: ${{ matrix.db }}
+        run: |
+          if [[ "$DB" != "default" ]]; then
+            bash compose.sh --db "$DB" -- up -d --build
+          else
+            bash compose.sh -- up -d --build
+          fi
+        timeout-minutes: 10
+
+      - name: Wait for proxy health
+        run: |
+          for i in $(seq 1 30); do
+            if curl -sf http://localhost:8080/api/health > /dev/null; then
+              echo "Proxy healthy after ${i}s"
+              break
+            fi
+            sleep 1
+          done
+          curl -sf http://localhost:8080/api/health
+
+      - name: Seed Gitea
+        run: bash docker/gitea-setup.sh
+
+      - name: Run proxy smoke tests
+        run: |
+          source test/gitea/tokens.env
+          export GIT_PASSWORD="${GITEA_TESTUSER_TOKEN}"
+          export GIT_USERNAME="me"
+          export GIT_REPO="gitea/test-owner/test-repo.git"
+          export GIT_AUTHOR_NAME="test-user"
+          export GIT_EMAIL="testuser@example.com"
+          # Proxy mode only — store-and-forward passes queue for approval and hang
+          bash test/proxy-pass.sh
+          bash test/proxy-fail-secrets.sh
+
+      - name: Dump proxy logs on failure
+        if: failure()
+        env:
+          DB: ${{ matrix.db }}
+        run: |
+          if [[ "$DB" != "default" ]]; then
+            bash compose.sh --db "$DB" -- logs git-proxy-java
+          else
+            bash compose.sh -- logs git-proxy-java
+          fi
+
   dependency-submission:
     name: CI / Dependency Submission
     runs-on: ubuntu-latest

Dockerfile

@@ -1,4 +1,4 @@
-# syntax=docker/dockerfile:1
+# syntax=docker/dockerfile:1@sha256:2780b5c3bab67f1f76c781860de469442999ed1a0d7992a5efdf2cffc0e3d769
 
 # ── Build stage ──────────────────────────────────────────────────────────────
 FROM docker.io/eclipse-temurin:21-jdk@sha256:e58e492628c1428ceb838afc1a1b8762673d5eaa09296f560c363daea0fdcf3b AS builder
@@ -51,6 +51,8 @@ FROM docker.io/eclipse-temurin:21-jre@sha256:ff65ff0d43c73d2b675eb4b758665a5cb48
 
 WORKDIR /app
 
+RUN apt-get update && apt-get install -y --no-install-recommends git && rm -rf /var/lib/apt/lists/*
+
 # Copy the built distribution
 COPY --from=builder \
     /workspace/git-proxy-java-dashboard/build/install/git-proxy-java-dashboard/ /app/

compose.sh

@@ -48,10 +48,13 @@ if [[ -n "$DB" && "$DB" != "postgres" && "$DB" != "mongo" ]]; then
   exit 1
 fi
 
-if command -v podman &>/dev/null; then
+if command -v docker &>/dev/null && docker info &>/dev/null 2>&1; then
+  COMPOSE="docker compose"
+elif command -v podman &>/dev/null && podman info &>/dev/null 2>&1; then
   COMPOSE="podman compose"
 else
-  COMPOSE="docker compose"
+  echo "No working container runtime found (tried docker, podman)" >&2
+  exit 1
 fi
 
 ARGS=(-f docker-compose.yml)

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

@@ -40,8 +40,8 @@
  *   <li>System PATH - falls back to a {@code gitleaks} binary already installed on the host/container
  * </ol>
  *
- * <p>If no binary is found, scanning is skipped and an empty {@link Optional} is returned (fail-open). Pushes are never
- * blocked due to scanner unavailability.
+ * <p>If no binary is found or the scan errors, an empty {@link Optional} is returned. Whether the push is blocked is
+ * determined by the caller — callers with {@code secret-scan: enabled: true} block the push (fail-closed).
  *
  * <p>The bundled binary extracted from the JAR is deleted on JVM shutdown. Binaries downloaded to {@code installDir}
  * persist across restarts.
@@ -58,6 +58,9 @@ public class GitleaksRunner {
     /** Classpath resource name for the pre-bundled gitleaks binary (opt-in, not shipped by default). */
     private static final String BUNDLED_BINARY_RESOURCE = "gitleaks";
 
+    /** Exit code gitleaks returns when findings are present (distinct from error exit codes). */
+    private static final int FINDINGS_EXIT_CODE = 2;
+
     private static final String DEFAULT_INSTALL_DIR =
             System.getProperty("user.home") + "/.cache/git-proxy-java/gitleaks";
 
@@ -132,13 +135,13 @@ public Optional<List<Finding>> scan(String diff, SecretScanConfig config) {
             if (exitCode == 0) {
                 log.debug("gitleaks: no findings");
                 return Optional.of(Collections.emptyList());
-            } else if (exitCode == 1) {
+            } else if (exitCode == FINDINGS_EXIT_CODE) {
                 List<Finding> findings = readFindings(reportFile);
                 enrichFindings(findings, diff);
                 log.debug("gitleaks: {} finding(s)", findings.size());
                 return Optional.of(findings);
             } else {
-                log.warn("gitleaks exited with code {} - secret scanning skipped (fail-open)", exitCode);
+                log.warn("gitleaks exited with code {} - treat as scanner error", exitCode);
                 return Optional.empty();
             }
 
@@ -214,12 +217,12 @@ public Optional<List<Finding>> scanGit(Path repoDir, String commitFrom, String c
             if (exitCode == 0) {
                 log.debug("gitleaks git: no findings");
                 return Optional.of(Collections.emptyList());
-            } else if (exitCode == 1) {
+            } else if (exitCode == FINDINGS_EXIT_CODE) {
                 List<Finding> findings = readFindings(reportFile);
                 log.debug("gitleaks git: {} finding(s)", findings.size());
                 return Optional.of(findings);
             } else {
-                log.warn("gitleaks git exited with code {} - secret scanning skipped (fail-open)", exitCode);
+                log.warn("gitleaks git exited with code {} - treat as scanner error", exitCode);
                 return Optional.empty();
             }
 
@@ -429,6 +432,8 @@ private static List<String> buildCommand(Path binaryPath, Path reportFile, Path
         cmd.add("json");
         cmd.add("--report-path");
         cmd.add(reportFile.toString());
+        cmd.add("--exit-code");
+        cmd.add(String.valueOf(FINDINGS_EXIT_CODE));
 
         if (configFilePath != null) {
             cmd.add("--config");
@@ -449,6 +454,8 @@ private static List<String> buildGitCommand(Path binaryPath, String logOpts, Pat
         cmd.add(reportFile.toString());
         cmd.add("--no-banner");
         cmd.add("--redact");
+        cmd.add("--exit-code");
+        cmd.add(String.valueOf(FINDINGS_EXIT_CODE));
 
         if (configFilePath != null) {
             cmd.add("--config");

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

@@ -25,7 +25,8 @@
  * to limit scanning to commits not already reachable from any existing ref; branch updates use
  * {@code commitFrom..commitTo}.
  *
- * <p>If the scanner is unavailable the hook continues (fail-open), recording a SKIPPED step.
+ * <p>If the scanner is unavailable and secret scanning is enabled, the push is blocked (fail-closed). If scanning is
+ * disabled the hook records a SKIPPED step.
  */
 @Slf4j
 @RequiredArgsConstructor
@@ -61,7 +62,6 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
             Optional<List<GitleaksRunner.Finding>> result = runner.scanGit(repoDir, commitFrom, commitTo, config);
 
             if (result.isEmpty()) {
-                // Fail-open: scanner unavailable or errored - GitleaksRunner already logged the detail
                 scannerUnavailable = true;
                 continue;
             }
@@ -72,13 +72,19 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
             }
         }
 
-        if (scannerUnavailable && allViolations.isEmpty()) {
-            pushContext.addStep(PushStep.builder()
-                    .stepName(STEP_NAME)
-                    .stepOrder(ORDER)
-                    .status(StepStatus.SKIPPED)
-                    .build());
-            return;
+        if (scannerUnavailable) {
+            if (config.isEnabled()) {
+                String msg = "Secret scanning failed — scanner error or unavailable. "
+                        + "Push blocked because secret-scan is enabled. Check server logs for details.";
+                allViolations.add(new Violation(msg, msg, sym(CROSS_MARK) + "  " + msg));
+            } else {
+                pushContext.addStep(PushStep.builder()
+                        .stepName(STEP_NAME)
+                        .stepOrder(ORDER)
+                        .status(StepStatus.SKIPPED)
+                        .build());
+                return;
+            }
         }
 
         if (allViolations.isEmpty()) {

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/SecretScanningFilter.java

@@ -25,6 +25,9 @@
  * correctly because gitleaks has full file-path context when operating in git mode.
  *
  * <p>Runs at order 340, after {@link ScanDiffFilter} (order 300), within the content filters range (200-399).
+ *
+ * <p>If the scanner errors or is unavailable when scanning is enabled, the push is blocked (fail-closed) with a clear
+ * error message. Check server logs for the underlying cause.
  */
 @Slf4j
 public class SecretScanningFilter extends AbstractGitProxyFilter {
@@ -95,7 +98,10 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
                 runner.scanGit(repo.getDirectory().toPath(), commitFrom, commitTo, config);
 
         if (result.isEmpty()) {
-            // Fail-open: scanner unavailable - GitleaksRunner already logged the detail
+            String msg = "Secret scanning failed — scanner error or unavailable. "
+                    + "Push blocked because secret-scan is enabled. Check server logs for details.";
+            log.error("Secret scanner returned no result — blocking push (fail-closed)");
+            recordIssue(request, msg, sym(CROSS_MARK) + "  " + msg);
             return;
         }
 

git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java

@@ -162,13 +162,18 @@ private void applyInfoRefsRules(HttpServletRequest request, HttpServletResponse
                 log.debug("Blocking /info/refs — matched deny rule: {}", d.ruleId());
                 if (effectiveOp == HttpOperation.FETCH && fetchStore != null) recordFetch(request, false);
                 setResult(request, GitRequestDetails.GitResult.REJECTED, "Repository blocked by deny rule");
-                response.sendError(provider.getBlockedInfoRefsStatus());
+                response.sendError(
+                        provider.getBlockedInfoRefsStatus(),
+                        "Repository access denied: this repository has been explicitly blocked by an administrator.");
             }
             case UrlRuleEvaluator.Result.NotAllowed n -> {
                 log.debug("Blocking /info/refs — no rule matched");
                 if (effectiveOp == HttpOperation.FETCH && fetchStore != null) recordFetch(request, false);
                 setResult(request, GitRequestDetails.GitResult.REJECTED, "Repository not in allow rules");
-                response.sendError(provider.getBlockedInfoRefsStatus());
+                response.sendError(
+                        provider.getBlockedInfoRefsStatus(),
+                        "Repository access denied: this repository is not in the allow list."
+                                + " Contact an administrator to add it.");
             }
             case UrlRuleEvaluator.Result.Allowed a -> {
                 /* pass through — request is permitted */