diff --git a/cron/lifecycle_guard.py b/cron/lifecycle_guard.py index ba783c94969b6..ee73aa6639112 100644 --- a/cron/lifecycle_guard.py +++ b/cron/lifecycle_guard.py @@ -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( diff --git a/tests/hermes_cli/test_gateway_restart_loop.py b/tests/hermes_cli/test_gateway_restart_loop.py index 09682010b89bf..029dc4a9eb521 100644 --- a/tests/hermes_cli/test_gateway_restart_loop.py +++ b/tests/hermes_cli/test_gateway_restart_loop.py @@ -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)