From d869cc1ff698d322a83cfd4aef090e83c8ae1a78 Mon Sep 17 00:00:00 2001 From: "ochan.kwon" Date: Sat, 18 Apr 2026 21:48:50 +0900 Subject: [PATCH] fix: redact sensitive fields and prevent credential loss on edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #988 Unify the "write-only secrets" pattern across MCP services, Models, WebSearch providers, and DataSources. Fix two related defects: 1. GET responses leaked plaintext credentials — GetMCPServiceByID while List masked, all Model / WebSearchProvider / DataSource GET responses unconditionally. 2. When the frontend echoed a masked placeholder back on save, the update path overwrote the stored secret with the mask — silently wiping the real credential. MCP surfaced this most visibly: ListMCPServices masked Token to "first4****last4" and McpServiceDialog round-tripped that masked string back into the DB. - internal/types/secret.go introduces the shared sentinel RedactedSecretPlaceholder = "***" and two helpers: - IsRedactedOrEmpty(s): true for "" or the sentinel - PreserveIfRedacted(incoming, existing): merge predicate - Each resource adds a RedactSensitiveData (mutates-in-place) or Redacted (returns-a-copy) method and a MergeUpdate contract: empty or placeholder preserves, any other value replaces, and a write-only Clear* bool flag (gorm:"-", json omitempty) wipes with precedence over preserve / replace. Clear* never persists. - Every GET / List / Create / Update response path now redacts. VectorStore's pre-existing "***" literal is refactored onto the shared constant for consistency. All four dialogs — McpServiceDialog, ModelEditorDialog, WebSearchSettings, DataSourceEditorDialog — never pre-fill secret inputs. The stored-state signal is collapsed into the input placeholder: "•••••••• (Enter new value to replace)" when the server reports a stored value, falling back to the connector-specific placeholder otherwise. The input itself carries the "something is there" cue, matching how password dots render. An opt-in "Remove this credential" checkbox appears in edit mode when a value is stored; checking it disables the input and triggers a danger-themed confirmation dialog before submit. DataSource rolls the clear up to a connector-level "Remove all credentials" flag since its credentials are per-connector atomic bundles (e.g. Feishu app_id + app_secret). i18n updates across en-US, zh-CN, ko-KR, ru-RU: secret.storedPlaceholder / clearHint / confirmClearTitle / confirmClearBody. - MCP update handler's manual map-to-struct projection was silently dropping clear_api_key / clear_token / custom_headers on partial updates. Pull them off the map explicitly. - MCP configChanged detection now snapshots URL / StdioConfig / TransportType before the merge so connection recycling still triggers on real transport changes (not just whenever AuthConfig is present). - MCP CustomHeaders: nil preserves existing, non-nil replaces. Was always overwriting, silently wiping headers on any auth-bearing update. - GetMCPServiceByID now redacts (was plaintext while List masked). - WebSearchProvider update preserves Name / Description when empty instead of clobbering them. - WebSearchProvider TestProviderRaw rejects the "***" placeholder up front with an actionable message rather than forwarding it to the upstream provider API as a literal key. - Model builtin hideSensitiveInfo now preserves non-secret metadata (SupportsVision, Provider, ExtraConfig, InterfaceType, ParameterSize, EmbeddingParameters) that were previously zeroed out for builtin model cards. - Replace local equalStringSlices / equalStringMaps helpers with stdlib slices.Equal / maps.Equal (Go 1.21+). - internal/types/{secret,mcp,model,web_search_provider,datasource}_test.go: table-driven coverage of merge, redact, clear, and write-only flag invariants for every resource. - internal/application/service/mcp_service_test.go: full service layer with a fake repo; covers ListMCPServices / GetMCPServiceByID redaction, UpdateMCPService partial-update honoring of Clear*, CustomHeaders preserve-on-nil / replace-on-non-nil, and clear-flag precedence over submitted values. Additive only. No changes to request or response JSON shape beyond the write-only Clear* flags (omitted by default via omitempty). No DB schema changes. No SDK struct field removals. Existing clients that don't know about the new flags continue to function unchanged. - DataSource.Config jsonb at-rest encryption. Pre-existing residual risk; tracked as a follow-up. - Renaming VectorStore.MaskSensitiveFields to match the new RedactSensitiveData / Redacted naming convention. Left alone to minimize diff; a follow-up rename PR can unify. --- client/mcp_service.go | 10 +- frontend/src/api/mcp-service.ts | 4 + frontend/src/api/model/index.ts | 6 + frontend/src/api/web-search-provider.ts | 3 + frontend/src/components/ModelEditorDialog.vue | 70 +++- frontend/src/i18n/locales/en-US.ts | 11 + frontend/src/i18n/locales/ko-KR.ts | 7 + frontend/src/i18n/locales/ru-RU.ts | 7 + frontend/src/i18n/locales/zh-CN.ts | 7 + .../settings/DataSourceEditorDialog.vue | 111 ++++- frontend/src/views/settings/ModelSettings.vue | 26 +- .../src/views/settings/WebSearchSettings.vue | 84 +++- .../settings/components/McpServiceDialog.vue | 186 ++++++++- .../application/service/datasource_service.go | 53 ++- internal/application/service/mcp_service.go | 123 ++++-- .../application/service/mcp_service_test.go | 395 ++++++++++++++++++ internal/handler/datasource.go | 6 + internal/handler/mcp_service.go | 30 ++ internal/handler/model.go | 92 ++-- internal/handler/web_search_provider.go | 46 +- internal/types/datasource.go | 102 ++++- internal/types/datasource_test.go | 206 +++++++++ internal/types/mcp.go | 90 +++- internal/types/mcp_test.go | 52 +++ internal/types/model.go | 51 +++ internal/types/model_test.go | 139 ++++++ internal/types/secret.go | 56 +++ internal/types/secret_test.go | 51 +++ internal/types/vectorstore.go | 8 +- internal/types/vectorstore_test.go | 8 +- internal/types/web_search_provider.go | 42 ++ internal/types/web_search_provider_test.go | 87 ++++ 32 files changed, 2039 insertions(+), 130 deletions(-) create mode 100644 internal/application/service/mcp_service_test.go create mode 100644 internal/types/datasource_test.go create mode 100644 internal/types/mcp_test.go create mode 100644 internal/types/model_test.go create mode 100644 internal/types/secret.go create mode 100644 internal/types/secret_test.go create mode 100644 internal/types/web_search_provider_test.go diff --git a/client/mcp_service.go b/client/mcp_service.go index fa19978f..c76a1c97 100644 --- a/client/mcp_service.go +++ b/client/mcp_service.go @@ -35,11 +35,19 @@ type MCPService struct { UpdatedAt string `json:"updated_at"` } -// MCPAuthConfig represents authentication configuration for MCP service +// MCPAuthConfig represents authentication configuration for MCP service. +// +// ClearAPIKey and ClearToken are write-only flags used in Update requests to +// explicitly remove a stored credential. The server never returns these +// fields in responses. type MCPAuthConfig struct { APIKey string `json:"api_key,omitempty"` Token string `json:"token,omitempty"` CustomHeaders map[string]string `json:"custom_headers,omitempty"` + + // Write-only clear flags. + ClearAPIKey bool `json:"clear_api_key,omitempty"` + ClearToken bool `json:"clear_token,omitempty"` } // MCPAdvancedConfig represents advanced configuration for MCP service diff --git a/frontend/src/api/mcp-service.ts b/frontend/src/api/mcp-service.ts index 1aa7a2ee..2b969dda 100644 --- a/frontend/src/api/mcp-service.ts +++ b/frontend/src/api/mcp-service.ts @@ -13,6 +13,10 @@ export interface MCPService { api_key?: string token?: string custom_headers?: Record + // Write-only flags — set to true in Update requests to explicitly remove + // the corresponding credential. The server never returns these fields. + clear_api_key?: boolean + clear_token?: boolean } advanced_config?: { timeout?: number diff --git a/frontend/src/api/model/index.ts b/frontend/src/api/model/index.ts index c108de4e..3ca7ac7a 100644 --- a/frontend/src/api/model/index.ts +++ b/frontend/src/api/model/index.ts @@ -26,6 +26,12 @@ export interface ModelConfig { // 会在调用远程模型 API 时附加到每个请求上。Authorization、Content-Type 等保留头会被忽略。 custom_headers?: Record; supports_vision?: boolean; // Whether the model accepts image/multimodal input + app_id?: string; + app_secret?: string; + // Write-only flags — set to true in Update requests to explicitly remove + // the corresponding credential. The server never returns these fields. + clear_api_key?: boolean; + clear_app_secret?: boolean; }; is_default?: boolean; is_builtin?: boolean; diff --git a/frontend/src/api/web-search-provider.ts b/frontend/src/api/web-search-provider.ts index 1a978d9b..5c0c8c95 100644 --- a/frontend/src/api/web-search-provider.ts +++ b/frontend/src/api/web-search-provider.ts @@ -13,6 +13,9 @@ export interface WebSearchProviderEntity { base_url?: string proxy_url?: string extra_config?: Record + // Write-only flag — set to true in Update requests to explicitly remove + // the stored API key. The server never returns this field. + clear_api_key?: boolean } is_default?: boolean created_at?: string diff --git a/frontend/src/components/ModelEditorDialog.vue b/frontend/src/components/ModelEditorDialog.vue index df8b45f1..6b694ed8 100644 --- a/frontend/src/components/ModelEditorDialog.vue +++ b/frontend/src/components/ModelEditorDialog.vue @@ -211,8 +211,16 @@ + + {{ t('secret.clearHint') }} + @@ -331,7 +339,7 @@