refactor(common): make filename redaction a shared utility

It was private to RecordFailure, which is the wrong home for a rule that
belongs wherever a document name might be written down. policy_processed_files
already stores the full path in plaintext, and that is the next caller.

Moves the pattern to common as FilenameRedaction.attemptRedaction, named for
what it is rather than what it guarantees, with the cases moved alongside it.
RecordFailurePrivacyTest keeps one check that a stored message goes through it
at all, which is the part that belongs to the failure package.

Also stops storing the downstream description for a kind we recognise: we
already have its own copy, so the raw text only adds somewhere for a name to
hide. Unrecognised failures keep theirs, since it is the only thing telling a
reviewer what went wrong.

Author: EthanHealy01

app/common/src/main/java/stirling/software/common/util/FilenameRedaction.java

@@ -0,0 +1,47 @@
+package stirling.software.common.util;
+
+import java.util.regex.Pattern;
+
+/**
+ * Removes anything shaped like a file name from free text, so a document's name is not persisted or
+ * logged alongside whatever went wrong with it.
+ *
+ * <p>Best-effort by nature: this reads prose written elsewhere and guesses. Somewhere that can
+ * avoid holding the name at all should do that instead and treat this as the backstop.
+ */
+public final class FilenameRedaction {
+
+    /** What a redacted name is replaced with. Recognisable, and short enough not to bloat a row. */
+    public static final String PLACEHOLDER = "<file>";
+
+    /**
+     * Name characters ending in one to three short extensions. Matched by shape, so a newly
+     * supported format needs no maintenance here.
+     *
+     * <p>Spaces are crossed only when the name is delimited by a quote, bracket or path separator:
+     * an undelimited one cannot be told from the sentence around it, and swallowing the sentence
+     * costs more than it saves. An all-digit extension is not one, which keeps {@code v2.14.2}
+     * intact, and the lookarounds keep it off dotted identifiers so {@code java.lang.Foo} survives
+     * a stack trace.
+     */
+    private static final Pattern FILE_NAME =
+            Pattern.compile(
+                    "(?<![\\w.])(?:(?<=[\"'(\\[/\\\\])[\\p{L}\\p{N}_%+&'()\\[\\]\\-]+"
+                            + "(?:[ ][\\p{L}\\p{N}_%+&'()\\[\\]\\-]+){0,6}"
+                            + "|[\\p{L}\\p{N}_%+&'()\\[\\]\\-]+)"
+                            + "(?:\\.(?![0-9]+(?:\\b|\\.))[\\p{L}\\p{N}]{1,8}){1,3}(?![\\w.])",
+                    Pattern.UNICODE_CHARACTER_CLASS);
+
+    private FilenameRedaction() {}
+
+    /**
+     * {@code text} with every file name replaced by {@link #PLACEHOLDER}. Null in, null out, so a
+     * caller with nothing to redact needs no branch of its own.
+     *
+     * <p>Best-effort: see the class note. Exactly what is and is not caught is pinned by {@code
+     * FilenameRedactionTest}.
+     */
+    public static String attemptRedaction(String text) {
+        return text == null ? null : FILE_NAME.matcher(text).replaceAll(PLACEHOLDER);
+    }
+}

app/common/src/test/java/stirling/software/common/util/FilenameRedactionTest.java

@@ -0,0 +1,72 @@
+package stirling.software.common.util;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * What the redaction does and does not catch. Best-effort by design, so these pin the boundary
+ * rather than claim completeness: a case that is only partly redacted is asserted as such.
+ */
+class FilenameRedactionTest {
+
+    @ParameterizedTest
+    @DisplayName("names are removed, whatever shape they come in")
+    @ValueSource(
+            strings = {
+                "Failed on report (final).pdf",
+                "Failed on \"Q3 Layoff List.pdf\"",
+                "Failed on /srv/in/Q3 Layoff List.pdf",
+                "Failed on severance-agreement.pdf.gz",
+                "Failed on termination.tar.gz",
+                "Failed on \u5c65\u6b74\u66f8.pdf",
+                "Failed on \u043e\u0442\u0447\u0451\u0442-\u0437\u0430\u0440\u043f\u043b\u0430\u0442\u0430.pdf",
+                "Failed on payslip%20march.pdf",
+                "Failed on invoice_2024.pdf",
+                "Failed on 'end of year (2024).xlsx'",
+                "Failed on C:\\Users\\dana\\Q4 Report.docx",
+                "Failed on Smith & Co - agreement.pdf",
+                "Failed on ../tmp/upload.PDF",
+            })
+    void areRedacted(String text) {
+        assertThat(FilenameRedaction.attemptRedaction(text))
+                .doesNotContainIgnoringCase(".pdf")
+                .doesNotContainIgnoringCase(".xlsx")
+                .doesNotContainIgnoringCase(".docx")
+                .contains(FilenameRedaction.PLACEHOLDER);
+    }
+
+    @ParameterizedTest
+    @DisplayName("text that only looks like a name is left alone")
+    @ValueSource(
+            strings = {
+                "Policy run failed: java.lang.NullPointerException",
+                "version v2.14.2 released",
+                "The PDF Document is passworded",
+            })
+    void areLeftAlone(String text) {
+        assertThat(FilenameRedaction.attemptRedaction(text)).isEqualTo(text);
+    }
+
+    @Test
+    @DisplayName("an undelimited spaced name is only partly removed")
+    void anUndelimitedSpacedNameIsPartlyRedacted() {
+        // The known gap. Crossing spaces without a delimiter would swallow the sentence too, and
+        // "<file>" on its own tells a reader nothing about what failed.
+        //
+        // TODO: harden. This asserts current behaviour, not desired behaviour. Somewhere that can
+        // avoid holding the name at all should do that rather than lean on this.
+        String redacted = FilenameRedaction.attemptRedaction("Failed on Q3 Layoff List.pdf");
+
+        assertThat(redacted).doesNotContain(".pdf").doesNotContain("List");
+        assertThat(redacted).as("a fragment survives, for now").contains("Q3 Layoff");
+    }
+
+    @Test
+    void nullInNullOut() {
+        assertThat(FilenameRedaction.attemptRedaction(null)).isNull();
+    }
+}

