fix: surface actionable validation step messages in push detail view (#246)

## Summary

- **Fixes #244**: `scanDiff` step content was showing the branch ref
(`ref: refs/heads/...`) instead of the blocked pattern and matching file
— `BlockedContentDiffCheck` was producing violations with null
`formattedDetail`, and `DiffScanningHook` was hardcoding the ref name as
content
- **scanSecrets**: both hook and filter now append a remediation hint
(rotate + scrub history) to each finding so the dashboard gives
actionable next steps
- **identityVerification WARN mode**: now emits per-violation sideband
warnings to the git client terminal so developers see the mismatch even
when the push isn't blocked
- **URL rule step consistency**: `RepositoryUrlRuleHook` now uses step
name `checkUrlRules` (not class name); fixes a duplicate step bug where
both `validationContext.addIssue` and `pushContext.addStep(FAIL)` were
called, producing two entries in the push detail view; wording aligned
with `UrlRuleAggregateFilter`
- **URL rules page display fixed**: `Repos.tsx` was using the old
`slug`/`owner`/`name` field shape; updated to
`target`/`value`/`matchType` from the `34aebb9` backend refactor — every
rule was showing as "any: *" and new rules created via UI were saving
with `value=null` (never matching)
- **Admin connectivity crash fixed**: `SpringWebConfig` sets `NON_NULL`
Jackson inclusion globally, so null `tls`/`http` fields are omitted from
JSON rather than serialised as `null`; the frontend strict `=== null`
check fell through to `tls.status` and crashed — changed to `== null`
throughout and updated types to `| undefined`
- **Provider display**: all provider dropdowns (Add Rule, Add
Permission, Add SCM identity) and provider labels throughout the UI were
showing `host` (e.g. `github.com`) instead of `name` (e.g. `github`);
fixed across `Repos.tsx`, `UserDetail.tsx`, `Profile.tsx`; clone URL
construction switched to use `provider.proxyPath` from the API response;
`Repos.tsx` provider state typed as `Provider[]` instead of an ad-hoc
inline type
- **Terminology**: `createAccessRule`/`deleteAccessRule` →
`createUrlRule`/`deleteUrlRule` in `api.ts` and `Repos.tsx`; "Access
type" label → "Rule type" in the add-rule modal

## Test plan

- [ ] Unit tests: `BlockedContentDiffCheckTest` — assertions for
`formattedDetail` content and deduplication with first matching line
- [ ] Unit tests: `ScanDiffFilterTest` — assertion that `errorMessage`
contains the blocked pattern and `content` includes the matching line
- [ ] Server e2e tests: all 9/9 pass (proxy push flows, URL rule
enforcement, secret scanning)
- [ ] Dashboard LDAP e2e test failure is pre-existing and unrelated
(timing/container issue on corporate network; passes on CI)
- [ ] Manual: URL Rules tab shows correct target/value/matchType for
each rule; new rules created via UI save and evaluate correctly
- [ ] Manual: Admin → Provider Connectivity no longer crashes when
tls/http are absent (HTTP provider or TCP failure path)
- [ ] Manual: All provider dropdowns show friendly name (e.g. "github")
not hostname (e.g. "github.com"); clone URL in active repos view is
correct

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Author: coopernetes

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

@@ -61,7 +61,7 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
 
                 if (!violations.isEmpty()) {
                     for (Violation violation : violations) {
-                        validationContext.addIssue("scanDiff", violation.reason(), "ref: " + cmd.getRefName());
+                        validationContext.addIssue("scanDiff", violation.reason(), violation.formattedDetail());
                         logs.add("FAIL: " + violation.reason());
                     }
                     anyFailed = true;

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

@@ -191,7 +191,7 @@ private static String convertSymbolsToPlain(String content) {
         return content;
     }
 
-    private static String stripColors(String content) {
+    public static String stripColors(String content) {
         for (var color : AnsiColor.values()) {
             content = content.replace(color.getValue(), "");
         }

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

@@ -126,12 +126,18 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
             validationContext.addIssue(
                     STEP_NAME, "Commit identity does not match push user " + user.getUsername(), detail);
         } else {
-            // WARN mode — push proceeds; violations are surfaced via the validation summary, not
-            // immediate per-message streaming, so they appear in the correct order with context.
+            // WARN mode — push proceeds but developer is warned at the terminal and in the dashboard.
             log.warn(
                     "Identity verification warnings for push user '{}': {} mismatch(es)",
                     user.getUsername(),
                     violations.size());
+            rp.sendMessage(GitClientUtils.color(
+                    YELLOW,
+                    sym(WARNING) + "  Identity warning: " + violations.size() + " commit email(s) not registered to "
+                            + user.getUsername()));
+            for (String v : violations) {
+                rp.sendMessage("  " + v);
+            }
             pushContext.addStep(PushStep.builder()
                     .stepName(STEP_NAME)
                     .stepOrder(ORDER)

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

@@ -45,6 +45,7 @@ public class PushStorePersistenceHook {
      * sort validation steps in the same order as proxy mode.
      */
     private static final Map<String, Integer> HOOK_STEP_ORDER = Map.of(
+            "checkUrlRules", 100,
             "checkAuthorEmails", 2100,
             "checkCommitMessages", 2200,
             "scanDiff", 2300,
@@ -142,7 +143,7 @@ public PreReceiveHook validationResultHook(ValidationContext validationContext)
                                     .stepName(issue.hookName())
                                     .stepOrder(stepOrder)
                                     .status(StepStatus.FAIL)
-                                    .content(issue.detail())
+                                    .content(GitClientUtils.stripColors(issue.detail()))
                                     .errorMessage(issue.summary())
                                     .build());
                             fallbackOrder++;

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

@@ -44,7 +44,12 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
         String repoSlug = pushContext.getRepoSlug();
         if (repoSlug == null || repoSlug.isBlank()) {
             log.warn("No repoSlug in push context — cannot evaluate URL rules, blocking push (fail-closed)");
-            blockPush(rp, commands, "Repository path unavailable");
+            blockPush(
+                    rp,
+                    commands,
+                    "Repository path unavailable",
+                    "Push Blocked - Repository Not Allowed",
+                    "Repository path could not be determined. Contact an administrator.");
             return;
         }
 
@@ -59,44 +64,52 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
         switch (result) {
             case UrlRuleEvaluator.Result.Denied d -> {
                 log.debug("Push blocked by deny rule: {}", d.ruleId());
-                blockPush(rp, commands, "Repository denied by access rule");
+                blockPush(
+                        rp,
+                        commands,
+                        "Repository blocked by deny rule",
+                        "Push Blocked - Repository Denied",
+                        "Pushes to this repository are not permitted.\n\n"
+                                + "This repository has been explicitly denied by an administrator.");
             }
             case UrlRuleEvaluator.Result.Allowed a -> {
                 log.debug("Push allowed by rule: {}", a.ruleId());
                 recordPass();
             }
             case UrlRuleEvaluator.Result.NotAllowed n -> {
                 log.debug("Push blocked — no rule matched");
-                blockPush(rp, commands, "Repository is not in the allow list");
+                blockPush(
+                        rp,
+                        commands,
+                        "Repository not in allow list",
+                        "Push Blocked - Repository Not Allowed",
+                        "Pushes to this repository are not permitted.\n\n"
+                                + "Contact an administrator to add this repository to the allow rules.");
             }
         }
     }
 
-    private void blockPush(ReceivePack rp, Collection<ReceiveCommand> commands, String reason) {
-        String detail = GitClientUtils.format(
-                sym(NO_ENTRY) + "  Push Blocked - Repository Not Allowed",
-                sym(CROSS_MARK)
-                        + "  "
-                        + reason
-                        + ".\n\nContact an administrator to add this repository to the allow rules.",
-                RED,
-                null);
+    private void blockPush(
+            ReceivePack rp, Collection<ReceiveCommand> commands, String reason, String title, String message) {
+        String detail =
+                GitClientUtils.format(sym(NO_ENTRY) + "  " + title, sym(CROSS_MARK) + "  " + message, RED, null);
         if (validationContext != null) {
-            validationContext.addIssue("RepositoryUrlRuleHook", reason, detail);
+            validationContext.addIssue("checkUrlRules", reason, detail);
+            // PushStorePersistenceHook creates the FAIL step from the issue; don't also add it to pushContext
         } else {
             rp.sendMessage(detail);
             for (ReceiveCommand cmd : commands) {
                 if (cmd.getResult() == ReceiveCommand.Result.NOT_ATTEMPTED) {
                     cmd.setResult(ReceiveCommand.Result.REJECTED_OTHER_REASON, reason);
                 }
             }
+            pushContext.addStep(PushStep.builder()
+                    .stepName("checkUrlRules")
+                    .stepOrder(ORDER)
+                    .status(StepStatus.FAIL)
+                    .content(reason)
+                    .build());
         }
-        pushContext.addStep(PushStep.builder()
-                .stepName("checkUrlRules")
-                .stepOrder(ORDER)
-                .status(StepStatus.FAIL)
-                .content(reason)
-                .build());
     }
 
     private void recordPass() {

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

@@ -34,6 +34,8 @@ public class SecretScanningHook implements GitProxyHook {
 
     private static final int ORDER = 340;
     private static final String STEP_NAME = "scanSecrets";
+    private static final String REMEDIATION_HINT =
+            "→ Rotate any exposed credentials and remove the secret from your commit history before pushing.";
 
     private final SecretScanConfig config;
     private final ValidationContext validationContext;
@@ -68,15 +70,15 @@ public void onPreReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
 
             for (GitleaksRunner.Finding f : result.get()) {
                 String msg = f.toMessage();
-                allViolations.add(new Violation(msg, msg, sym(CROSS_MARK) + "  " + msg));
+                allViolations.add(new Violation(msg, msg, sym(CROSS_MARK) + "  " + msg + "\n" + REMEDIATION_HINT));
             }
         }
 
         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));
+                allViolations.add(new Violation(msg, msg, sym(CROSS_MARK) + "  " + msg + "\n" + REMEDIATION_HINT));
             } else {
                 pushContext.addStep(PushStep.builder()
                         .stepName(STEP_NAME)

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

@@ -109,7 +109,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
 
         } catch (Exception e) {
             log.warn("Skipping hidden commits check: {}", e.getMessage());
-            recordStep(request, StepStatus.SKIPPED, null, e.getMessage());
+            recordStep(request, StepStatus.SKIPPED, "", e.getMessage());
         }
     }
 

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

@@ -18,6 +18,7 @@
 import org.eclipse.jgit.transport.SideBandOutputStream;
 import org.finos.gitproxy.db.model.PushStep;
 import org.finos.gitproxy.db.model.StepStatus;
+import org.finos.gitproxy.git.GitClientUtils;
 import org.finos.gitproxy.git.GitRequestDetails;
 import org.finos.gitproxy.git.HttpOperation;
 // import org.springframework.core.Ordered;
@@ -130,7 +131,7 @@ default void doFilter(ServletRequest request, ServletResponse response, FilterCh
             // so we must not overwrite that with a spurious PASS.
             int stepsAfter = details != null ? details.getSteps().size() : 0;
             if (stepsAfter == stepsBefore) {
-                recordStep(httpRequest, StepStatus.PASS, null, null);
+                recordStep(httpRequest, StepStatus.PASS, "", "");
             }
         }
         chain.doFilter(request, response);
@@ -222,7 +223,7 @@ default void recordStep(HttpServletRequest request, StepStatus status, String re
                 .stepName(getStepName())
                 .stepOrder(this.getOrder())
                 .status(status)
-                .content(content)
+                .content(GitClientUtils.stripColors(content))
                 .blockedMessage(status == StepStatus.BLOCKED ? reason : null)
                 .errorMessage(status == StepStatus.FAIL ? reason : null)
                 .build();

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

@@ -1,8 +1,5 @@
 package org.finos.gitproxy.servlet.filter;
 
-import static org.finos.gitproxy.git.GitClientUtils.AnsiColor.*;
-import static org.finos.gitproxy.git.GitClientUtils.SymbolCodes.*;
-import static org.finos.gitproxy.git.GitClientUtils.sym;
 import static org.finos.gitproxy.servlet.GitProxyServlet.GIT_REQUEST_ATTR;
 
 import jakarta.servlet.http.HttpServletRequest;
@@ -11,15 +8,13 @@
 import java.util.List;
 import java.util.Set;
 import java.util.function.Supplier;
-import java.util.stream.Collectors;
 import lombok.extern.slf4j.Slf4j;
 import org.eclipse.jgit.lib.Repository;
 import org.finos.gitproxy.config.DiffScanConfig;
 import org.finos.gitproxy.db.model.PushStep;
 import org.finos.gitproxy.db.model.StepStatus;
 import org.finos.gitproxy.git.CommitInspectionService;
 import org.finos.gitproxy.git.DiffGenerationHook;
-import org.finos.gitproxy.git.GitClientUtils;
 import org.finos.gitproxy.git.GitRequestDetails;
 import org.finos.gitproxy.git.HttpOperation;
 import org.finos.gitproxy.provider.GitProxyProvider;
@@ -101,23 +96,17 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
 
             if (!violations.isEmpty()) {
                 log.warn("Diff scan found {} violation(s)", violations.size());
-                String title = sym(NO_ENTRY) + "  Push Blocked - Diff Contains Blocked Content";
-                String violationList = violations.stream()
-                        .map(v -> sym(CROSS_MARK) + "  " + v.reason())
-                        .collect(Collectors.joining("\n"));
-                String message = "Diff content contains blocked patterns:\n\n" + violationList;
-                recordIssue(
-                        request,
-                        "Diff contains blocked content",
-                        GitClientUtils.color(RED, GitClientUtils.format(title, message, RED, null)));
+                for (Violation v : violations) {
+                    recordIssue(request, v.reason(), v.formattedDetail());
+                }
             } else {
                 log.debug("Diff scan passed for {}..{}", fromCommit, toCommit);
-                recordStep(request, StepStatus.PASS, null, null);
+                recordStep(request, StepStatus.PASS, "", "");
             }
 
         } catch (Exception e) {
             log.warn("Skipping diff scan for push {}..{}: {}", fromCommit, toCommit, e.getMessage());
-            recordStep(request, StepStatus.SKIPPED, null, e.getMessage());
+            recordStep(request, StepStatus.SKIPPED, "", e.getMessage());
         }
     }
 }

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

@@ -33,6 +33,8 @@
 public class SecretScanningFilter extends AbstractGitProxyFilter {
 
     private static final int ORDER = 340;
+    private static final String REMEDIATION_HINT =
+            "→ Rotate any exposed credentials and remove the secret from your commit history before pushing.";
 
     private final Supplier<SecretScanConfig> configSupplier;
     private final GitleaksRunner runner;
@@ -114,7 +116,7 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons
         log.warn("Secret scan found {} finding(s)", findings.size());
         for (GitleaksRunner.Finding f : findings) {
             String msg = f.toMessage();
-            recordIssue(request, msg, sym(CROSS_MARK) + "  " + msg);
+            recordIssue(request, msg, sym(CROSS_MARK) + "  " + msg + "\n" + REMEDIATION_HINT);
         }
     }
 }