feat: add MCP server config, session labels, and HTTP tools

[jeremyeder]

Summary

Rebased and refined version of #629 with all review feedback addressed.

• MCP server configuration: Backend CRUD endpoints and frontend UI for managing MCP servers per workspace, including test connection, import/export (Claude Desktop and OpenCode formats)
• Session labels: Label editor integrated into create-session dialog with clickable label filtering in sessions list
• HTTP tools: UI tab and API routes for managing HTTP-based tools per workspace
• Operator: Inject MCP config into session pods when project-level MCP servers are configured

Review feedback addressed

• SecurityContext on MCP test pods (drop ALL capabilities, no privilege escalation)
• Arg/env validation in test handler (dangerousArgPatterns blocklist, blockedEnvVars)
• Error context in JSON parsing (truncated raw data preview in logs)
• Type guards for array assertions in MCP server import
• Loading/error states on all React Query consumers (Skeleton + Alert)
• Operator logs warnings for non-NotFound ConfigMap fetch errors

Refinements

• Deduplicated integration status cards in create-session dialog (~130 lines removed)
• Removed redundant data fetching in MCP config editor (children handle own loading)
• Aligned test route error logging with structured proxy error pattern

Closes #629

Test plan

• Create/edit/delete MCP server configs from the workspace MCP Servers tab
• Test connection to an MCP server via the test button
• Import/export MCP config in Claude Desktop and OpenCode formats
• Add labels when creating a session, verify they appear in the sessions list
• Click labels in sessions list to filter by label
• Add/remove HTTP tools from the workspace HTTP Tools tab
• Verify MCP config is injected into session pods when configured

🤖 Generated with Claude Code

[github-actions]

Claude Code Review

Summary

This PR adds MCP server configuration management, session label support, and HTTP tool management — a substantial feature set (~1937 additions). The core architecture is sound, but there are several security and pattern-adherence issues worth addressing before merge.

---

Issues by Severity

🔴 Critical Issues

1. Sensitive values stored in plain ConfigMap (mcp_config.go)

MCP server environment variables and HTTP tool headers (which typically include API keys, auth tokens, and credentials) are stored unencrypted in a ConfigMap. ConfigMaps are not encrypted at rest and are readable by any workload in the namespace.

// mcp_config.go — credentials land in a ConfigMap, not a Secret
cm.Data[key] = string(configJSON)
k8sClient.CoreV1().ConfigMaps(projectName).Update(...)

MCP env vars like GITHUB_TOKEN, OPENAI_API_KEY, etc. will be stored as plaintext. These should be stored in a Secret, with the ConfigMap holding only non-sensitive structural config (command, args).

---

2. Write operations bypass the validate-then-escalate pattern (mcp_config.go, mcp_test_server.go)

Both files use the user-scoped token (GetK8sClientsForRequest) directly for write operations (ConfigMap create/update, Pod create). The established project pattern requires:

1. Validate user token ✅
2. Perform explicit RBAC check ❌ (missing)
3. Use backend service account for privileged writes ❌ (missing)

// updateConfigMapKey — user token used directly for ConfigMap writes
k8sClient, _ := GetK8sClientsForRequest(c)
k8sClient.CoreV1().ConfigMaps(projectName).Create(...)  // should be K8sClient (service account)

// TestMcpServer — user token used directly for Pod creation
k8sClient, _ := GetK8sClientsForRequest(c)
k8sClient.CoreV1().Pods(projectName).Create(...)  // should be K8sClient (service account)

See .claude/patterns/k8s-client-usage.md Pattern 2 for the correct approach.

---

3. Internal K8s error exposed to user (mcp_test_server.go ~line 453)

c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("failed to create test pod: %v", err)})

This returns raw K8s API error details to the client. Per the error handling pattern: log internally with full details, return a generic user message:

log.Printf("Failed to create MCP test pod in %s: %v", projectName, err)
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to create test pod"})

---

🟡 Major Issues

4. No explicit RBAC check before pod creation in TestMcpServer

The handler validates the command allowlist and env blocklist correctly, but never asks K8s whether the authenticated user has permission to trigger MCP tests. If a user lacks the required permission they receive an opaque K8s error (leaking internals). An explicit SelfSubjectAccessReview check would give a clean 403.

---

5. docker and podman in allowedMcpCommands (mcp_test_server.go)

"docker":  true,
"podman":  true,

