Repository navigation
smith mcp list gibt die stderr-Ausgabe eines Servers ungefiltert aus #109
Description
Activity
Nachtrag aus dem Re-Review von #92: die stderr-Ausgabe des Servers ist nicht die einzige undichte Stelle in
smith mcp list.Die Fehlerzeilen geben zusätzlich smiths eigene, selbst zusammengesetzte Meldungen mit der vollständigen URL aus
mcp.jsonaus — inklusive Userinfo, Pfad, Query und Fragment:userinfo: could not reach the MCP server at http://LEAKUSER:LEAKPASS@127.0.0.1:1/v1/LEAKPATHTOKEN/mcp?token=LEAKQUERY#LEAKFRAG: …Das ist ein Geheimnis aus der Konfigurationsdatei, nicht die Beschwerde eines Servers — der Zielkonflikt aus der Beschreibung oben gilt dafür also nicht. In #92 hat nur die Spalte
COMMANDdas neuesafe_descriptionbekommen; die Fehlerzeile blieb unangetastet.Damit zerfällt das Issue in zwei Teile:
- Ohne Zielkonflikt, klar zu beheben: die selbst komponierten Meldungen durch dieselbe URL-Bereinigung schicken, die
smith doctorbenutzt (ServerSpec.scrub_urlsbzw.safe_description). - Mit Zielkonflikt, wie oben beschrieben: die stderr-Ausgabe des Servers, die man beim Debuggen eines nicht startenden Servers eigentlich sehen will.
Teil 1 sollte zuerst passieren — er kostet nichts an diagnostischem Wert.
Ebenfalls aus #92 übernehmenswert:
ServerSpec.scrub_urlsist ein reiner URL-Filter, kein allgemeiner Redaktor. Derelse ex.message-Zweig insrc/smith/mcp/manager.crschickt beliebigen Ausnahmetext hindurch — alles, was nicht wie eine URL aussieht, bleibt stehen.- Ohne Zielkonflikt, klar zu beheben: die selbst komponierten Meldungen durch dieselbe URL-Bereinigung schicken, die
Aufgeteilt, wie im Kommentar oben vorgeschlagen. Teil 1 — die selbst komponierten Meldungen, die die URL aus
mcp.jsonungekürzt ausgeben — ist jetzt #110 und kostet nichts an diagnostischem Wert.Dieses Issue trägt ab hier nur noch Teil 2: die stderr-Ausgabe des Servers und die vererbte Umgebung des Kindprozesses. Damit fällt die Priorität von
prio:1aufprio:2— der billige Teil ist ausgezogen, was bleibt, ist die Hälfte mit dem Zielkonflikt und mit echtem Aufwand.Zur offenen Frage: welche Variablen durchgereicht werden dürfen
Noch nicht entschieden. Zwei Befunde, die die Frage kleiner machen, als sie aussieht:
Der Spawn erbt heute alles.
StdioTransport.spawn_server(src/smith/mcp/protocol.cr:206-214) ruftProcess.newohneclear_envund legtenvaus dermcp.jsonnur darüber. Ein MCP-Server sieht damit jeden Provider-Key, den smith selbst benutzt.Der bewusste Durchreichweg existiert halb. Für HTTP-Server expandiert
expand_headers(src/smith/mcp/server_config.cr:263-284)${VAR}aus der Umgebung — „the intended way to hand over a secret without writing it into the file", inklusive Warnung bei nicht gesetzter Variable. Für stdio-Server gibt es das nicht:env: string_map(fields["env"]?)(Zeile 232) nimmt den Wert wörtlich.Damit zerfällt die Entscheidung in zwei viel handlichere:
${VAR}inenv:unterstützen, mit derselben Mechanik wieexpand_headers. Das ist eine kleine, für sich sinnvolle Änderung — sie entspricht auch der Konvention, die Claude Codesmcp.jsonbenutzt, und ist die Voraussetzung dafür, dass eine Allowlist überhaupt bedienbar ist.- Erst dann
clear_env: trueplus eine schmale Basisliste dessen, was ein Prozess zum Laufen braucht (PATH,HOME,USER,SHELL,TERM,LANG/LC_*,TMPDIR, dazuHTTP_PROXY/HTTPS_PROXY/NO_PROXY). Alles Weitere —GITHUB_TOKENund Verwandte — schreibt der Autor ausdrücklich als"env": {"GITHUB_TOKEN": "${GITHUB_TOKEN}"}hin.
Die Reihenfolge ist wichtig: Schritt 2 vor Schritt 1 bricht jeden Server, der heute auf eine geerbte Variable baut, ohne ihm einen Ersatzweg zu lassen.
Offen bleibt, ob Schritt 2 eine Übergangszeit braucht — etwa eine Warnung beim Start, wenn ein Server ohne
env:läuft und der Schutz greifen würde. Wert eines Spikes, bevor Code entsteht.- added a commit that references this issue
on Sep 4, 2026 Schritt 1 ausgegliedert: #122 trägt jetzt die
${VAR}-Expansion inenv:eines stdio-Servers — klein, für sich sinnvoll,ready-for-agent, und ohne Zielkonflikt.Dieses Issue trägt ab hier nur noch Schritt 2:
clear_env: trueplus die schmale Basis-Allowlist, und die offene Frage nach einer Übergangszeit. Damitready-for-human: was durchgereicht werden darf, ist eine Policy-Entscheidung, keine Implementierungsfrage, und die Antwort auf den Zielkonflikt (stderr weglassen, hinter ein Flag, oder maskieren) steht ebenfalls noch aus. #122 ist Vorbedingung — Schritt 2 vor Schritt 1 bricht jeden Server, der heute auf eine geerbte Variable baut.Am Code hat sich seit dem Kommentar oben nichts bewegt:
server_config.cr:232nimmtenv:weiter wörtlich,protocol.cr:206spawnt weiter ohneclear_env.Ein Befund aus der Triage von #114, der hierher gehört: die stderr-Ausgabe, für die dieses Issue seinen Zielkonflikt überhaupt in Kauf nimmt, ist bei einem sofort endenden Server zeitabhängig da oder nicht da — also genau im häufigsten Fall eines kaputt konfigurierten Servers. #114 zuerst macht die Abwägung hier erst belastbar; solange sie offen ist, wird ein diagnostischer Wert verteidigt, den es auf diesem Pfad nicht zuverlässig gibt.
- added a commit that references this issue
on Sep 10, 2026 Schritt 1 ist als #122 umgesetzt (PR #127):
${VAR}inenv:eines stdio-Servers. Damit ist die Vorbedingung erfüllt — ein Server kann ein Geheimnis jetzt ausdrücklich bekommen, statt es aus der geerbten Umgebung zu ziehen.Beim Review von #127 kamen zwei Punkte auf, die hierher gehören, weil sie erst mit
clear_envscharf werden:1. „Nicht gesetzt" wird zu „leer", und das ist bei
envetwas anderes als bei einem HeaderEin leerer Header ist wirkungslos. Eine leere Umgebungsvariable ist für das lesende Programm nicht dasselbe wie eine fehlende:
PYTHONPATH=""legt das Arbeitsverzeichnis auf den Importpfad- ein leeres
PATH,HOMEoderNODE_OPTIONSist ein anderes Programm als ein nicht gesetztes
Heute steht hinter der Warnung noch der geerbte Wert —
{"env": {"PYTHONPATH": "${MY_LIBS}"}}mit ungesetztemMY_LIBSübergibt zwar"", aber das Kind hätte ohnehin geerbt. Mitclear_envfällt dieses Netz weg. Dann ist die leere Variable das Einzige, was ankommt.Zu entscheiden, bevor Schritt 2 gebaut wird: ob eine nicht gesetzte Variable den Schlüssel weglassen sollte, statt ihn leer zu setzen. Das ist die Form, die die meisten
${VAR}-Implementierungen wählen. #122 hat es bewusst nicht geändert, weil die Spezifikation dort „leer übergeben" verlangt und der geerbte Wert das Risiko heute abfedert.2.
.smith/mcp.jsoneines Projekts ist nicht vertrauensgeprüft —clear_envallein erfüllt das Akzeptanzkriterium nichtcli.cr:613-618ruftdiscoverbedingungslos auf; derTrustStoredeckt Hooks ab, nichtmcp.json. Ein geklontes Repository bringt sein.smith/mcp.jsonalso ungefragt mit.Solange
${VAR}jeden beliebigen Namen nennen darf, führt{"env": {"X": "${OPENROUTER_API_KEY}"}}den Key durch genau das Loch, dasclear_envgerade geschlossen hat — ausdrücklich und ohne Warnung, weil die Variable ja gesetzt ist.Das ist keine neue Fähigkeit: dieselbe Datei kann heute schon
commandauf irgendetwas setzen, undclear_envändert daran nichts. Aber es heißt, dass das Akzeptanzkriterium hier — „Ein MCP-Server, der seine Umgebung nach stderr schreibt, bringt keinen Provider-Key in die Ausgabe" — mitclear_envallein nicht erfüllt ist, solange die Expansionsquelle unbeschränkt bleibt. Die Basis-Allowlist muss also vermutlich zwei Seiten haben: was durchgereicht wird, und was${VAR}überhaupt nennen darf.- added a commit that references this issue
on Sep 10, 2026 Korrektur zu Punkt 1 meines vorigen Kommentars. Ich hatte geschrieben, hinter der leeren Variablen stehe heute noch der geerbte Wert und
clear_envnehme dieses Netz weg. Das ist falsch, und zwar in die unangenehme Richtung — es lässt den heutigen Stand sicherer aussehen, als er ist.Nachgemessen, Elternprozess mit
PYTHONPATH=/opt/inherited-libs:explizit {"PYTHONPATH" => ""} -> Kind sieht [] set-but-empty: 1 dasselbe mit clear_env: true -> Kind sieht [] {"PYTHONPATH" => nil} -> absent: <ABSENT>Ein expliziter
env-Eintrag überschreibt den geerbten Wert. Eine leere Expansion fällt also nicht auf ihn zurück, sie ersetzt ihn. Undclear_envliefert exakt dasselbe Ergebnis — es nimmt hier nichts weg.Was
clear_envwegnimmt, ist der Rückfall für eine Variable, die gar kein Eintrag nennt. Das ist eine andere Gefahr, und ich hatte beide vermengt.Die Folge ist das Gegenteil dessen, was ich geschrieben hatte: das ist kein Risiko, das mit #109 erst entsteht. Es ist seit #122 live, für jeden, der einen Eintrag mit einer nicht gesetzten Variablen schreibt. Für #109 heißt das nur, dass die Entscheidung nicht auf
clear_envwarten muss.Die dritte Zeile oben ist der Grund, warum die Alternative billig ist: Crystals
Processliest einen nil-Wert als „diese Variable nicht setzen". Den Schlüssel wegzulassen statt ihn leer zu setzen kostet also,ServerSpec#envaufHash(String, String?)zu erweitern —spawn_serverreicht die Map unverändert durch.Punkt 2 des vorigen Kommentars (
.smith/mcp.jsonist nicht vertrauensgeprüft,${VAR}darf jeden Namen nennen) steht unverändert.- added a commit that references this issue
on Sep 10, 2026
Beim Review von #92 (
smith doctor) gefunden. Bewusst dort nicht mitbehoben, weil die Abwägung eine andere ist als beim Diagnose-Command.Ist-Zustand
ServerHandle#failure_messagehängtstderr_tail.last(3)an die Fehlermeldung, undlist_mcp_serversgibt diese Meldung aus. Gleichzeitig startetStdioTransport.spawn_serverdas Kind ohneclear_env— es erbt also die komplette Umgebung des Elternprozesses, inklusive aller API-Keys.Ein Server, der beim Scheitern seine Umgebung nach stderr schreibt, bringt den Key damit auf stdout:
{"envecho": {"command": "/bin/sh", "args": ["-c", "env | grep -o 'OPENROUTER_API_KEY=.*' >&2; exit 1"]}}smith doctor(#92) hat das für sich gelöst:ServerHandlestellt dort einerror_summaryohne stderr bereit, undServerSpec#safe_descriptionkürzt Argumente und URLs.Warum es hier trotzdem offen ist
Bei
smith mcp listfragt man genau danach, warum ein Server nicht startet — und die eigene Fehlermeldung des Servers ist die Antwort. Sie wegzulassen nimmt dem Command seinen Zweck. Der Fall ist damit ein echter Zielkonflikt, kein Versehen.Denkbare Auflösungen
clear_envplus eine ausdrückliche Liste durchgereichter Variablen). Das ist die Wurzel: ein MCP-Server hat keinen Grund, die Provider-Keys des Harness zu sehen. Berührt allerdings Server, die heute auf geerbte Variablen bauen — braucht deshalb einen Weg, Variablen bewusst durchzureichen.smith mcp list --verbose).Die erste Variante ist die einzige, die auch schützt, wenn der Server seine Umgebung woandershin schreibt.
Akzeptanzkriterien
smith mcp listcrystal specgrün,crystal tool format --checksauber