fix: configure multi-user MCP secrets after create

[thedadams]

This change also migrates existing multi-user MCP servers to store their cofiguration in credentials.

Issue: https://github.com/obot-platform/sensitive-issues/issues/66

[entelligence-ai-pr-reviews]

EntelligenceAI PR Summary

Introduces a one-time MCP server manifest-to-credentials migration and supporting infrastructure, along with a UI fix to prevent secret transmission.

• pkg/controller/migrate.go: New migration function extracts plaintext env vars and remote config headers from MCPServer specs, stores them as GPTScript credentials, and clears the original fields; gated by migrations table
• pkg/controller/controller.go: Migration called during PreStart with proper error handling
• pkg/controller/migrate_test.go: Three new unit tests covering extraction, no-op, and credential context construction
• pkg/gateway/db/migrations.go & db.go: migrateIfEntryNotFoundInMigrationsTable exported as MigrateIfEntryNotFoundInMigrationsTable with GoDoc; all call sites updated
• pkg/gateway/client/client.go: Exposes MigrateIfEntryNotFoundInMigrationsTable as a client method; fixes emailsWithExplictRoles typo
• pkg/gateway/client/identity.go: Fixes same typo in EnsureIdentity
• ui/user/src/lib/components/admin/CatalogServerForm.svelte: Strips secret values from manifest before API submission

---

Confidence Score: 3/5 - Review Recommended

Likely safe but review recommended — this PR introduces a thoughtful one-time migration of MCP server credentials with proper gating via a migrations table, but three unresolved concerns from prior reviews warrant attention before merging. Specifically, in pkg/controller/migrate.go, the migration clears server.Spec.Manifest fields even when a credential already exists without verifying or updating the existing credential's contents, which could silently lose data. Additionally, the use of plain client.Update without retry logic means a transient Kubernetes resourceVersion conflict during PreStart will permanently mark the migration as failed, and the long-running Kubernetes work (credential creation) being wrapped in a gateway DB transaction in pkg/gateway/client/client.go risks holding the transaction open too long.

Key Findings:

• In migrateMCPServerManifestValuesToCredentialsOnce (migrate.go:L73-L164), when a credential already exists the code clears the manifest's env vars and headers but does not verify or merge the existing credential — if the existing credential is stale or incomplete, the plaintext values are lost with no recovery path.
• The client.Update call in the migration has no retry/conflict handling; a transient resourceVersion mismatch will cause PreStart to return an error and the migration to be considered failed, potentially leaving MCPServer objects in a partially-migrated state on every subsequent startup.
• In pkg/gateway/client/client.go:L82-L104, wrapping the migration function (which performs external Kubernetes API calls) inside a gateway DB transaction means the transaction can be held open for an unbounded duration, risking database lock contention or timeouts under load.
• The PR's core goal — moving plaintext secrets out of MCPServer specs and into GPTScript credentials — is architecturally sound and the migration gate pattern is a clean approach; the issues are primarily around error handling and transactional safety rather than the migration logic itself.
• 1 previous unresolved comment(s) likely resolved in latest diff (score-only signal; thread status unchanged)

Files requiring special attention

• pkg/controller/migrate.go
• pkg/gateway/client/client.go
• pkg/controller/controller.go

[entelligence-ai-pr-reviews]

Introduces a one-time migration to move sensitive MCP server manifest config values into GPTScript credentials and prevents secret values from being sent in server creation requests.

• pkg/controller/migrate.go: Core migration logic with migrateMCPServerManifestValuesToCredentialsOnce, guarded by migrations table; helpers mcpServerCredentialContext and extractAndClearMCPServerConfigValues
• pkg/controller/controller.go: Migration invoked in PreStart before admin workspace init
• pkg/controller/migrate_test.go: Three unit tests covering extraction, no-op, and credential context cases
• pkg/gateway/db/migrations.go + db.go: migrateIfEntryNotFoundInMigrationsTable exported as MigrateIfEntryNotFoundInMigrationsTable
• pkg/gateway/client/client.go: New exported MigrateIfEntryNotFoundInMigrationsTable method using GORM transactions; typo fix
• pkg/gateway/client/identity.go: Typo fix emailsWithExplictRoles → emailsWithExplicitRoles
• ui/user/src/lib/components/admin/CatalogServerForm.svelte: omitSecretValuesFromServerManifest strips secret values from manifest before createServerFn call

[Copilot]

Pull request overview

This PR updates multi-user MCP server creation/configuration to avoid persisting secret values in server manifests, and adds a startup migration to move existing stored manifest values into GPTScript credentials.

Changes:

• UI: Strip env[].value / remoteConfig.headers[].value from the manifest payload when creating multi-user catalog servers, then configure secrets via the separate configure call.
• Gateway DB: Export the “run migration once” helper and add a client wrapper for it.
• Controller: Add a one-time migration at controller startup to extract manifest values from existing multi-user MCP servers and store them in credentials (with unit tests for extraction/context helpers).

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
ui/user/src/lib/components/admin/CatalogServerForm.svelte	Omits secret values from manifest during server creation and continues configuring secrets via /configure.
pkg/gateway/db/migrations.go	Exports the migration guard helper (MigrateIfEntryNotFoundInMigrationsTable).
pkg/gateway/db/db.go	Updates callers to use exported migration guard helper.
pkg/gateway/client/client.go	Fixes typo in explicit-role email map name; adds a gateway-client migration guard wrapper.
pkg/gateway/client/identity.go	Fixes typo in explicit-role email map usage.
pkg/controller/migrate.go	Adds controller startup migration to move multi-user MCP manifest values into credentials and clear them from specs.
pkg/controller/controller.go	Runs the new migration during PreStart.
pkg/controller/migrate_test.go	Adds unit tests for manifest value extraction and credential context generation.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.