Skip to content

Refuse long-lived server startup with missing secrets#499

Open
jstar0 wants to merge 1 commit into
docker:mainfrom
jstar0:fix/refuse-missing-long-lived-secrets
Open

Refuse long-lived server startup with missing secrets#499
jstar0 wants to merge 1 commit into
docker:mainfrom
jstar0:fix/refuse-missing-long-lived-secrets

Conversation

@jstar0

@jstar0 jstar0 commented May 30, 2026

Copy link
Copy Markdown

Summary

Fixes #433.

Long-lived local MCP server containers can keep running with placeholder secret values if they are started before required secrets are available. Once that container is cached, later calls can continue using the stale environment even after the user stores the real secret.

This change makes local long-lived Docker servers require verified required secrets before a client/container is created. If the secrets engine cannot be queried, fallback se:// URIs are not synthesized for those long-lived local servers, so missing secrets remain visible and the gateway returns a clear error instead of caching a container with a placeholder value.

Changes

  • Skip unverified fallback secret URIs for local long-lived Docker servers.
  • Avoid backfilling those fallback URIs through another server that declares the same secret name.
  • Return an error before creating or caching a long-lived client when required secrets are missing.
  • Treat empty required secret values as missing for local long-lived Docker servers.
  • Keep failed mcp-add validation attempts from enabling the server in memory.
  • Preserve existing fallback/placeholder behavior for short-lived servers.
  • Preserve remote server behavior because remote servers do not receive local Docker container environment variables.
  • Add regression coverage for missing required secrets, fallback URI handling, global --long-lived, remote boundaries, and short-lived compatibility.

Risk

This is scoped to required secrets for local long-lived Docker servers. It does not add secret-store watching or container recreation on secret changes, and it does not change short-lived or remote server startup behavior.

Verification

go test ./pkg/gateway -run 'TestBuildFallbackURIsSkipsLongLivedLocalSecrets|TestBuildFallbackURIsKeepsShortLivedLocalSecrets|TestBuildFallbackURIsDoesNotBackfillSharedLongLivedSecrets|TestAcquireClientRefusesLongLivedServerWithMissingSecrets|TestAcquireClientRefusesLongLivedServerWithEmptySecretValue|TestAcquireClientRefusesGlobalLongLivedServerWithMissingSecrets|TestAddServerHandlerDoesNotAppendServerWhenValidationFails|TestApplyConfigShortLivedMissingSecretKeepsPlaceholder|TestLongLived' -count=1
go test ./pkg/gateway -count=1
go test ./pkg/gateway ./pkg/catalog ./pkg/workingset -count=1
go test -race ./pkg/gateway -run 'TestAcquireClientRefusesLongLivedServerWithMissingSecrets|TestAcquireClientNoDuplicatesUnderConcurrency|TestBuildFallbackURIsSkipsLongLivedLocalSecrets|TestBuildFallbackURIsDoesNotBackfillSharedLongLivedSecrets' -count=1
go test ./... -short -count=1

@jstar0 jstar0 requested a review from a team as a code owner May 30, 2026 08:49
@cutecatfann

Copy link
Copy Markdown
Contributor

All checks pass and this generally looks good, but misssing some verified signatures on the PR :)

@jstar0 jstar0 force-pushed the fix/refuse-missing-long-lived-secrets branch from bbc1d12 to 070949e Compare June 8, 2026 01:29
@jstar0 jstar0 force-pushed the fix/refuse-missing-long-lived-secrets branch from 070949e to 8acef6e Compare June 24, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Long-lived containers permanently retain stale secrets (<UNKNOWN>) after secret is stored

2 participants