Skip to content

fix(security): 屏蔽 admin 账号接口返回的敏感凭证字段#2271

Open
StarryKira wants to merge 1 commit intoWei-Shaw:mainfrom
StarryKira:fix/redact-account-credentials
Open

fix(security): 屏蔽 admin 账号接口返回的敏感凭证字段#2271
StarryKira wants to merge 1 commit intoWei-Shaw:mainfrom
StarryKira:fix/redact-account-credentials

Conversation

@StarryKira
Copy link
Copy Markdown
Contributor

@StarryKira StarryKira commented May 7, 2026

背景

Account.Credentials 是一个 JSONB map,混合存放可编辑的非敏感配置(base_urlmodel_mappingproject_id 等)与敏感秘钥(OAuth access/refresh/id token、api_keyaws_secret_access_key、Vertex private_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_keysession_key/cookieaws_secret_access_key/aws_session_tokenservice_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 修复脱敏后会崩的两处兜底:
    • apikey 流程:不再读 currentCredentials.api_key,仅在 credentials_status.has_api_key !== true 时要求用户输入新值,否则让后端合并保留旧 key。
    • Vertex 流程:SA-JSON 存在性校验改读 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.goUpdateAccount 端到端三种场景(保留旧 token / 显式覆盖 / 空 map 跳过)。

兼容性

  • 旧前端 + 新后端:旧前端 ...currentCredentials spread 拿不到 token 字段,PUT 时不会带回来,后端合并保留旧值。完全无感。
  • 新前端 + 旧后端credentials_statusundefined,前端可 fallback 到 credentials[key] !== undefined 判存在性。

Test plan

  • go build ./...
  • go test -tags=unit ./...(仅 pre-existing macOS TestResolvePageImagePath 挂,与本 PR 无关)
  • go test -tags=integration ./...
  • golangci-lint run ./...(本 PR 0 新增问题)
  • pnpm typecheck
  • pnpm lint:check(0 errors)
  • 手动:GET /admin/accounts 响应里不应包含 access_token/refresh_token/api_key/私钥;credentials_status.has_* 反映存在性。
  • 手动:编辑 OAuth 账号,仅改 notes,提交后刷新——既有 token 仍工作(merge 保留生效)。
  • 手动:编辑 API-Key 账号,留空 key 输入框,提交——旧 key 保留;输入新 key,提交——新 key 生效。
  • 手动:POST /admin/accounts/:id/refresh 触发 OAuth 刷新——服务端 DB 更新,响应体不再回显新 token。
  • 手动:GET /admin/accounts/data 仍包含完整 credentials(管理员备份路径,按设计保留)。

🤖 Generated with Claude Code

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>
Copilot AI review requested due to automatic review settings May 7, 2026 19:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 UpdateAccount credentials handling from replace semantics to merge semantics that preserves omitted sensitive keys.
  • Frontend: update the edit flow to rely on credentials_status for “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
@StarryKira
Copy link
Copy Markdown
Contributor Author

@Wei-Shaw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants