Skip to content

fix(review): give score-breakdown.ts its own vocabulary instead of the public sanitizer - #6473

Merged
JSONbored merged 1 commit into
mainfrom
fix/score-breakdown-own-vocabulary
Jul 16, 2026
Merged

fix(review): give score-breakdown.ts its own vocabulary instead of the public sanitizer#6473
JSONbored merged 1 commit into
mainfrom
fix/score-breakdown-own-vocabulary

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

src/services/score-breakdown.ts's explainScoreBreakdown() piped its own contributor-facing "explain my score" copy through sanitizePublicComment (src/github/commands.ts), the shared sanitizer for genuinely public GitHub comment surfaces (@loopover mention-command replies, etc.).

This endpoint (POST /v1/scoring/explain-breakdown, and the matching MCP tool) is gated by requireContributorAccess — an authenticated, per-contributor "explain your own score" surface, not a public GitHub comment. Its copy legitimately and pervasively says "score" and "credibility" by design. Reusing the public sanitizer mangled its own output: "saturated near the score cap" → "saturated near the private context cap", "Contributor credibility evidence..." → "Contributor private context evidence...".

Gives score-breakdown.ts its own small, local, curated sanitizer — keeping only a token/local-filesystem-path safety net (this endpoint's output is entirely computed, structured score data, so that's the only genuine residual risk) — instead of the full gittensor-economic-vocabulary substitution. Mirrors the established, documented pattern already used by agent-action-explanation-card.ts / miner-dashboard-recommendations.ts (see src/signals/redaction.ts's note above PUBLIC_UNSAFE_TERMS).

Unblocks a planned follow-up: now that this file no longer depends on sanitizePublicComment, that shared sanitizer can safely add bare-score redaction (already added to queue-intelligence.ts's sanitizer in #6460) without breaking this feature — closing a gap that PR deliberately left open for exactly this reason.

Test plan

  • New regression test asserting the feature's own core vocabulary (score/credibility) survives byte-for-byte, not just "no forbidden wallet/hotkey word leaked"
  • All 25 existing/new tests in test/unit/score-breakdown.test.ts pass; score-breakdown.ts at 100% line/branch coverage (one pre-existing, unrelated branch gap noted, not introduced by this change)
  • Full local gate (npm run test:ci) green

explainScoreBreakdown() piped its own contributor-facing "explain my score"
copy through sanitizePublicComment (src/github/commands.ts), the shared
sanitizer for genuinely public GitHub comment surfaces. This endpoint
(POST /v1/scoring/explain-breakdown, gated by requireContributorAccess) is
authenticated and scoped to the requesting contributor's OWN score, not a
public GitHub comment -- so "score" and "credibility" are its legitimate
core vocabulary, not a leak. The shared sanitizer was mangling both terms
into "private context" throughout this feature's own output.

Mirrors the established, documented pattern in
agent-action-explanation-card.ts / miner-dashboard-recommendations.ts (see
src/signals/redaction.ts's note above PUBLIC_UNSAFE_TERMS): this endpoint's
output is entirely computed, structured score data, so the only genuine
residual risk is an accidentally-embedded token or local filesystem path --
keep just that minimal safety net instead of the full gittensor-economic-
vocabulary substitution.

Unblocks a planned follow-up: once this file no longer depends on
sanitizePublicComment, that shared sanitizer can safely add bare-"score"
redaction (already added to queue-intelligence.ts's sanitizer in #6460)
without breaking this feature.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored JSONbored self-assigned this Jul 16, 2026
@JSONbored
JSONbored merged commit 1fc392b into main Jul 16, 2026
6 checks passed
@JSONbored
JSONbored deleted the fix/score-breakdown-own-vocabulary branch July 16, 2026 09:34
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 16, 2026
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
17244 3 17241 13
View the top 3 failed test(s) by shortest run time
test/unit/backfill.test.ts > GitHub backfill > marks comment, label, and check repair impacts disabled by repo settings
Stack Traces | 0.521s run time
AssertionError: expected { metadata: 'read', …(3) } to not have property "contents"

- Expected:
undefined

+ Received:
"write"

 ❯ test/unit/backfill.test.ts:1203:44
test/unit/backfill.test.ts > GitHub backfill > repair diagnostics require contents:write for merge autonomy (#audit-install-health display)
Stack Traces | 0.549s run time
AssertionError: expected 'write' to be 'read' // Object.is equality

Expected: "read"
Received: "write"

 ❯ test/unit/backfill.test.ts:1267:54
test/integration/api.test.ts > api routes > serves installation repair diagnostics and refreshes installation health
Stack Traces | 0.967s run time
AssertionError: expected { …(11) } to match object { installation: { …(3) }, …(3) }
(8 matching properties omitted from actual)

- Expected
+ Received

@@ -17,8 +17,8 @@
      "path": ".../777/repair/refresh",
    },
    "requiredPermissions": {
      "issues": "write",
      "metadata": "read",
-     "pull_requests": "read",
+     "pull_requests": "write",
    },
  }

 ❯ test/integration/api.test.ts:2195:24

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

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.

1 participant