fix(cron): lifecycle guard — never crash on binary referenced paths, stop matching lifecycle words inside SQL/text
Two live failures on the same guard (cron/lifecycle_guard.py), both of which blocked legitimate diagnostics from inside the gateway: 1. Crash class: the referenced-script walk read compiled binaries as if they were shell scripts. Reading/inspecting a referenced file is now best-effort by construction: executable magic numbers (ELF, PE, Mach-O fat/thin) short-circuit before any full read via a 4KB sniff, NUL-bearing heads are skipped as non-scripts, and unreadable paths of every kind (NUL bytes in the token, ENAMETOOLONG, missing files) degrade to "nothing to scan" instead of raising. A second fail-safe layer wraps the pure-string fallback so the boundary function stays total even if the tokenizer itself fails. 2. False-positive class: the lifecycle regex matched its command shapes inside DATA arguments — SQL string literals passed to sqlite3/psql and grep/rg/journalctl patterns hunting for the lifecycle string in logs. Added a fail-closed second-pass exemption: on a raw regex hit, re-scan with data-sink executables' arguments masked; only a match that survives (i.e. sits in command position) blocks. Masking is skipped for pipes into shells/xargs, command/process substitution, sqlite3 dot-commands and psql backslash escapes, so it can only ever allow, never miss. Behavioral tests: exact live false-positive shapes as negatives, the smuggling shapes as positives, the kill-primitive positive catalog unchanged, and an adversarial never-raises suite (NUL bytes, non-UTF-8, /dev/*, directories, missing files, magic-prefix binaries).
This commit is contained in:
Teknium
parent
863e313185
commit
70de958921
|
|
@ -109,6 +109,42 @@ _MAX_REFERENCED_SCRIPT_BYTES = 1024 * 1024
|
|||
_MAX_REFERENCED_SCRIPT_DEPTH = 8
|
||||
_CONTROL_CHARS = frozenset(";&|()")
|
||||
|
||||
# Executables whose arguments are DATA, not commands: search patterns, SQL
|
||||
# statements, log filters. None of these can execute their argument text, so
|
||||
# a lifecycle-shaped string inside their arguments (a grep pattern hunting
|
||||
# for `systemctl restart hermes-gateway` in syslog, a SQL LIKE literal over a
|
||||
# restart-events table) is diagnostics, not a lifecycle command. Deliberately
|
||||
# conservative: no `awk` (system()), no `sed` (`s///e`), no `echo`/`printf`
|
||||
# (routinely piped into a shell), no `mysql` (`\\!` and `system` escapes).
|
||||
_DATA_SINK_EXECUTABLES = frozenset(
|
||||
{"grep", "egrep", "fgrep", "rg", "ag", "ack", "journalctl", "sqlite3", "psql"}
|
||||
)
|
||||
# Argument shapes that can smuggle execution back INTO a data sink: command
|
||||
# and process substitution anywhere, sqlite3 dot-commands (`.shell ...`),
|
||||
# psql backslash escapes (`\! ...`). Any hit disables masking for the whole
|
||||
# segment — fail closed to the plain regex verdict.
|
||||
_UNSAFE_DATA_ARG_MARKERS = ("`", "$(", "<(", ">(", "\\!")
|
||||
# A data sink piped into a shell/interpreter can feed matched lines straight
|
||||
# to execution (`grep 'systemctl restart hermes-gateway' f | sh`); never mask
|
||||
# such a line.
|
||||
_PIPE_TO_INTERPRETER = re.compile(
|
||||
r"\|\s*&?\s*(?:sudo\s+)?(?:sh|bash|dash|ksh|zsh|xargs|eval|source)\b"
|
||||
)
|
||||
|
||||
# Executable-image magic numbers: ELF, PE/COFF, Mach-O (universal + thin,
|
||||
# both endiannesses). A referenced file starting with one of these is a
|
||||
# compiled binary, never a shell script — don't read or scan it at all.
|
||||
_BINARY_MAGIC_PREFIXES = (
|
||||
b"\x7fELF",
|
||||
b"MZ",
|
||||
b"\xca\xfe\xba\xbe",
|
||||
b"\xcf\xfa\xed\xfe",
|
||||
b"\xce\xfa\xed\xfe",
|
||||
b"\xfe\xed\xfa\xce",
|
||||
b"\xfe\xed\xfa\xcf",
|
||||
)
|
||||
_BINARY_SNIFF_BYTES = 4096
|
||||
|
||||
|
||||
|
||||
|
||||
|
|
@ -173,6 +209,102 @@ def contains_launchctl_submit_command(command: str) -> bool:
|
|||
return False
|
||||
|
||||
|
||||
def _mask_data_sink_arguments(text: str) -> str:
|
||||
"""Replace data-sink executables' arguments with a neutral placeholder.
|
||||
|
||||
The lifecycle regex is command-shaped, but it cannot tell an EXECUTED
|
||||
``systemctl restart hermes-gateway`` from the same characters appearing
|
||||
as *data* — a grep/rg pattern, a journalctl filter, a SQL string literal
|
||||
passed to sqlite3/psql. Those diagnostics commands were being rejected
|
||||
(false positives blocking legitimate cron prompts), e.g.::
|
||||
|
||||
grep -c 'systemctl restart hermes-gateway' /var/log/syslog
|
||||
sqlite3 db "SELECT msg FROM log WHERE msg LIKE '%systemctl restart hermes-gateway%'"
|
||||
|
||||
This masker shell-tokenizes each line and, for command segments whose
|
||||
executable is a known data sink (``_DATA_SINK_EXECUTABLES``), replaces
|
||||
every argument with ``arg``. The caller then re-runs the lifecycle regex
|
||||
on the masked text: a match that survives masking sits OUTSIDE any data
|
||||
argument and is a real command.
|
||||
|
||||
Strictly fail-closed: masking is skipped (leaving the original,
|
||||
regex-matching text in place) whenever the line pipes into a shell or
|
||||
interpreter, any argument carries an execution-capable marker
|
||||
(substitution, sqlite3 ``.``-commands, psql ``\\!``), or the line cannot
|
||||
be tokenized at all. Masking can therefore only ever ALLOW a command the
|
||||
plain regex would have blocked — never block one it would have allowed —
|
||||
so it runs solely as a second-pass exemption check.
|
||||
"""
|
||||
lines_out: list[str] = []
|
||||
changed = False
|
||||
for line in text.splitlines() or [text]:
|
||||
if _PIPE_TO_INTERPRETER.search(line):
|
||||
lines_out.append(line)
|
||||
continue
|
||||
try:
|
||||
lexer = shlex.shlex(line, posix=True, punctuation_chars=";&|()")
|
||||
lexer.whitespace_split = True
|
||||
lexer.commenters = "#"
|
||||
tokens = list(lexer)
|
||||
except ValueError:
|
||||
lines_out.append(line)
|
||||
continue
|
||||
|
||||
segments: list[list[str]] = []
|
||||
current: list[str] = []
|
||||
for token in tokens:
|
||||
if token and set(token) <= _CONTROL_CHARS:
|
||||
segments.append(current)
|
||||
segments.append([token])
|
||||
current = []
|
||||
continue
|
||||
current.append(token)
|
||||
segments.append(current)
|
||||
|
||||
rebuilt: list[str] = []
|
||||
for segment in segments:
|
||||
if not segment:
|
||||
continue
|
||||
index = _command_token_index(segment)
|
||||
if index is not None and Path(segment[index]).name in _DATA_SINK_EXECUTABLES:
|
||||
arguments = segment[index + 1 :]
|
||||
if not any(
|
||||
argument.startswith(".")
|
||||
or any(marker in argument for marker in _UNSAFE_DATA_ARG_MARKERS)
|
||||
for argument in arguments
|
||||
):
|
||||
changed = True
|
||||
rebuilt.extend(segment[: index + 1])
|
||||
rebuilt.extend("arg" for _ in arguments)
|
||||
continue
|
||||
rebuilt.extend(segment)
|
||||
lines_out.append(" ".join(rebuilt))
|
||||
if not changed:
|
||||
return text
|
||||
return "\n".join(lines_out)
|
||||
|
||||
|
||||
def _lifecycle_command_scan_with_data_exemption(text: str) -> bool:
|
||||
"""Lifecycle-regex scan that exempts matches living inside data arguments.
|
||||
|
||||
Two-pass: the cheap regex first (the overwhelmingly common no-match case
|
||||
pays nothing extra); on a raw match, re-scan with data-sink arguments
|
||||
masked out. Only a match that survives masking — i.e. one in actual
|
||||
command position — blocks.
|
||||
"""
|
||||
if not contains_gateway_lifecycle_command(text):
|
||||
return False
|
||||
normalized = _SHELL_LINE_CONTINUATION.sub(" ", text)
|
||||
return contains_gateway_lifecycle_command(_mask_data_sink_arguments(normalized))
|
||||
|
||||
|
||||
def _direct_lifecycle_scan(command: str) -> bool:
|
||||
"""Pure-string direct scans: lifecycle regex (data-exempted) + submit."""
|
||||
return _lifecycle_command_scan_with_data_exemption(
|
||||
command
|
||||
) or contains_launchctl_submit_command(command)
|
||||
|
||||
|
||||
def _expand_candidate_path(candidate: str) -> Optional[Path]:
|
||||
"""Sanitize a tokenized path candidate at the ingestion boundary.
|
||||
|
||||
|
|
@ -309,10 +441,23 @@ def _read_referenced_script(path: Path) -> tuple[Optional[str], bool]:
|
|||
metadata = os.fstat(descriptor)
|
||||
if not stat.S_ISREG(metadata.st_mode):
|
||||
return None, True
|
||||
# Read a bounded chunk first — even for oversized files, the first
|
||||
# chunk tells us if this is a binary (NUL bytes) that should be
|
||||
# skipped as "nothing to scan" rather than failing closed (#76762).
|
||||
data = os.read(descriptor, _MAX_REFERENCED_SCRIPT_BYTES + 1)
|
||||
# Sniff a small prefix first: files that are clearly compiled
|
||||
# binaries (executable magic, or NUL bytes in the head) are never
|
||||
# shell scripts, so skip them WITHOUT reading the rest — reading a
|
||||
# megabyte of machine code just to discard it wastes the guard's
|
||||
# budget and (pre-#77703) fed decoded garbage into the recursion.
|
||||
data = os.read(descriptor, _BINARY_SNIFF_BYTES)
|
||||
if data.startswith(_BINARY_MAGIC_PREFIXES) or b"\x00" in data:
|
||||
return None, False
|
||||
# Read the remainder (bounded). Loop because os.read may return
|
||||
# short for non-regular-file-backed descriptors.
|
||||
while len(data) <= _MAX_REFERENCED_SCRIPT_BYTES:
|
||||
chunk = os.read(
|
||||
descriptor, _MAX_REFERENCED_SCRIPT_BYTES + 1 - len(data)
|
||||
)
|
||||
if not chunk:
|
||||
break
|
||||
data += chunk
|
||||
except OSError:
|
||||
return None, False
|
||||
finally:
|
||||
|
|
@ -364,9 +509,7 @@ def _contains_unsafe_gateway_action(
|
|||
visited: set[Path],
|
||||
read_remote_script: Optional[_ReadRemoteScriptFn] = None,
|
||||
) -> bool:
|
||||
if contains_gateway_lifecycle_command(command) or contains_launchctl_submit_command(
|
||||
command
|
||||
):
|
||||
if _direct_lifecycle_scan(command):
|
||||
return True
|
||||
if depth >= _MAX_REFERENCED_SCRIPT_DEPTH:
|
||||
return True
|
||||
|
|
@ -458,9 +601,15 @@ def contains_gateway_lifecycle_command_or_referenced_script(
|
|||
exc_info=True,
|
||||
)
|
||||
# Pure string scans of the top-level command — cannot raise.
|
||||
return contains_gateway_lifecycle_command(
|
||||
command
|
||||
) or contains_launchctl_submit_command(command)
|
||||
try:
|
||||
return _direct_lifecycle_scan(command)
|
||||
except Exception:
|
||||
# The data-argument masker tokenizes arbitrary text; if even
|
||||
# that fails, fall to the raw regex + submit scan so the guard
|
||||
# stays total.
|
||||
return contains_gateway_lifecycle_command(
|
||||
command
|
||||
) or contains_launchctl_submit_command(command)
|
||||
|
||||
|
||||
|
||||
|
|
@ -548,7 +697,7 @@ def check_gateway_lifecycle(
|
|||
# `hermes gateway restart` embedded in a .py script is still
|
||||
# blocked. Non-regular/oversized script files still fail closed
|
||||
# via the lifecycle-shaped sentinel in _read_script_for_scanning.
|
||||
unsafe = contains_gateway_lifecycle_command(combined)
|
||||
unsafe = _lifecycle_command_scan_with_data_exemption(combined)
|
||||
else:
|
||||
script_dir = _resolve_script_directory(script) if script else None
|
||||
unsafe = contains_gateway_lifecycle_command_or_referenced_script(
|
||||
|
|
|
|||
|
|
@ -1043,3 +1043,144 @@ class TestCronCreateLifecycleBlockExtra:
|
|||
assert rc == 1
|
||||
out = capsys.readouterr().out
|
||||
assert "Blocked" in out
|
||||
|
||||
class TestLifecycleGuardDataArgumentExemption:
|
||||
"""Lifecycle words inside DATA arguments (SQL text, grep patterns) must
|
||||
not block; the same words in command position must. Reproduces the two
|
||||
live false positives (Aug 2026): a sqlite3 SELECT over restart-history
|
||||
text and a grep for the lifecycle string in syslog."""
|
||||
|
||||
def _scan(self, command, **kwargs):
|
||||
from cron.lifecycle_guard import (
|
||||
contains_gateway_lifecycle_command_or_referenced_script,
|
||||
)
|
||||
return contains_gateway_lifecycle_command_or_referenced_script(
|
||||
command, **kwargs
|
||||
)
|
||||
|
||||
@pytest.mark.parametrize("command", [
|
||||
# Exact live false-positive shapes: SQL string literals carrying the
|
||||
# full lifecycle command as text.
|
||||
'sqlite3 db "SELECT msg FROM log WHERE msg LIKE '
|
||||
"'%systemctl restart hermes-gateway%'\"",
|
||||
'psql -c "SELECT * FROM events WHERE cmd = '
|
||||
"'systemctl stop hermes-gateway'\"",
|
||||
# grep/rg pattern arguments hunting for the lifecycle string.
|
||||
"grep -c 'systemctl restart hermes-gateway' /var/log/syslog",
|
||||
"rg 'hermes gateway restart' /home/user/.hermes/logs/",
|
||||
"journalctl -u hermes-gateway --grep 'systemctl restart hermes-gateway'",
|
||||
# SQL with stop/restart column/value words but no command shape.
|
||||
'sqlite3 stats.db "SELECT stop_time, restart_reason FROM '
|
||||
'hermes_gateway_restarts"',
|
||||
"psql -c \"SELECT count(*) FROM events WHERE action IN "
|
||||
"('stop','restart') AND service LIKE '%gateway%'\"",
|
||||
])
|
||||
def test_data_argument_lifecycle_text_not_blocked(self, command):
|
||||
assert self._scan(command) is False
|
||||
|
||||
@pytest.mark.parametrize("command", [
|
||||
# Execution smuggled through or around a data sink must still block.
|
||||
'sqlite3 db ".shell hermes gateway restart"',
|
||||
'psql -c "\\! systemctl restart hermes-gateway"',
|
||||
"grep 'systemctl restart hermes-gateway' cmds.txt | sh",
|
||||
"grep gateway f | xargs systemctl restart hermes-gateway",
|
||||
'grep "$(systemctl restart hermes-gateway)" f',
|
||||
"grep 'restart' log; systemctl restart hermes-gateway",
|
||||
'sqlite3 db "SELECT 1"; hermes gateway stop',
|
||||
# Plain lifecycle commands are unaffected by the exemption.
|
||||
"hermes gateway restart",
|
||||
"sudo systemctl stop hermes-gateway",
|
||||
])
|
||||
def test_command_position_lifecycle_still_blocked(self, command):
|
||||
assert self._scan(command) is True
|
||||
|
||||
def test_python_script_branch_gets_the_same_exemption(self, tmp_path):
|
||||
"""check_gateway_lifecycle's .py branch scans the combined
|
||||
prompt+script text with the direct regex; a shell-shaped diagnostic
|
||||
command in the PROMPT (the live false-positive shape) must not block
|
||||
a job that runs a clean .py script. Note the exemption is
|
||||
fail-closed: the same SQL buried in non-shell-shaped Python source
|
||||
(e.g. inside a subprocess.run list literal) stays blocked because
|
||||
the masker cannot prove it is data."""
|
||||
from cron.lifecycle_guard import check_gateway_lifecycle
|
||||
script = tmp_path / "report.py"
|
||||
script.write_text("print('nightly report')\n", encoding="utf-8")
|
||||
prompt = (
|
||||
'sqlite3 db "SELECT msg FROM log '
|
||||
"WHERE msg LIKE '%systemctl restart hermes-gateway%'\""
|
||||
)
|
||||
check_gateway_lifecycle(prompt, str(script))
|
||||
|
||||
|
||||
class TestLifecycleGuardNeverRaises:
|
||||
"""The guard must return a verdict for every input — binary referenced
|
||||
paths, NUL bytes, non-UTF-8, /dev/* nodes, directories, missing files —
|
||||
never crash (the live 'ValueError: embedded null byte' class)."""
|
||||
|
||||
def _scan(self, command, **kwargs):
|
||||
from cron.lifecycle_guard import (
|
||||
contains_gateway_lifecycle_command_or_referenced_script,
|
||||
)
|
||||
return contains_gateway_lifecycle_command_or_referenced_script(
|
||||
command, **kwargs
|
||||
)
|
||||
|
||||
def test_command_referencing_elf_binary_returns_false(self, tmp_path):
|
||||
"""The exact live crash shape: a command referencing a compiled
|
||||
executable path (e.g. a venv python) must scan as 'nothing', not
|
||||
crash on the binary's decoded bytes."""
|
||||
binary = tmp_path / "python3.11"
|
||||
binary.write_bytes(b"\x7fELF\x02\x01\x01" + bytes(64) + b"\x90" * 256)
|
||||
assert self._scan(f"{binary} -m json.tool /tmp/x.json") is False
|
||||
|
||||
@pytest.mark.parametrize("command", [
|
||||
"run /tmp/foo\x00bar/baz.sh",
|
||||
"bash ./run\x00me.sh",
|
||||
"bash /nonexistent/deeply/missing.sh",
|
||||
"bash /" + "a" * 4096 + ".sh", # ENAMETOOLONG
|
||||
])
|
||||
def test_adversarial_paths_never_raise(self, command):
|
||||
assert self._scan(command, cwd="/tmp") is False
|
||||
|
||||
def test_non_utf8_referenced_file_never_raises(self, tmp_path):
|
||||
weird = tmp_path / "weird.sh"
|
||||
weird.write_bytes(b"\xff\xfe\x00\x01 not really a script")
|
||||
assert self._scan(f"bash {weird}") is False
|
||||
|
||||
def test_directory_and_dev_null_fail_closed_not_crash(self, tmp_path):
|
||||
# Non-regular files are suspicious (fail closed = blocked), but the
|
||||
# important contract is: verdict, not exception.
|
||||
assert self._scan(f"bash {tmp_path}") is True
|
||||
assert self._scan("bash /dev/null") is True
|
||||
|
||||
def test_magic_prefix_binaries_skipped_without_full_read(self, tmp_path):
|
||||
"""Executable magic (ELF/PE/Mach-O) short-circuits the read: the
|
||||
guard must not treat compiled binaries as scripts at all."""
|
||||
from cron.lifecycle_guard import _read_referenced_script
|
||||
for name, magic in [
|
||||
("elf", b"\x7fELF"),
|
||||
("pe", b"MZ"),
|
||||
("macho", b"\xcf\xfa\xed\xfe"),
|
||||
("fat", b"\xca\xfe\xba\xbe"),
|
||||
]:
|
||||
path = tmp_path / name
|
||||
# No NUL after the magic — proves the magic check itself fires.
|
||||
path.write_bytes(magic + b"ABCDEF" * 10)
|
||||
text, unsafe = _read_referenced_script(path)
|
||||
assert text is None, name
|
||||
assert unsafe is False, name
|
||||
|
||||
def test_check_gateway_lifecycle_adversarial_script_values(self, tmp_path):
|
||||
"""check_gateway_lifecycle must never raise anything but the
|
||||
documented GatewayLifecycleBlocked for junk script values."""
|
||||
from cron.lifecycle_guard import (
|
||||
GatewayLifecycleBlocked,
|
||||
check_gateway_lifecycle,
|
||||
)
|
||||
binary = tmp_path / "prog"
|
||||
binary.write_bytes(b"\x7fELF" + bytes(128))
|
||||
for value in ("nul\x00byte.sh", str(binary), "/nonexistent/x.sh"):
|
||||
check_gateway_lifecycle("clean prompt", value) # must not raise
|
||||
for value in ("/dev/null", str(tmp_path)):
|
||||
with pytest.raises(GatewayLifecycleBlocked):
|
||||
check_gateway_lifecycle("clean prompt", value)
|
||||
|
|
|
|||
Loading…
Reference in New Issue