Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions components/backend/handlers/integrations_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ func GetIntegrationsStatus(c *gin.Context) {
// GitLab status
response["gitlab"] = getGitLabStatusForUser(ctx, userID)

// MCP server credentials status
response["mcpServers"] = getMCPServerStatusForUser(ctx, userID)

c.JSON(http.StatusOK, response)
}

Expand Down
383 changes: 383 additions & 0 deletions components/backend/handlers/mcp_credentials.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,383 @@
package handlers

import (
"context"
"encoding/json"
"fmt"
"log"
"net/http"
"regexp"
"sort"
"strings"
"time"

"github.com/gin-gonic/gin"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

// MCPServerCredentials represents generic credentials for an MCP server
type MCPServerCredentials struct {
UserID string `json:"userId"`
ServerName string `json:"serverName"`
Fields map[string]string `json:"fields"`
UpdatedAt time.Time `json:"updatedAt"`
}

const mcpCredentialsSecretName = "mcp-server-credentials"

// validServerName matches lowercase alphanumeric with hyphens, max 63 chars
var validServerNameRegex = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,61}[a-z0-9]$`)

func isValidServerName(name string) bool {
if len(name) < 1 || len(name) > 63 {
return false
}
// Allow single character names
if len(name) == 1 {
return name[0] >= 'a' && name[0] <= 'z' || name[0] >= '0' && name[0] <= '9'
}
return validServerNameRegex.MatchString(name)
}

func mcpSecretKey(serverName, userID string) string {
return serverName + ":" + userID
Comment on lines +45 to +46
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

What characters are allowed in Kubernetes Secret.data keys in the core/v1 Secret API?

💡 Result:

In the core/v1 Secret API, the keys in Secret.data (and Secret.stringData) must consist only of:

  • Alphanumeric characters ([A-Za-z0-9])
  • Hyphen (-)
  • Underscore (_)
  • Dot (.)

Sources: Kubernetes docs and the Go API type comment for Secret.Data. [1] [2]


🏁 Script executed:

cat -n components/backend/handlers/mcp_credentials.go | head -100

Repository: ambient-code/platform

Length of output: 3623


🏁 Script executed:

cat -n components/backend/handlers/mcp_credentials.go | tail -200

Repository: ambient-code/platform

Length of output: 7927


Secret key format violates Kubernetes API specification and token auth must use request-scoped clients.

The colon (:) in mcpSecretKey() is not allowed in core/v1.Secret.data keys (only alphanumeric, hyphen, underscore, dot are valid). Additionally, storeMCPCredentials(), getMCPCredentials(), deleteMCPCredentials(), and getMCPServerStatusForUser() use the package-level backend service account (K8sClient) instead of passing request-scoped clients from GetK8sClientsForRequest(c), violating the requirement that user-facing API operations must use token auth. Finally, getMCPCredentials() returns a raw error when the Secret doesn't exist (line 313–316), causing a 500 response on first session fetch; it should normalize this to a "not found" state matching the key-not-found path (line 318–319).

Suggested fix
 import (
 	"context"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
 	"log"
@@
 func mcpSecretKey(serverName, userID string) string {
-	return serverName + ":" + userID
+	encodedUserID := base64.RawURLEncoding.EncodeToString([]byte(userID))
+	return serverName + "." + encodedUserID
 }
@@
-	suffix := ":" + userID
+	suffix := "." + base64.RawURLEncoding.EncodeToString([]byte(userID))

Also requires passing request-scoped clients (reqK8s) to helper functions and normalizing missing-Secret errors.

Also applies to: 371–375, 249–303, 305–328, 330–362, 364–382

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/mcp_credentials.go` around lines 45 - 46,
mcpSecretKey currently uses a colon which is invalid for Secret.data keys and
several helpers use the package-level K8sClient instead of request-scoped
clients; change mcpSecretKey(serverName, userID) to build a key using only
allowed characters (e.g., join with "-" or "." and sanitize/escape any
disallowed chars) so it matches Kubernetes key rules, update
storeMCPCredentials, getMCPCredentials, deleteMCPCredentials, and
getMCPServerStatusForUser to accept and use a passed-in reqK8s (obtained from
GetK8sClientsForRequest(c)) instead of referencing K8sClient, and in
getMCPCredentials normalize the “Secret not found” path by returning a
not-found/empty credentials result (instead of propagating the raw error that
causes a 500) so it matches the existing key-not-found handling.

}

