fix(review): collapse every model-supplied value a log line interpolates

src/main/java/dev/thiagogonzaga/thrillhousebot/LogSafe.java

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2026 Thiago Gonzaga
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package dev.thiagogonzaga.thrillhousebot;
+
+import java.util.regex.Pattern;
+
+/**
+ * The single place untrusted text is flattened before it is interpolated into a log line. Every
+ * value a log statement splices in that the bot did not choose itself — a GitHub error body, a
+ * model-supplied finding title or path — goes through here, so the "a field forges a record" class
+ * of defect is fixed in one place rather than re-litigated at each new call site (#731, #740,
+ * #742).
+ *
+ * <p>This is the log-destined counterpart to {@code MarkdownSafe}, and the two are deliberately
+ * separate. {@code MarkdownSafe.oneLine} feeds text the bot posts to GitHub, where the wider class
+ * below has a real rendering cost; a log record is a diagnostic identity rather than a rendering
+ * surface, and pays that cost gladly. Widening the markdown collapser instead would change what the
+ * bot posts.
+ */
+public final class LogSafe {
+
+  /**
+   * Collapses the whitespace of an untrusted string so one value stays on one log line.
+   *
+   * <p>Wider than {@code \s}, which java.util.regex reads as the ASCII six ({@code [
+   * \t\n\x0B\f\r]}) unless the pattern asks for Unicode character classes. CR and LF being
+   * collapsed closes the classic forged-record vector, but NEL (U+0085), LINE SEPARATOR (U+2028),
+   * PARAGRAPH SEPARATOR (U+2029), NUL and the ANSI escape all survived it (#731) — and a log
+   * viewer, a terminal, or a JSON/ECS shipper may treat any of them as a record boundary or as a
+   * screen-control sequence. A caller that reaches for this class has already decided its value is
+   * attacker-influenced text on its way to a log file and is already paying for a collapse pass on
+   * that basis; this is that pass covering what it claims to.
+   *
+   * <p>{@code \p{IsCc}} is the Unicode general category rather than POSIX {@code \p{Cntrl}}, so it
+   * reaches the C1 controls (U+0080–U+009F, NEL among them) as well as C0 and DEL.
+   *
+   * <p>{@code \p{IsZs}} covers the space separators {@code \s} leaves behind — NBSP (U+00A0), the
+   * EM/EN and figure spaces (U+2000–U+200A), the narrow NBSP (U+202F), the medium mathematical
+   * space (U+205F) and the ideographic space (U+3000). Without it the class contradicted the
+   * contract this method's javadoc states, and inconsistently: {@code String.strip()} reads {@code
+   * Character.isWhitespace}, which is true for U+2003 and U+3000 but false for U+00A0, U+2007 and
+   * U+202F, so the first pair were trimmed at the ends yet never collapsed inside, and the second
+   * three survived at every position. They forge no boundary, which is why this is the cheapest of
+   * the four classes to justify; they are here because two values differing only by one of them
+   * render identically in a log, which is the same harm as the invisible characters below.
+   *
+   * <p>{@code \p{IsCf}} is here for the same harm rather than for line integrity: bidi overrides
+   * and isolates (RLO, LRM, LRI) reorder what an operator reads, and the invisible joiners and
+   * spaces (ZWJ, ZWNJ, ZWSP, the BOM, the soft hyphen) let two different values render identically
+   * — both forge a record's meaning as surely as a forged boundary forges its extent. The accepted
+   * cost is that an echoed user string loses its grapheme clusters: an emoji ZWJ sequence or an
+   * Indic conjunct is split apart. A logged value is a diagnostic identity rather than a rendering
+   * surface, and which characters arrived is the question it exists to answer. Replacing with a
+   * space rather than deleting is part of the same bargain — deletion would let {@code
+   * admin<ZWSP>istrator} close up into a different real word, a space cannot.
+   */
+  private static final Pattern WHITESPACE =
+      Pattern.compile("[\\s\\p{IsZs}\\p{IsCc}\\p{IsCf}\\u2028\\u2029]+");
+
+  private LogSafe() {}
+
+  /**
+   * An untrusted string flattened to one log-safe line: every run of whitespace, control and format
+   * characters becomes a single space, and the ends are trimmed so a value that began or ended with
+   * such a run does not leave a stray space in the line. A {@code null} value flattens to the empty
+   * string, so a caller never has to guard for one.
+   */
+  public static String oneLine(String value) {
+    if (value == null) {
+      return "";
+    }
+    return WHITESPACE.matcher(value).replaceAll(" ").strip();
+  }
+}

src/main/java/dev/thiagogonzaga/thrillhousebot/github/GitHubApiError.java

