diff --git a/.release-please-manifest.json b/.release-please-manifest.json index de6c1735bb..4048f3a89d 100644 --- a/.release-please-manifest.json +++ b/.release-please-manifest.json @@ -1,6 +1,6 @@ { "packages/loopover-mcp": "3.4.0", - "packages/loopover-engine": "3.4.0", + "packages/loopover-engine": "3.4.1", "packages/loopover-miner": "3.4.0", "packages/loopover-ui-kit": "1.1.0" } diff --git a/package-lock.json b/package-lock.json index 24413e15f9..3c81680d20 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21868,7 +21868,7 @@ }, "packages/loopover-engine": { "name": "@loopover/engine", - "version": "3.4.0", + "version": "3.4.1", "license": "AGPL-3.0-only", "dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.3.205", diff --git a/packages/loopover-engine/package.json b/packages/loopover-engine/package.json index 1354230603..0f33617c82 100644 --- a/packages/loopover-engine/package.json +++ b/packages/loopover-engine/package.json @@ -1,6 +1,6 @@ { "name": "@loopover/engine", - "version": "3.4.0", + "version": "3.4.1", "license": "AGPL-3.0-only", "type": "module", "description": "Shared deterministic engine logic for the LoopOver review stack and loopover-miner.", diff --git a/packages/loopover-miner/expected-engine.version b/packages/loopover-miner/expected-engine.version index 18091983f5..47b322c971 100644 --- a/packages/loopover-miner/expected-engine.version +++ b/packages/loopover-miner/expected-engine.version @@ -1 +1 @@ -3.4.0 +3.4.1 diff --git a/scripts/check-engine-parity.ts b/scripts/check-engine-parity.ts index 6f23e6d560..37af8db0ad 100644 --- a/scripts/check-engine-parity.ts +++ b/scripts/check-engine-parity.ts @@ -616,6 +616,7 @@ export function runEngineParityChecks(options: { changedFiles?: readonly string[]; baseEngineVersion?: string | null; headEngineVersion?: string | null; + execGit?: EngineParityExecGit; }): { failures: string[]; pairsChecked: EngineParityPair[]; @@ -641,8 +642,24 @@ export function runEngineParityChecks(options: { headEngineVersion = null; } } - const baseEngineVersion = options.baseEngineVersion ?? headEngineVersion; - const changedFiles = options.changedFiles ?? listChangedEngineParityFiles({ root: options.root }); + const changedFiles = + options.changedFiles ?? + listChangedEngineParityFiles({ root: options.root, ...(options.execGit ? { execGit: options.execGit } : {}) }); + // Mirror listChangedEngineParityFiles' real git-diff default above: without this, an un-overridden + // baseEngineVersion silently aliased to headEngineVersion, so checkGateDecisionVersionBump could never + // observe a real version bump on any branch that diverges from origin/main (#7981 side-discovery). + let baseEngineVersion = options.baseEngineVersion; + if (baseEngineVersion === undefined) { + baseEngineVersion = + changedFiles.length > 0 + ? (readEnginePackageVersionAtRef({ + root: options.root, + ref: process.env.LOOPOVER_ENGINE_PARITY_BASE_REF ?? process.env.GITHUB_BASE_SHA ?? "origin/main", + readFile, + ...(options.execGit ? { execGit: options.execGit } : {}), + }) ?? headEngineVersion) + : headEngineVersion; + } const versionBump = changedFiles.length > 0 && headEngineVersion ? checkGateDecisionVersionBump({ @@ -666,21 +683,7 @@ export function runEngineParityChecks(options: { /** @internal Exported for subprocess-free unit tests of the CLI success/failure paths. */ export function runEngineParityMain(root: string = process.cwd()): number { - const changedFiles = listChangedEngineParityFiles({ root }); - const headEngineVersion = readEnginePackageVersionAtRef({ root, ref: "HEAD" }); - const baseEngineVersion = - changedFiles.length > 0 - ? readEnginePackageVersionAtRef({ - root, - ref: process.env.LOOPOVER_ENGINE_PARITY_BASE_REF ?? process.env.GITHUB_BASE_SHA ?? "origin/main", - }) ?? headEngineVersion - : headEngineVersion; - const { failures, pairsChecked, versionSkew } = runEngineParityChecks({ - root, - changedFiles, - baseEngineVersion, - headEngineVersion, - }); + const { failures, pairsChecked, versionSkew } = runEngineParityChecks({ root }); if (failures.length > 0) { console.error(`Engine-parity check found ${failures.length} issue(s):`); diff --git a/src/review/content-lane-wire.ts b/src/review/content-lane-wire.ts index 0bdd8f10c1..6964d8cdd2 100644 --- a/src/review/content-lane-wire.ts +++ b/src/review/content-lane-wire.ts @@ -54,8 +54,13 @@ const SURFACE_MANUAL_CODE = "surface_lane_manual"; const SURFACE_UNKNOWN_VALIDATOR_CODE = "surface_lane_unknown_validator_id"; const SURFACE_TITLE = "Registry surface review"; -function surfaceFinding(code: string, severity: AdvisorySeverity, summary: string): AdvisoryFinding { - return { code, title: SURFACE_TITLE, severity, detail: summary, publicText: summary }; +// `alreadyPublicSafe` defaults to false: `unregisteredValidatorIdFinding` below also calls this helper with a +// summary that interpolates an OPERATOR-supplied `.loopover.yml` validatorId string, which -- unlike the fixed, +// engineer-authored assessment messages `runSurfaceReview` produces (registry-logic.ts) -- is external content +// this function has no way to vouch for. Only `surfaceVerdictToGate` below, whose `summary` comes solely from +// that fixed assessment vocabulary, opts in explicitly. +function surfaceFinding(code: string, severity: AdvisorySeverity, summary: string, alreadyPublicSafe = false): AdvisoryFinding { + return { code, title: SURFACE_TITLE, severity, detail: summary, publicText: summary, alreadyPublicSafe }; } /** A diagnostic (non-blocking) finding for a `.loopover.yml` `contentLane.validatorId` that doesn't match any @@ -82,10 +87,10 @@ export function surfaceVerdictToGate(result: SurfaceReviewResult): { return { evaluation: { enabled: true, conclusion: "success", title: SURFACE_TITLE, summary, blockers: [], warnings: [] }, finding: null }; } if (result.verdict === "manual") { - const finding = surfaceFinding(SURFACE_MANUAL_CODE, "warning", summary); + const finding = surfaceFinding(SURFACE_MANUAL_CODE, "warning", summary, true); return { evaluation: { enabled: true, conclusion: "neutral", title: SURFACE_TITLE, summary, blockers: [], warnings: [finding] }, finding }; } - const finding = surfaceFinding(SURFACE_REJECT_CODE, "critical", summary); + const finding = surfaceFinding(SURFACE_REJECT_CODE, "critical", summary, true); return { evaluation: { enabled: true, conclusion: "failure", title: SURFACE_TITLE, summary, blockers: [finding], warnings: [] }, finding }; } diff --git a/src/review/content-lane/registry-logic.ts b/src/review/content-lane/registry-logic.ts index f5c627ef49..9d2d3eef66 100644 --- a/src/review/content-lane/registry-logic.ts +++ b/src/review/content-lane/registry-logic.ts @@ -97,11 +97,22 @@ export interface Assessment { reason?: string; } -/** Exact port of containsSecretLikeText — runs on JSON.stringify(candidate). */ +// #7981: `hotkey`/`coldkey` used to be bare, unqualified alternatives (unlike `wallet`, already scoped to the +// phrase "wallet path") -- but a Bittensor/Gittensor "hotkey" is the standard PUBLIC miner identifier (an SS58 +// address), not secret material, and appears routinely in ordinary registry content: API paths +// (`/miners/hotkey/{hotkey}`), field names (`miner_hotkey`), even a note explicitly DENYING any such data ("No +// wallet/hotkey data" -- the literal trigger for metagraphed #7589/#7591). Because `assessSubnetDocument` scans +// the WHOLE document, not just the diff, one such mention anywhere in a file permanently blocks every future PR +// touching it. Scope hot/coldkey the same way `wallet path` already is: require adjacency to something that +// actually indicates key material (a keystore file path, a private-key/password/mnemonic/seed qualifier) +// rather than a bare word match. +const SECRET_LIKE_TEXT_PATTERN = + /\bgh[pousr]_[A-Za-z0-9_]{20,}\b|\bgithub_pat_[A-Za-z0-9_]{20,}\b|BEGIN [A-Z ]*PRIVATE KEY|seed phrase|mnemonic|wallet path|(?:hot|cold)key[ _-]?(?:path|private[ _-]?key|password|mnemonic|seed)|private[ _-]?(?:hot|cold)key/i; + +/** Runs on JSON.stringify(candidate). See SECRET_LIKE_TEXT_PATTERN's own comment for the hotkey/coldkey scoping + * fix (#7981) -- every other alternative is unchanged from the original port. */ export function containsSecretLikeText(value: string): boolean { - return /\bgh[pousr]_[A-Za-z0-9_]{20,}\b|\bgithub_pat_[A-Za-z0-9_]{20,}\b|BEGIN [A-Z ]*PRIVATE KEY|seed phrase|mnemonic|wallet path|hotkey|coldkey/i.test( - String(value || ""), - ); + return SECRET_LIKE_TEXT_PATTERN.test(String(value || "")); } const TRACKING_PARAMS = new Set([ diff --git a/src/review/unified-comment-bridge.ts b/src/review/unified-comment-bridge.ts index 9fbcbc6eda..cd6fa06abc 100644 --- a/src/review/unified-comment-bridge.ts +++ b/src/review/unified-comment-bridge.ts @@ -257,12 +257,17 @@ export function buildDualReviewNotes(args: { : []; // FIX D1: fold the gate's own hard blockers into the reviewer blockers (so a non-AI gate failure populates // "Why this is blocked"). Exclude `ai_consensus_defect` (already surfaced via consensusDefect → appears once) - // and scrub each through the same public-safe boundary as Nits, DROPPING any that still leaks a private term. + // and scrub each through the same public-safe boundary as Nits, DROPPING any that still leaks a private term + // -- UNLESS the finding declares itself `alreadyPublicSafe` (a fixed, engineer-authored message with no + // interpolated contributor/AI content, e.g. the content lane's deterministic surface-review findings): the + // scrub exists to catch a private rubric term LEAKING into dynamically assembled text, not to mangle a static + // string an engineer already reviewed into the uninformative "[context]" placeholder (#7981). const gateBlockerLines = (args.gateBlockers ?? []) .filter((finding) => finding.code !== "ai_consensus_defect") - .map(gateBlockerLine) - .filter(Boolean) - .map((line) => publicSafeNit(line)) + .map((finding) => { + const line = gateBlockerLine(finding); + return line && finding.alreadyPublicSafe ? line : publicSafeNit(line); + }) .filter((line): line is string => line !== null); const blockers = [...consensusBlocker, ...gateBlockerLines]; // Nits are the only renderer input not already routed through an existing public-safe filter (the gate's diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 7026379da9..e608bd300f 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -511,7 +511,10 @@ export function formatCheckRunOutput( const publicLines = advisoryResult.findings.flatMap((f) => { if (!f.publicText) return []; const label = f.severity === "warning" ? "⚠️" : "ℹ️"; - return [`${label} ${sanitizeForCheckRun(f.publicText)}`]; + // `alreadyPublicSafe` (#7981): a fixed, engineer-authored message with no interpolated contributor/AI + // content skips the scrub — see AdvisoryFinding's own doc comment for why (the same reasoning + // unified-comment-bridge.ts's gateBlockerLines applies to the PR-comment rendering of this same finding). + return [`${label} ${f.alreadyPublicSafe ? f.publicText : sanitizeForCheckRun(f.publicText)}`]; }); text = publicLines.length === 0 ? "No detailed findings are published in check runs." : publicLines.join("\n"); } diff --git a/src/types.ts b/src/types.ts index bd8e3cc5fb..7edd9db6a9 100644 --- a/src/types.ts +++ b/src/types.ts @@ -529,6 +529,17 @@ export type AdvisoryFinding = { detail: string; action?: string; publicText?: string; + /** True when `detail`/`publicText` is a fixed, engineer-authored message with no interpolated contributor + * content or AI-generated text (e.g. the content lane's deterministic surface-review findings — see + * `surfaceFinding` in content-lane-wire.ts). `PRIVATE_FORBIDDEN_TERMS`/`CHECK_RUN_FORBIDDEN_TERMS` exist to + * catch a private scoring-rubric term (reward/trust-score/wallet/hotkey/...) LEAKING into dynamically + * assembled or AI-generated text — applying that same scrub to a static string an engineer already wrote + * and reviewed only replaces a real word with the confusing, uninformative "[context]" placeholder (#7981: + * "Subnet document appears to include secret, wallet, PAT, or private-key material" rendered as "...secret, + * [context], PAT..." to both contributors and the maintainer debugging the close). Absent/false preserves + * today's scrub-everything behavior; only a producer that has audited its own text for private-rubric leaks + * should set this to true. */ + alreadyPublicSafe?: boolean; /** Calibrated confidence in [0,1] for an AI-judgment finding (`ai_consensus_defect` / `ai_review_split`) — the * reviewer's own probability that the flagged blocker is a real defect (#8). The gate's `aiReviewCloseConfidence` * floor and `aiReviewLowConfidenceDisposition` (#4603) use it: a sub-floor finding still blocks the gate when diff --git a/test/unit/check-engine-parity-script.test.ts b/test/unit/check-engine-parity-script.test.ts index f060397770..9a81f3e6f3 100644 --- a/test/unit/check-engine-parity-script.test.ts +++ b/test/unit/check-engine-parity-script.test.ts @@ -249,6 +249,102 @@ describe("check-engine-parity script", () => { }, }).failures).toEqual([]); }); + + // Regression guard for a real gap found while landing #7981: runEngineParityChecks previously aliased an + // un-overridden baseEngineVersion straight to headEngineVersion, so a genuine version bump could never be + // detected unless the caller passed baseEngineVersion explicitly (only runEngineParityMain did). That made + // a single-sided gate-decision edit + version bump fail this check on any branch that had actually diverged + // from origin/main, even though the bump was correct. + it("resolves an un-overridden baseEngineVersion via git, mirroring the changedFiles default (#7981 side-discovery)", () => { + const gateBody = [ + "export function evaluateGateCheck() {}", + "function evaluateGateCheckCore() {}", + "function isConfiguredGateBlocker() {}", + "export function buildPullRequestAdvisory() {}", + ].join("\n"); + const readFile = (_root: string, relativePath: string) => { + if (relativePath === "packages/loopover-engine/package.json") return JSON.stringify({ version: "0.3.0" }); + if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) return gateBody; + if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) return gateBody; + if (relativePath === "packages/loopover-miner/expected-engine.version") return "0.3.0\n"; + throw new Error(`unexpected read: ${relativePath}`); + }; + const execGit = vi.fn((args: string[]) => { + if (args[0] === "show" && args[1] === "origin/main:packages/loopover-engine/package.json") { + return JSON.stringify({ version: "0.2.0" }); + } + throw new Error(`unexpected git invocation: ${args.join(" ")}`); + }); + + const result = runEngineParityChecks({ + root: "/fake", + readFile, + listDir: () => [], + resolveInstalled: () => "0.3.0", + changedFiles: [GATE_DECISION_TWIN_PAIR.hostRelative, "packages/loopover-engine/package.json"], + headEngineVersion: "0.3.0", + execGit, + // baseEngineVersion intentionally omitted -- this is what previously aliased to headEngineVersion. + }); + + expect(result.failures.some((failure) => failure.includes("Gate-decision logic change"))).toBe(false); + expect(execGit).toHaveBeenCalledWith(["show", "origin/main:packages/loopover-engine/package.json"], "/fake"); + }); + + it("still fails a single-sided edit when git-resolved baseEngineVersion is unavailable (falls back to headEngineVersion, so no increase is observed)", () => { + const gateBody = [ + "export function evaluateGateCheck() {}", + "function evaluateGateCheckCore() {}", + "function isConfiguredGateBlocker() {}", + "export function buildPullRequestAdvisory() {}", + ].join("\n"); + const readFile = (_root: string, relativePath: string) => { + if (relativePath === "packages/loopover-engine/package.json") return JSON.stringify({ version: "0.3.0" }); + if (relativePath === GATE_DECISION_TWIN_PAIR.hostRelative) return gateBody; + if (relativePath === GATE_DECISION_TWIN_PAIR.engineRelative) return gateBody; + if (relativePath === "packages/loopover-miner/expected-engine.version") return "0.3.0\n"; + throw new Error(`unexpected read: ${relativePath}`); + }; + const execGit = vi.fn(() => { + throw new Error("git show failed"); + }); + + const result = runEngineParityChecks({ + root: "/fake", + readFile, + listDir: () => [], + resolveInstalled: () => "0.3.0", + changedFiles: [GATE_DECISION_TWIN_PAIR.hostRelative, "packages/loopover-engine/package.json"], + headEngineVersion: "0.3.0", + execGit, + }); + + expect(result.failures.some((failure) => failure.includes("Gate-decision logic change"))).toBe(true); + }); + + it("skips git resolution and uses headEngineVersion when changedFiles is explicitly empty", () => { + const execGit = vi.fn(() => { + throw new Error("execGit should not be called when changedFiles is empty"); + }); + const result = runEngineParityChecks({ + root: "/fake", + readFile: (_root, relativePath) => { + if (relativePath === "packages/loopover-engine/package.json") return JSON.stringify({ version: "0.3.0" }); + if (relativePath === "packages/loopover-miner/expected-engine.version") return "0.3.0\n"; + throw new Error(`unexpected read: ${relativePath}`); + }, + listDir: () => [], + resolveInstalled: () => "0.3.0", + changedFiles: [], + headEngineVersion: "0.3.0", + execGit, + }); + // With no changedFiles, checkGateDecisionVersionBump never runs at all -- so no version-bump-shaped + // failure appears regardless of the (unrelated) "could not load twin pair" noise from the other four + // named twin pairs this readFile stub doesn't answer for. + expect(result.failures.some((failure) => failure.includes("Gate-decision logic change"))).toBe(false); + expect(execGit).not.toHaveBeenCalled(); + }); }); describe("named twin-pair coverage (#4605)", () => { diff --git a/test/unit/content-lane-registry-logic.test.ts b/test/unit/content-lane-registry-logic.test.ts index 48adc329fb..1d95407e11 100644 --- a/test/unit/content-lane-registry-logic.test.ts +++ b/test/unit/content-lane-registry-logic.test.ts @@ -38,12 +38,41 @@ describe("toCoreVerdict", () => { }); describe("containsSecretLikeText", () => { - it("detects PATs / private keys / wallet terms", () => { + it("detects PATs / private keys / wallet-path / mnemonic terms", () => { expect(containsSecretLikeText("ghp_" + "a".repeat(25))).toBe(true); + expect(containsSecretLikeText("github_pat_" + "a".repeat(25))).toBe(true); expect(containsSecretLikeText("BEGIN PRIVATE KEY")).toBe(true); - expect(containsSecretLikeText("my coldkey is ...")).toBe(true); + expect(containsSecretLikeText("BEGIN RSA PRIVATE KEY")).toBe(true); + expect(containsSecretLikeText("here is the seed phrase")).toBe(true); + expect(containsSecretLikeText("the mnemonic is ...")).toBe(true); + expect(containsSecretLikeText("wallet path: ~/.bittensor/wallets")).toBe(true); expect(containsSecretLikeText("totally benign text")).toBe(false); }); + + // #7981: hotkey/coldkey used to be bare, unqualified matches -- confirmed root cause of 4 mis-closed + // metagraphed PRs (#7469, #7589, #7591, #7594) in one day. A Bittensor "hotkey" is a PUBLIC miner + // identifier, not secret material, and shows up routinely in ordinary registry content. + it("still flags genuine hotkey/coldkey key-material leaks (path/private-key/password/mnemonic/seed adjacency)", () => { + expect(containsSecretLikeText("hotkey path: ~/.bittensor/wallets/default/hotkeys/default")).toBe(true); + expect(containsSecretLikeText("coldkey path: ~/.bittensor/wallets/default/coldkey")).toBe(true); + expect(containsSecretLikeText("hotkey private key: 0xabc123")).toBe(true); + expect(containsSecretLikeText("hotkey_private_key=0xabc123")).toBe(true); + expect(containsSecretLikeText("coldkey password: hunter2")).toBe(true); + expect(containsSecretLikeText("hotkey mnemonic: abandon abandon...")).toBe(true); + expect(containsSecretLikeText("coldkey seed: abandon abandon...")).toBe(true); + expect(containsSecretLikeText("private hotkey: 0xabc123")).toBe(true); + expect(containsSecretLikeText("private coldkey: 0xabc123")).toBe(true); + }); + + it("no longer flags a bare hotkey/coldkey mention with no key-material qualifier (#7981 regression)", () => { + // The exact 4 incident payloads (or the substrings that actually tripped the old bare-word match). + expect(containsSecretLikeText("GET /api/v1/miners/hotkey/{hotkey} on api.affine.io")).toBe(false); + expect(containsSecretLikeText('{"id":"sn-120-affine-miner-by-hotkey","name":"Affine miner detail by hotkey"}')).toBe(false); + expect(containsSecretLikeText('{"notes":"block_number, miner_hotkey, uid, model_revision"}')).toBe(false); + expect(containsSecretLikeText("The /api/v1/benchmark/chunks route requires a sourceDate parameter. No wallet/hotkey data.")).toBe(false); + expect(containsSecretLikeText("my coldkey is a public SS58 address")).toBe(false); + expect(containsSecretLikeText("this endpoint returns the coldkey balance")).toBe(false); + }); }); describe("normalizePublicUrl", () => { diff --git a/test/unit/content-lane-wire.test.ts b/test/unit/content-lane-wire.test.ts index 19eb8ed1ef..e4ff17748e 100644 --- a/test/unit/content-lane-wire.test.ts +++ b/test/unit/content-lane-wire.test.ts @@ -41,6 +41,11 @@ describe("surfaceVerdictToGate", () => { expect(finding?.code).toBe("surface_lane_reject"); // Regression guard: deterministic surface blockers remain outside AI-judgment telemetry. expect(AI_JUDGMENT_BLOCKER_CODES.has(evaluation.blockers[0]!.code)).toBe(false); + // #7981: the assessment summary is fixed, engineer-authored text (never interpolated contributor/AI + // content) -- it must opt out of the public-text privacy scrub so a static word like "wallet" in the + // secret-detection message never gets mangled into the confusing "[context]" placeholder. + expect(finding?.alreadyPublicSafe).toBe(true); + expect(evaluation.blockers[0]?.alreadyPublicSafe).toBe(true); }); it("manual → neutral with a warning (not a failing required check)", () => { @@ -49,6 +54,7 @@ describe("surfaceVerdictToGate", () => { expect(evaluation.blockers).toEqual([]); expect(evaluation.warnings).toHaveLength(1); expect(finding?.code).toBe("surface_lane_manual"); + expect(finding?.alreadyPublicSafe).toBe(true); }); it("falls back to a default summary when the verdict carries none", () => { diff --git a/test/unit/rules.test.ts b/test/unit/rules.test.ts index f65ed391df..c12331e5ea 100644 --- a/test/unit/rules.test.ts +++ b/test/unit/rules.test.ts @@ -870,6 +870,29 @@ describe("advisory rules", () => { } }); + // #7981: a fixed, engineer-authored finding message (e.g. the content lane's deterministic secret-detection + // text) must render verbatim in the check-run output when it declares alreadyPublicSafe, instead of having a + // static word like "wallet" mangled into the confusing "[context]" placeholder. + it("formatCheckRunOutput does NOT sanitize a finding's publicText when alreadyPublicSafe is set", () => { + const advisory = buildPullRequestAdvisory(repo, null); + const alreadySafe = { + ...advisory, + findings: [ + { + code: "surface_lane_reject", + title: "Registry surface review", + severity: "critical" as const, + detail: "Subnet document appears to include secret, wallet, PAT, or private-key material.", + publicText: "Subnet document appears to include secret, wallet, PAT, or private-key material.", + alreadyPublicSafe: true, + }, + ], + }; + const out = formatCheckRunOutput(alreadySafe, "standard"); + expect(out.text).toContain("Subnet document appears to include secret, wallet, PAT, or private-key material."); + expect(out.text).not.toContain("[context]"); + }); + it("formatCheckRunOutput publishes only explicit public finding text", () => { const advisory = buildPullRequestAdvisory(repo, null); const output = formatCheckRunOutput( diff --git a/test/unit/unified-comment-bridge.test.ts b/test/unit/unified-comment-bridge.test.ts index 33f087b96b..3b0099e201 100644 --- a/test/unit/unified-comment-bridge.test.ts +++ b/test/unit/unified-comment-bridge.test.ts @@ -1005,6 +1005,58 @@ describe("gate blockers render in 'Why this is blocked' (FIX D1)", () => { expect(body).toContain("[context]"); }); + // #7981: metagraphed #7469/#7589/#7591/#7594 showed the reviewer's OWN static secret-detection message + // ("...include secret, wallet, PAT, or private-key material.") scrubbed into the confusing "...secret, + // [context], PAT..." — the privacy scrub above exists to catch a private term LEAKING into dynamically + // assembled/AI-generated text, not to mangle a fixed, engineer-authored string nobody ever leaked anything + // into. A finding that declares `alreadyPublicSafe: true` must render verbatim. + it("does NOT scrub a gate blocker's own static message when the finding declares alreadyPublicSafe (#7981)", () => { + const body = buildUnifiedCommentBody({ + gate: gate({ + conclusion: "failure", + summary: "A hard blocker was found.", + blockers: [ + { + code: "surface_lane_reject", + severity: "critical", + title: "Registry surface review", + detail: "Subnet document appears to include secret, wallet, PAT, or private-key material.", + alreadyPublicSafe: true, + }, + ], + }), + panelRows, + readinessTotal: 10, + changedFiles: 1, + footerMarkdown: footer, + }); + expect(body).toContain("Subnet document appears to include secret, wallet, PAT, or private-key material."); + expect(body).not.toContain("[context]"); + }); + + it("still scrubs the SAME blocker text when alreadyPublicSafe is absent (default stays scrub-everything)", () => { + const body = buildUnifiedCommentBody({ + gate: gate({ + conclusion: "failure", + summary: "A hard blocker was found.", + blockers: [ + { + code: "surface_lane_reject", + severity: "critical", + title: "Registry surface review", + detail: "Subnet document appears to include secret, wallet, PAT, or private-key material.", + }, + ], + }), + panelRows, + readinessTotal: 10, + changedFiles: 1, + footerMarkdown: footer, + }); + expect(body).not.toContain("Subnet document appears to include secret, wallet, PAT, or private-key material."); + expect(body).toContain("[context]"); + }); + // gittensory PR #5347: the real-world `summary` evaluateGateCheckCore produces for a "failure" conclusion is // LITERALLY `blockers.map(f => title + action).join("; ")` -- not the short hand-authored string the tests // above use. The earlier tests in this describe block never exercise that realistic value, so they never