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

3 Commits

Author SHA1 Message Date
Michal
0793285105 chore: ignore .claude/worktrees
Some checks failed
CI/CD / lint (pull_request) Successful in 1m13s
CI/CD / test (pull_request) Successful in 1m29s
CI/CD / typecheck (pull_request) Successful in 2m54s
CI/CD / smoke (pull_request) Failing after 2m0s
CI/CD / build (pull_request) Successful in 6m22s
CI/CD / publish (pull_request) Has been skipped
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.
2026-08-20 23:33:26 +01:00
Michal
370fd0a034 feat(secrets): opt-in injected secret delivery, scoped per server
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
2026-08-20 23:33:20 +01:00
Michal
f8427959e5 test(secrets): cover per-server identity containment
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
2026-08-20 23:13:49 +01:00