fix: redact sensitive fields and prevent credential loss on edit
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -13,6 +13,10 @@ export interface MCPService {
|
||||
api_key?: string
|
||||
token?: string
|
||||
custom_headers?: Record<string, 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_token?: boolean
|
||||
}
|
||||
advanced_config?: {
|
||||
timeout?: number
|
||||
|
||||
@@ -26,6 +26,12 @@ export interface ModelConfig {
|
||||
// 会在调用远程模型 API 时附加到每个请求上。Authorization、Content-Type 等保留头会被忽略。
|
||||
custom_headers?: Record<string, string>;
|
||||
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;
|
||||
|
||||
@@ -13,6 +13,9 @@ export interface WebSearchProviderEntity {
|
||||
base_url?: string
|
||||
proxy_url?: string
|
||||
extra_config?: Record<string, string>
|
||||
// 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
|
||||
|
||||
@@ -211,8 +211,16 @@
|
||||
<t-input
|
||||
v-model="formData.apiKey"
|
||||
type="password"
|
||||
:placeholder="$t('model.editor.apiKeyPlaceholder')"
|
||||
:disabled="formData.clearApiKey"
|
||||
:placeholder="apiKeyPlaceholder"
|
||||
/>
|
||||
<t-checkbox
|
||||
v-if="isEdit && formData.hasExistingApiKey"
|
||||
v-model="formData.clearApiKey"
|
||||
class="clear-credential"
|
||||
>
|
||||
{{ t('secret.clearHint') }}
|
||||
</t-checkbox>
|
||||
</div>
|
||||
|
||||
<!-- 自定义 HTTP Header(类似 OpenAI Python SDK 的 extra_headers) -->
|
||||
@@ -331,7 +339,7 @@
|
||||
|
||||
<script setup lang="ts">
|
||||
import { ref, watch, computed, onUnmounted, nextTick } from 'vue'
|
||||
import { MessagePlugin } from 'tdesign-vue-next'
|
||||
import { MessagePlugin, DialogPlugin } from 'tdesign-vue-next'
|
||||
import { checkOllamaModels, checkRemoteModel, testEmbeddingModel, checkRerankModel, checkASRModel, listOllamaModels, downloadOllamaModel, getDownloadProgress, checkOllamaStatus, listModelProviders, type OllamaModelInfo, type ModelProviderOption } from '@/api/initialization'
|
||||
import { getWeKnoraCloudStatus } from '@/api/model'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
@@ -352,6 +360,12 @@ interface ModelFormData {
|
||||
modelName: string
|
||||
baseUrl?: string
|
||||
apiKey?: string
|
||||
// True when the server returned '***' for parameters.api_key, meaning a
|
||||
// key is currently stored server-side. Drives the "Set / Not set" badge.
|
||||
hasExistingApiKey?: boolean
|
||||
// Write-only flag set by the user to explicitly remove the stored key on
|
||||
// save. Reset to false on every dialog open.
|
||||
clearApiKey?: boolean
|
||||
dimension?: number
|
||||
interfaceType?: 'ollama' | 'openai'
|
||||
isDefault: boolean
|
||||
@@ -539,6 +553,17 @@ const dialogVisible = computed({
|
||||
|
||||
const isEdit = computed(() => !!props.modelData)
|
||||
|
||||
// Placeholder hint for the API key field. On edit with a stored key, swap
|
||||
// in the shared "bullets + Enter new value to replace" placeholder so the
|
||||
// input itself carries the "something is there" signal. Otherwise fall back
|
||||
// to the generic creation-mode placeholder.
|
||||
const apiKeyPlaceholder = computed(() => {
|
||||
if (isEdit.value && formData.value.hasExistingApiKey) {
|
||||
return t('secret.storedPlaceholder')
|
||||
}
|
||||
return t('model.editor.apiKeyPlaceholder')
|
||||
})
|
||||
|
||||
const formRef = ref()
|
||||
const saving = ref(false)
|
||||
const modelChecked = ref(false)
|
||||
@@ -745,8 +770,14 @@ watch(() => props.visible, (val) => {
|
||||
|
||||
if (props.modelData) {
|
||||
// 编辑:始终用最新的 modelData 覆盖
|
||||
// Always reset apiKey / clearApiKey on every dialog open so the
|
||||
// invariant "non-empty apiKey means the user typed it" holds.
|
||||
// hasExistingApiKey flows through unchanged so the badge keeps showing
|
||||
// "Set" when the server said '***'.
|
||||
formData.value = {
|
||||
...props.modelData,
|
||||
apiKey: '',
|
||||
clearApiKey: false,
|
||||
customHeaders: Array.isArray(props.modelData.customHeaders)
|
||||
? props.modelData.customHeaders.map(h => ({ key: h.key, value: h.value }))
|
||||
: []
|
||||
@@ -781,6 +812,8 @@ const resetForm = () => {
|
||||
modelName: '',
|
||||
baseUrl: '',
|
||||
apiKey: '',
|
||||
hasExistingApiKey: false,
|
||||
clearApiKey: false,
|
||||
dimension: undefined, // 默认不填,让用户手动输入或通过检测按钮获取
|
||||
interfaceType: undefined,
|
||||
isDefault: false,
|
||||
@@ -1117,13 +1150,30 @@ const handleConfirm = async () => {
|
||||
|
||||
// 执行表单验证
|
||||
await formRef.value?.validate()
|
||||
|
||||
// Confirm irrevocable credential removal before emitting.
|
||||
if (formData.value.clearApiKey) {
|
||||
const ok = await new Promise<boolean>((resolve) => {
|
||||
const d = DialogPlugin.confirm({
|
||||
header: t('secret.confirmClearTitle'),
|
||||
body: t('secret.confirmClearBody'),
|
||||
confirmBtn: { content: t('common.confirm'), theme: 'danger' },
|
||||
cancelBtn: t('common.cancel'),
|
||||
onConfirm: () => { d.hide(); resolve(true) },
|
||||
onCancel: () => { d.hide(); resolve(false) },
|
||||
onClose: () => { d.hide(); resolve(false) },
|
||||
})
|
||||
})
|
||||
if (!ok) return
|
||||
}
|
||||
|
||||
saving.value = true
|
||||
|
||||
|
||||
// 如果是新增且没有 id,生成一个
|
||||
if (!formData.value.id) {
|
||||
formData.value.id = generateId()
|
||||
}
|
||||
|
||||
|
||||
emit('confirm', { ...formData.value })
|
||||
dialogVisible.value = false
|
||||
// 保存成功后重置草稿,下次打开新增模型时是空白
|
||||
@@ -1658,6 +1708,18 @@ const handleCancel = () => {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Destructive-action checkbox for "Remove this credential". Styled to match
|
||||
// the pattern used in McpServiceDialog so the two dialogs read identically.
|
||||
.clear-credential {
|
||||
display: inline-flex;
|
||||
margin-top: 8px;
|
||||
|
||||
:deep(.t-checkbox__label) {
|
||||
color: var(--td-error-color);
|
||||
font-size: 13px;
|
||||
}
|
||||
}
|
||||
</style>
|
||||
|
||||
<!-- 非 scoped 样式:t-select popup 渲染到 body 下,scoped 样式无法覆盖 -->
|
||||
|
||||
@@ -4284,4 +4284,15 @@ export default {
|
||||
createdAt: 'Created at',
|
||||
},
|
||||
},
|
||||
secret: {
|
||||
// Placeholder shown inside a credential input when a value is currently
|
||||
// stored server-side. The bullet run doubles as a visual "something is
|
||||
// there" cue (matching how the password type renders dots); the
|
||||
// parenthetical tells the user what typing will do.
|
||||
storedPlaceholder: '•••••••• (Enter new value to replace)',
|
||||
clearHint: 'Remove this credential',
|
||||
confirmClearTitle: 'Confirm credential removal',
|
||||
confirmClearBody:
|
||||
'This permanently deletes the stored credential. Integrations using it will stop working. Continue?',
|
||||
},
|
||||
}
|
||||
|
||||
@@ -4347,4 +4347,11 @@ export default {
|
||||
createdAt: "생성 시각",
|
||||
},
|
||||
},
|
||||
secret: {
|
||||
storedPlaceholder: "•••••••• (새 값을 입력하면 교체됩니다)",
|
||||
clearHint: "이 credential 제거",
|
||||
confirmClearTitle: "credential 제거 확인",
|
||||
confirmClearBody:
|
||||
"저장된 credential이 영구 삭제되며, 이를 사용하는 통합이 중단됩니다. 계속할까요?",
|
||||
},
|
||||
};
|
||||
|
||||
@@ -4247,4 +4247,11 @@ export default {
|
||||
createdAt: 'Создано',
|
||||
},
|
||||
},
|
||||
secret: {
|
||||
storedPlaceholder: '•••••••• (Введите новое значение для замены)',
|
||||
clearHint: 'Удалить эти учётные данные',
|
||||
confirmClearTitle: 'Подтверждение удаления учётных данных',
|
||||
confirmClearBody:
|
||||
'Это действие безвозвратно удалит сохранённые учётные данные. Интеграции, использующие их, перестанут работать. Продолжить?',
|
||||
},
|
||||
}
|
||||
|
||||
@@ -4280,4 +4280,11 @@ export default {
|
||||
createdAt: "创建时间",
|
||||
},
|
||||
},
|
||||
secret: {
|
||||
storedPlaceholder: "•••••••• (输入新值以替换)",
|
||||
clearHint: "删除此凭据",
|
||||
confirmClearTitle: "确认删除凭据",
|
||||
confirmClearBody:
|
||||
"此操作将永久删除已保存的凭据,依赖它的集成将停止工作。是否继续?",
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
<script setup lang="ts">
|
||||
import { ref, computed, watch } from 'vue'
|
||||
import { MessagePlugin } from 'tdesign-vue-next'
|
||||
import { MessagePlugin, DialogPlugin } from 'tdesign-vue-next'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
import {
|
||||
createDataSource,
|
||||
@@ -28,6 +28,38 @@ const isEdit = computed(() => !!props.dataSource)
|
||||
const step = ref(0)
|
||||
const submitting = ref(false)
|
||||
|
||||
// Fixed placeholder returned by the server for redacted credential strings.
|
||||
// Must match internal/types/secret.go → RedactedSecretPlaceholder.
|
||||
const REDACTED_PLACEHOLDER = '***'
|
||||
|
||||
// Per-field: is this specific credential key stored server-side? Drives the
|
||||
// per-input placeholder swap to the "••••••• (Enter new value to replace)"
|
||||
// hint when the server marked it present.
|
||||
function hasFieldCredential(key: string): boolean {
|
||||
return props.dataSource?.config?.credentials?.[key] === REDACTED_PLACEHOLDER
|
||||
}
|
||||
|
||||
// Connector-level rollup: true when any credential key is stored. Used to
|
||||
// decide whether to render the "Remove all credentials" checkbox.
|
||||
const hasExistingCredentials = computed(() => {
|
||||
const creds = props.dataSource?.config?.credentials
|
||||
if (!creds) return false
|
||||
return Object.values(creds).some(v => v === REDACTED_PLACEHOLDER)
|
||||
})
|
||||
|
||||
// Placeholder for a given credential input: stored → shared hint,
|
||||
// otherwise the connector-specific default (e.g. "ntn_xxxx" for Notion).
|
||||
function credentialPlaceholder(field: { key: string; placeholder: string }): string {
|
||||
return hasFieldCredential(field.key)
|
||||
? t('secret.storedPlaceholder')
|
||||
: field.placeholder
|
||||
}
|
||||
|
||||
// Explicit "remove all credentials" flag. Because credentials form an atomic
|
||||
// set per connector (e.g. an OAuth pair) we only support all-or-nothing
|
||||
// clearing, mirroring the backend DataSourceConfig.ClearCredentials contract.
|
||||
const clearCredentials = ref(false)
|
||||
|
||||
// Form data
|
||||
const form = ref({
|
||||
name: '',
|
||||
@@ -213,10 +245,22 @@ watch(visible, (v) => {
|
||||
selectedResourceIds.value = []
|
||||
|
||||
if (isEdit.value && props.dataSource) {
|
||||
// Reset clear intent on every open so an aborted deletion doesn't
|
||||
// carry over to the next edit session.
|
||||
clearCredentials.value = false
|
||||
// Credentials are intentionally NOT pre-filled from the server response
|
||||
// — even though the server returns '***' for stored secrets, the form
|
||||
// holds an empty map. This keeps the "non-empty means user typed it"
|
||||
// invariant that the save path relies on, and the badge reflects the
|
||||
// stored state via hasExistingCredentials instead.
|
||||
form.value = {
|
||||
name: props.dataSource.name,
|
||||
type: props.dataSource.type,
|
||||
config: props.dataSource.config || { credentials: {}, resource_ids: [], settings: {} },
|
||||
config: {
|
||||
credentials: {},
|
||||
resource_ids: props.dataSource.config?.resource_ids || [],
|
||||
settings: props.dataSource.config?.settings || {},
|
||||
},
|
||||
sync_schedule: props.dataSource.sync_schedule,
|
||||
sync_mode: props.dataSource.sync_mode,
|
||||
conflict_strategy: props.dataSource.conflict_strategy,
|
||||
@@ -225,6 +269,7 @@ watch(visible, (v) => {
|
||||
selectedResourceIds.value = form.value.config?.resource_ids || []
|
||||
tempDsId.value = props.dataSource.id
|
||||
} else {
|
||||
clearCredentials.value = false
|
||||
form.value = {
|
||||
name: '',
|
||||
type: '',
|
||||
@@ -405,8 +450,45 @@ function prevStep() {
|
||||
step.value--
|
||||
}
|
||||
|
||||
// Prompt the user before irrevocable credential removal.
|
||||
function confirmClearIfNeeded(): Promise<boolean> {
|
||||
if (!clearCredentials.value) return Promise.resolve(true)
|
||||
return new Promise((resolve) => {
|
||||
const d = DialogPlugin.confirm({
|
||||
header: t('secret.confirmClearTitle'),
|
||||
body: t('secret.confirmClearBody'),
|
||||
confirmBtn: { content: t('common.confirm'), theme: 'danger' },
|
||||
cancelBtn: t('common.cancel'),
|
||||
onConfirm: () => { d.hide(); resolve(true) },
|
||||
onCancel: () => { d.hide(); resolve(false) },
|
||||
onClose: () => { d.hide(); resolve(false) },
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// Build the config payload for Create / Update requests. In edit mode this
|
||||
// applies write-only secret semantics:
|
||||
// - clearCredentials checked → send clear_credentials: true (backend wipes)
|
||||
// - user typed credential values → send them as-is
|
||||
// - user left fields blank → send empty credentials map (backend preserves
|
||||
// every existing credential key individually)
|
||||
function buildConfigPayload(): Record<string, unknown> {
|
||||
const cfg: Record<string, unknown> = {
|
||||
credentials: clearCredentials.value ? {} : { ...form.value.config.credentials },
|
||||
resource_ids: form.value.config.resource_ids,
|
||||
settings: form.value.config.settings,
|
||||
}
|
||||
if (clearCredentials.value) {
|
||||
cfg.clear_credentials = true
|
||||
}
|
||||
return cfg
|
||||
}
|
||||
|
||||
// --- Final submit ---
|
||||
async function handleSubmit() {
|
||||
const ok = await confirmClearIfNeeded()
|
||||
if (!ok) return
|
||||
|
||||
form.value.config.resource_ids = selectedResourceIds.value
|
||||
submitting.value = true
|
||||
try {
|
||||
@@ -415,12 +497,14 @@ async function handleSubmit() {
|
||||
if (tempDsId.value) {
|
||||
await updateDataSource(tempDsId.value, {
|
||||
...form.value,
|
||||
config: buildConfigPayload(),
|
||||
knowledge_base_id: props.kbId,
|
||||
status: 'active',
|
||||
} as any)
|
||||
} else {
|
||||
const res = await createDataSource({
|
||||
...form.value,
|
||||
config: buildConfigPayload(),
|
||||
knowledge_base_id: props.kbId,
|
||||
status: 'active',
|
||||
} as any)
|
||||
@@ -575,6 +659,14 @@ const stepTitles = computed(() => [
|
||||
</a>
|
||||
</div>
|
||||
|
||||
<!--
|
||||
Credential fields. When the server returned '***' for a field the
|
||||
per-input placeholder swaps to the shared "••••••• (Enter new value
|
||||
to replace)" hint so the input carries the "something is stored"
|
||||
signal without a separate badge. The "Remove all credentials"
|
||||
checkbox below handles the destructive clear for the whole set
|
||||
(DataSource credentials are per-connector atomic bundles).
|
||||
-->
|
||||
<div v-for="field in currentDef?.fields || []" :key="field.key" class="form-item">
|
||||
<label class="form-label">
|
||||
{{ t(field.labelKey) }}
|
||||
@@ -582,12 +674,19 @@ const stepTitles = computed(() => [
|
||||
</label>
|
||||
<t-input
|
||||
v-model="form.config.credentials[field.key]"
|
||||
:placeholder="field.placeholder"
|
||||
:disabled="clearCredentials"
|
||||
:placeholder="credentialPlaceholder(field)"
|
||||
:type="field.secret ? 'password' : 'text'"
|
||||
/>
|
||||
<div v-if="field.hintKey" class="form-hint">{{ t(field.hintKey) }}</div>
|
||||
</div>
|
||||
|
||||
<div v-if="isEdit && hasExistingCredentials" class="form-item">
|
||||
<t-checkbox v-model="clearCredentials" class="clear-credential">
|
||||
{{ t('secret.clearHint') }}
|
||||
</t-checkbox>
|
||||
</div>
|
||||
|
||||
<div class="form-actions">
|
||||
<t-button variant="outline" :loading="testing" @click="testConnection">
|
||||
{{ t('datasource.testConnection') }}
|
||||
@@ -886,6 +985,12 @@ const stepTitles = computed(() => [
|
||||
.form-item { margin-bottom: 16px; }
|
||||
.form-label { display: block; font-size: 13px; font-weight: 500; margin-bottom: 6px; color: var(--td-text-color-primary); }
|
||||
.required-mark { color: var(--td-error-color); margin-left: 2px; }
|
||||
|
||||
/* Destructive-action checkbox — red label, matches the other 3 dialogs. */
|
||||
.clear-credential :deep(.t-checkbox__label) {
|
||||
color: var(--td-error-color);
|
||||
font-size: 13px;
|
||||
}
|
||||
.form-tip { font-size: 12px; color: var(--td-text-color-placeholder); margin: 4px 0 12px; }
|
||||
.form-hint { font-size: 12px; color: var(--td-text-color-placeholder); margin-top: 6px; line-height: 1.5; }
|
||||
.form-actions { display: flex; align-items: center; gap: 8px; margin-top: 12px; }
|
||||
|
||||
@@ -138,6 +138,13 @@ const backendTypeToModelType: Record<string, ModelType> = {
|
||||
}
|
||||
|
||||
// 将后端模型格式转换为旧的前端格式(附带 _modelType 便于渲染)
|
||||
// NOTE: apiKey is intentionally NEVER pre-filled from the server response.
|
||||
// The server returns '***' as a redacted placeholder when a key is stored,
|
||||
// and '' when it is not. We expose this as hasExistingApiKey so the editor
|
||||
// dialog can render a "Set / Not set" badge, while the apiKey field starts
|
||||
// blank — this preserves the invariant "non-empty formData.apiKey means the
|
||||
// user typed something" that the save path relies on to decide preserve /
|
||||
// replace / clear.
|
||||
function convertToLegacyFormat(model: ModelConfig) {
|
||||
return {
|
||||
id: model.id!,
|
||||
@@ -145,7 +152,8 @@ function convertToLegacyFormat(model: ModelConfig) {
|
||||
source: model.source,
|
||||
modelName: model.name,
|
||||
baseUrl: model.parameters.base_url || '',
|
||||
apiKey: model.parameters.api_key || '',
|
||||
apiKey: '',
|
||||
hasExistingApiKey: model.parameters.api_key === '***',
|
||||
provider: model.parameters.provider || '',
|
||||
dimension: model.parameters.embedding_parameters?.dimension,
|
||||
isBuiltin: model.is_builtin || false,
|
||||
@@ -283,6 +291,20 @@ const handleModelSave = async (modelData: any) => {
|
||||
}
|
||||
}
|
||||
|
||||
// 将前端格式转换为后端格式.
|
||||
// Three-state api_key semantics (write-only secrets pattern):
|
||||
// - clearApiKey set → send { clear_api_key: true }
|
||||
// - user typed a value → send { api_key: "..." }
|
||||
// - empty (default on edit) → omit api_key → server preserves
|
||||
// - empty on create → omit api_key → no stored secret
|
||||
const trimmedApiKey = (modelData.apiKey ?? '').trim()
|
||||
const apiKeyFields: { api_key?: string; clear_api_key?: boolean } =
|
||||
modelData.clearApiKey
|
||||
? { clear_api_key: true }
|
||||
: trimmedApiKey
|
||||
? { api_key: trimmedApiKey }
|
||||
: {}
|
||||
|
||||
const apiModelData: ModelConfig = {
|
||||
name: modelData.modelName.trim(),
|
||||
type: getModelType(currentModelType.value),
|
||||
@@ -290,7 +312,7 @@ const handleModelSave = async (modelData: any) => {
|
||||
description: '',
|
||||
parameters: {
|
||||
base_url: modelData.baseUrl?.trim() || '',
|
||||
api_key: modelData.apiKey?.trim() || '',
|
||||
...apiKeyFields,
|
||||
provider: modelData.provider || '',
|
||||
...(Object.keys(customHeadersMap).length > 0 ? { custom_headers: customHeadersMap } : {}),
|
||||
...(currentModelType.value === 'embedding' && modelData.dimension ? {
|
||||
|
||||
@@ -98,13 +98,22 @@
|
||||
:placeholder="t('webSearchSettings.baseUrlPlaceholder')"
|
||||
/>
|
||||
</t-form-item>
|
||||
<t-form-item v-if="selectedProviderType?.requires_api_key" :label="t('webSearchSettings.apiKeyLabel')" name="parameters.api_key">
|
||||
<!--
|
||||
API key is mandatory for every requires_api_key=true provider
|
||||
(Bing / Google / Tavily / Ollama / Baidu — see
|
||||
validateProviderParameters in service/web_search_provider.go),
|
||||
so the "Remove this credential" affordance shown in the other
|
||||
three dialogs is intentionally omitted here. Revoking access
|
||||
means deleting the whole provider row from the list.
|
||||
-->
|
||||
<div v-if="selectedProviderType?.requires_api_key" class="credential-field">
|
||||
<label class="credential-label">{{ t('webSearchSettings.apiKeyLabel') }}</label>
|
||||
<t-input
|
||||
v-model="providerForm.parameters.api_key"
|
||||
type="password"
|
||||
:placeholder="editingProvider ? t('webSearchSettings.apiKeyUnchanged') : t('webSearchSettings.apiKeyPlaceholder')"
|
||||
:placeholder="apiKeyPlaceholder"
|
||||
/>
|
||||
</t-form-item>
|
||||
</div>
|
||||
<t-form-item v-if="selectedProviderType?.requires_engine_id" :label="t('webSearchSettings.engineIdLabel')" name="parameters.engine_id">
|
||||
<t-input v-model="providerForm.parameters.engine_id" :placeholder="t('webSearchSettings.engineIdLabel')" />
|
||||
</t-form-item>
|
||||
@@ -179,6 +188,10 @@ const testingId = ref<string | null>(null)
|
||||
const saving = ref(false)
|
||||
const formRef = ref<any>()
|
||||
|
||||
// Fixed placeholder returned by the server for redacted secrets. Must match
|
||||
// internal/types/secret.go → RedactedSecretPlaceholder.
|
||||
const REDACTED_PLACEHOLDER = '***'
|
||||
|
||||
const providerForm = ref<{
|
||||
name: string
|
||||
provider: string
|
||||
@@ -198,6 +211,23 @@ const selectedProviderType = computed(() => {
|
||||
return providerTypes.value.find(pt => pt.id === providerForm.value.provider)
|
||||
})
|
||||
|
||||
// "Is an API key currently stored?" is signaled by the server returning the
|
||||
// fixed REDACTED_PLACEHOLDER in the response. Drive the label badge from this.
|
||||
const hasExistingApiKey = computed(() => {
|
||||
return editingProvider.value?.parameters?.api_key === REDACTED_PLACEHOLDER
|
||||
})
|
||||
|
||||
// In edit mode with a stored key, swap the placeholder to the shared
|
||||
// "bullets + Enter new value to replace" hint. Otherwise fall back to the
|
||||
// provider-specific placeholder (creation) or the "leave blank to keep"
|
||||
// copy used when no key is stored yet.
|
||||
const apiKeyPlaceholder = computed(() => {
|
||||
if (!editingProvider.value) return t('webSearchSettings.apiKeyPlaceholder')
|
||||
return hasExistingApiKey.value
|
||||
? t('secret.storedPlaceholder')
|
||||
: t('webSearchSettings.apiKeyUnchanged')
|
||||
})
|
||||
|
||||
const isProviderFree = (providerType: WebSearchProviderTypeInfo) => {
|
||||
// "Free" here means no upstream-paid credentials are required. Self-hosted
|
||||
// providers (requires_base_url) are still free to use even though they need
|
||||
@@ -253,6 +283,8 @@ const editProvider = (entity: WebSearchProviderEntity) => {
|
||||
provider: entity.provider,
|
||||
description: entity.description || '',
|
||||
parameters: {
|
||||
// Never pre-fill the api_key — even the redacted placeholder from the
|
||||
// server is ignored so that "non-empty means user typed it" holds.
|
||||
api_key: '',
|
||||
engine_id: entity.parameters?.engine_id || '',
|
||||
base_url: entity.parameters?.base_url || '',
|
||||
@@ -273,18 +305,29 @@ const saveProvider = async () => {
|
||||
|
||||
saving.value = true
|
||||
try {
|
||||
// Build the parameters payload using two-state semantics:
|
||||
// - user typed a value → send api_key (replaces stored)
|
||||
// - empty + editing → omit api_key (server preserves stored value)
|
||||
// - empty + creating → omit api_key (no secret to store yet)
|
||||
// No clear path: every requires_api_key provider mandates a value, so
|
||||
// "revoke" means deleting the provider row, not zeroing the key.
|
||||
const paramsOut: WebSearchProviderEntity['parameters'] = {
|
||||
engine_id: providerForm.value.parameters.engine_id,
|
||||
base_url: providerForm.value.parameters.base_url,
|
||||
proxy_url: providerForm.value.parameters.proxy_url,
|
||||
}
|
||||
if (providerForm.value.parameters.api_key) {
|
||||
paramsOut.api_key = providerForm.value.parameters.api_key
|
||||
}
|
||||
|
||||
const data: Partial<WebSearchProviderEntity> = {
|
||||
name: providerForm.value.name.trim() || selectedProviderType.value?.name || providerForm.value.provider,
|
||||
provider: providerForm.value.provider as any,
|
||||
description: providerForm.value.description,
|
||||
parameters: { ...providerForm.value.parameters },
|
||||
parameters: paramsOut,
|
||||
is_default: providerForm.value.is_default,
|
||||
}
|
||||
|
||||
if (editingProvider.value && !data.parameters!.api_key) {
|
||||
delete data.parameters!.api_key
|
||||
}
|
||||
|
||||
if (editingProvider.value) {
|
||||
await updateWebSearchProvider(editingProvider.value.id!, data)
|
||||
MessagePlugin.success(t('webSearchSettings.toasts.providerUpdated'))
|
||||
@@ -471,6 +514,31 @@ onMounted(async () => {
|
||||
margin: 20px 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Credential field: stacks the label row, password input, and the optional
|
||||
* "Remove this credential" checkbox vertically. Matches the pattern in
|
||||
* McpServiceDialog and ModelEditorDialog so the whole UI reads consistently.
|
||||
*/
|
||||
.credential-field {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 8px;
|
||||
margin-bottom: 20px;
|
||||
}
|
||||
|
||||
.credential-label {
|
||||
display: block;
|
||||
font-size: 14px;
|
||||
color: var(--td-text-color-primary);
|
||||
}
|
||||
|
||||
.clear-credential {
|
||||
:deep(.t-checkbox__label) {
|
||||
color: var(--td-error-color);
|
||||
font-size: 13px;
|
||||
}
|
||||
}
|
||||
|
||||
.credentials-hint {
|
||||
margin-bottom: 12px;
|
||||
font-size: 13px;
|
||||
|
||||
@@ -34,8 +34,8 @@
|
||||
</t-form-item>
|
||||
|
||||
<!-- URL for SSE/HTTP Streamable -->
|
||||
<t-form-item
|
||||
:label="t('mcpServiceDialog.serviceUrl')"
|
||||
<t-form-item
|
||||
:label="t('mcpServiceDialog.serviceUrl')"
|
||||
name="url"
|
||||
>
|
||||
<t-input v-model="formData.url" :placeholder="t('mcpServiceDialog.serviceUrlPlaceholder')" />
|
||||
@@ -50,20 +50,47 @@
|
||||
<!-- Authentication Config -->
|
||||
<t-collapse :default-value="[]">
|
||||
<t-collapse-panel :header="t('mcpServiceDialog.authConfig')" value="auth">
|
||||
<t-form-item :label="t('mcpServiceDialog.apiKey')">
|
||||
<!--
|
||||
Credential fields follow the "write-only secrets" pattern.
|
||||
When a value is currently stored on the server the placeholder
|
||||
is swapped for a bullet run + "Enter new value to replace"
|
||||
hint, so the visual "something is there" signal is carried by
|
||||
the input itself rather than by a separate Set/Not-set badge.
|
||||
The Remove checkbox still offers an explicit clear path.
|
||||
-->
|
||||
<div class="credential-field">
|
||||
<label class="credential-label">{{ t('mcpServiceDialog.apiKey') }}</label>
|
||||
<t-input
|
||||
v-model="formData.auth_config.api_key"
|
||||
type="password"
|
||||
:placeholder="t('mcpServiceDialog.optional')"
|
||||
:disabled="clearApiKey"
|
||||
:placeholder="apiKeyPlaceholder"
|
||||
/>
|
||||
</t-form-item>
|
||||
<t-form-item :label="t('mcpServiceDialog.bearerToken')">
|
||||
<t-checkbox
|
||||
v-if="mode === 'edit' && hasExistingApiKey"
|
||||
v-model="clearApiKey"
|
||||
class="clear-credential"
|
||||
>
|
||||
{{ t('secret.clearHint') }}
|
||||
</t-checkbox>
|
||||
</div>
|
||||
|
||||
<div class="credential-field">
|
||||
<label class="credential-label">{{ t('mcpServiceDialog.bearerToken') }}</label>
|
||||
<t-input
|
||||
v-model="formData.auth_config.token"
|
||||
type="password"
|
||||
:placeholder="t('mcpServiceDialog.optional')"
|
||||
:disabled="clearToken"
|
||||
:placeholder="tokenPlaceholder"
|
||||
/>
|
||||
</t-form-item>
|
||||
<t-checkbox
|
||||
v-if="mode === 'edit' && hasExistingToken"
|
||||
v-model="clearToken"
|
||||
class="clear-credential"
|
||||
>
|
||||
{{ t('secret.clearHint') }}
|
||||
</t-checkbox>
|
||||
</div>
|
||||
</t-collapse-panel>
|
||||
|
||||
<!-- Advanced Config -->
|
||||
@@ -100,7 +127,7 @@
|
||||
|
||||
<script setup lang="ts">
|
||||
import { ref, watch, computed } from 'vue'
|
||||
import { MessagePlugin } from 'tdesign-vue-next'
|
||||
import { MessagePlugin, DialogPlugin } from 'tdesign-vue-next'
|
||||
import type { FormInstanceFunctions, FormRule } from 'tdesign-vue-next'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
import {
|
||||
@@ -128,6 +155,10 @@ const formRef = ref<FormInstanceFunctions>()
|
||||
const submitting = ref(false)
|
||||
const { t } = useI18n()
|
||||
|
||||
// Fixed placeholder returned by the server for sensitive fields whose values
|
||||
// are withheld. Must match internal/types/secret.go → RedactedSecretPlaceholder.
|
||||
const REDACTED_PLACEHOLDER = '***'
|
||||
|
||||
const formData = ref({
|
||||
name: '',
|
||||
description: '',
|
||||
@@ -145,11 +176,40 @@ const formData = ref({
|
||||
}
|
||||
})
|
||||
|
||||
// Explicit "remove stored credential" flags. Reset to false on every open/reset.
|
||||
const clearApiKey = ref(false)
|
||||
const clearToken = ref(false)
|
||||
|
||||
// "Is a credential currently stored?" is signaled by the server returning the
|
||||
// fixed REDACTED_PLACEHOLDER instead of an empty string. Drive the label
|
||||
// badge and placeholder hint from this signal.
|
||||
const hasExistingApiKey = computed(
|
||||
() => props.service?.auth_config?.api_key === REDACTED_PLACEHOLDER
|
||||
)
|
||||
const hasExistingToken = computed(
|
||||
() => props.service?.auth_config?.token === REDACTED_PLACEHOLDER
|
||||
)
|
||||
|
||||
// Placeholders swap to the shared "stored credential" hint when the server
|
||||
// signals a value is present (parameters come back as '***'). Otherwise
|
||||
// the field is genuinely empty and we fall through to the normal
|
||||
// "Optional" placeholder that appears in add mode too.
|
||||
const apiKeyPlaceholder = computed(() =>
|
||||
hasExistingApiKey.value
|
||||
? t('secret.storedPlaceholder')
|
||||
: t('mcpServiceDialog.optional')
|
||||
)
|
||||
const tokenPlaceholder = computed(() =>
|
||||
hasExistingToken.value
|
||||
? t('secret.storedPlaceholder')
|
||||
: t('mcpServiceDialog.optional')
|
||||
)
|
||||
|
||||
const rules: Record<string, FormRule[]> = {
|
||||
name: [{ required: true, message: t('mcpServiceDialog.rules.nameRequired') as string, type: 'error' }],
|
||||
transport_type: [{ required: true, message: t('mcpServiceDialog.rules.transportRequired') as string, type: 'error' }],
|
||||
url: [
|
||||
{
|
||||
{
|
||||
validator: (val: string) => {
|
||||
if (!val || val.trim() === '') {
|
||||
return { result: false, message: t('mcpServiceDialog.rules.urlRequired') as string, type: 'error' }
|
||||
@@ -171,7 +231,10 @@ const dialogVisible = computed({
|
||||
set: (value) => emit('update:visible', value)
|
||||
})
|
||||
|
||||
// Reset form function - defined before watch to avoid hoisting issues
|
||||
// Reset form function — defined before watch to avoid hoisting issues.
|
||||
// Sensitive fields are always blank: the watch below does not copy the
|
||||
// server-provided redacted placeholder into formData, and clear flags are
|
||||
// reset so a reopen does not carry over an aborted deletion intent.
|
||||
const resetForm = () => {
|
||||
formData.value = {
|
||||
name: '',
|
||||
@@ -189,10 +252,16 @@ const resetForm = () => {
|
||||
retry_delay: 1
|
||||
}
|
||||
}
|
||||
clearApiKey.value = false
|
||||
clearToken.value = false
|
||||
formRef.value?.clearValidate()
|
||||
}
|
||||
|
||||
// Watch service prop to initialize form
|
||||
// Watch service prop to initialize form.
|
||||
// Sensitive fields (api_key, token) are NEVER pre-filled from the server
|
||||
// response — even when the server returns '***' indicating a stored value.
|
||||
// This keeps the "non-empty means user typed it" invariant that the submit
|
||||
// logic relies on to decide between preserve and replace.
|
||||
watch(
|
||||
() => props.service,
|
||||
(service) => {
|
||||
@@ -206,8 +275,8 @@ watch(
|
||||
transport_type: transportType as 'sse' | 'http-streamable',
|
||||
url: service.url || '',
|
||||
auth_config: {
|
||||
api_key: service.auth_config?.api_key || '',
|
||||
token: service.auth_config?.token || ''
|
||||
api_key: '',
|
||||
token: ''
|
||||
},
|
||||
advanced_config: {
|
||||
timeout: service.advanced_config?.timeout || 30,
|
||||
@@ -215,6 +284,8 @@ watch(
|
||||
retry_delay: service.advanced_config?.retry_delay || 1
|
||||
}
|
||||
}
|
||||
clearApiKey.value = false
|
||||
clearToken.value = false
|
||||
} else {
|
||||
resetForm()
|
||||
}
|
||||
@@ -222,24 +293,67 @@ watch(
|
||||
{ immediate: true }
|
||||
)
|
||||
|
||||
// Handle submit
|
||||
// Prompt the user before irrevocable credential removal.
|
||||
const confirmClearIfNeeded = (): Promise<boolean> => {
|
||||
if (!clearApiKey.value && !clearToken.value) return Promise.resolve(true)
|
||||
return new Promise((resolve) => {
|
||||
const d = DialogPlugin.confirm({
|
||||
header: t('secret.confirmClearTitle'),
|
||||
body: t('secret.confirmClearBody'),
|
||||
confirmBtn: { content: t('common.confirm'), theme: 'danger' },
|
||||
cancelBtn: t('common.cancel'),
|
||||
onConfirm: () => {
|
||||
d.hide()
|
||||
resolve(true)
|
||||
},
|
||||
onCancel: () => {
|
||||
d.hide()
|
||||
resolve(false)
|
||||
},
|
||||
onClose: () => {
|
||||
d.hide()
|
||||
resolve(false)
|
||||
}
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
// Handle submit — three-state auth_config payload:
|
||||
// - clear flag set → send { clear_api_key: true } / { clear_token: true }
|
||||
// - user typed a value → send the value
|
||||
// - otherwise → omit the field, server preserves the stored value
|
||||
// If auth_config would be empty, omit the key entirely.
|
||||
const handleSubmit = async () => {
|
||||
const valid = await formRef.value?.validate()
|
||||
if (!valid) return
|
||||
|
||||
const proceed = await confirmClearIfNeeded()
|
||||
if (!proceed) return
|
||||
|
||||
submitting.value = true
|
||||
try {
|
||||
const authPayload: Record<string, unknown> = {}
|
||||
if (clearApiKey.value) {
|
||||
authPayload.clear_api_key = true
|
||||
} else if (formData.value.auth_config.api_key) {
|
||||
authPayload.api_key = formData.value.auth_config.api_key
|
||||
}
|
||||
if (clearToken.value) {
|
||||
authPayload.clear_token = true
|
||||
} else if (formData.value.auth_config.token) {
|
||||
authPayload.token = formData.value.auth_config.token
|
||||
}
|
||||
|
||||
const data: Partial<MCPService> = {
|
||||
name: formData.value.name,
|
||||
description: formData.value.description,
|
||||
enabled: formData.value.enabled,
|
||||
transport_type: formData.value.transport_type,
|
||||
auth_config: {
|
||||
api_key: formData.value.auth_config.api_key || undefined,
|
||||
token: formData.value.auth_config.token || undefined
|
||||
},
|
||||
advanced_config: formData.value.advanced_config,
|
||||
url: formData.value.url || undefined
|
||||
url: formData.value.url || undefined,
|
||||
}
|
||||
if (Object.keys(authPayload).length > 0) {
|
||||
data.auth_config = authPayload as MCPService['auth_config']
|
||||
}
|
||||
|
||||
if (props.mode === 'add') {
|
||||
@@ -269,5 +383,37 @@ const handleClose = () => {
|
||||
|
||||
<style scoped lang="less">
|
||||
/* Stdio-related styles removed as stdio transport is disabled for security reasons */
|
||||
|
||||
/**
|
||||
* Credential field: stacks the label row, the password input, and the
|
||||
* optional "Remove this credential" checkbox vertically. Replaces the
|
||||
* t-form-item-based layout for the auth-config fields so the inline badge
|
||||
* doesn't get clipped by the form's 120px label-width and the clear
|
||||
* checkbox doesn't sit alongside the input.
|
||||
*/
|
||||
.credential-field {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 8px;
|
||||
margin-bottom: 20px;
|
||||
|
||||
&:last-child {
|
||||
margin-bottom: 0;
|
||||
}
|
||||
}
|
||||
|
||||
.credential-label {
|
||||
display: block;
|
||||
font-size: 14px;
|
||||
color: var(--td-text-color-primary);
|
||||
line-height: 22px;
|
||||
}
|
||||
|
||||
.clear-credential {
|
||||
:deep(.t-checkbox__label) {
|
||||
color: var(--td-error-color);
|
||||
font-size: 13px;
|
||||
}
|
||||
}
|
||||
</style>
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"fmt"
|
||||
"mime/multipart"
|
||||
"net/textproto"
|
||||
"reflect"
|
||||
"time"
|
||||
|
||||
"github.com/Tencent/WeKnora/internal/datasource"
|
||||
@@ -15,6 +16,7 @@ import (
|
||||
"github.com/Tencent/WeKnora/internal/tracing/langfuse"
|
||||
"github.com/Tencent/WeKnora/internal/types"
|
||||
"github.com/Tencent/WeKnora/internal/types/interfaces"
|
||||
secutils "github.com/Tencent/WeKnora/internal/utils"
|
||||
"github.com/hibiken/asynq"
|
||||
)
|
||||
|
||||
@@ -153,8 +155,55 @@ func (s *DataSourceService) UpdateDataSource(ctx context.Context, ds *types.Data
|
||||
return nil, datasource.ErrDataSourceInvalid
|
||||
}
|
||||
|
||||
// Validate new configuration if changed
|
||||
if ds.Type != existing.Type || string(ds.Config) != string(existing.Config) {
|
||||
// Apply write-only secret merge semantics to the Config jsonb:
|
||||
// empty / redacted credential strings preserve existing values,
|
||||
// ClearCredentials wipes the whole map, other values replace. This
|
||||
// blocks the NELO-1299-style round-trip bug where a frontend that
|
||||
// echoed back the redacted placeholder would overwrite real secrets.
|
||||
//
|
||||
// We also remember the parsed configs for the post-merge "changed?"
|
||||
// comparison — a raw-bytes compare against ds.Config is unreliable
|
||||
// because Go map iteration order makes the re-serialized JSON's key
|
||||
// ordering nondeterministic, causing string(merged) != string(existing)
|
||||
// to spuriously trip every time.
|
||||
var mergedCfg, existingParsedCfg *types.DataSourceConfig
|
||||
if len(ds.Config) > 0 {
|
||||
incomingCfg, parseIncErr := ds.ParseConfig()
|
||||
existingCfg, parseExErr := existing.ParseConfig()
|
||||
if parseIncErr == nil && parseExErr == nil && incomingCfg != nil {
|
||||
baseExisting := types.DataSourceConfig{}
|
||||
if existingCfg != nil {
|
||||
baseExisting = *existingCfg
|
||||
}
|
||||
if incomingCfg.ClearCredentials {
|
||||
logger.Infof(ctx, "DataSource credentials cleared by user: id=%s",
|
||||
secutils.SanitizeForLog(ds.ID))
|
||||
}
|
||||
merged := incomingCfg.MergeUpdate(baseExisting)
|
||||
if blob, err := merged.ToJSON(); err == nil {
|
||||
ds.Config = blob
|
||||
}
|
||||
mergedCfg = &merged
|
||||
existingParsedCfg = existingCfg
|
||||
}
|
||||
}
|
||||
|
||||
// Validate new configuration if changed. Compare parsed structures to
|
||||
// avoid re-running (potentially expensive) OAuth / live-connection
|
||||
// validators on every update that only re-orders JSON keys.
|
||||
//
|
||||
// Skip validation entirely when the user is explicitly revoking
|
||||
// credentials (ClearCredentials=true). Live validators (e.g. Notion's
|
||||
// /me API) require a working secret by definition; trying to validate
|
||||
// after a wipe would always fail and leave the row un-cleared, defeating
|
||||
// the whole point of the "Remove all credentials" affordance.
|
||||
configActuallyChanged := true
|
||||
if mergedCfg != nil && existingParsedCfg != nil {
|
||||
configActuallyChanged = !reflect.DeepEqual(*mergedCfg, *existingParsedCfg)
|
||||
}
|
||||
clearingCredentials := mergedCfg != nil && !mergedCfg.HasCredentials() &&
|
||||
existingParsedCfg != nil && existingParsedCfg.HasCredentials()
|
||||
if !clearingCredentials && (ds.Type != existing.Type || configActuallyChanged) {
|
||||
if err := s.validateDataSourceConfig(ctx, ds); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -3,6 +3,8 @@ package service
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"maps"
|
||||
"slices"
|
||||
"time"
|
||||
|
||||
"github.com/Tencent/WeKnora/internal/logger"
|
||||
@@ -53,7 +55,12 @@ func (s *mcpServiceService) CreateMCPService(ctx context.Context, service *types
|
||||
return nil
|
||||
}
|
||||
|
||||
// GetMCPServiceByID retrieves an MCP service by ID
|
||||
// GetMCPServiceByID retrieves an MCP service by ID.
|
||||
//
|
||||
// Sensitive fields are redacted before the service is returned so that the
|
||||
// single-resource GET behaves consistently with the list endpoint. Builtin
|
||||
// services still route through HideSensitiveInfo which clears additional
|
||||
// fields (URL, Headers, EnvVars, StdioConfig) on top of the redaction.
|
||||
func (s *mcpServiceService) GetMCPServiceByID(
|
||||
ctx context.Context,
|
||||
tenantID uint64,
|
||||
@@ -69,6 +76,10 @@ func (s *mcpServiceService) GetMCPServiceByID(
|
||||
return nil, fmt.Errorf("MCP service not found")
|
||||
}
|
||||
|
||||
if service.IsBuiltin {
|
||||
return service.HideSensitiveInfo(), nil
|
||||
}
|
||||
service.RedactSensitiveData()
|
||||
return service, nil
|
||||
}
|
||||
|
||||
@@ -80,12 +91,14 @@ func (s *mcpServiceService) ListMCPServices(ctx context.Context, tenantID uint64
|
||||
return nil, fmt.Errorf("failed to list MCP services: %w", err)
|
||||
}
|
||||
|
||||
// Mask sensitive data for list view
|
||||
// Redact sensitive data before returning so secrets never leave the server.
|
||||
// Builtin services go through HideSensitiveInfo which clears additional
|
||||
// fields (URL, headers, env_vars, stdio_config) beyond redaction.
|
||||
for i, service := range services {
|
||||
if service.IsBuiltin {
|
||||
services[i] = service.HideSensitiveInfo()
|
||||
} else {
|
||||
service.MaskSensitiveData()
|
||||
service.RedactSensitiveData()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -141,6 +154,57 @@ func (s *mcpServiceService) UpdateMCPService(ctx context.Context, service *types
|
||||
// Store old enabled state BEFORE any updates
|
||||
oldEnabled := existing.Enabled
|
||||
|
||||
// Snapshot pre-merge values of fields that drive configChanged. We need
|
||||
// this because the in-place merge below reassigns pointer fields such as
|
||||
// existing.URL = service.URL, after which any post-merge comparison
|
||||
// between service.URL and existing.URL would trivially match.
|
||||
preURL := ""
|
||||
preURLSet := existing.URL != nil
|
||||
if preURLSet {
|
||||
preURL = *existing.URL
|
||||
}
|
||||
var preStdioCommand string
|
||||
var preStdioArgs []string
|
||||
preStdioSet := existing.StdioConfig != nil
|
||||
if preStdioSet {
|
||||
preStdioCommand = existing.StdioConfig.Command
|
||||
preStdioArgs = append([]string(nil), existing.StdioConfig.Args...)
|
||||
}
|
||||
preTransportType := existing.TransportType
|
||||
preAuthSet := existing.AuthConfig != nil
|
||||
var preAuthAPIKey, preAuthToken string
|
||||
var preAuthHeaders map[string]string
|
||||
if preAuthSet {
|
||||
preAuthAPIKey = existing.AuthConfig.APIKey
|
||||
preAuthToken = existing.AuthConfig.Token
|
||||
// Copy the map so subsequent mutations via merged.CustomHeaders don't
|
||||
// alias into the snapshot.
|
||||
if existing.AuthConfig.CustomHeaders != nil {
|
||||
preAuthHeaders = make(map[string]string, len(existing.AuthConfig.CustomHeaders))
|
||||
maps.Copy(preAuthHeaders, existing.AuthConfig.CustomHeaders)
|
||||
}
|
||||
}
|
||||
|
||||
// AuthConfig merge is applied unconditionally (both partial and full
|
||||
// updates), so a request body like {"clear_token": true} by itself is
|
||||
// honored instead of being silently dropped when Name is empty.
|
||||
// Audit-log explicit clear operations before the merge absorbs the flag.
|
||||
if service.AuthConfig != nil {
|
||||
if service.AuthConfig.ClearAPIKey {
|
||||
logger.GetLogger(ctx).Infof(
|
||||
"MCP auth cleared by user: id=%s field=api_key",
|
||||
secutils.SanitizeForLog(service.ID),
|
||||
)
|
||||
}
|
||||
if service.AuthConfig.ClearToken {
|
||||
logger.GetLogger(ctx).Infof(
|
||||
"MCP auth cleared by user: id=%s field=token",
|
||||
secutils.SanitizeForLog(service.ID),
|
||||
)
|
||||
}
|
||||
existing.AuthConfig = service.AuthConfig.MergeUpdate(existing.AuthConfig)
|
||||
}
|
||||
|
||||
// Merge updates: only update fields that are provided (non-zero or explicitly set)
|
||||
// This ensures that false values for enabled field are properly updated
|
||||
// Handler ensures that service.Enabled is only set if "enabled" key exists in the request
|
||||
@@ -148,7 +212,7 @@ func (s *mcpServiceService) UpdateMCPService(ctx context.Context, service *types
|
||||
// or if we're updating other fields (indicating full update)
|
||||
// For enabled field, we'll update it if this is a partial update (only enabled) or if it's explicitly set
|
||||
if service.Name == "" {
|
||||
// Partial update - only update enabled field
|
||||
// Partial update - only update enabled field (AuthConfig already merged above).
|
||||
existing.Enabled = service.Enabled
|
||||
} else {
|
||||
// Full update - update all fields including enabled
|
||||
@@ -172,9 +236,6 @@ func (s *mcpServiceService) UpdateMCPService(ctx context.Context, service *types
|
||||
if service.Headers != nil {
|
||||
existing.Headers = service.Headers
|
||||
}
|
||||
if service.AuthConfig != nil {
|
||||
existing.AuthConfig = service.AuthConfig
|
||||
}
|
||||
if service.AdvancedConfig != nil {
|
||||
existing.AdvancedConfig = service.AdvancedConfig
|
||||
}
|
||||
@@ -188,25 +249,39 @@ func (s *mcpServiceService) UpdateMCPService(ctx context.Context, service *types
|
||||
return fmt.Errorf("failed to update MCP service: %w", err)
|
||||
}
|
||||
|
||||
// Check if critical configuration changed (URL/StdioConfig, transport type, or auth config)
|
||||
// Check if critical configuration changed (URL / StdioConfig / transport
|
||||
// type / auth config). Comparisons MUST be against the pre-merge
|
||||
// snapshots captured above — after the in-place merge, service.URL and
|
||||
// existing.URL point to the same memory, making any post-merge compare
|
||||
// vacuously equal. Saving the dialog without touching these fields must
|
||||
// not recycle the live MCP client connection (that's the NELO-1299
|
||||
// regression we're preventing).
|
||||
configChanged := false
|
||||
if service.URL != nil && existing.URL != nil && *service.URL != *existing.URL {
|
||||
currURLSet := existing.URL != nil
|
||||
switch {
|
||||
case currURLSet != preURLSet:
|
||||
configChanged = true
|
||||
} else if (service.URL != nil) != (existing.URL != nil) {
|
||||
case currURLSet && *existing.URL != preURL:
|
||||
configChanged = true
|
||||
}
|
||||
if service.StdioConfig != nil && existing.StdioConfig != nil {
|
||||
if service.StdioConfig.Command != existing.StdioConfig.Command ||
|
||||
!equalStringSlices(service.StdioConfig.Args, existing.StdioConfig.Args) {
|
||||
configChanged = true
|
||||
}
|
||||
} else if (service.StdioConfig != nil) != (existing.StdioConfig != nil) {
|
||||
currStdioSet := existing.StdioConfig != nil
|
||||
switch {
|
||||
case currStdioSet != preStdioSet:
|
||||
configChanged = true
|
||||
case currStdioSet && (existing.StdioConfig.Command != preStdioCommand ||
|
||||
!slices.Equal(existing.StdioConfig.Args, preStdioArgs)):
|
||||
configChanged = true
|
||||
}
|
||||
if service.TransportType != "" && service.TransportType != existing.TransportType {
|
||||
if existing.TransportType != preTransportType {
|
||||
configChanged = true
|
||||
}
|
||||
if service.AuthConfig != nil {
|
||||
currAuthSet := existing.AuthConfig != nil
|
||||
switch {
|
||||
case currAuthSet != preAuthSet:
|
||||
configChanged = true
|
||||
case currAuthSet && (existing.AuthConfig.APIKey != preAuthAPIKey ||
|
||||
existing.AuthConfig.Token != preAuthToken ||
|
||||
!maps.Equal(existing.AuthConfig.CustomHeaders, preAuthHeaders)):
|
||||
configChanged = true
|
||||
}
|
||||
name := secutils.SanitizeForLog(existing.Name)
|
||||
@@ -393,15 +468,3 @@ func (s *mcpServiceService) GetMCPServiceResources(
|
||||
return resources, nil
|
||||
}
|
||||
|
||||
// equalStringSlices compares two string slices for equality
|
||||
func equalStringSlices(a, b []string) bool {
|
||||
if len(a) != len(b) {
|
||||
return false
|
||||
}
|
||||
for i := range a {
|
||||
if a[i] != b[i] {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -0,0 +1,395 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/Tencent/WeKnora/internal/mcp"
|
||||
"github.com/Tencent/WeKnora/internal/types"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// fakeMCPRepo is a minimal in-memory implementation of
|
||||
// interfaces.MCPServiceRepository for testing the service-layer merge logic
|
||||
// without depending on the database.
|
||||
type fakeMCPRepo struct {
|
||||
store map[string]*types.MCPService
|
||||
}
|
||||
|
||||
func newFakeMCPRepo() *fakeMCPRepo {
|
||||
return &fakeMCPRepo{store: make(map[string]*types.MCPService)}
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) Create(_ context.Context, s *types.MCPService) error {
|
||||
r.store[s.ID] = s
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) GetByID(_ context.Context, _ uint64, id string) (*types.MCPService, error) {
|
||||
s, ok := r.store[id]
|
||||
if !ok {
|
||||
return nil, nil
|
||||
}
|
||||
// return a deep-ish copy so service-layer mutations to the returned value
|
||||
// do not leak back into the fake store, mirroring how GORM produces a
|
||||
// fresh struct per query
|
||||
cp := *s
|
||||
if s.AuthConfig != nil {
|
||||
ac := *s.AuthConfig
|
||||
cp.AuthConfig = &ac
|
||||
}
|
||||
if s.AdvancedConfig != nil {
|
||||
adv := *s.AdvancedConfig
|
||||
cp.AdvancedConfig = &adv
|
||||
}
|
||||
return &cp, nil
|
||||
}
|
||||
|
||||
func cloneService(s *types.MCPService) *types.MCPService {
|
||||
cp := *s
|
||||
if s.AuthConfig != nil {
|
||||
ac := *s.AuthConfig
|
||||
cp.AuthConfig = &ac
|
||||
}
|
||||
if s.AdvancedConfig != nil {
|
||||
adv := *s.AdvancedConfig
|
||||
cp.AdvancedConfig = &adv
|
||||
}
|
||||
return &cp
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) List(_ context.Context, _ uint64) ([]*types.MCPService, error) {
|
||||
out := make([]*types.MCPService, 0, len(r.store))
|
||||
for _, s := range r.store {
|
||||
out = append(out, cloneService(s))
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) ListEnabled(ctx context.Context, tenantID uint64) ([]*types.MCPService, error) {
|
||||
return r.List(ctx, tenantID)
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) ListByIDs(_ context.Context, _ uint64, ids []string) ([]*types.MCPService, error) {
|
||||
out := make([]*types.MCPService, 0, len(ids))
|
||||
for _, id := range ids {
|
||||
if s, ok := r.store[id]; ok {
|
||||
out = append(out, cloneService(s))
|
||||
}
|
||||
}
|
||||
return out, nil
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) Update(_ context.Context, s *types.MCPService) error {
|
||||
r.store[s.ID] = s
|
||||
return nil
|
||||
}
|
||||
|
||||
func (r *fakeMCPRepo) Delete(_ context.Context, _ uint64, id string) error {
|
||||
delete(r.store, id)
|
||||
return nil
|
||||
}
|
||||
|
||||
// seedService plants a service with the given auth credentials into the fake
|
||||
// repo and returns the service ID. Convenience helper used by every test.
|
||||
func seedService(t *testing.T, repo *fakeMCPRepo, apiKey, token string) string {
|
||||
t.Helper()
|
||||
s := &types.MCPService{
|
||||
ID: "svc-test",
|
||||
TenantID: 1,
|
||||
Name: "test",
|
||||
Enabled: true,
|
||||
TransportType: types.MCPTransportSSE,
|
||||
AuthConfig: &types.MCPAuthConfig{
|
||||
APIKey: apiKey,
|
||||
Token: token,
|
||||
},
|
||||
}
|
||||
require.NoError(t, repo.Create(context.Background(), s))
|
||||
return s.ID
|
||||
}
|
||||
|
||||
// newTestService wires up a mcpServiceService with a fresh fake repo and a
|
||||
// real (empty) MCPManager. CloseClient on an empty manager is a no-op, so
|
||||
// tests can safely exercise the configChanged code paths.
|
||||
func newTestService() (*mcpServiceService, *fakeMCPRepo) {
|
||||
repo := newFakeMCPRepo()
|
||||
svc := &mcpServiceService{
|
||||
mcpServiceRepo: repo,
|
||||
mcpManager: mcp.NewMCPManager(),
|
||||
}
|
||||
return svc, repo
|
||||
}
|
||||
|
||||
// buildFullUpdate returns an UpdateMCPService input that exercises the
|
||||
// "full update" branch (service.Name is non-empty), which is the branch that
|
||||
// actually merges AuthConfig.
|
||||
func buildFullUpdate(id string, auth *types.MCPAuthConfig) *types.MCPService {
|
||||
return &types.MCPService{
|
||||
ID: id,
|
||||
TenantID: 1,
|
||||
Name: "test", // non-empty → full update branch
|
||||
Enabled: true,
|
||||
TransportType: types.MCPTransportSSE,
|
||||
AuthConfig: auth,
|
||||
}
|
||||
}
|
||||
|
||||
func TestUpdateMCPService_PreservesSecretsOnEmptyOrRedacted(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
t.Run("nil AuthConfig preserves existing", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, nil)
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey)
|
||||
assert.Equal(t, "stored-token", got.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("empty string preserves existing", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{APIKey: "", Token: ""})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey)
|
||||
assert.Equal(t, "stored-token", got.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("redacted placeholder preserves existing (NELO-1299 round-trip)", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
APIKey: types.RedactedSecretPlaceholder,
|
||||
Token: types.RedactedSecretPlaceholder,
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey)
|
||||
assert.Equal(t, "stored-token", got.AuthConfig.Token)
|
||||
})
|
||||
}
|
||||
|
||||
func TestUpdateMCPService_ReplacesOnExplicitValue(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
APIKey: "new-api",
|
||||
Token: "new-token",
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "new-api", got.AuthConfig.APIKey)
|
||||
assert.Equal(t, "new-token", got.AuthConfig.Token)
|
||||
}
|
||||
|
||||
func TestUpdateMCPService_MixedFields(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
// Replace token only, leave api_key as redacted placeholder → preserve
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
APIKey: types.RedactedSecretPlaceholder,
|
||||
Token: "new-token",
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey, "api_key should be preserved")
|
||||
assert.Equal(t, "new-token", got.AuthConfig.Token, "token should be replaced")
|
||||
}
|
||||
|
||||
func TestUpdateMCPService_ClearFlags(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
t.Run("ClearToken removes stored token, preserves api_key", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
APIKey: types.RedactedSecretPlaceholder,
|
||||
ClearToken: true,
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey)
|
||||
assert.Empty(t, got.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("ClearAPIKey removes stored api_key, preserves token", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
ClearAPIKey: true,
|
||||
Token: types.RedactedSecretPlaceholder,
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Empty(t, got.AuthConfig.APIKey)
|
||||
assert.Equal(t, "stored-token", got.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("ClearToken takes precedence over submitted value", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
Token: "will-be-ignored",
|
||||
ClearToken: true,
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Empty(t, got.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("clear flags are not persisted", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{ClearToken: true})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
require.NotNil(t, got.AuthConfig)
|
||||
assert.False(t, got.AuthConfig.ClearToken, "ClearToken must not be persisted")
|
||||
assert.False(t, got.AuthConfig.ClearAPIKey, "ClearAPIKey must not be persisted")
|
||||
})
|
||||
|
||||
t.Run("clear on already-empty field is a no-op", func(t *testing.T) {
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "") // token not set
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{ClearToken: true})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey)
|
||||
assert.Empty(t, got.AuthConfig.Token)
|
||||
})
|
||||
}
|
||||
|
||||
func TestListMCPServices_RedactsSecrets(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
seedService(t, repo, "real-api", "real-token")
|
||||
|
||||
got, err := svc.ListMCPServices(ctx, 1)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, got, 1)
|
||||
assert.Equal(t, types.RedactedSecretPlaceholder, got[0].AuthConfig.APIKey)
|
||||
assert.Equal(t, types.RedactedSecretPlaceholder, got[0].AuthConfig.Token)
|
||||
|
||||
// And the underlying store is untouched
|
||||
stored := repo.store["svc-test"]
|
||||
assert.Equal(t, "real-api", stored.AuthConfig.APIKey)
|
||||
assert.Equal(t, "real-token", stored.AuthConfig.Token)
|
||||
}
|
||||
|
||||
func TestGetMCPServiceByID_RedactsSecrets(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "real-api", "real-token")
|
||||
|
||||
got, err := svc.GetMCPServiceByID(ctx, 1, id)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, got.AuthConfig)
|
||||
assert.Equal(t, types.RedactedSecretPlaceholder, got.AuthConfig.APIKey)
|
||||
assert.Equal(t, types.RedactedSecretPlaceholder, got.AuthConfig.Token)
|
||||
|
||||
// underlying store unchanged
|
||||
stored := repo.store[id]
|
||||
assert.Equal(t, "real-api", stored.AuthConfig.APIKey)
|
||||
assert.Equal(t, "real-token", stored.AuthConfig.Token)
|
||||
}
|
||||
|
||||
func TestGetMCPServiceByID_EmptySecretsStayEmpty(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "", "") // both empty
|
||||
|
||||
got, err := svc.GetMCPServiceByID(ctx, 1, id)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, got.AuthConfig)
|
||||
assert.Empty(t, got.AuthConfig.APIKey, "empty secret must stay empty, not be redacted")
|
||||
assert.Empty(t, got.AuthConfig.Token)
|
||||
}
|
||||
|
||||
// Partial update: service.Name == "" is interpreted as "only touch enabled".
|
||||
// A clear flag sent in such a body was previously dropped; the refactor now
|
||||
// merges AuthConfig unconditionally so single-field clears round-trip.
|
||||
func TestUpdateMCPService_PartialUpdateHonorsClearFlag(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
|
||||
// Partial update body: no name, enabled flipped, ClearToken set.
|
||||
upd := &types.MCPService{
|
||||
ID: id,
|
||||
TenantID: 1,
|
||||
Enabled: true, // keep enabled, but simulate a "flag only" request
|
||||
AuthConfig: &types.MCPAuthConfig{
|
||||
ClearToken: true,
|
||||
},
|
||||
}
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "stored-api", got.AuthConfig.APIKey, "APIKey preserved in partial update")
|
||||
assert.Empty(t, got.AuthConfig.Token, "ClearToken must clear Token even in partial update")
|
||||
}
|
||||
|
||||
// CustomHeaders: nil means "no change" (preserve existing). Sending a
|
||||
// non-nil (including empty) map replaces. Previously the merge always
|
||||
// overwrote, silently wiping headers on any auth_config-bearing request.
|
||||
func TestUpdateMCPService_CustomHeadersPreserveOnNil(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
// Seed with custom headers.
|
||||
repo.store[id].AuthConfig.CustomHeaders = map[string]string{"X-Tenant": "acme"}
|
||||
|
||||
// Update with AuthConfig but nil CustomHeaders → preserve.
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
Token: "new-token",
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, "acme", got.AuthConfig.CustomHeaders["X-Tenant"],
|
||||
"nil CustomHeaders in request must preserve existing headers")
|
||||
}
|
||||
|
||||
func TestUpdateMCPService_CustomHeadersReplaceOnNonNil(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo := newTestService()
|
||||
id := seedService(t, repo, "stored-api", "stored-token")
|
||||
repo.store[id].AuthConfig.CustomHeaders = map[string]string{"X-Tenant": "acme"}
|
||||
|
||||
upd := buildFullUpdate(id, &types.MCPAuthConfig{
|
||||
APIKey: types.RedactedSecretPlaceholder,
|
||||
Token: types.RedactedSecretPlaceholder,
|
||||
CustomHeaders: map[string]string{"X-Replaced": "yes"},
|
||||
})
|
||||
require.NoError(t, svc.UpdateMCPService(ctx, upd))
|
||||
|
||||
got := repo.store[id]
|
||||
assert.Equal(t, map[string]string{"X-Replaced": "yes"}, got.AuthConfig.CustomHeaders,
|
||||
"non-nil CustomHeaders must replace the stored map")
|
||||
}
|
||||
@@ -114,6 +114,7 @@ func (h *DataSourceHandler) CreateDataSource(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
ds.RedactSensitiveData()
|
||||
c.JSON(http.StatusCreated, ds)
|
||||
}
|
||||
|
||||
@@ -142,6 +143,7 @@ func (h *DataSourceHandler) GetDataSource(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
ds.RedactSensitiveData()
|
||||
c.JSON(http.StatusOK, ds)
|
||||
}
|
||||
|
||||
@@ -179,6 +181,9 @@ func (h *DataSourceHandler) ListDataSources(c *gin.Context) {
|
||||
if dataSources == nil {
|
||||
dataSources = make([]*types.DataSource, 0)
|
||||
}
|
||||
for _, ds := range dataSources {
|
||||
ds.RedactSensitiveData()
|
||||
}
|
||||
c.JSON(http.StatusOK, dataSources)
|
||||
}
|
||||
|
||||
@@ -224,6 +229,7 @@ func (h *DataSourceHandler) UpdateDataSource(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
ds.RedactSensitiveData()
|
||||
c.JSON(http.StatusOK, ds)
|
||||
}
|
||||
|
||||
|
||||
@@ -81,6 +81,9 @@ func (h *MCPServiceHandler) CreateMCPService(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Redact before echoing so the Create response honors the same
|
||||
// "secrets never leave the server" invariant as List/Get/Update.
|
||||
service.RedactSensitiveData()
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"success": true,
|
||||
"data": service,
|
||||
@@ -278,6 +281,29 @@ func (h *MCPServiceHandler) UpdateMCPService(c *gin.Context) {
|
||||
if token, ok := authConfig["token"].(string); ok {
|
||||
service.AuthConfig.Token = token
|
||||
}
|
||||
// Write-only clear flags — must be explicitly pulled off the map
|
||||
// since this handler intentionally uses a manual map-to-struct
|
||||
// projection to support partial updates (see "Use map to handle
|
||||
// partial updates" comment above). A struct-based bind would have
|
||||
// picked these up automatically.
|
||||
if clearAPIKey, ok := authConfig["clear_api_key"].(bool); ok {
|
||||
service.AuthConfig.ClearAPIKey = clearAPIKey
|
||||
}
|
||||
if clearToken, ok := authConfig["clear_token"].(bool); ok {
|
||||
service.AuthConfig.ClearToken = clearToken
|
||||
}
|
||||
// CustomHeaders — nil preserves, non-nil replaces (see
|
||||
// MCPAuthConfig.MergeUpdate). Only copy when the key is present so
|
||||
// the preserve-on-nil branch is reachable.
|
||||
if customHeaders, ok := authConfig["custom_headers"].(map[string]interface{}); ok {
|
||||
headers := make(map[string]string, len(customHeaders))
|
||||
for k, v := range customHeaders {
|
||||
if s, ok := v.(string); ok {
|
||||
headers[k] = s
|
||||
}
|
||||
}
|
||||
service.AuthConfig.CustomHeaders = headers
|
||||
}
|
||||
}
|
||||
if advancedConfig, ok := updateData["advanced_config"].(map[string]interface{}); ok {
|
||||
service.AdvancedConfig = &types.MCPAdvancedConfig{}
|
||||
@@ -299,6 +325,10 @@ func (h *MCPServiceHandler) UpdateMCPService(c *gin.Context) {
|
||||
}
|
||||
|
||||
logger.Infof(ctx, "MCP service updated successfully: %s", secutils.SanitizeForLog(serviceID))
|
||||
// Redact before echoing so the Update response honors the same
|
||||
// "secrets never leave the server" invariant as List/Get/Create, and so
|
||||
// write-only Clear* flags from the request body are never echoed back.
|
||||
service.RedactSensitiveData()
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"success": true,
|
||||
"data": service,
|
||||
|
||||
+61
-31
@@ -30,34 +30,54 @@ func NewModelHandler(service interfaces.ModelService) *ModelHandler {
|
||||
return &ModelHandler{service: service}
|
||||
}
|
||||
|
||||
// hideSensitiveInfo hides sensitive information (APIKey, BaseURL) for builtin models
|
||||
// Returns a copy of the model with sensitive fields cleared if it's a builtin model
|
||||
// hideSensitiveInfo prepares a Model for inclusion in a response body.
|
||||
//
|
||||
// For builtin models, APIKey and BaseURL are stripped entirely and only the
|
||||
// non-credential fields (embedding dimensions, parameter size) survive on a
|
||||
// fresh copy — builtin models are shared across tenants and must never leak
|
||||
// their underlying provider configuration.
|
||||
//
|
||||
// For tenant-owned models the sensitive fields (APIKey, AppSecret) are
|
||||
// replaced by the shared RedactedSecretPlaceholder via RedactSensitiveData,
|
||||
// on a copy so the in-memory representation stays usable by subsequent
|
||||
// code paths. Empty values stay empty so the frontend can render "set vs
|
||||
// not set" without guesswork.
|
||||
func hideSensitiveInfo(model *types.Model) *types.Model {
|
||||
if !model.IsBuiltin {
|
||||
return model
|
||||
}
|
||||
|
||||
// Create a copy with sensitive information hidden
|
||||
return &types.Model{
|
||||
ID: model.ID,
|
||||
TenantID: model.TenantID,
|
||||
Name: model.Name,
|
||||
Type: model.Type,
|
||||
Source: model.Source,
|
||||
Description: model.Description,
|
||||
Parameters: types.ModelParameters{
|
||||
// Hide APIKey and BaseURL for builtin models
|
||||
BaseURL: "",
|
||||
APIKey: "",
|
||||
// Keep other parameters like embedding dimensions
|
||||
EmbeddingParameters: model.Parameters.EmbeddingParameters,
|
||||
ParameterSize: model.Parameters.ParameterSize,
|
||||
},
|
||||
IsBuiltin: model.IsBuiltin,
|
||||
Status: model.Status,
|
||||
CreatedAt: model.CreatedAt,
|
||||
UpdatedAt: model.UpdatedAt,
|
||||
if model.IsBuiltin {
|
||||
return &types.Model{
|
||||
ID: model.ID,
|
||||
TenantID: model.TenantID,
|
||||
Name: model.Name,
|
||||
Type: model.Type,
|
||||
Source: model.Source,
|
||||
Description: model.Description,
|
||||
Parameters: types.ModelParameters{
|
||||
// Hide APIKey and BaseURL for builtin models.
|
||||
BaseURL: "",
|
||||
APIKey: "",
|
||||
// Preserve non-credential fields so the frontend can still
|
||||
// render correct capability metadata (vision support, Ollama
|
||||
// parameter size, interface type, provider identifier, and
|
||||
// any embedding dimensions). These were previously zeroed
|
||||
// out, which caused builtin chat cards to look like their
|
||||
// multimodal / provider metadata was unset.
|
||||
EmbeddingParameters: model.Parameters.EmbeddingParameters,
|
||||
ParameterSize: model.Parameters.ParameterSize,
|
||||
InterfaceType: model.Parameters.InterfaceType,
|
||||
Provider: model.Parameters.Provider,
|
||||
ExtraConfig: model.Parameters.ExtraConfig,
|
||||
SupportsVision: model.Parameters.SupportsVision,
|
||||
},
|
||||
IsBuiltin: model.IsBuiltin,
|
||||
Status: model.Status,
|
||||
CreatedAt: model.CreatedAt,
|
||||
UpdatedAt: model.UpdatedAt,
|
||||
}
|
||||
}
|
||||
// Tenant-owned model: redact secrets on a copy, preserving everything else.
|
||||
cp := *model
|
||||
cp.RedactSensitiveData()
|
||||
return &cp
|
||||
}
|
||||
|
||||
// CreateModelRequest defines the structure for model creation requests
|
||||
@@ -310,12 +330,22 @@ func (h *ModelHandler) UpdateModel(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
}
|
||||
// Preserve backend-managed fields not sent by frontend
|
||||
req.Parameters.ParameterSize = model.Parameters.ParameterSize
|
||||
if req.Parameters.ExtraConfig == nil {
|
||||
req.Parameters.ExtraConfig = model.Parameters.ExtraConfig
|
||||
// Apply write-only secret merge semantics before overwriting the stored
|
||||
// parameters: empty or redacted APIKey/AppSecret preserves the stored
|
||||
// value, the respective ClearXxx flag wipes it, any other value replaces.
|
||||
merged := req.Parameters.MergeUpdate(model.Parameters)
|
||||
// Preserve backend-managed fields not sent by frontend.
|
||||
merged.ParameterSize = model.Parameters.ParameterSize
|
||||
if merged.ExtraConfig == nil {
|
||||
merged.ExtraConfig = model.Parameters.ExtraConfig
|
||||
}
|
||||
model.Parameters = req.Parameters
|
||||
if req.Parameters.ClearAPIKey {
|
||||
logger.Infof(ctx, "Model API key cleared by user: id=%s", id)
|
||||
}
|
||||
if req.Parameters.ClearAppSecret {
|
||||
logger.Infof(ctx, "Model app_secret cleared by user: id=%s", id)
|
||||
}
|
||||
model.Parameters = merged
|
||||
|
||||
model.Source = req.Source
|
||||
model.Type = req.Type
|
||||
|
||||
@@ -108,6 +108,7 @@ func (h *WebSearchProviderHandler) CreateProvider(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
provider.RedactSensitiveData()
|
||||
c.JSON(http.StatusCreated, gin.H{
|
||||
"success": true,
|
||||
"data": provider,
|
||||
@@ -131,6 +132,9 @@ func (h *WebSearchProviderHandler) ListProviders(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
for _, p := range providers {
|
||||
p.RedactSensitiveData()
|
||||
}
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"success": true,
|
||||
"data": providers,
|
||||
@@ -166,6 +170,7 @@ func (h *WebSearchProviderHandler) GetProvider(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
provider.RedactSensitiveData()
|
||||
c.JSON(http.StatusOK, gin.H{
|
||||
"success": true,
|
||||
"data": provider,
|
||||
@@ -212,14 +217,37 @@ func (h *WebSearchProviderHandler) UpdateProvider(c *gin.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Merge incoming parameters into the existing ones using write-only
|
||||
// secret semantics: empty / redacted APIKey preserves the stored value,
|
||||
// ClearAPIKey explicitly wipes it, any other value replaces. Non-secret
|
||||
// fields flow through from the request directly.
|
||||
mergedParams := req.Parameters.MergeUpdate(existing.Parameters)
|
||||
if req.Parameters.ClearAPIKey {
|
||||
logger.Infof(ctx, "WebSearchProvider API key cleared by user: id=%s",
|
||||
secutils.SanitizeForLog(id))
|
||||
}
|
||||
|
||||
// Preserve existing values for top-level metadata fields when the
|
||||
// request omits them (empty string from the JSON decoder). Without this,
|
||||
// a partial update that only flips IsDefault would clobber Name and
|
||||
// Description on the stored record.
|
||||
mergedName := req.Name
|
||||
if mergedName == "" {
|
||||
mergedName = existing.Name
|
||||
}
|
||||
mergedDescription := req.Description
|
||||
if mergedDescription == "" {
|
||||
mergedDescription = existing.Description
|
||||
}
|
||||
|
||||
// Build updated entity, keeping immutable fields from existing
|
||||
provider := &types.WebSearchProviderEntity{
|
||||
ID: id,
|
||||
TenantID: tenantID,
|
||||
Name: secutils.SanitizeForLog(req.Name),
|
||||
Name: secutils.SanitizeForLog(mergedName),
|
||||
Provider: existing.Provider, // Provider type is immutable after creation
|
||||
Description: secutils.SanitizeForLog(req.Description),
|
||||
Parameters: req.Parameters,
|
||||
Description: secutils.SanitizeForLog(mergedDescription),
|
||||
Parameters: mergedParams,
|
||||
IsDefault: req.IsDefault,
|
||||
}
|
||||
|
||||
@@ -232,6 +260,7 @@ func (h *WebSearchProviderHandler) UpdateProvider(c *gin.Context) {
|
||||
// Re-fetch to get the full stored state
|
||||
updated, _ := h.repo.GetByID(ctx, tenantID, id)
|
||||
if updated != nil {
|
||||
updated.RedactSensitiveData()
|
||||
c.JSON(http.StatusOK, gin.H{"success": true, "data": updated})
|
||||
} else {
|
||||
c.JSON(http.StatusOK, gin.H{"success": true})
|
||||
@@ -371,8 +400,17 @@ func (h *WebSearchProviderHandler) TestProviderRaw(c *gin.Context) {
|
||||
c.JSON(http.StatusOK, gin.H{"success": true})
|
||||
}
|
||||
|
||||
// doTestSearch creates a temporary provider and runs a simple test query
|
||||
// doTestSearch creates a temporary provider and runs a simple test query.
|
||||
//
|
||||
// The provider would otherwise try to authenticate against the upstream API
|
||||
// with the redacted placeholder (which is guaranteed to fail with a
|
||||
// confusing error). Reject it up front with an actionable message so the
|
||||
// user knows they should type a real key or test against the saved config
|
||||
// via /test instead.
|
||||
func (h *WebSearchProviderHandler) doTestSearch(ctx context.Context, providerType string, params types.WebSearchProviderParameters) error {
|
||||
if params.APIKey == types.RedactedSecretPlaceholder {
|
||||
return fmt.Errorf("api_key looks like the redacted placeholder; enter your real key or call POST /api/v1/web-search-providers/{id}/test to exercise the saved credential")
|
||||
}
|
||||
logger.Infof(ctx, "[WebSearch][Test] testing provider type=%s", providerType)
|
||||
searchProvider, err := h.registry.CreateProvider(providerType, params)
|
||||
if err != nil {
|
||||
|
||||
@@ -194,7 +194,15 @@ func (s *SyncLog) BeforeCreate(tx *gorm.DB) error {
|
||||
}
|
||||
|
||||
// DataSourceConfig represents the unencrypted configuration structure
|
||||
// Each connector type will have its own specific fields
|
||||
// Each connector type will have its own specific fields.
|
||||
//
|
||||
// ClearCredentials is a write-only flag used in Update requests to
|
||||
// explicitly remove the stored credential map. Because credentials are
|
||||
// per-connector atomic sets (e.g. an OAuth token pair, a GitHub PAT, a
|
||||
// Confluence API token), individual-field removal is intentionally not
|
||||
// supported — revoking one field in isolation would leave the connector in
|
||||
// a broken half-configured state. The flag is never persisted to storage
|
||||
// and never surfaces in responses (omitempty + zero value).
|
||||
type DataSourceConfig struct {
|
||||
// Common fields applicable to most connectors
|
||||
Type string `json:"type"`
|
||||
@@ -207,6 +215,98 @@ type DataSourceConfig struct {
|
||||
|
||||
// Connector-specific configuration
|
||||
Settings map[string]interface{} `json:"settings"`
|
||||
|
||||
// Write-only flag — request-only, never persisted or returned.
|
||||
ClearCredentials bool `json:"clear_credentials,omitempty"`
|
||||
}
|
||||
|
||||
// HasCredentials reports whether the credentials map carries any value at
|
||||
// all. Used by the Update path to detect the "Remove all credentials"
|
||||
// transition (existing.HasCredentials() && !merged.HasCredentials()) and
|
||||
// skip live-connector validation that would otherwise reject the wipe.
|
||||
func (d DataSourceConfig) HasCredentials() bool {
|
||||
return len(d.Credentials) > 0
|
||||
}
|
||||
|
||||
// Redacted returns a copy of the config with every string value in
|
||||
// Credentials replaced by RedactedSecretPlaceholder so it can be returned to
|
||||
// the client without exposing stored secrets. Empty string values stay empty
|
||||
// so the frontend can still distinguish "set (hidden)" from "not set" per
|
||||
// key. Non-string credential values (rare but possible for numeric IDs or
|
||||
// structured settings) pass through unchanged.
|
||||
//
|
||||
// Named "Redacted" rather than "RedactSensitiveData" because it returns a
|
||||
// copy instead of mutating in place, matching the naming convention of
|
||||
// ConnectionConfig.MaskSensitiveFields in types/vectorstore.go.
|
||||
func (d DataSourceConfig) Redacted() DataSourceConfig {
|
||||
cp := d
|
||||
// ClearCredentials is a request-only signal; never echo it in responses.
|
||||
cp.ClearCredentials = false
|
||||
if d.Credentials == nil {
|
||||
return cp
|
||||
}
|
||||
redacted := make(map[string]interface{}, len(d.Credentials))
|
||||
for k, v := range d.Credentials {
|
||||
if s, ok := v.(string); ok && s != "" {
|
||||
redacted[k] = RedactedSecretPlaceholder
|
||||
} else {
|
||||
redacted[k] = v
|
||||
}
|
||||
}
|
||||
cp.Credentials = redacted
|
||||
return cp
|
||||
}
|
||||
|
||||
// MergeUpdate applies write-only secret merge semantics for Update requests.
|
||||
//
|
||||
// - ClearCredentials=true → wipe the entire credentials map
|
||||
// - Otherwise, each string credential key is merged individually using
|
||||
// PreserveIfRedacted: empty or the redacted placeholder preserves the
|
||||
// existing value, any other value replaces. Non-string credential
|
||||
// values from incoming always overwrite (they aren't redacted either).
|
||||
// - Non-secret fields (Type, ResourceIDs, Settings) flow from incoming
|
||||
// directly.
|
||||
// - The write-only ClearCredentials flag is cleared on the merged result.
|
||||
func (d DataSourceConfig) MergeUpdate(existing DataSourceConfig) DataSourceConfig {
|
||||
merged := d
|
||||
merged.ClearCredentials = false
|
||||
if d.ClearCredentials {
|
||||
merged.Credentials = nil
|
||||
return merged
|
||||
}
|
||||
|
||||
mergedCreds := make(map[string]interface{}, len(existing.Credentials)+len(d.Credentials))
|
||||
for k, v := range existing.Credentials {
|
||||
mergedCreds[k] = v
|
||||
}
|
||||
for k, v := range d.Credentials {
|
||||
if s, ok := v.(string); ok {
|
||||
if IsRedactedOrEmpty(s) {
|
||||
continue // preserve existing value for this key
|
||||
}
|
||||
}
|
||||
mergedCreds[k] = v
|
||||
}
|
||||
merged.Credentials = mergedCreds
|
||||
return merged
|
||||
}
|
||||
|
||||
// RedactSensitiveData parses the Config jsonb, redacts string values in the
|
||||
// Credentials map, and writes the result back. No-op when Config is empty or
|
||||
// fails to parse (the caller continues to return the DataSource with
|
||||
// whatever Config representation it has — better than dropping the response).
|
||||
func (d *DataSource) RedactSensitiveData() {
|
||||
if len(d.Config) == 0 {
|
||||
return
|
||||
}
|
||||
parsed, err := d.ParseConfig()
|
||||
if err != nil || parsed == nil {
|
||||
return
|
||||
}
|
||||
redacted := parsed.Redacted()
|
||||
if blob, err := redacted.ToJSON(); err == nil {
|
||||
d.Config = blob
|
||||
}
|
||||
}
|
||||
|
||||
// Resource represents a syncable resource (document, folder, space) from external system
|
||||
|
||||
@@ -0,0 +1,206 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestDataSourceConfig_Redacted(t *testing.T) {
|
||||
t.Run("redacts all string values in Credentials", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{
|
||||
Type: "github",
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": "real-pat",
|
||||
"installation_id": "real-id",
|
||||
},
|
||||
ResourceIDs: []string{"repo-1"},
|
||||
}
|
||||
out := cfg.Redacted()
|
||||
assert.Equal(t, RedactedSecretPlaceholder, out.Credentials["access_token"])
|
||||
assert.Equal(t, RedactedSecretPlaceholder, out.Credentials["installation_id"])
|
||||
assert.Equal(t, "github", out.Type, "non-credential fields preserved")
|
||||
assert.Equal(t, []string{"repo-1"}, out.ResourceIDs)
|
||||
})
|
||||
|
||||
t.Run("leaves empty string credentials empty", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{"token": ""},
|
||||
}
|
||||
out := cfg.Redacted()
|
||||
assert.Equal(t, "", out.Credentials["token"],
|
||||
"empty must stay empty to signal 'not set' to the frontend")
|
||||
})
|
||||
|
||||
t.Run("non-string credential values pass through unchanged", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{
|
||||
"token": "secret",
|
||||
"installation_id": float64(12345),
|
||||
"metadata": map[string]interface{}{"nested": "value"},
|
||||
},
|
||||
}
|
||||
out := cfg.Redacted()
|
||||
assert.Equal(t, RedactedSecretPlaceholder, out.Credentials["token"])
|
||||
assert.Equal(t, float64(12345), out.Credentials["installation_id"])
|
||||
assert.NotNil(t, out.Credentials["metadata"])
|
||||
})
|
||||
|
||||
t.Run("does not mutate original", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{"token": "secret"},
|
||||
}
|
||||
_ = cfg.Redacted()
|
||||
assert.Equal(t, "secret", cfg.Credentials["token"])
|
||||
})
|
||||
|
||||
t.Run("nil credentials map stays nil", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{Type: "github"}
|
||||
out := cfg.Redacted()
|
||||
assert.Nil(t, out.Credentials)
|
||||
})
|
||||
|
||||
t.Run("ClearCredentials flag stripped from response", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{ClearCredentials: true, Type: "github"}
|
||||
out := cfg.Redacted()
|
||||
assert.False(t, out.ClearCredentials, "write-only flag must not echo back")
|
||||
})
|
||||
}
|
||||
|
||||
func TestDataSourceConfig_MergeUpdate(t *testing.T) {
|
||||
existing := DataSourceConfig{
|
||||
Type: "github",
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": "stored-pat",
|
||||
"installation_id": "stored-id",
|
||||
},
|
||||
ResourceIDs: []string{"old-repo"},
|
||||
}
|
||||
|
||||
t.Run("empty credential value preserves existing", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
Type: "github",
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": "",
|
||||
},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-pat", out.Credentials["access_token"])
|
||||
assert.Equal(t, "stored-id", out.Credentials["installation_id"],
|
||||
"keys not present in incoming carry over from existing")
|
||||
})
|
||||
|
||||
t.Run("redacted placeholder preserves existing", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": RedactedSecretPlaceholder,
|
||||
},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-pat", out.Credentials["access_token"])
|
||||
})
|
||||
|
||||
t.Run("real value replaces existing", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": "new-pat",
|
||||
},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "new-pat", out.Credentials["access_token"])
|
||||
assert.Equal(t, "stored-id", out.Credentials["installation_id"],
|
||||
"other keys preserved from existing")
|
||||
})
|
||||
|
||||
t.Run("mixed preserve and replace", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": RedactedSecretPlaceholder, // preserve
|
||||
"installation_id": "new-id", // replace
|
||||
},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-pat", out.Credentials["access_token"])
|
||||
assert.Equal(t, "new-id", out.Credentials["installation_id"])
|
||||
})
|
||||
|
||||
t.Run("ClearCredentials wipes entire map", func(t *testing.T) {
|
||||
in := DataSourceConfig{ClearCredentials: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Nil(t, out.Credentials,
|
||||
"ClearCredentials removes all credential keys atomically")
|
||||
})
|
||||
|
||||
t.Run("ClearCredentials takes precedence over submitted values", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
ClearCredentials: true,
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": "will-be-ignored",
|
||||
},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Nil(t, out.Credentials)
|
||||
})
|
||||
|
||||
t.Run("ClearCredentials flag never persists on merged result", func(t *testing.T) {
|
||||
in := DataSourceConfig{ClearCredentials: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.False(t, out.ClearCredentials, "write-only flag must not leak to storage")
|
||||
})
|
||||
|
||||
t.Run("non-string incoming values always overwrite", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
Credentials: map[string]interface{}{
|
||||
"installation_id": float64(999),
|
||||
},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, float64(999), out.Credentials["installation_id"],
|
||||
"non-string values are never treated as redacted/empty")
|
||||
})
|
||||
|
||||
t.Run("non-credential fields flow from incoming", func(t *testing.T) {
|
||||
in := DataSourceConfig{
|
||||
Type: "notion",
|
||||
ResourceIDs: []string{"new-repo"},
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "notion", out.Type)
|
||||
assert.Equal(t, []string{"new-repo"}, out.ResourceIDs)
|
||||
})
|
||||
}
|
||||
|
||||
func TestDataSource_RedactSensitiveData(t *testing.T) {
|
||||
t.Run("redacts string credentials in Config jsonb", func(t *testing.T) {
|
||||
cfg := DataSourceConfig{
|
||||
Type: "github",
|
||||
Credentials: map[string]interface{}{
|
||||
"access_token": "real-pat",
|
||||
},
|
||||
}
|
||||
blob, err := json.Marshal(cfg)
|
||||
require.NoError(t, err)
|
||||
|
||||
ds := &DataSource{Config: JSON(blob)}
|
||||
ds.RedactSensitiveData()
|
||||
|
||||
parsed, err := ds.ParseConfig()
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, parsed.Credentials["access_token"])
|
||||
})
|
||||
|
||||
t.Run("no-op on empty config", func(t *testing.T) {
|
||||
ds := &DataSource{Config: nil}
|
||||
ds.RedactSensitiveData() // must not panic
|
||||
assert.Nil(t, ds.Config)
|
||||
})
|
||||
|
||||
t.Run("no-op on malformed config", func(t *testing.T) {
|
||||
ds := &DataSource{Config: JSON(`not valid json`)}
|
||||
ds.RedactSensitiveData() // must not panic
|
||||
// Config remains unchanged — better than dropping the response.
|
||||
assert.Equal(t, JSON(`not valid json`), ds.Config)
|
||||
})
|
||||
}
|
||||
+73
-17
@@ -41,11 +41,60 @@ type MCPService struct {
|
||||
// MCPHeaders represents HTTP headers as a map
|
||||
type MCPHeaders map[string]string
|
||||
|
||||
// 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. They are not persisted (gorm:"-") and
|
||||
// never appear in responses (omitempty + zero value). Without these flags an
|
||||
// empty APIKey/Token submitted in an Update request is interpreted as "no
|
||||
// change", so this is the only way to intentionally clear a stored secret.
|
||||
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" gorm:"-"`
|
||||
ClearToken bool `json:"clear_token,omitempty" gorm:"-"`
|
||||
}
|
||||
|
||||
// MergeUpdate applies write-only secret merge semantics for Update requests.
|
||||
// The receiver is the incoming request DTO; existing is the currently stored
|
||||
// value (may be nil for new services).
|
||||
//
|
||||
// Behavior:
|
||||
// - ClearAPIKey / ClearToken take precedence — wipe the corresponding field.
|
||||
// - Otherwise APIKey / Token use PreserveIfRedacted: empty or the redacted
|
||||
// placeholder preserves existing, any other value replaces.
|
||||
// - CustomHeaders: nil (absent in request) preserves existing; any non-nil
|
||||
// value (including an empty map) replaces. Callers that want to clear
|
||||
// headers explicitly should send {} rather than omitting the field.
|
||||
// - Write-only clear flags are cleared on the merged result so they never
|
||||
// leak to storage.
|
||||
func (c *MCPAuthConfig) MergeUpdate(existing *MCPAuthConfig) *MCPAuthConfig {
|
||||
merged := &MCPAuthConfig{}
|
||||
if existing != nil {
|
||||
*merged = *existing
|
||||
merged.ClearAPIKey = false
|
||||
merged.ClearToken = false
|
||||
}
|
||||
if c == nil {
|
||||
return merged
|
||||
}
|
||||
if c.ClearAPIKey {
|
||||
merged.APIKey = ""
|
||||
} else {
|
||||
merged.APIKey = PreserveIfRedacted(c.APIKey, merged.APIKey)
|
||||
}
|
||||
if c.ClearToken {
|
||||
merged.Token = ""
|
||||
} else {
|
||||
merged.Token = PreserveIfRedacted(c.Token, merged.Token)
|
||||
}
|
||||
if c.CustomHeaders != nil {
|
||||
merged.CustomHeaders = c.CustomHeaders
|
||||
}
|
||||
return merged
|
||||
}
|
||||
|
||||
// MCPAdvancedConfig represents advanced configuration for MCP service
|
||||
@@ -228,16 +277,30 @@ func GetDefaultAdvancedConfig() *MCPAdvancedConfig {
|
||||
}
|
||||
}
|
||||
|
||||
// MaskSensitiveData masks sensitive information in the MCP service for display
|
||||
func (m *MCPService) MaskSensitiveData() {
|
||||
if m.AuthConfig != nil {
|
||||
if m.AuthConfig.APIKey != "" {
|
||||
m.AuthConfig.APIKey = maskString(m.AuthConfig.APIKey)
|
||||
}
|
||||
if m.AuthConfig.Token != "" {
|
||||
m.AuthConfig.Token = maskString(m.AuthConfig.Token)
|
||||
}
|
||||
// RedactSensitiveData replaces secret values in the MCP service with
|
||||
// RedactedSecretPlaceholder while preserving the "set vs not set" distinction
|
||||
// (empty values stay empty). Mutates the receiver in place; callers that need
|
||||
// to retain the originals should pass a copy.
|
||||
//
|
||||
// This replaces the previous MaskSensitiveData implementation which returned
|
||||
// a "first4****last4" slice of the original secret, leaking 8 characters of
|
||||
// entropy per value. The new implementation returns a fixed placeholder with
|
||||
// no information about the underlying secret.
|
||||
func (m *MCPService) RedactSensitiveData() {
|
||||
if m.AuthConfig == nil {
|
||||
return
|
||||
}
|
||||
if m.AuthConfig.APIKey != "" {
|
||||
m.AuthConfig.APIKey = RedactedSecretPlaceholder
|
||||
}
|
||||
if m.AuthConfig.Token != "" {
|
||||
m.AuthConfig.Token = RedactedSecretPlaceholder
|
||||
}
|
||||
// Write-only flags must never reach the client. omitempty alone is
|
||||
// insufficient because a true value serializes; explicitly clear them so
|
||||
// any response path that runs RedactSensitiveData is safe by construction.
|
||||
m.AuthConfig.ClearAPIKey = false
|
||||
m.AuthConfig.ClearToken = false
|
||||
}
|
||||
|
||||
// HideSensitiveInfo returns a copy of the MCP service with sensitive fields cleared for builtin services
|
||||
@@ -255,10 +318,3 @@ func (m *MCPService) HideSensitiveInfo() *MCPService {
|
||||
return ©
|
||||
}
|
||||
|
||||
// maskString masks a string, showing only first 4 and last 4 characters
|
||||
func maskString(s string) string {
|
||||
if len(s) <= 8 {
|
||||
return "****"
|
||||
}
|
||||
return s[:4] + "****" + s[len(s)-4:]
|
||||
}
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestMCPService_RedactSensitiveData(t *testing.T) {
|
||||
t.Run("redacts both api_key and token when set", func(t *testing.T) {
|
||||
m := &MCPService{
|
||||
AuthConfig: &MCPAuthConfig{
|
||||
APIKey: "real-api",
|
||||
Token: "real-token",
|
||||
},
|
||||
}
|
||||
m.RedactSensitiveData()
|
||||
assert.Equal(t, RedactedSecretPlaceholder, m.AuthConfig.APIKey)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, m.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("leaves empty secrets empty", func(t *testing.T) {
|
||||
m := &MCPService{AuthConfig: &MCPAuthConfig{}}
|
||||
m.RedactSensitiveData()
|
||||
assert.Empty(t, m.AuthConfig.APIKey, "empty must stay empty to signal 'not set' to the frontend")
|
||||
assert.Empty(t, m.AuthConfig.Token)
|
||||
})
|
||||
|
||||
t.Run("noop on nil AuthConfig", func(t *testing.T) {
|
||||
m := &MCPService{AuthConfig: nil}
|
||||
assert.NotPanics(t, func() { m.RedactSensitiveData() })
|
||||
assert.Nil(t, m.AuthConfig)
|
||||
})
|
||||
|
||||
t.Run("zeroes write-only clear flags so they never echo back", func(t *testing.T) {
|
||||
// Regression for response-side leak: a true Clear* bool serializes
|
||||
// despite json:"...,omitempty" because true is not a zero value.
|
||||
// Any response path that runs RedactSensitiveData must produce JSON
|
||||
// without the clear_api_key / clear_token keys.
|
||||
m := &MCPService{
|
||||
AuthConfig: &MCPAuthConfig{
|
||||
APIKey: "real-api",
|
||||
Token: "real-token",
|
||||
ClearAPIKey: true,
|
||||
ClearToken: true,
|
||||
},
|
||||
}
|
||||
m.RedactSensitiveData()
|
||||
assert.False(t, m.AuthConfig.ClearAPIKey, "ClearAPIKey must be reset before response")
|
||||
assert.False(t, m.AuthConfig.ClearToken, "ClearToken must be reset before response")
|
||||
})
|
||||
}
|
||||
@@ -77,6 +77,57 @@ type ModelParameters struct {
|
||||
// WeKnoraCloud 厂商专用凭证
|
||||
AppID string `yaml:"app_id,omitempty" json:"app_id,omitempty"`
|
||||
AppSecret string `yaml:"app_secret,omitempty" json:"app_secret,omitempty"` // AES-256 加密存储,实际承载上游 API Key
|
||||
|
||||
// Write-only flags: set to true in Update requests to explicitly remove
|
||||
// the corresponding credential. Never persisted (gorm:"-"), never
|
||||
// returned in responses (omitempty + zero value).
|
||||
ClearAPIKey bool `yaml:"-" json:"clear_api_key,omitempty" gorm:"-"`
|
||||
ClearAppSecret bool `yaml:"-" json:"clear_app_secret,omitempty" gorm:"-"`
|
||||
}
|
||||
|
||||
// RedactSensitiveData replaces secret values (APIKey, AppSecret) in
|
||||
// ModelParameters with RedactedSecretPlaceholder when set. Empty values stay
|
||||
// empty so the frontend can distinguish "set (hidden)" from "not set".
|
||||
// Mutates the receiver in place.
|
||||
func (m *Model) RedactSensitiveData() {
|
||||
if m.Parameters.APIKey != "" {
|
||||
m.Parameters.APIKey = RedactedSecretPlaceholder
|
||||
}
|
||||
if m.Parameters.AppSecret != "" {
|
||||
m.Parameters.AppSecret = RedactedSecretPlaceholder
|
||||
}
|
||||
// Write-only flags must never reach the client. omitempty alone is
|
||||
// insufficient because a true value serializes; explicitly clear them so
|
||||
// any response path that runs RedactSensitiveData is safe by construction.
|
||||
m.Parameters.ClearAPIKey = false
|
||||
m.Parameters.ClearAppSecret = false
|
||||
}
|
||||
|
||||
// MergeUpdate applies write-only secret merge semantics for Update requests:
|
||||
//
|
||||
// - ClearAPIKey / ClearAppSecret take precedence — when true, the
|
||||
// corresponding field becomes empty
|
||||
// - Otherwise APIKey and AppSecret use PreserveIfRedacted: empty or the
|
||||
// redacted placeholder preserves the existing value, any other value
|
||||
// replaces
|
||||
// - Non-secret fields flow from incoming directly
|
||||
// - Write-only clear flags are cleared on the merged result so they never
|
||||
// leak to storage
|
||||
func (p ModelParameters) MergeUpdate(existing ModelParameters) ModelParameters {
|
||||
merged := p
|
||||
if p.ClearAPIKey {
|
||||
merged.APIKey = ""
|
||||
} else {
|
||||
merged.APIKey = PreserveIfRedacted(p.APIKey, existing.APIKey)
|
||||
}
|
||||
if p.ClearAppSecret {
|
||||
merged.AppSecret = ""
|
||||
} else {
|
||||
merged.AppSecret = PreserveIfRedacted(p.AppSecret, existing.AppSecret)
|
||||
}
|
||||
merged.ClearAPIKey = false
|
||||
merged.ClearAppSecret = false
|
||||
return merged
|
||||
}
|
||||
|
||||
// Model represents the AI model
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestModel_RedactSensitiveData(t *testing.T) {
|
||||
t.Run("redacts both APIKey and AppSecret when set", func(t *testing.T) {
|
||||
m := &Model{
|
||||
Parameters: ModelParameters{
|
||||
APIKey: "real-api",
|
||||
AppSecret: "real-secret",
|
||||
BaseURL: "https://example.com",
|
||||
},
|
||||
}
|
||||
m.RedactSensitiveData()
|
||||
assert.Equal(t, RedactedSecretPlaceholder, m.Parameters.APIKey)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, m.Parameters.AppSecret)
|
||||
assert.Equal(t, "https://example.com", m.Parameters.BaseURL,
|
||||
"non-secret fields must be preserved")
|
||||
})
|
||||
|
||||
t.Run("leaves empty secrets empty", func(t *testing.T) {
|
||||
m := &Model{
|
||||
Parameters: ModelParameters{BaseURL: "https://example.com"},
|
||||
}
|
||||
m.RedactSensitiveData()
|
||||
assert.Empty(t, m.Parameters.APIKey)
|
||||
assert.Empty(t, m.Parameters.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("redacts only set secret when one is empty", func(t *testing.T) {
|
||||
m := &Model{
|
||||
Parameters: ModelParameters{APIKey: "real-api"}, // AppSecret empty
|
||||
}
|
||||
m.RedactSensitiveData()
|
||||
assert.Equal(t, RedactedSecretPlaceholder, m.Parameters.APIKey)
|
||||
assert.Empty(t, m.Parameters.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("zeroes write-only clear flags so they never echo back", func(t *testing.T) {
|
||||
// Regression for response-side leak: a true Clear* bool serializes
|
||||
// despite json:"...,omitempty" because true is not a zero value.
|
||||
// Any response path that runs RedactSensitiveData must produce JSON
|
||||
// without the clear_* keys.
|
||||
m := &Model{
|
||||
Parameters: ModelParameters{
|
||||
APIKey: "real-api",
|
||||
ClearAPIKey: true,
|
||||
ClearAppSecret: true,
|
||||
},
|
||||
}
|
||||
m.RedactSensitiveData()
|
||||
assert.False(t, m.Parameters.ClearAPIKey, "ClearAPIKey must be reset before response")
|
||||
assert.False(t, m.Parameters.ClearAppSecret, "ClearAppSecret must be reset before response")
|
||||
})
|
||||
}
|
||||
|
||||
func TestModelParameters_MergeUpdate(t *testing.T) {
|
||||
existing := ModelParameters{
|
||||
APIKey: "stored-api",
|
||||
AppSecret: "stored-secret",
|
||||
BaseURL: "https://stored.example.com",
|
||||
Provider: "openai",
|
||||
}
|
||||
|
||||
t.Run("empty secrets preserve existing", func(t *testing.T) {
|
||||
in := ModelParameters{BaseURL: "https://new.example.com"}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-api", out.APIKey)
|
||||
assert.Equal(t, "stored-secret", out.AppSecret)
|
||||
assert.Equal(t, "https://new.example.com", out.BaseURL,
|
||||
"non-secret fields flow from incoming")
|
||||
})
|
||||
|
||||
t.Run("redacted placeholders preserve existing", func(t *testing.T) {
|
||||
in := ModelParameters{
|
||||
APIKey: RedactedSecretPlaceholder,
|
||||
AppSecret: RedactedSecretPlaceholder,
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-api", out.APIKey)
|
||||
assert.Equal(t, "stored-secret", out.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("real values replace existing", func(t *testing.T) {
|
||||
in := ModelParameters{APIKey: "new-api", AppSecret: "new-secret"}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "new-api", out.APIKey)
|
||||
assert.Equal(t, "new-secret", out.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("mixed preserve and replace", func(t *testing.T) {
|
||||
in := ModelParameters{
|
||||
APIKey: RedactedSecretPlaceholder, // preserve
|
||||
AppSecret: "new-secret", // replace
|
||||
}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-api", out.APIKey)
|
||||
assert.Equal(t, "new-secret", out.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("ClearAPIKey wipes only APIKey", func(t *testing.T) {
|
||||
in := ModelParameters{ClearAPIKey: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Empty(t, out.APIKey)
|
||||
assert.Equal(t, "stored-secret", out.AppSecret,
|
||||
"ClearAPIKey must not affect AppSecret")
|
||||
})
|
||||
|
||||
t.Run("ClearAppSecret wipes only AppSecret", func(t *testing.T) {
|
||||
in := ModelParameters{ClearAppSecret: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-api", out.APIKey)
|
||||
assert.Empty(t, out.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("both clear flags wipe both secrets", func(t *testing.T) {
|
||||
in := ModelParameters{ClearAPIKey: true, ClearAppSecret: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Empty(t, out.APIKey)
|
||||
assert.Empty(t, out.AppSecret)
|
||||
})
|
||||
|
||||
t.Run("clear takes precedence over submitted value", func(t *testing.T) {
|
||||
in := ModelParameters{APIKey: "will-be-ignored", ClearAPIKey: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Empty(t, out.APIKey)
|
||||
})
|
||||
|
||||
t.Run("clear flags never persist on merged result", func(t *testing.T) {
|
||||
in := ModelParameters{ClearAPIKey: true, ClearAppSecret: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.False(t, out.ClearAPIKey, "write-only flag must not leak to storage")
|
||||
assert.False(t, out.ClearAppSecret, "write-only flag must not leak to storage")
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,56 @@
|
||||
// Package types — shared secret redaction helpers for the "write-only secrets"
|
||||
// pattern applied across MCP services, Models, WebSearch providers,
|
||||
// DataSources, and VectorStores.
|
||||
//
|
||||
// The contract is:
|
||||
//
|
||||
// - API responses replace sensitive values with RedactedSecretPlaceholder
|
||||
// when the value is set, and leave it as the empty string when it is not.
|
||||
// Clients can therefore distinguish "set (hidden)" from "not set" without
|
||||
// ever seeing the secret itself.
|
||||
//
|
||||
// - API Update requests treat an absent field, the empty string, or the
|
||||
// RedactedSecretPlaceholder as "no change requested" (preserve), and any
|
||||
// other value as an explicit replacement.
|
||||
//
|
||||
// - Explicit removal of a stored secret is expressed with a dedicated
|
||||
// write-only boolean flag on the request DTO (e.g. ClearAPIKey) rather
|
||||
// than by sending an empty string, because the empty string is already
|
||||
// reserved for "preserve".
|
||||
package types
|
||||
|
||||
// RedactedSecretPlaceholder is the fixed value returned in API responses
|
||||
// whenever a sensitive field is set but withheld from the client. The empty
|
||||
// string ("") is reserved for the orthogonal "not set" state, which lets the
|
||||
// frontend distinguish the two states without an extra boolean field.
|
||||
//
|
||||
// The value matches the placeholder already used by
|
||||
// ConnectionConfig.MaskSensitiveFields on VectorStore responses; this package
|
||||
// promotes it to a shared constant so every resource agrees on the same
|
||||
// sentinel.
|
||||
const RedactedSecretPlaceholder = "***"
|
||||
|
||||
// IsRedactedOrEmpty reports whether s should be treated as "no change
|
||||
// requested" in an Update* request. It returns true for:
|
||||
//
|
||||
// - "" — the field was absent from the client payload or explicitly cleared
|
||||
// on the form without using the dedicated Clear* flag
|
||||
// - RedactedSecretPlaceholder — the client echoed back the value it received
|
||||
// in a GET response (for example, a legacy frontend that pre-fills the
|
||||
// edit form with the redacted value)
|
||||
//
|
||||
// Any other value is an explicit replacement and must be persisted.
|
||||
func IsRedactedOrEmpty(s string) bool {
|
||||
return s == "" || s == RedactedSecretPlaceholder
|
||||
}
|
||||
|
||||
// PreserveIfRedacted returns existing when incoming is empty or the redacted
|
||||
// placeholder, otherwise returns incoming. Call this from every Update*
|
||||
// service that accepts secret fields in its request DTO to keep the preserve
|
||||
// semantics identical across resources.
|
||||
func PreserveIfRedacted(incoming, existing string) string {
|
||||
if IsRedactedOrEmpty(incoming) {
|
||||
return existing
|
||||
}
|
||||
return incoming
|
||||
}
|
||||
@@ -0,0 +1,51 @@
|
||||
package types
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestIsRedactedOrEmpty(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
in string
|
||||
want bool
|
||||
}{
|
||||
{"empty string is redacted-or-empty", "", true},
|
||||
{"fixed placeholder is redacted-or-empty", RedactedSecretPlaceholder, true},
|
||||
{"short real secret is not redacted", "ab", false},
|
||||
{"real secret longer than placeholder is not redacted", "real-bearer-token", false},
|
||||
{"secret containing placeholder substring is not redacted", "prefix***suffix", false},
|
||||
{"single asterisk is not redacted", "*", false},
|
||||
{"double asterisk is not redacted", "**", false},
|
||||
{"four asterisks is not redacted (distinct from 3)", "****", false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := IsRedactedOrEmpty(tt.in); got != tt.want {
|
||||
t.Errorf("IsRedactedOrEmpty(%q) = %v, want %v", tt.in, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPreserveIfRedacted(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
incoming string
|
||||
existing string
|
||||
want string
|
||||
}{
|
||||
{"empty incoming preserves existing", "", "stored-secret", "stored-secret"},
|
||||
{"placeholder incoming preserves existing", RedactedSecretPlaceholder, "stored-secret", "stored-secret"},
|
||||
{"real incoming replaces existing", "new-secret", "stored-secret", "new-secret"},
|
||||
{"real incoming replaces empty existing", "new-secret", "", "new-secret"},
|
||||
{"empty incoming with empty existing stays empty", "", "", ""},
|
||||
{"placeholder incoming with empty existing stays empty", RedactedSecretPlaceholder, "", ""},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := PreserveIfRedacted(tt.incoming, tt.existing); got != tt.want {
|
||||
t.Errorf("PreserveIfRedacted(%q, %q) = %q, want %q",
|
||||
tt.incoming, tt.existing, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -194,14 +194,16 @@ func (c ConnectionConfig) GetEndpoint() string {
|
||||
return ""
|
||||
}
|
||||
|
||||
// MaskSensitiveFields returns a copy with Password and APIKey masked.
|
||||
// MaskSensitiveFields returns a copy with Password and APIKey replaced by the
|
||||
// shared RedactedSecretPlaceholder. Empty values stay empty so the frontend
|
||||
// can distinguish "set (hidden)" from "not set" without an extra flag.
|
||||
func (c ConnectionConfig) MaskSensitiveFields() ConnectionConfig {
|
||||
masked := c
|
||||
if masked.Password != "" {
|
||||
masked.Password = "***"
|
||||
masked.Password = RedactedSecretPlaceholder
|
||||
}
|
||||
if masked.APIKey != "" {
|
||||
masked.APIKey = "***"
|
||||
masked.APIKey = RedactedSecretPlaceholder
|
||||
}
|
||||
return masked
|
||||
}
|
||||
|
||||
@@ -214,8 +214,8 @@ func TestNewVectorStoreResponse(t *testing.T) {
|
||||
|
||||
t.Run("masks sensitive fields", func(t *testing.T) {
|
||||
resp := NewVectorStoreResponse(store, "user", false)
|
||||
assert.Equal(t, "***", resp.ConnectionConfig.Password)
|
||||
assert.Equal(t, "***", resp.ConnectionConfig.APIKey)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, resp.ConnectionConfig.Password)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, resp.ConnectionConfig.APIKey)
|
||||
assert.Equal(t, "http://es:9200", resp.ConnectionConfig.Addr) // non-sensitive preserved
|
||||
})
|
||||
|
||||
@@ -574,8 +574,8 @@ func TestConnectionConfig_MaskSensitiveFields(t *testing.T) {
|
||||
APIKey: "sk-api-key",
|
||||
}
|
||||
masked := c.MaskSensitiveFields()
|
||||
assert.Equal(t, "***", masked.Password)
|
||||
assert.Equal(t, "***", masked.APIKey)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, masked.Password)
|
||||
assert.Equal(t, RedactedSecretPlaceholder, masked.APIKey)
|
||||
assert.Equal(t, "http://es:9200", masked.Addr)
|
||||
assert.Equal(t, "elastic", masked.Username)
|
||||
})
|
||||
|
||||
@@ -67,6 +67,12 @@ func (e *WebSearchProviderEntity) BeforeCreate(tx *gorm.DB) (err error) {
|
||||
// API keys are encrypted at rest using AES-GCM.
|
||||
// BaseURL is intentionally NOT included — each provider type uses a hardcoded
|
||||
// official API endpoint to prevent SSRF attacks.
|
||||
//
|
||||
// ClearAPIKey is a write-only flag used in Update requests to explicitly
|
||||
// remove the stored API key. It is never persisted and never returned in
|
||||
// responses. Without this flag an empty APIKey in an Update request is
|
||||
// interpreted as "no change", so this is the only way to intentionally clear
|
||||
// a stored key without deleting the entire provider record.
|
||||
type WebSearchProviderParameters struct {
|
||||
// API key for the search provider (encrypted in DB)
|
||||
APIKey string `yaml:"api_key" json:"api_key,omitempty"`
|
||||
@@ -80,6 +86,42 @@ type WebSearchProviderParameters struct {
|
||||
ProxyURL string `yaml:"proxy_url" json:"proxy_url,omitempty"`
|
||||
// Provider-specific extra configuration for future extensibility
|
||||
ExtraConfig map[string]string `yaml:"extra_config" json:"extra_config,omitempty"`
|
||||
|
||||
// Write-only clear flag — never persisted, never returned.
|
||||
ClearAPIKey bool `yaml:"-" json:"clear_api_key,omitempty" gorm:"-"`
|
||||
}
|
||||
|
||||
// RedactSensitiveData replaces APIKey with RedactedSecretPlaceholder when a
|
||||
// value is set. Empty values stay empty so the frontend can distinguish
|
||||
// "set (hidden)" from "not set". Mutates the receiver in place.
|
||||
func (e *WebSearchProviderEntity) RedactSensitiveData() {
|
||||
if e.Parameters.APIKey != "" {
|
||||
e.Parameters.APIKey = RedactedSecretPlaceholder
|
||||
}
|
||||
// Write-only flag must never reach the client. omitempty alone is
|
||||
// insufficient because a true value serializes; explicitly clear it so
|
||||
// any response path that runs RedactSensitiveData is safe by construction.
|
||||
e.Parameters.ClearAPIKey = false
|
||||
}
|
||||
|
||||
// MergeUpdate applies write-only secret merge semantics for Update requests:
|
||||
//
|
||||
// - ClearAPIKey takes precedence — when true, APIKey becomes empty
|
||||
// - Otherwise APIKey uses PreserveIfRedacted: empty or the redacted
|
||||
// placeholder preserves existing, any other value replaces
|
||||
// - Non-secret fields (EngineID, ProxyURL, ExtraConfig) are replaced
|
||||
// directly from incoming
|
||||
// - The write-only ClearAPIKey flag is cleared on the merged result so it
|
||||
// never leaks to storage
|
||||
func (p WebSearchProviderParameters) MergeUpdate(existing WebSearchProviderParameters) WebSearchProviderParameters {
|
||||
merged := p
|
||||
if p.ClearAPIKey {
|
||||
merged.APIKey = ""
|
||||
} else {
|
||||
merged.APIKey = PreserveIfRedacted(p.APIKey, existing.APIKey)
|
||||
}
|
||||
merged.ClearAPIKey = false
|
||||
return merged
|
||||
}
|
||||
|
||||
// Value implements the driver.Valuer interface.
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
package types
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestWebSearchProviderEntity_RedactSensitiveData(t *testing.T) {
|
||||
t.Run("redacts set api key", func(t *testing.T) {
|
||||
e := &WebSearchProviderEntity{
|
||||
Parameters: WebSearchProviderParameters{APIKey: "real-key", EngineID: "cse-id"},
|
||||
}
|
||||
e.RedactSensitiveData()
|
||||
assert.Equal(t, RedactedSecretPlaceholder, e.Parameters.APIKey)
|
||||
assert.Equal(t, "cse-id", e.Parameters.EngineID, "non-secret fields preserved")
|
||||
})
|
||||
|
||||
t.Run("leaves empty api key empty", func(t *testing.T) {
|
||||
e := &WebSearchProviderEntity{
|
||||
Parameters: WebSearchProviderParameters{APIKey: "", EngineID: "cse-id"},
|
||||
}
|
||||
e.RedactSensitiveData()
|
||||
assert.Empty(t, e.Parameters.APIKey, "empty must stay empty to signal 'not set' to the frontend")
|
||||
})
|
||||
|
||||
t.Run("zeroes write-only clear flag so it never echoes back", func(t *testing.T) {
|
||||
// Regression for response-side leak: a true ClearAPIKey serializes
|
||||
// despite json:"...,omitempty" because true is not a zero value.
|
||||
e := &WebSearchProviderEntity{
|
||||
Parameters: WebSearchProviderParameters{APIKey: "real-key", ClearAPIKey: true},
|
||||
}
|
||||
e.RedactSensitiveData()
|
||||
assert.False(t, e.Parameters.ClearAPIKey, "ClearAPIKey must be reset before response")
|
||||
})
|
||||
}
|
||||
|
||||
func TestWebSearchProviderParameters_MergeUpdate(t *testing.T) {
|
||||
existing := WebSearchProviderParameters{
|
||||
APIKey: "stored-key",
|
||||
EngineID: "stored-engine",
|
||||
ProxyURL: "http://stored-proxy",
|
||||
}
|
||||
|
||||
t.Run("empty api key preserves existing", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{APIKey: "", EngineID: "new-engine"}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-key", out.APIKey)
|
||||
assert.Equal(t, "new-engine", out.EngineID, "non-secret fields flow from incoming")
|
||||
})
|
||||
|
||||
t.Run("redacted placeholder preserves existing", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{APIKey: RedactedSecretPlaceholder}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "stored-key", out.APIKey)
|
||||
})
|
||||
|
||||
t.Run("real value replaces existing", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{APIKey: "new-key"}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Equal(t, "new-key", out.APIKey)
|
||||
})
|
||||
|
||||
t.Run("ClearAPIKey wipes stored value", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{ClearAPIKey: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Empty(t, out.APIKey)
|
||||
})
|
||||
|
||||
t.Run("ClearAPIKey takes precedence over submitted value", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{APIKey: "will-be-ignored", ClearAPIKey: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.Empty(t, out.APIKey)
|
||||
})
|
||||
|
||||
t.Run("ClearAPIKey flag never persists on merged result", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{ClearAPIKey: true}
|
||||
out := in.MergeUpdate(existing)
|
||||
assert.False(t, out.ClearAPIKey, "write-only flag must not leak to storage")
|
||||
})
|
||||
|
||||
t.Run("merge against empty existing", func(t *testing.T) {
|
||||
in := WebSearchProviderParameters{APIKey: RedactedSecretPlaceholder}
|
||||
out := in.MergeUpdate(WebSearchProviderParameters{})
|
||||
assert.Empty(t, out.APIKey, "placeholder against empty preserves empty")
|
||||
})
|
||||
}
|
||||
Reference in New Issue
Block a user