// ConnectMCPServer handles POST /api/auth/mcp/:serverName/connect
func ConnectMCPServer(c *gin.Context) {
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

userID := c.GetString("userID")
if userID == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User authentication required"})
return
}
if !isValidUserID(userID) {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid user identifier"})
return
}

serverName := c.Param("serverName")
if !isValidServerName(serverName) {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid server name: must be lowercase alphanumeric with hyphens, 1-63 chars"})
return
}

var req struct {
Fields map[string]string `json:"fields" binding:"required"`
}
if err := c.ShouldBindJSON(&req); err != nil {
c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})
return
}
if len(req.Fields) == 0 {
c.JSON(http.StatusBadRequest, gin.H{"error": "At least one credential field is required"})
return
}

creds := &MCPServerCredentials{
UserID: userID,
ServerName: serverName,
Fields: req.Fields,
UpdatedAt: time.Now(),
}

if err := storeMCPCredentials(c.Request.Context(), creds); err != nil {
log.Printf("Failed to store MCP credentials for server %s, user %s: %v", serverName, userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to save MCP credentials"})
return
}

log.Printf("✓ Stored MCP credentials for server %s, user %s", serverName, userID)
c.JSON(http.StatusOK, gin.H{
"message": "MCP server credentials saved",
"serverName": serverName,
})
}

// GetMCPServerStatus handles GET /api/auth/mcp/:serverName/status
func GetMCPServerStatus(c *gin.Context) {
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

userID := c.GetString("userID")
if userID == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User authentication required"})
return
}

serverName := c.Param("serverName")
if !isValidServerName(serverName) {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid server name"})
return
}

creds, err := getMCPCredentials(c.Request.Context(), serverName, userID)
if err != nil {
if errors.IsNotFound(err) {
c.JSON(http.StatusOK, gin.H{"connected": false, "serverName": serverName})
return
}
log.Printf("Failed to get MCP credentials for server %s, user %s: %v", serverName, userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to check MCP server status"})
return
}

if creds == nil {
c.JSON(http.StatusOK, gin.H{"connected": false, "serverName": serverName})
return
}

fieldNames := make([]string, 0, len(creds.Fields))
for k := range creds.Fields {
fieldNames = append(fieldNames, k)
}
sort.Strings(fieldNames)

c.JSON(http.StatusOK, gin.H{
"connected": true,
"serverName": serverName,
"fieldNames": fieldNames,
"updatedAt": creds.UpdatedAt.Format(time.RFC3339),
})
}

