Refuse long-lived server startup with missing secrets

pkg/gateway/activateprofile.go

@@ -70,9 +70,10 @@ func (g *Gateway) convertWorkingSetToConfiguration(ctx context.Context, ws worki
 		// Build secrets configs
 		namespace := ""
 		configs = append(configs, ServerSecretConfig{
-			Secrets:   server.Snapshot.Server.Secrets,
-			OAuth:     server.Snapshot.Server.OAuth,
-			Namespace: namespace,
+			Secrets:                server.Snapshot.Server.Secrets,
+			OAuth:                  server.Snapshot.Server.OAuth,
+			Namespace:              namespace,
+			RequireVerifiedSecrets: localLongLivedServer(server.Snapshot.Server, g.LongLived),
 		})
 
 		// Add tools

pkg/gateway/clientpool.go

@@ -83,6 +83,21 @@ func (cp *clientPool) longLived(serverConfig *catalog.ServerConfig, config *clie
 	return serverConfig.Spec.LongLived || cp.LongLived
 }
 
+func (cp *clientPool) missingRequiredSecrets(serverConfig *catalog.ServerConfig, config *clientConfig) []string {
+	if serverConfig.IsRemote() || !cp.longLived(serverConfig, config) {
+		return nil
+	}
+
+	var missing []string
+	for _, secret := range serverConfig.Spec.Secrets {
+		if value, ok := serverConfig.Secrets[secret.Name]; !ok || value == "" {
+			missing = append(missing, secret.Name)
+		}
+	}
+
+	return missing
+}
+
 func (cp *clientPool) AcquireClient(ctx context.Context, serverConfig *catalog.ServerConfig, config *clientConfig) (mcpclient.Client, error) {
 	var getter *clientGetter
 	c := ctx
@@ -101,6 +116,10 @@ func (cp *clientPool) AcquireClient(ctx context.Context, serverConfig *catalog.S
 
 	// No client found, create a new one
 	if getter == nil {
+		if missing := cp.missingRequiredSecrets(serverConfig, config); len(missing) > 0 {
+			return nil, fmt.Errorf("required secret(s) missing for long-lived server %q: %s", serverConfig.Name, strings.Join(missing, ", "))
+		}
+
 		// If the client is long running, save it for later
 		if cp.longLived(serverConfig, config) {
 			// Double-checked locking: re-check under write lock to avoid duplicate containers

pkg/gateway/clientpool_test.go

@@ -119,6 +119,20 @@ secrets:
 	assert.Equal(t, []string{"DB_PASSWORD=my-actual-db-password", "API_KEY=my-actual-api-key"}, env)
 }
 
+func TestApplyConfigShortLivedMissingSecretKeepsPlaceholder(t *testing.T) {
+	catalogYAML := `
+secrets:
+  - name: short.api_key
+    env: API_KEY
+`
+
+	args, env := argsAndEnv(t, "short", catalogYAML, "", nil)
+
+	assert.Contains(t, args, "-e")
+	assert.Contains(t, args, "API_KEY")
+	assert.Equal(t, []string{"API_KEY=<UNKNOWN>"}, env)
+}
+
 func TestApplyConfigMountAs(t *testing.T) {
 	hostPath := t.TempDir()
 	expectedHostPath, err := cleanDockerHostPath(hostPath)
@@ -658,6 +672,104 @@ func TestLongLivedRemoteServer(t *testing.T) {
 	assert.True(t, cp.longLived(remoteServer, cfg), "Remote server must always be long-lived")
 }
 
+func TestAcquireClientRefusesLongLivedServerWithMissingSecrets(t *testing.T) {
+	t.Setenv("PATH", t.TempDir())
+
+	session := &mcp.ServerSession{}
+	cp := &clientPool{
+		keptClients: make(map[clientKey]keptClient),
+	}
+
+	server := &catalog.ServerConfig{
+		Name: "apify-mcp-server",
+		Spec: catalog.Server{
+			Type:      "server",
+			Image:     "mcp/apify:latest",
+			LongLived: true,
+			Secrets: []catalog.Secret{
+				{Name: "apify-mcp-server.apify_token", Env: "APIFY_TOKEN"},
+			},
+		},
+		Config:  map[string]any{},
+		Secrets: map[string]string{},
+	}
+	cfg := &clientConfig{serverSession: session}
+
+	client, err := cp.AcquireClient(context.Background(), server, cfg)
+
+	require.Error(t, err)
+	assert.Nil(t, client)
+	assert.Contains(t, err.Error(), "required secret")
+	assert.Contains(t, err.Error(), "apify-mcp-server.apify_token")
+	assert.Empty(t, cp.keptClients)
+}
+
+func TestAcquireClientRefusesLongLivedServerWithEmptySecretValue(t *testing.T) {
+	t.Setenv("PATH", t.TempDir())
+
+	session := &mcp.ServerSession{}
+	cp := &clientPool{
+		keptClients: make(map[clientKey]keptClient),
+	}
+
+	server := &catalog.ServerConfig{
+		Name: "apify-mcp-server",
+		Spec: catalog.Server{
+			Type:      "server",
+			Image:     "mcp/apify:latest",
+			LongLived: true,
+			Secrets: []catalog.Secret{
+				{Name: "apify-mcp-server.apify_token", Env: "APIFY_TOKEN"},
+			},
+		},
+		Config: map[string]any{},
+		Secrets: map[string]string{
+			"apify-mcp-server.apify_token": "",
+		},
+	}
+	cfg := &clientConfig{serverSession: session}
+
+	client, err := cp.AcquireClient(context.Background(), server, cfg)
+
+	require.Error(t, err)
+	assert.Nil(t, client)
+	assert.Contains(t, err.Error(), "required secret")
+	assert.Contains(t, err.Error(), "apify-mcp-server.apify_token")
+	assert.Empty(t, cp.keptClients)
+}
+
+func TestAcquireClientRefusesGlobalLongLivedServerWithMissingSecrets(t *testing.T) {
+	t.Setenv("PATH", t.TempDir())
+
+	session := &mcp.ServerSession{}
+	cp := &clientPool{
+		Options:     Options{LongLived: true},
+		keptClients: make(map[clientKey]keptClient),
+	}
+
+	server := &catalog.ServerConfig{
+		Name: "apify-mcp-server",
+		Spec: catalog.Server{
+			Type:  "server",
+			Image: "mcp/apify:latest",
+			Secrets: []catalog.Secret{
+				{Name: "apify-mcp-server.apify_token", Env: "APIFY_TOKEN"},
+			},
+		},
+		Config:  map[string]any{},
+		Secrets: map[string]string{},
+	}
+	cfg := &clientConfig{serverSession: session}
+
+	client, err := cp.AcquireClient(context.Background(), server, cfg)
+
+	require.Error(t, err)
+	assert.Nil(t, client)
+	assert.Contains(t, err.Error(), "required secret")
+	assert.Contains(t, err.Error(), "apify-mcp-server.apify_token")
+	assert.Empty(t, cp.keptClients)
+}
+
 func TestReleaseClientsForSession(t *testing.T) {
 	sess1 := &mcp.ServerSession{}
 	sess2 := &mcp.ServerSession{}

pkg/gateway/configuration.go

@@ -310,6 +310,7 @@ type FileBasedConfiguration struct {
 	OciRef             []string         // OCI references to fetch server definitions from
 	MCPRegistryServers []catalog.Server // Servers fetched from MCP registries
 	Watch              bool
+	LongLived          bool
 	McpOAuthDcrEnabled bool
 
 	docker docker.Client
@@ -550,8 +551,9 @@ func (c *FileBasedConfiguration) readOnce(ctx context.Context) (Configuration, e
 		for _, serverName := range serverNames {
 			server := servers[serverName]
 			configs = append(configs, ServerSecretConfig{
-				Secrets: server.Secrets,
-				OAuth:   server.OAuth,
+				Secrets:                server.Secrets,
+				OAuth:                  server.OAuth,
+				RequireVerifiedSecrets: localLongLivedServer(server, c.LongLived),
 			})
 		}
 		return BuildSecretsURIs(ctx, configs)

pkg/gateway/configuration_workingset.go

@@ -87,9 +87,10 @@ func (c *WorkingSetConfiguration) readOnce(ctx context.Context, dao db.DAO) (Con
 		// 	namespace = server.Secrets + "_"
 		// }
 		configs = append(configs, ServerSecretConfig{
-			Secrets:   server.Snapshot.Server.Secrets,
-			OAuth:     server.Snapshot.Server.OAuth,
-			Namespace: namespace,
+			Secrets:                server.Snapshot.Server.Secrets,
+			OAuth:                  server.Snapshot.Server.OAuth,
+			Namespace:              namespace,
+			RequireVerifiedSecrets: localLongLivedServer(server.Snapshot.Server, c.config.LongLived),
 		})
 	}
 	secrets := BuildSecretsURIs(ctx, configs)

pkg/gateway/mcpadd.go

@@ -89,20 +89,17 @@ func addServerHandler(g *Gateway, clientConfig *clientConfig) mcp.ToolHandler {
 			}
 		}
 
-		// Append the new server to the current serverNames if not already present
 		alreadyEnabled := slices.Contains(g.configuration.serverNames, serverName)
-		if !alreadyEnabled {
-			g.configuration.serverNames = append(g.configuration.serverNames, serverName)
-		}
 
 		// Check if all required secrets are set
 		var missingSecrets []string
 		var availableSecrets map[string]string
 		if serverConfig != nil && len(serverConfig.Spec.Secrets) > 0 {
 			// BuildSecretsURIs only includes secrets that exist in Secrets Engine
 			configs := []ServerSecretConfig{{
-				Secrets: serverConfig.Spec.Secrets,
-				OAuth:   serverConfig.Spec.OAuth,
+				Secrets:                serverConfig.Spec.Secrets,
+				OAuth:                  serverConfig.Spec.OAuth,
+				RequireVerifiedSecrets: localLongLivedServer(serverConfig.Spec, g.LongLived),
 			}}
 			availableSecrets = BuildSecretsURIs(ctx, configs)
 
@@ -212,6 +209,12 @@ func addServerHandler(g *Gateway, clientConfig *clientConfig) mcp.ToolHandler {
 			}, nil
 		}
 
+		// Append the new server after validating required inputs so failed add
+		// attempts do not leave it enabled in memory.
+		if !slices.Contains(g.configuration.serverNames, serverName) {
+			g.configuration.serverNames = append(g.configuration.serverNames, serverName)
+		}
+
 		// Merge available secrets into configuration so container creation can find them.
 		// This is needed because g.configuration.secrets is built at startup and doesn't
 		// include secrets for dynamically added servers.

pkg/gateway/mcpadd_test.go

@@ -0,0 +1,47 @@
+package gateway
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/modelcontextprotocol/go-sdk/mcp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/docker/mcp-gateway/pkg/catalog"
+)
+
+func TestAddServerHandlerDoesNotAppendServerWhenValidationFails(t *testing.T) {
+	g := &Gateway{
+		configuration: Configuration{
+			serverNames: []string{"existing"},
+			servers: map[string]catalog.Server{
+				"needs-config": {
+					Image: "mcp/needs-config:latest",
+					Config: []any{
+						map[string]any{"name": "url", "type": "string"},
+					},
+				},
+			},
+			config: map[string]map[string]any{},
+		},
+	}
+	handler := addServerHandler(g, nil)
+	args, err := json.Marshal(map[string]any{"name": "needs-config"})
+	require.NoError(t, err)
+
+	result, err := handler(context.Background(), &mcp.CallToolRequest{
+		Session: &mcp.ServerSession{},
+		Params: &mcp.CallToolParamsRaw{
+			Arguments: args,
+		},
+	})
+
+	require.NoError(t, err)
+	require.Len(t, result.Content, 1)
+	text, ok := result.Content[0].(*mcp.TextContent)
+	require.True(t, ok)
+	assert.Contains(t, text.Text, "The server was not added")
+	assert.Equal(t, []string{"existing"}, g.configuration.serverNames)
+}

pkg/gateway/run.go

@@ -110,6 +110,7 @@ func NewGateway(config Config, docker docker.Client) *Gateway {
 			OciRef:             config.OciRef,
 			MCPRegistryServers: config.MCPRegistryServers,
 			Watch:              config.Watch,
+			LongLived:          config.LongLived,
 			McpOAuthDcrEnabled: config.McpOAuthDcrEnabled,
 			docker:             docker,
 		}

pkg/gateway/secrets_uri.go

@@ -10,9 +10,10 @@ import (
 
 // ServerSecretConfig contains the secret definitions and OAuth config for a server.
 type ServerSecretConfig struct {
-	Secrets   []catalog.Secret // Secret definitions from server catalog
-	OAuth     *catalog.OAuth   // OAuth config (if present, OAuth takes priority over direct secret)
-	Namespace string           // Prefix for map keys (used by WorkingSet for namespacing)
+	Secrets                []catalog.Secret // Secret definitions from server catalog
+	OAuth                  *catalog.OAuth   // OAuth config (if present, OAuth takes priority over direct secret)
+	Namespace              string           // Prefix for map keys (used by WorkingSet for namespacing)
+	RequireVerifiedSecrets bool             // Do not synthesize fallback URIs when existence cannot be checked.
 }
 
 // BuildSecretsURIs generates se:// URIs for secrets with OAuth priority handling.
@@ -21,9 +22,10 @@ type ServerSecretConfig struct {
 // actually exist in the store (OAuth token checked first, then direct secret).
 //
 // When the secrets engine is unreachable (e.g. MSIX-sandboxed clients on Windows
-// cannot follow AF_UNIX reparse points), URIs are generated for all declared secrets
-// since we cannot check existence. Docker Desktop resolves se:// URIs at container
-// runtime via named pipes, which are unaffected by MSIX restrictions.
+// cannot follow AF_UNIX reparse points), URIs are generated for declared secrets
+// unless their server requires verified secrets. Docker Desktop resolves se://
+// URIs at container runtime via named pipes, which are unaffected by MSIX
+// restrictions.
 func BuildSecretsURIs(ctx context.Context, configs []ServerSecretConfig) map[string]string {
 	allSecrets, err := secret.GetSecrets(ctx)
 	if err != nil {
@@ -38,13 +40,38 @@ func BuildSecretsURIs(ctx context.Context, configs []ServerSecretConfig) map[str
 	return buildVerifiedURIs(configs, availableSecrets)
 }
 
+func localLongLivedServer(server catalog.Server, globalLongLived bool) bool {
+	if server.Remote.URL != "" || server.SSEEndpoint != "" {
+		return false
+	}
+	return server.LongLived || globalLongLived
+}
+
 // buildFallbackURIs generates se:// URIs for all declared secrets without checking
 // existence. Used when the secrets engine is unreachable. OAuth URIs are preferred
 // when configured (matching the normal priority order).
 func buildFallbackURIs(configs []ServerSecretConfig) map[string]string {
 	secretNameToURI := make(map[string]string)
+	requireVerified := make(map[string]struct{})
 
 	for _, cfg := range configs {
+		if !cfg.RequireVerifiedSecrets {
+			continue
+		}
+		for _, s := range cfg.Secrets {
+			if err := secret.ValidateSecretName(s.Name); err != nil {
+				log.Logf("Warning: skipping secret with invalid name %q: %v", s.Name, err)
+				continue
+			}
+			requireVerified[cfg.Namespace+s.Name] = struct{}{}
+		}
+	}
+
+	for _, cfg := range configs {
+		if cfg.RequireVerifiedSecrets {
+			continue
+		}
+
 		secretToOAuthID := oauthMapping(cfg)
 
 		for _, s := range cfg.Secrets {
@@ -53,6 +80,9 @@ func buildFallbackURIs(configs []ServerSecretConfig) map[string]string {
 				continue
 			}
 			secretName := cfg.Namespace + s.Name
+			if _, ok := requireVerified[secretName]; ok {
+				continue
+			}
 			if oauthSecretID, ok := secretToOAuthID[s.Name]; ok {
 				secretNameToURI[secretName] = "se://" + oauthSecretID
 			} else {