app/proprietary/src/main/java/stirling/software/proprietary/failure/PolicyFailureRecorder.java

@@ -48,6 +48,15 @@ public void recordRunFailureAs(
         record(kind, runId, policyId, actor, null, detail);
     }
 
+    /**
+     * The downstream message is kept only for a failure we could not classify, where it is the one
+     * thing telling a reviewer what went wrong. A recognised kind already carries its own copy, so
+     * the raw text adds nothing except somewhere for a document name to hide.
+     */
+    private static String diagnosticFor(FailureKind kind, String detail) {
+        return kind == FailureKind.UNKNOWN ? detail : null;
+    }
+
     private void record(
             FailureKind kind,
             String runId,
@@ -58,7 +67,13 @@ private void record(
         try {
             store.record(
                     RecordFailure.forRun(
-                            kind, teamFor(policyId), actor, policyId, runId, fileIdentity, detail));
+                            kind,
+                            teamFor(policyId),
+                            actor,
+                            policyId,
+                            runId,
+                            fileIdentity,
+                            diagnosticFor(kind, detail)));
         } catch (RuntimeException e) {
             // Deliberately swallowed: see the class comment.
             log.warn("Could not record failure event for run {} (kind {})", runId, kind.getId(), e);

app/proprietary/src/main/java/stirling/software/proprietary/failure/RecordFailure.java

@@ -4,7 +4,8 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.HexFormat;
-import java.util.regex.Pattern;
+
+import stirling.software.common.util.FilenameRedaction;
 
 /**
  * Everything needed to record one failure. Every reference field is nullable, because a failure can
@@ -22,26 +23,6 @@ public record RecordFailure(
         String fileId,
         String detail) {
 
-    /**
-     * Anything shaped like a file name: name characters ending in one to three short extensions.
-     * Matched by shape, so a newly supported format needs no maintenance here.
-     *
-     * <p>Best-effort, and deliberately so in one place: spaces are crossed only when the name is
-     * delimited, because an undelimited one cannot be told from the sentence around it. {@code
-     * RecordFailurePrivacyTest} pins exactly what is and is not caught.
-     *
-     * <p>TODO: store the parsed Problem Details fields rather than the stringified exception, so
-     * this is a backstop instead of the mechanism. We own the producer; we should not be reading
-     * our own structured data back out of prose.
-     */
-    private static final Pattern FILE_PATH_OR_NAME =
-            Pattern.compile(
-                    "(?<![\\w.])(?:(?<=[\"'(\\[/\\\\])[\\p{L}\\p{N}_%+&'()\\[\\]\\-]+"
-                            + "(?:[ ][\\p{L}\\p{N}_%+&'()\\[\\]\\-]+){0,6}"
-                            + "|[\\p{L}\\p{N}_%+&'()\\[\\]\\-]+)"
-                            + "(?:\\.(?![0-9]+(?:\\b|\\.))[\\p{L}\\p{N}]{1,8}){1,3}(?![\\w.])",
-                    Pattern.UNICODE_CHARACTER_CLASS);
-
     /** Upper bound on a stored message, so one enormous stack trace cannot fill the column. */
     private static final int MAX_DETAIL_LENGTH = 2_000;
 
@@ -54,7 +35,7 @@ public record RecordFailure(
         }
         // Sanitised here rather than at each call site, since this record is the only way a row is
         // written. Capped too: an unclassified failure carries a raw message of unbounded length.
-        detail = truncate(withoutFileNames(detail));
+        detail = truncate(FilenameRedaction.attemptRedaction(detail));
     }
 
     /** A processor-side failure with no file or source context, e.g. a run that failed outright. */
@@ -113,14 +94,6 @@ public String dedupKey() {
         }
     }
 
