fix(lifecycle-guard): close NUL-byte bypass in referenced-script scanning

[ruochu88s]

What does this PR do?

Closes a bypass in the lifecycle guard: a single NUL byte in a script path
makes the guard skip scanning a script that the shell will still execute.

_read_referenced_script() refuses to os.open() a path containing an embedded
NUL and reports "nothing to scan". That refusal is correct and was hardened
recently (#76762, #77703, and 9a9cf6a). But treating it as "nothing to scan" at
the call site is exploitable, because a POSIX shell drops NUL bytes from a
word:

bash danger\x00.sh      # guard sees an unopenable token -> skipped
                        # shell actually executes danger.sh

So any script the guard is meant to catch can be smuggled past it by inserting
one NUL byte into the path. This is the exploitable half of the same NUL class:
upstream fixed "the guard must not crash", this fixes "the guard must not be
silently bypassed".

Why this approach

When a referenced path cannot be read and contains a NUL, rescan the path the
shell would actually resolve (NUL bytes removed) before falling through to the
remote reader. The guard should inspect what will really run, not a token the
shell never uses verbatim.

The change is deliberately narrow to avoid new false positives:

• it only triggers when the read already failed and a NUL is present, so the
  common path is completely untouched;
• an all-NUL word strips to empty and is skipped rather than probing the cwd;
• a stripped path that does not exist stays "nothing to scan";
• the remote-reader fallback, visited set, and recursion depth limits are
  unchanged.

Stripping NULs earlier (in _iter_referenced_shell_scripts) was the alternative,
but that would silently rewrite tokens for every consumer of that iterator.
Handling it at the point of the failed read keeps the blast radius to exactly the
case that is currently exploitable.

Related Issue

Same class as #76762 / #77703, which addressed the crash. This addresses the
bypass that remains once the guard stops crashing:

• 9a9cf6a fix(cron): tolerate NUL bytes in referenced-script paths at os.open
  added the ValueError catch. I confirmed on current main that no
  NUL-stripping rescan exists, so the bypass is still open.

I'm treating this as security-relevant, so I've kept the reproduction in the
tests rather than writing an exploit into the issue tracker. Happy to move the
discussion wherever maintainers prefer.

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• 🔒 Security fix

Changes Made

• cron/lifecycle_guard.py
  • In _contains_unsafe_gateway_action(), when _read_referenced_script()
    returns no text and the path contains a NUL, retry the read against the
    NUL-stripped path (the form the shell resolves) and honour an unsafe
    verdict from it.
• tests/cron/test_lifecycle_guard_nul_path_bypass.py — new, 7 tests.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(lifecycle-guard):)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix (single commit)
• I've run the relevant suite and all tests pass (see below)
• I've added tests for my changes
• I've tested on my platform: Windows 10

tests/cron/test_lifecycle_guard_nul_path_bypass.py    7 passed, 0 failed

Test	Asserts
test_nul_in_path_does_not_bypass_the_guard	the smuggled bash danger\x00.sh form is detected
test_nul_in_source_directive_is_also_covered	same smuggling through source is detected
test_plain_unsafe_script_is_still_detected	ordinary detection is unchanged
test_safe_script_with_nul_is_not_flagged	no false positive from stripping
test_nul_path_with_no_real_file_is_not_flagged	nonexistent stripped path stays unscanned
test_all_nul_word_does_not_crash	all-NUL word strips to empty safely
test_guard_never_raises_on_nul_path	the guard returns a verdict, never propagates ValueError

I verified the fix is load-bearing: reverting the rescan turns exactly the
three security assertions red (nul_in_path, nul_in_source,
never_raises), while the false-positive guards stay green.

A note on the test style

The scripts are passed by name with cwd= rather than as absolute paths.
shlex treats backslashes as escapes, so a Windows absolute path is silently
mangled (C:\Users\... → C:Users...) and the resulting candidate never
resolves — which would make the assertions pass for the wrong reason. I hit this
while writing the tests; flagging it in case it's worth a note for other
Windows contributors.

Documentation & Housekeeping

• I've updated relevant documentation — N/A (rationale is an inline comment
  at the fix site)
• I've updated cli-config.yaml.example — N/A (no config keys)
• I've updated CONTRIBUTING.md or AGENTS.md — N/A
• I've considered cross-platform impact — the bypass is POSIX-shell
  semantics; the fix is pure string/path logic with no platform branches
• I've updated tool descriptions/schemas — N/A (guard verdicts unchanged for
  all non-NUL input)

[spfcraze]

This was generated by AI during triage.

Summary:
The all-NUL-word branch probes the cwd directory and marks it unsafe, so bash \x00\x00 returns True from the guard on POSIX and the PR's own test_all_nul_word_does_not_crash (which asserts False) fails on Linux/macOS.

Problems:

• cron/lifecycle_guard.py:176 joins the token with cwd before the new hunk strips NULs, so for bash \x00\x00 the stripped path is the cwd directory itself (non-empty), and if str(stripped): always passes — the description's "an all-NUL word strips to empty and is skipped" case requires a path that is not cwd-joined.
• cron/lifecycle_guard.py:270-271 returns (None, True) for any non-regular file, and os.open on a directory succeeds on POSIX, so the retry marks the cwd directory unsafe and the hunk returns True where the test asserts False.

Solution:
Strip the NUL from the raw token before _resolve_terminal_script_path joins it with cwd (line 176), so an all-NUL word genuinely strips to empty and is skipped; or gate the retry on stripped.is_file() so a directory result is skipped instead of treated as unsafe.

---

Checked against 0b84aa4 — the PR head when this was written — and 01a1037, main at the same moment.

[kshitijk4poor]

Closing as superseded by #80258, which fixes this whole bug class architecturally rather than per-callsite: path candidates are sanitized once at the ingestion boundary (NUL/empty/unexpandable tokens rejected before any OS call), text from any read_remote_script callback is sanitized at the recursion boundary (NUL = binary = nothing to scan; >1 MiB = fail closed), the remote fallback read is bounded at the source (head -c, so oversized binaries never cross the wire), and the public guard is total by construction — an unexpected walk failure logs and falls back to the direct-scan verdict instead of breaking every terminal command.

Your report and fix targeted a real member of this class — thank you. The per-callsite patches kept leaving sibling frames exposed (#76762 → #77703 → #77780 → #78256 each crashed one frame away from the previous fix), which is why we went with the boundary fix instead of merging the fragments individually. #80258 carries regression tests for the NUL-path, binary-callback, oversized-read, unset-HOME, and walk-crash cases plus an adversarial never-raises sweep.