fix: 收口 GeminiAnalyzer 主分析链路的 LLM prompt/response 日志… (#877) - #897
fix: 收口 GeminiAnalyzer 主分析链路的 LLM prompt/response 日志… (#877)#897ZhuLinsen wants to merge 42 commits into
Conversation
github-actions
Bot
added
ai
configuration
documentation
🤖 自动审查报告
📁 修改的文件
🧠 AI 代码审查意见结论Ready to Merge 结构化审查结果必要性通过。该 PR 有明确的业务价值和隐私修复动机，旨在解决 Issue #877 中描述的 LLM Prompt/Response 日志默认输出敏感内容的问题。 关联性通过。PR 描述明确关联并关闭了 Issue #877。 类型
描述完整性完整。PR 描述包含了背景、影响范围、变更范围、验证命令与结果、兼容性与风险、回滚方案和验收标准，信息全面。 风险级别中。本次改动涉及核心分析链路的日志输出行为，尤其是敏感信息脱敏的正则表达式。如果正则表达式不完善，可能导致敏感信息泄露或日志格式异常。然而，PR 提供了详细的实现说明和全面的回归测试（包括敏感内容缺失和受控预览），大大降低了实际风险。 必改项无。 建议项无。 详细审查意见
综上所述，本次 PR 的实现方案经过深思熟虑，解决了明确的问题，并辅以充分的测试和文档更新，可以安全合并。
|
There was a problem hiding this comment.
Pull request overview
This PR fixes unintended leakage of LLM prompt/response previews into default INFO logs for GeminiAnalyzer, ensuring that any content preview is only emitted in an explicit debug mode and is sanitized + truncated.
Changes:
- Remove default INFO/DEBUG logging of raw prompt and raw model response bodies; keep only non-content metadata (model, lengths, timing).
- Add a controlled debug-only preview builder that normalizes to one line, redacts common sensitive tokens, and truncates output.
- Add regression tests for default vs debug behavior and update
docs/CHANGELOG.mdto document the privacy fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/analyzer.py |
Gates prompt/response previews behind explicit debug switches and sanitizes/truncates preview content. |
src/logging_config.py |
Adds a process-level flag to indicate whether sensitive previews are allowed, set during logging setup. |
tests/test_analyzer_logging.py |
Adds regression tests asserting no content leakage by default and controlled redacted previews in debug. |
docs/CHANGELOG.md |
Documents the logging privacy fix (Issue #877). |
| logger.info( | ||
| f"[LLM返回] {model_name} 响应成功, 耗时 {elapsed:.2f}s, 响应长度 {len(response_text)} 字符" | ||
| ) |
There was a problem hiding this comment.
[LLM返回] 这一行使用的是 model_name = config.litellm_model，但实际调用 _call_litellm() 可能会因 fallback 返回不同的 model_used。这会导致日志误报实际使用的模型，排查问题时容易产生误导。建议在响应成功日志中改用 model_used（或同时输出 configured model 与实际 model_used）。
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7df2c9d60
ℹ️ 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".
| level = console_level | ||
| else: | ||
| level = logging.DEBUG if debug else logging.INFO | ||
| set_sensitive_log_preview_enabled(debug or level <= logging.DEBUG) |
There was a problem hiding this comment.
The new preview gate uses level <= logging.DEBUG, which also evaluates true for NOTSET (0). In the API entrypoint (server.py), LOG_LEVEL=NOTSET is converted to logging.NOTSET and passed as console_level, so this branch unexpectedly enables LLM content previews even though no explicit debug switch was requested. That re-opens prompt/response preview logging in a non-debug configuration path and conflicts with the stated requirement to allow previews only under explicit debug modes.
Useful? React with 👍 / 👎.
ZhuLinsen
changed the title
ZhuLinsen
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ad8893d87
ℹ️ 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".
| re.compile( | ||
| r"(?i)\b(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=]\s*(['\"]?)[^\s,;]+(\2)" | ||
| ), |
There was a problem hiding this comment.
The new redaction patterns only match unquoted assignments (for example api_key=...), so common JSON forms such as "api_key":"sk-live-..." or "password":"..." are not sanitized before logging; _sanitize_llm_log_preview('{"api_key":"sk-live-123456"}') currently returns the raw secret. In explicit debug mode this still leaks credentials into log previews whenever prompt/response text includes JSON payloads, which defeats the intended “redacted preview” safety guarantee.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 日志会落出 LLM prompt/response 正文预览，存在明确的敏感信息暴露风险；本 PR 的修复目标清晰，且与 #877 直接对应。
- 是否有对应 issue：有（Fixes #877）
- PR 类型：fix + 收口
GeminiAnalyzer默认日志内容预览、修正响应元数据日志，并补充对应回归测试。 - description 完整性：完整 + 已覆盖模板要求的背景、范围、Issue、验证命令与结果、兼容性/风险、回滚方案；结合 AGENTS 要求，本次也同步更新了
docs/CHANGELOG.md。 - 是否可直接合入：不可 + 就当前 head 的本地 diff 看，未发现新的阻断性正确性问题，但结构化事实显示当前与目标分支存在明确冲突，需先解决冲突后才能合入。
主要问题
- [Process blocker] 结构化事实显示当前 merge 状态为“存在冲突，当前不能直接合并”。按 AGENTS 的默认工作流，这属于合入前必须先处理的阻断项；建议冲突解决后再基于最终 diff 复核一次
src/analyzer.py、src/logging_config.py与tests/test_analyzer_logging.py的一致性。置信度：高
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c60a511b3e
ℹ️ 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 is_sensitive_log_preview_enabled(): | ||
| return True |
There was a problem hiding this comment.
_should_log_llm_content_preview short-circuits to True whenever the process-global flag is set, so once setup_logging(..., debug=True) enables it, later config changes that disable debug mode no longer take effect. This is observable in the runtime-reload path (Config.reset_instance() in SystemConfigService.update) because logging setup is not rerun there: after switching DEBUG/LOG_LEVEL back to non-debug, analyzer calls still log prompt/response previews. That keeps sensitive previews enabled beyond the explicit debug window the fix is trying to enforce.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 日志会落出 LLM prompt/response 正文预览，存在明确的敏感信息暴露风险，本 PR 的修复目标与 #877 直接对应。
- 是否有对应 issue：有（Fixes #877）
- PR 类型：fix + 主要改动集中在
src/analyzer.py/src/logging_config.py的日志行为收口，并补充了对应回归测试与docs/CHANGELOG.md。 - description 完整性：完整 + 无阻断性缺失项；模板要求的背景、范围、Issue、验证命令与结果、兼容性/风险、回滚方案都已覆盖。
- 是否可直接合入：不可 + 当前分支存在明确合并冲突；另外日志预览开关仍有一处运行时正确性问题，会让“仅显式调试时输出预览”的承诺在配置热更新后失效。
主要问题
- [Correctness blocker]
src/analyzer.py/src/logging_config.py：_should_log_llm_content_preview()仍把进程级_sensitive_log_preview_enabled作为最高优先级短路条件，而仓库现有SystemConfigService.update(..., reload_now=True)只会重载运行时配置，不会重新执行setup_logging()。这意味着进程一旦以DEBUG启动并把该全局标志置为True，后续即使通过系统配置把DEBUG/LOG_LEVEL改回非调试，GeminiAnalyzer.analyze()仍会继续输出 prompt/response 调试预览。这个行为与 PR 描述和 acceptance criteria 中“只在显式调试模式下输出”的语义不一致，而且tests/test_analyzer_logging.py目前还把这类短路行为写成了期望值，没有覆盖“调试 -> 非调试”热更新路径。置信度：高。 - [Process blocker] 结构化事实显示当前 PR head 存在明确合并冲突；在冲突解决并基于新的最终 head 复核前，不建议直接合入。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e80e63a9c1
ℹ️ 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".
| else: | ||
| runtime_config = config | ||
| return bool( |
There was a problem hiding this comment.
main.py --debug enables sensitive preview via setup_logging(debug=True), but GeminiAnalyzer.analyze() now calls _should_log_llm_content_preview(config) with a Config object whose debug/log_level can still be False/INFO (the common CLI-only debug path). Because this branch skips is_sensitive_log_preview_enabled(), prompt/response previews are suppressed even in explicit debug runs, which breaks the stated behavior that explicit debug mode should emit controlled previews.
Useful? React with 👍 / 👎.
| re.compile( | ||
| r'(?i)"(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)"\s*:\s*"[^"]*"' | ||
| ), |
There was a problem hiding this comment.
The sanitizer only has a quoted-key pattern for double-quoted JSON, so Python-style dict payloads with single quotes are not redacted; for example _sanitize_llm_log_preview("{'api_key':'sk-live-123','password':'x'}") still returns raw secrets. In debug mode this leaks credentials into the new preview logs, undermining the intended “redacted preview” safety guarantee for common prompt/response payload formats.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 日志会落出 LLM prompt/response 正文预览，存在明确的敏感信息暴露风险，本 PR 的修复目标与
#877直接对应 - 是否有对应 issue：有（
Fixes #877） - PR 类型：fix + 主要是在
src/analyzer.py/src/logging_config.py收口默认日志内容预览，并补充对应回归测试与docs/CHANGELOG.md - description 完整性：完整 + 模板要求的背景、范围、Issue、验证命令与结果、兼容性/风险、回滚方案均已覆盖；README 未更新原因未单列，但文档落点已说明到
docs/CHANGELOG.md - 是否可直接合入：不可 + 当前分支存在冲突，且还有 2 个 correctness blocker 未解决：
main.py --debug这条显式调试链路没有真正打开新的 preview gate，且单引号字典形态的凭据值仍会漏过脱敏
主要问题
- [Correctness blocker]
src/analyzer.py/main.py：GeminiAnalyzer.analyze()现在固定调用_should_log_llm_content_preview(config)，而该函数在传入config时不会读取setup_logging(debug=True)写入的进程级开关；main.py --debug又只把args.debug传给setup_logging()，不会回写到config.debug。结果是 CLI 显式调试模式下，config.debug仍通常为false、config.log_level仍通常为INFO，最终不会输出 PR 描述中承诺的“显式调试模式受控预览”。当前新增测试只覆盖了config.debug=True/LOG_LEVEL=DEBUG，没有覆盖这条现有主入口链路。置信度：高 - [Correctness blocker]
src/analyzer.py：当前_LLM_SENSITIVE_PATTERNS只覆盖双引号 JSON 字段和未加引号的key=value/key:value形式，对 Python dict / 单引号 payload 不生效；例如{'password':'hunter2','api_key':'sk-live-123'}经过_sanitize_llm_log_preview()后仍会原样保留敏感值。这不是“完全新出现”的日志问题，而是本 PR 在受控调试预览这条新路径上继续扩大了仓库里已有 debug/raw 日志模式的暴露面。置信度：高
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 311ecf1997
ℹ️ 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".
| r"(?i)\b(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=]\s*(['\"]?)[^\s,;]+(\2)" | ||
| ), | ||
| r"\1=\2[REDACTED]\3", |
There was a problem hiding this comment.
The unquoted-assignment sanitizer regex truncates secret values at commas/semicolons/spaces ([^\s,;]+), so quoted credentials containing those characters are only partially redacted and the remainder is still logged. For example, _sanitize_llm_log_preview("password='abc,def'") currently yields password=[REDACTED],def', which leaks part of the secret in debug previews and breaks the intended credential-redaction guarantee.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 日志会落出 LLM prompt/response 正文预览，修复日志暴露面与
#877直接相关。 - 是否有对应 issue：有（
Fixes #877） - PR 类型：fix + 主要改动集中在
src/analyzer.py/src/logging_config.py的日志行为收口，并补充了回归测试与docs/CHANGELOG.md。 - description 完整性：完整 + 背景、范围、Issue、验证命令与结果、兼容性、回滚方案和文档落点均已说明。
- 是否可直接合入：不可 + 当前分支存在合并冲突；另外调试预览的脱敏实现仍有凭据残留风险，未达到“脱敏且截断的受控预览”验收标准。
主要问题
- [Correctness blocker]
src/analyzer.py:69-77的脱敏正则对password="abc,def"、password="abc def"、token="a,b;c d"这类“带引号且值内包含逗号/分号/空格”的赋值格式只会替换到第一个分隔符前，后半段仍会保留在日志预览中。本地按当前 head 复现，_sanitize_llm_log_preview('password=\"abc,def\"')会得到password=[REDACTED],def\"。而src/analyzer.py:1248-1279在显式调试模式下会直接写入该预览，所以敏感片段仍可能落盘。现有测试只覆盖了 JSON/单引号字段场景，未覆盖这类赋值格式。置信度：高 - [Process blocker] 结构化事实已标明当前 PR 存在合并冲突；即使 CI 为 success，按仓库当前状态也不能直接合并，需先消除冲突后再确认最终 head 未引入新的日志回归。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 134d93fd20
ℹ️ 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".
| ), | ||
| ( | ||
| re.compile( | ||
| r'(?i)(["\'])(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\1\s*:\s*(["\'])[^"\']*\3' |
There was a problem hiding this comment.
The JSON redaction regex only allows values matching [^"']*, so it stops when it hits an apostrophe inside a double-quoted value (or an escaped quote) and fails to sanitize the full secret. In debug preview mode this leaks credential material that the new “redacted preview” path is meant to protect; for example, _sanitize_llm_log_preview('{"password":"don\'t-share"}') currently returns the raw password, and values like "abc\"def" are only partially redacted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68092b0e3c
ℹ️ 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".
| ), | ||
| ( | ||
| re.compile( | ||
| r"(?i)\b(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=]\s*(?!['\"])[^\s,;]+" |
There was a problem hiding this comment.
The fallback redaction regex for unquoted assignments only masks up to the first whitespace/comma/semicolon, so multi-word secrets still leak in debug previews. For example, _sanitize_llm_log_preview("password=correct horse battery staple") currently produces password=[REDACTED] horse battery staple, exposing most of the credential material even though this path is intended to provide redacted previews.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 日志会输出 LLM prompt/response 预览，确实扩大了敏感信息落盘与共享链路暴露面，本 PR 的修复目标与
#877直接对应 - 是否有对应 issue：有（
Fixes #877） - PR 类型：fix + 主要是在
src/analyzer.py/src/logging_config.py收口默认日志预览，并补充对应回归测试与docs/CHANGELOG.md - description 完整性：不完整 + 按
AGENTS.md，未更新README.md时应明确说明原因及信息落点；当前描述写明了落点在docs/CHANGELOG.md，但没有单独交代不更新README.md的原因 - 是否可直接合入：不可 + 当前分支存在明确合并冲突，且调试预览脱敏对未加引号且含空格/分隔符的凭据仍可能泄露尾部内容
主要问题
- [Correctness blocker]
src/analyzer.py：_sanitize_llm_log_preview()对未加引号的赋值只会遮蔽到首个空白/逗号/分号前，尾部内容仍会留在调试预览里。当前实现下，password=my secret phrase会变成password=[REDACTED] secret phrase，token=abc,def ghi会变成token=[REDACTED],def ghi。这与本 PR“显式调试模式下也只输出脱敏且受控预览”的目标不一致，仍有凭据片段泄露风险；tests/test_analyzer_logging.py目前也未覆盖这类未加引号场景。置信度：高。 - [Process blocker] 结构化事实已明确当前 PR 与目标分支存在冲突；按当前 head 不能直接合并，解冲突后还需要基于最终 diff 重新确认日志脱敏行为。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87b0394ea3
ℹ️ 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".
| r"(?i)\b(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=]\s*(?!['\"])([A-Za-z0-9._~+/:=@#%?!-]+(?:\s+(?!\b(?:api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=])[A-Za-z0-9._~+/:=@#%?!-]+)*)" | ||
| ), | ||
| r"\1=[REDACTED]", |
There was a problem hiding this comment.
The unquoted credential regex only consumes a restricted character set, so values containing common punctuation are only partially masked and the remainder is still logged in debug previews (for example, password=abc$123 becomes password=[REDACTED]$123, similarly for &, ,, ;). Because this code path is intended to provide safe redacted previews, leaving trailing secret fragments in logs is a privacy regression whenever prompts/responses include unquoted credentials with those characters.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 日志会落出 LLM prompt/response 正文预览，收口日志暴露面与
#877直接相关。 - 是否有对应 issue：有（
Fixes #877） - PR 类型：fix + 主要改动集中在
src/analyzer.py/src/logging_config.py的日志行为修复，并补充了回归测试与docs/CHANGELOG.md。 - description 完整性：不完整 + 按
AGENTS.md/ PR 模板，未更新README.md时需要在描述中明确写出原因和文档落点；当前只列出了docs/CHANGELOG.md，但没有单独说明为何无需更新README.md。 - 是否可直接合入：不可 + 当前与目标分支存在冲突；同时脱敏逻辑仍有真实泄露场景，未完全满足“调试模式仅输出脱敏预览”的修复目标。
主要问题
- [Correctness blocker]
src/analyzer.py新增的未加引号凭据脱敏分支只匹配[A-Za-z0-9._~+/:=@#%?!-]这类字符，遇到常见标点会只做部分脱敏，尾部内容仍会进入调试预览。当前 head 下，_sanitize_llm_log_preview("password=abc,def")实际会得到password=[REDACTED],def，password=abc&def会得到password=[REDACTED]&def。这意味着 debug log 里仍可能泄露真实 secret 片段，和本 PR 要解决的问题正面冲突。tests/test_analyzer_logging.py目前只补了“带引号的标点值”和“未加引号的空格值”用例，没有覆盖这类未加引号且含标点的输入，置信度：高。 - [Process blocker] 本仓库
AGENTS.md明确要求用户可见行为变更同步更新相关文档，并在未更新README.md时说明原因与文档落点。当前只有docs/CHANGELOG.md增加了一条记录，但现有运行文档docs/full-guide.md仍只写python main.py --debug # 调试模式（详细日志），没有补充“默认不再输出正文预览、仅显式调试时输出脱敏且截断的受控预览”这一新增语义；PR 描述也未单独解释为什么不需要更新README.md。这属于已有通用文档存在，但本次新增语义未说明。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8920dd894d
ℹ️ 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 is_sensitive_log_preview_enabled(): | ||
| return True |
There was a problem hiding this comment.
The early return on is_sensitive_log_preview_enabled() makes preview logging sticky for the process lifetime once debug is enabled at startup. Fresh evidence from this commit: SystemConfigService.update() reloads config with Config.reset_instance() (src/services/system_config_service.py) but does not re-run setup_logging, so after switching DEBUG/LOG_LEVEL back to non-debug, _should_log_llm_content_preview() still returns True and keeps emitting prompt/response previews. This breaks the expectation that disabling debug should immediately stop sensitive previews until debug is explicitly re-enabled.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 链路会把 LLM prompt/response 正文预览写入日志，本 PR 收口暴露面与
#877直接相关 - 是否有对应 issue：有（#877，PR 描述为
Fixes #877） - PR 类型：fix + 主要改动集中在
src/analyzer.py/src/logging_config.py的日志行为修复，并补充了回归测试与docs/CHANGELOG.md - description 完整性：不完整 + 已补充背景、范围、验证、风险和回滚，但未按
AGENTS.md明确说明未更新README.md的原因；目前只能看出信息落点在docs/CHANGELOG.md - 是否可直接合入：不可 + 当前存在合并冲突；同时
_sanitize_llm_log_preview仍有漏脱敏场景，会让调试预览残留敏感值尾段，需先修正
主要问题
- [Correctness blocker]
src/analyzer.py的_sanitize_llm_log_preview对未加引号的凭据值仍有真实漏脱敏场景。按当前 head 本地复现，password=abc]def会得到password=[REDACTED]]def，password=abc}def会得到password=[REDACTED]}def，password=abc)def会得到password=[REDACTED])def。也就是说正则只遮住了前半段，尾部片段仍会写进调试预览，和本 PR “仅输出脱敏且受控预览”的目标不一致。tests/test_analyzer_logging.py目前只覆盖$、&、,、;等字符，没有覆盖这类情况，回归保护也不足。置信度：高
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 链路会把 LLM prompt/response 正文预览写入日志，扩大敏感内容落盘与共享链路暴露面，本 PR 的修复目标与
#877直接对应 - 是否有对应 issue：有（
Fixes #877） - PR 类型：fix + 主要改动集中在
src/analyzer.py/src/logging_config.py的日志行为收口，并补了回归测试与docs/CHANGELOG.md - description 完整性：不完整 + 已覆盖背景、范围、验证、风险和回滚，但未按
AGENTS.md显式说明这次未更新README.md的原因；文档落点虽可从 Scope 看出包含docs/CHANGELOG.md，描述里没有把这件事单独说清楚 - 是否可直接合入：不可 + 当前存在明确冲突，且
src/analyzer.py的脱敏规则仍遗漏Authorization: Basic ...这类常见认证头，未完全满足“调试模式下仅输出脱敏且截断的受控预览”的修复目标
主要问题
[Correctness blocker]src/analyzer.py的_LLM_SENSITIVE_PATTERNS目前只对Authorization: Bearer ...做了专门脱敏。我按当前实现验证，Authorization: Basic dXNlcjpwYXNz仍会原样出现在调试预览里。这意味着一旦 prompt/response 中出现 Basic Auth 头，完整凭据仍会落入 debug 日志，和本 PR 自述的“只输出脱敏预览”不一致。[Process blocker]结构化事实已经明确当前分支存在冲突。CI 虽然是 success，但在冲突解决前这条 PR 不能直接合入。[Nice to have]仓库里已经有README.md/docs/full-guide.md这类通用运行与日志文档，但本次新增的“默认不打正文预览、仅在显式调试下输出脱敏截断预览”语义目前只写进了docs/CHANGELOG.md；如果决定不更新 README/完整指南，建议至少在 PR 描述里按AGENTS.md把原因和文档落点说清楚。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7215e44523
ℹ️ 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".
| ), | ||
| ( | ||
| re.compile( | ||
| r"(?i)\b(api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=]\s*(?!['\"])([A-Za-z0-9._~+/:=@#%?!$&,;-]+(?:\s+(?!\b(?:api[_-]?key|access[_-]?token|refresh[_-]?token|token|secret|password|passwd|session[_-]?id)\b\s*[:=])[A-Za-z0-9._~+/:=@#%?!$&,;-]+)*)" |
There was a problem hiding this comment.
The unquoted-assignment redaction regex still only consumes a restricted ASCII character class, so secrets containing other characters are leaked in debug previews. Fresh evidence from this commit: _sanitize_llm_log_preview('password=秘密123') returns the raw value, and _sanitize_llm_log_preview('password=abc|123') leaves the |123 tail. Because these previews are emitted in explicit debug mode specifically to be "safe" after sanitization, this pattern can expose credential material whenever passwords/tokens include non-ASCII or unsupported punctuation.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 + 当前默认 INFO 链路会把 LLM prompt/response 预览写入日志，扩大敏感内容落盘与共享链路暴露面，本 PR 的修复目标与 #877 直接对应
- 是否有对应 issue：有（
Fixes #877） - PR 类型：fix + 主要是在
src/analyzer.py/src/logging_config.py收口日志预览行为，并补充tests/test_analyzer_logging.py回归测试与docs/CHANGELOG.md记录 - description 完整性：不完整 + 描述中的改动范围和 diff 统计仍写成“2 个文件、+16/-2”，与本地实际 4 个文件、403/+10 不一致，且遗漏了
src/logging_config.py、docs/CHANGELOG.md；另外按AGENTS.md，未更新README.md时应明确说明原因和信息实际落点，当前描述里仍未补齐 - 是否可直接合入：不可 + 结构化事实已明确当前存在冲突，需先解决实际 merge conflict；同时 PR 描述需同步到当前真实改动范围后再合入更稳妥
主要问题
- 当前不能直接合并：结构化事实明确给出“存在冲突，当前不能直接合并”。这不是单纯的
mergeable_state=blocked推断，而是已有实际冲突，需要先解开再判断最终可合入性。置信度：高 - PR 描述与实际改动不一致：本地 diff 显示本 PR 实际修改了
src/analyzer.py、src/logging_config.py、tests/test_analyzer_logging.py、docs/CHANGELOG.md，但描述仍停留在旧的 2 文件 / +16 -2 范围，也没有按AGENTS.md说明为何未更新README.md以及本次信息实际落到哪里；这会直接影响评审、回滚和后续追踪。置信度：高
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
4d91fed to
b98028c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b98028cfed
ℹ️ 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".
| re.compile(rf"(?i)\b(authorization)\s*[:=]\s*(?P<value>{_LLM_SENSITIVE_ASSIGNMENT_VALUE_PATTERN})"), | ||
| _replace_authorization_preview, |
There was a problem hiding this comment.
The unquoted authorization matcher only recognizes a standalone authorization token (\b(authorization)), so underscore-normalized forms like proxy_authorization=Basic ... are not sanitized and can still leak proxy credentials in debug preview logs. This matters when headers are serialized with underscore keys (common in framework-normalized payloads or env-style dumps), because _sanitize_llm_log_preview is intended to be the privacy guard before emitting prompt/response previews.
Useful? React with 👍 / 👎.
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 — 与 #877 对应，收口
GeminiAnalyzerprompt/response 日志正文泄露风险明确，改动聚焦于敏感日志输出与 debug 控制，目标清晰且必要。 - 是否有对应 issue：有（#877）
- PR 类型：fix — 本次是日志脱敏/输出开关与回归测试修复，不是新增功能或重构性变更。
- description 完整性：不完整 + 缺少对
docs/CHANGELOG.md新增条目格式问题的明确说明，且未清晰阐明若用户可见行为（日志策略）变更的文档落点与边界说明。 - 是否可直接合入：不可 + 首要阻断点是变更日志格式问题未闭环。
主要问题
- [Process blocker]
docs/CHANGELOG.md：[Unreleased]中新增了### 修复分类标题，且本次补丁在正式版本区块新增了重复的## [3.11.0] - 2026-03-27，属于AGENTS.md要求的扁平格式违规与版本区块重复。依据目前 diff 片段可见同一文件出现该重复结构，存在与发布脚本提取版本说明不一致/冲突风险，应先按规范修正（仅保留[Unreleased]扁平条目，移除重复 release 区块）。
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
- Remove ### category header from [Unreleased] section - Remove duplicate [3.11.0] release block - Keep single properly formatted - [修复] entry for #877
ZhuLinsen
left a comment
ZhuLinsen
left a comment
There was a problem hiding this comment.
评审结论
- 必要性：通过 — 该改动直接对应 #877 所述的日志敏感信息暴露风险，收口
GeminiAnalyzerprompt/response 正文预览属于明确且必要的安全修复。 - 是否有对应 issue：有（#877）
- PR 类型：fix — 本次聚焦于现有主链路日志脱敏与开关控制，未引入新功能、重构或新流程。
- description 完整性：不完整 — 未明确说明用户可见行为变化的文档落点（尤其是“常规运行不再输出正文预览、仅调试模式输出受控预览”的边界与影响说明），目前只覆盖
docs/CHANGELOG.md，建议在通用文档中补一句说明该行为变化。 - 是否可直接合入：可 — 当前无明确阻断性代码风险，CI 状态为成功；
mergeable_state=blocked本身不构成合入阻断理由。
主要问题
- 未发现阻断性问题
🤖 此回复由 OpenReview Bot 自动生成，仅供参考。如有疑问请 @维护者。
PR Type
Background And Problem
+742 / -11。Scope Of Change
docs/CHANGELOG.mdsrc/analyzer.pysrc/logging_config.pytests/test_analyzer_logging.pyDocumentation And Changelog
docs/CHANGELOG.md。README.md或专题文档；如用户可见行为发生变化，请补充文档落点。Issue Link
Closes #877
Verification Commands And Results
关键输出/结论 / Key output & conclusion:
Compatibility And Risk
docs/CHANGELOG.md,src/analyzer.py,src/logging_config.py,tests/test_analyzer_logging.py，建议按文件范围复核。Rollback Plan
git revert <merge-commit>回滚本 PR 提交，重点确认docs/CHANGELOG.md,src/analyzer.py,src/logging_config.py,tests/test_analyzer_logging.py恢复正常。Acceptance Criteria
Implementation
● Read analyzer.py
│ src/analyzer.py
└ L120:155 (34 lines read)
● Read CHANGELOG.md
│ docs/CHANGELOG.md
└ L1:25 (24 lines read)
Now I can see both issues clearly. Let me fix them:
(-?[\w.]+)to(-?[\w.]+(?:[eE][+-]?\d+)?)to handle signed exponents### 修复header and the duplicate non-flat entry, keep only the flat- [修复]line● Edit analyzer.py +1 -1
└ src/analyzer.py
● Edit CHANGELOG.md -3
└ docs/CHANGELOG.md
Now let me verify the fixes and run validation:
● Verify regex fix for signed exponents (shell)
│ cd /tmp/autocode-runs/autocode-issue-877-bug-info-llm-prompt-response-1774933384 && python -c
│ "
│ import re
│
│ # Reproduce the regex from the fixed code…
└ 25 lines...
The regex issue is that
[\w.]+greedily consumes theein1e-3, so the optional exponent group never matches. Let me fix with a broader character class:● Edit analyzer.py +1 -1
└ src/analyzer.py
● Verify regex fix handles all cases (shell)
│ cd /tmp/autocode-runs/autocode-issue-877-bug-info-llm-prompt-response-1774933384 && python -c
│ "
│ import re
│
│ _LLM_SENSITIVE_FIELD_NAME_PATTERN = r'(?:token|key|secret|password|passwd|pwd|auth|credential|…
└ 37 lines...
All regex tests pass. Now verify the CHANGELOG and run py_compile + tests:
● Read CHANGELOG.md
│ docs/CHANGELOG.md
└ L10:22 (12 lines read)
● Validate changed Python files compile (shell)
│ cd /tmp/autocode-runs/autocode-issue-877-bug-info-llm-prompt-response-1774933384 && python -m
│ py_compile src/analyzer.py && python -m py_compile src/logging_config.py && echo "py_compile OK"
└ 2 lines...
CHANGELOG is now flat format, no
###headers, no duplicates. Let me run the relevant tests:● Run analyzer logging tests (shell)
│ cd /tmp/autocode-runs/autocode-issue-877-bug-info-llm-prompt-response-1774933384 && python -m
│ pytest tests/test_analyzer_logging.py -v --no-header --tb=short 2>&1 | tail -30
└ 31 lines...
All 99 tests pass. Let me verify the diff is correct:
● Review final diff (shell)
│ cd /tmp/autocode-runs/autocode-issue-877-bug-info-llm-prompt-response-1774933384 && git
│ --no-pager diff --stat && echo "---" && git --no-pager diff
└ 32 lines...
修复完成，两处改动摘要如下：
(-?[\w.]+)改为(-?[\w.+\-]+)，修复科学计数法带符号指数（如1e-3、-1.5e-10）匹配不完整导致敏感值尾部泄露的问题。[Unreleased]段内违规的### 修复分类标题及其下的重复条目，仅保留扁平格式- [修复]条目，符合 AGENTS.md 规范。Checklist
docs/CHANGELOG.md同步仍需确认；如涉及用户可见变更，请在合并前补充原因与文档落点 / Documentation anddocs/CHANGELOG.mdsync still needs confirmation before merge when user-visible behavior changes