-    /**
-     * Strip file paths and names out of a failure message. The engine's own messages already omit
-     * them; a message forwarded from a downstream tool is outside this package's control.
-     */
-    private static String withoutFileNames(String detail) {
-        return detail == null ? null : FILE_PATH_OR_NAME.matcher(detail).replaceAll("<file>");
-    }
-
     private static String truncate(String detail) {
         if (detail == null || detail.length() <= MAX_DETAIL_LENGTH) {
             return detail;

app/proprietary/src/test/java/stirling/software/proprietary/failure/PolicyFailureRecorderTest.java

@@ -100,8 +100,28 @@ void classifiesTheCauseAndStoresTheRunsOwnMessage() {
             assertThat(event.policyId()).isEqualTo("policy-1");
             assertThat(event.actor()).isEqualTo("dana@example.com");
             assertThat(event.origin()).isEqualTo(FailureOrigin.POLICY);
-            // The run's message, not the exception's: that is what the operator saw.
-            assertThat(event.detail()).isEqualTo("Policy run failed: locked");
+            // A recognised kind carries its own copy, so the downstream text is dropped: it adds
+            // nothing a reviewer needs and is where a document name would otherwise survive.
+            assertThat(event.detail()).isNull();
+        }
+
+        @Test
+        void keepsTheMessageForAFailureItCouldNotClassify() {
+            // The opposite case, and the reason the message is stored at all: with no kind to
+            // describe it, this text is the only thing telling a reviewer what went wrong.
+            when(policyStore.get("policy-1")).thenReturn(Optional.of(policy("policy-1", TEAM)));
+
+            recorder.recordRunFailure(
+                    "run-1",
+                    "policy-1",
+                    "dana@example.com",
+                    null,
+                    "Policy run failed: something we do not recognise",
+                    new RuntimeException("boom"));
+
+            FileRunEvent event = store.list(TEAM, null, null, 10).getFirst();
+            assertThat(event.kind()).isEqualTo(FailureKind.UNKNOWN);
+            assertThat(event.detail()).contains("something we do not recognise");
         }
 
         @Test

app/proprietary/src/test/java/stirling/software/proprietary/failure/RecordFailurePrivacyTest.java

@@ -164,55 +164,18 @@ void neverCutsBetweenTheHalvesOfASurrogatePair() {
     }
 
     @Nested
-    @DisplayName("the shapes a document name actually takes")
-    class RealisticNames {
-
-        /** Raised in review: the first pass only handled a single hyphenated ASCII token. */
-        @ParameterizedTest
-        @ValueSource(
-                strings = {
-                    "Failed on report (final).pdf",
-                    "Failed on \"Q3 Layoff List.pdf\"",
-                    "Failed on /srv/in/Q3 Layoff List.pdf",
-                    "Failed on severance-agreement.pdf.gz",
-                    "Failed on termination.tar.gz",
-                    "Failed on \u5c65\u6b74\u66f8.pdf",
-                    "Failed on \u043e\u0442\u0447\u0451\u0442-\u0437\u0430\u0440\u043f\u043b\u0430\u0442\u0430.pdf",
-                    "Failed on payslip%20march.pdf",
-                    "Failed on invoice_2024.pdf",
-                    "Failed on 'end of year (2024).xlsx'",
-                    "Failed on C:\\Users\\dana\\Q4 Report.docx",
-                    "Failed on Smith & Co - agreement.pdf",
-                    "Failed on ../tmp/upload.PDF",
-                })
-        void areRedacted(String message) {
-            assertThat(withDetail(message).detail()).doesNotContain(".pdf").contains("<file>");
-        }
+    @DisplayName("redaction is applied on the way in")
+    class Redaction {
 
+        /**
+         * Which shapes are caught is {@code FilenameRedactionTest}'s job, in the module that owns
+         * the rule. All this needs to know is that no row is written without it.
+         */
         @Test
-        void anUndelimitedSpacedNameIsOnlyPartlyRedacted() {
-            // The deliberate limit: crossing spaces without a delimiter would swallow the sentence
-            // too, and "<file>" alone tells a reviewer nothing about what failed. So the extension
-            // and the token carrying it go, and the leading words stay.
-            //
-            // TODO: harden. This asserts a known gap rather than desired behaviour — "Q3 Layoff"
-            // is still a fragment of a real document name. When the server stops forwarding a
-            // downstream tool's message verbatim, this should assert full redaction instead.
-            String stored = withDetail("Failed on Q3 Layoff List.pdf").detail();
-
-            assertThat(stored).doesNotContain(".pdf").doesNotContain("List");
-            assertThat(stored).as("a fragment of the name survives, for now").contains("Q3 Layoff");
-        }
+        void everyStoredMessageGoesThroughIt() {
+            String stored = withDetail("Failed on \"Q3 Layoff List.pdf\"").detail();
 
-        @ParameterizedTest
-        @ValueSource(
-                strings = {
-                    "Policy run failed: java.lang.NullPointerException",
-                    "version v2.14.2 released",
-                    "The PDF Document is passworded",
-                })
-        void areLeftAlone(String message) {
-            assertThat(withDetail(message).detail()).isEqualTo(message);
+            assertThat(stored).doesNotContain("Q3 Layoff List.pdf").contains("<file>");
         }
     }
 }