Skip to content

Check-run redaction (CHECK_RUN_FORBIDDEN_TERMS) missing terms sanitizePublicComment already fixed for the same leak class #7074

Description

@JSONbored

Context

This codebase has one documented "canonical" public/private redaction primitive, PUBLIC_UNSAFE_TERMS / isPublicSafeText in src/signals/redaction.ts (lines 1-50), whose header comment states: "Any text destined for a PUBLIC surface ... must pass isPublicSafeText first, so a single regex governs redaction and new surfaces cannot drift their own copy," and explicitly names the only two sanctioned exceptions (agent-action-explanation-card.ts and miner-dashboard-recommendations.ts, each "curated for their surface").

However, the GitHub check-run output surface — a genuinely public surface (visible in the PR's Checks tab to anyone) — is redacted by a third, independently-maintained regex, CHECK_RUN_FORBIDDEN_TERMS / sanitizeForCheckRun, duplicated byte-for-byte in both src/rules/advisory.ts (line 325-330) and packages/loopover-engine/src/advisory/gate-advisory.ts (line 29-34):

const CHECK_RUN_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?)\b/gi;

This vocabulary is missing several terms that the canonical PUBLIC_UNSAFE_TERMS covers, and — critically — the codebase has already found and fixed this exact class of leak once, in the sibling PR-comment sanitizer sanitizePublicComment (src/github/commands.ts, lines 1841-1866). That function's own comment (lines 1851-1854) documents the fix: "'cohort' and standalone miner-/human-originated / 'raw trust' (not just the 'raw trust score' compound) leaked through this sanitizer: no entry above catches a bare mention of any of these" — and it now redacts cohorts?, miner[-_\s]?originated, human[-_\s]?originated, bare raw trust, mnemonics?, seed phrases?, and bare rankings? (line 1849, 1859-1860).

None of those terms were ever ported to CHECK_RUN_FORBIDDEN_TERMS. Concretely, mnemonic(s), seed phrase(s), cohort(s), miner-originated/human-originated, bare ranking(s), and bare raw trust (only the raw trust score(s) compound is covered) can all leak through sanitizeForCheckRun into check-run titles/summaries/annotation text today, even though the same leak class was identified and closed for the PR-comment surface.

sanitizeForCheckRun is applied to finding titles/details/actions (which can include maintainer- or contributor-influenced text, e.g. a .loopover.yml pre-merge check's configured name, or a PR title) via formatCheckRunOutput/formatGateCheckOutput in both src/rules/advisory.ts and its packages/loopover-engine twin.

Requirements

  • CHECK_RUN_FORBIDDEN_TERMS in both src/rules/advisory.ts and packages/loopover-engine/src/advisory/gate-advisory.ts must be updated to additionally redact (at minimum): mnemonics?, seed\s?phrases?, cohorts?, miner[-_\s]?originated, human[-_\s]?originated, bare raw\s+trust (not only the raw trust score(s) compound), and bare rankings?.
  • Both files must be edited in the same PR with the identical updated regex — this is the GATE_DECISION_TWIN_PAIR enforced by scripts/check-engine-parity.ts's checkGateDecisionVersionBump: a single-sided edit to only one of these two files fails CI unless packages/loopover-engine/package.json's version is also bumped, so this PR must either touch both files identically or bump that version — touching both files identically is the correct fix here (do not bump the engine package version; these are deliberately "kept structurally divergent" files that stay in lock-step ONLY on the specific gate-decision markers scripts/check-engine-parity.ts checks, and this redaction constant must be updated on both sides).
  • Do not converge sanitizeForCheckRun/CHECK_RUN_FORBIDDEN_TERMS onto PUBLIC_UNSAFE_TERMS from src/signals/redaction.ts as part of this fix — src/rules/advisory.ts is explicitly documented (file header) as an intentionally-divergent twin from the engine copy that must not import the signals/** subsystem; keep CHECK_RUN_FORBIDDEN_TERMS as its own regex, only add the missing terms.

Deliverables

  • CHECK_RUN_FORBIDDEN_TERMS updated identically in src/rules/advisory.ts and packages/loopover-engine/src/advisory/gate-advisory.ts to cover the terms listed above.
  • A regression test (in the host-side advisory test suite) asserting sanitizeForCheckRun/formatCheckRunOutput/formatGateCheckOutput redacts a title/detail containing mnemonic, seed phrase, cohort, miner-originated, human-originated, raw trust, and ranking to [context], mirroring the existing sanitizePublicComment test coverage for the same terms.
  • An equivalent regression test added to packages/loopover-engine's own test suite for its copy of sanitizeForCheckRun.

Test Coverage Requirements

Both src/rules/advisory.ts (under src/**) and packages/loopover-engine/src/advisory/gate-advisory.ts (a separate package with its own test/coverage setup) are measured — 99%+ patch coverage on every changed line and branch in both files, exercised by the new regression tests above.

Expected Outcome

The GitHub check-run public surface redacts the same private/economic-identity vocabulary the PR-comment surface already redacts (having already fixed this exact leak once), so mnemonic/seed phrase/cohort/miner-originated/human-originated/bare raw trust/bare ranking can no longer leak through a check-run title, summary, or annotation.

Links & Resources

  • src/rules/advisory.ts (CHECK_RUN_FORBIDDEN_TERMS, line 325-326; sanitizeForCheckRun, line 328-330)
  • packages/loopover-engine/src/advisory/gate-advisory.ts (CHECK_RUN_FORBIDDEN_TERMS, line 29-30; sanitizeForCheckRun, line 32-34)
  • src/github/commands.ts (sanitizePublicComment, lines 1841-1866) — the sibling surface that already fixed this exact leak class, and the source of the additional terms this issue asks to port
  • src/signals/redaction.ts (PUBLIC_UNSAFE_TERMS, isPublicSafeText) — the documented "canonical" primitive this surface diverges from
  • scripts/check-engine-parity.ts (GATE_DECISION_TWIN_PAIR, checkGateDecisionVersionBump) — the CI check governing why both files must change together

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