Skip to content

fix(security): 脱敏传输层错误中泄露的上游密钥#429

Merged
tbphp merged 1 commit into
mainfrom
fix/gemini-key-leak-transport-error
Jul 2, 2026
Merged

fix(security): 脱敏传输层错误中泄露的上游密钥#429
tbphp merged 1 commit into
mainfrom
fix/gemini-key-leak-transport-error

Conversation

@tbphp

@tbphp tbphp commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Summary

修复 GHSA-v6w2-mrg5-vc9r:Gemini 通道把上游 API key 放在 URL query(?key=xxx),当上游请求发生传输层错误(超时/连接失败/DNS)时,Go 的 *url.Error 字符串包含完整 URL(含明文 key),此前被原样返回给客户端并写入请求日志,任何持有该分组 proxy key 的普通消费者都能拿到运营者的上游 provider key。

  • 新增 utils.RedactSecret(text, secret):把 text 中出现的 secret 子串替换为已有的 MaskAPIKey 脱敏形式
  • internal/proxy/server.goerrorMessage/parsedError 计算完成、被用于返回客户端/写日志/更新 key 状态之前,统一调用 RedactSecret 剔除 apiKey.KeyValue
  • 对所有渠道统一生效(不只是 Gemini),成本相同但更稳妥;OpenAI/Anthropic 用 header 传 key 本就不受影响

Test plan

  • TDD:新增 internal/utils/string_utils_test.go,覆盖多次出现替换、空 secret、secret 不存在三种场景,RED→GREEN 验证
  • go build ./... / go vet ./... / go test ./... 全部通过
  • 端到端真实验证:本地启动服务,创建 Gemini 分组 + 指向不可达上游触发真实传输层错误(connection refused),确认:
    • 返回给客户端的 UPSTREAM_ERROR 消息中 key 已脱敏为 sk-t****7890
    • 持久化到 request_logs.error_message 的内容同样已脱敏(注意日志记录里另有一个独立的 key_value 字段,那是管理员本就可见的"本次请求使用了哪个 key"审计字段,与本漏洞无关,不在修复范围内)

Gemini 通道将上游 key 放入 URL query,传输层错误(超时/连接失败)
时 Go 的 url.Error 会把完整 URL 带入错误信息,未脱敏地返回给客户端
并写入请求日志(GHSA-v6w2-mrg5-vc9r)

在 errorMessage/parsedError 计算完成后统一调用 RedactSecret 替换掉
apiKey.KeyValue 子串,对所有渠道生效

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3d83f16239

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

if secret == "" {
return text
}
return strings.ReplaceAll(text, secret, MaskAPIKey(secret))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Redact URL-encoded key values as well

For Gemini requests, ModifyRequest puts the upstream key into the query string via q.Set(...) and q.Encode(), so a key containing escaped characters such as +, /, = or spaces appears in *url.Error as its percent-encoded form. This replacement only searches for the raw apiKey.KeyValue, so transport errors for those keys still return and persist the encoded credential in full; redact the query-escaped form too before returning/logging the error.

Useful? React with 👍 / 👎.

if secret == "" {
return text
}
return strings.ReplaceAll(text, secret, MaskAPIKey(secret))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use a redaction token for short secrets

When the upstream key is 8 bytes or shorter, MaskAPIKey returns the original key, so this ReplaceAll replaces the secret with itself and leaves the transport error unchanged. Since key import only rejects empty strings, a group can have a short upstream key and still leak it to the client/log on the exact error path this helper is meant to protect; use a replacement that always differs from the secret.

Useful? React with 👍 / 👎.

@tbphp tbphp merged commit 3028b1f into main Jul 2, 2026
4 checks passed
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.

1 participant