@@ -15,6 +15,7 @@
  */
 package dev.thiagogonzaga.thrillhousebot.github;
 
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import jakarta.ws.rs.WebApplicationException;
 import jakarta.ws.rs.core.Response;
 import java.time.DateTimeException;
@@ -166,34 +167,6 @@ public final class GitHubApiError {
           "(?i)secondary rate limit|abuse detection|rate limit exceeded"
               + "|blocked from (?:content creation|creating content)");
 
-  /**
-   * Collapses the whitespace of a body so one failure stays on one log line.
-   *
-   * <p>Wider than {@code \s}, which java.util.regex reads as the ASCII six ({@code [
-   * \t\n\x0B\f\r]}) unless the pattern asks for Unicode character classes. CR and LF being
-   * collapsed closes the classic forged-record vector, but NEL (U+0085), LINE SEPARATOR (U+2028),
-   * PARAGRAPH SEPARATOR (U+2029), NUL and the ANSI escape all survived it (#731) — and a log
-   * viewer, a terminal, or a JSON/ECS shipper may treat any of them as a record boundary or as a
-   * screen-control sequence. This class documents a body as attacker-influenced text on its way to
-   * a log file and already pays for a collapse pass on that basis; this is that pass covering what
-   * it claims to.
-   *
-   * <p>{@code \p{IsCc}} is the Unicode general category rather than POSIX {@code \p{Cntrl}}, so it
-   * reaches the C1 controls (U+0080–U+009F, NEL among them) as well as C0 and DEL.
-   *
-   * <p>{@code \p{IsCf}} is here for the same harm rather than for line integrity: bidi overrides
-   * and isolates (RLO, LRM, LRI) reorder what an operator reads, and the invisible joiners and
-   * spaces (ZWJ, ZWNJ, ZWSP, the BOM, the soft hyphen) let two different bodies render identically
-   * — both forge a record's meaning as surely as a forged boundary forges its extent. The accepted
-   * cost is that an echoed user string loses its grapheme clusters: an emoji ZWJ sequence or an
-   * Indic conjunct is split apart. A {@code body=} field is a diagnostic identity rather than a
-   * rendering surface, and which characters arrived is the question it exists to answer. Replacing
-   * with a space rather than deleting is part of the same bargain — deletion would let {@code
-   * admin<ZWSP>istrator} close up into a different real word, a space cannot.
-   */
-  private static final Pattern WHITESPACE =
-      Pattern.compile("[\\s\\p{IsCc}\\p{IsCf}\\u2028\\u2029]+");
-
   /** Backoff used when GitHub throttles without saying for how long. */
   static final Duration FALLBACK_DELAY = Duration.ofSeconds(5);
 
@@ -588,7 +561,7 @@ private static Body clean(String raw) {
     if (raw == null) {
       return Body.UNREADABLE;
     }
-    var collapsed = WHITESPACE.matcher(raw).replaceAll(" ").strip();
+    var collapsed = LogSafe.oneLine(raw);
     var bounded = cutTo(collapsed, MAX_BODY_CHARS * 2);
     var redacted = redactCredentials(bounded);
     var capped = cutTo(redacted, MAX_BODY_CHARS);

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingDeduplicator.java

@@ -15,6 +15,7 @@
  */
 package dev.thiagogonzaga.thrillhousebot.review;
 
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.review.ai.FindingVerificationService;
 import dev.thiagogonzaga.thrillhousebot.review.ai.ReviewResponse;
 import io.quarkus.logging.Log;
@@ -54,7 +55,10 @@ public ReviewResponse dedupe(ReviewResponse response) {
       if (cluster.size() > 1) {
         Log.infof(
             "Merging %d duplicate findings at %s:%d ('%s')",
-            cluster.size(), cluster.get(0).file(), cluster.get(0).line(), cluster.get(0).title());
+            cluster.size(),
+            LogSafe.oneLine(cluster.get(0).file()),
+            cluster.get(0).line(),
+            LogSafe.oneLine(cluster.get(0).title()));
       }
       merged.add(merge(cluster));
     }

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingPipeline.java

@@ -17,6 +17,7 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.config.BotIdentity;
 import dev.thiagogonzaga.thrillhousebot.dashboard.ReviewSession;
 import dev.thiagogonzaga.thrillhousebot.github.GitHubPullRequestClient;
