Stdio MCP porting: structure travels, secrets never do - #20
Merged
Merged
Conversation
◈ PR Lens
Architecture 
16 components touched across 4 lanes. Inside the changed components — 2 viewsComponent view — Stdio MCP Export & Classification How command-based MCP servers are scanned, sanitized, and packed without secret values. 
Component view — Stdio MCP Apply & Registration How untrusted command definitions are re-validated and registered with freshly resolved secrets. 
Data flow 
The other flows — 1 sequence
View
Tip Push a commit and the comment redraws for the new head. A slow older run never overwrites a newer one. 🪧 More tips
Thanks for using PR Lens! It's built by Coldtea, free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
…er do Supersedes the earlier rule that command-based servers are never applied: hooks are code and port with double consent, so stdio servers get the same treatment — with the one inviolable line intact. Classification extracts the re-creatable shape (command, args, env NAMES) and drops env values at extraction, so no downstream layer can see one. Portability is earned: any machine-local absolute path, bare script filename, or localhost URL keeps the server blocked. Export carries the structure only behind a repeatable --mcp <name>, symmetric to hooks, and the travel picker gains an opt-in MCP group whose hints show the command and the env names it needs. Apply revalidates the untrusted manifest entry in full (name shape, control-byte-free strings, env-name shape, size caps) before assembling the claude mcp add argv — no shell anywhere. Env values resolve on the target: the guided apply asks with a new masked prompt (asterisks on screen, plaintext only in memory; plain mode says its input is visible), and the static path reads the machine's own environment, failing closed by variable name before any file writes — deliberately no flag, so secrets never touch argv or shell history. A missing command binary warns and never refuses. Seven new tests: classification with a planted env value swept from every output, consent-gated manifest shape, picker group and flag echo, argv construction with every refusal, the static-path env round trip through a shim, the guided flow end to end proving the typed secret reaches argv but never a rendered frame, and the masked prompt unit. Threat model and README updated accordingly.
ohansFavour
force-pushed
the
feat/stdio-mcp
branch
from
September 17, 2026 03:15
992ff3a to
1b718b4
Compare
…ion, the gate reruns on apply Dry runs plan with a placeholder resolver, so a resolved secret cannot exist to be echoed, and every argv display goes through a masker that replaces env values with the name and an ellipsis — including claude's own stderr when a registration fails, which could otherwise quote the argv back into our error message. String hygiene now rejects the entire C0 range: CR, LF and TAB were accepted, and a CR-bearing arg could both forge the consent frame and register. Validation now precedes display everywhere — the guided flow validates a stdio entry through the full gate before building any consent text, so a hostile manifest refuses the whole apply without a single byte of it reaching a frame, and the static path's hint lines sanitize names and commands it never validated. The consent block itself is no longer one truncatable line: command and args wrap across full lines with no elision, because that display is the security boundary. The stdio branch now performs the same complete re-derivation the remote branch always did: the exact export-side classifier reruns over the untrusted entry, closing the blocked-class smuggling hole and the portability-gate bypasses found live (script filenames with directory prefixes or in the command position, more script extensions, private and shorthand-IP URLs in args). Token-shaped args are refused at classification: an argument that looks like a credential is a value, and values never travel — the fix is moving it to env. Five new tests: dry-run placeholder round trip both with and without the env set, C0 rejection, the re-derivation refusal table, the hostile-manifest consent-forgery attempt asserting the forged text reaches no frame and nothing is written, and the wrap/mask units.
…sanitize, and the hygiene class covers C1 and bidi The wrap width now follows the live terminal (columns minus the note prefix and continuation indent), so the zero-elision promise holds on the default 80-column terminal where the fixed width was quietly re-truncated by the renderer — an attacker controls layout, so a fixed hidden tail was addressable. The one error message that can carry an unvalidated server name sanitizes it first, closing the last raw path to stderr. And string hygiene grows past C0+DEL to everything a terminal or a reader can be steered by: the C1 range (U+009B is a single-codepoint CSI), zero-widths, bidi and directional-isolate controls, line and paragraph separators, and the BOM — commands and args have no legitimate use for any of them. displayString strips the same class. Two new tests: the 80-column consent wrap proving the tail of a long arg stays on screen, and the hostile-name refusal asserting raw ESC never reaches the message while C1 and bidi args refuse outright.
wrapDisplay accumulates visual cells instead of slicing by JS characters, so CJK and emoji content can no longer overflow the renderer's budget and hide consent text behind an ellipsis on narrow terminals — the same boundary finding, now closed for non-ASCII too. String hygiene collapses from a growing codepoint list to the Unicode properties that define the category: Cc (every control, C0 through C1) and Cf (every invisible format character — zero-widths, bidi and isolate controls, BOM, Arabic letter mark, soft hyphen, word joiner), plus the Zl/Zp separators explicitly. Invisibles let a displayed command read identically to a different argv; none of them have a legitimate place in a command line. displayString strips the same class. Tests: a 120-CJK-character arg keeps its tail marker through the wrap with every line inside the cell budget, and U+061C, U+2060 and U+00AD args refuse.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Command-based (stdio) MCP servers become portable as STRUCTURE, never secrets — superseding the earlier rule that they are never applied.
--mcp <name>(symmetric to hooks); opt-in picker group with command hints.claude mcp add --transport stdio --env K=V -- cmd args, execFile, no shell). Env values resolve on the target — masked prompt in the guided flow,process.envin the static path failing closed by name. Deliberately no--mcp-envflag: secrets never touch argv or shell history. Missing binary warns, never refuses.187 tests, typecheck clean. A planted env value is asserted absent from every manifest, bundle byte, rendered frame, and log.