fix(github): take the credential value floors to a single character

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;
@@ -82,15 +83,19 @@ public final class GitHubApiError {
    * across every pattern — split apart, prefixes here and the value shapes beside it, only because
    * one alternation of all four shapes is more than the regex complexity budget allows.
    *
-   * <p>Four value characters, not ten (#746). The sigil is the whole discriminator here — {@code
-   * ghp_} and {@code github_pat_} do not occur in prose, so the length floor was buying nothing the
-   * sigil did not already buy, while costing the one thing that matters: a token the bound before
-   * redaction severs below the floor stops matching and reaches the log with its first nine
-   * characters intact. This is the same reasoning #740 applied to the JWT alternative below and did
-   * not carry across to the shapes that still needed it.
+   * <p>One value character, not ten (#746, #757). The sigil is the whole discriminator here —
+   * {@code ghp_} and {@code github_pat_} do not occur in prose, so the length floor was buying
+   * nothing the sigil did not already buy, while costing the one thing that matters: a token the
+   * bound before redaction severs below the floor stops matching and reaches the log with whatever
+   * the cut left of it intact. #746 made that argument and then stopped at four, which leaves the
+   * same window three characters wide instead of closing it: a cut leaving one, two or three value
+   * characters is still under the floor and still reaches the warn line. One is where the argument
+   * ends, because a cut that leaves no value characters at all strands the sigil by itself, and a
+   * sigil is not secret. This is the same reasoning #740 applied to the JWT alternative below and
+   * did not carry across to the shapes that still needed it.
    */
   private static final Pattern CREDENTIAL_SHAPED_PREFIX =
-      Pattern.compile("(?i)(gh[pousr]_\\w{4,})|(github_pat_\\w{4,})");
+      Pattern.compile("(?i)(gh[pousr]_\\w+)|(github_pat_\\w+)");
 
   /**
    * The bearer shape, and with {@link #JWT_SHAPED_VALUE} below it the value half of {@link
@@ -124,9 +129,9 @@ public final class GitHubApiError {
    * matched, since widening the anchor would mask ordinary prose beginning {@code ey} for a shape
    * no issuer emits.
    *
-   * <p>The value takes the same four-character floor as {@link #CREDENTIAL_SHAPED_PREFIX}, for the
-   * same reason: the {@code Bearer } that precedes it is the discriminator, and ten characters only
-   * meant the bound could sever a header value into something that no longer looked like one.
+   * <p>The value takes the same one-character floor as {@link #CREDENTIAL_SHAPED_PREFIX}, for the
+   * same reason: the {@code Bearer } that precedes it is the discriminator, and any floor above one
+   * only meant the bound could sever a header value into something that no longer looked like one.
    *
    * <p>The word {@code bearer} is consumed with the value rather than left in the line, which does
    * mask the noun in prose such as {@code missing bearer token}. Masking only the value was tried
@@ -135,7 +140,7 @@ public final class GitHubApiError {
    * leftmost-match design above exists to swallow.
    */
   private static final Pattern BEARER_SHAPED_VALUE =
-      Pattern.compile("bearer\\s+[\\w.~+/=-]{4,}", Pattern.CASE_INSENSITIVE);
+      Pattern.compile("bearer\\s+[\\w.~+/=-]+", Pattern.CASE_INSENSITIVE);
 
   /**
    * The JWT shape, kept apart from {@link #BEARER_SHAPED_VALUE} rather than alternated with it: one
@@ -162,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);
 
@@ -428,6 +405,12 @@ private boolean blocksContentCreation() {
   /**
    * One line naming everything that separates one GitHub failure from another: the status, the
    * throttling headers when present, and the body. This is the line that was missing in #568.
+   *
+   * <p>The headers go through {@link LogSafe} in {@link #append} for the reason the body already
+   * does. A header is response data from the configured API host, so on the hosts this class's
+   * threat model names — a GHES or reverse-proxy error page, a misconfigured base URL, a
+   * compromised endpoint — it is as attacker-influenced as the body beside it, and it was reaching
+   * this line verbatim while the body was being collapsed one field away.
    */
   public String diagnostics() {
     var text = new StringBuilder("status=").append(status);
@@ -441,7 +424,7 @@ public String diagnostics() {
 
   private static void append(StringBuilder text, String name, String value) {
     if (value != null && !value.isBlank()) {
-      text.append(' ').append(name).append('=').append(value);
+      text.append(' ').append(name).append('=').append(LogSafe.oneLine(value));
     }
   }
 
@@ -584,7 +567,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;
@@ -719,15 +720,15 @@ private boolean postFindingCommentRoutes(
     if (line.isEmpty()) {
       Log.debugf(
           "Line for %s:%d is outside the PR diff — filing the finding on the file instead",
-          finding.file(), finding.line());
+          LogSafe.oneLine(finding.file()), finding.line());
       return postFileLevelComment(target, finding, findingId);
     }
 
     var resolvedLine = line.getAsInt();
     if (resolvedLine != finding.line()) {
       Log.debugf(
           "Adjusted inline comment line for %s from %d to %d",
-          finding.file(), finding.line(), resolvedLine);
+          LogSafe.oneLine(finding.file()), finding.line(), resolvedLine);
     }
 
     // A GitHub suggestion overwrites the whole commented range, so multi-line old code needs a
@@ -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);
   }
 
@@ -786,9 +787,11 @@ private boolean postFindingCommentRoutes(
    */
   private static String rejectionReason(RuntimeException e) {
     if (e instanceof WebApplicationException w) {
-      return GitHubApiError.of(w).map(GitHubApiError::diagnostics).orElseGet(e::toString);
+      return GitHubApiError.of(w)
+          .map(GitHubApiError::diagnostics)
+          .orElseGet(() -> LogSafe.oneLine(e.toString()));
     }
-    return e.toString();
+    return LogSafe.oneLine(e.toString());
   }
 
   /**
@@ -829,7 +832,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;
     }
   }
@@ -876,7 +879,7 @@ private Optional<String> tryPostInlineComment(
       Log.debugf(
           e,
           "Inline comment rejected for %s:%d (suggestion=%s): %s",
-          finding.file(),
+          LogSafe.oneLine(finding.file()),
           endLine,
           includeSuggestion,
           reason);