diff --git a/agent/redact.py b/agent/redact.py index 7cf0a49daf..2bd631228e 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -230,8 +230,11 @@ _ENV_ASSIGN_LOWER_RE = re.compile( # bare secret-word key only at line start (optionally after ``export``), so conversational ``I have # password=foo`` mid-sentence is left alone. _SECRET_CFG_NAMES = r"(?:api[ _.\-]?key|token|secret|passwd|password|credential|auth)" -# Rendered line-number prefix (``5|line`` from read_file, ``6:line`` from grep -n / cat -n). -_LINE_NUMBER_GUTTER = r"[0-9]+[|:][ \t]*" +# Rendered line-number prefix: ``5|line`` (read_file), ``6:line`` (grep -n), ``7-line`` (grep -A/-B/-C +# context lines) and `` 8\tline`` (cat -n / nl: right-aligned number + TAB). Callers put the ONLY +# leading ``[ \t]*`` in front of it — stacking a second whitespace run around an optional gutter made +# the anchored passes quadratic on long indented lines (2s per 5k spaces). +_LINE_NUMBER_GUTTER = r"(?:[0-9]+(?:[|:\-]|\t)[ \t]*)?" _CFG_VALUE = r"(['\"]?)([^\s&]+?)\2(?=[\s&]|$)" # Linear pre-gate for the _CFG_*_RE subs: no secret keyword => neither can match. _CFG_SECRET_WORD_RE = re.compile(_SECRET_CFG_NAMES, re.IGNORECASE) @@ -253,11 +256,11 @@ _CFG_DOTTED_RE = re.compile( ) # Line-anchored bare key: ``password=…`` / ``export api_key=…`` at start of line. # ``{_LINE_NUMBER_GUTTER}``: line-numbered dumps put the key behind a rendered gutter — -# ``read_file`` emits ``5| ADS_API_TOKEN: …`` and ``grep -n`` / ``cat -n`` emit -# ``6: ADS_API_TOKEN: …``. Anchored at ``^`` without it, none of those matched, so the -# rendered read of a secret-bearing file leaked what the raw text masked. +# ``read_file`` emits ``5| ADS_API_TOKEN: …``, ``grep -n`` emits ``6: ADS_API_TOKEN: …`` +# and ``cat -n`` emits `` 7\tADS_API_TOKEN: …``. Anchored at ``^`` without it, none of those +# matched, so the rendered read of a secret-bearing file leaked what the raw text masked. _CFG_ANCHORED_RE = re.compile( - rf"(^(?:[ \t]*{_LINE_NUMBER_GUTTER})?[ \t]*(?:export[ \t]+)?[A-Za-z0-9_\-]*{_SECRET_CFG_NAMES}[A-Za-z0-9_\-]*)={_CFG_VALUE}", + rf"(^[ \t]*{_LINE_NUMBER_GUTTER}(?:export[ \t]+)?[A-Za-z0-9_\-]*{_SECRET_CFG_NAMES}[A-Za-z0-9_\-]*)={_CFG_VALUE}", re.IGNORECASE | re.MULTILINE, ) @@ -270,7 +273,7 @@ _CFG_ANCHORED_RE = re.compile( # stays backtrackable (see _CFG_DOTTED_RE). _YAML_CFG_NAMES = r"(?:api[ _.\-]?key|token|secret|passwd|password|credential)" _YAML_ASSIGN_RE = re.compile( - rf"(^(?:[ \t]*+{_LINE_NUMBER_GUTTER})?[ \t]*+[A-Za-z0-9_.\-]*{_YAML_CFG_NAMES}[A-Za-z0-9_.\-]*+)(:[ \t]*+)(?!['\"])([^\s&]++)", + rf"(^[ \t]*+{_LINE_NUMBER_GUTTER}[A-Za-z0-9_.\-]*{_YAML_CFG_NAMES}[A-Za-z0-9_.\-]*+)(:[ \t]*+)(?!['\"])([^\s&]++)", re.IGNORECASE | re.MULTILINE, ) @@ -305,6 +308,11 @@ _STRONG_KEY_KEYWORD_RE = re.compile( r"|key[ _.\\-]?material|secret|passwd|password|pass|pw|credential|auth|bearer", re.IGNORECASE, ) +# Password-class keys mask any literal value; for other keys a value that starts like ``$HOME/...``, +# ``/usr/...`` or ``~/...`` references a variable or a path, not a credential, even under a strong key +# (``SSH_AUTH_SOCK=$HOME/.ssh/agent.sock``, ``DOCKER_AUTH_CONFIG=/home/u/.docker``). +_PASSWORD_KEY_RE = re.compile(r"passwd|password|pass|pw", re.IGNORECASE) +_PATH_OR_VAR_VALUE_RE = re.compile(r"[$/~]") def _is_word_start(s: str, i: int) -> bool: @@ -370,6 +378,10 @@ def _should_redact_assignment(key: str, value: str, *, check_keyword: bool) -> b return False if check_keyword and not _key_has_secret_keyword(key): return False + # A shell rc's ``SSH_AUTH_SOCK=$HOME/.ssh/agent.sock`` is configuration the agent must keep + # readable; only password-class keys mask a path/variable reference. + if _PATH_OR_VAR_VALUE_RE.match(value) and not _has_word_bounded_keyword(key, _PASSWORD_KEY_RE): + return False return (_has_word_bounded_keyword(key, _STRONG_KEY_KEYWORD_RE) or _looks_like_opaque_credential(value)) @@ -1030,7 +1042,7 @@ def _is_secret_file_arg(arg: str) -> bool: return True # ``config.yaml`` plus the ``config.yaml.good.`` / ``.corrupt.`` copies Hermes # writes under ``backups/config/`` — same contents, same secrets. - if parts[-1] != "config.yaml" and not parts[-1].startswith("config.yaml."): + if parts[-1] != "config.yaml" and not parts[-1].startswith(("config.yaml.good.", "config.yaml.corrupt.")): return False return hermes_home or ".hermes" in parts[:-1] or _is_under_hermes_home(path) diff --git a/tests/agent/test_redact.py b/tests/agent/test_redact.py index ca402dcc63..f7cd435e1b 100644 --- a/tests/agent/test_redact.py +++ b/tests/agent/test_redact.py @@ -2,6 +2,7 @@ import ast import logging +import time import pytest @@ -1259,7 +1260,10 @@ class TestSecretFileAssignmentRedaction: ("FOO_API_KEY={tok}", "«redacted-secret»"), # dotenv ('{{"api_key": "{tok}"}}', "«redacted-secret»"), # JSON ("5| ADS_API_TOKEN: {tok}", "«redacted-secret»"), # read_file line gutter - ("108:ADS_API_TOKEN: {tok}", "«redacted-secret»"), # grep -n / cat -n gutter + ("108:ADS_API_TOKEN: {tok}", "«redacted-secret»"), # grep -n gutter + ("108- ADS_API_TOKEN: {tok}", "«redacted-secret»"), # grep -A/-B/-C context gutter + (" 108\tADS_API_TOKEN: {tok}", "«redacted-secret»"), # cat -n / nl gutter (number + TAB) + (" 108\texport FOO_TOKEN={tok}", "«redacted-secret»"), ("GITHUB_TOKEN: ghp_S1abcdefghijklmnopqrstuvwxyz0Pn2T", "«redacted:ghp_…»"), # prefix label kept ]) def test_secret_file_masks_assignment_with_non_reusable_sentinel(self, template, sentinel): @@ -1272,9 +1276,17 @@ class TestSecretFileAssignmentRedaction: def test_unclassified_read_and_non_secret_scalars_are_untouched(self): for text in ("MAX_TOKENS: 100", '{"apiKey": "test"}', "api_key: test", f"5|ADS_API_TOKEN: {self.SYNTH}"): assert redact_sensitive_text(text, force=True, file_read=True) == text - out = redact_sensitive_text(f"ADS_API_TOKEN: {self.SYNTH}\n5|MAX_TOKENS: 100\n", force=True, - file_read=True, secret_file=True) - assert self.SYNTH not in out and "5|MAX_TOKENS: 100" in out + # Strong-key names whose value is a variable/path reference are shell-rc configuration, not + # secrets; the agent must still be able to read and edit them (password-class keys mask anyway). + rc = "export SSH_AUTH_SOCK=$HOME/.ssh/agent.sock\nexport DOCKER_AUTH_CONFIG=/home/u/.docker\n" + out = redact_sensitive_text(rc + f"ADS_API_TOKEN: {self.SYNTH}\n5|MAX_TOKENS: 100\nDB_PASSWORD=~/pw\n", + force=True, file_read=True, secret_file=True) + assert out.startswith(rc) and self.SYNTH not in out and "5|MAX_TOKENS: 100" in out and "~/pw" not in out + # The gutter-tolerant anchors must stay linear: a wide indented line is not a stall. + wide = "1|" + " " * 20000 + "token:" + started = time.perf_counter() + assert redact_sensitive_text(wide, force=True, file_read=True, secret_file=True) == wide + assert time.perf_counter() - started < 1.0 class TestHermesHomePathClassification: @@ -1292,6 +1304,7 @@ class TestHermesHomePathClassification: assert _is_secret_file_arg(str(home / "config.yaml")) assert _is_secret_file_arg(str(home / "profiles" / "coder" / "config.yaml")) assert _is_secret_file_arg(str(home / "backups" / "config" / "config.yaml.good.20260914-184559")) + assert not _is_secret_file_arg(str(home / "config.yaml.pdf")) # only the backups/config/ copies assert not _is_secret_file_arg(str(tmp_path / "proj" / "config.yaml")) assert not _is_secret_file_arg("config.yaml") # relative, not resolvable to the home