Skip to content

fix(miner): sanitize discover summary titles#4485

Merged
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-terminal-output-vulnerability
Jul 10, 2026
Merged

fix(miner): sanitize discover summary titles#4485
JSONbored merged 2 commits into
mainfrom
codex/propose-fix-for-terminal-output-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The human-readable discover summary interpolated raw GitHub issue titles into terminal output, allowing attacker-controlled ANSI/OSC/control/bidi sequences and newlines to manipulate terminal display; the renderer must sanitize titles before printing.

Description

  • Add sanitizeDiscoverDisplayText to packages/gittensory-miner/lib/discover-cli.js to strip OSC/ANSI escape sequences, C0/C1 control characters, and bidi controls, collapse whitespace, trim, and bound displayed titles to 240 characters.
  • Use the sanitizer in renderDiscoverSummary when rendering top-candidates so the default human-readable output is safe while leaving JSON output unchanged.
  • Export the sanitizer in packages/gittensory-miner/lib/discover-cli.d.ts for typed consumers.
  • Add regression unit tests in test/unit/miner-discover-cli.test.ts that verify stripping of terminal control sequences, bounding of display length, and nullish handling.

Testing

  • Ran npm run build:miner and npx vitest run test/unit/miner-discover-cli.test.ts, and the unit test suite for the updated file passed (all tests green).
  • Ran npm run typecheck and npm run command-reference:check, both completed successfully with no type or command-reference errors.
  • Attempted the full gate with npm run test:ci and npm audit --audit-level=moderate, but the full test:ci run was blocked/impeded by environment/network-dependent setup (actionlint fallback and a pre-existing cf-typegen drift) and the registry audit endpoint returned 403, so those checks could not be completed in this environment.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 9, 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 6628ae8 Commit Preview URL

Branch Preview URL
Jul 10 2026, 07:01 AM

@superagent-security

Copy link
Copy Markdown
Contributor

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

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

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.12%. Comparing base (63d5a0f) to head (6628ae8).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4485   +/-   ##
=======================================
  Coverage   94.12%   94.12%           
=======================================
  Files         430      430           
  Lines       38181    38181           
  Branches    13922    13922           
=======================================
  Hits        35939    35939           
  Misses       1585     1585           
  Partials      657      657           
🚀 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 commented Jul 9, 2026

Copy link
Copy Markdown

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-10 07:17:35 UTC

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

⏸️ 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 sanitizes GitHub issue titles before they're interpolated into the human-readable `discover` summary line, stripping OSC/ANSI escape sequences, C0/C1 control characters, and bidi override characters, collapsing whitespace, and bounding length to 240 chars. The fix is applied at the correct site (`renderDiscoverSummary`'s title interpolation, `discover-cli.js:90-91`) and only affects the human-readable path — JSON output is left untouched, which is safe since `JSON.stringify` already escapes control characters (including ESC/`\u001b`) as literal `\uXXXX` text rather than raw bytes, so no terminal-control injection is possible there. The new regression test exercises the real `renderDiscoverSummary` call with an actual attacker-shaped title (embedded ESC/OSC/bidi/newline), confirming both the newline-collapse (single-line output) and control-sequence stripping, and a second test confirms nullish handling and the 240-char bound.

Nits — 4 non-blocking
  • discover-cli.js: the OSC pattern `\u001b\][\s\S]*?(?:\u0007|\u001b\\)` requires a terminator to match — an unterminated OSC sequence falls through to `CONTROL_CHARACTER_PATTERN`, which still neutralizes the leading ESC byte but leaves any trailing payload text visible; worth a comment noting this is an accepted trade-off rather than an oversight.
  • consider adding a test case for an unterminated OSC sequence (no `\u0007`/`ST`) to document the fallback behavior explicitly rather than leaving it implicit.
  • Add a short comment above `OSC_SEQUENCE_PATTERN`/`ANSI_ESCAPE_PATTERN` noting the JSON-output path is safe by construction (via `JSON.stringify` escaping), since a future reader may otherwise wonder why `sanitizeDiscoverDisplayText` isn't also applied there.
  • Since `sanitizeDiscoverDisplayText` is now exported and typed, consider reusing it in `src/mcp/find-opportunities.ts`'s title exposure if that hosted output is ever rendered to a terminal-like surface — not required for this PR's scope, just a forward pointer.

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: 48 registered-repo PR(s), 40 merged, 334 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 48 PR(s), 334 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: not available
  • Official Gittensor activity: 48 PR(s), 334 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 9, 2026
@JSONbored JSONbored merged commit 37ff2ef into main Jul 10, 2026
13 checks passed
@JSONbored JSONbored deleted the codex/propose-fix-for-terminal-output-vulnerability branch July 10, 2026 07:20
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

None yet

Development

Successfully merging this pull request may close these issues.

1 participant