Skip to content

fix(mcp): sanitize validate-config terminal output#4090

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-untrusted-output-in-cli-warnings
Jul 8, 2026
Merged

fix(mcp): sanitize validate-config terminal output#4090
JSONbored merged 1 commit into
mainfrom
codex/fix-untrusted-output-in-cli-warnings

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The validate-config CLI printed parser warnings from untrusted repository manifests directly to stdout, allowing ANSI/OSC and C0/C1 control bytes from manifest values to reach a user's terminal and potentially manipulate terminal state or clipboard.

Description

  • Add sanitizePlainTextTerminalOutput in packages/gittensory-mcp/bin/gittensory-mcp.js and apply it to plain-text warning output emitted by the validate-config CLI so ANSI/OSC and C0/C1 control bytes are stripped before writing to stdout.
  • Preserve machine-readable behaviour by leaving --json output untouched (the raw warnings remain in JSON responses).
  • Extend the test fixture server in test/unit/support/mcp-cli-harness.ts with a validateConfigWarnings injection point so tests can exercise API-returned warnings.
  • Add a regression unit test in test/unit/mcp-cli-validate-config.test.ts that asserts terminal sequences (OSC 52, BEL, ANSI) are removed from plain output but preserved in --json output.

Testing

  • Ran npx vitest run test/unit/mcp-cli-validate-config.test.ts, which passed (3 tests passed).
  • Ran npm run build:mcp, which completed successfully (node --check verification passed).
  • Ran npm run command-reference:check and npm run typecheck, both of which completed successfully.
  • Attempted npm run test:ci, which did not complete because cf-typegen drift was detected by the local check (generated Cloudflare types reported stale), preventing the full suite from finishing.
  • Attempted npm audit --audit-level=moderate, which could not complete due to the registry audit endpoint returning 403 Forbidden in this environment.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 8, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 49644df Commit Preview URL

Branch Preview URL
Jul 08 2026, 12:05 AM

@codecov

codecov Bot commented Jul 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.63%. Comparing base (9ba80b6) to head (49644df).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4090   +/-   ##
=======================================
  Coverage   93.63%   93.63%           
=======================================
  Files         384      384           
  Lines       35822    35822           
  Branches    13146    13146           
=======================================
  Hits        33543    33543           
  Misses       1618     1618           
  Partials      661      661           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gittensory-orb gittensory-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. label Jul 8, 2026
@gittensory-orb

gittensory-orb Bot commented Jul 8, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-08 00:14:44 UTC

3 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR adds `sanitizePlainTextTerminalOutput` (packages/gittensory-mcp/bin/gittensory-mcp.js) to strip ANSI CSI/OSC/DCS escape sequences and remaining C0/C1 control bytes from `validate-config`'s plain-text warning output, leaving `--json` output raw. The regex correctly handles CSI (`\x1b[...`), OSC (`\x1b]...` terminated by BEL or ST), and other escape forms, with a second pass mopping up any leftover control bytes including a bare ESC. The regression test in test/unit/mcp-cli-validate-config.test.ts exercises a real OSC52+BEL+CSI payload injected through the fixture server's new `validateConfigWarnings` option (not a fabricated shape) and correctly asserts the JSON path stays untouched. The fix is scoped only to validate-config's warnings loop and doesn't touch `lintPrTextCli`'s `- ${fix}` output a few lines above in the same file, which prints from the same class of untrusted API/manifest-derived data.

Nits — 5 non-blocking
  • lintPrTextCli's `process.stdout.write(`- ${fix}\n`)` (bin/gittensory-mcp.js, just above the new function) prints untrusted fix text without the same sanitization — worth reusing `sanitizePlainTextTerminalOutput` there too if fixes can originate from repo/manifest content, for consistency with the threat model this PR just established.
  • The bare C0/C1-byte stripping branch (`\x00-\x08`, `\x0b-\x0c`, `\x0e-\x1f`, `\x7f-\x9f`) isn't independently exercised by a test case that omits an ESC prefix — the current test only proves it via bytes adjacent to escape sequences.
  • No linked issue is referenced in the PR description; per this repo's contribution convention, external PRs should close or reference an eligible open issue even for security fixes like this one.
  • Export or reuse `sanitizePlainTextTerminalOutput` for other plain-text stdout sinks in this CLI (e.g. lint-pr-text's fixes list) that render untrusted manifest/API-derived strings.
  • Add a standalone control-byte test case (e.g. a lone `\x07` with no preceding ESC) to lock in the second `.replace()` pass independently of the ANSI-sequence pass.

Concerns raised — review before merging

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 51 registered-repo PR(s), 43 merged, 496 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 496 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 51 PR(s), 496 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@gittensory-orb gittensory-orb Bot added the manual-review Gittensor contributor context label Jul 8, 2026
@JSONbored JSONbored merged commit a844972 into main Jul 8, 2026
12 checks passed
@JSONbored JSONbored deleted the codex/fix-untrusted-output-in-cli-warnings branch July 8, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex gittensor:bug Gittensor-scored bug fix — scores a 0.5x multiplier. manual-review Gittensor contributor context

Projects

Development

Successfully merging this pull request may close these issues.

1 participant