From 6cf7ff175f6d65e55c9687beb1bc431cf8e74647 Mon Sep 17 00:00:00 2001 From: clumsypsc Date: Thu, 1 Oct 2026 17:40:00 +0800 Subject: [PATCH 1/2] fix(evals): keep non-secret environment values out of telemetry redaction telemetry._redact() treated any environment variable whose *name* merely contained API_KEY/_TOKEN/_SECRET/_PASSWORD/_DSN as a secret and then ran a blind str.replace() of its whole value over every payload string. A non-secret config flag is enough to trip this: with X_API_KEY_HELPER_DISABLED=1 in the environment, every "1" inside a payload was rewritten to "[REDACTED]", so an eval event recorded ".../pytest-201/projects/target-repo" as ".../pytest-20[REDACTED]/...". Because the substitution ran before the real credentials were matched, a genuine DSN containing "1" was only partially masked as well. Only values long enough to be credential material are considered now, and they are replaced in a single pass on token boundaries, so unrelated content and longer identifiers are left intact while real secrets stay masked. --- agent/evals/telemetry.py | 31 +++++++++++++++-- tests/unit/test_eval_telemetry.py | 58 +++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 3 deletions(-) create mode 100644 tests/unit/test_eval_telemetry.py diff --git a/agent/evals/telemetry.py b/agent/evals/telemetry.py index 6e3505f..e3032e2 100644 --- a/agent/evals/telemetry.py +++ b/agent/evals/telemetry.py @@ -10,14 +10,39 @@ _LOCK = threading.Lock() _SECRET_RE = re.compile(r"(?i)(api[_-]?key|token|password|secret)(\s*[=:]\s*)([^\s,;]+)") +_ENV_SECRET_MARKERS = ("API_KEY", "_TOKEN", "_SECRET", "_PASSWORD", "_DSN") +# 变量名命中标记不代表取值就是密钥材料：仓库里允许 X_API_KEY_HELPER_DISABLED=1 +# 这类布尔开关存在。取值过短时（0/1/true/no）把它当密钥做子串替换，除了会改写 +# payload 里的无关内容，还会先把文本改掉，让后面真正的长密钥再也匹配不上。 +_ENV_SECRET_MIN_LENGTH = 8 + + +def _environment_secrets() -> tuple[str, ...]: + """收集需要脱敏的环境变量取值，长取值排在前，保证优先匹配更长的密钥。""" + + secrets = { + value + for key, value in os.environ.items() + if len(value) >= _ENV_SECRET_MIN_LENGTH + and any(marker in key.upper() for marker in _ENV_SECRET_MARKERS) + } + return tuple(sorted(secrets, key=len, reverse=True)) + + +def _redact_environment_secrets(text: str) -> str: + """只替换独立出现的密钥取值，不改写更长 token 的内部片段。""" + + secrets = _environment_secrets() + if not secrets: + return text + alternation = "|".join(re.escape(secret) for secret in secrets) + return re.sub(rf"(? Any: if isinstance(value, str): redacted = _SECRET_RE.sub(r"\1\2[REDACTED]", value) - for key, secret in os.environ.items(): - if any(marker in key.upper() for marker in ("API_KEY", "_TOKEN", "_SECRET", "_PASSWORD", "_DSN")) and secret: - redacted = redacted.replace(secret, "[REDACTED]") + redacted = _redact_environment_secrets(redacted) return redacted[:8000] if isinstance(value, dict): return {str(key): _redact(item) for key, item in value.items()} diff --git a/tests/unit/test_eval_telemetry.py b/tests/unit/test_eval_telemetry.py new file mode 100644 index 0000000..dbb55ba --- /dev/null +++ b/tests/unit/test_eval_telemetry.py @@ -0,0 +1,58 @@ +from __future__ import annotations + +import json +from pathlib import Path + +from agent.evals import telemetry + + +def test_short_config_flag_is_not_treated_as_a_secret(monkeypatch) -> None: + """变量名命中 ``API_KEY`` 不代表取值是密钥材料。 + + 仓库允许 ``X_API_KEY_HELPER_DISABLED=1`` 这类布尔开关存在。把这种短取值 + 当成密钥做全局子串替换，会连同 payload 里任意同名片段一起改写。 + """ + + monkeypatch.setenv("CODING_AGENT_API_KEY_HELPER_DISABLED", "1") + payload = {"repo_path": "C:/work/pytest-201/projects/target-repo", "attempt": "1"} + + assert telemetry._redact(payload) == payload + + +def test_recorded_event_keeps_workspace_path_intact(tmp_path: Path, monkeypatch) -> None: + """回归：evals 事件里的工作区路径不能被环境开关改写。""" + + monkeypatch.setenv("CODING_AGENT_EVAL_MODE", "1") + monkeypatch.setenv("CODING_AGENT_API_KEY_HELPER_DISABLED", "1") + monkeypatch.setenv("EVAL_EVENT_FILE", str(tmp_path / "events.jsonl")) + repo_path = "C:/work/pytest-201/projects/target-repo" + + telemetry.record_eval_event("workspace_binding", {"repo_path": repo_path}) + + event = json.loads((tmp_path / "events.jsonl").read_text(encoding="utf-8").strip()) + assert event["payload"]["repo_path"] == repo_path + + +def test_environment_secret_is_still_redacted(monkeypatch) -> None: + """真实密钥取值仍然必须被脱敏。""" + + dsn = "postgresql://eval:sup3rsecret@127.0.0.1:5432/eval_db" + monkeypatch.setenv("POSTGRES_DSN", dsn) + + redacted = telemetry._redact({"connection": dsn, "note": f"connected via {dsn}"}) + + assert dsn not in json.dumps(redacted) + assert redacted == {"connection": "[REDACTED]", "note": "connected via [REDACTED]"} + + +def test_long_secret_is_redacted_without_corrupting_longer_tokens(monkeypatch) -> None: + """只替换独立出现的取值：``postgres`` 不应改写 ``postgresql``。""" + + monkeypatch.setenv("POSTGRES_PASSWORD", "postgres") + + assert telemetry._redact("driver=postgresql host=db") == "driver=postgresql host=db" + assert telemetry._redact("password is postgres") == "password is [REDACTED]" + + +def test_name_value_secret_pattern_is_still_masked() -> None: + assert telemetry._redact("token=abc123def456") == "token=[REDACTED]" From 472cbe77dba98ce96505eb3fa097f7ef4537c292 Mon Sep 17 00:00:00 2001 From: clumsypsc Date: Thu, 1 Oct 2026 18:07:43 +0800 Subject: [PATCH 2/2] fix(evals): treat underscore as an identifier character in redaction Address the two [P3] items from review. 1) Compound identifiers were still split. _redact_environment_secrets() used [A-Za-z0-9] as its token boundary, so a secret value still matched inside identifiers: with POSTGRES_PASSWORD=postgres, "/work/postgres_backup" was recorded as "/work/[REDACTED]_backup" and "postgres_user" as "[REDACTED]_user". Underscore is an identifier character and now belongs to the boundary; hyphen stays a separator because it delimits paths and CLI arguments. A standalone secret is still replaced. 2) The short-value contract is now explicit instead of documented wrong. Values shorter than _ENV_SECRET_MIN_LENGTH are plain configuration and are never substituted, in any form. That includes JSON: {"password": "s3cr3t"} keeps the value, because _SECRET_RE requires '=' or ':' directly after the marker name and the quote blocks the match. Only unquoted named forms such as password=s3cr3t are still masked. Tests cover both directions, plus a nested-payload case pinning that threshold-length secrets stay masked. --- agent/evals/telemetry.py | 13 +++++++++-- tests/unit/test_eval_telemetry.py | 38 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/agent/evals/telemetry.py b/agent/evals/telemetry.py index e3032e2..e209bc1 100644 --- a/agent/evals/telemetry.py +++ b/agent/evals/telemetry.py @@ -14,7 +14,16 @@ # 变量名命中标记不代表取值就是密钥材料：仓库里允许 X_API_KEY_HELPER_DISABLED=1 # 这类布尔开关存在。取值过短时（0/1/true/no）把它当密钥做子串替换，除了会改写 # payload 里的无关内容，还会先把文本改掉，让后面真正的长密钥再也匹配不上。 +# +# 契约：短于该长度的取值一律按普通配置处理，任何形式都不替换——包括 JSON 的 +# 引号形式。{"password": "s3cr3t"} 不会被 _SECRET_RE 拦截，因为该正则要求标记名 +# 后面紧跟 = 或 :，引号挡在中间时匹配不到；具名短密钥只有写成 password=s3cr3t +# 这种无引号形式才会被替换。 _ENV_SECRET_MIN_LENGTH = 8 +# 取值替换的边界：出现在标识符字符内部时一律不替换，所以 postgres 不会把 +# postgresql 或 postgres_user 切成 [REDACTED]。下划线属于标识符字符，连字符不是 +# ——它在路径和命令行里是分隔符，仍算合法边界。 +_IDENTIFIER_CHAR = r"[A-Za-z0-9_]" def _environment_secrets() -> tuple[str, ...]: @@ -30,13 +39,13 @@ def _environment_secrets() -> tuple[str, ...]: def _redact_environment_secrets(text: str) -> str: - """只替换独立出现的密钥取值，不改写更长 token 的内部片段。""" + """只替换作为完整标识符出现的密钥取值。""" secrets = _environment_secrets() if not secrets: return text alternation = "|".join(re.escape(secret) for secret in secrets) - return re.sub(rf"(? Any: diff --git a/tests/unit/test_eval_telemetry.py b/tests/unit/test_eval_telemetry.py index dbb55ba..20bb258 100644 --- a/tests/unit/test_eval_telemetry.py +++ b/tests/unit/test_eval_telemetry.py @@ -54,5 +54,43 @@ def test_long_secret_is_redacted_without_corrupting_longer_tokens(monkeypatch) - assert telemetry._redact("password is postgres") == "password is [REDACTED]" +def test_compound_identifiers_are_not_split_by_a_secret_value(monkeypatch) -> None: + """下划线属于标识符字符：``postgres_user`` 不应变成 ``[REDACTED]_user``。""" + + monkeypatch.setenv("POSTGRES_PASSWORD", "postgres") + payload = {"repo_path": "/work/postgres_backup", "id": "postgres_user"} + + assert telemetry._redact(payload) == payload + + +def test_values_shorter_than_the_threshold_are_plain_configuration(monkeypatch) -> None: + """契约：短于 ``_ENV_SECRET_MIN_LENGTH`` 的取值不是密钥材料，任何形式都不替换。 + + 包含 JSON 的引号形式——``_SECRET_RE`` 要求标记名后紧跟 ``=`` 或 ``:``， + 引号挡在中间时匹配不到，因此 ``{"password": "s3cr3t"}`` 保持原样。只有 + 无引号的 ``password=s3cr3t`` 才会被 ``_SECRET_RE`` 拦截。 + """ + + monkeypatch.setenv("REVIEW_PASSWORD", "s3cr3t") + payload = {"password": "s3cr3t"} + + assert telemetry._redact(payload) == payload + assert telemetry._redact('{"password": "s3cr3t"}') == '{"password": "s3cr3t"}' + assert telemetry._redact("password=s3cr3t") == "password=[REDACTED]" + + +def test_long_environment_secret_is_redacted_inside_nested_payloads(monkeypatch) -> None: + """对照：达到阈值的密钥在字典取值与 JSON 字符串里都仍然被脱敏。""" + + dsn = "postgresql://eval:sup3rsecret@127.0.0.1:5432/eval_db" + monkeypatch.setenv("POSTGRES_DSN", dsn) + embedded = json.dumps({"dsn": dsn}) + + redacted = telemetry._redact({"payload": {"content": embedded}}) + + assert dsn in embedded + assert redacted["payload"]["content"] == '{"dsn": "[REDACTED]"}' + + def test_name_value_secret_pattern_is_still_masked() -> None: assert telemetry._redact("token=abc123def456") == "token=[REDACTED]"