Skip to content

Commit e984709

Browse files
committed
fix: fail when discover of auth creds fail
instead of falling back to unauthenticated, make the process fail when there is a problem in discovering auth config
1 parent 66fff4a commit e984709

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

pkg/vmcp/workloads/k8s.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ func (d *k8sDiscoverer) GetWorkloadAsVMCPBackend(ctx context.Context, workloadNa
9898
// Convert MCPServer to Backend
9999
backend := d.mcpServerToBackend(ctx, mcpServer)
100100

101+
// If auth discovery failed, mcpServerToBackend returns nil
102+
if backend == nil {
103+
logger.Warnf("Skipping workload %s due to auth discovery failure", workloadName)
104+
return nil, nil
105+
}
106+
101107
// Skip workloads without a URL (not accessible)
102108
if backend.BaseURL == "" {
103109
logger.Debugf("Skipping workload %s without URL", workloadName)
@@ -182,8 +188,11 @@ func (d *k8sDiscoverer) mcpServerToBackend(ctx context.Context, mcpServer *mcpv1
182188

183189
// Discover and populate authentication configuration from MCPServer
184190
if err := d.discoverAuthConfig(ctx, mcpServer, backend); err != nil {
185-
// Log warning but don't fail - backend can still be used without auth
186-
logger.Warnf("Failed to discover auth config for MCPServer %s: %v", mcpServer.Name, err)
191+
// If auth discovery fails, we must fail - don't silently allow unauthorized access
192+
// This is a security-critical operation: if auth is configured but fails to load,
193+
// we should not proceed without it
194+
logger.Errorf("Failed to discover auth config for MCPServer %s: %v", mcpServer.Name, err)
195+
return nil
187196
}
188197

189198
return backend

pkg/vmcp/workloads/k8s_test.go

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -255,11 +255,10 @@ func TestDiscoverAuth_AuthConfigNotFound(t *testing.T) {
255255
ctx := context.Background()
256256
backend, err := discoverer.GetWorkloadAsVMCPBackend(ctx, "test-server")
257257

258-
// Should still return the backend but without auth (logs warning)
258+
// Should return nil backend when auth config is referenced but not found
259+
// This is security-critical: fail closed rather than allowing unauthorized access
259260
require.NoError(t, err)
260-
require.NotNil(t, backend)
261-
assert.Empty(t, backend.AuthStrategy)
262-
assert.Nil(t, backend.AuthMetadata)
261+
require.Nil(t, backend, "Should return nil backend when auth config is missing")
263262
}
264263

265264
func TestDiscoverAuth_SecretNotFound(t *testing.T) {
@@ -312,11 +311,10 @@ func TestDiscoverAuth_SecretNotFound(t *testing.T) {
312311
ctx := context.Background()
313312
backend, err := discoverer.GetWorkloadAsVMCPBackend(ctx, "test-server")
314313

315-
// Should still return the backend but without auth (logs warning)
314+
// Should return nil backend when secret is missing
315+
// This is security-critical: fail closed rather than allowing unauthorized access
316316
require.NoError(t, err)
317-
require.NotNil(t, backend)
318-
assert.Empty(t, backend.AuthStrategy)
319-
assert.Nil(t, backend.AuthMetadata)
317+
require.Nil(t, backend, "Should return nil backend when secret is missing")
320318
}
321319

322320
func TestMCPServerToBackend_BasicFields(t *testing.T) {

0 commit comments

Comments
 (0)