From f10736816a93d801ad1e59c95f8ac4605be910a9 Mon Sep 17 00:00:00 2001 From: kapelame Date: Thu, 23 Jul 2026 21:12:06 +0800 Subject: [PATCH 1/8] feat: bind shared secrets when enabling capabilities --- apps/web/src/i18n/locales/en-US/admin.json | 3 - apps/web/src/i18n/locales/zh-CN/admin.json | 3 - apps/web/src/lib/api-capabilities.ts | 9 +- apps/web/src/lib/api-marketplace.ts | 4 +- apps/web/src/lib/api-types.ts | 1 + apps/web/src/lib/credential-kind-ui.ts | 8 + .../src/pages/admin/agents/AgentConfigTab.tsx | 188 +++++++++++++++--- .../mcp-directory/MCPDirectory.tsx | 4 +- catalog/mcp/catalog.json | 4 +- catalog/mcp/embed.go | 3 - docs/openapi/openapi.yaml | 8 +- server/internal/api/mcpdirectory/handler.go | 68 +++---- .../internal/api/mcpdirectory/handler_test.go | 59 +++--- server/internal/api/mcpdirectory/oauth.go | 17 +- .../internal/api/mcpdirectory/oauth_scope.go | 12 +- server/internal/auth/mcpoauth/credential.go | 8 +- server/internal/db/queries/store.sql | 4 +- server/internal/db/sqlc/store.sql.go | 11 +- server/internal/dev/capability_routes.go | 63 +++++- server/internal/dev/routes.go | 2 +- server/internal/dev/routes_agents.go | 2 +- server/internal/dev/routes_capability_test.go | 31 +++ server/internal/dev/routes_test.go | 2 +- server/internal/mcpcatalog/catalog_test.go | 14 +- server/internal/mcpcatalog/loader.go | 20 +- server/internal/store/capabilities.go | 104 ++++++++-- .../store/capabilities_binding_test.go | 79 ++++++++ .../store/capabilities_pinning_mode_test.go | 2 +- server/internal/store/credential_kinds.go | 2 + server/internal/store/mcp_directory.go | 13 +- server/internal/store/mcp_directory_test.go | 6 +- server/internal/store/oauth_secret_test.go | 7 +- server/internal/store/store.go | 7 +- ...00010_notion_mcp_oauth_credential_kind.sql | 85 ++++++++ tests/e2e/mcp-directory.spec.ts | 8 +- 35 files changed, 644 insertions(+), 217 deletions(-) create mode 100644 server/internal/store/capabilities_binding_test.go create mode 100644 server/migrations/000010_notion_mcp_oauth_credential_kind.sql diff --git a/apps/web/src/i18n/locales/en-US/admin.json b/apps/web/src/i18n/locales/en-US/admin.json index a8f7cb61..2035d468 100644 --- a/apps/web/src/i18n/locales/en-US/admin.json +++ b/apps/web/src/i18n/locales/en-US/admin.json @@ -893,9 +893,6 @@ "description": "Browse curated hosted MCP servers and import their configuration into this workspace.", "verified": "Verified", "securityNotice": "Import only saves the configuration and does not run it immediately. The MCP can execute in a Runtime only after you enable it and bind it to an Agent.", - "source": { - "builtin": "Built-in catalog" - }, "filters": { "category": "Connector categories", "allCategories": "All categories", diff --git a/apps/web/src/i18n/locales/zh-CN/admin.json b/apps/web/src/i18n/locales/zh-CN/admin.json index f417e851..a8bca348 100644 --- a/apps/web/src/i18n/locales/zh-CN/admin.json +++ b/apps/web/src/i18n/locales/zh-CN/admin.json @@ -893,9 +893,6 @@ "description": "浏览经过筛选的托管 MCP 服务,并将配置导入当前工作区。", "verified": "已验证", "securityNotice": "导入只会保存配置,不会立即运行。启用并绑定 Agent 后,该 MCP 才可能在 Runtime 中执行。", - "source": { - "builtin": "内置目录" - }, "filters": { "category": "连接器分类", "allCategories": "全部分类", diff --git a/apps/web/src/lib/api-capabilities.ts b/apps/web/src/lib/api-capabilities.ts index 753a38ee..f1b4d753 100644 --- a/apps/web/src/lib/api-capabilities.ts +++ b/apps/web/src/lib/api-capabilities.ts @@ -320,14 +320,19 @@ export function useEnableAgentCapabilityMutation( ) { const qc = useQueryClient() return useMutation({ - mutationFn: ({ capabilityVersionID, configuration, pinningMode }: { capabilityVersionID: string; configuration?: Record; pinningMode?: "latest" | "pinned" }) => { + mutationFn: ({ capabilityVersionID, configuration, pinningMode, credentialBindings }: { capabilityVersionID: string; configuration?: Record; pinningMode?: "latest" | "pinned"; credentialBindings?: Record }) => { if (!workspaceID || !agentID) throw new Error("workspace and agent are required") - return enableAgentCapability(workspaceID, agentID, capabilityVersionID, { configuration, pinning_mode: pinningMode }) + return enableAgentCapability(workspaceID, agentID, capabilityVersionID, { + configuration, + credential_bindings: credentialBindings, + pinning_mode: pinningMode, + }) }, retry: noUnreachableRetry, onSuccess: () => { if (workspaceID && agentID) { qc.invalidateQueries({ queryKey: KEY_AGENT_CAPABILITIES(workspaceID, agentID) }) + qc.invalidateQueries({ queryKey: ["admin", "agent", workspaceID, agentID] }) } }, }) diff --git a/apps/web/src/lib/api-marketplace.ts b/apps/web/src/lib/api-marketplace.ts index 46a4249f..e30a9918 100644 --- a/apps/web/src/lib/api-marketplace.ts +++ b/apps/web/src/lib/api-marketplace.ts @@ -113,8 +113,6 @@ export interface MCPDirectoryItem { export interface MCPDirectoryListResponse { items: MCPDirectoryItem[] - updated_at: string - source: "builtin" } export interface MCPDirectoryImportResponse { @@ -207,7 +205,7 @@ async function listEnabledAgents(workspaceID: string | null, capabilityID: strin } async function listMCPDirectory(workspaceID: string | null): Promise { - if (!workspaceID) return { items: [], updated_at: "", source: "builtin" } + if (!workspaceID) return { items: [] } return apiRequest(`/api/v1/workspaces/${encodeURIComponent(workspaceID)}/mcp-directory`) } diff --git a/apps/web/src/lib/api-types.ts b/apps/web/src/lib/api-types.ts index 805201ab..0194fcb5 100644 --- a/apps/web/src/lib/api-types.ts +++ b/apps/web/src/lib/api-types.ts @@ -348,6 +348,7 @@ export interface AgentCapability { export interface EnableAgentCapabilityRequest { configuration?: Record + credential_bindings?: Record /** See AgentCapability.pinning_mode. Empty defaults to "pinned" server-side. */ pinning_mode?: "latest" | "pinned" } diff --git a/apps/web/src/lib/credential-kind-ui.ts b/apps/web/src/lib/credential-kind-ui.ts index 66223ff2..d486f06a 100644 --- a/apps/web/src/lib/credential-kind-ui.ts +++ b/apps/web/src/lib/credential-kind-ui.ts @@ -34,6 +34,10 @@ export const CREDENTIAL_KIND_LABELS = { zh: "Notion 集成 token", en: "Notion Integration Token", }, + notion_mcp_oauth: { + zh: "Notion MCP OAuth", + en: "Notion MCP OAuth", + }, jira_api_token: { zh: "Jira API Token", en: "Jira API Token", @@ -47,6 +51,7 @@ export const CREDENTIAL_KIND_OPTIONS: KnownCredentialKind[] = [ "slack_bot_token", "postgres_dsn", "notion_integration", + "notion_mcp_oauth", "jira_api_token", ] @@ -66,6 +71,9 @@ export const CREDENTIAL_KIND_META: Record credential.kind === kind) } +function secretCredentialKind(secret: Secret) { + const value = secret.metadata?.credential_kind_code + return typeof value === "string" ? value.trim() : "" +} + +function sharedSecretsForKind(secrets: Secret[], kind: string) { + return secrets.filter((secret) => { + const secretKind = secretCredentialKind(secret) + return secret.kind === "capability_inline" + && secret.status === "active" + && (secretKind === "" || secretKind === kind) + }) +} + +function sharedSecretBindingID(agent: Agent, kind: string) { + const bindings = agent.config?.credential_bindings + if (!bindings || typeof bindings !== "object" || Array.isArray(bindings)) return "" + const binding = (bindings as Record)[kind] + if (!binding || typeof binding !== "object" || Array.isArray(binding)) return "" + const value = binding as Record + return value.source === "shared" && typeof value.secret_id === "string" ? value.secret_id : "" +} + +function hasUsableCredential(agent: Agent, credentials: UserCredential[], sharedSecrets: Secret[], kind: string) { + const sharedID = sharedSecretBindingID(agent, kind) + if (sharedID && sharedSecrets.some((secret) => secret.id === sharedID)) return true + return agent.visibility !== "public" && hasCredentialKind(credentials, kind) +} + +function catalogIDFromVersion(version: CapabilityVersion | undefined) { + const payload = version?.source_payload + if (!payload || typeof payload !== "object" || Array.isArray(payload)) return "" + const catalogID = (payload as Record).catalog_id + return typeof catalogID === "string" ? catalogID.trim() : "" +} + function useCapabilityVersions( workspaceID: string | null, capability: Capability | undefined, @@ -166,6 +203,7 @@ function CapabilityCard({ agent, workspaceID, credentials, + sharedSecrets, mode, onToast, }: { @@ -173,6 +211,7 @@ function CapabilityCard({ agent: Agent workspaceID: string | null credentials: UserCredential[] + sharedSecrets: Secret[] mode: "enabled" | "available" onToast: (message: string) => void }) { @@ -182,7 +221,9 @@ function CapabilityCard({ const { latest, versions, versionsQ } = useCapabilityVersions(workspaceID, capability, mode === "enabled") const boundVersion = versions.find((version) => version.id === binding?.capability_version_id) ?? (binding?.capability_version_id && capability?.pinned_version ? { id: binding.capability_version_id, capability_id: capability.id, version: capability.pinned_version, created_at: capability.latest_version_created_at ?? capability.created_at } as CapabilityVersion : undefined) const versionDeleted = !!binding && !versionsQ.isLoading && !boundVersion && !capability?.latest_version_id - const missingCredential = capability ? requiredCredentialKinds(capability).some((rc) => !hasCredentialKind(credentials, rc.kind)) : false + const missingCredential = capability + ? requiredCredentialKinds(capability).some((rc) => !hasUsableCredential(agent, credentials, sharedSecrets, rc.kind)) + : false const fromMarketplace = !!capability?.from_marketplace || (!!capability?.source_workspace_id && capability.source_workspace_id !== workspaceID) const deprecated = !!capability?.deprecated_at const border = mode === "available" ? "border-dashed border-line-strong" : "border-line" @@ -250,7 +291,7 @@ function CapabilityCard({ /> )} - +
@@ -267,6 +308,7 @@ function CapabilityCard({ agent={agent} capability={capability} credentials={credentials} + sharedSecrets={sharedSecrets} workspaceID={workspaceID} onToast={onToast} /> @@ -297,7 +339,17 @@ function CapabilityCard({ ) } -function CredentialStatus({ capability, credentials }: { capability: Capability; credentials: UserCredential[] }) { +function CredentialStatus({ + capability, + agent, + credentials, + sharedSecrets, +}: { + capability: Capability + agent: Agent + credentials: UserCredential[] + sharedSecrets: Secret[] +}) { const { t, i18n } = useTranslation("admin") const requiredCreds = capability.required_credentials ?? [] if (requiredCreds.length === 0) { @@ -306,16 +358,19 @@ function CredentialStatus({ capability, credentials }: { capability: Capability; return (
{requiredCreds.map((rc) => { - const credential = credentials.find((cred) => cred.kind === rc.kind) + const sharedID = sharedSecretBindingID(agent, rc.kind) + const sharedSecret = sharedSecrets.find((secret) => secret.id === sharedID) + const credential = agent.visibility === "public" ? undefined : credentials.find((cred) => cred.kind === rc.kind) + const available = sharedSecret ?? credential const label = credentialKindLabel(rc.kind, i18n.language, rc.kind) return ( -
- {credential ? ( - {t("agents.detail.capabilities.credential.present", { kind: label, name: credential.display_name || t("agents.detail.capabilities.credential.defaultName") })} +
+ {available ? ( + {t("agents.detail.capabilities.credential.present", { kind: label, name: sharedSecret?.name || credential?.display_name || t("agents.detail.capabilities.credential.defaultName") })} ) : ( {t("agents.detail.capabilities.credential.missing", { kind: label })} )} - + {!sharedSecret && }
) })} @@ -362,30 +417,63 @@ function VersionSelect({ versions, value, onChange }: { versions: CapabilityVers ) } -function EnableCredentialStatusList({ +function EnableCredentialBindingList({ requiredKinds, credentials, + sharedSecrets, + publicAgent, + bindings, + onChange, }: { requiredKinds: { kind: string }[] credentials: UserCredential[] + sharedSecrets: Secret[] + publicAgent: boolean + bindings: Record + onChange: (kind: string, secretID: string) => void }) { - const { t } = useTranslation("admin") + const { t, i18n } = useTranslation("admin") return ( -
+
{requiredKinds.map((rc) => { - const has = hasCredentialKind(credentials, rc.kind) + const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind) + const selectedSecretID = bindings[rc.kind] ?? "" + const hasPersonal = !publicAgent && hasCredentialKind(credentials, rc.kind) + const ready = !!selectedSecretID || hasPersonal return (
- {has ? "✓" : "⚠"} - {rc.kind} - {!has && {t("credentialCheck.personalYouMissing")}} + + + {!ready && ( +

+ {publicAgent ? t("credentialCheck.sharedNoneAvailable") : t("credentialCheck.personalYouMissing")} +

+ )}
) })} @@ -398,6 +486,7 @@ function CapabilityVersionDialog({ agent, capability, credentials = [], + sharedSecrets = [], workspaceID, binding, triggerLabel, @@ -408,6 +497,7 @@ function CapabilityVersionDialog({ agent: Agent capability: Capability credentials?: UserCredential[] + sharedSecrets?: Secret[] workspaceID: string | null binding?: AgentCapability triggerLabel?: string @@ -418,19 +508,49 @@ function CapabilityVersionDialog({ const [open, setOpen] = useState(false) const mut = useEnableAgentCapabilityMutation(workspaceID, agent.id) const [selected, setSelected] = useState(binding?.capability_version_id ?? "") + const [credentialBindingChoices, setCredentialBindingChoices] = useState>({}) const { latest, versions, versionsQ } = useCapabilityVersions(workspaceID, capability, open) const selectedVersion = selected ? versions.find((version) => version.id === selected) ?? (mode === "enable" ? latest : versions[0]) : mode === "enable" ? latest : versions[0] - const requiredKinds = mode === "enable" ? requiredCredentialKinds(capability) : [] - const missingRequiredCredential = requiredKinds.some((rc) => !hasCredentialKind(credentials, rc.kind)) + const requiredKinds = useMemo( + () => mode === "enable" ? requiredCredentialKinds(capability) : [], + [capability, mode], + ) + const catalogID = catalogIDFromVersion(selectedVersion) + const defaultCredentialBindings = useMemo(() => { + const defaults: Record = {} + for (const rc of requiredKinds) { + const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind) + const oauthSecret = catalogID + ? kindSecrets.find((secret) => secret.auth_type === "oauth2" && secret.provider === catalogID) + : undefined + if (oauthSecret) defaults[rc.kind] = oauthSecret.id + else if (agent.visibility === "public" && kindSecrets[0]) defaults[rc.kind] = kindSecrets[0].id + } + return defaults + }, [agent.visibility, catalogID, requiredKinds, sharedSecrets]) + const credentialBindings = { ...defaultCredentialBindings, ...credentialBindingChoices } + const missingRequiredCredential = requiredKinds.some((rc) => { + const selectedSecretID = credentialBindings[rc.kind] + if (selectedSecretID && sharedSecretsForKind(sharedSecrets, rc.kind).some((secret) => secret.id === selectedSecretID)) { + return false + } + return agent.visibility === "public" || !hasCredentialKind(credentials, rc.kind) + }) const canSubmit = !!selectedVersion && !mut.isPending && (mode === "enable" ? !missingRequiredCredential : selectedVersion.id !== binding?.capability_version_id) const submit = () => { if (!selectedVersion) return - mut.mutate({ capabilityVersionID: selectedVersion.id }, { + const sharedBindings = Object.fromEntries( + Object.entries(credentialBindings).filter(([, secretID]) => secretID !== ""), + ) + mut.mutate({ + capabilityVersionID: selectedVersion.id, + credentialBindings: mode === "enable" ? sharedBindings : undefined, + }, { onSuccess: () => { setOpen(false) onToast(mode === "enable" @@ -476,7 +596,14 @@ function CapabilityVersionDialog({ {versionsQ.isLoading ? : }
{requiredKinds.length > 0 ? ( - + setCredentialBindingChoices((current) => ({ ...current, [kind]: secretID }))} + /> ) : (
{t("agents.detail.capabilities.enableDialog.noCredential")} @@ -573,7 +700,12 @@ export function AgentConfigTab({ const agentCapabilitiesQ = useAgentCapabilitiesQuery(workspaceID, agent.id) const workspaceCapabilitiesQ = useCapabilitiesQuery(workspaceID) const credentialsQ = useMyCredentials() + const secretsQ = useSecrets(workspaceID) const credentials = credentialsQ.data?.credentials ?? [] + const sharedSecrets = useMemo( + () => (secretsQ.data?.secrets ?? []).filter((secret) => secret.kind === "capability_inline" && secret.status === "active"), + [secretsQ.data?.secrets], + ) const installedCapabilities = agentCapabilitiesQ.data?.installed ?? [] const availableCapabilities = agentCapabilitiesQ.data?.available ?? workspaceCapabilitiesQ.data?.capabilities ?? [] const installedIDs = new Set(installedCapabilities.map((item) => item.capability_id)) @@ -606,6 +738,7 @@ export function AgentConfigTab({ enabledCaps={enabledCaps} installable={installable} credentials={credentials} + sharedSecrets={sharedSecrets} loading={agentCapabilitiesQ.isLoading || workspaceCapabilitiesQ.isLoading} error={agentCapabilitiesQ.error ?? workspaceCapabilitiesQ.error} onToast={onToast} @@ -621,6 +754,7 @@ function ConfigCapabilitiesSection({ enabledCaps, installable, credentials, + sharedSecrets, loading, error, onToast, @@ -631,6 +765,7 @@ function ConfigCapabilitiesSection({ enabledCaps: Array<{ binding: AgentCapability; capability?: Capability }> installable: Capability[] credentials: UserCredential[] + sharedSecrets: Secret[] loading: boolean error: unknown onToast: (message: string) => void @@ -700,6 +835,7 @@ function ConfigCapabilitiesSection({ agent={agent} workspaceID={workspaceID} credentials={credentials} + sharedSecrets={sharedSecrets} mode="enabled" onToast={onToast} /> @@ -715,6 +851,7 @@ function ConfigCapabilitiesSection({ workspaceID={workspaceID} installable={installable} credentials={credentials} + sharedSecrets={sharedSecrets} onToast={onToast} /> @@ -728,6 +865,7 @@ function AddCapabilityDialog({ workspaceID, installable, credentials, + sharedSecrets, onToast, }: { open: boolean @@ -736,6 +874,7 @@ function AddCapabilityDialog({ workspaceID: string | null installable: Capability[] credentials: UserCredential[] + sharedSecrets: Secret[] onToast: (message: string) => void }) { const { t } = useTranslation("admin") @@ -777,6 +916,7 @@ function AddCapabilityDialog({ agent={agent} workspaceID={workspaceID} credentials={credentials} + sharedSecrets={sharedSecrets} mode="available" onToast={(msg) => { onToast(msg) diff --git a/apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectory.tsx b/apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectory.tsx index 2aa4720b..62d41167 100644 --- a/apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectory.tsx +++ b/apps/web/src/pages/admin/capabilities/mcp-directory/MCPDirectory.tsx @@ -2,7 +2,6 @@ import { useEffect, useMemo, useState } from "react" import { useTranslation } from "react-i18next" import { Check, PackageCheck, Server } from "lucide-react" -import { Badge } from "../../../../components/ui/badge" import { Button } from "../../../../components/ui/button" import { EmptyState } from "../../../../components/ui/empty-state" import { ErrorState } from "../../../../components/ui/error-state" @@ -154,7 +153,7 @@ export function MCPDirectory({ return (
-
+
@@ -162,7 +161,6 @@ export function MCPDirectory({

{t("capabilities.mcpDirectory.description")}

- {directoryQ.data?.source ? {t(`capabilities.mcpDirectory.source.${directoryQ.data.source}`)} : null}
diff --git a/catalog/mcp/catalog.json b/catalog/mcp/catalog.json index 47cb37ba..fe232983 100644 --- a/catalog/mcp/catalog.json +++ b/catalog/mcp/catalog.json @@ -78,11 +78,11 @@ "verified": true, "categories": ["Productivity", "Knowledge"], "featured_rank": 4, - "version": "1.0.0", + "version": "1.0.1", "transport": "streamable-http", "authentication": { "type": "oauth2", - "credential_kind": "notion_integration" + "credential_kind": "notion_mcp_oauth" }, "server": { "name": "notion", diff --git a/catalog/mcp/embed.go b/catalog/mcp/embed.go index 11c9d9ad..ce1c20e5 100644 --- a/catalog/mcp/embed.go +++ b/catalog/mcp/embed.go @@ -4,6 +4,3 @@ import _ "embed" //go:embed catalog.json var CatalogJSON []byte - -//go:embed catalog.schema.json -var CatalogSchemaJSON []byte diff --git a/docs/openapi/openapi.yaml b/docs/openapi/openapi.yaml index 4b2ee477..0825c0a7 100644 --- a/docs/openapi/openapi.yaml +++ b/docs/openapi/openapi.yaml @@ -242,6 +242,10 @@ definitions: configuration: additionalProperties: {} type: object + credential_bindings: + additionalProperties: + type: string + type: object pinning_mode: description: |- PinningMode is "latest" or "pinned". Empty falls back to the @@ -1196,10 +1200,6 @@ definitions: items: $ref: '#/definitions/mcpdirectory.itemResponse' type: array - source: - type: string - updated_at: - type: string type: object password.errorResponse: properties: diff --git a/server/internal/api/mcpdirectory/handler.go b/server/internal/api/mcpdirectory/handler.go index 79cceb89..5f7c0e66 100644 --- a/server/internal/api/mcpdirectory/handler.go +++ b/server/internal/api/mcpdirectory/handler.go @@ -21,7 +21,7 @@ import ( ) type catalogLoader interface { - Load(ctx context.Context) (mcpcatalog.Snapshot, error) + Load() (mcpcatalog.Catalog, error) } type directoryStore interface { @@ -71,9 +71,7 @@ type itemResponse struct { } type listResponse struct { - Items []itemResponse `json:"items"` - UpdatedAt string `json:"updated_at"` - Source string `json:"source"` + Items []itemResponse `json:"items"` } type importResponse struct { @@ -89,7 +87,6 @@ type sourcePayload struct { SourceFormat string `json:"source_format"` CatalogID string `json:"catalog_id"` CatalogVersion string `json:"catalog_version"` - CatalogSource string `json:"catalog_source"` } func RegisterRoutes(r chi.Router, deps Deps) { @@ -114,28 +111,24 @@ func RegisterRoutes(r chi.Router, deps Deps) { // @Failure 503 {object} errorResponse // @Router /api/v1/workspaces/{workspaceID}/mcp-directory [get] func (h *handler) list(w http.ResponseWriter, r *http.Request) { - workspaceID, ok := h.authorize(w, r, false) + workspaceID, ok := h.authorize(w, r) if !ok { return } - snapshot, installs, ok := h.load(w, r, workspaceID) + catalog, installs, ok := h.load(w, r, workspaceID) if !ok { return } byCatalog := installMap(installs) - connected, ok := h.connectedCatalogIDs(w, r, workspaceID) + connected, ok := h.connectedCatalogIDs(w, r, workspaceID, catalog) if !ok { return } - items := make([]itemResponse, 0, len(snapshot.Catalog.Items)) - for _, item := range snapshot.Catalog.Items { + items := make([]itemResponse, 0, len(catalog.Items)) + for _, item := range catalog.Items { items = append(items, summarizeItem(item, byCatalog[item.ID], connected[item.ID])) } - writeJSON(w, http.StatusOK, listResponse{ - Items: items, - UpdatedAt: snapshot.Catalog.UpdatedAt, - Source: string(snapshot.Source), - }) + writeJSON(w, http.StatusOK, listResponse{Items: items}) } // get godoc @@ -150,20 +143,20 @@ func (h *handler) list(w http.ResponseWriter, r *http.Request) { // @Failure 404 {object} errorResponse // @Router /api/v1/workspaces/{workspaceID}/mcp-directory/{catalogID} [get] func (h *handler) get(w http.ResponseWriter, r *http.Request) { - workspaceID, ok := h.authorize(w, r, false) + workspaceID, ok := h.authorize(w, r) if !ok { return } - snapshot, installs, ok := h.load(w, r, workspaceID) + catalog, installs, ok := h.load(w, r, workspaceID) if !ok { return } - item, found := snapshot.Find(chi.URLParam(r, "catalogID")) + item, found := catalog.Find(chi.URLParam(r, "catalogID")) if !found { writeError(w, http.StatusNotFound, "connector_not_found") return } - connected, ok := h.connectedCatalogIDs(w, r, workspaceID) + connected, ok := h.connectedCatalogIDs(w, r, workspaceID, catalog) if !ok { return } @@ -192,11 +185,11 @@ func (h *handler) importItem(w http.ResponseWriter, r *http.Request) { if !ok { return } - snapshot, installs, ok := h.load(w, r, workspaceID) + catalog, installs, ok := h.load(w, r, workspaceID) if !ok { return } - item, found := snapshot.Find(chi.URLParam(r, "catalogID")) + item, found := catalog.Find(chi.URLParam(r, "catalogID")) if !found { writeError(w, http.StatusNotFound, "connector_not_found") return @@ -206,7 +199,7 @@ func (h *handler) importItem(w http.ResponseWriter, r *http.Request) { return } if item.Authentication.EffectiveType() == "oauth2" { - connected, ok := h.connectedCatalogIDs(w, r, workspaceID) + connected, ok := h.connectedCatalogIDs(w, r, workspaceID, catalog) if !ok { return } @@ -220,10 +213,9 @@ func (h *handler) importItem(w http.ResponseWriter, r *http.Request) { SourceFormat: "mcp_catalog", CatalogID: item.ID, CatalogVersion: item.Version, - CatalogSource: string(snapshot.Source), }) if err != nil { - writeError(w, http.StatusInternalServerError, "catalog_source_encode_failed") + writeError(w, http.StatusInternalServerError, "catalog_provenance_encode_failed") return } result, err := h.deps.Store.ImportCapability(r.Context(), store.ImportCapabilityInput{ @@ -259,12 +251,8 @@ func (h *handler) importItem(w http.ResponseWriter, r *http.Request) { }) } -func (h *handler) authorize(w http.ResponseWriter, r *http.Request, admin bool) (string, bool) { - allowed := []string{"owner", "admin", "member", "viewer"} - if admin { - allowed = []string{"owner", "admin"} - } - return h.authorizeRoles(w, r, allowed...) +func (h *handler) authorize(w http.ResponseWriter, r *http.Request) (string, bool) { + return h.authorizeRoles(w, r, "owner", "admin", "member", "viewer") } func (h *handler) authorizeRoles(w http.ResponseWriter, r *http.Request, allowed ...string) (string, bool) { @@ -291,18 +279,18 @@ func (h *handler) authorizeRoles(w http.ResponseWriter, r *http.Request, allowed return workspaceID, true } -func (h *handler) load(w http.ResponseWriter, r *http.Request, workspaceID string) (mcpcatalog.Snapshot, []store.MCPDirectoryInstall, bool) { - snapshot, err := h.deps.Catalog.Load(r.Context()) +func (h *handler) load(w http.ResponseWriter, r *http.Request, workspaceID string) (mcpcatalog.Catalog, []store.MCPDirectoryInstall, bool) { + catalog, err := h.deps.Catalog.Load() if err != nil { writeError(w, http.StatusServiceUnavailable, "mcp_catalog_unavailable") - return mcpcatalog.Snapshot{}, nil, false + return mcpcatalog.Catalog{}, nil, false } installs, err := h.deps.Store.ListMCPDirectoryInstalls(r.Context(), workspaceID) if err != nil { writeError(w, http.StatusInternalServerError, "directory_install_state_failed") - return mcpcatalog.Snapshot{}, nil, false + return mcpcatalog.Catalog{}, nil, false } - return snapshot, installs, true + return catalog, installs, true } func installMap(installs []store.MCPDirectoryInstall) map[string]store.MCPDirectoryInstall { @@ -339,7 +327,7 @@ func summarizeItem(item mcpcatalog.Item, install store.MCPDirectoryInstall, conn } } -func (h *handler) connectedCatalogIDs(w http.ResponseWriter, r *http.Request, workspaceID string) (map[string]bool, bool) { +func (h *handler) connectedCatalogIDs(w http.ResponseWriter, r *http.Request, workspaceID string, catalog mcpcatalog.Catalog) (map[string]bool, bool) { result := map[string]bool{} if h.deps.WorkspaceCredentials == nil { return result, true @@ -356,9 +344,13 @@ func (h *handler) connectedCatalogIDs(w http.ResponseWriter, r *http.Request, wo metadataString(candidate.Metadata, "workspace_id") != strings.TrimSpace(workspaceID) { continue } - if catalogID := metadataString(candidate.Metadata, "catalog_id"); catalogID != "" { - result[catalogID] = true + catalogID := strings.TrimSpace(candidate.Provider) + item, found := catalog.Find(catalogID) + if !found || item.Authentication.EffectiveType() != "oauth2" || + metadataString(candidate.Metadata, "credential_kind_code") != item.Authentication.CredentialKind { + continue } + result[catalogID] = true } return result, true } diff --git a/server/internal/api/mcpdirectory/handler_test.go b/server/internal/api/mcpdirectory/handler_test.go index 57ddeb76..46d3864a 100644 --- a/server/internal/api/mcpdirectory/handler_test.go +++ b/server/internal/api/mcpdirectory/handler_test.go @@ -21,11 +21,11 @@ const ( ) type fakeCatalog struct { - snapshot mcpcatalog.Snapshot - err error + catalog mcpcatalog.Catalog + err error } -func (f fakeCatalog) Load(context.Context) (mcpcatalog.Snapshot, error) { return f.snapshot, f.err } +func (f fakeCatalog) Load() (mcpcatalog.Catalog, error) { return f.catalog, f.err } type fakeDirectoryStore struct { role string @@ -68,11 +68,11 @@ func (f *fakeDirectoryStore) ImportCapability(_ context.Context, input store.Imp f.imported = &input if f.importErr != nil { if f.concurrentInstall { - f.installs = append(f.installs, store.MCPDirectoryInstall{CatalogID: "context7", CatalogVersion: "1.0.0", CapabilityID: testCapabilityID}) + f.installs = append(f.installs, store.MCPDirectoryInstall{CatalogID: "context7", CapabilityID: testCapabilityID}) } return store.ImportCapabilityResult{}, f.importErr } - f.installs = append(f.installs, store.MCPDirectoryInstall{CatalogID: "context7", CatalogVersion: "1.0.0", CapabilityID: testCapabilityID}) + f.installs = append(f.installs, store.MCPDirectoryInstall{CatalogID: "context7", CapabilityID: testCapabilityID}) return store.ImportCapabilityResult{Capability: store.CapabilityRead{ID: testCapabilityID, Name: input.Name, Type: input.Type}}, nil } @@ -119,7 +119,7 @@ func TestDirectoryImportUsesServerCatalogAndCreatesNoSecretsOrBindings(t *testin if err := json.Unmarshal(input.SourcePayload, &source); err != nil { t.Fatal(err) } - if source.SourceFormat != "mcp_catalog" || source.CatalogID != "context7" || source.CatalogSource != "builtin" { + if source.SourceFormat != "mcp_catalog" || source.CatalogID != "context7" || source.CatalogVersion != "1.0.0" { t.Fatalf("source=%+v", source) } }) @@ -140,32 +140,41 @@ func TestDirectoryImportIsIdempotent(t *testing.T) { } func TestOAuthDirectoryItemRequiresWorkspaceConnectionBeforeImport(t *testing.T) { - snapshot := testSnapshot() - snapshot.Catalog.Items = []mcpcatalog.Item{{ + catalog := testCatalog() + catalog.Items = []mcpcatalog.Item{{ ID: "notion", Name: "Notion", Description: "Search Notion.", Publisher: mcpcatalog.Publisher{Name: "Notion", URL: "https://www.notion.so"}, Verified: true, Categories: []string{"Productivity"}, FeaturedRank: 1, Version: "1.0.0", Transport: "streamable-http", - Authentication: mcpcatalog.Authentication{Type: "oauth2", CredentialKind: "notion_integration"}, + Authentication: mcpcatalog.Authentication{Type: "oauth2", CredentialKind: "notion_mcp_oauth"}, Server: mcpcatalog.Server{Name: "notion", URL: "https://mcp.notion.com/mcp"}, }} fs := &fakeDirectoryStore{role: "admin"} credentials := &fakeWorkspaceCredentialStore{} - rec := requestWithDeps(t, fs, credentials, snapshot, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") + rec := requestWithDeps(t, fs, credentials, catalog, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") if rec.Code != http.StatusConflict || fs.imported != nil { t.Fatalf("status=%d imported=%v body=%s", rec.Code, fs.imported != nil, rec.Body.String()) } credentials.secrets = []store.SecretRead{{ - ID: "secret-1", Kind: "capability_inline", AuthType: "oauth2", Status: "active", - Metadata: map[string]any{"workspace_id": testWorkspaceID, "catalog_id": "notion"}, + ID: "secret-1", Kind: "capability_inline", Provider: "notion", AuthType: "oauth2", Status: "active", + Metadata: map[string]any{"workspace_id": testWorkspaceID, "credential_kind_code": "notion_integration"}, }} - rec = requestWithDeps(t, fs, credentials, snapshot, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") + rec = requestWithDeps(t, fs, credentials, catalog, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") + if rec.Code != http.StatusConflict || fs.imported != nil { + t.Fatalf("legacy Notion token must not satisfy MCP OAuth: status=%d imported=%v body=%s", rec.Code, fs.imported != nil, rec.Body.String()) + } + + credentials.secrets = []store.SecretRead{{ + ID: "secret-2", Kind: "capability_inline", Provider: "notion", AuthType: "oauth2", Status: "active", + Metadata: map[string]any{"workspace_id": testWorkspaceID, "credential_kind_code": "notion_mcp_oauth"}, + }} + rec = requestWithDeps(t, fs, credentials, catalog, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") if rec.Code != http.StatusCreated { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } header := fs.imported.Spec.MCP.Servers[0].Headers["Authorization"] - if header.Prefix != "Bearer " || header.CredentialKindCode != "notion_integration" { + if header.Prefix != "Bearer " || header.CredentialKindCode != "notion_mcp_oauth" { t.Fatalf("authorization header = %+v", header) } } @@ -188,15 +197,15 @@ func TestDirectoryUnknownCatalogItem(t *testing.T) { func TestDirectoryDetailIncludesStreamableHTTPURL(t *testing.T) { fs := &fakeDirectoryStore{role: "member"} - snapshot := testSnapshot() - snapshot.Catalog.Items = []mcpcatalog.Item{{ + catalog := testCatalog() + catalog.Items = []mcpcatalog.Item{{ ID: "docs", Name: "Docs", Description: "Search docs.", Publisher: mcpcatalog.Publisher{Name: "Publisher", URL: "https://example.com"}, Verified: true, Categories: []string{"Documentation"}, FeaturedRank: 1, Version: "1.0.0", Transport: "streamable-http", Server: mcpcatalog.Server{Name: "docs", URL: "https://docs.example.com/mcp"}, }} - rec := requestWithSnapshot(t, fs, snapshot, http.MethodGet, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/docs") + rec := requestWithCatalog(t, fs, catalog, http.MethodGet, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/docs") if rec.Code != http.StatusOK { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } @@ -224,14 +233,14 @@ func TestDirectoryRejectsInvalidWorkspaceID(t *testing.T) { } func request(t *testing.T, fs *fakeDirectoryStore, method, path string) *httptest.ResponseRecorder { - return requestWithSnapshot(t, fs, testSnapshot(), method, path) + return requestWithCatalog(t, fs, testCatalog(), method, path) } -func requestWithSnapshot(t *testing.T, fs *fakeDirectoryStore, snapshot mcpcatalog.Snapshot, method, path string) *httptest.ResponseRecorder { - return requestWithDeps(t, fs, nil, snapshot, method, path) +func requestWithCatalog(t *testing.T, fs *fakeDirectoryStore, catalog mcpcatalog.Catalog, method, path string) *httptest.ResponseRecorder { + return requestWithDeps(t, fs, nil, catalog, method, path) } -func requestWithDeps(t *testing.T, fs *fakeDirectoryStore, credentials workspaceCredentialStore, snapshot mcpcatalog.Snapshot, method, path string) *httptest.ResponseRecorder { +func requestWithDeps(t *testing.T, fs *fakeDirectoryStore, credentials workspaceCredentialStore, catalog mcpcatalog.Catalog, method, path string) *httptest.ResponseRecorder { t.Helper() router := chi.NewRouter() router.Use(func(next http.Handler) http.Handler { @@ -239,14 +248,14 @@ func requestWithDeps(t *testing.T, fs *fakeDirectoryStore, credentials workspace next.ServeHTTP(w, r.WithContext(auth.WithUserID(r.Context(), testUserID))) }) }) - RegisterRoutes(router, Deps{Catalog: fakeCatalog{snapshot: snapshot}, Store: fs, WorkspaceCredentials: credentials}) + RegisterRoutes(router, Deps{Catalog: fakeCatalog{catalog: catalog}, Store: fs, WorkspaceCredentials: credentials}) rec := httptest.NewRecorder() router.ServeHTTP(rec, httptest.NewRequest(method, path, nil)) return rec } -func testSnapshot() mcpcatalog.Snapshot { - return mcpcatalog.Snapshot{Source: mcpcatalog.SourceBuiltin, Catalog: mcpcatalog.Catalog{ +func testCatalog() mcpcatalog.Catalog { + return mcpcatalog.Catalog{ SchemaVersion: 1, UpdatedAt: "2026-07-22T00:00:00Z", Items: []mcpcatalog.Item{{ @@ -256,7 +265,7 @@ func testSnapshot() mcpcatalog.Snapshot { Version: "1.0.0", Transport: "streamable-http", Server: mcpcatalog.Server{Name: "context7", URL: "https://mcp.context7.com/mcp"}, }}, - }} + } } func decodeResponse(t *testing.T, rec *httptest.ResponseRecorder, target any) { diff --git a/server/internal/api/mcpdirectory/oauth.go b/server/internal/api/mcpdirectory/oauth.go index 6a613efa..71ba1df1 100644 --- a/server/internal/api/mcpdirectory/oauth.go +++ b/server/internal/api/mcpdirectory/oauth.go @@ -161,12 +161,12 @@ func (h *handler) oauthCallback(w http.ResponseWriter, r *http.Request) { } func (h *handler) oauthItem(w http.ResponseWriter, r *http.Request) (mcpcatalog.Item, bool) { - snapshot, err := h.deps.Catalog.Load(r.Context()) + catalog, err := h.deps.Catalog.Load() if err != nil { writeError(w, http.StatusServiceUnavailable, "mcp_catalog_unavailable") return mcpcatalog.Item{}, false } - item, found := snapshot.Find(chi.URLParam(r, "catalogID")) + item, found := catalog.Find(chi.URLParam(r, "catalogID")) if !found { writeError(w, http.StatusNotFound, "connector_not_found") return mcpcatalog.Item{}, false @@ -205,11 +205,11 @@ func (h *handler) decryptOAuthCookie(encoded string) (oauthCookie, error) { return oauthCookie{}, err } result := oauthCookie{ - WorkspaceID: stringField(payload, "workspace_id"), - CatalogID: stringField(payload, "catalog_id"), - UserID: stringField(payload, "user_id"), + WorkspaceID: metadataString(payload, "workspace_id"), + CatalogID: metadataString(payload, "catalog_id"), + UserID: metadataString(payload, "user_id"), } - if err := json.Unmarshal([]byte(stringField(payload, "transaction")), &result.Transaction); err != nil { + if err := json.Unmarshal([]byte(metadataString(payload, "transaction")), &result.Transaction); err != nil { return oauthCookie{}, err } return result, nil @@ -281,8 +281,3 @@ func (h *handler) clearOAuthCookie(w http.ResponseWriter, workspaceID, catalogID func oauthCookiePath(workspaceID, catalogID string) string { return "/api/v1/workspaces/" + url.PathEscape(workspaceID) + "/mcp-directory/" + url.PathEscape(catalogID) + "/oauth" } - -func stringField(payload map[string]any, key string) string { - value, _ := payload[key].(string) - return strings.TrimSpace(value) -} diff --git a/server/internal/api/mcpdirectory/oauth_scope.go b/server/internal/api/mcpdirectory/oauth_scope.go index 17f542f5..4c60446c 100644 --- a/server/internal/api/mcpdirectory/oauth_scope.go +++ b/server/internal/api/mcpdirectory/oauth_scope.go @@ -22,7 +22,7 @@ func (h *handler) saveWorkspaceOAuthCredential( if err != nil { return err } - existing, found, err := h.workspaceOAuthCredentialRead(ctx, workspaceID, item, false) + existing, found, err := h.workspaceOAuthCredentialRead(ctx, workspaceID, item) if err != nil { return err } @@ -39,9 +39,6 @@ func (h *handler) saveWorkspaceOAuthCredential( Masked: secrets.MaskPayload(payload), CreatedBy: createdBy, CredentialKindCode: item.Authentication.CredentialKind, - Metadata: map[string]any{ - "catalog_id": item.ID, - }, }, encrypted) return err } @@ -50,20 +47,17 @@ func (h *handler) workspaceOAuthCredentialRead( ctx context.Context, workspaceID string, item mcpcatalog.Item, - activeOnly bool, ) (store.SecretRead, bool, error) { workspaceSecrets, err := h.deps.WorkspaceCredentials.ListSecrets(ctx, workspaceID, 1000) if err != nil { return store.SecretRead{}, false, err } for _, candidate := range workspaceSecrets { - if activeOnly && candidate.Status != "active" { - continue - } if candidate.Kind != "capability_inline" || candidate.AuthType != "oauth2" || metadataString(candidate.Metadata, "workspace_id") != strings.TrimSpace(workspaceID) || - metadataString(candidate.Metadata, "catalog_id") != item.ID { + strings.TrimSpace(candidate.Provider) != item.ID || + metadataString(candidate.Metadata, "credential_kind_code") != item.Authentication.CredentialKind { continue } return candidate, true, nil diff --git a/server/internal/auth/mcpoauth/credential.go b/server/internal/auth/mcpoauth/credential.go index 680825f0..752cbd22 100644 --- a/server/internal/auth/mcpoauth/credential.go +++ b/server/internal/auth/mcpoauth/credential.go @@ -12,13 +12,17 @@ func (c Credential) Payload() map[string]any { payload := map[string]any{ "provider": CredentialProvider, "access_token": c.AccessToken, - "refresh_token": c.RefreshToken, "client_id": c.ClientID, - "client_secret": c.ClientSecret, "token_endpoint_auth_method": c.TokenEndpointAuthMethod, "token_endpoint": c.TokenEndpoint, "resource": c.Resource, } + if c.RefreshToken != "" { + payload["refresh_token"] = c.RefreshToken + } + if c.ClientSecret != "" { + payload["client_secret"] = c.ClientSecret + } if !c.ExpiresAt.IsZero() { payload["expires_at"] = c.ExpiresAt.UTC().Format(time.RFC3339) } diff --git a/server/internal/db/queries/store.sql b/server/internal/db/queries/store.sql index 1534ae21..3d891dd7 100644 --- a/server/internal/db/queries/store.sql +++ b/server/internal/db/queries/store.sql @@ -3592,11 +3592,9 @@ order by c.name asc, c.created_at desc; -- name: ListMCPDirectoryInstalls :many -- Catalog provenance lives on capability versions rather than the capability --- row. Keep the newest matching provenance per catalog id so a later catalog --- re-import can update catalog_version without creating a second install. +-- row. Keep the newest matching provenance per catalog id. select distinct on (cv.source_payload->>'catalog_id') coalesce(cv.source_payload->>'catalog_id', '')::text as catalog_id, - coalesce(cv.source_payload->>'catalog_version', '')::text as catalog_version, c.id::text as capability_id from capability c join capability_version cv on cv.capability_id = c.id diff --git a/server/internal/db/sqlc/store.sql.go b/server/internal/db/sqlc/store.sql.go index dea29370..6f73a0ac 100644 --- a/server/internal/db/sqlc/store.sql.go +++ b/server/internal/db/sqlc/store.sql.go @@ -8283,7 +8283,6 @@ func (q *Queries) ListIdleSandboxBindings(ctx context.Context, arg ListIdleSandb const listMCPDirectoryInstalls = `-- name: ListMCPDirectoryInstalls :many select distinct on (cv.source_payload->>'catalog_id') coalesce(cv.source_payload->>'catalog_id', '')::text as catalog_id, - coalesce(cv.source_payload->>'catalog_version', '')::text as catalog_version, c.id::text as capability_id from capability c join capability_version cv on cv.capability_id = c.id @@ -8296,14 +8295,12 @@ order by cv.source_payload->>'catalog_id', cv.created_at desc, cv.id desc ` type ListMCPDirectoryInstallsRow struct { - CatalogID string `json:"catalog_id"` - CatalogVersion string `json:"catalog_version"` - CapabilityID string `json:"capability_id"` + CatalogID string `json:"catalog_id"` + CapabilityID string `json:"capability_id"` } // Catalog provenance lives on capability versions rather than the capability -// row. Keep the newest matching provenance per catalog id so a later catalog -// re-import can update catalog_version without creating a second install. +// row. Keep the newest matching provenance per catalog id. func (q *Queries) ListMCPDirectoryInstalls(ctx context.Context, workspaceID pgtype.UUID) ([]ListMCPDirectoryInstallsRow, error) { rows, err := q.db.Query(ctx, listMCPDirectoryInstalls, workspaceID) if err != nil { @@ -8313,7 +8310,7 @@ func (q *Queries) ListMCPDirectoryInstalls(ctx context.Context, workspaceID pgty items := []ListMCPDirectoryInstallsRow{} for rows.Next() { var i ListMCPDirectoryInstallsRow - if err := rows.Scan(&i.CatalogID, &i.CatalogVersion, &i.CapabilityID); err != nil { + if err := rows.Scan(&i.CatalogID, &i.CapabilityID); err != nil { return nil, err } items = append(items, i) diff --git a/server/internal/dev/capability_routes.go b/server/internal/dev/capability_routes.go index 0acbcaf8..8cf6bae0 100644 --- a/server/internal/dev/capability_routes.go +++ b/server/internal/dev/capability_routes.go @@ -61,7 +61,8 @@ type credentialBody struct { } type agentCapabilityBody struct { - Configuration map[string]any `json:"configuration"` + Configuration map[string]any `json:"configuration"` + CredentialBindings map[string]string `json:"credential_bindings,omitempty"` // PinningMode is "latest" or "pinned". Empty falls back to the // store-side default (pinned), but the create/edit dialogs always // send a value so the server doesn't have to guess. @@ -1417,7 +1418,52 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid json"}) return } - enabled, err := runtimeStore.EnableAgentCapability(r.Context(), agentID, versionID, body.Configuration, body.PinningMode) + agentRecord, err := runtimeStore.GetAgent(r.Context(), agentID) + if err != nil { + writeCapabilityError(w, err, "failed to get agent") + return + } + requiredKinds := make(map[string]bool, len(version.RequiredCredentials)) + for _, required := range version.RequiredCredentials { + if required.Required { + requiredKinds[required.Kind] = true + } + } + bindings := make(map[string]string, len(body.CredentialBindings)) + for rawKind, rawSecretID := range body.CredentialBindings { + kind := strings.TrimSpace(rawKind) + secretID := strings.TrimSpace(rawSecretID) + if !requiredKinds[kind] { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding kind is not required by this capability"}) + return + } + if !isUUID(secretID) { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret_id must be a valid uuid"}) + return + } + secret, err := runtimeStore.GetSecretPayload(r.Context(), agent.WorkspaceID, secretID) + if err != nil || secret.Status != "active" || secret.Kind != "capability_inline" { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret is unavailable"}) + return + } + secretKind := strings.TrimSpace(metadataStringValue(secret.Metadata, "credential_kind_code")) + if secretKind != "" && secretKind != kind { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret has the wrong credential kind"}) + return + } + bindings[kind] = secretID + } + if agentRecord.Visibility == agentVisibilityPublic { + existing, _ := agentRecord.Config["credential_bindings"].(map[string]any) + for kind := range requiredKinds { + if bindings[kind] != "" || sharedCredentialBindingExists(existing[kind]) { + continue + } + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "public agents require a shared secret for every capability credential"}) + return + } + } + enabled, err := runtimeStore.EnableAgentCapability(r.Context(), agentID, versionID, body.Configuration, body.PinningMode, bindings) if err != nil { writeCapabilityError(w, err, "failed to enable agent capability") return @@ -1426,6 +1472,19 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { } } +func metadataStringValue(metadata map[string]any, key string) string { + value, _ := metadata[key].(string) + return value +} + +func sharedCredentialBindingExists(value any) bool { + binding, ok := value.(map[string]any) + if !ok || strings.TrimSpace(fmt.Sprint(binding["source"])) != "shared" { + return false + } + return isUUID(strings.TrimSpace(fmt.Sprint(binding["secret_id"]))) +} + // deleteAgentCapability uninstalls a capability version from the agent. // // @Summary Uninstall a capability from an agent diff --git a/server/internal/dev/routes.go b/server/internal/dev/routes.go index 75afd771..c128c063 100644 --- a/server/internal/dev/routes.go +++ b/server/internal/dev/routes.go @@ -79,7 +79,7 @@ type RuntimeStore interface { SoftDeleteUserCredential(ctx context.Context, credentialID string) (store.UserCredentialRead, error) ListAgentCapabilities(ctx context.Context, agentID string) ([]store.AgentCapabilityRead, error) GetEnabledMarketplaceCapabilitiesForAgent(ctx context.Context, agentID string) ([]store.EnabledCapabilityRead, error) - EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string) (store.AgentCapabilityRead, error) + EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string, credentialBindings map[string]string) (store.AgentCapabilityRead, error) UpgradeAgentCapability(ctx context.Context, agentID string, capabilityID string, newVersionID string, pinningMode string) (store.AgentCapabilityRead, error) UninstallWorkspaceMarketplaceCapability(ctx context.Context, targetWorkspaceID string, sourceCapabilityID string) (int64, error) DeleteAgentCapability(ctx context.Context, agentID string, capabilityVersionID string) error diff --git a/server/internal/dev/routes_agents.go b/server/internal/dev/routes_agents.go index f5f46608..2b413dd6 100644 --- a/server/internal/dev/routes_agents.go +++ b/server/internal/dev/routes_agents.go @@ -624,7 +624,7 @@ func syncAgentCapabilities( if cap.fromMarketplace { mode = store.PinningModePinned } - if _, err := rs.EnableAgentCapability(ctx, agentID, latestVersionID, nil, mode); err != nil { + if _, err := rs.EnableAgentCapability(ctx, agentID, latestVersionID, nil, mode, nil); err != nil { log.Bg().Warn("syncAgentCapabilities: enable failed, skipping", "capability_id", cap.capabilityID, "name", name, "version_id", latestVersionID, "err", err) continue diff --git a/server/internal/dev/routes_capability_test.go b/server/internal/dev/routes_capability_test.go index 5cbf180c..f63e953c 100644 --- a/server/internal/dev/routes_capability_test.go +++ b/server/internal/dev/routes_capability_test.go @@ -198,6 +198,37 @@ func TestCapabilityAgentEnableRBACWorkspaceAndUniqueUpdate(t *testing.T) { assertSingleAgentCapability(t, db, ownedPA, capID, v2) } +func TestCapabilityEnablePersistsSelectedSharedSecret(t *testing.T) { + r, db := capabilityTestRouter(t, map[string]string{testUserAID: "member"}, nil) + capID, versionID, _ := insertCapabilityVersions(t, db, store.DefaultDevFixtureIDs().WorkspaceID, "Shared Secret MCP") + agentID := insertAgentForOwner(t, db, testUserAID, "shared-secret-agent") + secretID := "00000000-0000-0000-0000-000000000099" + if _, err := db.Exec(context.Background(), ` + insert into secrets(id, slug, name, kind, provider, auth_type, encrypted_payload, key_version, status, metadata, created_by, created_at, updated_at) + values ($1, 'shared-github-test', 'Shared GitHub', 'capability_inline', 'inline', 'literal', '\x01'::bytea, 'v1', 'active', $2::jsonb, $3, now(), now()) + `, secretID, `{"workspace_id":"`+store.DefaultDevFixtureIDs().WorkspaceID+`","credential_kind_code":"github_pat"}`, testUserAID); err != nil { + t.Fatalf("insert shared secret: %v", err) + } + + res := serveCapabilityRoute(t, r, http.MethodPost, + "/api/v1/workspaces/"+store.DefaultDevFixtureIDs().WorkspaceID+"/agents/"+agentID+"/capabilities/"+versionID+"/enable", + `{"credential_bindings":{"github_pat":"`+secretID+`"}}`, testUserAID) + if res.Code != http.StatusOK { + t.Fatalf("enable expected 200, got %d: %s", res.Code, res.Body.String()) + } + assertSingleAgentCapability(t, db, agentID, capID, versionID) + var storedSecretID string + if err := db.QueryRow(context.Background(), ` + select config #>> '{credential_bindings,github_pat,secret_id}' + from agents where id = $1 + `, agentID).Scan(&storedSecretID); err != nil { + t.Fatalf("read stored binding: %v", err) + } + if storedSecretID != secretID { + t.Fatalf("stored secret_id=%q want %q", storedSecretID, secretID) + } +} + func TestCapabilityMarketplacePublishLifecycleSecretCheckAndDeleteRollback(t *testing.T) { r, db := capabilityTestRouter(t, map[string]string{store.DefaultDevFixtureIDs().UserID: "admin"}, nil) capID, _, _ := insertCapabilityVersions(t, db, store.DefaultDevFixtureIDs().WorkspaceID, "Marketplace Secret") diff --git a/server/internal/dev/routes_test.go b/server/internal/dev/routes_test.go index 8d204127..5bbc1b1c 100644 --- a/server/internal/dev/routes_test.go +++ b/server/internal/dev/routes_test.go @@ -2693,7 +2693,7 @@ func (stubRuntimeStore) GetEnabledMarketplaceCapabilitiesForAgent(ctx context.Co return []store.EnabledCapabilityRead{}, nil } -func (stubRuntimeStore) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string) (store.AgentCapabilityRead, error) { +func (stubRuntimeStore) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string, credentialBindings map[string]string) (store.AgentCapabilityRead, error) { return store.AgentCapabilityRead{ID: "00000000-0000-0000-0000-000000000c04", AgentID: agentID, CapabilityID: "00000000-0000-0000-0000-000000000c01", CapabilityVersionID: versionID, Enabled: true, PinningMode: pinningMode}, nil } diff --git a/server/internal/mcpcatalog/catalog_test.go b/server/internal/mcpcatalog/catalog_test.go index c0a6d906..d9cb8ce7 100644 --- a/server/internal/mcpcatalog/catalog_test.go +++ b/server/internal/mcpcatalog/catalog_test.go @@ -1,26 +1,22 @@ package mcpcatalog import ( - "context" "encoding/json" "strings" "testing" ) func TestBuiltinCatalogLoads(t *testing.T) { - snapshot, err := New(Options{}).Load(context.Background()) + catalog, err := New(Options{}).Load() if err != nil { t.Fatalf("Load: %v", err) } - if snapshot.Source != SourceBuiltin { - t.Fatalf("source = %q", snapshot.Source) - } want := []string{"context7", "exa", "firecrawl", "notion"} - if len(snapshot.Catalog.Items) != len(want) { - t.Fatalf("items = %d, want %d", len(snapshot.Catalog.Items), len(want)) + if len(catalog.Items) != len(want) { + t.Fatalf("items = %d, want %d", len(catalog.Items), len(want)) } for index, id := range want { - item := snapshot.Catalog.Items[index] + item := catalog.Items[index] if item.ID != id { t.Fatalf("item[%d] = %q, want %q", index, item.ID, id) } @@ -29,7 +25,7 @@ func TestBuiltinCatalogLoads(t *testing.T) { } if item.ID == "notion" { header := item.CanonicalSpec().MCP.Servers[0].Headers["Authorization"] - if header.Prefix != "Bearer " || header.CredentialKindCode != "notion_integration" { + if header.Prefix != "Bearer " || header.CredentialKindCode != "notion_mcp_oauth" { t.Fatalf("notion authorization header = %+v", header) } } diff --git a/server/internal/mcpcatalog/loader.go b/server/internal/mcpcatalog/loader.go index 050164ba..cd920f37 100644 --- a/server/internal/mcpcatalog/loader.go +++ b/server/internal/mcpcatalog/loader.go @@ -1,22 +1,12 @@ package mcpcatalog import ( - "context" "fmt" "strings" mcpcatalogdata "github.com/MiniMax-AI-Dev/parsar/catalog/mcp" ) -type Source string - -const SourceBuiltin Source = "builtin" - -type Snapshot struct { - Catalog Catalog - Source Source -} - type Options struct { BuiltinJSON []byte } @@ -35,16 +25,16 @@ func New(options Options) *Loader { return &Loader{builtin: builtin, builtinErr: builtinErr} } -func (l *Loader) Load(_ context.Context) (Snapshot, error) { +func (l *Loader) Load() (Catalog, error) { if l.builtinErr != nil { - return Snapshot{}, fmt.Errorf("load builtin catalog: %w", l.builtinErr) + return Catalog{}, fmt.Errorf("load builtin catalog: %w", l.builtinErr) } - return Snapshot{Catalog: l.builtin, Source: SourceBuiltin}, nil + return l.builtin, nil } -func (s Snapshot) Find(id string) (Item, bool) { +func (c Catalog) Find(id string) (Item, bool) { id = strings.TrimSpace(id) - for _, item := range s.Catalog.Items { + for _, item := range c.Items { if item.ID == id { return item, true } diff --git a/server/internal/store/capabilities.go b/server/internal/store/capabilities.go index 53ad6491..72c0e362 100644 --- a/server/internal/store/capabilities.go +++ b/server/internal/store/capabilities.go @@ -853,39 +853,107 @@ func normalizePinningMode(mode string) string { } } -func (s *Store) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string) (AgentCapabilityRead, error) { - version, err := s.GetCapabilityVersion(ctx, versionID) +func (s *Store) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string, credentialBindings map[string]string) (AgentCapabilityRead, error) { + agentUUID, err := uuid(agentID) if err != nil { return AgentCapabilityRead{}, err } - config, err := json.Marshal(nonNilMap(configuration)) + versionUUID, err := uuid(versionID) if err != nil { return AgentCapabilityRead{}, err } - mode := normalizePinningMode(pinningMode) - now := time.Now().UTC() - params := sqlc.CreateAgentCapabilityParams{ID: mustUUID(newID()), AgentID: mustUUID(agentID), CapabilityID: mustUUID(version.CapabilityID), CapabilityVersionID: mustUUID(version.ID), Enabled: true, Configuration: config, PinningMode: mode, Now: timestamptz(now)} - row, err := sqlc.New(s.db).CreateAgentCapability(ctx, params) - if err == nil { - return agentCapabilityFromCreateRow(row), nil + tx, err := beginTx(ctx, s.db) + if err != nil { + return AgentCapabilityRead{}, err + } + defer tx.Rollback(ctx) + queries := sqlc.New(tx) + versionRow, err := queries.GetCapabilityVersion(ctx, versionUUID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return AgentCapabilityRead{}, fmt.Errorf("%w: %s", ErrUnknownCapabilityVersion, versionID) + } + return AgentCapabilityRead{}, err + } + agentRow, err := queries.GetAgentForUpdate(ctx, agentUUID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return AgentCapabilityRead{}, fmt.Errorf("%w: %s", ErrUnknownAgent, agentID) + } + return AgentCapabilityRead{}, err + } + if len(credentialBindings) > 0 { + agentConfig := decodeJSONMap(agentRow.Config) + bindings, _ := agentConfig["credential_bindings"].(map[string]any) + bindings = cloneAnyMap(bindings) + for kind, secretID := range credentialBindings { + bindings[kind] = map[string]any{"source": "shared", "secret_id": secretID} + } + agentConfig["credential_bindings"] = bindings + encodedAgentConfig, err := json.Marshal(agentConfig) + if err != nil { + return AgentCapabilityRead{}, err + } + if _, err := queries.UpdateAgentCRUD(ctx, sqlc.UpdateAgentCRUDParams{ + ID: agentUUID, + Name: agentRow.Name, + Description: agentRow.Description, + ConnectorType: agentRow.ConnectorType, + Config: encodedAgentConfig, + Now: timestamptz(time.Now().UTC()), + }); err != nil { + return AgentCapabilityRead{}, err + } } - if !isUniqueViolation(err) { + capabilityConfig, err := json.Marshal(nonNilMap(configuration)) + if err != nil { return AgentCapabilityRead{}, err } - existingRows, err := sqlc.New(s.db).ListAgentCapabilitiesByAgent(ctx, mustUUID(agentID)) + mode := normalizePinningMode(pinningMode) + now := time.Now().UTC() + existingRows, err := queries.ListAgentCapabilitiesByAgent(ctx, agentUUID) if err != nil { return AgentCapabilityRead{}, err } + var enabled AgentCapabilityRead for _, existing := range existingRows { - if existing.CapabilityID == version.CapabilityID { - updated, err := sqlc.New(s.db).UpdateAgentCapability(ctx, sqlc.UpdateAgentCapabilityParams{ID: mustUUID(existing.ID), CapabilityVersionID: mustUUID(version.ID), Enabled: true, Configuration: config, PinningMode: mode, Now: timestamptz(now)}) - if err != nil { - return AgentCapabilityRead{}, err - } - return agentCapabilityFromUpdateRow(updated), nil + if existing.CapabilityID != versionRow.CapabilityID { + continue + } + updated, err := queries.UpdateAgentCapability(ctx, sqlc.UpdateAgentCapabilityParams{ + ID: mustUUID(existing.ID), + CapabilityVersionID: versionUUID, + Enabled: true, + Configuration: capabilityConfig, + PinningMode: mode, + Now: timestamptz(now), + }) + if err != nil { + return AgentCapabilityRead{}, err + } + enabled = agentCapabilityFromUpdateRow(updated) + break + } + if enabled.ID == "" { + created, err := queries.CreateAgentCapability(ctx, sqlc.CreateAgentCapabilityParams{ + ID: mustUUID(newID()), + AgentID: agentUUID, + CapabilityID: mustUUID(versionRow.CapabilityID), + CapabilityVersionID: versionUUID, + Enabled: true, + Configuration: capabilityConfig, + PinningMode: mode, + Now: timestamptz(now), + }) + if err != nil { + return AgentCapabilityRead{}, err } + enabled = agentCapabilityFromCreateRow(created) } - return AgentCapabilityRead{}, err + if err := tx.Commit(ctx); err != nil { + return AgentCapabilityRead{}, err + } + return enabled, nil } func (s *Store) UpgradeAgentCapability(ctx context.Context, agentID string, capabilityID string, newVersionID string, pinningMode string) (AgentCapabilityRead, error) { diff --git a/server/internal/store/capabilities_binding_test.go b/server/internal/store/capabilities_binding_test.go new file mode 100644 index 00000000..ee0a1736 --- /dev/null +++ b/server/internal/store/capabilities_binding_test.go @@ -0,0 +1,79 @@ +package store + +import ( + "context" + "testing" +) + +func TestEnableAgentCapabilityPersistsCredentialBindingAtomically(t *testing.T) { + db := openTestDB(t) + ctx := context.Background() + st := New(db) + ids := mustSeedDevFixture(t, ctx, st) + + capability, err := st.CreateCapability(ctx, CreateCapabilityInput{ + WorkspaceID: ids.WorkspaceID, + CreatorID: ids.UserID, + Type: "mcp", + Name: "atomic-credential-binding", + InitialVersion: &CreateCapabilityVersionInput{ + Version: "1.0.0", + CreatorID: ids.UserID, + Content: map[string]any{"mcpServers": map[string]any{"test": map[string]any{"command": "true"}}}, + }, + }) + if err != nil { + t.Fatalf("CreateCapability: %v", err) + } + versions, err := st.ListCapabilityVersions(ctx, capability.ID) + if err != nil || len(versions) != 1 { + t.Fatalf("ListCapabilityVersions: versions=%+v err=%v", versions, err) + } + agent, err := st.CreateAgent(ctx, CreateAgentInput{ + WorkspaceID: ids.WorkspaceID, + Name: "Atomic Binding Agent", + ConnectorType: "agent_daemon", + AgentConfig: map[string]any{"daemon_mode": "sandbox", "agent_kind": "opencode"}, + CreatedBy: ids.UserID, + }) + if err != nil { + t.Fatalf("CreateAgent: %v", err) + } + secretID := "00000000-0000-0000-0000-000000000099" + bindings := map[string]string{"notion_mcp_oauth": secretID} + + if _, err := st.EnableAgentCapability(ctx, agent.Agent.ID, versions[0].ID, nil, "invalid", bindings); err == nil { + t.Fatal("EnableAgentCapability with invalid pinning mode unexpectedly succeeded") + } + afterFailure, err := st.GetAgent(ctx, agent.Agent.ID) + if err != nil { + t.Fatalf("GetAgent after rollback: %v", err) + } + if _, exists := afterFailure.Config["credential_bindings"]; exists { + t.Fatalf("credential binding survived rolled-back enable: %+v", afterFailure.Config) + } + installed, err := st.ListAgentCapabilities(ctx, agent.Agent.ID) + if err != nil || len(installed) != 0 { + t.Fatalf("agent capability survived rolled-back enable: installed=%+v err=%v", installed, err) + } + + if _, err := st.EnableAgentCapability(ctx, agent.Agent.ID, versions[0].ID, nil, PinningModePinned, bindings); err != nil { + t.Fatalf("EnableAgentCapability: %v", err) + } + afterSuccess, err := st.GetAgent(ctx, agent.Agent.ID) + if err != nil { + t.Fatalf("GetAgent after success: %v", err) + } + storedBindings, ok := afterSuccess.Config["credential_bindings"].(map[string]any) + if !ok { + t.Fatalf("credential_bindings=%#v", afterSuccess.Config["credential_bindings"]) + } + stored, ok := storedBindings["notion_mcp_oauth"].(map[string]any) + if !ok || stored["source"] != "shared" || stored["secret_id"] != secretID { + t.Fatalf("stored binding=%#v", storedBindings["notion_mcp_oauth"]) + } + installed, err = st.ListAgentCapabilities(ctx, agent.Agent.ID) + if err != nil || len(installed) != 1 || installed[0].CapabilityID != capability.ID { + t.Fatalf("installed=%+v err=%v", installed, err) + } +} diff --git a/server/internal/store/capabilities_pinning_mode_test.go b/server/internal/store/capabilities_pinning_mode_test.go index 28bf4c52..ff12ca15 100644 --- a/server/internal/store/capabilities_pinning_mode_test.go +++ b/server/internal/store/capabilities_pinning_mode_test.go @@ -165,7 +165,7 @@ func TestGetEnabledCapabilitiesForAgent_PinningModeLatestFields(t *testing.T) { // cv.* still reflect v1 (we didn't rewrite capability_version_id); // latest_* still reflect v2; PinningMode is now "latest". The // daemon resolver's resolveVersionFields then picks v2 fields. - if _, err := st.EnableAgentCapability(ctx, created.Agent.ID, v1ID, nil, PinningModeLatest); err != nil { + if _, err := st.EnableAgentCapability(ctx, created.Agent.ID, v1ID, nil, PinningModeLatest, nil); err != nil { t.Fatalf("EnableAgentCapability flip to latest: %v", err) } enabled, err = st.GetEnabledCapabilitiesForAgent(ctx, created.Agent.ID) diff --git a/server/internal/store/credential_kinds.go b/server/internal/store/credential_kinds.go index 1c01a82a..2653ae92 100644 --- a/server/internal/store/credential_kinds.go +++ b/server/internal/store/credential_kinds.go @@ -22,6 +22,7 @@ var builtInCredentialKindSeeds = []builtInCredentialKindSeed{ {Code: "teams_app_password", DisplayName: "Teams App Password", Description: "Microsoft Teams Bot AAD client secret", Source: CredentialKindSourceUserDefined}, {Code: "postgres_dsn", DisplayName: "Postgres \u8fde\u63a5\u4e32", Description: "Postgres DSN", Source: CredentialKindSourceUserDefined}, {Code: "notion_integration", DisplayName: "Notion \u96c6\u6210 token", Description: "Notion Integration Token", Source: CredentialKindSourceUserDefined}, + {Code: "notion_mcp_oauth", DisplayName: "Notion MCP OAuth", Description: "Notion MCP OAuth access token", Source: CredentialKindSourcePlatformOAuth}, {Code: "jira_api_token", DisplayName: "Jira API Token", Description: "Atlassian Jira API Token", Source: CredentialKindSourceUserDefined}, {Code: "openai_api_key", DisplayName: "OpenAI API Key", Description: "Personal OpenAI API key (sk-...)", Source: CredentialKindSourcePlatformModel}, {Code: "anthropic_api_key", DisplayName: "Anthropic API Key", Description: "Personal Anthropic API key (sk-ant-...)", Source: CredentialKindSourcePlatformModel}, @@ -35,6 +36,7 @@ var SupportedCredentialKinds = []string{ "teams_app_password", "postgres_dsn", "notion_integration", + "notion_mcp_oauth", "jira_api_token", "openai_api_key", "anthropic_api_key", diff --git a/server/internal/store/mcp_directory.go b/server/internal/store/mcp_directory.go index 6361eed7..4a883e7d 100644 --- a/server/internal/store/mcp_directory.go +++ b/server/internal/store/mcp_directory.go @@ -8,12 +8,10 @@ import ( ) // MCPDirectoryInstall identifies the workspace capability created from one -// MCP Directory catalog item. CatalogVersion is retained for future update -// detection; v1 only reports it. +// MCP Directory catalog item. type MCPDirectoryInstall struct { - CatalogID string `json:"catalog_id"` - CatalogVersion string `json:"catalog_version"` - CapabilityID string `json:"capability_id"` + CatalogID string + CapabilityID string } func (s *Store) ListMCPDirectoryInstalls(ctx context.Context, workspaceID string) ([]MCPDirectoryInstall, error) { @@ -28,9 +26,8 @@ func (s *Store) ListMCPDirectoryInstalls(ctx context.Context, workspaceID string installs := make([]MCPDirectoryInstall, 0, len(rows)) for _, row := range rows { installs = append(installs, MCPDirectoryInstall{ - CatalogID: row.CatalogID, - CatalogVersion: row.CatalogVersion, - CapabilityID: row.CapabilityID, + CatalogID: row.CatalogID, + CapabilityID: row.CapabilityID, }) } return installs, nil diff --git a/server/internal/store/mcp_directory_test.go b/server/internal/store/mcp_directory_test.go index d0ca7b3b..9c3a7c9e 100644 --- a/server/internal/store/mcp_directory_test.go +++ b/server/internal/store/mcp_directory_test.go @@ -18,7 +18,7 @@ func TestMCPDirectoryImportPersistsProvenanceWithoutSecretsOrBindings(t *testing if err := db.QueryRow(ctx, `select count(*) from secrets`).Scan(&secretsBefore); err != nil { t.Fatal(err) } - source := json.RawMessage(`{"source_format":"mcp_catalog","catalog_id":"filesystem","catalog_version":"1.0.0","catalog_source":"builtin"}`) + source := json.RawMessage(`{"source_format":"mcp_catalog","catalog_id":"filesystem","catalog_version":"1.0.0"}`) result, err := st.ImportCapability(ctx, ImportCapabilityInput{ WorkspaceID: ids.WorkspaceID, Name: "Directory Filesystem", @@ -54,7 +54,7 @@ func TestMCPDirectoryImportPersistsProvenanceWithoutSecretsOrBindings(t *testing if err != nil { t.Fatalf("ListMCPDirectoryInstalls: %v", err) } - if len(installs) != 1 || installs[0].CatalogID != "filesystem" || installs[0].CatalogVersion != "1.0.0" || installs[0].CapabilityID != result.Capability.ID { + if len(installs) != 1 || installs[0].CatalogID != "filesystem" || installs[0].CapabilityID != result.Capability.ID { t.Fatalf("installs=%+v", installs) } @@ -80,7 +80,7 @@ func TestMCPDirectoryImportPersistsProvenanceWithoutSecretsOrBindings(t *testing if err := json.Unmarshal(stored, &provenance); err != nil { t.Fatal(err) } - if provenance["catalog_id"] != "filesystem" || provenance["catalog_source"] != "builtin" { + if provenance["catalog_id"] != "filesystem" || provenance["catalog_version"] != "1.0.0" { t.Fatalf("source_payload=%s", stored) } } diff --git a/server/internal/store/oauth_secret_test.go b/server/internal/store/oauth_secret_test.go index 8a9e95b9..780e271e 100644 --- a/server/internal/store/oauth_secret_test.go +++ b/server/internal/store/oauth_secret_test.go @@ -21,14 +21,13 @@ func TestCapabilitySecretIsScopedToWorkspaceAndCanRotate(t *testing.T) { AuthType: "oauth2", Masked: "configured", CreatedBy: ids.UserID, - CredentialKindCode: "notion_integration", - Metadata: map[string]any{"catalog_id": "notion"}, + CredentialKindCode: "notion_mcp_oauth", }, []byte(`{"token":"first"}`)) if err != nil { t.Fatalf("CreateSecret: %v", err) } - if created.Metadata["workspace_id"] != ids.WorkspaceID || created.Metadata["catalog_id"] != "notion" { - t.Fatalf("metadata = %+v", created.Metadata) + if created.Provider != "notion" || created.Metadata["workspace_id"] != ids.WorkspaceID { + t.Fatalf("secret = %+v", created) } otherWorkspaceID := "00000000-0000-0000-0000-000000000099" diff --git a/server/internal/store/store.go b/server/internal/store/store.go index fef8af92..8cfe74ea 100644 --- a/server/internal/store/store.go +++ b/server/internal/store/store.go @@ -711,7 +711,6 @@ type CreateSecretInput struct { // secret to a single credential_kinds.code. Used by the agent-creation // shared-binding picker to filter secrets by the kind they hold. CredentialKindCode string - Metadata map[string]any } type SecretRead struct { @@ -5711,11 +5710,7 @@ func (s *Store) ListWorkspaceUsageLogs(ctx context.Context, workspaceID string, func (s *Store) CreateSecret(ctx context.Context, input CreateSecretInput, encryptedPayload []byte) (SecretRead, error) { now := time.Now().UTC() createdBy := nullableUUID(input.CreatedBy) - metaPayload := make(map[string]any, len(input.Metadata)+3) - for key, value := range input.Metadata { - metaPayload[key] = value - } - metaPayload["masked"] = strings.TrimSpace(input.Masked) + metaPayload := map[string]any{"masked": strings.TrimSpace(input.Masked)} if code := strings.TrimSpace(input.CredentialKindCode); code != "" { metaPayload["credential_kind_code"] = code } diff --git a/server/migrations/000010_notion_mcp_oauth_credential_kind.sql b/server/migrations/000010_notion_mcp_oauth_credential_kind.sql new file mode 100644 index 00000000..32d25f99 --- /dev/null +++ b/server/migrations/000010_notion_mcp_oauth_credential_kind.sql @@ -0,0 +1,85 @@ +-- +goose Up + +INSERT INTO credential_kinds ( + code, display_name, description, source, built_in +) +VALUES ( + 'notion_mcp_oauth', + 'Notion MCP OAuth', + 'Notion MCP OAuth access token', + 'platform_oauth', + TRUE +) +ON CONFLICT DO NOTHING; + +UPDATE secrets +SET metadata = jsonb_set( + metadata, + '{credential_kind_code}', + '"notion_mcp_oauth"'::jsonb, + TRUE +) +WHERE kind = 'capability_inline' + AND provider = 'notion' + AND auth_type = 'oauth2' + AND metadata ->> 'credential_kind_code' = 'notion_integration'; + +UPDATE capability_version +SET canonical_spec = replace( + canonical_spec::text, + '"notion_integration"', + '"notion_mcp_oauth"' + )::jsonb, + required_credentials = replace( + required_credentials::text, + '"notion_integration"', + '"notion_mcp_oauth"' + )::jsonb, + source_payload = jsonb_set( + source_payload, + '{catalog_version}', + '"1.0.1"'::jsonb, + TRUE + ) +WHERE source_payload ->> 'source_format' = 'mcp_catalog' + AND source_payload ->> 'catalog_id' = 'notion' + AND canonical_spec::text LIKE '%"notion_integration"%'; + +-- +goose Down + +DELETE FROM credential_kinds +WHERE code = 'notion_mcp_oauth' + AND built_in = TRUE; + +UPDATE secrets +SET metadata = jsonb_set( + metadata, + '{credential_kind_code}', + '"notion_integration"'::jsonb, + TRUE +) +WHERE kind = 'capability_inline' + AND provider = 'notion' + AND auth_type = 'oauth2' + AND metadata ->> 'credential_kind_code' = 'notion_mcp_oauth'; + +UPDATE capability_version +SET canonical_spec = replace( + canonical_spec::text, + '"notion_mcp_oauth"', + '"notion_integration"' + )::jsonb, + required_credentials = replace( + required_credentials::text, + '"notion_mcp_oauth"', + '"notion_integration"' + )::jsonb, + source_payload = jsonb_set( + source_payload, + '{catalog_version}', + '"1.0.0"'::jsonb, + TRUE + ) +WHERE source_payload ->> 'source_format' = 'mcp_catalog' + AND source_payload ->> 'catalog_id' = 'notion' + AND canonical_spec::text LIKE '%"notion_mcp_oauth"%'; diff --git a/tests/e2e/mcp-directory.spec.ts b/tests/e2e/mcp-directory.spec.ts index 540cfc6f..2553d6a3 100644 --- a/tests/e2e/mcp-directory.spec.ts +++ b/tests/e2e/mcp-directory.spec.ts @@ -131,16 +131,12 @@ async function mockApp( }); if (path === `/api/v1/workspaces/${WORKSPACE_ID}/mcp-directory`) { if (directoryOverride && (await directoryOverride(route))) return; - return json(route, { - items: directoryItems, - updated_at: "2026-07-23T00:00:00Z", - source: "builtin", - }); + return json(route, { items: directoryItems }); } if (path === `/api/v1/workspaces/${WORKSPACE_ID}/mcp-directory/context7`) return json(route, { ...directoryItems[0], url: "https://mcp.context7.com/mcp" }); if (path === `/api/v1/workspaces/${WORKSPACE_ID}/mcp-directory/context7/import`) - return json(route, { installed: true, capability_id: CAPABILITY_ID, created: true }, 201); + return json(route, { installed: true, capability_id: CAPABILITY_ID }, 201); return json(route, {}); }); } From c638c6a3ec4dd6771f38179a1135c492cf5844ec Mon Sep 17 00:00:00 2001 From: kapelame Date: Thu, 23 Jul 2026 21:39:08 +0800 Subject: [PATCH 2/8] refactor: keep notion oauth migration minimal --- ...00010_notion_mcp_oauth_credential_kind.sql | 71 ++----------------- 1 file changed, 5 insertions(+), 66 deletions(-) diff --git a/server/migrations/000010_notion_mcp_oauth_credential_kind.sql b/server/migrations/000010_notion_mcp_oauth_credential_kind.sql index 32d25f99..3c09cbd3 100644 --- a/server/migrations/000010_notion_mcp_oauth_credential_kind.sql +++ b/server/migrations/000010_notion_mcp_oauth_credential_kind.sql @@ -1,5 +1,8 @@ -- +goose Up +-- OAuth payloads remain in the existing secrets table. This row only +-- registers the distinct kind used by Capability credential_ref validation. + INSERT INTO credential_kinds ( code, display_name, description, source, built_in ) @@ -12,74 +15,10 @@ VALUES ( ) ON CONFLICT DO NOTHING; -UPDATE secrets -SET metadata = jsonb_set( - metadata, - '{credential_kind_code}', - '"notion_mcp_oauth"'::jsonb, - TRUE -) -WHERE kind = 'capability_inline' - AND provider = 'notion' - AND auth_type = 'oauth2' - AND metadata ->> 'credential_kind_code' = 'notion_integration'; - -UPDATE capability_version -SET canonical_spec = replace( - canonical_spec::text, - '"notion_integration"', - '"notion_mcp_oauth"' - )::jsonb, - required_credentials = replace( - required_credentials::text, - '"notion_integration"', - '"notion_mcp_oauth"' - )::jsonb, - source_payload = jsonb_set( - source_payload, - '{catalog_version}', - '"1.0.1"'::jsonb, - TRUE - ) -WHERE source_payload ->> 'source_format' = 'mcp_catalog' - AND source_payload ->> 'catalog_id' = 'notion' - AND canonical_spec::text LIKE '%"notion_integration"%'; - -- +goose Down +-- Rollback only; normal startup does not execute this section. + DELETE FROM credential_kinds WHERE code = 'notion_mcp_oauth' AND built_in = TRUE; - -UPDATE secrets -SET metadata = jsonb_set( - metadata, - '{credential_kind_code}', - '"notion_integration"'::jsonb, - TRUE -) -WHERE kind = 'capability_inline' - AND provider = 'notion' - AND auth_type = 'oauth2' - AND metadata ->> 'credential_kind_code' = 'notion_mcp_oauth'; - -UPDATE capability_version -SET canonical_spec = replace( - canonical_spec::text, - '"notion_mcp_oauth"', - '"notion_integration"' - )::jsonb, - required_credentials = replace( - required_credentials::text, - '"notion_mcp_oauth"', - '"notion_integration"' - )::jsonb, - source_payload = jsonb_set( - source_payload, - '{catalog_version}', - '"1.0.0"'::jsonb, - TRUE - ) -WHERE source_payload ->> 'source_format' = 'mcp_catalog' - AND source_payload ->> 'catalog_id' = 'notion' - AND canonical_spec::text LIKE '%"notion_mcp_oauth"%'; From 882236d32ef63bae4ae8032cc95395bd7dd1417b Mon Sep 17 00:00:00 2001 From: kapelame <168134658+kapelame@users.noreply.github.com> Date: Thu, 23 Jul 2026 21:56:21 +0800 Subject: [PATCH 3/8] refactor: generalize MCP OAuth credential binding --- apps/web/src/lib/api-capabilities.ts | 9 +- apps/web/src/lib/api-types.ts | 1 - apps/web/src/lib/credential-kind-ui.ts | 10 +- .../src/pages/admin/agents/AgentConfigTab.tsx | 74 ++++++++---- catalog/mcp/catalog.json | 3 +- catalog/mcp/catalog.schema.json | 12 +- docs/openapi/openapi.yaml | 4 - server/internal/api/mcpdirectory/handler.go | 2 +- .../internal/api/mcpdirectory/handler_test.go | 6 +- .../internal/api/mcpdirectory/oauth_scope.go | 4 +- .../agentdaemon/capability_runtime.go | 4 + .../agentdaemon/capability_runtime_test.go | 92 ++++++++++++-- .../agentdaemon/model_injection_test.go | 6 +- server/internal/dev/capability_routes.go | 80 ++++++++++-- server/internal/dev/routes.go | 2 +- server/internal/dev/routes_agents.go | 2 +- server/internal/dev/routes_capability_test.go | 41 ++++++- server/internal/dev/routes_test.go | 2 +- server/internal/mcpcatalog/catalog_test.go | 8 +- server/internal/mcpcatalog/types.go | 10 +- server/internal/mcpcatalog/validate.go | 14 +-- server/internal/store/capabilities.go | 114 ++++-------------- .../store/capabilities_binding_test.go | 79 ------------ .../store/capabilities_pinning_mode_test.go | 2 +- server/internal/store/credential_kinds.go | 4 +- server/internal/store/oauth_secret_test.go | 2 +- ...l => 000010_mcp_oauth_credential_kind.sql} | 8 +- 27 files changed, 314 insertions(+), 281 deletions(-) delete mode 100644 server/internal/store/capabilities_binding_test.go rename server/migrations/{000010_notion_mcp_oauth_credential_kind.sql => 000010_mcp_oauth_credential_kind.sql} (79%) diff --git a/apps/web/src/lib/api-capabilities.ts b/apps/web/src/lib/api-capabilities.ts index f1b4d753..753a38ee 100644 --- a/apps/web/src/lib/api-capabilities.ts +++ b/apps/web/src/lib/api-capabilities.ts @@ -320,19 +320,14 @@ export function useEnableAgentCapabilityMutation( ) { const qc = useQueryClient() return useMutation({ - mutationFn: ({ capabilityVersionID, configuration, pinningMode, credentialBindings }: { capabilityVersionID: string; configuration?: Record; pinningMode?: "latest" | "pinned"; credentialBindings?: Record }) => { + mutationFn: ({ capabilityVersionID, configuration, pinningMode }: { capabilityVersionID: string; configuration?: Record; pinningMode?: "latest" | "pinned" }) => { if (!workspaceID || !agentID) throw new Error("workspace and agent are required") - return enableAgentCapability(workspaceID, agentID, capabilityVersionID, { - configuration, - credential_bindings: credentialBindings, - pinning_mode: pinningMode, - }) + return enableAgentCapability(workspaceID, agentID, capabilityVersionID, { configuration, pinning_mode: pinningMode }) }, retry: noUnreachableRetry, onSuccess: () => { if (workspaceID && agentID) { qc.invalidateQueries({ queryKey: KEY_AGENT_CAPABILITIES(workspaceID, agentID) }) - qc.invalidateQueries({ queryKey: ["admin", "agent", workspaceID, agentID] }) } }, }) diff --git a/apps/web/src/lib/api-types.ts b/apps/web/src/lib/api-types.ts index 0194fcb5..805201ab 100644 --- a/apps/web/src/lib/api-types.ts +++ b/apps/web/src/lib/api-types.ts @@ -348,7 +348,6 @@ export interface AgentCapability { export interface EnableAgentCapabilityRequest { configuration?: Record - credential_bindings?: Record /** See AgentCapability.pinning_mode. Empty defaults to "pinned" server-side. */ pinning_mode?: "latest" | "pinned" } diff --git a/apps/web/src/lib/credential-kind-ui.ts b/apps/web/src/lib/credential-kind-ui.ts index d486f06a..fb386b0b 100644 --- a/apps/web/src/lib/credential-kind-ui.ts +++ b/apps/web/src/lib/credential-kind-ui.ts @@ -34,9 +34,9 @@ export const CREDENTIAL_KIND_LABELS = { zh: "Notion 集成 token", en: "Notion Integration Token", }, - notion_mcp_oauth: { - zh: "Notion MCP OAuth", - en: "Notion MCP OAuth", + mcp_oauth: { + zh: "MCP OAuth", + en: "MCP OAuth", }, jira_api_token: { zh: "Jira API Token", @@ -51,7 +51,7 @@ export const CREDENTIAL_KIND_OPTIONS: KnownCredentialKind[] = [ "slack_bot_token", "postgres_dsn", "notion_integration", - "notion_mcp_oauth", + "mcp_oauth", "jira_api_token", ] @@ -71,7 +71,7 @@ export const CREDENTIAL_KIND_META: Record { const secretKind = secretCredentialKind(secret) - return secret.kind === "capability_inline" + const matchesKind = secret.kind === "capability_inline" && secret.status === "active" && (secretKind === "" || secretKind === kind) + if (!matchesKind) return false + if (kind !== "mcp_oauth") return true + return secretKind === kind && !!catalogID + && secret.auth_type === "oauth2" + && secret.provider === catalogID }) } -function sharedSecretBindingID(agent: Agent, kind: string) { - const bindings = agent.config?.credential_bindings - if (!bindings || typeof bindings !== "object" || Array.isArray(bindings)) return "" +function credentialBinding(config: Record | undefined, kind: string) { + const bindings = config?.credential_bindings + if (!bindings || typeof bindings !== "object" || Array.isArray(bindings)) return undefined const binding = (bindings as Record)[kind] - if (!binding || typeof binding !== "object" || Array.isArray(binding)) return "" + if (!binding || typeof binding !== "object" || Array.isArray(binding)) return undefined const value = binding as Record - return value.source === "shared" && typeof value.secret_id === "string" ? value.secret_id : "" + if (value.source !== "personal" && value.source !== "shared") return undefined + return { + source: value.source, + secretID: value.source === "shared" && typeof value.secret_id === "string" ? value.secret_id : "", + } +} + +function boundSharedSecretID(agent: Agent, binding: AgentCapability | undefined, kind: string) { + const capabilityBinding = credentialBinding(binding?.configuration, kind) + if (capabilityBinding) return capabilityBinding.secretID + return credentialBinding(agent.config, kind)?.secretID ?? "" } -function hasUsableCredential(agent: Agent, credentials: UserCredential[], sharedSecrets: Secret[], kind: string) { - const sharedID = sharedSecretBindingID(agent, kind) - if (sharedID && sharedSecrets.some((secret) => secret.id === sharedID)) return true +function hasUsableCredential(agent: Agent, binding: AgentCapability | undefined, credentials: UserCredential[], sharedSecrets: Secret[], kind: string, catalogID: string) { + const sharedID = boundSharedSecretID(agent, binding, kind) + if (sharedID && sharedSecretsForKind(sharedSecrets, kind, catalogID).some((secret) => secret.id === sharedID)) return true return agent.visibility !== "public" && hasCredentialKind(credentials, kind) } @@ -220,9 +235,10 @@ function CapabilityCard({ const binding = item.binding const { latest, versions, versionsQ } = useCapabilityVersions(workspaceID, capability, mode === "enabled") const boundVersion = versions.find((version) => version.id === binding?.capability_version_id) ?? (binding?.capability_version_id && capability?.pinned_version ? { id: binding.capability_version_id, capability_id: capability.id, version: capability.pinned_version, created_at: capability.latest_version_created_at ?? capability.created_at } as CapabilityVersion : undefined) + const catalogID = catalogIDFromVersion(boundVersion ?? latest) const versionDeleted = !!binding && !versionsQ.isLoading && !boundVersion && !capability?.latest_version_id const missingCredential = capability - ? requiredCredentialKinds(capability).some((rc) => !hasUsableCredential(agent, credentials, sharedSecrets, rc.kind)) + ? requiredCredentialKinds(capability).some((rc) => !hasUsableCredential(agent, binding, credentials, sharedSecrets, rc.kind, catalogID)) : false const fromMarketplace = !!capability?.from_marketplace || (!!capability?.source_workspace_id && capability.source_workspace_id !== workspaceID) const deprecated = !!capability?.deprecated_at @@ -291,7 +307,7 @@ function CapabilityCard({ /> )} - +
@@ -341,14 +357,18 @@ function CapabilityCard({ function CredentialStatus({ capability, + binding, agent, credentials, sharedSecrets, + catalogID, }: { capability: Capability + binding?: AgentCapability agent: Agent credentials: UserCredential[] sharedSecrets: Secret[] + catalogID: string }) { const { t, i18n } = useTranslation("admin") const requiredCreds = capability.required_credentials ?? [] @@ -358,8 +378,8 @@ function CredentialStatus({ return (
{requiredCreds.map((rc) => { - const sharedID = sharedSecretBindingID(agent, rc.kind) - const sharedSecret = sharedSecrets.find((secret) => secret.id === sharedID) + const sharedID = boundSharedSecretID(agent, binding, rc.kind) + const sharedSecret = sharedSecretsForKind(sharedSecrets, rc.kind, catalogID).find((secret) => secret.id === sharedID) const credential = agent.visibility === "public" ? undefined : credentials.find((cred) => cred.kind === rc.kind) const available = sharedSecret ?? credential const label = credentialKindLabel(rc.kind, i18n.language, rc.kind) @@ -421,6 +441,7 @@ function EnableCredentialBindingList({ requiredKinds, credentials, sharedSecrets, + catalogID, publicAgent, bindings, onChange, @@ -428,6 +449,7 @@ function EnableCredentialBindingList({ requiredKinds: { kind: string }[] credentials: UserCredential[] sharedSecrets: Secret[] + catalogID: string publicAgent: boolean bindings: Record onChange: (kind: string, secretID: string) => void @@ -436,7 +458,7 @@ function EnableCredentialBindingList({ return (
{requiredKinds.map((rc) => { - const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind) + const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind, catalogID) const selectedSecretID = bindings[rc.kind] ?? "" const hasPersonal = !publicAgent && hasCredentialKind(credentials, rc.kind) const ready = !!selectedSecretID || hasPersonal @@ -521,10 +543,8 @@ function CapabilityVersionDialog({ const defaultCredentialBindings = useMemo(() => { const defaults: Record = {} for (const rc of requiredKinds) { - const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind) - const oauthSecret = catalogID - ? kindSecrets.find((secret) => secret.auth_type === "oauth2" && secret.provider === catalogID) - : undefined + const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind, catalogID) + const oauthSecret = kindSecrets.find((secret) => rc.kind === "mcp_oauth") if (oauthSecret) defaults[rc.kind] = oauthSecret.id else if (agent.visibility === "public" && kindSecrets[0]) defaults[rc.kind] = kindSecrets[0].id } @@ -533,7 +553,7 @@ function CapabilityVersionDialog({ const credentialBindings = { ...defaultCredentialBindings, ...credentialBindingChoices } const missingRequiredCredential = requiredKinds.some((rc) => { const selectedSecretID = credentialBindings[rc.kind] - if (selectedSecretID && sharedSecretsForKind(sharedSecrets, rc.kind).some((secret) => secret.id === selectedSecretID)) { + if (selectedSecretID && sharedSecretsForKind(sharedSecrets, rc.kind, catalogID).some((secret) => secret.id === selectedSecretID)) { return false } return agent.visibility === "public" || !hasCredentialKind(credentials, rc.kind) @@ -544,12 +564,19 @@ function CapabilityVersionDialog({ const submit = () => { if (!selectedVersion) return - const sharedBindings = Object.fromEntries( - Object.entries(credentialBindings).filter(([, secretID]) => secretID !== ""), + const capabilityBindings = Object.fromEntries( + requiredKinds.map(({ kind }) => { + const secretID = credentialBindings[kind] + return [kind, secretID + ? { source: "shared", secret_id: secretID } + : { source: "personal" }] + }), ) mut.mutate({ capabilityVersionID: selectedVersion.id, - credentialBindings: mode === "enable" ? sharedBindings : undefined, + configuration: mode === "enable" + ? { credential_bindings: capabilityBindings } + : binding?.configuration, }, { onSuccess: () => { setOpen(false) @@ -600,6 +627,7 @@ function CapabilityVersionDialog({ requiredKinds={requiredKinds} credentials={credentials} sharedSecrets={sharedSecrets} + catalogID={catalogID} publicAgent={agent.visibility === "public"} bindings={credentialBindings} onChange={(kind, secretID) => setCredentialBindingChoices((current) => ({ ...current, [kind]: secretID }))} diff --git a/catalog/mcp/catalog.json b/catalog/mcp/catalog.json index fe232983..1e62800d 100644 --- a/catalog/mcp/catalog.json +++ b/catalog/mcp/catalog.json @@ -81,8 +81,7 @@ "version": "1.0.1", "transport": "streamable-http", "authentication": { - "type": "oauth2", - "credential_kind": "notion_mcp_oauth" + "type": "oauth2" }, "server": { "name": "notion", diff --git a/catalog/mcp/catalog.schema.json b/catalog/mcp/catalog.schema.json index e608447d..318e516a 100644 --- a/catalog/mcp/catalog.schema.json +++ b/catalog/mcp/catalog.schema.json @@ -42,16 +42,8 @@ "additionalProperties": false, "required": ["type"], "properties": { - "type": { "enum": ["none", "oauth2"] }, - "credential_kind": { - "type": "string", - "pattern": "^[a-z][a-z0-9_]*$" - } - }, - "if": { - "properties": { "type": { "const": "oauth2" } } - }, - "then": { "required": ["credential_kind"] } + "type": { "enum": ["none", "oauth2"] } + } }, "item": { "type": "object", diff --git a/docs/openapi/openapi.yaml b/docs/openapi/openapi.yaml index 0825c0a7..edd898cc 100644 --- a/docs/openapi/openapi.yaml +++ b/docs/openapi/openapi.yaml @@ -242,10 +242,6 @@ definitions: configuration: additionalProperties: {} type: object - credential_bindings: - additionalProperties: - type: string - type: object pinning_mode: description: |- PinningMode is "latest" or "pinned". Empty falls back to the diff --git a/server/internal/api/mcpdirectory/handler.go b/server/internal/api/mcpdirectory/handler.go index 5f7c0e66..4097f440 100644 --- a/server/internal/api/mcpdirectory/handler.go +++ b/server/internal/api/mcpdirectory/handler.go @@ -347,7 +347,7 @@ func (h *handler) connectedCatalogIDs(w http.ResponseWriter, r *http.Request, wo catalogID := strings.TrimSpace(candidate.Provider) item, found := catalog.Find(catalogID) if !found || item.Authentication.EffectiveType() != "oauth2" || - metadataString(candidate.Metadata, "credential_kind_code") != item.Authentication.CredentialKind { + metadataString(candidate.Metadata, "credential_kind_code") != mcpcatalog.OAuthCredentialKind { continue } result[catalogID] = true diff --git a/server/internal/api/mcpdirectory/handler_test.go b/server/internal/api/mcpdirectory/handler_test.go index 46d3864a..d2cdcfca 100644 --- a/server/internal/api/mcpdirectory/handler_test.go +++ b/server/internal/api/mcpdirectory/handler_test.go @@ -146,7 +146,7 @@ func TestOAuthDirectoryItemRequiresWorkspaceConnectionBeforeImport(t *testing.T) Publisher: mcpcatalog.Publisher{Name: "Notion", URL: "https://www.notion.so"}, Verified: true, Categories: []string{"Productivity"}, FeaturedRank: 1, Version: "1.0.0", Transport: "streamable-http", - Authentication: mcpcatalog.Authentication{Type: "oauth2", CredentialKind: "notion_mcp_oauth"}, + Authentication: mcpcatalog.Authentication{Type: "oauth2"}, Server: mcpcatalog.Server{Name: "notion", URL: "https://mcp.notion.com/mcp"}, }} fs := &fakeDirectoryStore{role: "admin"} @@ -167,14 +167,14 @@ func TestOAuthDirectoryItemRequiresWorkspaceConnectionBeforeImport(t *testing.T) credentials.secrets = []store.SecretRead{{ ID: "secret-2", Kind: "capability_inline", Provider: "notion", AuthType: "oauth2", Status: "active", - Metadata: map[string]any{"workspace_id": testWorkspaceID, "credential_kind_code": "notion_mcp_oauth"}, + Metadata: map[string]any{"workspace_id": testWorkspaceID, "credential_kind_code": mcpcatalog.OAuthCredentialKind}, }} rec = requestWithDeps(t, fs, credentials, catalog, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") if rec.Code != http.StatusCreated { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } header := fs.imported.Spec.MCP.Servers[0].Headers["Authorization"] - if header.Prefix != "Bearer " || header.CredentialKindCode != "notion_mcp_oauth" { + if header.Prefix != "Bearer " || header.CredentialKindCode != mcpcatalog.OAuthCredentialKind { t.Fatalf("authorization header = %+v", header) } } diff --git a/server/internal/api/mcpdirectory/oauth_scope.go b/server/internal/api/mcpdirectory/oauth_scope.go index 4c60446c..b3b93e18 100644 --- a/server/internal/api/mcpdirectory/oauth_scope.go +++ b/server/internal/api/mcpdirectory/oauth_scope.go @@ -38,7 +38,7 @@ func (h *handler) saveWorkspaceOAuthCredential( AuthType: "oauth2", Masked: secrets.MaskPayload(payload), CreatedBy: createdBy, - CredentialKindCode: item.Authentication.CredentialKind, + CredentialKindCode: mcpcatalog.OAuthCredentialKind, }, encrypted) return err } @@ -57,7 +57,7 @@ func (h *handler) workspaceOAuthCredentialRead( candidate.AuthType != "oauth2" || metadataString(candidate.Metadata, "workspace_id") != strings.TrimSpace(workspaceID) || strings.TrimSpace(candidate.Provider) != item.ID || - metadataString(candidate.Metadata, "credential_kind_code") != item.Authentication.CredentialKind { + metadataString(candidate.Metadata, "credential_kind_code") != mcpcatalog.OAuthCredentialKind { continue } return candidate, true, nil diff --git a/server/internal/connector/agentdaemon/capability_runtime.go b/server/internal/connector/agentdaemon/capability_runtime.go index a56d9ded..f022b630 100644 --- a/server/internal/connector/agentdaemon/capability_runtime.go +++ b/server/internal/connector/agentdaemon/capability_runtime.go @@ -583,6 +583,10 @@ func (c *Connector) resolveMCPCapability( // table lookup) or, by default, per-user user_credentials keyed by // the conversation initiator. bindings := ParseCredentialBindings(in.AgentConfig) + // A binding configured on this Agent-Capability pair overrides the + // legacy agent-wide default, allowing two OAuth connectors to use + // different workspace Secrets even though both use mcp_oauth. + mergeBindings(bindings, cap.Configuration) credentialValues, sharedSecretIDs, missing, err := c.resolveCredentialValues(ctx, in, cap, credentialCache, bindings) if err != nil { return nil, nil, nil, err diff --git a/server/internal/connector/agentdaemon/capability_runtime_test.go b/server/internal/connector/agentdaemon/capability_runtime_test.go index 25f6f7ed..28125dac 100644 --- a/server/internal/connector/agentdaemon/capability_runtime_test.go +++ b/server/internal/connector/agentdaemon/capability_runtime_test.go @@ -12,6 +12,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/server/internal/auth/mcpoauth" "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" "github.com/MiniMax-AI-Dev/parsar/server/internal/connector" + "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" ) @@ -468,10 +469,18 @@ func TestResolveCapabilityAdditions_UsesWorkspaceOAuthHeader(t *testing.T) { "Authorization": { Mode: canonical.EnvModeCredentialRef, Prefix: "Bearer ", - CredentialKindCode: "notion_integration", + CredentialKindCode: mcpcatalog.OAuthCredentialKind, }, }, - }}, []store.RequiredCredential{{Kind: "notion_integration", Required: true}}) + }}, []store.RequiredCredential{{Kind: mcpcatalog.OAuthCredentialKind, Required: true}}) + row.Configuration = map[string]any{ + "credential_bindings": map[string]any{ + mcpcatalog.OAuthCredentialKind: map[string]any{ + "source": "shared", + "secret_id": "secret-1", + }, + }, + } c := &Connector{ capabilities: stubCapabilityStore{rows: []store.EnabledCapabilityRead{row}}, modelResolver: &fakeModelResolver{secret: store.SecretPayload{ @@ -483,14 +492,6 @@ func TestResolveCapabilityAdditions_UsesWorkspaceOAuthHeader(t *testing.T) { } in := defaultPromptInput() in.ConversationInitiatorID = "" - in.AgentConfig = map[string]any{ - "credential_bindings": map[string]any{ - "notion_integration": map[string]any{ - "source": "shared", - "secret_id": "secret-1", - }, - }, - } got, err := c.resolveCapabilityAdditions(context.Background(), in, "claude_code") if err != nil { t.Fatalf("resolveCapabilityAdditions: %v", err) @@ -501,6 +502,77 @@ func TestResolveCapabilityAdditions_UsesWorkspaceOAuthHeader(t *testing.T) { } } +func TestResolveCapabilityAdditions_UsesCapabilityScopedOAuthBindings(t *testing.T) { + svc := testSecretsService(t) + newCredential := func(token, resource string) mcpoauth.Credential { + return mcpoauth.Credential{ + AccessToken: token, + ClientID: "client-1", + TokenEndpointAuthMethod: "none", + TokenEndpoint: resource + "/token", + Resource: resource, + } + } + newRow := func(id, name, serverName, url, secretID string) store.EnabledCapabilityRead { + row := newMCPRow(t, id, name, []canonical.MCPServer{{ + Name: serverName, + Transport: canonical.MCPTransportStreamableHTTP, + URL: url, + Headers: map[string]canonical.EnvValue{ + "Authorization": { + Mode: canonical.EnvModeCredentialRef, + Prefix: "Bearer ", + CredentialKindCode: mcpcatalog.OAuthCredentialKind, + }, + }, + }}, []store.RequiredCredential{{Kind: mcpcatalog.OAuthCredentialKind, Required: true}}) + row.Configuration = map[string]any{ + "credential_bindings": map[string]any{ + mcpcatalog.OAuthCredentialKind: map[string]any{ + "source": "shared", + "secret_id": secretID, + }, + }, + } + return row + } + notionURL := "https://mcp.notion.com/mcp" + githubURL := "https://api.githubcopilot.com/mcp" + resolver := &fakeModelResolver{secrets: map[string]store.SecretPayload{ + "secret-notion": { + SecretRead: store.SecretRead{ID: "secret-notion", Status: "active"}, + EncryptedPayload: encryptPayload(t, svc, newCredential("notion-token", notionURL).Payload()), + }, + "secret-github": { + SecretRead: store.SecretRead{ID: "secret-github", Status: "active"}, + EncryptedPayload: encryptPayload(t, svc, newCredential("github-token", githubURL).Payload()), + }, + }} + c := &Connector{ + capabilities: stubCapabilityStore{rows: []store.EnabledCapabilityRead{ + newRow("mcp-notion", "Notion", "notion", notionURL, "secret-notion"), + newRow("mcp-github", "GitHub", "github", githubURL, "secret-github"), + }}, + modelResolver: resolver, + secrets: svc, + log: discardLogger(), + } + in := defaultPromptInput() + in.ConversationInitiatorID = "" + got, err := c.resolveCapabilityAdditions(context.Background(), in, "claude_code") + if err != nil { + t.Fatalf("resolveCapabilityAdditions: %v", err) + } + notionHeaders := got.MCPServers["notion"].(map[string]any)["headers"].(map[string]string) + githubHeaders := got.MCPServers["github"].(map[string]any)["headers"].(map[string]string) + if notionHeaders["Authorization"] != "Bearer notion-token" { + t.Fatalf("notion Authorization = %q", notionHeaders["Authorization"]) + } + if githubHeaders["Authorization"] != "Bearer github-token" { + t.Fatalf("github Authorization = %q", githubHeaders["Authorization"]) + } +} + func TestResolveCapabilityAdditions_MCPWithCredential(t *testing.T) { svc := testSecretsService(t) ciphertext := encryptPayload(t, svc, map[string]any{"token": "ghp_realtoken123"}) diff --git a/server/internal/connector/agentdaemon/model_injection_test.go b/server/internal/connector/agentdaemon/model_injection_test.go index 499bcf39..4e4520be 100644 --- a/server/internal/connector/agentdaemon/model_injection_test.go +++ b/server/internal/connector/agentdaemon/model_injection_test.go @@ -18,6 +18,7 @@ import ( type fakeModelResolver struct { runtime store.ModelRuntime secret store.SecretPayload + secrets map[string]store.SecretPayload modelErr error secretErr error @@ -60,10 +61,13 @@ func (f *fakeModelResolver) ResolveModelRuntimeForUser(_ context.Context, _, _ s return f.runtime, nil } -func (f *fakeModelResolver) GetSecretPayload(_ context.Context, _, _ string) (store.SecretPayload, error) { +func (f *fakeModelResolver) GetSecretPayload(_ context.Context, _, secretID string) (store.SecretPayload, error) { if f.secretErr != nil { return store.SecretPayload{}, f.secretErr } + if secret, ok := f.secrets[secretID]; ok { + return secret, nil + } return f.secret, nil } diff --git a/server/internal/dev/capability_routes.go b/server/internal/dev/capability_routes.go index 8cf6bae0..76cd64f3 100644 --- a/server/internal/dev/capability_routes.go +++ b/server/internal/dev/capability_routes.go @@ -15,6 +15,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/server/internal/auth" "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" + "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" "github.com/go-chi/chi/v5" @@ -61,8 +62,7 @@ type credentialBody struct { } type agentCapabilityBody struct { - Configuration map[string]any `json:"configuration"` - CredentialBindings map[string]string `json:"credential_bindings,omitempty"` + Configuration map[string]any `json:"configuration"` // PinningMode is "latest" or "pinned". Empty falls back to the // store-side default (pinned), but the create/edit dialogs always // send a value so the server doesn't have to guess. @@ -1429,14 +1429,22 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { requiredKinds[required.Kind] = true } } - bindings := make(map[string]string, len(body.CredentialBindings)) - for rawKind, rawSecretID := range body.CredentialBindings { + bindings, err := capabilityCredentialBindings(body.Configuration) + if err != nil { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": err.Error()}) + return + } + catalogID := catalogIDFromSourcePayload(version.SourcePayload) + for rawKind, binding := range bindings { kind := strings.TrimSpace(rawKind) - secretID := strings.TrimSpace(rawSecretID) if !requiredKinds[kind] { writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding kind is not required by this capability"}) return } + if binding.Source == "personal" { + continue + } + secretID := binding.SecretID if !isUUID(secretID) { writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret_id must be a valid uuid"}) return @@ -1451,19 +1459,30 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret has the wrong credential kind"}) return } - bindings[kind] = secretID + if kind == mcpcatalog.OAuthCredentialKind && catalogID != "" && + (secretKind != kind || secret.AuthType != "oauth2" || strings.TrimSpace(secret.Provider) != catalogID) { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret belongs to a different MCP connector"}) + return + } } if agentRecord.Visibility == agentVisibilityPublic { existing, _ := agentRecord.Config["credential_bindings"].(map[string]any) for kind := range requiredKinds { - if bindings[kind] != "" || sharedCredentialBindingExists(existing[kind]) { + if binding, explicitlyConfigured := bindings[kind]; explicitlyConfigured { + if binding.Source == "shared" { + continue + } + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "public agents require a shared secret for every capability credential"}) + return + } + if sharedCredentialBindingExists(existing[kind]) { continue } writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "public agents require a shared secret for every capability credential"}) return } } - enabled, err := runtimeStore.EnableAgentCapability(r.Context(), agentID, versionID, body.Configuration, body.PinningMode, bindings) + enabled, err := runtimeStore.EnableAgentCapability(r.Context(), agentID, versionID, body.Configuration, body.PinningMode) if err != nil { writeCapabilityError(w, err, "failed to enable agent capability") return @@ -1472,11 +1491,56 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { } } +type capabilityCredentialBinding struct { + Source string + SecretID string +} + +func capabilityCredentialBindings(configuration map[string]any) (map[string]capabilityCredentialBinding, error) { + result := map[string]capabilityCredentialBinding{} + raw, exists := configuration["credential_bindings"] + if !exists || raw == nil { + return result, nil + } + bindings, ok := raw.(map[string]any) + if !ok { + return nil, fmt.Errorf("configuration.credential_bindings must be an object") + } + for rawKind, rawBinding := range bindings { + kind := strings.TrimSpace(rawKind) + binding, ok := rawBinding.(map[string]any) + source := strings.TrimSpace(fmt.Sprint(binding["source"])) + if kind == "" || !ok || (source != "personal" && source != "shared") { + return nil, fmt.Errorf("configuration.credential_bindings entries must use personal or shared source") + } + secretID := strings.TrimSpace(fmt.Sprint(binding["secret_id"])) + if source == "shared" && secretID == "" { + return nil, fmt.Errorf("configuration.credential_bindings[%s].secret_id is required", kind) + } + result[kind] = capabilityCredentialBinding{Source: source, SecretID: secretID} + } + return result, nil +} + func metadataStringValue(metadata map[string]any, key string) string { value, _ := metadata[key].(string) return value } +func catalogIDFromSourcePayload(sourcePayload json.RawMessage) string { + if len(sourcePayload) == 0 { + return "" + } + var source struct { + SourceFormat string `json:"source_format"` + CatalogID string `json:"catalog_id"` + } + if err := json.Unmarshal(sourcePayload, &source); err != nil || source.SourceFormat != "mcp_catalog" { + return "" + } + return strings.TrimSpace(source.CatalogID) +} + func sharedCredentialBindingExists(value any) bool { binding, ok := value.(map[string]any) if !ok || strings.TrimSpace(fmt.Sprint(binding["source"])) != "shared" { diff --git a/server/internal/dev/routes.go b/server/internal/dev/routes.go index c128c063..75afd771 100644 --- a/server/internal/dev/routes.go +++ b/server/internal/dev/routes.go @@ -79,7 +79,7 @@ type RuntimeStore interface { SoftDeleteUserCredential(ctx context.Context, credentialID string) (store.UserCredentialRead, error) ListAgentCapabilities(ctx context.Context, agentID string) ([]store.AgentCapabilityRead, error) GetEnabledMarketplaceCapabilitiesForAgent(ctx context.Context, agentID string) ([]store.EnabledCapabilityRead, error) - EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string, credentialBindings map[string]string) (store.AgentCapabilityRead, error) + EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string) (store.AgentCapabilityRead, error) UpgradeAgentCapability(ctx context.Context, agentID string, capabilityID string, newVersionID string, pinningMode string) (store.AgentCapabilityRead, error) UninstallWorkspaceMarketplaceCapability(ctx context.Context, targetWorkspaceID string, sourceCapabilityID string) (int64, error) DeleteAgentCapability(ctx context.Context, agentID string, capabilityVersionID string) error diff --git a/server/internal/dev/routes_agents.go b/server/internal/dev/routes_agents.go index 2b413dd6..f5f46608 100644 --- a/server/internal/dev/routes_agents.go +++ b/server/internal/dev/routes_agents.go @@ -624,7 +624,7 @@ func syncAgentCapabilities( if cap.fromMarketplace { mode = store.PinningModePinned } - if _, err := rs.EnableAgentCapability(ctx, agentID, latestVersionID, nil, mode, nil); err != nil { + if _, err := rs.EnableAgentCapability(ctx, agentID, latestVersionID, nil, mode); err != nil { log.Bg().Warn("syncAgentCapabilities: enable failed, skipping", "capability_id", cap.capabilityID, "name", name, "version_id", latestVersionID, "err", err) continue diff --git a/server/internal/dev/routes_capability_test.go b/server/internal/dev/routes_capability_test.go index f63e953c..89e138c5 100644 --- a/server/internal/dev/routes_capability_test.go +++ b/server/internal/dev/routes_capability_test.go @@ -212,15 +212,15 @@ func TestCapabilityEnablePersistsSelectedSharedSecret(t *testing.T) { res := serveCapabilityRoute(t, r, http.MethodPost, "/api/v1/workspaces/"+store.DefaultDevFixtureIDs().WorkspaceID+"/agents/"+agentID+"/capabilities/"+versionID+"/enable", - `{"credential_bindings":{"github_pat":"`+secretID+`"}}`, testUserAID) + `{"configuration":{"credential_bindings":{"github_pat":{"source":"shared","secret_id":"`+secretID+`"}}}}`, testUserAID) if res.Code != http.StatusOK { t.Fatalf("enable expected 200, got %d: %s", res.Code, res.Body.String()) } assertSingleAgentCapability(t, db, agentID, capID, versionID) var storedSecretID string if err := db.QueryRow(context.Background(), ` - select config #>> '{credential_bindings,github_pat,secret_id}' - from agents where id = $1 + select configuration #>> '{credential_bindings,github_pat,secret_id}' + from agent_capabilities where agent_id = $1 `, agentID).Scan(&storedSecretID); err != nil { t.Fatalf("read stored binding: %v", err) } @@ -229,6 +229,41 @@ func TestCapabilityEnablePersistsSelectedSharedSecret(t *testing.T) { } } +func TestCapabilityEnableRejectsOAuthSecretFromDifferentCatalogConnector(t *testing.T) { + r, db := capabilityTestRouter(t, map[string]string{testUserAID: "member"}, nil) + capID, versionID, _ := insertCapabilityVersions(t, db, store.DefaultDevFixtureIDs().WorkspaceID, "Notion MCP") + agentID := insertAgentForOwner(t, db, testUserAID, "notion-agent") + secretID := "00000000-0000-0000-0000-000000000098" + if _, err := db.Exec(context.Background(), ` + update capability_version + set source_payload = '{"source_format":"mcp_catalog","catalog_id":"notion"}'::jsonb, + required_credentials = '[{"kind":"mcp_oauth","required":true}]'::jsonb + where id = $1 + `, versionID); err != nil { + t.Fatalf("mark catalog capability: %v", err) + } + if _, err := db.Exec(context.Background(), ` + insert into secrets(id, slug, name, kind, provider, auth_type, encrypted_payload, key_version, status, metadata, created_by, created_at, updated_at) + values ($1, 'github-oauth-test', 'GitHub OAuth', 'capability_inline', 'github', 'oauth2', '\x01'::bytea, 'v1', 'active', $2::jsonb, $3, now(), now()) + `, secretID, `{"workspace_id":"`+store.DefaultDevFixtureIDs().WorkspaceID+`","credential_kind_code":"mcp_oauth"}`, testUserAID); err != nil { + t.Fatalf("insert shared secret: %v", err) + } + + res := serveCapabilityRoute(t, r, http.MethodPost, + "/api/v1/workspaces/"+store.DefaultDevFixtureIDs().WorkspaceID+"/agents/"+agentID+"/capabilities/"+versionID+"/enable", + `{"configuration":{"credential_bindings":{"mcp_oauth":{"source":"shared","secret_id":"`+secretID+`"}}}}`, testUserAID) + if res.Code != http.StatusUnprocessableEntity || !strings.Contains(res.Body.String(), "different MCP connector") { + t.Fatalf("enable with wrong connector secret expected 422, got %d: %s", res.Code, res.Body.String()) + } + var count int + if err := db.QueryRow(context.Background(), `select count(*) from agent_capabilities where agent_id = $1 and capability_id = $2`, agentID, capID).Scan(&count); err != nil { + t.Fatalf("count agent capabilities: %v", err) + } + if count != 0 { + t.Fatalf("agent capability was created with a mismatched connector secret") + } +} + func TestCapabilityMarketplacePublishLifecycleSecretCheckAndDeleteRollback(t *testing.T) { r, db := capabilityTestRouter(t, map[string]string{store.DefaultDevFixtureIDs().UserID: "admin"}, nil) capID, _, _ := insertCapabilityVersions(t, db, store.DefaultDevFixtureIDs().WorkspaceID, "Marketplace Secret") diff --git a/server/internal/dev/routes_test.go b/server/internal/dev/routes_test.go index 5bbc1b1c..8d204127 100644 --- a/server/internal/dev/routes_test.go +++ b/server/internal/dev/routes_test.go @@ -2693,7 +2693,7 @@ func (stubRuntimeStore) GetEnabledMarketplaceCapabilitiesForAgent(ctx context.Co return []store.EnabledCapabilityRead{}, nil } -func (stubRuntimeStore) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string, credentialBindings map[string]string) (store.AgentCapabilityRead, error) { +func (stubRuntimeStore) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string) (store.AgentCapabilityRead, error) { return store.AgentCapabilityRead{ID: "00000000-0000-0000-0000-000000000c04", AgentID: agentID, CapabilityID: "00000000-0000-0000-0000-000000000c01", CapabilityVersionID: versionID, Enabled: true, PinningMode: pinningMode}, nil } diff --git a/server/internal/mcpcatalog/catalog_test.go b/server/internal/mcpcatalog/catalog_test.go index d9cb8ce7..eb329832 100644 --- a/server/internal/mcpcatalog/catalog_test.go +++ b/server/internal/mcpcatalog/catalog_test.go @@ -25,7 +25,7 @@ func TestBuiltinCatalogLoads(t *testing.T) { } if item.ID == "notion" { header := item.CanonicalSpec().MCP.Servers[0].Headers["Authorization"] - if header.Prefix != "Bearer " || header.CredentialKindCode != "notion_mcp_oauth" { + if header.Prefix != "Bearer " || header.CredentialKindCode != OAuthCredentialKind { t.Fatalf("notion authorization header = %+v", header) } } @@ -44,9 +44,9 @@ func TestCatalogValidationRejectsInvalidContent(t *testing.T) { {"insecure URL", func(c *Catalog) { c.Items[0].Server.URL = "http://example.com/mcp" }, "https URL"}, {"embedded credentials", func(c *Catalog) { c.Items[0].Server.URL = "https://token@example.com/mcp" }, "embedded credentials"}, {"featured rank", func(c *Catalog) { c.Items[0].FeaturedRank = 0 }, "featured_rank"}, - {"oauth credential kind", func(c *Catalog) { - c.Items[0].Authentication = Authentication{Type: "oauth2", CredentialKind: "Not Valid"} - }, "credential_kind"}, + {"authentication type", func(c *Catalog) { + c.Items[0].Authentication = Authentication{Type: "api_key"} + }, "unsupported"}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { diff --git a/server/internal/mcpcatalog/types.go b/server/internal/mcpcatalog/types.go index e173c616..44f3379a 100644 --- a/server/internal/mcpcatalog/types.go +++ b/server/internal/mcpcatalog/types.go @@ -4,7 +4,10 @@ import ( "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" ) -const SchemaVersion = 1 +const ( + SchemaVersion = 1 + OAuthCredentialKind = "mcp_oauth" +) type Catalog struct { SchemaVersion int `json:"schema_version"` @@ -30,8 +33,7 @@ type Item struct { } type Authentication struct { - Type string `json:"type,omitempty"` - CredentialKind string `json:"credential_kind,omitempty"` + Type string `json:"type,omitempty"` } func (a Authentication) EffectiveType() string { @@ -62,7 +64,7 @@ func (i Item) CanonicalSpec() canonical.Spec { "Authorization": { Mode: canonical.EnvModeCredentialRef, Prefix: "Bearer ", - CredentialKindCode: i.Authentication.CredentialKind, + CredentialKindCode: OAuthCredentialKind, }, } } diff --git a/server/internal/mcpcatalog/validate.go b/server/internal/mcpcatalog/validate.go index 9b26263b..311d905a 100644 --- a/server/internal/mcpcatalog/validate.go +++ b/server/internal/mcpcatalog/validate.go @@ -14,10 +14,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" ) -var ( - idPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]*$`) - credentialPattern = regexp.MustCompile(`^[a-z][a-z0-9_]*$`) -) +var idPattern = regexp.MustCompile(`^[a-z0-9][a-z0-9._-]*$`) func Decode(data []byte) (Catalog, error) { decoder := json.NewDecoder(bytes.NewReader(data)) @@ -92,14 +89,7 @@ func (i Item) Validate() error { return fmt.Errorf("item %q transport %q is unsupported", i.ID, i.Transport) } switch i.Authentication.EffectiveType() { - case "none": - if strings.TrimSpace(i.Authentication.CredentialKind) != "" { - return fmt.Errorf("item %q authentication credential_kind requires oauth2", i.ID) - } - case "oauth2": - if !credentialPattern.MatchString(i.Authentication.CredentialKind) { - return fmt.Errorf("item %q authentication credential_kind %q is invalid", i.ID, i.Authentication.CredentialKind) - } + case "none", "oauth2": default: return fmt.Errorf("item %q authentication type %q is unsupported", i.ID, i.Authentication.Type) } diff --git a/server/internal/store/capabilities.go b/server/internal/store/capabilities.go index 72c0e362..69013440 100644 --- a/server/internal/store/capabilities.go +++ b/server/internal/store/capabilities.go @@ -8,9 +8,9 @@ import ( "strings" "time" + "github.com/MiniMax-AI-Dev/parsar/server/internal/db/sqlc" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" - "github.com/MiniMax-AI-Dev/parsar/server/internal/db/sqlc" ) type RequiredCredential struct { @@ -20,10 +20,10 @@ type RequiredCredential struct { } type EnabledCapabilityRead struct { - AgentCapabilityID string `json:"agent_capability_id"` - AgentID string `json:"agent_id"` - Enabled bool `json:"enabled"` - Configuration map[string]any `json:"configuration"` + AgentCapabilityID string `json:"agent_capability_id"` + AgentID string `json:"agent_id"` + Enabled bool `json:"enabled"` + Configuration map[string]any `json:"configuration"` // PinningMode is 'latest' or 'pinned'. In 'latest' mode the daemon // resolver ignores OssKey/SHA256/CanonicalSpec/Version on this struct // and uses LatestOssKey/LatestSHA256/LatestCanonicalSpec/LatestVersion @@ -853,107 +853,39 @@ func normalizePinningMode(mode string) string { } } -func (s *Store) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string, credentialBindings map[string]string) (AgentCapabilityRead, error) { - agentUUID, err := uuid(agentID) - if err != nil { - return AgentCapabilityRead{}, err - } - versionUUID, err := uuid(versionID) +func (s *Store) EnableAgentCapability(ctx context.Context, agentID string, versionID string, configuration map[string]any, pinningMode string) (AgentCapabilityRead, error) { + version, err := s.GetCapabilityVersion(ctx, versionID) if err != nil { return AgentCapabilityRead{}, err } - tx, err := beginTx(ctx, s.db) + config, err := json.Marshal(nonNilMap(configuration)) if err != nil { return AgentCapabilityRead{}, err } - defer tx.Rollback(ctx) - queries := sqlc.New(tx) - versionRow, err := queries.GetCapabilityVersion(ctx, versionUUID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return AgentCapabilityRead{}, fmt.Errorf("%w: %s", ErrUnknownCapabilityVersion, versionID) - } - return AgentCapabilityRead{}, err - } - agentRow, err := queries.GetAgentForUpdate(ctx, agentUUID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - return AgentCapabilityRead{}, fmt.Errorf("%w: %s", ErrUnknownAgent, agentID) - } - return AgentCapabilityRead{}, err - } - if len(credentialBindings) > 0 { - agentConfig := decodeJSONMap(agentRow.Config) - bindings, _ := agentConfig["credential_bindings"].(map[string]any) - bindings = cloneAnyMap(bindings) - for kind, secretID := range credentialBindings { - bindings[kind] = map[string]any{"source": "shared", "secret_id": secretID} - } - agentConfig["credential_bindings"] = bindings - encodedAgentConfig, err := json.Marshal(agentConfig) - if err != nil { - return AgentCapabilityRead{}, err - } - if _, err := queries.UpdateAgentCRUD(ctx, sqlc.UpdateAgentCRUDParams{ - ID: agentUUID, - Name: agentRow.Name, - Description: agentRow.Description, - ConnectorType: agentRow.ConnectorType, - Config: encodedAgentConfig, - Now: timestamptz(time.Now().UTC()), - }); err != nil { - return AgentCapabilityRead{}, err - } + mode := normalizePinningMode(pinningMode) + now := time.Now().UTC() + params := sqlc.CreateAgentCapabilityParams{ID: mustUUID(newID()), AgentID: mustUUID(agentID), CapabilityID: mustUUID(version.CapabilityID), CapabilityVersionID: mustUUID(version.ID), Enabled: true, Configuration: config, PinningMode: mode, Now: timestamptz(now)} + row, err := sqlc.New(s.db).CreateAgentCapability(ctx, params) + if err == nil { + return agentCapabilityFromCreateRow(row), nil } - capabilityConfig, err := json.Marshal(nonNilMap(configuration)) - if err != nil { + if !isUniqueViolation(err) { return AgentCapabilityRead{}, err } - mode := normalizePinningMode(pinningMode) - now := time.Now().UTC() - existingRows, err := queries.ListAgentCapabilitiesByAgent(ctx, agentUUID) + existingRows, err := sqlc.New(s.db).ListAgentCapabilitiesByAgent(ctx, mustUUID(agentID)) if err != nil { return AgentCapabilityRead{}, err } - var enabled AgentCapabilityRead for _, existing := range existingRows { - if existing.CapabilityID != versionRow.CapabilityID { - continue - } - updated, err := queries.UpdateAgentCapability(ctx, sqlc.UpdateAgentCapabilityParams{ - ID: mustUUID(existing.ID), - CapabilityVersionID: versionUUID, - Enabled: true, - Configuration: capabilityConfig, - PinningMode: mode, - Now: timestamptz(now), - }) - if err != nil { - return AgentCapabilityRead{}, err - } - enabled = agentCapabilityFromUpdateRow(updated) - break - } - if enabled.ID == "" { - created, err := queries.CreateAgentCapability(ctx, sqlc.CreateAgentCapabilityParams{ - ID: mustUUID(newID()), - AgentID: agentUUID, - CapabilityID: mustUUID(versionRow.CapabilityID), - CapabilityVersionID: versionUUID, - Enabled: true, - Configuration: capabilityConfig, - PinningMode: mode, - Now: timestamptz(now), - }) - if err != nil { - return AgentCapabilityRead{}, err + if existing.CapabilityID == version.CapabilityID { + updated, err := sqlc.New(s.db).UpdateAgentCapability(ctx, sqlc.UpdateAgentCapabilityParams{ID: mustUUID(existing.ID), CapabilityVersionID: mustUUID(version.ID), Enabled: true, Configuration: config, PinningMode: mode, Now: timestamptz(now)}) + if err != nil { + return AgentCapabilityRead{}, err + } + return agentCapabilityFromUpdateRow(updated), nil } - enabled = agentCapabilityFromCreateRow(created) } - if err := tx.Commit(ctx); err != nil { - return AgentCapabilityRead{}, err - } - return enabled, nil + return AgentCapabilityRead{}, err } func (s *Store) UpgradeAgentCapability(ctx context.Context, agentID string, capabilityID string, newVersionID string, pinningMode string) (AgentCapabilityRead, error) { diff --git a/server/internal/store/capabilities_binding_test.go b/server/internal/store/capabilities_binding_test.go deleted file mode 100644 index ee0a1736..00000000 --- a/server/internal/store/capabilities_binding_test.go +++ /dev/null @@ -1,79 +0,0 @@ -package store - -import ( - "context" - "testing" -) - -func TestEnableAgentCapabilityPersistsCredentialBindingAtomically(t *testing.T) { - db := openTestDB(t) - ctx := context.Background() - st := New(db) - ids := mustSeedDevFixture(t, ctx, st) - - capability, err := st.CreateCapability(ctx, CreateCapabilityInput{ - WorkspaceID: ids.WorkspaceID, - CreatorID: ids.UserID, - Type: "mcp", - Name: "atomic-credential-binding", - InitialVersion: &CreateCapabilityVersionInput{ - Version: "1.0.0", - CreatorID: ids.UserID, - Content: map[string]any{"mcpServers": map[string]any{"test": map[string]any{"command": "true"}}}, - }, - }) - if err != nil { - t.Fatalf("CreateCapability: %v", err) - } - versions, err := st.ListCapabilityVersions(ctx, capability.ID) - if err != nil || len(versions) != 1 { - t.Fatalf("ListCapabilityVersions: versions=%+v err=%v", versions, err) - } - agent, err := st.CreateAgent(ctx, CreateAgentInput{ - WorkspaceID: ids.WorkspaceID, - Name: "Atomic Binding Agent", - ConnectorType: "agent_daemon", - AgentConfig: map[string]any{"daemon_mode": "sandbox", "agent_kind": "opencode"}, - CreatedBy: ids.UserID, - }) - if err != nil { - t.Fatalf("CreateAgent: %v", err) - } - secretID := "00000000-0000-0000-0000-000000000099" - bindings := map[string]string{"notion_mcp_oauth": secretID} - - if _, err := st.EnableAgentCapability(ctx, agent.Agent.ID, versions[0].ID, nil, "invalid", bindings); err == nil { - t.Fatal("EnableAgentCapability with invalid pinning mode unexpectedly succeeded") - } - afterFailure, err := st.GetAgent(ctx, agent.Agent.ID) - if err != nil { - t.Fatalf("GetAgent after rollback: %v", err) - } - if _, exists := afterFailure.Config["credential_bindings"]; exists { - t.Fatalf("credential binding survived rolled-back enable: %+v", afterFailure.Config) - } - installed, err := st.ListAgentCapabilities(ctx, agent.Agent.ID) - if err != nil || len(installed) != 0 { - t.Fatalf("agent capability survived rolled-back enable: installed=%+v err=%v", installed, err) - } - - if _, err := st.EnableAgentCapability(ctx, agent.Agent.ID, versions[0].ID, nil, PinningModePinned, bindings); err != nil { - t.Fatalf("EnableAgentCapability: %v", err) - } - afterSuccess, err := st.GetAgent(ctx, agent.Agent.ID) - if err != nil { - t.Fatalf("GetAgent after success: %v", err) - } - storedBindings, ok := afterSuccess.Config["credential_bindings"].(map[string]any) - if !ok { - t.Fatalf("credential_bindings=%#v", afterSuccess.Config["credential_bindings"]) - } - stored, ok := storedBindings["notion_mcp_oauth"].(map[string]any) - if !ok || stored["source"] != "shared" || stored["secret_id"] != secretID { - t.Fatalf("stored binding=%#v", storedBindings["notion_mcp_oauth"]) - } - installed, err = st.ListAgentCapabilities(ctx, agent.Agent.ID) - if err != nil || len(installed) != 1 || installed[0].CapabilityID != capability.ID { - t.Fatalf("installed=%+v err=%v", installed, err) - } -} diff --git a/server/internal/store/capabilities_pinning_mode_test.go b/server/internal/store/capabilities_pinning_mode_test.go index ff12ca15..28bf4c52 100644 --- a/server/internal/store/capabilities_pinning_mode_test.go +++ b/server/internal/store/capabilities_pinning_mode_test.go @@ -165,7 +165,7 @@ func TestGetEnabledCapabilitiesForAgent_PinningModeLatestFields(t *testing.T) { // cv.* still reflect v1 (we didn't rewrite capability_version_id); // latest_* still reflect v2; PinningMode is now "latest". The // daemon resolver's resolveVersionFields then picks v2 fields. - if _, err := st.EnableAgentCapability(ctx, created.Agent.ID, v1ID, nil, PinningModeLatest, nil); err != nil { + if _, err := st.EnableAgentCapability(ctx, created.Agent.ID, v1ID, nil, PinningModeLatest); err != nil { t.Fatalf("EnableAgentCapability flip to latest: %v", err) } enabled, err = st.GetEnabledCapabilitiesForAgent(ctx, created.Agent.ID) diff --git a/server/internal/store/credential_kinds.go b/server/internal/store/credential_kinds.go index 2653ae92..c7e2a6e9 100644 --- a/server/internal/store/credential_kinds.go +++ b/server/internal/store/credential_kinds.go @@ -22,7 +22,7 @@ var builtInCredentialKindSeeds = []builtInCredentialKindSeed{ {Code: "teams_app_password", DisplayName: "Teams App Password", Description: "Microsoft Teams Bot AAD client secret", Source: CredentialKindSourceUserDefined}, {Code: "postgres_dsn", DisplayName: "Postgres \u8fde\u63a5\u4e32", Description: "Postgres DSN", Source: CredentialKindSourceUserDefined}, {Code: "notion_integration", DisplayName: "Notion \u96c6\u6210 token", Description: "Notion Integration Token", Source: CredentialKindSourceUserDefined}, - {Code: "notion_mcp_oauth", DisplayName: "Notion MCP OAuth", Description: "Notion MCP OAuth access token", Source: CredentialKindSourcePlatformOAuth}, + {Code: "mcp_oauth", DisplayName: "MCP OAuth", Description: "OAuth credential for hosted MCP connectors", Source: CredentialKindSourcePlatformOAuth}, {Code: "jira_api_token", DisplayName: "Jira API Token", Description: "Atlassian Jira API Token", Source: CredentialKindSourceUserDefined}, {Code: "openai_api_key", DisplayName: "OpenAI API Key", Description: "Personal OpenAI API key (sk-...)", Source: CredentialKindSourcePlatformModel}, {Code: "anthropic_api_key", DisplayName: "Anthropic API Key", Description: "Personal Anthropic API key (sk-ant-...)", Source: CredentialKindSourcePlatformModel}, @@ -36,7 +36,7 @@ var SupportedCredentialKinds = []string{ "teams_app_password", "postgres_dsn", "notion_integration", - "notion_mcp_oauth", + "mcp_oauth", "jira_api_token", "openai_api_key", "anthropic_api_key", diff --git a/server/internal/store/oauth_secret_test.go b/server/internal/store/oauth_secret_test.go index 780e271e..0f4f4709 100644 --- a/server/internal/store/oauth_secret_test.go +++ b/server/internal/store/oauth_secret_test.go @@ -21,7 +21,7 @@ func TestCapabilitySecretIsScopedToWorkspaceAndCanRotate(t *testing.T) { AuthType: "oauth2", Masked: "configured", CreatedBy: ids.UserID, - CredentialKindCode: "notion_mcp_oauth", + CredentialKindCode: "mcp_oauth", }, []byte(`{"token":"first"}`)) if err != nil { t.Fatalf("CreateSecret: %v", err) diff --git a/server/migrations/000010_notion_mcp_oauth_credential_kind.sql b/server/migrations/000010_mcp_oauth_credential_kind.sql similarity index 79% rename from server/migrations/000010_notion_mcp_oauth_credential_kind.sql rename to server/migrations/000010_mcp_oauth_credential_kind.sql index 3c09cbd3..102808ac 100644 --- a/server/migrations/000010_notion_mcp_oauth_credential_kind.sql +++ b/server/migrations/000010_mcp_oauth_credential_kind.sql @@ -7,9 +7,9 @@ INSERT INTO credential_kinds ( code, display_name, description, source, built_in ) VALUES ( - 'notion_mcp_oauth', - 'Notion MCP OAuth', - 'Notion MCP OAuth access token', + 'mcp_oauth', + 'MCP OAuth', + 'OAuth credential for hosted MCP connectors', 'platform_oauth', TRUE ) @@ -20,5 +20,5 @@ ON CONFLICT DO NOTHING; -- Rollback only; normal startup does not execute this section. DELETE FROM credential_kinds -WHERE code = 'notion_mcp_oauth' +WHERE code = 'mcp_oauth' AND built_in = TRUE; From 4a7ffc67fe657fe267575c3024cc6f7408d9cfb0 Mon Sep 17 00:00:00 2001 From: kapelame Date: Fri, 24 Jul 2026 14:52:18 +0800 Subject: [PATCH 4/8] refactor: centralize capability credential validation --- server/internal/api/mcpdirectory/handler.go | 3 +- .../internal/api/mcpdirectory/handler_test.go | 5 +- .../internal/api/mcpdirectory/oauth_scope.go | 5 +- server/internal/capability/credential_kind.go | 2 + .../capability/credentialbinding/binding.go | 99 ++++++++++++++ .../credentialbinding/binding_test.go | 47 +++++++ .../agentdaemon/capability_runtime_test.go | 14 +- .../agentdaemon/credential_binding.go | 59 ++------- .../internal/dev/agent_credential_binding.go | 29 ++--- .../dev/capability_credential_binding.go | 113 ++++++++++++++++ .../dev/capability_credential_binding_test.go | 51 ++++++++ server/internal/dev/capability_routes.go | 122 +----------------- server/internal/dev/routes_agents.go | 54 +++++++- server/internal/dev/routes_test.go | 88 +++++++++++++ server/internal/mcpcatalog/catalog_test.go | 4 +- server/internal/mcpcatalog/types.go | 8 +- 16 files changed, 496 insertions(+), 207 deletions(-) create mode 100644 server/internal/capability/credentialbinding/binding.go create mode 100644 server/internal/capability/credentialbinding/binding_test.go create mode 100644 server/internal/dev/capability_credential_binding.go create mode 100644 server/internal/dev/capability_credential_binding_test.go diff --git a/server/internal/api/mcpdirectory/handler.go b/server/internal/api/mcpdirectory/handler.go index 4097f440..325fc119 100644 --- a/server/internal/api/mcpdirectory/handler.go +++ b/server/internal/api/mcpdirectory/handler.go @@ -15,6 +15,7 @@ import ( "github.com/MiniMax-AI-Dev/parsar/server/internal/auth" "github.com/MiniMax-AI-Dev/parsar/server/internal/auth/mcpoauth" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" @@ -347,7 +348,7 @@ func (h *handler) connectedCatalogIDs(w http.ResponseWriter, r *http.Request, wo catalogID := strings.TrimSpace(candidate.Provider) item, found := catalog.Find(catalogID) if !found || item.Authentication.EffectiveType() != "oauth2" || - metadataString(candidate.Metadata, "credential_kind_code") != mcpcatalog.OAuthCredentialKind { + metadataString(candidate.Metadata, "credential_kind_code") != capability.CredentialKindMCPOAuth { continue } result[catalogID] = true diff --git a/server/internal/api/mcpdirectory/handler_test.go b/server/internal/api/mcpdirectory/handler_test.go index d2cdcfca..929918cb 100644 --- a/server/internal/api/mcpdirectory/handler_test.go +++ b/server/internal/api/mcpdirectory/handler_test.go @@ -10,6 +10,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/MiniMax-AI-Dev/parsar/server/internal/auth" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" ) @@ -167,14 +168,14 @@ func TestOAuthDirectoryItemRequiresWorkspaceConnectionBeforeImport(t *testing.T) credentials.secrets = []store.SecretRead{{ ID: "secret-2", Kind: "capability_inline", Provider: "notion", AuthType: "oauth2", Status: "active", - Metadata: map[string]any{"workspace_id": testWorkspaceID, "credential_kind_code": mcpcatalog.OAuthCredentialKind}, + Metadata: map[string]any{"workspace_id": testWorkspaceID, "credential_kind_code": capability.CredentialKindMCPOAuth}, }} rec = requestWithDeps(t, fs, credentials, catalog, http.MethodPost, "/api/v1/workspaces/"+testWorkspaceID+"/mcp-directory/notion/import") if rec.Code != http.StatusCreated { t.Fatalf("status=%d body=%s", rec.Code, rec.Body.String()) } header := fs.imported.Spec.MCP.Servers[0].Headers["Authorization"] - if header.Prefix != "Bearer " || header.CredentialKindCode != mcpcatalog.OAuthCredentialKind { + if header.Prefix != "Bearer " || header.CredentialKindCode != capability.CredentialKindMCPOAuth { t.Fatalf("authorization header = %+v", header) } } diff --git a/server/internal/api/mcpdirectory/oauth_scope.go b/server/internal/api/mcpdirectory/oauth_scope.go index b3b93e18..d851f3b7 100644 --- a/server/internal/api/mcpdirectory/oauth_scope.go +++ b/server/internal/api/mcpdirectory/oauth_scope.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/MiniMax-AI-Dev/parsar/server/internal/auth/mcpoauth" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" @@ -38,7 +39,7 @@ func (h *handler) saveWorkspaceOAuthCredential( AuthType: "oauth2", Masked: secrets.MaskPayload(payload), CreatedBy: createdBy, - CredentialKindCode: mcpcatalog.OAuthCredentialKind, + CredentialKindCode: capability.CredentialKindMCPOAuth, }, encrypted) return err } @@ -57,7 +58,7 @@ func (h *handler) workspaceOAuthCredentialRead( candidate.AuthType != "oauth2" || metadataString(candidate.Metadata, "workspace_id") != strings.TrimSpace(workspaceID) || strings.TrimSpace(candidate.Provider) != item.ID || - metadataString(candidate.Metadata, "credential_kind_code") != mcpcatalog.OAuthCredentialKind { + metadataString(candidate.Metadata, "credential_kind_code") != capability.CredentialKindMCPOAuth { continue } return candidate, true, nil diff --git a/server/internal/capability/credential_kind.go b/server/internal/capability/credential_kind.go index 9a212eb1..d9ea017b 100644 --- a/server/internal/capability/credential_kind.go +++ b/server/internal/capability/credential_kind.go @@ -1,5 +1,7 @@ package capability +const CredentialKindMCPOAuth = "mcp_oauth" + type CredentialKindMeta struct { Name string ZhCN string diff --git a/server/internal/capability/credentialbinding/binding.go b/server/internal/capability/credentialbinding/binding.go new file mode 100644 index 00000000..1fe29a31 --- /dev/null +++ b/server/internal/capability/credentialbinding/binding.go @@ -0,0 +1,99 @@ +package credentialbinding + +import ( + "errors" + "fmt" + "strings" +) + +type Source string + +const ( + SourcePersonal Source = "personal" + SourceShared Source = "shared" +) + +type Binding struct { + Source Source + SecretID string +} + +func (b Binding) IsShared() bool { + return b.Source == SourceShared && strings.TrimSpace(b.SecretID) != "" +} + +// ParseStrict validates the complete credential_bindings payload for API +// writes. Malformed entries are rejected instead of silently falling back. +func ParseStrict(config map[string]any) (map[string]Binding, error) { + return parse(config, true) +} + +// ParseLenient reads persisted bindings for runtime use. Invalid legacy rows +// are ignored so one malformed entry does not prevent an agent from starting. +func ParseLenient(config map[string]any) map[string]Binding { + bindings, _ := parse(config, false) + return bindings +} + +func MergeLenient(target map[string]Binding, config map[string]any) { + for kind, binding := range ParseLenient(config) { + target[kind] = binding + } +} + +func parse(config map[string]any, strict bool) (map[string]Binding, error) { + result := map[string]Binding{} + if len(config) == 0 { + return result, nil + } + raw, exists := config["credential_bindings"] + if !exists || raw == nil { + return result, nil + } + bindings, ok := raw.(map[string]any) + if !ok { + if strict { + return nil, errors.New("credential_bindings must be an object") + } + return result, nil + } + for rawKind, rawBinding := range bindings { + kind := strings.TrimSpace(rawKind) + binding, ok := rawBinding.(map[string]any) + if kind == "" || !ok { + if strict { + return nil, errors.New("credential_bindings entries must be non-empty objects") + } + continue + } + source := Source(strings.TrimSpace(stringValue(binding["source"]))) + secretID := strings.TrimSpace(stringValue(binding["secret_id"])) + switch source { + case SourcePersonal: + result[kind] = Binding{Source: SourcePersonal} + case SourceShared: + if secretID == "" { + if strict { + return nil, fmt.Errorf("credential_bindings[%s].secret_id is required for shared source", kind) + } + continue + } + result[kind] = Binding{Source: SourceShared, SecretID: secretID} + case "": + if strict { + return nil, fmt.Errorf("credential_bindings[%s].source must be personal or shared", kind) + } + result[kind] = Binding{Source: SourcePersonal} + default: + if strict { + return nil, fmt.Errorf("credential_bindings[%s].source must be personal or shared", kind) + } + } + } + return result, nil +} + +func stringValue(value any) string { + valueString, _ := value.(string) + return valueString +} diff --git a/server/internal/capability/credentialbinding/binding_test.go b/server/internal/capability/credentialbinding/binding_test.go new file mode 100644 index 00000000..53300e6e --- /dev/null +++ b/server/internal/capability/credentialbinding/binding_test.go @@ -0,0 +1,47 @@ +package credentialbinding + +import ( + "strings" + "testing" +) + +func TestParseStrict(t *testing.T) { + bindings, err := ParseStrict(map[string]any{ + "credential_bindings": map[string]any{ + "github_pat": map[string]any{"source": "personal"}, + "mcp_oauth": map[string]any{"source": "shared", "secret_id": "secret-1"}, + }, + }) + if err != nil { + t.Fatalf("ParseStrict returned error: %v", err) + } + if bindings["github_pat"].Source != SourcePersonal { + t.Fatalf("personal binding = %#v", bindings["github_pat"]) + } + if !bindings["mcp_oauth"].IsShared() || bindings["mcp_oauth"].SecretID != "secret-1" { + t.Fatalf("shared binding = %#v", bindings["mcp_oauth"]) + } + + _, err = ParseStrict(map[string]any{ + "credential_bindings": map[string]any{ + "mcp_oauth": map[string]any{"source": "shared"}, + }, + }) + if err == nil || !strings.Contains(err.Error(), "secret_id") { + t.Fatalf("missing secret_id error = %v", err) + } +} + +func TestParseLenientDropsMalformedBindings(t *testing.T) { + bindings := ParseLenient(map[string]any{ + "credential_bindings": map[string]any{ + "valid": map[string]any{"source": "shared", "secret_id": "secret-1"}, + "missing": map[string]any{"source": "shared"}, + "unknown": map[string]any{"source": "other"}, + "legacy": map[string]any{}, + }, + }) + if len(bindings) != 2 || !bindings["valid"].IsShared() || bindings["legacy"].Source != SourcePersonal { + t.Fatalf("bindings = %#v", bindings) + } +} diff --git a/server/internal/connector/agentdaemon/capability_runtime_test.go b/server/internal/connector/agentdaemon/capability_runtime_test.go index 28125dac..7c5ed430 100644 --- a/server/internal/connector/agentdaemon/capability_runtime_test.go +++ b/server/internal/connector/agentdaemon/capability_runtime_test.go @@ -10,9 +10,9 @@ import ( "time" "github.com/MiniMax-AI-Dev/parsar/server/internal/auth/mcpoauth" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" "github.com/MiniMax-AI-Dev/parsar/server/internal/connector" - "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" ) @@ -469,13 +469,13 @@ func TestResolveCapabilityAdditions_UsesWorkspaceOAuthHeader(t *testing.T) { "Authorization": { Mode: canonical.EnvModeCredentialRef, Prefix: "Bearer ", - CredentialKindCode: mcpcatalog.OAuthCredentialKind, + CredentialKindCode: capability.CredentialKindMCPOAuth, }, }, - }}, []store.RequiredCredential{{Kind: mcpcatalog.OAuthCredentialKind, Required: true}}) + }}, []store.RequiredCredential{{Kind: capability.CredentialKindMCPOAuth, Required: true}}) row.Configuration = map[string]any{ "credential_bindings": map[string]any{ - mcpcatalog.OAuthCredentialKind: map[string]any{ + capability.CredentialKindMCPOAuth: map[string]any{ "source": "shared", "secret_id": "secret-1", }, @@ -522,13 +522,13 @@ func TestResolveCapabilityAdditions_UsesCapabilityScopedOAuthBindings(t *testing "Authorization": { Mode: canonical.EnvModeCredentialRef, Prefix: "Bearer ", - CredentialKindCode: mcpcatalog.OAuthCredentialKind, + CredentialKindCode: capability.CredentialKindMCPOAuth, }, }, - }}, []store.RequiredCredential{{Kind: mcpcatalog.OAuthCredentialKind, Required: true}}) + }}, []store.RequiredCredential{{Kind: capability.CredentialKindMCPOAuth, Required: true}}) row.Configuration = map[string]any{ "credential_bindings": map[string]any{ - mcpcatalog.OAuthCredentialKind: map[string]any{ + capability.CredentialKindMCPOAuth: map[string]any{ "source": "shared", "secret_id": secretID, }, diff --git a/server/internal/connector/agentdaemon/credential_binding.go b/server/internal/connector/agentdaemon/credential_binding.go index 151904d5..675907c7 100644 --- a/server/internal/connector/agentdaemon/credential_binding.go +++ b/server/internal/connector/agentdaemon/credential_binding.go @@ -2,6 +2,8 @@ package agentdaemon import ( "strings" + + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/credentialbinding" ) // CredentialBindingSource discriminates how an agent-level credential @@ -13,26 +15,17 @@ import ( // SecretID; the same plaintext is served to every caller. // // Bindings are read from agent_config.credential_bindings[]. -type CredentialBindingSource string +type CredentialBindingSource = credentialbinding.Source const ( - CredentialBindingPersonal CredentialBindingSource = "personal" - CredentialBindingShared CredentialBindingSource = "shared" + CredentialBindingPersonal = credentialbinding.SourcePersonal + CredentialBindingShared = credentialbinding.SourceShared ) // CredentialBinding is the parsed agent-level binding for one credential // kind. Source=="" is treated as personal (back-compat with agents created // before credential_bindings existed). -type CredentialBinding struct { - Source CredentialBindingSource - SecretID string -} - -// IsShared returns true when this binding should bypass user_credentials -// lookup and serve a workspace secret instead. -func (b CredentialBinding) IsShared() bool { - return b.Source == CredentialBindingShared && strings.TrimSpace(b.SecretID) != "" -} +type CredentialBinding = credentialbinding.Binding // ParseCredentialBindings extracts the credential_bindings map from the // agent_config. @@ -41,47 +34,11 @@ func (b CredentialBinding) IsShared() bool { // dropped silently to avoid hard-failing a run on a malformed config; // callers fall back to personal in that case. func ParseCredentialBindings(agentConfig map[string]any) map[string]CredentialBinding { - out := map[string]CredentialBinding{} - mergeBindings(out, agentConfig) - return out + return credentialbinding.ParseLenient(agentConfig) } func mergeBindings(out map[string]CredentialBinding, cfg map[string]any) { - if len(cfg) == 0 { - return - } - raw, ok := cfg["credential_bindings"] - if !ok { - return - } - m, ok := raw.(map[string]any) - if !ok { - return - } - for kind, entry := range m { - kind = strings.TrimSpace(kind) - if kind == "" { - continue - } - obj, ok := entry.(map[string]any) - if !ok { - continue - } - source, _ := obj["source"].(string) - secretID, _ := obj["secret_id"].(string) - switch CredentialBindingSource(strings.TrimSpace(source)) { - case CredentialBindingShared: - if strings.TrimSpace(secretID) == "" { - continue - } - out[kind] = CredentialBinding{ - Source: CredentialBindingShared, - SecretID: strings.TrimSpace(secretID), - } - case CredentialBindingPersonal, "": - out[kind] = CredentialBinding{Source: CredentialBindingPersonal} - } - } + credentialbinding.MergeLenient(out, cfg) } // ParseModelCredentialBinding extracts the optional model_credential_binding diff --git a/server/internal/dev/agent_credential_binding.go b/server/internal/dev/agent_credential_binding.go index 70e143a6..4f5341fb 100644 --- a/server/internal/dev/agent_credential_binding.go +++ b/server/internal/dev/agent_credential_binding.go @@ -7,6 +7,7 @@ import ( "os" "strings" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/credentialbinding" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" ) @@ -116,27 +117,13 @@ func validateAgentVisibilityBindings(visibility string, cfg map[string]any) erro if strings.TrimSpace(visibility) != agentVisibilityPublic { return nil } - // Per-capability credential bindings. - if raw, ok := cfg["credential_bindings"]; ok { - if raw != nil { - bindings, ok := raw.(map[string]any) - if !ok { - return errors.New("credential_bindings must be an object") - } - for kind, entry := range bindings { - obj, ok := entry.(map[string]any) - if !ok { - return fmt.Errorf("credential_bindings[%s] must be an object", kind) - } - source, _ := obj["source"].(string) - if strings.TrimSpace(source) != "shared" { - return fmt.Errorf("public agents cannot use personal credentials (credential_bindings[%s].source=%q)", kind, source) - } - secretID, _ := obj["secret_id"].(string) - if strings.TrimSpace(secretID) == "" { - return fmt.Errorf("credential_bindings[%s].secret_id is required for shared source", kind) - } - } + bindings, err := credentialbinding.ParseStrict(cfg) + if err != nil { + return err + } + for kind, binding := range bindings { + if !binding.IsShared() { + return fmt.Errorf("public agents cannot use personal credentials (credential_bindings[%s].source=%q)", kind, binding.Source) } } // Optional model-level binding. diff --git a/server/internal/dev/capability_credential_binding.go b/server/internal/dev/capability_credential_binding.go new file mode 100644 index 00000000..7fdec415 --- /dev/null +++ b/server/internal/dev/capability_credential_binding.go @@ -0,0 +1,113 @@ +package dev + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/credentialbinding" + "github.com/MiniMax-AI-Dev/parsar/server/internal/store" +) + +type credentialBindingSecretStore interface { + GetSecretPayload(ctx context.Context, workspaceID string, secretID string) (store.SecretPayload, error) +} + +type capabilityCredentialBindingValidationInput struct { + WorkspaceID string + AgentVisibility string + AgentConfig map[string]any + Version store.CapabilityVersionRead + Configuration map[string]any +} + +func validateCapabilityCredentialBindings( + ctx context.Context, + secretStore credentialBindingSecretStore, + input capabilityCredentialBindingValidationInput, +) error { + bindings, err := credentialbinding.ParseStrict(input.Configuration) + if err != nil { + return fmt.Errorf("configuration.%w", err) + } + requiredKinds := make(map[string]bool, len(input.Version.RequiredCredentials)) + for _, required := range input.Version.RequiredCredentials { + kind := strings.TrimSpace(required.Kind) + if required.Required && kind != "" { + requiredKinds[kind] = true + } + } + for kind := range bindings { + if !requiredKinds[kind] { + return errors.New("credential binding kind is not required by this capability") + } + } + + agentBindings := credentialbinding.ParseLenient(input.AgentConfig) + for kind := range requiredKinds { + binding, configured := bindings[kind] + if !configured { + binding, configured = agentBindings[kind] + } + if !configured || binding.Source == credentialbinding.SourcePersonal { + if strings.TrimSpace(input.AgentVisibility) == agentVisibilityPublic { + return errors.New("public agents require a shared secret for every capability credential") + } + continue + } + if err := validateSharedCapabilityCredential(ctx, secretStore, input.WorkspaceID, kind, binding.SecretID, input.Version.SourcePayload); err != nil { + return err + } + } + return nil +} + +func validateSharedCapabilityCredential( + ctx context.Context, + secretStore credentialBindingSecretStore, + workspaceID string, + kind string, + secretID string, + sourcePayload json.RawMessage, +) error { + secretID = strings.TrimSpace(secretID) + if !isUUID(secretID) { + return errors.New("credential binding secret_id must be a valid uuid") + } + secret, err := secretStore.GetSecretPayload(ctx, workspaceID, secretID) + if err != nil || secret.Status != "active" || secret.Kind != "capability_inline" { + return errors.New("credential binding secret is unavailable") + } + secretKind := strings.TrimSpace(metadataStringValue(secret.Metadata, "credential_kind_code")) + if secretKind != "" && secretKind != kind { + return errors.New("credential binding secret has the wrong credential kind") + } + catalogID := catalogIDFromSourcePayload(sourcePayload) + if kind == capability.CredentialKindMCPOAuth && catalogID != "" && + (secretKind != kind || secret.AuthType != "oauth2" || strings.TrimSpace(secret.Provider) != catalogID) { + return errors.New("credential binding secret belongs to a different MCP connector") + } + return nil +} + +func metadataStringValue(metadata map[string]any, key string) string { + value, _ := metadata[key].(string) + return value +} + +func catalogIDFromSourcePayload(sourcePayload json.RawMessage) string { + if len(sourcePayload) == 0 { + return "" + } + var source struct { + SourceFormat string `json:"source_format"` + CatalogID string `json:"catalog_id"` + } + if err := json.Unmarshal(sourcePayload, &source); err != nil || source.SourceFormat != "mcp_catalog" { + return "" + } + return strings.TrimSpace(source.CatalogID) +} diff --git a/server/internal/dev/capability_credential_binding_test.go b/server/internal/dev/capability_credential_binding_test.go new file mode 100644 index 00000000..7fc4c229 --- /dev/null +++ b/server/internal/dev/capability_credential_binding_test.go @@ -0,0 +1,51 @@ +package dev + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" + "github.com/MiniMax-AI-Dev/parsar/server/internal/store" +) + +type credentialBindingSecretStub struct { + provider string +} + +func (s credentialBindingSecretStub) GetSecretPayload(context.Context, string, string) (store.SecretPayload, error) { + return store.SecretPayload{SecretRead: store.SecretRead{ + Kind: "capability_inline", + Provider: s.provider, + AuthType: "oauth2", + Status: "active", + Metadata: map[string]any{"credential_kind_code": capability.CredentialKindMCPOAuth}, + }}, nil +} + +func TestValidateCapabilityCredentialBindingsChecksAgentFallbackProvider(t *testing.T) { + input := capabilityCredentialBindingValidationInput{ + WorkspaceID: "00000000-0000-0000-0000-000000000002", + AgentConfig: map[string]any{ + "credential_bindings": map[string]any{ + capability.CredentialKindMCPOAuth: map[string]any{ + "source": "shared", + "secret_id": "00000000-0000-0000-0000-000000000099", + }, + }, + }, + Version: store.CapabilityVersionRead{ + SourcePayload: json.RawMessage(`{"source_format":"mcp_catalog","catalog_id":"notion"}`), + RequiredCredentials: []store.RequiredCredential{{Kind: capability.CredentialKindMCPOAuth, Required: true}}, + }, + } + + err := validateCapabilityCredentialBindings(context.Background(), credentialBindingSecretStub{provider: "github"}, input) + if err == nil || !strings.Contains(err.Error(), "different MCP connector") { + t.Fatalf("provider mismatch error = %v", err) + } + if err := validateCapabilityCredentialBindings(context.Background(), credentialBindingSecretStub{provider: "notion"}, input); err != nil { + t.Fatalf("matching provider returned error: %v", err) + } +} diff --git a/server/internal/dev/capability_routes.go b/server/internal/dev/capability_routes.go index 76cd64f3..9b995943 100644 --- a/server/internal/dev/capability_routes.go +++ b/server/internal/dev/capability_routes.go @@ -15,7 +15,6 @@ import ( "github.com/MiniMax-AI-Dev/parsar/server/internal/auth" "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" - "github.com/MiniMax-AI-Dev/parsar/server/internal/mcpcatalog" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" "github.com/go-chi/chi/v5" @@ -1423,65 +1422,16 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { writeCapabilityError(w, err, "failed to get agent") return } - requiredKinds := make(map[string]bool, len(version.RequiredCredentials)) - for _, required := range version.RequiredCredentials { - if required.Required { - requiredKinds[required.Kind] = true - } - } - bindings, err := capabilityCredentialBindings(body.Configuration) - if err != nil { + if err := validateCapabilityCredentialBindings(r.Context(), runtimeStore, capabilityCredentialBindingValidationInput{ + WorkspaceID: agent.WorkspaceID, + AgentVisibility: agentRecord.Visibility, + AgentConfig: agentRecord.Config, + Version: version, + Configuration: body.Configuration, + }); err != nil { writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": err.Error()}) return } - catalogID := catalogIDFromSourcePayload(version.SourcePayload) - for rawKind, binding := range bindings { - kind := strings.TrimSpace(rawKind) - if !requiredKinds[kind] { - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding kind is not required by this capability"}) - return - } - if binding.Source == "personal" { - continue - } - secretID := binding.SecretID - if !isUUID(secretID) { - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret_id must be a valid uuid"}) - return - } - secret, err := runtimeStore.GetSecretPayload(r.Context(), agent.WorkspaceID, secretID) - if err != nil || secret.Status != "active" || secret.Kind != "capability_inline" { - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret is unavailable"}) - return - } - secretKind := strings.TrimSpace(metadataStringValue(secret.Metadata, "credential_kind_code")) - if secretKind != "" && secretKind != kind { - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret has the wrong credential kind"}) - return - } - if kind == mcpcatalog.OAuthCredentialKind && catalogID != "" && - (secretKind != kind || secret.AuthType != "oauth2" || strings.TrimSpace(secret.Provider) != catalogID) { - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "credential binding secret belongs to a different MCP connector"}) - return - } - } - if agentRecord.Visibility == agentVisibilityPublic { - existing, _ := agentRecord.Config["credential_bindings"].(map[string]any) - for kind := range requiredKinds { - if binding, explicitlyConfigured := bindings[kind]; explicitlyConfigured { - if binding.Source == "shared" { - continue - } - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "public agents require a shared secret for every capability credential"}) - return - } - if sharedCredentialBindingExists(existing[kind]) { - continue - } - writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "public agents require a shared secret for every capability credential"}) - return - } - } enabled, err := runtimeStore.EnableAgentCapability(r.Context(), agentID, versionID, body.Configuration, body.PinningMode) if err != nil { writeCapabilityError(w, err, "failed to enable agent capability") @@ -1491,64 +1441,6 @@ func enableAgentCapability(runtimeStore RuntimeStore) http.HandlerFunc { } } -type capabilityCredentialBinding struct { - Source string - SecretID string -} - -func capabilityCredentialBindings(configuration map[string]any) (map[string]capabilityCredentialBinding, error) { - result := map[string]capabilityCredentialBinding{} - raw, exists := configuration["credential_bindings"] - if !exists || raw == nil { - return result, nil - } - bindings, ok := raw.(map[string]any) - if !ok { - return nil, fmt.Errorf("configuration.credential_bindings must be an object") - } - for rawKind, rawBinding := range bindings { - kind := strings.TrimSpace(rawKind) - binding, ok := rawBinding.(map[string]any) - source := strings.TrimSpace(fmt.Sprint(binding["source"])) - if kind == "" || !ok || (source != "personal" && source != "shared") { - return nil, fmt.Errorf("configuration.credential_bindings entries must use personal or shared source") - } - secretID := strings.TrimSpace(fmt.Sprint(binding["secret_id"])) - if source == "shared" && secretID == "" { - return nil, fmt.Errorf("configuration.credential_bindings[%s].secret_id is required", kind) - } - result[kind] = capabilityCredentialBinding{Source: source, SecretID: secretID} - } - return result, nil -} - -func metadataStringValue(metadata map[string]any, key string) string { - value, _ := metadata[key].(string) - return value -} - -func catalogIDFromSourcePayload(sourcePayload json.RawMessage) string { - if len(sourcePayload) == 0 { - return "" - } - var source struct { - SourceFormat string `json:"source_format"` - CatalogID string `json:"catalog_id"` - } - if err := json.Unmarshal(sourcePayload, &source); err != nil || source.SourceFormat != "mcp_catalog" { - return "" - } - return strings.TrimSpace(source.CatalogID) -} - -func sharedCredentialBindingExists(value any) bool { - binding, ok := value.(map[string]any) - if !ok || strings.TrimSpace(fmt.Sprint(binding["source"])) != "shared" { - return false - } - return isUUID(strings.TrimSpace(fmt.Sprint(binding["secret_id"]))) -} - // deleteAgentCapability uninstalls a capability version from the agent. // // @Summary Uninstall a capability from an agent diff --git a/server/internal/dev/routes_agents.go b/server/internal/dev/routes_agents.go index f5f46608..da5dbb70 100644 --- a/server/internal/dev/routes_agents.go +++ b/server/internal/dev/routes_agents.go @@ -343,8 +343,38 @@ func createAgent(runtimeStore RuntimeStore, agentDaemonSandbox AgentDaemonSandbo return } initialCapabilities := make([]store.InitialAgentCapabilityInput, 0, len(req.InitialCapabilities)) - for _, capability := range req.InitialCapabilities { - initialCapabilities = append(initialCapabilities, store.InitialAgentCapabilityInput{CapabilityVersionID: capability.CapabilityVersionID, Configuration: capability.Configuration, PinningMode: capability.PinningMode}) + for _, requested := range req.InitialCapabilities { + versionID := strings.TrimSpace(requested.CapabilityVersionID) + if !isUUID(versionID) { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": "capability_version_id must be a valid uuid"}) + return + } + version, err := runtimeStore.GetCapabilityVersion(r.Context(), versionID) + if err != nil { + writeCapabilityError(w, err, "failed to get capability version") + return + } + capabilityRecord, err := runtimeStore.GetCapability(r.Context(), version.CapabilityID) + if err != nil { + writeCapabilityError(w, err, "failed to get capability") + return + } + if capabilityRecord.WorkspaceID != workspaceID && + (capabilityRecord.Visibility != "public" || capabilityRecord.DeprecatedAt != nil || capabilityRecord.Status != "active") { + writeJSON(w, http.StatusForbidden, map[string]string{"error": "marketplace capability is unavailable"}) + return + } + if err := validateCapabilityCredentialBindings(r.Context(), runtimeStore, capabilityCredentialBindingValidationInput{ + WorkspaceID: workspaceID, + AgentVisibility: req.Visibility, + AgentConfig: req.Config, + Version: version, + Configuration: requested.Configuration, + }); err != nil { + writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"error": err.Error()}) + return + } + initialCapabilities = append(initialCapabilities, store.InitialAgentCapabilityInput{CapabilityVersionID: versionID, Configuration: requested.Configuration, PinningMode: requested.PinningMode}) } result, err := runtimeStore.CreateAgent(r.Context(), store.CreateAgentInput{WorkspaceID: workspaceID, Name: req.Name, Description: req.Description, ConnectorType: req.ConnectorType, SystemPrompt: req.SystemPrompt, DefaultModelID: req.DefaultModelID, Capabilities: req.Capabilities, CapabilitiesSet: hasCaps, InitialCapabilities: initialCapabilities, Runtime: "", AgentConfig: req.Config, Visibility: req.Visibility, Slug: req.Slug, CreatedBy: actorIDFromRequest(r)}) if err != nil { @@ -543,6 +573,10 @@ func syncAgentCapabilities( for _, ac := range existing { existingByCapID[ac.CapabilityID] = ac } + agent, err := rs.GetAgent(ctx, agentID) + if err != nil { + return fmt.Errorf("syncAgentCapabilities: get agent: %w", err) + } // 2. Resolve desired names. A name can come from this workspace's own // capabilities, OR from the marketplace (a public capability published @@ -624,6 +658,22 @@ func syncAgentCapabilities( if cap.fromMarketplace { mode = store.PinningModePinned } + version, err := rs.GetCapabilityVersion(ctx, latestVersionID) + if err != nil { + log.Bg().Warn("syncAgentCapabilities: get version failed, skipping", + "capability_id", cap.capabilityID, "name", name, "version_id", latestVersionID, "err", err) + continue + } + if err := validateCapabilityCredentialBindings(ctx, rs, capabilityCredentialBindingValidationInput{ + WorkspaceID: workspaceID, + AgentVisibility: agent.Visibility, + AgentConfig: agent.Config, + Version: version, + }); err != nil { + log.Bg().Warn("syncAgentCapabilities: credential validation failed, skipping", + "capability_id", cap.capabilityID, "name", name, "version_id", latestVersionID, "err", err) + continue + } if _, err := rs.EnableAgentCapability(ctx, agentID, latestVersionID, nil, mode); err != nil { log.Bg().Warn("syncAgentCapabilities: enable failed, skipping", "capability_id", cap.capabilityID, "name", name, "version_id", latestVersionID, "err", err) diff --git a/server/internal/dev/routes_test.go b/server/internal/dev/routes_test.go index 8d204127..8e3f0b91 100644 --- a/server/internal/dev/routes_test.go +++ b/server/internal/dev/routes_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/MiniMax-AI-Dev/parsar/server/internal/auth" + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" gatewaypkg "github.com/MiniMax-AI-Dev/parsar/server/internal/gateway" "github.com/MiniMax-AI-Dev/parsar/server/internal/secrets" "github.com/MiniMax-AI-Dev/parsar/server/internal/store" @@ -2472,6 +2473,48 @@ type stubRuntimeStore struct { httpEndpoint string } +type createAgentOAuthValidationStore struct { + stubRuntimeStore + secretProvider string + createCalls int +} + +func (s *createAgentOAuthValidationStore) GetCapabilityVersion(context.Context, string) (store.CapabilityVersionRead, error) { + return store.CapabilityVersionRead{ + ID: "00000000-0000-0000-0000-000000000c02", + CapabilityID: "00000000-0000-0000-0000-000000000c01", + Version: "1.0.0", + SourcePayload: json.RawMessage(`{"source_format":"mcp_catalog","catalog_id":"notion"}`), + RequiredCredentials: []store.RequiredCredential{{Kind: capability.CredentialKindMCPOAuth, Required: true}}, + }, nil +} + +func (s *createAgentOAuthValidationStore) GetCapability(context.Context, string) (store.CapabilityRead, error) { + return store.CapabilityRead{ + ID: "00000000-0000-0000-0000-000000000c01", + WorkspaceID: "00000000-0000-0000-0000-000000000002", + Type: "mcp", + Visibility: "workspace", + Status: "active", + }, nil +} + +func (s *createAgentOAuthValidationStore) GetSecretPayload(context.Context, string, string) (store.SecretPayload, error) { + return store.SecretPayload{SecretRead: store.SecretRead{ + ID: "00000000-0000-0000-0000-000000000099", + Kind: "capability_inline", + Provider: s.secretProvider, + AuthType: "oauth2", + Status: "active", + Metadata: map[string]any{"credential_kind_code": capability.CredentialKindMCPOAuth}, + }}, nil +} + +func (s *createAgentOAuthValidationStore) CreateAgent(ctx context.Context, input store.CreateAgentInput) (store.CreateAgentResult, error) { + s.createCalls++ + return s.stubRuntimeStore.CreateAgent(ctx, input) +} + type roleStubStore struct { stubRuntimeStore roles map[string]string @@ -4046,6 +4089,51 @@ func TestCreateAgentAPIHappyPathAndNameConflict(t *testing.T) { } } +func TestCreateAgentValidatesInitialCapabilityOAuthBinding(t *testing.T) { + const body = `{ + "name":"OAuth Agent", + "connector_type":"agent_daemon", + "visibility":"workspace", + "initial_capabilities":[{ + "capability_version_id":"00000000-0000-0000-0000-000000000c02", + "configuration":{"credential_bindings":{"mcp_oauth":{"source":"shared","secret_id":"00000000-0000-0000-0000-000000000099"}}} + }], + "config":{"daemon_mode":"sandbox","agent_kind":"opencode"} + }` + + t.Run("rejects a secret from another connector", func(t *testing.T) { + storeStub := &createAgentOAuthValidationStore{secretProvider: "github"} + r := chi.NewRouter() + RegisterRoutesWithStore(r, storeStub) + req := withTestUser(httptest.NewRequest(http.MethodPost, "/api/v1/workspaces/00000000-0000-0000-0000-000000000002/agents", strings.NewReader(body))) + req.Header.Set("Content-Type", "application/json") + res := httptest.NewRecorder() + r.ServeHTTP(res, req) + if res.Code != http.StatusUnprocessableEntity || !strings.Contains(res.Body.String(), "different MCP connector") { + t.Fatalf("expected 422 connector mismatch, got %d: %s", res.Code, res.Body.String()) + } + if storeStub.createCalls != 0 { + t.Fatalf("CreateAgent calls = %d, want 0", storeStub.createCalls) + } + }) + + t.Run("accepts a matching connector secret", func(t *testing.T) { + storeStub := &createAgentOAuthValidationStore{secretProvider: "notion"} + r := chi.NewRouter() + RegisterRoutesWithStore(r, storeStub) + req := withTestUser(httptest.NewRequest(http.MethodPost, "/api/v1/workspaces/00000000-0000-0000-0000-000000000002/agents", strings.NewReader(body))) + req.Header.Set("Content-Type", "application/json") + res := httptest.NewRecorder() + r.ServeHTTP(res, req) + if res.Code != http.StatusCreated { + t.Fatalf("expected 201, got %d: %s", res.Code, res.Body.String()) + } + if storeStub.createCalls != 1 { + t.Fatalf("CreateAgent calls = %d, want 1", storeStub.createCalls) + } + }) +} + func TestUpdateAgentAPIHappyPathAndImmutableSlug(t *testing.T) { r := chi.NewRouter() RegisterRoutesWithStore(r, stubRuntimeStore{}) diff --git a/server/internal/mcpcatalog/catalog_test.go b/server/internal/mcpcatalog/catalog_test.go index eb329832..40e71e39 100644 --- a/server/internal/mcpcatalog/catalog_test.go +++ b/server/internal/mcpcatalog/catalog_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "strings" "testing" + + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" ) func TestBuiltinCatalogLoads(t *testing.T) { @@ -25,7 +27,7 @@ func TestBuiltinCatalogLoads(t *testing.T) { } if item.ID == "notion" { header := item.CanonicalSpec().MCP.Servers[0].Headers["Authorization"] - if header.Prefix != "Bearer " || header.CredentialKindCode != OAuthCredentialKind { + if header.Prefix != "Bearer " || header.CredentialKindCode != capability.CredentialKindMCPOAuth { t.Fatalf("notion authorization header = %+v", header) } } diff --git a/server/internal/mcpcatalog/types.go b/server/internal/mcpcatalog/types.go index 44f3379a..1d04f1f4 100644 --- a/server/internal/mcpcatalog/types.go +++ b/server/internal/mcpcatalog/types.go @@ -1,13 +1,11 @@ package mcpcatalog import ( + "github.com/MiniMax-AI-Dev/parsar/server/internal/capability" "github.com/MiniMax-AI-Dev/parsar/server/internal/capability/canonical" ) -const ( - SchemaVersion = 1 - OAuthCredentialKind = "mcp_oauth" -) +const SchemaVersion = 1 type Catalog struct { SchemaVersion int `json:"schema_version"` @@ -64,7 +62,7 @@ func (i Item) CanonicalSpec() canonical.Spec { "Authorization": { Mode: canonical.EnvModeCredentialRef, Prefix: "Bearer ", - CredentialKindCode: OAuthCredentialKind, + CredentialKindCode: capability.CredentialKindMCPOAuth, }, } } From 4e5cbd2db29a2e7ec466426f57d44aa9f07ffd2b Mon Sep 17 00:00:00 2001 From: kapelame Date: Sun, 26 Jul 2026 16:13:47 +0800 Subject: [PATCH 5/8] refactor: share credential binding picker --- .../admin/CredentialBindingSelect.tsx | 46 ++++++++++++++ .../components/admin/CredentialCheckPanel.tsx | 45 ++++++-------- apps/web/src/lib/credential-bindings.ts | 47 ++++++++++++++ .../src/pages/admin/agents/AgentConfigTab.tsx | 61 +++---------------- 4 files changed, 121 insertions(+), 78 deletions(-) create mode 100644 apps/web/src/components/admin/CredentialBindingSelect.tsx create mode 100644 apps/web/src/lib/credential-bindings.ts diff --git a/apps/web/src/components/admin/CredentialBindingSelect.tsx b/apps/web/src/components/admin/CredentialBindingSelect.tsx new file mode 100644 index 00000000..111e0d34 --- /dev/null +++ b/apps/web/src/components/admin/CredentialBindingSelect.tsx @@ -0,0 +1,46 @@ +import type { Secret } from "../../lib/api-types" + +interface CredentialBindingSelectProps { + value: string + secrets: Secret[] + allowPersonal: boolean + allowCreateNew?: boolean + personalLabel: string + sharedLabel: string + personalPlaceholder?: string + createNewLabel?: string + onChange: (value: string) => void + className?: string +} + +/** Shared source selector used by Agent creation and Capability enabling. */ +export function CredentialBindingSelect({ + value, + secrets, + allowPersonal, + allowCreateNew = false, + personalLabel, + sharedLabel, + personalPlaceholder, + createNewLabel, + onChange, + className = "h-7 w-full rounded border border-line bg-surface px-2 text-sm", +}: CredentialBindingSelectProps) { + return ( + + ) +} diff --git a/apps/web/src/components/admin/CredentialCheckPanel.tsx b/apps/web/src/components/admin/CredentialCheckPanel.tsx index 271e4577..05ead6d7 100644 --- a/apps/web/src/components/admin/CredentialCheckPanel.tsx +++ b/apps/web/src/components/admin/CredentialCheckPanel.tsx @@ -4,6 +4,7 @@ import { Check, ChevronDown, ChevronRight, Eye, EyeOff, ExternalLink, Loader2, S import { Button } from "../ui/button" import { Input } from "../ui/input" +import { CredentialBindingSelect } from "./CredentialBindingSelect" import { useMyCredentials } from "../../lib/api-credentials" import { credentialKindLabel, @@ -12,12 +13,9 @@ import { type KnownCredentialKind, } from "../../lib/credential-kind-ui" import type { AgentInlineNewSecret, RequiredCredential, Secret } from "../../lib/api-types" +import { hasCredentialKind, sharedSecretsForKind, type PerKindBindingChoice } from "../../lib/credential-bindings" -/** PerKindBinding is the per-credential decision made in the picker. */ -export type PerKindBindingChoice = - | { source: "personal" } - | { source: "shared"; existing_secret_id: string } - | { source: "shared"; new_secret: { display_name: string; plaintext: string } } +export type { PerKindBindingChoice } from "../../lib/credential-bindings" interface CredentialCheckPanelProps { /** Only entries with required===true should be passed. */ @@ -171,7 +169,7 @@ export function CredentialCheckPanel({ for (const rc of requiredKinds) { const choice = choices[rc.kind] if (choice?.source !== "personal") continue - if (!(credentials ?? []).some((c) => c.kind === rc.kind)) { + if (!hasCredentialKind(credentials ?? [], rc.kind)) { // Personal but the creator has not configured this kind. Allow // the pick (other callers may have it), but signal invalid so // the create button stays disabled until they add it OR switch @@ -222,15 +220,8 @@ export function CredentialCheckPanel({ {requiredKinds.map((rc) => { const { displayName, placeholder, getUrl } = getKindMeta(rc.kind) const choice = choices[rc.kind] - const hasPersonalCredential = (credentials ?? []).some((c) => c.kind === rc.kind) - const kindSecrets = sharedSecrets.filter((s) => { - if (s.kind !== "capability_inline") return false - const metaCode = (s.metadata as { credential_kind_code?: unknown } | undefined)?.credential_kind_code - // Untagged legacy secrets surface for every kind (operator's - // responsibility to pick the right one); new secrets are - // always tagged so this only matters for pre-2026-06 rows. - return typeof metaCode !== "string" || metaCode === "" || metaCode === rc.kind - }) + const hasPersonalCredential = hasCredentialKind(credentials ?? [], rc.kind) + const kindSecrets = sharedSecretsForKind(sharedSecrets, rc.kind) return (
@@ -304,29 +295,29 @@ export function CredentialCheckPanel({ {choice?.source === "shared" && (
{kindSecrets.length > 0 && ( - + /> )} {kindSecrets.length === 0 && expandedNewSecretFor !== rc.kind && !("new_secret" in choice) && (