fix(mcp): sanitize every terminal path that prints API-controlled text

[luciferlive112116]

Summary

sanitizePlainTextTerminalOutput strips ANSI escapes and control characters, but guarded exactly one call site: validateConfigCli's warning loop. Every other command that printed server-controlled free text wrote it straight to the terminal. A hostile API response could therefore repaint the screen, erase the lines above it, or park a convincing fake verdict beside the real one — the terminal cannot tell this CLI's text from the payload's.

Command	Now-sanitized fields
slop-risk	finding.title, finding.detail, score line (slopRisk, band)
issue-slop	same
decision-pack	payload.summary, cache.rerunGuidance
repo-decision	decision.nextActions[], cache.rerunGuidance
maintain status / queue	action.id, actionClass, pullNumber, reason, summary, target
doctor	check.status, check.name, check.detail, check.remediation

issue-slop is the sharpest of these: the body it assesses is routinely a third party's issue text, so hostile input is the expected case, not a hypothetical one.

Both slop commands' score lines are sanitized alongside their findings. The whole payload is the API's (apiPost), so leaving band raw would have kept the exact command exploitable by the exact response the findings are being protected from.

What I verified rather than assumed

The issue asked me to confirm, not assume, before skipping any field. Three findings:

• decisionPackToolSummary — skipped, safe. It interpolates login (the user's own --login/env value) and payload.freshness, and freshness only reaches the string inside an equality guard against the literals "stale"/"rebuilding". The API cannot route text of its own choosing through it.
• repoDecisionToolSummary — skipped, safe. It takes only login and repoFullName, both typed by the user on their own command line. No payload text reaches it.
• cache.rerunGuidance — sanitized, and this one surprised me. Its only in-repo producer is a local literal, which suggested it was safe. But getDecisionPackWithCache returns the API payload verbatim on the success path, so a response carrying its own cache object prints straight through. The local literal is only the fallback path. Sanitized accordingly.

Also worth recording: doctor's details already pass through sanitizeDiagnosticText, but that redacts tokens and local paths and is indifferent to escape sequences — the two are orthogonal. So the terminal pass is applied at the print boundary, where it covers every check source (API errors, npm-registry errors, compatibility reports) at once.

Why --json is deliberately left alone

JSON.stringify escapes U+001B (and the rest of U+0000–U+001F) into its six-character escaped form, so an escape sequence cannot survive into the printed document — and stripping bytes there would corrupt the machine-readable contract callers parse. Rather than trust that reasoning, a test pins it: it asserts --json emits no raw escape and that the payload still round-trips byte-for-byte.

Validation

• New suite test/unit/mcp-cli-terminal-sanitization.test.ts — 7/7 pass. Each test drives a real command against a fixture answering with a colour + cursor-up + line-erase + OSC title-set + NUL payload.
• All 6 injection tests fail against the pre-fix CLI — verified by stashing only the source fix. (The pre-fix output made grep report "binary file matches", which is the bug demonstrating itself.) The --json test passes on both sides, which is exactly its purpose.
• Tests assert the text is sanitized, not dropped — the readable tail still reaches the operator, minus its teeth.
• 7 suites for every touched command — 70 passed; the only 2 failures (--body-file non-regular inputs) are pre-existing on clean main with my change absent (Windows symlink restriction; they pass on CI's Linux).
• 4 suites that consume the modified harness — 39/39 pass; the new fixture option is optional and off by default.
• npm run typecheck — 0 errors. npm run build:mcp — passes. git diff --check — clean. Rebased on latest main, no base conflict.
• Verified programmatically that no raw control byte was introduced into any changed file — every escape is written as a six-character escape sequence in source.

Coverage

No patch surface: coverage is collected over src/**, packages/loopover-engine/src/**, and packages/loopover-miner/lib/**. This PR touches only packages/loopover-mcp/** (not collected) and test/** (ignored).

Scope

• One coherent change; wanted paths (packages/, test/). validateConfigCli's already-sanitized path is untouched, as required.
• No secrets/tokens/wallet/trust-score/reward terms; no changelog, site/, CNAME, or lovable changes.

Safety

• This is the security fix itself: it closes terminal-escape injection via a malicious API response across every named command, matching the protection validateConfigCli already had.
• No behaviour change for well-formed payloads — the CLI emits no intentional ANSI of its own, so there is no colour to preserve.

Closes #6261

[superagent-security]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.60%. Comparing base (fe1b96a) to head (7506cda).
⚠️ Report is 23 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6396   +/-   ##
=======================================
  Coverage   95.60%   95.60%           
=======================================
  Files         599      601    +2     
  Lines       47301    47310    +9     
  Branches    15046    15050    +4     
=======================================
+ Hits        45223    45232    +9     
  Misses       1291     1291           
  Partials      787      787           

Flag	Coverage Δ
shard-1	44.01% <ø> (-0.02%)	⬇️
shard-2	36.78% <ø> (+0.12%)	⬆️
shard-3	32.46% <ø> (-0.12%)	⬇️
shard-4	34.71% <ø> (-0.01%)	⬇️
shard-5	31.57% <ø> (+<0.01%)	⬆️
shard-6	44.97% <ø> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

[loopover-orb]

Tip

✅ LoopOver review result - approve/merge recommended

_{Review updated: 2026-07-16 06:54:05 UTC}

3 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

✅ Suggested Action - Approve/Merge

• safe to merge

Review summary
This PR extends `sanitizePlainTextTerminalOutput` from a single call site to every command that prints API-controlled free text (slop-risk, issue-slop, decision-pack, repo-decision, maintain status/queue, doctor), while correctly leaving --json output and pure user-input interpolations (login, repoFullName, freshness-guarded literals) unsanitized. The reasoning for the two skipped call sites (decisionPackToolSummary, repoDecisionToolSummary) is traced correctly against the visible diff — freshness is guarded by literal equality and login/repoFullName are user-typed. The added test suite drives real commands against a fixture server injecting ANSI/control-char payloads and asserts they're neutralized, which is real coverage of the actual code paths rather than fabricated scenarios.

Nits — 5 non-blocking

• packages/loopover-mcp/bin/loopover-mcp.js:2787 — `actions.slice(0, 3)` uses an unexplained magic number 3; consider naming it or leaving a comment on why 3 specifically.
• The long comment blocks above `sanitizePlainTextTerminalOutput` and each call site are verbose for a one-line sanitizer call; could be trimmed to a single line per site without losing the rationale.
• doctor's `check.detail` note mentions `sanitizeDiagnosticText` already redacting tokens/paths — worth double-checking the two sanitizers don't conflict or double-process the same string in a way that mangles legitimate diagnostic text.
• Consider extracting the repeated `sanitizePlainTextTerminalOutput(...)` field lists (e.g., in maintain status/queue) into a small helper to reduce duplication across the four call sites doing the same action-formatting pattern.
• The --json pinning test is a nice touch; consider adding one asserting doctor's `--json` path is similarly unsanitized/verbatim for symmetry with the other commands.

Decision drivers

• ✅ Code review — No blockers (1 reviewer)
• ✅ Gate result — Passing (No configured blocker found.)

Context & advisory signals — never blocks the verdict

Signal	Result	Evidence
Linked issue	✅ Linked	#6261
Related work	✅ No active overlap found	No same-issue or scoped active PR overlap found.
Change scope	❌ 8/20	High review scope from cached public metadata (1 linked issue).
Validation posture	✅ 25/25	PR body includes validation/test evidence.
Contributor workload	✅ 10/10	Author activity: 143 registered-repo PR(s), 81 merged, 32 issue(s).
Contributor context	✅ Confirmed Gittensor contributor	luciferlive112116; Gittensor profile; 143 PR(s), 32 issue(s).
Improvement	✅ Minor	risk: clean · value: minor

Review context

• Author: luciferlive112116
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository is configured for direct PR review.
• Public profile languages: not available
• Official Gittensor activity: 143 PR(s), 32 issue(s).
• PR-specific overlap: none found.

Contributor next steps

• Start here: Add a concise scope and risk note.
• Then work through the remaining 1 step in the Signals table above.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
• Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
• Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

• @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
• @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
• A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

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

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

• Re-run LoopOver review