These commands cannot realistically function as MCP stdio servers, and having them in the allowlist adds unnecessary surface area. Consider removing them unless there is a concrete validated use-case.

---

6. MCP mount path may be incorrect (operator/internal/handlers/sessions.go)

MountPath: "/home/user/.mcp.json",
SubPath:   "mcp.json",

Claude Code reads MCP configuration from ~/.config/claude/claude_desktop_config.json or ~/.claude.json depending on version — not ~/.mcp.json. If the path is wrong, MCP servers will be configured but silently ignored at runtime. Worth verifying against the runner's actual config lookup before merge.

---

7. context.TODO() in operator reconciliation (operator/internal/handlers/sessions.go)

config.K8sClient.CoreV1().ConfigMaps(sessionNamespace).Get(context.TODO(), ...)

context.TODO() bypasses cancellation and tracing. Should use the event handler's context or context.Background() with a timeout, consistent with other calls in the same file.

---

🔵 Minor Issues

8. Untyped JSON round-trip in Go (mcp_config.go)

Config is parsed into map[string]interface{} and re-serialized. Typed structs mirroring the frontend's McpConfigData / HttpToolsData would add compile-time safety and make the API contract explicit.

9. err.Error() returned to user for JSON binding (mcp_config.go)

c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()})

For consistency with project error handling patterns, return a fixed string like "Invalid request body".

10. Comma-separated args don't handle embedded commas (mcp-server-dialog.tsx)

const parsedArgs = args.split(",").map((a) => a.trim()).filter(Boolean);

Arguments containing commas would be split incorrectly. Consider a tag/pill input similar to LabelEditor.

11. Label selector built without client-side validation (sessions-section.tsx)

const labelSelector = Object.entries(labelFilters)
  .map(([k, v]) => `${k}=${v}`)
  .join(',') || undefined;

Special characters in filter values produce an invalid selector that K8s rejects with a generic failure in the UI. Adding client-side validation of K8s label key/value format would give a better UX.

12. Missing aria-label on icon-only dropdown triggers

The MoreHorizontal icon buttons in mcp-servers-tab.tsx and http-tools-tab.tsx need aria-label="Server options" / aria-label="Tool options" for screen reader accessibility.

13. as const on dynamic array (use-sessions.ts)

queryKey: [...sessionKeys.list(projectName, params), labelSelector ?? ""] as const,

as const on a spread of a non-literal array provides no useful type narrowing. Can be removed.

---

Positive Highlights

• SecurityContext on test pods correctly configured (AllowPrivilegeEscalation: false, Drop ALL capabilities) with CPU/memory resource limits — exactly the pattern from security-standards.md.
• Input validation (allowedMcpCommands, dangerousArgPatterns, blockedEnvVars) is thorough and shows the security feedback from feat: add MCP server config, session labels, and HTTP tools #629 was acted on.
• Frontend loading/error states — every new React Query consumer has Skeleton loading and Alert error states, no omissions.
• React Query structure correctly separates API client layer (services/api/) from hook layer (services/queries/), per react-query-usage.md.
• createProxyRouteHandlers helper — clean DRY abstraction that makes future proxy routes trivial.
• IntegrationCard refactor — ~130 lines of duplicated JSX removed cleanly.
• Import/export multi-format support — toInternal/toOpenCode type guards handle Claude Desktop and OpenCode formats correctly.
• Clickable label filtering — intuitive UX, clean implementation.
• Operator non-NotFound warning — correctly distinguishes transient errors from missing ConfigMaps.

---

Recommendations

1. (Blocking) Move sensitive MCP env vars and HTTP tool auth headers into a Secret, not a ConfigMap.
2. (Blocking) Follow the validate-then-escalate pattern in mcp_config.go and mcp_test_server.go: explicit RBAC check, then service account for writes.
3. (Before merge) Fix TestMcpServer to return a generic error on pod creation failure; log full details server-side only.
4. (Before merge) Verify the MCP config mount path /home/user/.mcp.json against actual Claude Code config file lookup in the runner.
5. (Nice to have) Replace context.TODO() in the operator with a proper context.
6. (Nice to have) Remove docker/podman from the command allowlist or add explicit justification.

🤖 Generated with Claude Code

---

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.

[jeremyeder]

Superseded by #697 (labels) and upcoming MCP-only PR. Splitting for easier review.