diff --git a/frontend/src/api/initialization/index.ts b/frontend/src/api/initialization/index.ts index 3e668cb3..e03971a4 100644 --- a/frontend/src/api/initialization/index.ts +++ b/frontend/src/api/initialization/index.ts @@ -3,26 +3,42 @@ import i18n from '@/i18n' const t = (key: string) => i18n.global.t(key) +// GET /initialization/config/:kbId exposes credential presence, not values. +export interface ModelCredentialStatus { + apiKey?: boolean; +} + +export interface COSCredentialStatus { + secretId?: boolean; + secretKey?: boolean; +} + // 初始化配置数据类型 export interface InitializationConfig { llm: { source: string; modelName: string; baseUrl?: string; + /** @deprecated Use credentials.apiKey from GET responses */ apiKey?: string; + credentials?: ModelCredentialStatus; }; embedding: { source: string; modelName: string; baseUrl?: string; + /** @deprecated Use credentials.apiKey from GET responses */ apiKey?: string; dimension?: number; // 添加embedding维度字段 + credentials?: ModelCredentialStatus; }; rerank: { modelName: string; baseUrl: string; + /** @deprecated Use credentials.apiKey from GET responses */ apiKey?: string; enabled: boolean; + credentials?: ModelCredentialStatus; }; multimodal: { enabled: boolean; @@ -30,16 +46,21 @@ export interface InitializationConfig { vlm?: { modelName: string; baseUrl: string; + /** @deprecated Use credentials.apiKey from GET responses */ apiKey?: string; interfaceType?: string; // "ollama" or "openai" + credentials?: ModelCredentialStatus; }; cos?: { - secretId: string; - secretKey: string; region: string; bucketName: string; appId: string; pathPrefix?: string; + /** @deprecated Use credentials from GET responses */ + secretId?: string; + /** @deprecated Use credentials from GET responses */ + secretKey?: string; + credentials?: COSCredentialStatus; }; minio?: { bucketName: string; diff --git a/internal/handler/dto/mcp.go b/internal/handler/dto/mcp.go index df3c22c0..068b39d3 100644 --- a/internal/handler/dto/mcp.go +++ b/internal/handler/dto/mcp.go @@ -93,6 +93,9 @@ func NewMCPServiceResponse(ctx context.Context, svc *types.MCPService) *MCPServi if !includeDetail { resp.Headers = nil resp.EnvVars = nil + resp.URL = nil + resp.StdioConfig = nil + resp.AdvancedConfig = nil } if svc.AuthConfig != nil { auth := &MCPAuthConfigResponse{ diff --git a/internal/handler/dto/mcp_test.go b/internal/handler/dto/mcp_test.go index ef869fc4..0d63869d 100644 --- a/internal/handler/dto/mcp_test.go +++ b/internal/handler/dto/mcp_test.go @@ -74,17 +74,27 @@ func TestMCPServiceResponse_BuiltinStripsTenantConfig(t *testing.T) { } func TestMCPServiceResponse_ViewerStripsIntegrationDetail(t *testing.T) { + url := "https://tenant-private.example.com" svc := &types.MCPService{ ID: "svc-2", + URL: &url, Headers: types.MCPHeaders{"Authorization": "Bearer secret"}, EnvVars: types.MCPEnvVars{"TOKEN": "secret"}, + StdioConfig: &types.MCPStdioConfig{ + Command: "npx", + Args: []string{"-y", "mcp-server"}, + }, + AdvancedConfig: &types.MCPAdvancedConfig{}, AuthConfig: &types.MCPAuthConfig{ CustomHeaders: map[string]string{"X-Auth": "secret"}, }, } resp := NewMCPServiceResponse(viewerContext(), svc) + assert.Nil(t, resp.URL) assert.Nil(t, resp.Headers) assert.Nil(t, resp.EnvVars) + assert.Nil(t, resp.StdioConfig) + assert.Nil(t, resp.AdvancedConfig) assert.NotNil(t, resp.AuthConfig) assert.Nil(t, resp.AuthConfig.CustomHeaders) } diff --git a/internal/handler/dto/model.go b/internal/handler/dto/model.go index 4c6b1b32..9fd88e11 100644 --- a/internal/handler/dto/model.go +++ b/internal/handler/dto/model.go @@ -72,6 +72,7 @@ func NewModelResponse(ctx context.Context, m *types.Model) *ModelResponse { if !CanViewIntegrationSecrets(ctx) { params.ExtraConfig = nil params.CustomHeaders = nil + params.BaseURL = "" } if m.IsBuiltin { // Builtin: strip everything that could reveal per-tenant config. diff --git a/internal/handler/dto/model_test.go b/internal/handler/dto/model_test.go index 2e8bcf96..6206bf12 100644 --- a/internal/handler/dto/model_test.go +++ b/internal/handler/dto/model_test.go @@ -74,11 +74,13 @@ func TestModelResponse_ViewerStripsIntegrationDetail(t *testing.T) { m := &types.Model{ ID: "m-2", Parameters: types.ModelParameters{ + BaseURL: "https://tenant-private.example.com", CustomHeaders: map[string]string{"Authorization": "Bearer secret"}, - ExtraConfig: map[string]string{"region": "cn-hangzhou"}, + ExtraConfig: map[string]string{"region": "cn-hangzhou"}, }, } resp := NewModelResponse(viewerContext(), m) + assert.Empty(t, resp.Parameters.BaseURL) assert.Nil(t, resp.Parameters.CustomHeaders) assert.Nil(t, resp.Parameters.ExtraConfig) } diff --git a/internal/handler/dto/tenant.go b/internal/handler/dto/tenant.go index a2f07dc2..cc5d7a26 100644 --- a/internal/handler/dto/tenant.go +++ b/internal/handler/dto/tenant.go @@ -83,3 +83,14 @@ func NewTenantResponses(ctx context.Context, tenants []*types.Tenant) []*TenantR } return out } + +// NewTenantResponsesCrossTenant redacts every tenant as Viewer regardless of +// the caller's active-tenant role. Used by cross-tenant list/search endpoints +// where the caller's home-tenant role must not unlock other tenants' secrets. +func NewTenantResponsesCrossTenant(tenants []*types.Tenant) []*TenantResponse { + out := make([]*TenantResponse, 0, len(tenants)) + for _, t := range tenants { + out = append(out, NewTenantResponseWithRole(t, types.TenantRoleViewer)) + } + return out +} diff --git a/internal/handler/dto/tenant_test.go b/internal/handler/dto/tenant_test.go index 7c1eae5a..b7e98a48 100644 --- a/internal/handler/dto/tenant_test.go +++ b/internal/handler/dto/tenant_test.go @@ -47,6 +47,15 @@ func TestTenantResponse_AdminGetsRedactedIntegrationConfigs(t *testing.T) { assert.Equal(t, types.RedactedSecretPlaceholder, resp.StorageEngineConfig.MinIO.SecretAccessKey) } +func TestTenantResponsesCrossTenant_RedactsEvenForOwnerContext(t *testing.T) { + tenant := sampleSecretTenant() + body, err := json.Marshal(NewTenantResponsesCrossTenant([]*types.Tenant{tenant})) + require.NoError(t, err) + s := string(body) + assert.NotContains(t, s, "tenant-api-key-123") + assert.NotContains(t, s, "parser-secret-123") +} + func sampleSecretTenant() *types.Tenant { return &types.Tenant{ ID: 42, diff --git a/internal/handler/mcp_service.go b/internal/handler/mcp_service.go index 1b804a99..d7475076 100644 --- a/internal/handler/mcp_service.go +++ b/internal/handler/mcp_service.go @@ -85,7 +85,7 @@ func (h *MCPServiceHandler) CreateMCPService(c *gin.Context) { // construction — no runtime redaction needed. c.JSON(http.StatusOK, gin.H{ "success": true, - "data": dto.NewMCPServiceResponse(ctx,&service), + "data": dto.NewMCPServiceResponse(ctx, &service), }) } @@ -158,7 +158,7 @@ func (h *MCPServiceHandler) GetMCPService(c *gin.Context) { // so the cross-tenant builtin list does not leak per-tenant config. c.JSON(http.StatusOK, gin.H{ "success": true, - "data": dto.NewMCPServiceResponse(ctx,service), + "data": dto.NewMCPServiceResponse(ctx, service), }) } @@ -351,7 +351,7 @@ func (h *MCPServiceHandler) UpdateMCPService(c *gin.Context) { } c.JSON(http.StatusOK, gin.H{ "success": true, - "data": dto.NewMCPServiceResponse(ctx,stored), + "data": dto.NewMCPServiceResponse(ctx, stored), }) } diff --git a/internal/handler/model.go b/internal/handler/model.go index f5d00af9..45f9909b 100644 --- a/internal/handler/model.go +++ b/internal/handler/model.go @@ -118,7 +118,7 @@ func (h *ModelHandler) CreateModel(c *gin.Context) { c.JSON(http.StatusCreated, gin.H{ "success": true, - "data": dto.NewModelResponse(ctx,model), + "data": dto.NewModelResponse(ctx, model), }) } @@ -163,7 +163,7 @@ func (h *ModelHandler) GetModel(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "success": true, - "data": dto.NewModelResponse(ctx,model), + "data": dto.NewModelResponse(ctx, model), }) } @@ -634,7 +634,7 @@ func (h *ModelHandler) UpdateModel(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "success": true, - "data": dto.NewModelResponse(ctx,model), + "data": dto.NewModelResponse(ctx, model), }) } diff --git a/internal/handler/tenant.go b/internal/handler/tenant.go index ff28ba49..2ee6f046 100644 --- a/internal/handler/tenant.go +++ b/internal/handler/tenant.go @@ -890,7 +890,7 @@ func (h *TenantHandler) ListAllTenants(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "success": true, "data": gin.H{ - "items": tenants, + "items": dto.NewTenantResponsesCrossTenant(tenants), }, }) } @@ -960,7 +960,7 @@ func (h *TenantHandler) SearchTenants(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "success": true, "data": gin.H{ - "items": tenants, + "items": dto.NewTenantResponsesCrossTenant(tenants), "total": total, "page": page, "page_size": pageSize, @@ -1070,14 +1070,6 @@ func (h *TenantHandler) updateTenantWebSearchConfigInternal(c *gin.Context) { return } - cfg = *types.EffectiveWebSearchConfig(&cfg) - - // Validate configuration - if cfg.MaxResults < 1 || cfg.MaxResults > 50 { - c.Error(errors.NewBadRequestError("max_results must be between 1 and 50")) - return - } - tenant, _ := types.TenantInfoFromContext(ctx) if tenant == nil { logger.Error(ctx, "Tenant is empty") @@ -1085,6 +1077,14 @@ func (h *TenantHandler) updateTenantWebSearchConfigInternal(c *gin.Context) { return } + cfg = *types.MergeWebSearchConfigForUpdate(&cfg, tenant.WebSearchConfig) + + // Validate configuration + if cfg.MaxResults < 1 || cfg.MaxResults > 50 { + c.Error(errors.NewBadRequestError("max_results must be between 1 and 50")) + return + } + tenant.WebSearchConfig = &cfg updatedTenant, err := h.service.UpdateTenant(ctx, tenant) if err != nil { @@ -1167,7 +1167,8 @@ func (h *TenantHandler) updateTenantParserEngineConfigInternal(c *gin.Context) { c.Error(errors.NewBadRequestError("Tenant is empty")) return } - tenant.ParserEngineConfig = &cfg + merged := types.MergeParserEngineConfigForUpdate(&cfg, tenant.ParserEngineConfig) + tenant.ParserEngineConfig = merged updatedTenant, err := h.service.UpdateTenant(ctx, tenant) if err != nil { if appErr, ok := errors.IsAppError(err); ok { @@ -1232,7 +1233,8 @@ func (h *TenantHandler) updateTenantStorageEngineConfigInternal(c *gin.Context) c.Error(errors.NewBadRequestError("Tenant is empty")) return } - tenant.StorageEngineConfig = &cfg + merged := types.MergeStorageEngineConfigForUpdate(&cfg, tenant.StorageEngineConfig) + tenant.StorageEngineConfig = merged updatedTenant, err := h.service.UpdateTenant(ctx, tenant) if err != nil { if appErr, ok := errors.IsAppError(err); ok { diff --git a/internal/handler/tenant_secret_disclosure_test.go b/internal/handler/tenant_secret_disclosure_test.go index 76ab51a5..b6986860 100644 --- a/internal/handler/tenant_secret_disclosure_test.go +++ b/internal/handler/tenant_secret_disclosure_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" "github.com/Tencent/WeKnora/internal/middleware" @@ -18,6 +19,11 @@ type stubTenantService struct { tenant *types.Tenant } +func (s *stubTenantService) UpdateTenant(_ context.Context, tenant *types.Tenant) (*types.Tenant, error) { + s.tenant = tenant + return tenant, nil +} + func (s *stubTenantService) CreateTenant(context.Context, *types.Tenant) (*types.Tenant, error) { return nil, nil } @@ -27,9 +33,6 @@ func (s *stubTenantService) GetTenantByID(context.Context, uint64) (*types.Tenan func (s *stubTenantService) GetTenantsByIDs(context.Context, []uint64) (map[uint64]*types.Tenant, error) { return map[uint64]*types.Tenant{s.tenant.ID: s.tenant}, nil } -func (s *stubTenantService) UpdateTenant(context.Context, *types.Tenant) (*types.Tenant, error) { - return s.tenant, nil -} func (s *stubTenantService) DeleteTenant(context.Context, uint64) error { return nil } func (s *stubTenantService) ListTenants(context.Context) ([]*types.Tenant, error) { return []*types.Tenant{s.tenant}, nil @@ -71,6 +74,7 @@ func newTenantHandlerTestEngine(t *testing.T, role types.TenantRole, tenant *typ r.GET("/tenants", h.ListTenants) r.GET("/tenants/:id", h.GetTenant) r.GET("/tenants/kv/:key", h.GetTenantKV) + r.PUT("/tenants/kv/:key", h.UpdateTenantKV) return r } @@ -160,3 +164,18 @@ func TestGetTenantKVViewerAllowedForNonSecretKey(t *testing.T) { engine.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) } + +func TestPutTenantParserConfigAdminPreservesRedactedSecrets(t *testing.T) { + tenant := secretTenantFixture() + engine := newTenantHandlerTestEngine(t, types.TenantRoleAdmin, tenant) + + body := `{"mineru_api_key":"***","mineru_endpoint":"http://new-endpoint"}` + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPut, "/tenants/kv/parser-engine-config", strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + engine.ServeHTTP(rec, req) + require.Equal(t, http.StatusOK, rec.Code) + require.NotNil(t, tenant.ParserEngineConfig) + assert.Equal(t, "parser-secret-123", tenant.ParserEngineConfig.MinerUAPIKey) + assert.Equal(t, "http://new-endpoint", tenant.ParserEngineConfig.MinerUEndpoint) +} diff --git a/internal/handler/web_search_provider.go b/internal/handler/web_search_provider.go index f4dc46ee..d3f9ff66 100644 --- a/internal/handler/web_search_provider.go +++ b/internal/handler/web_search_provider.go @@ -111,7 +111,7 @@ func (h *WebSearchProviderHandler) CreateProvider(c *gin.Context) { c.JSON(http.StatusCreated, gin.H{ "success": true, - "data": dto.NewWebSearchProviderResponse(ctx,provider), + "data": dto.NewWebSearchProviderResponse(ctx, provider), }) } @@ -169,7 +169,7 @@ func (h *WebSearchProviderHandler) GetProvider(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "success": true, - "data": dto.NewWebSearchProviderResponse(ctx,provider), + "data": dto.NewWebSearchProviderResponse(ctx, provider), }) } @@ -263,7 +263,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 { - c.JSON(http.StatusOK, gin.H{"success": true, "data": dto.NewWebSearchProviderResponse(ctx,updated)}) + c.JSON(http.StatusOK, gin.H{"success": true, "data": dto.NewWebSearchProviderResponse(ctx, updated)}) } else { c.JSON(http.StatusOK, gin.H{"success": true}) } diff --git a/internal/types/config_redaction.go b/internal/types/config_redaction.go index 5d8487e5..99fbdd17 100644 --- a/internal/types/config_redaction.go +++ b/internal/types/config_redaction.go @@ -141,3 +141,112 @@ func CredentialsConfigForResponse(cfg *CredentialsConfig, maskSecrets bool) *Cre } return &out } + +// MergeWebSearchConfigForUpdate applies preserve semantics to secret fields on +// tenant KV PUT. +func MergeWebSearchConfigForUpdate(incoming, existing *WebSearchConfig) *WebSearchConfig { + out := *EffectiveWebSearchConfig(incoming) + var prev WebSearchConfig + if existing != nil { + prev = *EffectiveWebSearchConfig(existing) + } + out.APIKey = PreserveIfRedacted(out.APIKey, prev.APIKey) + out.ProxyURL = PreserveIfRedacted(out.ProxyURL, prev.ProxyURL) + return &out +} + +// MergeParserEngineConfigForUpdate applies preserve semantics to secret fields +// on tenant KV PUT. +func MergeParserEngineConfigForUpdate(incoming, existing *ParserEngineConfig) *ParserEngineConfig { + if incoming == nil { + return nil + } + out := *incoming + var prev ParserEngineConfig + if existing != nil { + prev = *existing + } + out.MinerUAPIKey = PreserveIfRedacted(out.MinerUAPIKey, prev.MinerUAPIKey) + out.PaddleOCRVLCloudToken = PreserveIfRedacted(out.PaddleOCRVLCloudToken, prev.PaddleOCRVLCloudToken) + return &out +} + +// MergeStorageEngineConfigForUpdate applies preserve semantics to provider +// secret fields on tenant KV PUT. +func MergeStorageEngineConfigForUpdate(incoming, existing *StorageEngineConfig) *StorageEngineConfig { + if incoming == nil { + return nil + } + out := *incoming + if out.MinIO != nil { + minio := *out.MinIO + var prev MinIOEngineConfig + if existing != nil && existing.MinIO != nil { + prev = *existing.MinIO + } + minio.AccessKeyID = PreserveIfRedacted(minio.AccessKeyID, prev.AccessKeyID) + minio.SecretAccessKey = PreserveIfRedacted(minio.SecretAccessKey, prev.SecretAccessKey) + out.MinIO = &minio + } + if out.COS != nil { + cos := *out.COS + var prev COSEngineConfig + if existing != nil && existing.COS != nil { + prev = *existing.COS + } + cos.SecretID = PreserveIfRedacted(cos.SecretID, prev.SecretID) + cos.SecretKey = PreserveIfRedacted(cos.SecretKey, prev.SecretKey) + out.COS = &cos + } + if out.TOS != nil { + tos := *out.TOS + var prev TOSEngineConfig + if existing != nil && existing.TOS != nil { + prev = *existing.TOS + } + tos.AccessKey = PreserveIfRedacted(tos.AccessKey, prev.AccessKey) + tos.SecretKey = PreserveIfRedacted(tos.SecretKey, prev.SecretKey) + out.TOS = &tos + } + if out.S3 != nil { + s3 := *out.S3 + var prev S3EngineConfig + if existing != nil && existing.S3 != nil { + prev = *existing.S3 + } + s3.AccessKey = PreserveIfRedacted(s3.AccessKey, prev.AccessKey) + s3.SecretKey = PreserveIfRedacted(s3.SecretKey, prev.SecretKey) + out.S3 = &s3 + } + if out.OSS != nil { + oss := *out.OSS + var prev OSSEngineConfig + if existing != nil && existing.OSS != nil { + prev = *existing.OSS + } + oss.AccessKey = PreserveIfRedacted(oss.AccessKey, prev.AccessKey) + oss.SecretKey = PreserveIfRedacted(oss.SecretKey, prev.SecretKey) + out.OSS = &oss + } + if out.KS3 != nil { + ks3 := *out.KS3 + var prev KS3EngineConfig + if existing != nil && existing.KS3 != nil { + prev = *existing.KS3 + } + ks3.AccessKey = PreserveIfRedacted(ks3.AccessKey, prev.AccessKey) + ks3.SecretKey = PreserveIfRedacted(ks3.SecretKey, prev.SecretKey) + out.KS3 = &ks3 + } + if out.OBS != nil { + obs := *out.OBS + var prev OBSEngineConfig + if existing != nil && existing.OBS != nil { + prev = *existing.OBS + } + obs.AccessKey = PreserveIfRedacted(obs.AccessKey, prev.AccessKey) + obs.SecretKey = PreserveIfRedacted(obs.SecretKey, prev.SecretKey) + out.OBS = &obs + } + return &out +} diff --git a/internal/types/config_redaction_test.go b/internal/types/config_redaction_test.go new file mode 100644 index 00000000..60098d0b --- /dev/null +++ b/internal/types/config_redaction_test.go @@ -0,0 +1,92 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestWebSearchConfigForResponse_MasksSecrets(t *testing.T) { + cfg := &WebSearchConfig{ + APIKey: "search-secret", + ProxyURL: "http://proxy.internal:8080", + } + resp := WebSearchConfigForResponse(cfg, true) + require.NotNil(t, resp) + assert.Empty(t, resp.APIKey) + assert.Equal(t, RedactedSecretPlaceholder, resp.ProxyURL) +} + +func TestWebSearchConfigForResponse_Unmasked(t *testing.T) { + cfg := &WebSearchConfig{APIKey: "search-secret", ProxyURL: "http://proxy"} + resp := WebSearchConfigForResponse(cfg, false) + require.NotNil(t, resp) + assert.Equal(t, "search-secret", resp.APIKey) + assert.Equal(t, "http://proxy", resp.ProxyURL) +} + +func TestMergeWebSearchConfigForUpdate_PreservesRedactedSecrets(t *testing.T) { + existing := &WebSearchConfig{APIKey: "stored-key", ProxyURL: "http://stored"} + incoming := &WebSearchConfig{ + APIKey: "", + ProxyURL: RedactedSecretPlaceholder, + MaxResults: 10, + } + merged := MergeWebSearchConfigForUpdate(incoming, existing) + require.NotNil(t, merged) + assert.Equal(t, "stored-key", merged.APIKey) + assert.Equal(t, "http://stored", merged.ProxyURL) + assert.Equal(t, 10, merged.MaxResults) +} + +func TestMergeParserEngineConfigForUpdate_PreservesRedactedSecrets(t *testing.T) { + existing := &ParserEngineConfig{ + MinerUAPIKey: "mineru-secret", + PaddleOCRVLCloudToken: "paddle-secret", + MinerUEndpoint: "http://mineru", + } + incoming := &ParserEngineConfig{ + MinerUAPIKey: RedactedSecretPlaceholder, + PaddleOCRVLCloudToken: RedactedSecretPlaceholder, + MinerUEndpoint: "http://mineru-new", + } + merged := MergeParserEngineConfigForUpdate(incoming, existing) + require.NotNil(t, merged) + assert.Equal(t, "mineru-secret", merged.MinerUAPIKey) + assert.Equal(t, "paddle-secret", merged.PaddleOCRVLCloudToken) + assert.Equal(t, "http://mineru-new", merged.MinerUEndpoint) +} + +func TestMergeStorageEngineConfigForUpdate_PreservesRedactedSecrets(t *testing.T) { + existing := &StorageEngineConfig{ + DefaultProvider: "minio", + MinIO: &MinIOEngineConfig{ + AccessKeyID: "access-id", + SecretAccessKey: "secret-key", + BucketName: "bucket", + }, + } + incoming := &StorageEngineConfig{ + DefaultProvider: "minio", + MinIO: &MinIOEngineConfig{ + AccessKeyID: RedactedSecretPlaceholder, + SecretAccessKey: RedactedSecretPlaceholder, + BucketName: "bucket-new", + }, + } + merged := MergeStorageEngineConfigForUpdate(incoming, existing) + require.NotNil(t, merged) + require.NotNil(t, merged.MinIO) + assert.Equal(t, "access-id", merged.MinIO.AccessKeyID) + assert.Equal(t, "secret-key", merged.MinIO.SecretAccessKey) + assert.Equal(t, "bucket-new", merged.MinIO.BucketName) +} + +func TestParserEngineConfigForResponse_NilSafe(t *testing.T) { + assert.Nil(t, ParserEngineConfigForResponse(nil, true)) +} + +func TestStorageEngineConfigForResponse_NilSafe(t *testing.T) { + assert.Nil(t, StorageEngineConfigForResponse(nil, true)) +} diff --git a/internal/types/secret.go b/internal/types/secret.go index 1fc6d2e7..6045acda 100644 --- a/internal/types/secret.go +++ b/internal/types/secret.go @@ -1,23 +1,32 @@ -// Package types — shared secret-handling helpers. +// Package types — shared secret redaction helpers. // -// Historically this file also held PreserveIfRedacted / IsRedactedOrEmpty -// to support the old "echo redacted placeholder back, server merges on -// Update" pattern shared by MCP / Model / WebSearch / DataSource. Those -// resources have since moved to the credential-resource pattern (a -// dedicated /credentials subresource), so the merge helpers were removed. +// VectorStore connection configs and tenant KV configs (web search, parser +// engine, storage engine) use the redacted-placeholder round-trip pattern: // -// The placeholder constant survives because VectorStore connection configs -// still inline-redact a Password / APIKey on response (see -// types/vectorstore.go → ConnectionConfig.MaskSensitiveFields); migrating -// VectorStore to the credential-resource pattern is left for a future PR -// because it would require introducing a separate connection record (the -// secret currently lives inline on the knowledge base config). +// - GET responses replace set secrets with RedactedSecretPlaceholder (or +// empty string when unset). +// - PUT requests treat "", or RedactedSecretPlaceholder as "preserve +// existing"; any other value replaces the stored secret. +// +// MCP / Model / WebSearch provider / DataSource credentials use a dedicated +// /credentials subresource instead; see internal/handler/dto/mcp.go. package types // RedactedSecretPlaceholder is the fixed value returned in API responses -// whenever a sensitive field is set but withheld from the client. Currently -// used only by VectorStore connection responses. New code should NOT -// introduce redacted-placeholder semantics — model the credential as a -// subresource and omit it from the main response shape instead (see -// internal/handler/dto/mcp.go for the template). +// whenever a sensitive field is set but withheld from the client. const RedactedSecretPlaceholder = "***" + +// IsRedactedOrEmpty reports whether s should be treated as "no change +// requested" in an Update request. +func IsRedactedOrEmpty(s string) bool { + return s == "" || s == RedactedSecretPlaceholder +} + +// PreserveIfRedacted returns existing when incoming is empty or the redacted +// placeholder, otherwise returns incoming. +func PreserveIfRedacted(incoming, existing string) string { + if IsRedactedOrEmpty(incoming) { + return existing + } + return incoming +} diff --git a/internal/types/secret_test.go b/internal/types/secret_test.go new file mode 100644 index 00000000..7ebf7e38 --- /dev/null +++ b/internal/types/secret_test.go @@ -0,0 +1,45 @@ +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}, + } + 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"}, + } + 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) + } + }) + } +}