fix(security): 屏蔽 admin 账号接口返回的敏感凭证字段#2271
Open
StarryKira wants to merge 1 commit intoWei-Shaw:mainfrom
Open
Conversation
Account.Credentials 是 JSONB map,混合存放可编辑的非敏感配置(base_url、 model_mapping、project_id 等)与敏感秘钥(OAuth access/refresh/id token、 API key、AWS secret、Vertex private key 等)。当前所有 admin 账号接口直接 透传该 map,token 经由浏览器 DevTools、抓包、日志等途径泄漏。 - service 包新增 SensitiveCredentialKeys 清单与 MergePreservingSensitiveCreds 作为单一权威定义。 - dto 层 RedactCredentials 在响应里剥离敏感子键,输出 credentials_status (has_<key> 布尔标识)告知前端存在性,不暴露原值。 - AccountFromServiceShallow 接入脱敏,覆盖 list、get、create、update、 refresh、batch、bulk-update、OAuth 创建等 9 个 handler。 - service.UpdateAccount 改为合并语义:incoming 没传敏感键则保留 existing, 让前端"全对象 PUT"流程在脱敏后无感工作;显式提供新 token 仍会覆盖。 - 前端 EditAccountModal 修复脱敏后会崩的两处兜底:apikey 必填检查和 Vertex SA JSON 存在性校验改读 credentials_status.has_*。 - 导出端点 /admin/accounts/data 走独立的 DataAccount 结构,按设计保留 完整 credentials 作为管理员备份路径。 测试:RedactCredentials 单元测试、mapper 端到端 JSON 断言(确认序列化 后无 token 子串)、UpdateAccount 合并语义三种场景(保留 / 覆盖 / 空 map 跳过)。 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the admin account APIs by ensuring sensitive credential sub-keys (OAuth tokens, API keys, cloud secret keys, private keys, etc.) are no longer returned in normal admin responses, while preserving existing sensitive values during “full object PUT” updates to avoid accidentally wiping credentials after redaction.
Changes:
- Backend: introduce a single sensitive-key allowlist, redact those keys from DTO responses, and add
credentials_status.has_<key>presence flags. - Backend: change
UpdateAccountcredentials handling from replace semantics to merge semantics that preserves omitted sensitive keys. - Frontend: update the edit flow to rely on
credentials_statusfor “already configured” checks (not reading redacted secrets), plus add unit tests covering redaction + merge behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/types/index.ts | Adds credentials_status to the Account type to reflect backend presence flags after redaction. |
| frontend/src/components/account/EditAccountModal.vue | Updates edit validation/submit logic to work with redacted credentials by using credentials_status. |
| backend/internal/service/admin_service.go | Switches UpdateAccount credential updates to merge-preserve sensitive keys instead of full replace. |
| backend/internal/service/admin_service_credentials_merge_test.go | Adds unit tests validating UpdateAccount merge semantics for sensitive keys. |
| backend/internal/service/account_credentials_redact.go | Defines sensitive credential key list, lookup helper, and merge function preserving omitted sensitive keys. |
| backend/internal/service/account_credentials_redact_test.go | Adds unit tests for the merge helper and sensitive-key detection. |
| backend/internal/handler/dto/types.go | Extends admin Account DTO with credentials_status and documents redaction behavior. |
| backend/internal/handler/dto/mappers.go | Applies credential redaction in AccountFromServiceShallow (affecting admin endpoints using it). |
| backend/internal/handler/dto/credentials_redact.go | Implements RedactCredentials + presence-status generation. |
| backend/internal/handler/dto/credentials_redact_test.go | Adds unit tests for redaction and presence-status behavior. |
| backend/internal/handler/dto/account_mapper_redact_test.go | Ensures DTO mapping JSON output does not contain sensitive token substrings. |
| backend/internal/handler/admin/account_data.go | Documents that the explicit admin export endpoint intentionally bypasses redaction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
3349
to
3354
| if (editApiKey.value.trim()) { | ||
| // User provided a new API key | ||
| newCredentials.api_key = editApiKey.value.trim() | ||
| } else if (currentCredentials.api_key) { | ||
| // Preserve existing api_key | ||
| newCredentials.api_key = currentCredentials.api_key | ||
| } else { | ||
| } else if (!props.account.credentials_status?.has_api_key) { | ||
| appStore.showError(t('admin.accounts.apiKeyIsRequired')) | ||
| return | ||
| } |
Comment on lines
+3436
to
+3439
| // SA JSON 已脱敏不再随 credentials 返回,存在性改读 credentials_status。 | ||
| const hasExistingServiceAccountJson = | ||
| props.account.credentials_status?.has_service_account_json || | ||
| props.account.credentials_status?.has_service_account |
Comment on lines
+15
to
+26
| var sensitiveCredentialKeySet = func() map[string]struct{} { | ||
| m := make(map[string]struct{}, len(SensitiveCredentialKeys)) | ||
| for _, k := range SensitiveCredentialKeys { | ||
| m[k] = struct{}{} | ||
| } | ||
| return m | ||
| }() | ||
|
|
||
| // IsSensitiveCredentialKey 判断指定键是否为敏感凭证子键。 | ||
| func IsSensitiveCredentialKey(key string) bool { | ||
| _, ok := sensitiveCredentialKeySet[key] | ||
| return ok |
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
Account.Credentials是一个 JSONB map,混合存放可编辑的非敏感配置(base_url、model_mapping、project_id等)与敏感秘钥(OAuth access/refresh/id token、api_key、aws_secret_access_key、Vertexprivate_key等)。当前AccountFromServiceShallow直接透传整个 map,所有 admin 账号端点(list / get / create / update / refresh / batch / bulk-update / OAuth 创建)的响应里都带着 token 原文。任何能拿到响应体的渠道——浏览器 DevTools、抓包、日志——都能看到 refresh token。本 PR 在响应层剥离敏感子键,同时把单条
UpdateAccount改为合并语义,让前端"全对象 PUT"流程在脱敏后无感工作。改动
后端
service.SensitiveCredentialKeys单一权威清单(OAuth token、api_key、session_key/cookie、aws_secret_access_key/aws_session_token、service_account_json/service_account/private_key)。service.MergePreservingSensitiveCreds:非敏感键由incoming决定;敏感键incoming没传则保留existing,显式传入则覆盖。dto.RedactCredentials:剥离敏感子键,输出credentials_status: { has_<key>: true }仅暴露存在性。AccountFromServiceShallow接入脱敏,9 个 admin 端点自动覆盖。service.UpdateAccount把整体替换换成合并;BulkUpdateAccounts保持原状(已通过 SQL||合并)。account_data.go(/admin/accounts/data导出端点)按设计绕过脱敏——它走独立的DataAccount结构作为管理员显式备份路径,加了注释防止未来误改。前端
Account类型新增credentials_status?: Record<string, boolean>。EditAccountModal.vue修复脱敏后会崩的两处兜底:currentCredentials.api_key,仅在credentials_status.has_api_key !== true时要求用户输入新值,否则让后端合并保留旧 key。credentials_status.has_service_account_json || has_service_account。测试
dto/credentials_redact_test.go:nil 输入、混合敏感+非敏感、空/nil/false 值不计为已配置、全敏感键覆盖、不修改入参。dto/account_mapper_redact_test.go:JSON 序列化后断言响应里不会出现 token 子串。service/account_credentials_redact_test.go:合并语义三种场景(保留 / 覆盖 / 非敏感删除)。service/admin_service_credentials_merge_test.go:UpdateAccount端到端三种场景(保留旧 token / 显式覆盖 / 空 map 跳过)。兼容性
...currentCredentialsspread 拿不到 token 字段,PUT 时不会带回来,后端合并保留旧值。完全无感。credentials_status为undefined,前端可 fallback 到credentials[key] !== undefined判存在性。Test plan
go build ./...go test -tags=unit ./...(仅 pre-existing macOSTestResolvePageImagePath挂,与本 PR 无关)go test -tags=integration ./...golangci-lint run ./...(本 PR 0 新增问题)pnpm typecheckpnpm lint:check(0 errors)GET /admin/accounts响应里不应包含access_token/refresh_token/api_key/私钥;credentials_status.has_*反映存在性。notes,提交后刷新——既有 token 仍工作(merge 保留生效)。POST /admin/accounts/:id/refresh触发 OAuth 刷新——服务端 DB 更新,响应体不再回显新 token。GET /admin/accounts/data仍包含完整 credentials(管理员备份路径,按设计保留)。🤖 Generated with Claude Code