fix(server): warn that a declared MCP server's env reaches the claude command line (#691) - #702
Merged
Merged
Conversation
… command line (#691) Follow-up to #691 step 6 (#701). `mcp-servers.ts` keeps a resolved credential out of every surface paddock owns — the boot log, an error, the Settings API — and that is complete. It is not the whole story, because of what happens downstream: // @herdctl/core, runner/runtime/cli-runtime.js const mcpServers = transformMcpServers(options.agent.mcp_servers); args.push("--mcp-config", JSON.stringify({ mcpServers })); // env and all A process argument is not private on Linux — /proc/<pid>/cmdline is world-readable with no hidepid, and ps prints it — so on the CLI runtime every declared server's `env` is legible to any local user for the lifetime of each `claude` invocation. The SDK runtime does not do this: it hands the same record to the SDK in-process and the spawned stdio server gets the value in its ENVIRONMENT, where /proc/<pid>/environ is owner-only, which is what Claude Code itself does. Which one runs is driveMode: `session` (default) is the SDK, `batch` is the CLI. Paddock cannot close this from here — the fix is upstream, where the Claude CLI's --mcp-config also accepts a file path — and refusing to attach the server would break the feature for the deployments most likely to need it. So it does what step 5 does about the fields the engine's schema cannot carry: refuses to be silent. An instance on `batch` with a credential-carrying declared server gets a WARNING naming the server; one on `session` gets the same as an info note, because one project pinning `driveMode: batch` brings the exposure back. Observed, not inferred: `test/integration/declared-mcp-argv.test.ts` drives a real turn through the CLI runtime and reads the token back out of the spawned process's argv (the fake `claude` now records `--mcp-config` and `--allowedTools` alongside the flags it already recorded). It is a CHARACTERISATION test — if that assertion ever fails, the engine has stopped doing this and the test and the warning should both be deleted rather than "fixed". The same test pins the other half in the opposite direction: `mcp__notion__*` reaches the same argv, which is what makes an attached server callable at all. Co-Authored-By: Claude <noreply@anthropic.com>
Deploying paddock with
|
| Latest commit: |
d31d71c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3e7daec2.paddock-7u2.pages.dev |
| Branch Preview URL: | https://feat-691-mcp-argv-exposure.paddock-7u2.pages.dev |
Merged
edspencer
pushed a commit
that referenced
this pull request
Aug 6, 2026
…ntee page #702 merged while this branch was open. env:VAR_NAME keeps a declared MCP server's credential out of the git-tracked config file; under driveMode: batch it does not keep it out of `ps`, because the engine serialises the whole definition into a --mcp-config argv element and /proc/<pid>/cmdline is world-readable. The default driveMode: session passes the record in-process, where the server gets its env owner-readable — what Claude Code itself does. #702 documented this on config-file.md. Three other places I had written the unqualified "tokens stay out of the file" claim now carry the limit too: the 0.62 What's New entry, the new what-Paddock-touches page (which gains a short section on credentials you hand Paddock), and the CLI --help. Documented as a property of driveMode: batch, not as a Paddock bug — the fix is upstream, and #702 ships a characterisation test that will fail if herdctl learns to pass --mcp-config as a file path. Co-Authored-By: Claude <noreply@anthropic.com>
edspencer
pushed a commit
that referenced
this pull request
Aug 6, 2026
herdctl 5.32.0 carries `headers` verbatim, so an `Authorization` bearer now rides in the same `--mcp-config` argv element #702 read an `env` token out of — and a bearer is the likelier long-lived credential of the two. A url server declared with headers and no `env` was the one shape that warning missed. Co-Authored-By: Claude <noreply@anthropic.com>
edspencer
added a commit
that referenced
this pull request
Aug 6, 2026
…ite audit (#703) * docs(website): a 0.62 What's New entry and a page for what Paddock touches The release that split one Claude-home lever into five independent keys had no What's New entry, and no page answering the question every incident behind it was really about: what does this thing actually read and write on my machine? The entry leads with the action rather than the mechanism. `instructions` defaults to `own`, so a curated ~/.claude/CLAUDE.md stops reaching agents on the CLI paths — and the startup notice that says so is written at `info`, which the npx launcher's quiet default filters out. Nobody is going to be told, so the docs have to be what tells them. Adds the `Reading older entries` aside the house style assumes exists, and marks 0.61.1's `--isolated-claude-home` advice as superseded rather than rewriting the entry. Co-Authored-By: Claude <noreply@anthropic.com> * docs(cli): list the fifth sharing lever in --help "Sharing your Claude Code state" listed transcripts, credentials, instructions and hooks, and omitted claude.mcpServers — which shipped alongside them. Adds it, plus a line for the sibling top-level mcpServers: block, which is the answer for an instance whose machine has no servers to borrow. Help text only. Co-Authored-By: Claude <noreply@anthropic.com> * docs: correct claims the v0.62 code contradicts An audit of all 48 pages against packages/server/src. The corrections, in rough order of blast radius: - The transcript home is <dataDir>/claude-home/projects/<enc>, not ~/.claude/projects/<enc>. Eight pages said the latter, including two giving operators a volume path to persist, and the storage figure. - A released chat under transcripts: host is still LISTED. config-file.md said it leaves the list; a passing test in the shipped code asserts the opposite (#693). - config-file.md said Paddock names the instructions key at startup so "you are told where". That notice is level info and the npx launcher sets LOG_LEVEL=warn, so on the common path it is filtered out. - README and NPM-README's docker run snippets omit PADDOCK_DANGEROUSLY_ALLOW_OPEN=1 and cannot start: the image binds 0.0.0.0, auth defaults to none, and the bind guard refuses. NPM-README also published on a routable address. - Dangling citations: ensureKeeperModel and KEEPER_DEFAULT_MODEL do not exist (ensureAgentModel / DEFAULT_MODEL); cfg.keeperDriveMode is cfg.driveMode; a config example used the pre-0.50 keeperDriveMode key. - overview.md warned that QueuedMessageStore's key separator is a space "unlike the others". It is NUL, like the others. Callout deleted. - environment.md claimed the Management API has no PADDOCK_* variables of its own. PADDOCK_MANAGEMENT_TRUSTED_PROXIES exists and was undocumented. - "hooks" now means two unrelated things; concepts/ and reference/ hooks pages now disambiguate from claude.hooks. - agent-capabilities said no config changes the toolset. Two keys widen it. - instance-settings omitted the five lever rows, and the deliberate absence of a row for the secret-bearing mcpServers: block. - self-mcp did not say paddock/paddock_manage are reserved names. Co-Authored-By: Claude <noreply@anthropic.com> * docs: carry #702's argv exposure into the release notes and the guarantee page #702 merged while this branch was open. env:VAR_NAME keeps a declared MCP server's credential out of the git-tracked config file; under driveMode: batch it does not keep it out of `ps`, because the engine serialises the whole definition into a --mcp-config argv element and /proc/<pid>/cmdline is world-readable. The default driveMode: session passes the record in-process, where the server gets its env owner-readable — what Claude Code itself does. #702 documented this on config-file.md. Three other places I had written the unqualified "tokens stay out of the file" claim now carry the limit too: the 0.62 What's New entry, the new what-Paddock-touches page (which gains a short section on credentials you hand Paddock), and the CLI --help. Documented as a property of driveMode: batch, not as a Paddock bug — the fix is upstream, and #702 ships a characterisation test that will fail if herdctl learns to pass --mcp-config as a file path. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: HomeLab Agent <homelab-infra@valfenda.net> Co-authored-by: Claude <noreply@anthropic.com>
edspencer
added a commit
that referenced
this pull request
Aug 6, 2026
… sse (#700) (#705) * feat(server): inherit host Claude Code plugins, carry MCP headers and sse (#700) Bump `@herdctl/core` to 5.32.0 and consume both capabilities herdctl#446 added. **Plugins.** A host plugin that provides an MCP server was invisible in Paddock on every setting: the SDK enables a discovered plugin from `enabledPlugins`, a user-settings-source key, and herdctl invokes every agent with `setting_sources: ["project"]`. `claude-plugins.ts` enumerates the host's installed plugin directories from the CLI's own `plugins/installed_plugins.json` registry and passes them as `agent.plugins`, which is a *session* plugin and needs no settings-source grant. Gated by `claude.instructions` rather than `claude.mcpServers` as #700 assumes: `plugins/` is bridged by `instructions`, alongside `agents/` and `commands/`, and under `instructions: own` Paddock already prints "your ~/.claude plugins are NOT loaded". `claude.mcpServers` decides only whether the plugins' own servers come too, via the SDK's `skipMcpDiscovery`. Each plugin server's allowlist pattern is derived, not read: the CLI registers a plugin's servers as `plugin:<plugin>:<server>` and normalises the name, so the pattern is `mcp__plugin_<plugin>_<server>__*` — matching the SDK's own documented `mcp__plugin_documents_docs__doc_export`. Using the declared name would have been silently wrong, and a missing pattern auto-denies every call with no prompt and nothing in the logs. **MCP server fields.** 5.32.0 carries `headers` and an explicit `type` verbatim, verified against the installed package through `addAgent` → `getAgents()` → `toSDKOptions()`. So #699's two stripping warnings are removed and both fields are passed on — which is what lets an OAuth server's stored token be found, as its key is a hash of `{type, url, headers}`. The instance's own `mcpServers:` block accepts both too, with `env:VAR` resolution and the never-print rule applied to header values. Co-Authored-By: Claude <noreply@anthropic.com> * fix(server): collapse duplicate plugin install paths before passing them One plugin can be recorded under several scopes in installed_plugins.json, and --plugin-dir'ing the same path twice loads it twice under one name. Co-Authored-By: Claude <noreply@anthropic.com> * fix(server): widen #702's argv-exposure warning to `headers` herdctl 5.32.0 carries `headers` verbatim, so an `Authorization` bearer now rides in the same `--mcp-config` argv element #702 read an `env` token out of — and a bearer is the likelier long-lived credential of the two. A url server declared with headers and no `env` was the one shape that warning missed. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: HomeLab Agent <homelab-infra@valfenda.net> Co-authored-by: Claude <noreply@anthropic.com>
edspencer
added a commit
that referenced
this pull request
Aug 8, 2026
… releases (#762) * chore(docs): start the 0.56-0.66 What's New pass * docs(website): split What's New, archiving 0.52 and older The page had grown to 1,150 lines and 30 releases, which is past the point where anyone scrolls it. Everything from 0.52 back to 0.29 moves verbatim to a new 'What's New — earlier releases' page; the main page keeps 0.53 and newer. Entries are moved unchanged — the archive is append-only and its entries are never rewritten, which is the same promise the page already makes about describing each release as it shipped. Image paths are unchanged because both pages sit in the same content directory. Adds the sidebar entry (Starlight does not auto-discover, so an unlisted page is invisible), a cross-link in both directions, and a note in the maintainer footer describing how to trim next time. * docs(website): What's New for 0.63 through 0.66.1 Five new entries. 0.66.0 leads on the Config screen redesign (#740) — the largest UX change in the range and the one a user meets every time — with a screenshot of the rail, live filter, env-override legend and the dirty dot. Also carries the breaking default-port move 4000 -> 7233 (#741) with the operator action spelled out, schemaVersion (#735), the import -> adopt rename (#748), the destructive-op interlock (#743) and the sub-agent bar fixes (#750). 0.66.1 is the queued-message release (#751): three silent-loss paths and the Stop-hands-it-back decision. 0.65 is promote_project (#668). 0.64 leads on path:/managed replacing repoBacked (#709) plus the Changes tab finally reporting on the checkout (#597). 0.63 is host plugin inheritance and MCP fidelity (#705), including the batch-mode argv credential disclosure (#702). Two claims deliberately qualified against the changelog's framing: the delete-then-send fix (#742) is called out as batch-only, since the default session mode was never affected; and the sub-agent bar is not described as absolute, because the settle heuristic can hold an interrupted sub-agent for ten minutes. Also corrects a live error: the 0.59.1-0.60 entry claimed in bold that --here leaves your ~/.claude alone. That was false for exactly those versions — --here linked ~/.claude/projects/<dir> at the workspace, and one report lost 30 transcripts to it before 0.61.1 stopped it. Replaced with a caution box pointing forward. * docs(website): a recorded demo of the 0.66 Config screen The filter and the rail only read in motion, so the entry gets both a still (rail counts, env chips, legend, dirty dot) and a 22s clip: filter by env-var name, Modified-only lens, then a rail jump that scrolls rather than swapping tabs. Shot on a v0.66.1 rig with no credentials. Two leaks were caught by looking at the frames rather than trusting the launcher: PADDOCK_GIT_AUTHOR_EMAIL was inherited and put a real internal address in the Git identity section, and the Advanced section printed the rig's scratch paths. The launcher is now an ALLOW-list — it drops every inherited PADDOCK_* and sets only what it needs — which is the fix issue #567 argues for, and the frames now show stock defaults and a ~/.paddock install path. * docs(website): backfill 0.56, 0.57, 0.58 and 0.61.0 These four were never written up — a gap in the middle of the page, not a tail. 0.61.0 in particular was only reachable by reading the 0.62 entry backwards, where it appears as the thing being removed. 0.61.0 leads on Paddock taking ownership of its Claude home, with a caution box narrating the whiplash: 0.61.0 isolates the home, 0.61.1 un-isolates the CLI because an isolated home cannot see a macOS Keychain login, and 0.62 replaces the mechanism entirely. Without that sentence the three entries read as contradicting each other. 0.58 carries its own caution: everything in it shipped in a CLI that, installed through npm, printed nothing and exited zero, and stayed that way across 0.57 and 0.59.0 until 0.59.1. An entry recommending it without that note would be recommending a broken install. The cause is described only as the run-directly guard, not attributed to a specific PR, because the changelog's own attribution does not line up with the release it shipped in. 0.57 is the environment system prompt, with the audit numbers that justified it and a note that three candidate rules were measured and cut. 0.56 is the npm publish, plus the correction that the claude CLI was never a prerequisite for chats. Also corrects 'several hundred boot log lines' in the 0.59.1-0.60 entry to match the changelog's actual figure (about thirty, down to nine). * docs(website): cut the new What's New entries roughly in half The nine entries added this pass ran to 314 lines. They read as changelog archaeology rather than release notes — explaining how each bug worked before saying what changed, and carrying detail nobody scanning a What's New page needs. Now 180 lines, of which 12 are the image and video markup. Each bullet is one idea in three or four lines: what a user notices first, then only the context that makes it land. What survives unchanged is the material that is genuinely load-bearing — the port change's operator action, the 'tabs partition' argument, the batch-only qualifier on the delete-then- send fix, and the two caution boxes, since those exist to stop a reader believing something false. * docs(website): plainer titles, and another pass on length Titles were making claims rather than saying what changed — 'Nothing you typed while it was busy goes missing', 'A Config screen you can navigate', 'A command-line worth running'. They now name the subject: Queued messages; Config screen, and a new default port; CLI output and flags; Environment system prompt; npm package. Prose down from 168 lines to 137 (originally 302). Mostly by cutting restatement — 0.61.0's four small turn-level fixes were four bullets saying the same shape of thing and are now one. * docs(website): bring the older entries into the same style Retitles every remaining entry to name its subject rather than make a claim, and cuts them to the same length as the new ones. 0.62's title becomes 'Granular host Claude inheritance options' as requested. Five levers instead of one -> Granular host Claude inheritance options Your ~/.claude, left alone and ... -> CLI login, and symlinks into your Claude home One command, on your own history -> npx install, --here, and confirmed adoption Bring your terminal history with you -> Adopting Claude Code CLI chats Claude, not "the keeper" -> The "keeper" rename, and Home's attention feeds Home says what it's holding -> Unread badge on the Home link Lengths: 0.62 115->51, 0.61.1 57->36, 0.59.1-0.60 45->29, 0.55 81->29, 0.54 70->42, 0.53 62->14. The page is 379 lines, down from 1,150. Everything load-bearing is kept: the breaking env-var table, the instructions: own warning, the leftover-symlink check with its shell command, and both caution boxes. All five media assets verified present in the built output. Frontmatter description rewritten — it still described 0.62 as 'five independent levers'. --------- Co-authored-by: HomeLab Agent <homelab-infra@valfenda.net>
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
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.
A follow-up to #701 (#691 step 6), which merged while this was being written. Not a re-do of it — one finding it does not cover, plus the test that proves the finding is real.
The finding
mcp-servers.tskeeps a resolved credential out of every surface Paddock owns: the boot log, error messages, the Settings API. That is complete and it holds. It is still not the whole story, because of what the engine does with the record afterwards:A process argument is not private on Linux.
/proc/<pid>/cmdlineis world-readable with nohidepid(this box included — checked/proc/mounts), andpsprints it. So on the CLI runtime every declared server'senv— the API token a user carefully passed byenv:VAR_NAMEreference so it would not be written to a file — is legible to any local user for the lifetime of eachclaudeinvocation.The SDK runtime does not do this. It hands the same record to the SDK in-process, and the stdio server it spawns receives the value in its environment, where
/proc/<pid>/environis owner-only. That is exactly what Claude Code itself does, so there is nothing to fix on that path.Which one runs is
driveMode:session(the default) is the SDK,batchis the CLI.Observed, not inferred
test/integration/declared-mcp-argv.test.tsdrives a real turn through the real CLI runtime with a server declared inpaddock.config.yamlwhose token comes from anenv:VAR_NAMEreference, and reads the token back out of the spawned process's argv. The fakeclaudenow records--mcp-configand--allowedToolsalongside the flags it already recorded (test/bin/claude, ~5 lines).It is a characterisation test, and the header says so: it pins behaviour Paddock does not want and cannot fix from here. If that assertion ever starts failing, that is good news — the engine has learned to pass
--mcp-configas a file path (the Claude CLI accepts one) — and the right response is to delete the test and the warning, not to "fix" it.The same test pins the other half in the opposite direction:
mcp__notion__*reaches the same argv, which is what makes an attached server callable at all rather than silently refused. Both halves of step 6 are now observable end-to-end, on a real spawn, in one test.What this changes
Nothing about what is attached, and nothing about what is logged for an instance that declares no
envvalues.declaredMcpNoticesgrows an optionaldriveMode, and when it is passed:driveMode: batch+ a declared server withenv→ warning naming the server;driveMode: session+ the same → the same text as an info note, because one project pinningdriveMode: batchfor itself brings the exposure back and the instance-level default does not say so;envvalues, or nodriveModepassed → nothing, so every existing caller and test is unchanged.Names only, never a value — the notice goes through the same discipline as the rest of the module, asserted in the unit test.
Refusing to attach the server instead would break the feature for exactly the deployments most likely to need it (a container, a server, an instance somebody else runs). So this takes step 5's posture towards what the engine's schema cannot carry: pass it, and refuse to be silent about it.
Why not upstream instead
It probably should be, and this does not block that: the Claude CLI's
--mcp-configaccepts a file path, socli-runtime.jscould write the JSON to a mode-0600 temp file and pass the path. Worth an issue onedspencer/herdctlalongside #445; I have not filed one. Until it exists, an instance onbatchdeserves to know.Note the exposure is not specific to declared servers — a host server inherited under
claude.mcpServers: hostwith anenvblock is passed the same way. This warns only about the declared ones, because those are the ones a user was told to hand a token to.Tests
Server 1846 → 1849 (+3, +1 file), web 941 unchanged.
env -u NODE_ENV npm testgreen; every fixture synthetic, the token exists only in the two files that assert on it, and nothing here starts an MCP server.npm run typecheckfails onreapChatSessioninherdctl.ts—@herdctl/core@5.29.1installed against a declared^5.31.0, present onmainuntouched, and it also blocks local E2E. CI installs fresh.🤖 Generated with Claude Code