@@ -1369,7 +1370,7 @@ ReviewResponse populateMissingAnchors(ReviewResponse response, DiffLineResolver
         if (fallback != null && !fallback.isBlank()) {
           Log.infof(
               "Populating missing content anchor for finding '%s' (%s:%d)",
-              finding.title(), finding.file(), finding.line());
+              LogSafe.oneLine(finding.title()), LogSafe.oneLine(finding.file()), finding.line());
           adjusted.add(
               new ReviewResponse.Finding(
                   finding.risk(),

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingQuoteValidator.java

@@ -15,6 +15,7 @@
  */
 package dev.thiagogonzaga.thrillhousebot.review;
 
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.review.ai.FindingVerificationService;
 import dev.thiagogonzaga.thrillhousebot.review.ai.ReviewResponse;
 import io.quarkus.logging.Log;
@@ -82,8 +83,8 @@ public ReviewResponse validate(ReviewResponse response, String diff) {
       }
       Log.infof(
           "Finding '%s' (%s:%d) %s" + DEMOTION_SUFFIX,
-          finding.title(),
-          finding.file(),
+          LogSafe.oneLine(finding.title()),
+          LogSafe.oneLine(finding.file()),
           finding.line(),
           reason);
       kept.add(withoutSuggestion(finding));

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FollowUpAnalyzer.java

@@ -17,6 +17,7 @@
 
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.config.BotIdentity;
 import dev.thiagogonzaga.thrillhousebot.config.ThrillhouseConfig;
 import dev.thiagogonzaga.thrillhousebot.github.GitHubCommentClient;
@@ -633,7 +634,10 @@ public ReviewResponse dropRepliedDuplicates(
       Log.infof(
           "Dropping re-raised finding '%s' (%s:%d) — a maintainer already replied to the prior"
               + " finding '%s' at the same location",
-          finding.title(), finding.file(), finding.line(), duplicateOf.title());
+          LogSafe.oneLine(finding.title()),
+          LogSafe.oneLine(finding.file()),
+          finding.line(),
+          LogSafe.oneLine(duplicateOf.title()));
     }
     if (!dropped) {
       return response;

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FrameworkFalsePositiveFilter.java

@@ -15,6 +15,7 @@
  */
 package dev.thiagogonzaga.thrillhousebot.review;
 
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.review.ai.FindingVerificationService;
 import dev.thiagogonzaga.thrillhousebot.review.ai.ReviewResponse;
 import io.quarkus.logging.Log;
@@ -75,7 +76,7 @@ public ReviewResponse filter(ReviewResponse response, String diff) {
             "Dropping finding '%s' (%s:%d) — it claims a missing no-arg constructor but the diff"
                 + " shows an injection-annotated constructor; constructor injection needs no no-arg"
                 + " constructor in CDI/Spring",
-            finding.title(), finding.file(), finding.line());
+            LogSafe.oneLine(finding.title()), LogSafe.oneLine(finding.file()), finding.line());
         changed = true;
         continue;
       }

src/main/java/dev/thiagogonzaga/thrillhousebot/review/ReviewPublisher.java

@@ -15,6 +15,7 @@
  */
 package dev.thiagogonzaga.thrillhousebot.review;
 
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.config.BotIdentity;
 import dev.thiagogonzaga.thrillhousebot.config.ThrillhouseConfig;
 import dev.thiagogonzaga.thrillhousebot.github.GitHubApiError;
@@ -767,7 +768,7 @@ private boolean postFindingCommentRoutes(
     }
     Log.warnf(
         "GitHub rejected inline comment for %s:%d (%s) — filing it on the file instead",
-        MarkdownSafe.oneLine(finding.file()), finding.line(), reason);
+        LogSafe.oneLine(finding.file()), finding.line(), reason);
     return postFileLevelComment(target, finding, findingId);
   }
 
@@ -829,7 +830,7 @@ private boolean postFileLevelComment(CommentTarget target, Finding finding, int
     } catch (RuntimeException e) {
       Log.warnf(
           "GitHub rejected the file-level thread for %s (%s) — the finding keeps no thread at all",
-          MarkdownSafe.oneLine(finding.file()), rejectionReason(e));
+          LogSafe.oneLine(finding.file()), rejectionReason(e));
       return false;
     }
   }

src/main/java/dev/thiagogonzaga/thrillhousebot/review/ai/FindingVerificationService.java

@@ -18,6 +18,7 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import dev.langchain4j.service.Result;
+import dev.thiagogonzaga.thrillhousebot.LogSafe;
 import dev.thiagogonzaga.thrillhousebot.config.ThrillhouseConfig;
 import dev.thiagogonzaga.thrillhousebot.review.Confidence;
 import dev.thiagogonzaga.thrillhousebot.review.PromptTemplateEscaper;
@@ -1228,7 +1229,10 @@ ReviewResponse apply(ReviewResponse response, VerificationResponse verification)
           rejected++;
           Log.infof(
               "Verifier rejected finding '%s' (%s:%d): %s",
-              finding.title(), finding.file(), finding.line(), verdict.reason());
+              LogSafe.oneLine(finding.title()),
+              LogSafe.oneLine(finding.file()),
+              finding.line(),
+              LogSafe.oneLine(verdict.reason()));
         }
         case "downgraded" -> {
           downgraded++;