diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 5aa9023f56..de80e4ca90 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -476,9 +476,14 @@ export function buildCheckRunAnnotations( level: CheckRunAnnotation["annotation_level"], title: string, message: string, + // `alreadyPublicSafe` (#7981, extended to annotations by #8321): same contract formatCheckRunOutput's + // `text` already honors -- a fixed, engineer-authored message with no interpolated contributor/AI content + // must not be scrubbed, or a deliberately-worded string gets mangled. The title keeps its sanitizer: + // there is no per-title safety flag on a finding, so nothing licenses skipping it. + alreadyPublicSafe = false, ) => { const safeTitle = sanitizeForCheckRun(title).slice(0, 255); - const safeMessage = sanitizeForCheckRun(message).slice(0, 65535); + const safeMessage = (alreadyPublicSafe ? message : sanitizeForCheckRun(message)).slice(0, 65535); if (!path || !safeTitle || !safeMessage) return; const key = `${path}:${safeTitle}:${safeMessage}`; if (seen.has(key)) return; @@ -532,6 +537,7 @@ export function buildCheckRunAnnotations( severityToAnnotationLevel(finding.severity), finding.title, finding.publicText, + finding.alreadyPublicSafe ?? false, ); } } diff --git a/test/unit/rules.test.ts b/test/unit/rules.test.ts index 03a4042c1c..534c972eda 100644 --- a/test/unit/rules.test.ts +++ b/test/unit/rules.test.ts @@ -936,6 +936,54 @@ describe("advisory rules", () => { expect(out.text).not.toContain("[context]"); }); + // #8321: the SAME finding is also rendered as an inline annotation, and buildCheckRunAnnotations used to + // scrub it unconditionally -- so one PR showed the message verbatim in the check-run text but mangled in the + // annotation. These two pin both arms of the flag on the annotation path. + describe("buildCheckRunAnnotations honors alreadyPublicSafe (#8321)", () => { + const SAFE_TEXT = "Subnet document appears to include secret, wallet, PAT, or private-key material."; + const annotationsFor = (alreadyPublicSafe: boolean) => { + const advisory = buildPullRequestAdvisory(repo, { + repoFullName: repo.fullName, number: 31, title: "Add lane doc", state: "open", + authorLogin: "contributor", authorAssociation: "NONE", labels: [], linkedIssues: [], + }); + const files: PullRequestFileRecord[] = [ + { repoFullName: repo.fullName, pullNumber: 31, path: "src/lane/doc.ts", additions: 5, deletions: 0, changes: 5, payload: {} }, + ]; + const collisions: CollisionReport = { + repoFullName: repo.fullName, generatedAt: "2026-06-10T00:00:00.000Z", + summary: { clusterCount: 0, highRiskCount: 0, itemsReviewed: 0 }, clusters: [], + }; + const withFinding = { + ...advisory, + findings: [ + { + code: "surface_lane_reject", + title: "Registry surface review", + severity: "critical" as const, + detail: SAFE_TEXT, + publicText: SAFE_TEXT, + ...(alreadyPublicSafe ? { alreadyPublicSafe: true } : {}), + }, + ], + }; + return buildCheckRunAnnotations(withFinding, { files, collisions, pullNumber: 31 }, "standard").annotations; + }; + + it("renders an alreadyPublicSafe message verbatim, matching formatCheckRunOutput", () => { + const entry = annotationsFor(true).find((a) => a.title === "Registry surface review"); + expect(entry, "expected an annotation for the finding").toBeTruthy(); + expect(entry!.message).toBe(SAFE_TEXT); + expect(entry!.message).not.toContain("[context]"); + }); + + it("still sanitizes the message when alreadyPublicSafe is absent (unchanged behavior)", () => { + const entry = annotationsFor(false).find((a) => a.title === "Registry surface review"); + expect(entry, "expected an annotation for the finding").toBeTruthy(); + expect(entry!.message).toContain("[context]"); + expect(entry!.message).not.toBe(SAFE_TEXT); + }); + }); + it("formatCheckRunOutput publishes only explicit public finding text", () => { const advisory = buildPullRequestAdvisory(repo, null); const output = formatCheckRunOutput(