Skip to content

Forbidden-term redaction lists have drifted between advisory.ts's check-run sanitizer and unified-comment-bridge.ts's Nit scrubber #8322

Description

@JSONbored

Context

src/review/unified-comment-bridge.ts (around line 70) documents its PRIVATE_FORBIDDEN_TERMS regex as mirroring the check-run sanitizer:

// Mirrors src/rules/advisory.ts CHECK_RUN_FORBIDDEN_TERMS (scrubbed → "[context]") and
// src/signals/engine.ts containsPrivatePublicTerm (drop if still present). Kept inline so this module
// stays a pure, dependency-light renderer-mapping seam.
const PRIVATE_FORBIDDEN_TERMS =
  /\b(?:rewards?|payouts?|farming|estimated\s+scores?|raw\s+trust\s+scores?|trust\s+scores?|score\s+estimates?|reward\s+estimates?|wallets?|hotkeys?|coldkeys?|reviewability|scoreability|private\s+signals?|likely_duplicate|reviewability\s*\d)\b/gi;

It currently does not mirror it. src/rules/advisory.ts's CHECK_RUN_FORBIDDEN_TERMS (line 381-382) is:

const CHECK_RUN_FORBIDDEN_TERMS =
  /\b(?:rewards?|payouts?|farming|estimated\s+scores?|raw\s+trust\s+scores?|raw\s+trust|trust\s+scores?|score\s+estimates?|reward\s+estimates?|wallets?|hotkeys?|coldkeys?|mnemonics?|seed\s?phrases?|cohorts?|miner[-_\s]?originated|human[-_\s]?originated|rankings?|reviewability|scoreability|private\s+signals?)\b/gi;

Diffing the two term lists:

  • advisory.ts has raw\s+trust (bare), mnemonics?, seed\s?phrases?, cohorts?, miner[-_\s]?originated, human[-_\s]?originated, rankings?none of these are in unified-comment-bridge.ts's PRIVATE_FORBIDDEN_TERMS, and its PRIVATE_DROP_TERMS fallback (used by publicSafeNit to drop a Nit that still trips after scrubbing) doesn't cover them either. So a contributor-facing PR-comment Nit containing e.g. "cohort" or "mnemonic" or "human-originated" passes through unredacted to a public GitHub comment via publicSafeNit, even though the identical term is correctly scrubbed on the check-run path.
  • unified-comment-bridge.ts has likely_duplicate and reviewability\s*\d (the latter added specifically to catch a term like "reviewability3" with no word boundary before the digit) — neither exists in advisory.ts's CHECK_RUN_FORBIDDEN_TERMS, so those terms could leak into check-run annotations/text instead.

This is the same recurring bug class as several previously-fixed issues in this repo (redaction term lists drifting between sibling public-surface sanitizers — see #7074, #5840, #8020, #6407), just a fresh, currently-unfixed instance between these two specific files. test/unit/unified-comment-bridge.test.ts documents the module's own internal invariant ("the scrub list is a superset of the drop terms") but never asserts parity with advisory.ts's list, so nothing currently catches this drift.

Requirements

  • Add the missing terms from advisory.ts's CHECK_RUN_FORBIDDEN_TERMS to unified-comment-bridge.ts's PRIVATE_FORBIDDEN_TERMS: raw\s+trust (bare, in addition to the existing raw\s+trust\s+scores?), mnemonics?, seed\s?phrases?, cohorts?, miner[-_\s]?originated, human[-_\s]?originated, rankings?. Also add the drop-fallback equivalents to PRIVATE_DROP_TERMS for at least cohort and mnemonic (the two most likely to appear in free-text Nits), following the existing style of that regex.
  • Add the missing terms from unified-comment-bridge.ts's PRIVATE_FORBIDDEN_TERMS to advisory.ts's CHECK_RUN_FORBIDDEN_TERMS: likely_duplicate and reviewability\s*\d.
  • Add a new test (in test/unit/rules.test.ts or test/unit/unified-comment-bridge.test.ts, whichever the contributor finds cleaner to wire against both modules' exports) that asserts term-set parity going forward: run a canonical list of every term-shaped test string (drawn from the union of both regexes after this fix) through both sanitizeForCheckRun (from src/rules/advisory.ts) and publicSafeNit/PRIVATE_FORBIDDEN_TERMS (from src/review/unified-comment-bridge.ts), and assert every term is redacted (or a Nit is dropped) by both. This must be a real, standalone test that would fail if either list regresses in the future — not just a static string comparison of the two regex sources (their exact regex syntax legitimately differs, e.g. one bare word vs a scored-suffix variant).

Deliverables

  • src/review/unified-comment-bridge.ts's PRIVATE_FORBIDDEN_TERMS and PRIVATE_DROP_TERMS updated with the missing terms listed above.
  • src/rules/advisory.ts's CHECK_RUN_FORBIDDEN_TERMS updated with likely_duplicate and reviewability\s*\d.
  • A new parity test proving every forbidden term is now redacted/dropped by both sanitizers.

Test Coverage Requirements

This repo's Codecov patch gate requires 99%+ coverage of changed lines and branches. Every newly-added term must have at least one test case proving it is actually matched and redacted/dropped by the sanitizer it was added to (not just added to the regex source with no assertion).

Expected Outcome

A term forbidden on one public-facing redaction path (check-run text/annotations, or PR-comment Nits) is forbidden on the other — the two lists can no longer silently diverge despite the file's own comment claiming they mirror each other.

Links & Resources

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions