feat(secrets): opt-in injected secret delivery, scoped per server #117

Merged

michal merged 3 commits from feat/per-server-identity-tests into main 2026-08-20 22:33:56 +00:00

Conversation 0 Commits 3 Files Changed 24 +860 -14

michal commented 2026-08-20 22:33:49 +00:00

Owner

Copy Link

Stops mcpd writing secret VALUES into pod specs. With secretDelivery: injector the pod fetches its own secrets from OpenBao under a ServiceAccount + role scoped to just that server. Opt-in per server, default env, so existing servers are unchanged.

Key guards, both tested: an opted-out server produces an identical manifest; an opted-in server still fails loudly on a bad secretRef (validateServerEnvRefs resolves and discards, preserving the e6cd735 check). Shell quoting is tested by executing a real /bin/sh over nine adversarial values and was confirmed to fail against naive quoting.

Docker/Podman fall back to inline resolution. Deleting a server revokes its identity.

Stops mcpd writing secret VALUES into pod specs. With secretDelivery: injector the pod fetches its own secrets from OpenBao under a ServiceAccount + role scoped to just that server. Opt-in per server, default env, so existing servers are unchanged. Key guards, both tested: an opted-out server produces an identical manifest; an opted-in server still fails loudly on a bad secretRef (validateServerEnvRefs resolves and discards, preserving the e6cd735 check). Shell quoting is tested by executing a real /bin/sh over nine adversarial values and was confirmed to fail against naive quoting. Docker/Podman fall back to inline resolution. Deleting a server revokes its identity.

michal added 3 commits 2026-08-20 22:33:49 +00:00

test(secrets): cover per-server identity containment ... f8427959e5

ServerIdentityService shipped without tests. Containment is the whole
point of the design, so it needs assertions rather than trust: a shared
role would let third-party MCP images (gitea-mcp, ha-mcp) read every
secret under secret/mcpctl/*, which is worse than the pod-spec exposure
it replaces.

Eight cases, all about what a server must NOT get: the grant covers only
the secrets that server declares; a secret referenced twice is one grant,
not two; inline env values never widen it; another server's secrets never
appear. Plus the ordering invariant (ServiceAccount before the role that
binds it — the reverse lets a pod start, fail to log in and crashloop
while the role is still being written), and that a backend which cannot
scope identities REFUSES rather than silently succeeding, which would
leave a pod believing it held access it never got.

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

feat(secrets): opt-in injected secret delivery, scoped per server ... 370fd0a034

Completes the path that stops mcpd writing secret VALUES into MCP server
pod specs. With `secretDelivery: injector`, the pod fetches its own
secrets from OpenBao through the agent injector, under a ServiceAccount
and role scoped to just that server's secrets — so the value never enters
etcd, and gitea-mcp cannot read the Grafana token.

Opt-in per server, defaulting to `env`. Every existing server is
bit-for-bit unchanged, and migrating is one reversible decision at a time
rather than a flag day.

The two invariants under most risk, both tested:

- **Opted-out servers produce an identical manifest.** No annotations, no
  serviceAccountName, automountServiceAccountToken still false.

- **Opted-in servers still fail LOUDLY on a bad ref.** Once mcpd stops
  reading a server's secrets, the check e6cd735 added no longer fires for
  it, and a typo'd secretRef would degrade into a vault-agent-init
  crashloop that mcpd reports as a generic pod failure — the same class of
  bug that had gitea-mcp running for weeks on an empty token while
  reporting healthy. `validateServerEnvRefs` resolves every ref and throws
  the value away, purely to keep that error. After the value cache it is a
  cache hit and costs nothing.

Shell quoting is the other silent-failure trap and is treated as part of
the contract: the agent renders `export NAME='value'` and the container
command sources it, so a value containing a space, `$`, a quote or a
newline would truncate and yield an empty token. `shellSingleQuote` is
tested by executing a real /bin/sh over nine adversarial values including
`'; export PWNED=1; '` — and those tests fail against naive quoting,
confirmed before keeping them.

`sh -c <script> arg0 arg1 …` preserves argv via $0/$@, and `exec` keeps
PID 1 as the real process, which matters because mcpd attaches to PID 1's
stdin/stdout for STDIO servers.

Docker/Podman declare `capabilities.secretRefs: false` and fall back to
inline resolution, so local development is untouched. Deleting a server
revokes its identity, after its pods are gone and best-effort — a role no
pod can authenticate as grants nothing, and failing the delete over it
would strand the row.

Per the CLI rules, `secretDelivery`/`entrypoint` are `create` flags,
round-trip through apply -f, and show in `describe server` — which now
also flags servers still inlining secrets into their pod spec.

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

chore: ignore .claude/worktrees ... 0793285105

Agent worktrees are scratch checkouts. Without this a plain `git add -A`
sweeps them in as embedded git repositories, which clone as empty
directories for everyone else.

michal added 2 commits 2026-08-20 22:33:49 +00:00

feat(secrets): opt-in injected secret delivery, scoped per server ... 370fd0a034

Completes the path that stops mcpd writing secret VALUES into MCP server
pod specs. With `secretDelivery: injector`, the pod fetches its own
secrets from OpenBao through the agent injector, under a ServiceAccount
and role scoped to just that server's secrets — so the value never enters
etcd, and gitea-mcp cannot read the Grafana token.

Opt-in per server, defaulting to `env`. Every existing server is
bit-for-bit unchanged, and migrating is one reversible decision at a time
rather than a flag day.

The two invariants under most risk, both tested:

- **Opted-out servers produce an identical manifest.** No annotations, no
  serviceAccountName, automountServiceAccountToken still false.

- **Opted-in servers still fail LOUDLY on a bad ref.** Once mcpd stops
  reading a server's secrets, the check e6cd735 added no longer fires for
  it, and a typo'd secretRef would degrade into a vault-agent-init
  crashloop that mcpd reports as a generic pod failure — the same class of
  bug that had gitea-mcp running for weeks on an empty token while
  reporting healthy. `validateServerEnvRefs` resolves every ref and throws
  the value away, purely to keep that error. After the value cache it is a
  cache hit and costs nothing.

Shell quoting is the other silent-failure trap and is treated as part of
the contract: the agent renders `export NAME='value'` and the container
command sources it, so a value containing a space, `$`, a quote or a
newline would truncate and yield an empty token. `shellSingleQuote` is
tested by executing a real /bin/sh over nine adversarial values including
`'; export PWNED=1; '` — and those tests fail against naive quoting,
confirmed before keeping them.

`sh -c <script> arg0 arg1 …` preserves argv via $0/$@, and `exec` keeps
PID 1 as the real process, which matters because mcpd attaches to PID 1's
stdin/stdout for STDIO servers.

Docker/Podman declare `capabilities.secretRefs: false` and fall back to
inline resolution, so local development is untouched. Deleting a server
revokes its identity, after its pods are gone and best-effort — a role no
pod can authenticate as grants nothing, and failing the delete over it
would strand the row.

Per the CLI rules, `secretDelivery`/`entrypoint` are `create` flags,
round-trip through apply -f, and show in `describe server` — which now
also flags servers still inlining secrets into their pod spec.

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

chore: ignore .claude/worktrees ... 0793285105

Agent worktrees are scratch checkouts. Without this a plain `git add -A`
sweeps them in as embedded git repositories, which clone as empty
directories for everyone else.

michal merged commit fef26a9f81 into main 2026-08-20 22:33:56 +00:00

michal referenced this issue from a commit 2026-08-20 22:33:59 +00:00

Merge pull request 'feat(secrets): opt-in injected secret delivery, scoped per server' (#117) from feat/per-server-identity-tests into main

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

michal

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: michal/mcpctl#117