fix(review): stop bare hotkey/coldkey mentions from false-positiving as secret leaks

.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"
 }

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.",

packages/loopover-miner/expected-engine.version

@@ -1 +1 @@
-3.4.0
+3.4.1

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):`);

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 };
 }
 

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([

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

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");
   }

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

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)", () => {

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", () => {