From 962edc3742c14351c583c128b4559a3a85ab2284 Mon Sep 17 00:00:00 2001 From: Thomas Cooper Date: Tue, 12 May 2026 22:54:39 -0400 Subject: [PATCH 1/4] fix: surface actionable validation step messages in push detail view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes scanDiff, scanSecrets, identityVerification WARN, and URL rule validation steps so the dashboard push detail panel and git client terminal both display specific, actionable error information instead of placeholder values (e.g. branch refs or missing context). Key changes: - BlockedContentDiffCheck: track violations as Map so formattedDetail includes the matching content, not null - DiffScanningHook: pass violation.formattedDetail() to addIssue instead of hardcoding the branch ref as the content - ScanDiffFilter: loop per violation (consistent with other filters) instead of collapsing all findings into a single step - SecretScanningHook/SecretScanningFilter: append remediation hint to each finding's formattedDetail so the dashboard shows actionable next steps - IdentityVerificationHook: emit per-violation sideband warnings in WARN mode so developers see the mismatch in their terminal even when push is not blocked - RepositoryUrlRuleHook: fix duplicate step bug — when validationContext is set, PushStorePersistenceHook already creates the FAIL step from the issue, so the explicit pushContext.addStep() was creating a second FAIL entry; also align step name to "checkUrlRules" (was class name "RepositoryUrlRuleHook") - UrlRuleAggregateFilter: align "not in allow list" wording and add NO_ENTRY symbol to NotAllowed title for parity with Denied case - PushStorePersistenceHook: add "checkUrlRules" (order 100) to HOOK_STEP_ORDER; strip ANSI codes from issue.detail() at storage time - GitProxyFilter.recordStep: strip ANSI from content before persisting; change null message args to empty strings throughout - GitClientUtils.stripColors: make public for use in persistence hooks - Rename createAccessRule/deleteAccessRule → createUrlRule/deleteUrlRule in api.ts and Repos.tsx to align with Java internals; update "Access type" label to "Rule type" closes #244 Co-Authored-By: Claude Sonnet 4.6 --- .../finos/gitproxy/git/DiffScanningHook.java | 2 +- .../finos/gitproxy/git/GitClientUtils.java | 2 +- .../git/IdentityVerificationHook.java | 10 +++- .../git/PushStorePersistenceHook.java | 3 +- .../gitproxy/git/RepositoryUrlRuleHook.java | 51 ++++++++++++------- .../gitproxy/git/SecretScanningHook.java | 6 ++- .../filter/CheckHiddenCommitsFilter.java | 2 +- .../servlet/filter/GitProxyFilter.java | 5 +- .../servlet/filter/ScanDiffFilter.java | 21 ++------ .../servlet/filter/SecretScanningFilter.java | 4 +- .../filter/UrlRuleAggregateFilter.java | 7 ++- .../validation/BlockedContentDiffCheck.java | 16 +++--- .../servlet/filter/ScanDiffFilterTest.java | 28 ++++++++++ .../BlockedContentDiffCheckTest.java | 29 +++++++++++ git-proxy-java-dashboard/frontend/src/api.ts | 8 +-- .../frontend/src/pages/Repos.tsx | 12 ++--- 16 files changed, 139 insertions(+), 67 deletions(-) diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/DiffScanningHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/DiffScanningHook.java index 254096ca..b8387bff 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/DiffScanningHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/DiffScanningHook.java @@ -61,7 +61,7 @@ public void onPreReceive(ReceivePack rp, Collection 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; diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitClientUtils.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitClientUtils.java index cd00982e..56f738b8 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/GitClientUtils.java +++ b/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(), ""); } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java index e2f36ac2..58925c93 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/IdentityVerificationHook.java @@ -126,12 +126,18 @@ public void onPreReceive(ReceivePack rp, Collection 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) diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java index 8e2ea963..542dbb8b 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/PushStorePersistenceHook.java +++ b/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 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++; diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java index e59d1df9..aaa5d9d2 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/RepositoryUrlRuleHook.java @@ -44,7 +44,12 @@ public void onPreReceive(ReceivePack rp, Collection 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,7 +64,13 @@ public void onPreReceive(ReceivePack rp, Collection 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()); @@ -67,22 +78,24 @@ public void onPreReceive(ReceivePack rp, Collection commands) { } 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 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 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) { @@ -90,13 +103,13 @@ private void blockPush(ReceivePack rp, Collection commands, Stri 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() { diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/SecretScanningHook.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/SecretScanningHook.java index fed8ce79..96a119cc 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/git/SecretScanningHook.java +++ b/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,7 +70,7 @@ public void onPreReceive(ReceivePack rp, Collection 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)); } } @@ -76,7 +78,7 @@ public void onPreReceive(ReceivePack rp, Collection commands) { 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) diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckHiddenCommitsFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckHiddenCommitsFilter.java index c0499acc..881bf68f 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/CheckHiddenCommitsFilter.java +++ b/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()); } } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/GitProxyFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/GitProxyFilter.java index 8775c7b1..251dc6c5 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/GitProxyFilter.java +++ b/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(); diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/ScanDiffFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/ScanDiffFilter.java index 46615d6e..86ae23bf 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/ScanDiffFilter.java +++ b/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,7 +8,6 @@ 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; @@ -19,7 +15,6 @@ 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()); } } } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/SecretScanningFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/SecretScanningFilter.java index d01fabef..a1c3618c 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/SecretScanningFilter.java +++ b/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 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); } } } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java index c3202f42..c1e8d016 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/servlet/filter/UrlRuleAggregateFilter.java @@ -121,16 +121,15 @@ public void doHttpFilter(HttpServletRequest request, HttpServletResponse respons private void sendNotAllowed(HttpServletRequest request, HttpServletResponse response, HttpOperation operation) throws java.io.IOException { String action = operation == HttpOperation.PUSH ? "Push" : "Fetch"; - String title = action + " blocked - Repository Not Allowed"; + String title = sym(NO_ENTRY) + " " + action + " Blocked - Repository Not Allowed"; String verb = operation == HttpOperation.PUSH ? "Pushes to" : "Fetches from"; String message = verb + " this repository are not permitted.\n" + "\n" - + "Contact an administrator to add this repository\n" - + "to the allow rules."; + + "Contact an administrator to add this repository to the allow rules."; rejectAndSendError( request, response, - "Repository not in allow rules", + "Repository not in allow list", GitClientUtils.formatForOperation(title, message, GitClientUtils.AnsiColor.RED, operation)); } diff --git a/git-proxy-java-core/src/main/java/org/finos/gitproxy/validation/BlockedContentDiffCheck.java b/git-proxy-java-core/src/main/java/org/finos/gitproxy/validation/BlockedContentDiffCheck.java index 1993da32..3f37649e 100644 --- a/git-proxy-java-core/src/main/java/org/finos/gitproxy/validation/BlockedContentDiffCheck.java +++ b/git-proxy-java-core/src/main/java/org/finos/gitproxy/validation/BlockedContentDiffCheck.java @@ -1,9 +1,9 @@ package org.finos.gitproxy.validation; -import java.util.LinkedHashSet; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.regex.Pattern; import lombok.RequiredArgsConstructor; import org.finos.gitproxy.config.CommitConfig; @@ -28,7 +28,8 @@ public Optional> check(String diff) { return Optional.of(List.of()); } - Set violations = new LinkedHashSet<>(); + // Map from violation summary → first matching line (putIfAbsent deduplicates per pattern+file) + Map violations = new LinkedHashMap<>(); String currentFile = null; for (String line : diff.lines().toList()) { @@ -45,20 +46,21 @@ public Optional> check(String diff) { for (String literal : block.getLiterals()) { if (content.toLowerCase().contains(literal.toLowerCase())) { String location = currentFile != null ? " in " + currentFile : ""; - violations.add("blocked term: \"" + literal + "\"" + location); + violations.putIfAbsent("blocked term: \"" + literal + "\"" + location, content.strip()); } } for (Pattern pattern : block.getPatterns()) { if (pattern.matcher(content).find()) { String location = currentFile != null ? " in " + currentFile : ""; - violations.add("blocked pattern: " + pattern.pattern() + location); + violations.putIfAbsent("blocked pattern: " + pattern.pattern() + location, content.strip()); } } } - return Optional.of( - violations.stream().map(v -> new Violation(v, v, null)).collect(java.util.stream.Collectors.toList())); + return Optional.of(violations.entrySet().stream() + .map(e -> new Violation(e.getKey(), e.getKey(), e.getKey() + "\n " + e.getValue())) + .collect(java.util.stream.Collectors.toList())); } /** Extracts the {@code b/} path from a {@code diff --git a/... b/...} header line. */ diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/ScanDiffFilterTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/ScanDiffFilterTest.java index 55a596c1..f62cec27 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/ScanDiffFilterTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/servlet/filter/ScanDiffFilterTest.java @@ -244,6 +244,34 @@ void nullRepository_skips() throws Exception { assertEquals(GitRequestDetails.GitResult.PENDING, details.getResult()); } + // ---- blocked step has specific errorMessage and matching line in content ---- + + @Test + void blockedLiteral_stepHasSpecificErrorMessageAndMatchingLine() throws Exception { + GitRequestDetails details = pushDetails(cleanCommit, blockedCommit); + FakeResponse resp = new FakeResponse(); + + filterWithLiteral("supersecret").doHttpFilter(mockRequest(details), resp.mock); + + PushStep scanStep = details.getSteps().stream() + .filter(s -> "scanDiff".equals(s.getStepName()) && s.getStatus() == StepStatus.FAIL) + .findFirst() + .orElseThrow(() -> new AssertionError("scanDiff FAIL step not found")); + + assertNotNull(scanStep.getErrorMessage(), "errorMessage must be set"); + assertTrue( + scanStep.getErrorMessage().contains("supersecret"), + "errorMessage should name the blocked term, got: " + scanStep.getErrorMessage()); + assertFalse( + scanStep.getErrorMessage().equals("Diff contains blocked content"), + "errorMessage must be violation-specific, not a generic label"); + + assertNotNull(scanStep.getContent(), "content must be set"); + assertTrue( + scanStep.getContent().contains("supersecret"), + "content should include the matching line, got: " + scanStep.getContent()); + } + // ---- diff step content contains the actual diff text ---- @Test diff --git a/git-proxy-java-core/src/test/java/org/finos/gitproxy/validation/BlockedContentDiffCheckTest.java b/git-proxy-java-core/src/test/java/org/finos/gitproxy/validation/BlockedContentDiffCheckTest.java index e535a361..85a5f8c5 100644 --- a/git-proxy-java-core/src/test/java/org/finos/gitproxy/validation/BlockedContentDiffCheckTest.java +++ b/git-proxy-java-core/src/test/java/org/finos/gitproxy/validation/BlockedContentDiffCheckTest.java @@ -100,6 +100,35 @@ void deduplicatesViolations_sameTermMultipleLines() { assertEquals(1, violations.size()); } + @Test + void violationFormattedDetailIncludesMatchingLine() { + BlockedContentDiffCheck check = checkWithLiterals("secret"); + List violations = check.check(SAMPLE_DIFF).orElseThrow(); + assertEquals(1, violations.size()); + String detail = violations.get(0).formattedDetail(); + assertNotNull(detail); + assertTrue(detail.contains("src/Foo.java"), "detail must reference the file"); + assertTrue(detail.contains("added line with secret content"), "detail must include the matching line"); + } + + @Test + void deduplication_formattedDetailContainsFirstMatchingLine() { + String diff = """ + diff --git a/a.txt b/a.txt + --- a/a.txt + +++ b/a.txt + @@ -1 +1,2 @@ + +first password occurrence + +second password occurrence + """; + BlockedContentDiffCheck check = checkWithLiterals("password"); + List violations = check.check(diff).orElseThrow(); + assertEquals(1, violations.size()); + assertTrue( + violations.get(0).formattedDetail().contains("first password occurrence"), + "formattedDetail should show the first matching line"); + } + @Test void extractFileName_standardHeader() { assertEquals( diff --git a/git-proxy-java-dashboard/frontend/src/api.ts b/git-proxy-java-dashboard/frontend/src/api.ts index 0f2bfc48..0d962d78 100644 --- a/git-proxy-java-dashboard/frontend/src/api.ts +++ b/git-proxy-java-dashboard/frontend/src/api.ts @@ -248,7 +248,7 @@ export async function addUserPermission( return res.json() } -export async function createAccessRule(rule: { +export async function createUrlRule(rule: { access: 'ALLOW' | 'DENY' operations: 'BOTH' | 'PUSH' | 'FETCH' slug?: string @@ -263,13 +263,13 @@ export async function createAccessRule(rule: { headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ ...rule, enabled: true, source: 'DB' }), }) - if (!res.ok) await parseErrorResponse(res, 'Failed to create access rule') + if (!res.ok) await parseErrorResponse(res, 'Failed to create URL rule') return res.json() } -export async function deleteAccessRule(id: string) { +export async function deleteUrlRule(id: string) { const res = await apiFetch(`/api/repos/rules/${encodeURIComponent(id)}`, { method: 'DELETE' }) - if (!res.ok) await parseErrorResponse(res, 'Failed to delete access rule') + if (!res.ok) await parseErrorResponse(res, 'Failed to delete URL rule') } export interface TcpResult { diff --git a/git-proxy-java-dashboard/frontend/src/pages/Repos.tsx b/git-proxy-java-dashboard/frontend/src/pages/Repos.tsx index 75fce865..071109d0 100644 --- a/git-proxy-java-dashboard/frontend/src/pages/Repos.tsx +++ b/git-proxy-java-dashboard/frontend/src/pages/Repos.tsx @@ -1,5 +1,5 @@ import { useEffect, useRef, useState } from 'react' -import { createAccessRule, deleteAccessRule, fetchProviders } from '../api' +import { createUrlRule, deleteUrlRule, fetchProviders } from '../api' interface ActiveRepo { provider: string @@ -183,7 +183,7 @@ function AddRuleModal({ const raw = form.pattern.trim() const encoded = form.patternType === 'REGEX' ? `regex:${raw}` : raw - const payload: Parameters[0] = { + const payload: Parameters[0] = { access: form.access, operations: form.operations, provider: form.provider || undefined, @@ -193,7 +193,7 @@ function AddRuleModal({ else if (form.targetType === 'owner') payload.owner = encoded else payload.name = encoded - const created = await createAccessRule(payload) + const created = await createUrlRule(payload) onCreated(created) onClose() } catch (e) { @@ -217,9 +217,9 @@ function AddRuleModal({
- {/* Access type */} + {/* Rule type */}
- +
{(['ALLOW', 'DENY'] as const).map((a) => (