fix: redact .env terminal output via detection instead of known-env-var list
Terminal output from file-read commands (cat, head, tail, ...) uses code_file=True, which skips the generic ENV-assignment redaction pass. Reading a .env file through the terminal therefore leaked any key whose value has no recognized vendor prefix (Mistral, Gemini AQ.*, tvly-dev-, bu_, Spotify client secrets). Detect file-read commands targeting .env-style basenames (mirroring agent/file_safety's blocked list) and route them to code_file=False so the existing ENV pass masks opaque values. Templates (.env.example, .env.sample, ...) are excluded. Salvaged from #61352 (145 commits of drift; conflict with the test-prune wave resolved by NOT resurrecting pruned tests). Authored by @ShaoRou459. Closes #61352
This commit is contained in:
parent
83902620c8
commit
cf755f5c42
|
|
@ -130,7 +130,10 @@ _PREFIX_PATTERNS = [
|
|||
r"GR1348941[A-Za-z0-9_\-]{10,}", # GitLab legacy runner registration token
|
||||
]
|
||||
|
||||
# ENV assignment patterns: KEY=value where KEY contains a secret-like name.
|
||||
|
||||
|
||||
|
||||
# Generic ENV assignment patterns: KEY=value where KEY contains a secret-like name.
|
||||
# Uppercase keys tolerate spaces around "=" (e.g. ``FOO_SECRET = bar``) because
|
||||
# an all-caps key is almost never prose/code.
|
||||
_SECRET_ENV_NAMES = r"(?:API_?KEY|TOKEN|SECRET|PASSWORD|PASSWD|CREDENTIAL|AUTH)"
|
||||
|
|
@ -720,6 +723,7 @@ def redact_sensitive_text(
|
|||
_prefix_sub = _mask_token_nonreusable if file_read else _mask_token
|
||||
text = _PREFIX_RE.sub(lambda m: _prefix_sub(m.group(1)), text)
|
||||
|
||||
|
||||
# ENV assignments: OPENAI_API_KEY=*** (skip for code files — false positives)
|
||||
if not code_file:
|
||||
if "=" in text:
|
||||
|
|
@ -888,6 +892,65 @@ def redact_sensitive_text(
|
|||
# fixtures, ``postgresql://{user}`` f-string templates). See issue #43025.
|
||||
_ENV_DUMP_COMMANDS = frozenset({"env", "printenv", "set", "export", "declare"})
|
||||
|
||||
# Commands that read file contents to stdout. When the target is a ``.env``
|
||||
# file, the output is a credential dump — the same as ``printenv`` — so the
|
||||
# ENV-assignment pass must run (code_file=False). Per AGENTS.md, ``.env`` is
|
||||
# for secrets only; behavioral settings belong in config.yaml, so running
|
||||
# the generic ENV redactor on ``.env`` content is the correct behavior.
|
||||
_FILE_READ_COMMANDS = frozenset({
|
||||
"cat", "head", "tail", "type", "bat", "less", "more", "nl",
|
||||
"zcat", "tac", "view", "batcat",
|
||||
})
|
||||
|
||||
# Basenames that are treated as ``.env`` files for redaction purposes.
|
||||
# Matches the list in ``agent/file_safety._BLOCKED_PROJECT_ENV_BASENAMES``
|
||||
# so the two defenses stay aligned: if file_tools blocks a read, and the
|
||||
# agent falls back to ``cat``, the terminal redactor still catches it.
|
||||
_ENV_FILE_BASENAMES = frozenset({
|
||||
".env", ".env.local", ".env.development", ".env.production",
|
||||
".env.test", ".env.staging", ".envrc",
|
||||
})
|
||||
|
||||
# Filename suffixes that look like ``.env`` but are NOT secret-bearing
|
||||
# (templates, examples). These are explicitly excluded so the agent can
|
||||
# read template files without redaction interfering.
|
||||
_ENV_FILE_EXCLUDE_SUFFIXES = (".example", ".sample", ".template", ".dist")
|
||||
|
||||
|
||||
def _command_reads_env_file(command: str) -> bool:
|
||||
"""Return True if ``command`` reads a ``.env`` file to stdout.
|
||||
|
||||
Detects file-read commands (``cat``, ``head``, ``tail``, etc.) where any
|
||||
argument ends with a ``.env``-style basename and is NOT a template
|
||||
(``.env.example``, ``.env.sample``). Handles pipelines and sequences.
|
||||
"""
|
||||
if not command:
|
||||
return False
|
||||
segments = re.split(r"[|;&]+", command)
|
||||
for seg in segments:
|
||||
seg = seg.strip()
|
||||
if not seg:
|
||||
continue
|
||||
# Use plain split() instead of shlex.split — shlex treats backslashes
|
||||
# as escape chars, which mangles Windows paths (``C:\Users\...\.env``).
|
||||
# We only need the command name and filename, so shell quoting is not
|
||||
# a concern here.
|
||||
tokens = seg.split()
|
||||
if not tokens or tokens[0] not in _FILE_READ_COMMANDS:
|
||||
continue
|
||||
# Check all arguments (skip flags like -n, -A, etc.)
|
||||
for arg in tokens[1:]:
|
||||
if arg.startswith("-"):
|
||||
continue
|
||||
# Strip any leading path to get the basename. Handle both / and \.
|
||||
basename = arg.rsplit("/", 1)[-1].rsplit("\\", 1)[-1]
|
||||
# Exclude templates/examples.
|
||||
if any(basename.endswith(suf) for suf in _ENV_FILE_EXCLUDE_SUFFIXES):
|
||||
continue
|
||||
if basename in _ENV_FILE_BASENAMES:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def is_env_dump_command(command: str | None) -> bool:
|
||||
"""Return True if ``command`` dumps environment variables to stdout.
|
||||
|
|
@ -923,10 +986,14 @@ def redact_terminal_output(
|
|||
Single redaction policy for ALL terminal-output surfaces — foreground
|
||||
``terminal`` results AND background ``process(action=poll/log/wait)``
|
||||
output — so they can't diverge. Picks ``code_file`` based on whether
|
||||
``command`` is an environment dump:
|
||||
``command`` is an environment dump or reads a ``.env`` file:
|
||||
|
||||
- env-dump command (``env``/``printenv``/``set``/``export``/``declare``)
|
||||
→ ``code_file=False`` so the ENV-assignment pass masks opaque tokens.
|
||||
- file-read command targeting a ``.env`` file (``cat .env``,
|
||||
``head .env.local``, etc.) → ``code_file=False`` for the same reason.
|
||||
Per AGENTS.md, ``.env`` files contain only secrets, so the generic
|
||||
ENV redactor is the correct pass — no list of known var names needed.
|
||||
- anything else (or unknown command) → ``code_file=True`` to avoid
|
||||
false positives on source/config dumps.
|
||||
|
||||
|
|
@ -935,7 +1002,8 @@ def redact_terminal_output(
|
|||
"""
|
||||
if not output:
|
||||
return output
|
||||
code_file = not is_env_dump_command(command or "")
|
||||
cmd = command or ""
|
||||
code_file = not (is_env_dump_command(cmd) or _command_reads_env_file(cmd))
|
||||
return redact_sensitive_text(output, force=force, code_file=code_file)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -677,9 +677,10 @@ class TestTerminalOutputRedaction:
|
|||
"""is_env_dump_command + redact_terminal_output — issue #43025.
|
||||
|
||||
Terminal/process stdout must be redacted on every surface (foreground
|
||||
`terminal` AND background `process(poll/log/wait)`). Env-dump commands get
|
||||
the ENV-assignment pass so opaque tokens (no vendor prefix) are masked;
|
||||
other commands stay on the code_file path to avoid false positives.
|
||||
`terminal` AND background `process(poll/log/wait)`). Env-dump commands
|
||||
and commands that read ``.env`` files get the ENV-assignment pass so
|
||||
opaque tokens (no vendor prefix) are masked; other commands stay on
|
||||
the code_file path to avoid false positives.
|
||||
"""
|
||||
|
||||
def test_is_env_dump_command_detection(self):
|
||||
|
|
@ -697,6 +698,110 @@ class TestTerminalOutputRedaction:
|
|||
assert not is_env_dump_command("")
|
||||
assert not is_env_dump_command(None)
|
||||
|
||||
# ── .env file detection (issue #61352 v2) ──
|
||||
|
||||
def test_command_reads_env_file_detection(self):
|
||||
from agent.redact import _command_reads_env_file
|
||||
# Basic detection
|
||||
assert _command_reads_env_file("cat .env")
|
||||
assert _command_reads_env_file("cat .env.local")
|
||||
assert _command_reads_env_file("cat .env.production")
|
||||
assert _command_reads_env_file("cat .envrc")
|
||||
assert _command_reads_env_file("head .env")
|
||||
assert _command_reads_env_file("tail .env")
|
||||
assert _command_reads_env_file("type .env")
|
||||
assert _command_reads_env_file("nl .env")
|
||||
assert _command_reads_env_file("bat .env")
|
||||
# With flags
|
||||
assert _command_reads_env_file("cat -n .env")
|
||||
assert _command_reads_env_file("cat -A .env")
|
||||
# With paths
|
||||
assert _command_reads_env_file("cat ~/.hermes/.env")
|
||||
assert _command_reads_env_file("cat /home/user/project/.env")
|
||||
assert _command_reads_env_file("cat ./config/.env.local")
|
||||
# In a pipeline / sequence
|
||||
assert _command_reads_env_file("cat .env | grep KEY")
|
||||
assert _command_reads_env_file("echo '---' && cat .env")
|
||||
# Windows-style backslash paths
|
||||
assert _command_reads_env_file("cat C:\\Users\\test\\.env")
|
||||
|
||||
def test_command_reads_env_file_excludes_templates(self):
|
||||
from agent.redact import _command_reads_env_file
|
||||
# Templates/examples should NOT trigger
|
||||
assert not _command_reads_env_file("cat .env.example")
|
||||
assert not _command_reads_env_file("cat .env.sample")
|
||||
assert not _command_reads_env_file("cat .env.template")
|
||||
assert not _command_reads_env_file("cat .env.dist")
|
||||
|
||||
def test_command_reads_env_file_rejects_non_env_files(self):
|
||||
from agent.redact import _command_reads_env_file
|
||||
assert not _command_reads_env_file("cat config.py")
|
||||
assert not _command_reads_env_file("cat README.md")
|
||||
assert not _command_reads_env_file("cat .envrc.bak") # .bak not in list
|
||||
assert not _command_reads_env_file("python app.py")
|
||||
assert not _command_reads_env_file("echo .env") # echo is not a file-read cmd
|
||||
assert not _command_reads_env_file("")
|
||||
assert not _command_reads_env_file(None)
|
||||
|
||||
|
||||
def test_cat_env_file_masks_opaque_token(self):
|
||||
"""cat .env → code_file=False → generic ENV pass redacts opaque keys."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = (
|
||||
"MISTRAL_API_KEY=abc123opaqueSecretValue\n"
|
||||
"NOUS_API_KEY=xyz789opaqueKey\n"
|
||||
"DEBUG=true\n"
|
||||
)
|
||||
red = redact_terminal_output(out, "cat .env")
|
||||
assert "abc123opaqueSecretValue" not in red
|
||||
assert "xyz789opaqueKey" not in red
|
||||
assert "DEBUG=true" in red # non-secret key preserved
|
||||
|
||||
def test_cat_env_file_with_flags_masks_opaque_token(self):
|
||||
"""cat -n .env → still detected as .env read."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = " 1\tMISTRAL_API_KEY=abc123opaqueSecretValue\n"
|
||||
red = redact_terminal_output(out, "cat -n .env")
|
||||
assert "abc123opaqueSecretValue" not in red
|
||||
|
||||
def test_cat_env_file_in_pipeline_masks_opaque_token(self):
|
||||
"""cat .env | grep KEY → still detected as .env read."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = "MISTRAL_API_KEY=abc123opaqueSecretValue"
|
||||
red = redact_terminal_output(out, "cat .env | grep MISTRAL")
|
||||
assert "abc123opaqueSecretValue" not in red
|
||||
|
||||
def test_cat_env_example_not_redacted_as_env(self):
|
||||
"""cat .env.example → NOT treated as .env read (template file)."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = "MISTRAL_API_KEY=placeholder_value_here"
|
||||
red = redact_terminal_output(out, "cat .env.example")
|
||||
# Should NOT be redacted by the ENV-assignment pass (code_file=True).
|
||||
# The placeholder value should survive since it has no vendor prefix.
|
||||
assert "placeholder_value_here" in red
|
||||
|
||||
def test_cat_env_local_masks_opaque_token(self):
|
||||
"""cat .env.local → detected as .env read."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = "CUSTOM_API_KEY=opaquecustomkey123456"
|
||||
red = redact_terminal_output(out, "cat .env.local")
|
||||
assert "opaquecustomkey123456" not in red
|
||||
|
||||
def test_cat_env_inline_comment_preserved(self):
|
||||
"""Inline comments after env values are preserved (issue #61352 review)."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = "MISTRAL_API_KEY=abc123secret # used for tests"
|
||||
red = redact_terminal_output(out, "cat .env")
|
||||
assert "abc123secret" not in red
|
||||
assert "MISTRAL_API_KEY=*** # used for tests" in red
|
||||
|
||||
def test_cat_env_export_inline_comment_preserved(self):
|
||||
"""export KEY=VALUE # comment — comment preserved."""
|
||||
from agent.redact import redact_terminal_output
|
||||
out = "export MISTRAL_API_KEY=abc123secret # prod key"
|
||||
red = redact_terminal_output(out, "cat .env")
|
||||
assert "abc123secret" not in red
|
||||
assert "export MISTRAL_API_KEY=*** # prod key" in red
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue