Skip to content

fix(cli): neutralise plugin and project strings in boss status, doctor and mcp list - #996

Closed
Antriksh1984 wants to merge 1 commit into
risa-labs-inc:devfrom
Antriksh1984:fix/cli-human-output-terminal-safety
Closed

Antriksh1984 wants to merge 1 commit into
risa-labs-inc:devfrom
Antriksh1984:fix/cli-human-output-terminal-safety

Conversation

@Antriksh1984

Copy link
Copy Markdown
Contributor

Stacked on #995 (it introduces TerminalText). Until that lands, this diff also shows its commits; the change in this PR is the last two commits.

Summary

Follow-up to #995, which made boss mcp ledger safe to print. The other human renderings have the same problem: they print strings that come from outside BOSS to an operator who is deciding what to trust.

  • boss mcp list printed each plugin's tool name, plugin id and description raw, and indented only the first line of a description. A multi-line description could therefore contain • git_status (boss-workspace) on its own line and read as a second, trusted tool in the catalogue.
  • boss mcp describe printed the name, plugin id and permissions raw.
  • boss status printed the active project name (a directory name, so anything a repo or a path can contain), version and the health summary raw. A project name containing a newline could forge a Health: OK line.
  • boss doctor printed finding text and area names raw. It already flattened line breaks, but an ESC sequence still reached the terminal.
  • Each of those also printed an unparseable server response raw as its fallback.

Every one of these strings is attacker-influenceable (an installed plugin, a cloned repository's directory name, a plugin quoted in a health finding). Printed raw, a newline forges an entry and an ESC sequence acts on the terminal. JSON output is unchanged.

Fix

Each of those fields now goes through TerminalText. Multi-line values (descriptions, the unparseable-response fallback) keep their lines but every line is indented, so no line of a description can start with a bullet or a heading of its own.

The three formatHuman* functions were private members of the Clikt command classes and had no tests. They move, unchanged, to internal top-level functions in HumanReports.kt so they can be tested; that move is behavior-neutral and is its own reviewable step in the history.

Tests

Test-first: the six new HumanReportsTerminalSafetyTest cases fail on the unfixed formatters (6 of 6) and pass after.

  • a description cannot forge a second entry in the tool list,
  • names, plugin ids and descriptions are neutralised in the list and in the detail view,
  • an unparseable list response is not printed raw,
  • the active project name and version cannot forge a Health: line or drive the terminal,
  • doctor finding text and area names are neutralised.

The control characters are built from code points in the test source rather than written into it, so the file is plain text.

All of ai.rever.boss.cli.* passes; ktlintCheck and detekt clean.

…r and mcp list

boss status, boss mcp list and their human-readable formatters printed
plugin names, project paths and MCP tool descriptions raw. Those
strings come from an installed plugin or the active project, not from
BOSS itself, so a terminal escape or a forged line in one of them
reached the operator's terminal unmodified.

Extracted the human-report formatters out of BossCommand.kt into
HumanReports.kt and routed every externally sourced field through
TerminalText.safe (from the mcp ledger CLI hardening) before printing.
@Antriksh1984
Antriksh1984 force-pushed the fix/cli-human-output-terminal-safety branch from 5cd1a5c to eeb7d2c Compare September 22, 2026 10:00
@Antriksh1984

Copy link
Copy Markdown
Contributor Author

Rebased onto current dev as one clean commit (was carrying stale duplicate history that made the diff unreviewable and the merge status CONFLICTING, the same issue found on #1406/#1407/#1417/#1420). Content unchanged. Tests, ktlintCheck and detekt pass on the rebased head.

@fluck-boss

Copy link
Copy Markdown
Collaborator

Thanks so much for this contribution, @Antriksh1984 - the work here is genuinely appreciated, and it's clear real care went into it.

This overlaps with #1001 ("fix(cli): neutralise plugin manifest text in boss plugin validate and link"), which is further along in review and will carry this change forward. To keep review attention focused, I'm closing this one in favor of #1001 - please do review and follow that PR, and you're very welcome to pick up any part of it that still needs help or take on another open issue.

Your contribution credit is noted here, and anything unique in this branch that #1001 misses is still welcome as a follow-up. Thanks again for contributing to BossConsole!

@fluck-boss fluck-boss closed this Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants