fix(secrets): injector-delivered servers must attach, not exec #120

michal · 2026-08-21T01:05:52Z

michal commented

2026-08-21 01:05:52 +00:00

With injected delivery the credentials exist only in PID 1 env (a shell sourced /vault/secrets/ and exec-ed the server). mcpd selected exec for packageName/command servers, which spawns a NEW process with empty credentials — server starts, answers tools/list, fails every authed call.

secretDelivery: injector now forces attach. No bouncer needed: PersistentStdio already holds one long-lived connection, it was pointed at the wrong process.

Inverts an earlier assumption: image-entrypoint servers were already on attach and would have worked; package-based ones were broken. Extracts chooseStdioMode() as a pure function with 6 tests.

With injected delivery the credentials exist only in PID 1 env (a shell sourced /vault/secrets/<name> and exec-ed the server). mcpd selected `exec` for packageName/command servers, which spawns a NEW process with empty credentials — server starts, answers tools/list, fails every authed call. secretDelivery: injector now forces `attach`. No bouncer needed: PersistentStdio already holds one long-lived connection, it was pointed at the wrong process. Inverts an earlier assumption: image-entrypoint servers were already on attach and would have worked; package-based ones were broken. Extracts chooseStdioMode() as a pure function with 6 tests.

michal added 1 commit 2026-08-21 01:05:52 +00:00

fix(secrets): injector-delivered servers must attach, not exec

CI/CD / lint (pull_request) Successful in 1m17s

Details

CI/CD / typecheck (pull_request) Successful in 2m41s

Details

CI/CD / test (pull_request) Successful in 1m28s

Details

CI/CD / build (pull_request) Successful in 2m18s

Details

CI/CD / smoke (pull_request) Failing after 3m3s

Details

CI/CD / publish (pull_request) Has been skipped

Details

0a83f71648

With injected delivery the credentials exist ONLY in PID 1's environment:
the container command is a shell that sources /vault/secrets/<name> and
execs the real server, so the values live in that process and nowhere
else — not in the pod spec, which is the whole point.

mcpd picks its STDIO mode from the server's shape: an explicit command or
a packageName selects `exec`, a bare dockerImage selects `attach`. `exec`
spawns a NEW process inside the container, which never sourced the file
and therefore starts with empty credentials. The server comes up, answers
tools/list, and fails every authenticated call — precisely the silent
empty-token failure this feature exists to prevent.

Seen as `Readiness check (list_datasources) failed: process exited 1` on a
pod whose PID 1 demonstrably held the token (verified via /proc/1/environ:
GRAFANA_URL set, token 46 chars, nothing in the pod spec).

So secretDelivery: injector now forces attach regardless of server shape.
Safe because wrapCommandForInjector execs rather than forks, so PID 1 IS
the server. It also means no bouncer or HTTP shim is needed — mcpd's
PersistentStdio already holds one long-lived connection; it was simply
pointed at the wrong process.

Note this inverts an assumption I had earlier: image-entrypoint servers
were already on the attach path and would have worked; it is the
package-based ones that were broken. gitea remains the sole exception, and
for the unrelated reason that it is distroless and has no shell to source
the file.

Extracts the decision as a pure `chooseStdioMode()` so it can be tested —
it is subtle and fails silently. Six cases pinned, including that env
delivery still execs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018vybEitX4FykeMatKe5Xki

michal merged commit beb57baf58 into main

2026-08-21 01:05:54 +00:00

michal referenced this issue from a commit

2026-08-21 01:05:55 +00:00

Merge pull request 'fix(secrets): injector-delivered servers must attach, not exec' (#120) from feat/injector-attach-mode into main

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: michal/mcpctl#120