// DisconnectMCPServer handles DELETE /api/auth/mcp/:serverName/disconnect
func DisconnectMCPServer(c *gin.Context) {
reqK8s, _ := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

userID := c.GetString("userID")
if userID == "" {
c.JSON(http.StatusUnauthorized, gin.H{"error": "User authentication required"})
return
}

serverName := c.Param("serverName")
if !isValidServerName(serverName) {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid server name"})
return
}

if err := deleteMCPCredentials(c.Request.Context(), serverName, userID); err != nil {
log.Printf("Failed to delete MCP credentials for server %s, user %s: %v", serverName, userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to disconnect MCP server"})
return
}

log.Printf("✓ Deleted MCP credentials for server %s, user %s", serverName, userID)
c.JSON(http.StatusOK, gin.H{"message": "MCP server disconnected"})
}

// GetMCPCredentialsForSession handles GET /api/projects/:project/agentic-sessions/:session/credentials/mcp/:serverName
func GetMCPCredentialsForSession(c *gin.Context) {
project := c.Param("projectName")
session := c.Param("sessionName")
serverName := c.Param("serverName")

reqK8s, reqDyn := GetK8sClientsForRequest(c)
if reqK8s == nil {
c.JSON(http.StatusUnauthorized, gin.H{"error": "Invalid or missing token"})
return
}

if !isValidServerName(serverName) {
c.JSON(http.StatusBadRequest, gin.H{"error": "Invalid server name"})
return
}

// Get userID from session CR
gvr := GetAgenticSessionV1Alpha1Resource()
obj, err := reqDyn.Resource(gvr).Namespace(project).Get(c.Request.Context(), session, v1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
c.JSON(http.StatusNotFound, gin.H{"error": "Session not found"})
return
}
log.Printf("Failed to get session %s/%s: %v", project, session, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get session"})
return
}

userID, found, err := unstructured.NestedString(obj.Object, "spec", "userContext", "userId")
if !found || err != nil || userID == "" {
log.Printf("Failed to extract userID from session %s/%s: found=%v, err=%v", project, session, found, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "User ID not found in session"})
return
}

// Verify authenticated user owns this session
authenticatedUserID := c.GetString("userID")
if authenticatedUserID != "" && authenticatedUserID != userID {
log.Printf("RBAC violation: user %s attempted to access MCP credentials for session owned by %s", authenticatedUserID, userID)
c.JSON(http.StatusForbidden, gin.H{"error": "Access denied: session belongs to different user"})
return
}

creds, err := getMCPCredentials(c.Request.Context(), serverName, userID)
if err != nil {
log.Printf("Failed to get MCP credentials for server %s, user %s: %v", serverName, userID, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to get MCP credentials"})
return
}

if creds == nil {
c.JSON(http.StatusNotFound, gin.H{"error": "MCP credentials not configured for server " + serverName})
return
}

c.JSON(http.StatusOK, gin.H{
"serverName": creds.ServerName,
"fields": creds.Fields,
})
}

// storeMCPCredentials stores MCP server credentials in a cluster-level Secret
func storeMCPCredentials(ctx context.Context, creds *MCPServerCredentials) error {
if creds == nil || creds.UserID == "" || creds.ServerName == "" {
return fmt.Errorf("invalid credentials payload")
}

key := mcpSecretKey(creds.ServerName, creds.UserID)

for i := 0; i < 3; i++ {
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
Comment on lines +249 to +257
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not switch these user-facing flows back to the backend service account.

The handlers authenticate with GetK8sClientsForRequest(c), but every Secret read/write in these helpers goes through the package-level K8sClient. That turns MCP credential CRUD and session fetch into backend-privileged operations instead of request-scoped ones. Pass the request-scoped Secret client into these helpers and use it end-to-end. Based on learnings: not applicable. As per coding guidelines "All user-facing API operations must use GetK8sClientsForRequest(c) for token auth, never the backend service account".

Also applies to: 306-313, 331-339, 365-366

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/mcp_credentials.go` around lines 249 - 257, The
helper functions (starting with storeMCPCredentials) are using the package-level
K8sClient which elevates requests to the backend service account; change these
helpers to accept a request-scoped Secret client (the Kubernetes
corev1.SecretInterface) as a parameter and replace all uses of
K8sClient.CoreV1().Secrets(Namespace) with that injected secret client; update
all callers (those that call storeMCPCredentials and the other helpers covering
the ranges ~306-313, ~331-339, ~365-366) to obtain the secret client from
GetK8sClientsForRequest(c) and pass it through so every Secret read/write is
performed with the request-scoped client.

if err != nil {
if errors.IsNotFound(err) {
secret = &corev1.Secret{
ObjectMeta: v1.ObjectMeta{
Name: mcpCredentialsSecretName,
Namespace: Namespace,
Labels: map[string]string{
"app": "ambient-code",
"ambient-code.io/provider": "mcp",
},
},
Type: corev1.SecretTypeOpaque,
Data: map[string][]byte{},
}
if _, cerr := K8sClient.CoreV1().Secrets(Namespace).Create(ctx, secret, v1.CreateOptions{}); cerr != nil && !errors.IsAlreadyExists(cerr) {
return fmt.Errorf("failed to create Secret: %w", cerr)
}
secret, err = K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
if err != nil {
return fmt.Errorf("failed to fetch Secret after create: %w", err)
}
} else {
return fmt.Errorf("failed to get Secret: %w", err)
}
}

if secret.Data == nil {
secret.Data = map[string][]byte{}
}

b, err := json.Marshal(creds)
if err != nil {
return fmt.Errorf("failed to marshal credentials: %w", err)
}
secret.Data[key] = b

if _, uerr := K8sClient.CoreV1().Secrets(Namespace).Update(ctx, secret, v1.UpdateOptions{}); uerr != nil {
if errors.IsConflict(uerr) {
continue
}
return fmt.Errorf("failed to update Secret: %w", uerr)
}
return nil
}
return fmt.Errorf("failed to update Secret after retries")
}
Comment on lines +249 to +303
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not pack every user's MCP credentials into one Secret.

Every connect/disconnect rewrites the same mcp-server-credentials object, and getMCPServerStatusForUser() later reads and scans that whole object to answer one user's Integrations page. That makes the write path a global conflict hotspot and turns status lookup into O(total stored MCP credentials). Please shard storage per user or per (server,user).

Also applies to: 365-382

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/mcp_credentials.go` around lines 249 - 303,
storeMCPCredentials currently packs all users' MCP credentials into a single
Secret (mcpCredentialsSecretName) causing global conflicts and O(N) reads;
change to shard storage per (server,user) by using
mcpSecretKey(creds.ServerName, creds.UserID) as the Secret name (or append it to
mcpCredentialsSecretName) and store the marshaled creds as the Secret.Data (not
as an entry in a shared map). Update the flow in storeMCPCredentials: compute
per-secret name via mcpSecretKey, Create/Get that single Secret (with same
labels/Type), set Secret.Data to the marshaled creds (or a single key like
"credentials"), and Update; preserve retry-on-conflict logic. Also update
getMCPServerStatusForUser (and any other reader) to read the per-(server,user)
Secret by the same mcpSecretKey instead of scanning the large shared secret.


// getMCPCredentials retrieves MCP server credentials for a user
func getMCPCredentials(ctx context.Context, serverName, userID string) (*MCPServerCredentials, error) {
if userID == "" || serverName == "" {
return nil, fmt.Errorf("serverName and userID are required")
}

key := mcpSecretKey(serverName, userID)

secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
if err != nil {
return nil, err
}
Comment on lines +313 to +316
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Treat a missing backing Secret as “not configured.”

getMCPCredentials() only returns nil when the entry is absent inside an existing Secret. On a fresh cluster, Secrets().Get() returns NotFound, which GetMCPCredentialsForSession() currently turns into a 500 instead of the normal no-credentials path.

Suggested fix
 	secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
 	if err != nil {
-		return nil, err
+		if errors.IsNotFound(err) {
+			return nil, nil
+		}
+		return nil, fmt.Errorf("failed to get Secret %q: %w", mcpCredentialsSecretName, err)
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
if err != nil {
return nil, err
}
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
return nil, nil
}
return nil, fmt.Errorf("failed to get Secret %q: %w", mcpCredentialsSecretName, err)
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@components/backend/handlers/mcp_credentials.go` around lines 313 - 316,
getMCPCredentials() currently propagates a Kubernetes NotFound error from
K8sClient.CoreV1().Secrets(...).Get which causes GetMCPCredentialsForSession()
to return a 500; treat a missing Secret as "not configured" by detecting a
NotFound error (errors.IsNotFound(err) / k8serrors.IsNotFound(err)) and return
(nil, nil) instead of the error. Update the getMCPCredentials function to check
the Get() error, return nil,nil for NotFound, and only return actual errors for
other failure cases so GetMCPCredentialsForSession() follows the normal
no-credentials path.


if secret.Data == nil || len(secret.Data[key]) == 0 {
return nil, nil
}

var creds MCPServerCredentials
if err := json.Unmarshal(secret.Data[key], &creds); err != nil {
return nil, fmt.Errorf("failed to parse credentials: %w", err)
}

return &creds, nil
}

// deleteMCPCredentials removes MCP server credentials for a user
func deleteMCPCredentials(ctx context.Context, serverName, userID string) error {
if userID == "" || serverName == "" {
return fmt.Errorf("serverName and userID are required")
}

key := mcpSecretKey(serverName, userID)

for i := 0; i < 3; i++ {
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
return nil
}
return fmt.Errorf("failed to get Secret: %w", err)
}

if secret.Data == nil || len(secret.Data[key]) == 0 {
return nil
}

delete(secret.Data, key)

if _, uerr := K8sClient.CoreV1().Secrets(Namespace).Update(ctx, secret, v1.UpdateOptions{}); uerr != nil {
if errors.IsConflict(uerr) {
continue
}
return fmt.Errorf("failed to update Secret: %w", uerr)
}
return nil
}
return fmt.Errorf("failed to update Secret after retries")
}

// getMCPServerStatusForUser returns status for all MCP servers a user has credentials for
func getMCPServerStatusForUser(ctx context.Context, userID string) gin.H {
secret, err := K8sClient.CoreV1().Secrets(Namespace).Get(ctx, mcpCredentialsSecretName, v1.GetOptions{})
if err != nil || secret.Data == nil {
return gin.H{}
}

suffix := ":" + userID
result := gin.H{}
for key := range secret.Data {
if strings.HasSuffix(key, suffix) {
serverName := strings.TrimSuffix(key, suffix)
result[serverName] = gin.H{
"connected": true,
"valid": true,
}
}
}
return result
}
Loading
Loading