Found by deep-audit round 6 (AUDIT6-A finding A4, second half). It was scoped out of #740 because that PR owned GitHubApiError while these two call sites live in ReviewPublisher, and it was never filed on its own — this issue closes that gap.
The error-body path now collapses whitespace, C0/C1 controls and Cf format controls before anything reaches a log line (#740). The finding path does not: finding.file() is model-supplied text interpolated straight into two warn-level lines in ReviewPublisher (around :752 and :814), the second of which #723 promoted from debug to warn — so it is written to production logs today.
A path carrying a line terminator splits the record and forges a second one; a path carrying bidi or other format controls reorders what an operator reads. That is the same log-forgery family #740 closed on the sibling surface, and it undercuts the diagnostic value #723 was added to provide: the whole point of those lines is that an operator can trust what a rejection says.
Fix direction: route both interpolations through the same one-line collapsing the error body uses (MarkdownSafe.oneLine or the shared collapse), rather than trusting the model's path string. Worth a sweep for other model-supplied values interpolated into log lines while in there.
Found by deep-audit round 6 (AUDIT6-A finding A4, second half). It was scoped out of #740 because that PR owned
GitHubApiErrorwhile these two call sites live inReviewPublisher, and it was never filed on its own — this issue closes that gap.The error-body path now collapses whitespace, C0/C1 controls and Cf format controls before anything reaches a log line (#740). The finding path does not:
finding.file()is model-supplied text interpolated straight into two warn-level lines inReviewPublisher(around:752and:814), the second of which #723 promoted from debug to warn — so it is written to production logs today.A path carrying a line terminator splits the record and forges a second one; a path carrying bidi or other format controls reorders what an operator reads. That is the same log-forgery family #740 closed on the sibling surface, and it undercuts the diagnostic value #723 was added to provide: the whole point of those lines is that an operator can trust what a rejection says.
Fix direction: route both interpolations through the same one-line collapsing the error body uses (
MarkdownSafe.oneLineor the shared collapse), rather than trusting the model's path string. Worth a sweep for other model-supplied values interpolated into log lines while in there.