hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

fix: linear gutter anchors, cat -n/grep-context gutters, keep rc path references readable

Browse Source 
The optional line-number gutter was written as `^(?:[ \t]*GUTTER)?[ \t]*`,
stacking two adjacent whitespace runs so _CFG_ANCHORED_RE / _YAML_ASSIGN_RE
went quadratic on any indented line with a secret keyword and no `=` (2s per
5k spaces, 50s at 20k) — and these run on every terminal output and file read.
The gutter now carries the single optional group and callers own the one
leading whitespace run. It also accepts `cat -n` / `nl` (number + TAB) and
`grep -A/-B/-C` (`5-`) gutters, which the comment claimed but leaked.

The strong-key branch masked SSH_AUTH_SOCK=$HOME/..., DOCKER_AUTH_CONFIG=/...
on shell rc reads, leaving the agent unable to edit them; a value starting with
$, / or ~ is a variable or path reference and is now kept unless the key is
password-class. _is_secret_file_arg only extends config.yaml to the
backups/config .good./.corrupt. copies, not config.yaml.pdf/.bak.

Review finding: quadratic gutter regex; cat -n/nl TAB and grep context gutters leaked; over-redaction of rc path references.
This commit is contained in:
teknium1
2026-09-14 21:28:28 -07:00
committed by Teknium
parent 6b6dd16de6
commit 979576d938
2 changed files with 37 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
agent/redact.py
View File

@@ -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.<stamp>`` / ``.corrupt.<stamp>`` 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)

21
tests/agent/test_redact.py
View File

@@ -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