Skip to content

fix(public-safety): redact score numbers in 'estimated score N -> M' wording - #930

Merged
JSONbored merged 6 commits into
JSONbored:mainfrom
galuis116:fix/sanitize-estimated-score-numbers
Jun 20, 2026
Merged

fix(public-safety): redact score numbers in 'estimated score N -> M' wording#930
JSONbored merged 6 commits into
JSONbored:mainfrom
galuis116:fix/sanitize-estimated-score-numbers

Conversation

@galuis116

Copy link
Copy Markdown
Contributor

Summary

sanitizePublicComment (the shared public-safety sanitizer) redacts the raw score-transition numbers (N -> M) only for the effective/projected prefixes — but not for the estimated score prefix, which is the exact wording the score engine (buildGateDeltas) actually emits. So a gate-delta explanation is only half-sanitized: the words estimated score become private context, but the raw internal scores survive.

// src/scoring/preview.ts — buildGateDeltas emits the "estimated score N -> M" wording
explanation: `Open PR pressure changes estimated score ${current...} -> ${best...}.`,

Running the real sanitizer on the real engine wording:

"Open PR pressure changes estimated score 32.5 -> 41.2."
  => "Open PR pressure changes private context 32.5 -> 41.2."   ← raw 32.5 -> 41.2 LEAKS

These strings flow into every gittensory_explain_score_breakdown / POST /v1/scoring/explain-breakdown response, which runs each one through this sanitizer precisely to be public-safe (src/services/score-breakdown.ts gateHighlightsForsanitizePublicComment(delta.explanation)). The estimated score is a product-defined private "scoreability" value (estimated score/score estimate/scoreability are forbidden public terms), so leaking the raw number is a scoreability leak by the project's own definition.

The sanitizer's contract — test/unit/github-commands.test.ts — asserts the score-transition numbers are redacted, but only for projected/effective; it never covered the estimated score prefix the engine uses, so the gap shipped green.

Fix

  • Add estimated to the number-stripping pattern, so estimated score N -> M is fully redacted (matches the engine's wording).
  • Add a catch-all that clears any residual numeric transition left behind after a phrase replacement (e.g. score estimate/score preview followed by N -> M), so the same class of half-redaction can't recur for sibling phrases.

Tests

Added regression assertions for the buildGateDeltas wording (estimated score 32.5 -> 41.2, ... 18 -> 27) and the residual catch-all (score estimate 5 → 9) — all asserting the numbers are gone, not just the words. Full unit suite green locally (2218 passed; only the known local-only CRLF gittensory-focus-manifest test fails locally, passes in CI).

Closes #929

…wording

sanitizePublicComment stripped the raw score-transition numbers only for the
effective/projected prefixes, but the score engine (buildGateDeltas) emits the
'estimated score N -> M' wording — so 'estimated score 32.5 -> 41.2' was only
half-redacted (words replaced, numbers leaked) into every explain-breakdown
response, which runs each string through this sanitizer to be public-safe.

Add 'estimated' to the number-stripping pattern and a catch-all that clears any
residual numeric transition left after a phrase replacement.
@galuis116
galuis116 requested a review from JSONbored as a code owner June 19, 2026 22:53
@dosubot dosubot Bot added the size:XS label Jun 19, 2026
@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #930 is no longer open. No action.

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

@ghost ghost added the gittensory:reviewed label Jun 19, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

🔍 Maintainer review

Reviewed 2 file(s) — ✅ all CI green.

Suggested action:Safe to merge — both reviewers found no blocking issues.

Reviewer A · gpt-oss-120b — recommends ✅ merge
The PR correctly expands the public comment sanitizer to also catch "estimated" score phrasing and adds a catch‑all rule to strip residual numeric transitions after replacement. No security concerns are introduced, and the added tests cover the new behavior. The change is limited to the sanitization logic, which is core but well‑contained.

Suggestions

  • Consider adding a comment explaining the intended scope of the catch‑all regex to aid future maintainers.
  • Run the full test suite to ensure no unrelated sanitization regressions appear.

Worth double-checking

  • The new regex could over‑redact if "private context" appears in legitimate (non‑sanitized) user content, though this is unlikely.

Reviewer B · nemotron-3-120b-a12b — recommends ✅ merge
The change adds 'estimated' to the score transition regex and includes a catch-all to redact residual numbers after phrase replacements, ensuring no score numbers leak in public output. Tests verify the new cases. The fix aligns with the public/private boundary invariant.

Worth double-checking

  • The catch-all regex could theoretically match non-score number transitions after 'private context', but the pattern is specific and the risk is low compared to leaking score data.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.61%. Comparing base (d0e549f) to head (20b4c75).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #930   +/-   ##
=======================================
  Coverage   96.61%   96.61%           
=======================================
  Files         110      110           
  Lines       14928    14928           
  Branches     5399     5399           
=======================================
  Hits        14422    14422           
  Misses        104      104           
  Partials      402      402           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 20, 2026
@dosubot dosubot Bot added the lgtm label Jun 20, 2026
@JSONbored
JSONbored merged commit e0ac11f into JSONbored:main Jun 20, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: sanitizePublicComment leaks raw scores in estimated-score wording

2 participants