Skip to content

fix: prevent lifecycle guard crashes on NUL paths - #79837

Closed
elmaynardo wants to merge 1 commit into
NousResearch:mainfrom
elmaynardo:fix/terminal-guard-nul-path
Closed

fix: prevent lifecycle guard crashes on NUL paths#79837
elmaynardo wants to merge 1 commit into
NousResearch:mainfrom
elmaynardo:fix/terminal-guard-nul-path

Conversation

@elmaynardo

Copy link
Copy Markdown

Summary

Prevents the terminal lifecycle guard from crashing when recursive referenced-script inspection reaches a path token containing an embedded NUL byte.

The observed trigger was a command that invoked a native binary by full path. Its decoded binary contents produced NUL-bearing tokens; os.open() raises ValueError for those tokens, while this recovery path previously caught only OSError.

Change

  • Catch ValueError alongside OSError in _read_referenced_script().
  • Preserve the existing safe fallback: unreadable or invalid candidate paths are not treated as scripts to scan.

Validation

  • Reproduced the failure with the local Matter Mach-O executable, then verified the guard returns normally after the change.
  • Ran python -m py_compile cron/lifecycle_guard.py.
  • Ran git diff --check.
  • Confirmed the existing regression test for an absolute-path native executable covers the no-crash behavior.

No configuration or public API changes.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management tool/terminal Terminal execution and process management duplicate This issue or pull request already exists labels Aug 6, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #78430. That earlier PR catches the embedded-NUL ValueError at os.open, whereas this patch catches after the descriptor would already need to be created.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists tool/terminal Terminal execution and process management type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants