From 74d3abb0e39c2de110281d0dd4c4f6982bbd85a9 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Fri, 2 Oct 2026 19:29:56 -0500 Subject: [PATCH 1/4] fix(guard): survive unlookable paths and judge each action under a deadline (#460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit decide() raised OSError (ENAMETOOLONG) on `cd x; ` repeated ~2000 times, and ValueError on a Write path holding a NUL: the walked cwd or target went to stat/resolve uncaught, breaking invariant 2. Such paths now degrade to an unknown repo (_is_worktree_root, _repo_root_for_action, _is_global_config_path). A `cd` chain stops at 4,096 chars (unknowable past PATH_MAX) and joins as text, so it is walked in linear time. Judging one action now runs under a cooperative wall-clock deadline (omind.deadline, 8 s inside the 15 s OMI guard hook timeout), checked at the walk, stage, rule and note-rule loop points. Past it the hard rules fail CLOSED ("command too large/complex to judge safely — split it or shorten it", rule omi-guard-deadline, event deadline-exceeded); the soft gates fail open. The hard rules always get a 3 s floor, so a slow note-rule lookup never turns a plain command into a deny. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011Zj1PCfsSSJgzLxi32jvde --- BACKLOG.md | 8 ++ CHANGELOG.md | 30 +++++++ pyproject.toml | 2 +- src/omind/__init__.py | 2 +- src/omind/compliance.py | 3 + src/omind/deadline.py | 116 ++++++++++++++++++++++++ src/omind/guard.py | 189 ++++++++++++++++++++++++++++++++++------ src/omind/rules.py | 8 +- tests/test_guard.py | 146 ++++++++++++++++++++++++++++++- uv.lock | 2 +- 10 files changed, 472 insertions(+), 34 deletions(-) create mode 100644 src/omind/deadline.py diff --git a/BACKLOG.md b/BACKLOG.md index bc59fe6..62baeb7 100644 --- a/BACKLOG.md +++ b/BACKLOG.md @@ -79,6 +79,14 @@ _Closed as not planned or rejected. Kept for the record; nothing here is pending ## Done +### Shipped — 2026-10-02 (v10.2.28) + +- [x] **guard: `decide()` raises OSError (ENAMETOOLONG) on long cd chains; add an overall fail-closed hook deadline** ([#460](https://github.com/CryptoJones/omind/issues/460)) — **v10.2.28** — _bug_ — + unlookable paths (too long, embedded NUL) degrade to an unknown repo; a `cd` chain is walked in + linear time; judging runs under an 8 s cooperative deadline past which the action is denied with + the #445 reason and a `deadline-exceeded` event; each `gh`/`git` fact lookup has its own timeout, + so a slow lookup leaves only that fact unknown. + ### Shipped — 2026-10-02 (v10.2.27) - [x] **guard hard rules: quoted wrapper switch values and unique-prefix long options hide sudo** ([#464](https://github.com/CryptoJones/omind/issues/464)) — **v10.2.27** — _bug_ — diff --git a/CHANGELOG.md b/CHANGELOG.md index f065cd0..f3d0b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,36 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [10.2.28] - 2026-10-02 + +### Fixed +- **The guard no longer raises on paths the filesystem cannot look up, and + judges each action under a deadline + ([#460](https://github.com/CryptoJones/omind/issues/460)).** `decide()` raised + `OSError` (`ENAMETOOLONG`) on `cd x; ` repeated about 2,000 times and on a long + `git -C` or Write path, and `ValueError` on a Write path holding a NUL: the walked + directory or target reached `stat`/`resolve` uncaught (invariant 2). Such a path + now degrades to an unknown repo. + - A `cd` chain stops growing past 4,096 characters (`PATH_MAX`; the directory is + unknowable there) and is joined as text, so `cd x; ` × 100,000 is walked in + linear time instead of quadratic. The note-rule view resolves and walks each + directory of a chain once per command, so `cd x; ` × 2,000 through + `check_action` with note rules takes about 1 s instead of 8. + - Judging one action runs under a cooperative wall-clock deadline + (`omind.deadline`, 8 s, inside the 15 s OMI guard hook timeout), checked at the + walk, stage, hard-rule and note-rule loop points. Past it, wherever it fires, + the action is denied with the #445 reason (`command too large/complex to judge + safely — split it or shorten it`, rule `omi-guard-deadline`) and a + `deadline-exceeded` event. Several gates after the hard rules are hard denies + too (global config, capability questions, repo work, operator note-rule + denies), so padding a command past the budget must not skip them. A hook + timeout used to return no verdict at all, which skipped every gate. + - Ordinary commands never get there: every `gh`/`git` fact lookup runs under its + own timeout (at most 3 s, and never into the 3 s left for judging), so a slow + lookup makes only that fact unknown. The hard rules always get at least 3 s + of their own. The steps after a verdict (the budget re-arm, the note excerpt, + the retrieval suggestion) run outside the deadline. + ## [10.2.27] - 2026-10-02 ### Fixed diff --git a/pyproject.toml b/pyproject.toml index f2c02ce..abad870 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "10.2.27" +version = "10.2.28" description = "Reproduce the OMI/Obsidian memory integration for AI agents, plus a local web app to view, edit, and add memory entries." readme = "README.md" requires-python = ">=3.10" diff --git a/src/omind/__init__.py b/src/omind/__init__.py index 6ffbb19..8983b85 100644 --- a/src/omind/__init__.py +++ b/src/omind/__init__.py @@ -2,4 +2,4 @@ # Copyright 2026 Aaron K. Clark """omind — OMI/Obsidian memory tooling for AI agents.""" -__version__ = "10.2.27" +__version__ = "10.2.28" diff --git a/src/omind/compliance.py b/src/omind/compliance.py index d87179a..9e5b79d 100644 --- a/src/omind/compliance.py +++ b/src/omind/compliance.py @@ -55,6 +55,9 @@ KIND_GATE_RESET = "gate-reset" #: A hard rule denied a command too costly to judge, not a real match (#445). KIND_BUDGET_EXCEEDED = "budget-exceeded" +#: Judging one action ran past the guard's wall-clock deadline (#460): the +#: hard rules denied it (``deny``) or the soft gates waved it on (``fail-open``). +KIND_DEADLINE_EXCEEDED = "deadline-exceeded" def compliance_log_path() -> Path: diff --git a/src/omind/deadline.py b/src/omind/deadline.py new file mode 100644 index 0000000..d779710 --- /dev/null +++ b/src/omind/deadline.py @@ -0,0 +1,116 @@ +"""A wall-clock budget for judging one action in the guard (#460). + +A hook the harness kills on its timeout returns no verdict at all, and a +missing verdict skips every gate. So judging one action runs under a deadline +well inside the hook timeout, and the guard decides what an exhausted budget +means: the hard rules fail CLOSED, the soft gates fail open. + +The deadline is cooperative: the walk, the stage loops and the rule loops call +:func:`check` at their loop points, and :func:`check` raises +:class:`DeadlineExceededError` once the budget is spent. ``signal.SIGALRM`` +would interrupt anything, but it is POSIX-only, fires only on the main +thread, and lands at an arbitrary bytecode; a watchdog thread cannot stop the +judging thread in Python at all. A check at each loop point works on every +platform and every thread, and always leaves the guard at a known point. + +The active deadline lives in a :class:`contextvars.ContextVar`, so concurrent +judges (threads, the web app) never share one. With no active deadline, +:func:`check` does nothing, so code that runs outside a judging scope (tests, +``omind rules`` tooling) behaves exactly as before. +""" + +from __future__ import annotations + +import contextlib +import contextvars +import time +from collections.abc import Iterator + +#: Seconds one action may spend being judged before the guard returns a +#: verdict anyway. The shortest hook timeout omind provisions for the OMI +#: guard is ``provision.OMI_GUARD_TIMEOUT`` (15 s); the budget leaves the +#: rest for the interpreter start (seconds under antivirus), the adapter's +#: rendering and the gate's retrieval suggestion. +JUDGE_BUDGET_SECONDS = 8.0 + +#: Seconds the hard rules may always take, however much of the budget an +#: earlier step spent (see :func:`at_least`). +HARD_RULE_FLOOR_SECONDS = 3.0 + + +class DeadlineExceededError(Exception): + """The active judging budget is spent (#460).""" + + +class Deadline: + """An absolute :func:`time.monotonic` instant past which judging stops.""" + + __slots__ = ("end",) + + def __init__(self, end: float) -> None: + self.end = end + + def expired(self) -> bool: + return time.monotonic() >= self.end + + +_ACTIVE: contextvars.ContextVar[Deadline | None] = contextvars.ContextVar( + "omind_judge_deadline", default=None +) + + +def active() -> Deadline | None: + """The deadline judging currently runs under, if any.""" + return _ACTIVE.get() + + +def expired() -> bool: + """Whether a deadline is active and spent.""" + current = _ACTIVE.get() + return current is not None and current.expired() + + +def check() -> None: + """Raise :class:`DeadlineExceededError` when the active deadline is spent. + A no-op outside a judging scope.""" + current = _ACTIVE.get() + if current is not None and time.monotonic() >= current.end: + raise DeadlineExceededError + + +@contextlib.contextmanager +def scope(seconds: float | None = None) -> Iterator[Deadline]: + """Judge under a deadline ``seconds`` from now (default + :data:`JUDGE_BUDGET_SECONDS`, read at call time so tests can shrink it). + An enclosing scope wins: a nested call keeps the outer deadline, so the + budget covers the whole action, not each step of it.""" + current = _ACTIVE.get() + if current is not None: + yield current + return + budget = JUDGE_BUDGET_SECONDS if seconds is None else seconds + created = Deadline(time.monotonic() + budget) + token = _ACTIVE.set(created) + try: + yield created + finally: + _ACTIVE.reset(token) + + +@contextlib.contextmanager +def at_least(seconds: float) -> Iterator[None]: + """Extend the active deadline so that at least ``seconds`` remain, for + the duration of the block. The hard rules run under this: they are + normally judged in milliseconds, so time an earlier, slower step spent + (a note rule's network lookup) must not turn a plain command into a + deadline deny. Outside a judging scope it opens one of ``seconds``.""" + current = _ACTIVE.get() + floor = time.monotonic() + seconds + if current is not None and current.end >= floor: + yield + return + token = _ACTIVE.set(Deadline(floor)) + try: + yield + finally: + _ACTIVE.reset(token) diff --git a/src/omind/guard.py b/src/omind/guard.py index f697a05..aa28e55 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -53,7 +53,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any, TextIO -from omind import compliance, filelock, paths, policy +from omind import compliance, deadline, filelock, paths, policy if TYPE_CHECKING: from omind import rules as rules_mod @@ -1531,10 +1531,11 @@ def _is_global_config_path(raw: str) -> bool: p = Path(raw).expanduser() if not p.is_absolute(): p = Path.cwd() / p - except (OSError, RuntimeError): + except (OSError, RuntimeError, ValueError): return False candidates = {p} - with contextlib.suppress(OSError): + # An embedded NUL makes resolve() raise ValueError (#460). + with contextlib.suppress(OSError, ValueError): candidates.add(p.resolve()) homes = {Path.home()} with contextlib.suppress(OSError): @@ -1668,7 +1669,20 @@ def _chdir(cwd: Path | None, step: str) -> Path | None: return None if target.is_absolute(): return target - return None if cwd is None else cwd / target + if cwd is None: + return None + # Joined as text, not with `/`: a Path built by `/` keeps every segment + # it was joined from, so a `cd x; cd x; …` chain re-joined all of them at + # each step (#460). + moved = Path(os.path.join(str(cwd), str(target))) + # Longer than the OS can name in one lookup: unknowable, and the chain + # stops growing here (#460). + return None if len(str(moved)) > _MAX_CWD_CHARS else moved + + +#: Longest directory path the walk keeps (#460): Linux's ``PATH_MAX``; macOS +#: allows 1,024 and Windows 260 unless long paths are enabled. +_MAX_CWD_CHARS = 4_096 #: A redirection operator alone (``>``, ``2>``, ``&>``, ``>&``): its target is @@ -1899,6 +1913,7 @@ def _shell_sites( sites: list[_ShellSite] = [] scopes: list[tuple[Path | None, tuple[Path | None, ...]]] = [] for pos, _kind, item in sorted(events, key=lambda e: (e[0], e[1])): + deadline.check() if item == "(": scopes.append((cwd, dirs)) continue @@ -2393,6 +2408,7 @@ def _enclosing_repo(candidates: list[Path]) -> Path | None: except OSError: cur = candidate.absolute() for parent in (cur, *cur.parents): + deadline.check() if _is_worktree_root(parent): return parent return None @@ -2405,7 +2421,12 @@ def _is_worktree_root(path: Path) -> bool: not turn every child path into a repository and demand an impossible freshness fetch.""" marker = path / ".git" - return marker.is_file() or (marker.is_dir() and (marker / "HEAD").is_file()) + try: + return marker.is_file() or (marker.is_dir() and (marker / "HEAD").is_file()) + except (OSError, ValueError): + # A path the filesystem cannot even look up (ENAMETOOLONG after a long + # `cd` chain, an embedded NUL) holds no worktree it can name (#460). + return False def _dir_repo(path: Path, memo: dict[Path, Path | None]) -> Path | None: @@ -2419,6 +2440,7 @@ def _dir_repo(path: Path, memo: dict[Path, Path | None]) -> Path | None: found = memo[parent] break visited.append(parent) + deadline.check() if _is_worktree_root(parent): found = parent break @@ -2431,8 +2453,17 @@ def _repo_root_for_action(action: dict[str, Any]) -> Path | None: candidates: list[Path] = [] raw_path = _action_path(action) if raw_path: - p = Path(raw_path).expanduser() - candidates.append(p if p.is_dir() else p.parent) + try: + p = Path(raw_path).expanduser() + except (OSError, RuntimeError, ValueError): + return None # an unknown `~user`: the action's repo is unknown + try: + is_dir = p.is_dir() + except (OSError, ValueError): + # Too long to look up, or an embedded NUL: not a directory. Its + # parents are walked as for any file (#460). + is_dir = False + candidates.append(p if is_dir else p.parent) else: base = _action_base_dir(action) # A Bash action carries no file path, so the repo was previously always @@ -3091,6 +3122,7 @@ def _site_write_repos( elif site.program in _EDITOR_SWITCHES: targets += _in_place_edit_targets(site.program, site.text) for word in targets: + deadline.check() key = (word, cwd) if key not in resolved: resolved[key] = _write_target(word, cwd) @@ -4150,6 +4182,7 @@ def _local_shell_subjects( for program, _args, pos, _end in stages: if program not in _LOCAL_SHELLS and program not in ("source", "."): continue + deadline.check() stop = ends[pos] try: tokens = _shell_tokens(text[pos:stop]) @@ -4265,7 +4298,8 @@ def _hard_rule_subjects(command: str) -> _HardSubjects: subjects through :func:`_local_code_texts`. Cached per command, so :func:`decide` searches once for the hard rules and both classifiers. - Pure and never raises: on any walk error only the command itself is + Pure, and raises only :class:`deadline.DeadlineExceededError` (#460), + which is never cached: on any other walk error only the command itself is judged, which is the pre-#430 behaviour. When the subject search fails partway, what it already found is kept (#449 review). @@ -4282,6 +4316,8 @@ def _subjects_of(command: str, *, expand: bool) -> _HardSubjects: ``expand``. Never raises.""" try: sites, _cwd, _local, bodies = _shell_walk(command) + except deadline.DeadlineExceededError: + raise except Exception: return _HardSubjects((command,)) code: list[str] = [command, *bodies] @@ -4301,12 +4337,16 @@ def _subjects_of(command: str, *, expand: bool) -> _HardSubjects: pass # keep what was already found: more text judged, never less try: for text in (command, *bodies): + deadline.check() _local_shell_subjects(text, code, words, runs) + except deadline.DeadlineExceededError: + raise except Exception: pass # keep what was already found: more text judged, never less quoted: list[str] = [] try: for site in sites: + deadline.check() if not site.opaque: continue if site.piped: @@ -4319,6 +4359,8 @@ def _subjects_of(command: str, *, expand: bool) -> _HardSubjects: if not has_body: continue # it reads stdin or a file: judged above quoted.append(re.sub(r"['\"]|\\n", "\n", site.text)) + except deadline.DeadlineExceededError: + raise except Exception: pass # likewise: keep the sites already read for expanded in expansions: @@ -4344,12 +4386,14 @@ def _hard_rule_hits(rule: policy.Rule, command: str, subjects: _HardSubjects) -> Raises :class:`policy.SearchBudgetExceededError` when nothing matched but a subject was too costly to judge (#445): the hard-rule callers deny that - with its own reason. May raise on a malformed rule; the callers skip that - rule.""" + with its own reason. Raises :class:`deadline.DeadlineExceededError` when + the judging budget runs out (#460). May raise on a malformed rule; the + callers skip that rule.""" over_budget = False def found(regex: re.Pattern[str] | None, text: str) -> bool: nonlocal over_budget + deadline.check() if regex is None: judged = rule.judge(text) elif rule.match == "command": @@ -4407,6 +4451,39 @@ def _log_budget_exceeded(rule: policy.Rule, command: str, session: str) -> None: ) +#: ``rule_id`` of the deny the hard rules return when judging one action ran +#: out of time (#460). +DEADLINE_RULE = "omi-guard-deadline" + + +def _log_deadline_exceeded( + command: str, session: str, *, outcome: str, severity: str, tool: str = "Bash" +) -> None: + """Record that judging one action ran out of time (#460): ``deny`` for + the hard rules, ``fail-open`` for the soft gates. Never raises.""" + with contextlib.suppress(Exception): + compliance.log_event( + compliance.KIND_DEADLINE_EXCEEDED, + session=session, + tool=tool, + command=command, + rule_id=DEADLINE_RULE, + severity=severity, + outcome=outcome, + ) + + +def _deadline_deny(command: str, session: str) -> Verdict: + """The hard rules' verdict when judging ``command`` ran out of time + (#460): they fail CLOSED, with the #445 budget reason.""" + _log_deadline_exceeded(command, session, outcome="deny", severity=policy.SEVERITY_HARD) + return Verdict( + allow=False, + reason=f"omi-guard (hard): {BUDGET_EXCEEDED_MESSAGE}", + rule_id=DEADLINE_RULE, + ) + + def _hard_policy_verdict(command: str, session: str = "") -> Verdict | None: """The deny for the first ``hard`` policy rule ``command`` matches, else None. @@ -4427,13 +4504,30 @@ def _hard_policy_verdict(command: str, session: str = "") -> Verdict | None: is denied with :data:`BUDGET_EXCEEDED_MESSAGE` and logged as a ``budget-exceeded`` event under ``session`` (#445): the hard rules fail CLOSED there, and only they do. + + Judging runs under the action's deadline, extended so at least + :data:`deadline.HARD_RULE_FLOOR_SECONDS` remain (#460). When it runs out + the hard rules fail CLOSED the same way, with :data:`DEADLINE_RULE` and a + ``deadline-exceeded`` event: no verdict at all (a hook timeout) would + skip every gate. """ + with deadline.at_least(deadline.HARD_RULE_FLOOR_SECONDS): + try: + return _judge_hard_rules(command, session) + except deadline.DeadlineExceededError: + return _deadline_deny(command, session) + + +def _judge_hard_rules(command: str, session: str) -> Verdict | None: + """:func:`_hard_policy_verdict` without its deadline handling. Raises + :class:`deadline.DeadlineExceededError` when the budget runs out.""" try: rules: list[policy.Rule] = list(policy.load_policy()) except Exception: rules = list(policy.SEED_RULES) subjects = _hard_rule_subjects(command) for rule in rules: + deadline.check() if rule.severity != policy.SEVERITY_HARD: continue # A single malformed rule must never brick the guard on EVERY tool call: @@ -4445,6 +4539,8 @@ def _hard_policy_verdict(command: str, session: str = "") -> Verdict | None: # the rules after it. try: hits = _hard_rule_hits(rule, command, subjects) + except deadline.DeadlineExceededError: + raise except policy.SearchBudgetExceededError: # Too costly to judge, and the rule's keyword is in it: a hard # rule fails CLOSED, with a reason that says why (#445). @@ -4472,31 +4568,59 @@ def decide(action: dict[str, Any], *, git_rules_missing: bool = False) -> Verdic """The harness-agnostic policy. See the module docstring for the schema. ``git_rules_missing`` (#358) waives ONLY the git-rules read demand, for a - vault that positively lacks the note; every other gate still runs.""" + vault that positively lacks the note; every other gate still runs. + + Judging runs under the action's wall-clock deadline (#460, see + :mod:`omind.deadline`), so a verdict comes back before the hook times + out. The hard rules fail CLOSED when it runs out + (:func:`_hard_policy_verdict`); every gate after them fails OPEN: the + action is allowed and a ``deadline-exceeded`` event says so.""" session = str(action.get("session") or "") command = str(action.get("command") or "") - repo = _repo_root_for_action(action) - # 1) Consulting OMI sets the per-turn sentinel and is always allowed. When - # the adapter knows what was consulted, record it (with target) so the - # verifier can judge relevance; otherwise just mark the gate consulted. - if action.get("is_omi_consult"): - target = str(action.get("consult_target") or "") - if target: - record_consult( + with deadline.scope(): + # 1) Consulting OMI sets the per-turn sentinel and is always allowed. When + # the adapter knows what was consulted, record it (with target) so the + # verifier can judge relevance; otherwise just mark the gate consulted. + if action.get("is_omi_consult"): + target = str(action.get("consult_target") or "") + if target: + record_consult( + session, + kind=str(action.get("consult_kind") or "consult"), + target=target, + tool=str(action.get("tool") or ""), + ) + else: + mark_consulted(session) + return Verdict(allow=True) + + # 2) Hard blocks — every ``hard`` rule in the data-driven policy. + hard = _hard_policy_verdict(command, session) + if hard is not None: + return hard + + try: + deadline.check() + return _soft_verdict(action, session, command, git_rules_missing) + except deadline.DeadlineExceededError: + _log_deadline_exceeded( + command or _action_path(action), session, - kind=str(action.get("consult_kind") or "consult"), - target=target, + outcome="fail-open", + severity=policy.SEVERITY_SOFT, tool=str(action.get("tool") or ""), ) - else: - mark_consulted(session) - return Verdict(allow=True) + return Verdict(allow=True) - # 2) Hard blocks — every ``hard`` rule in the data-driven policy. - hard = _hard_policy_verdict(command, session) - if hard is not None: - return hard + +def _soft_verdict( + action: dict[str, Any], session: str, command: str, git_rules_missing: bool +) -> Verdict: + """:func:`decide` past the hard rules: the repo, capability, global-config + and consult gates. Raises :class:`deadline.DeadlineExceededError` when the + judging budget runs out; :func:`decide` then fails open (#460).""" + repo = _repo_root_for_action(action) if repo is not None and _is_freshness_command(command): # Recorded optimistically here (PreToolUse cannot know the exit code); @@ -4767,7 +4891,16 @@ def check_action(action: dict[str, Any], omi_dir: Path | None = None) -> Verdict classifier fails OPEN via :func:`_fail_open_verdict`. A deny already decided before the exception still stands — only an undecided action is waved through. + + The whole check, note rules included, runs under one judging deadline + (#460); see :func:`decide` for what an exhausted one means. """ + with deadline.scope(): + return _check_action(action, omi_dir) + + +def _check_action(action: dict[str, Any], omi_dir: Path | None) -> Verdict: + """:func:`check_action` inside its deadline scope.""" verdict: Verdict | None = None try: verdict = _note_rules_verdict(action, omi_dir) diff --git a/src/omind/rules.py b/src/omind/rules.py index 0852128..3cc1f16 100644 --- a/src/omind/rules.py +++ b/src/omind/rules.py @@ -62,7 +62,7 @@ from pathlib import Path from typing import Any -from omind import filelock, paths, policy +from omind import deadline, filelock, paths, policy ACTION_DENY = "deny" ACTION_WARN = "warn" @@ -636,6 +636,7 @@ def _glob_matches(text: str, pattern: str) -> bool: def _rule_matches(text: str, pattern: str) -> bool: + deadline.check() # one call per site: a long chain stays inside the hook budget (#460) return _glob_matches(text, pattern) or _glob_matches(_canonical_git(text), pattern) @@ -832,6 +833,7 @@ def _judge( of pushes in one repo asks git for its branch once.""" if repo is None: return None + deadline.check() memo: dict[tuple[str, Path], Any] = {} if facts is None else facts def fact(name: str, fn: Any) -> Any: @@ -925,6 +927,9 @@ def evaluate( A git global option between ``git`` and its subcommand never hides a match (#414): ``*git push*`` matches ``git -C d push`` and ``git -c k=v push``, but not ``git log --grep push``. + + Raises :class:`deadline.DeadlineExceededError` when the guard's judging + budget runs out mid-evaluation; the guard then fails open (#460). """ tool = str(action.get("tool") or "") command = str(action.get("command") or "") @@ -934,6 +939,7 @@ def evaluate( code_texts: list[str] | None = None # per site, computed on first need facts: dict[tuple[str, Path], Any] = {} for rule in rules if rules is not None else load_rules(omi_dir): + deadline.check() if rule.invalid: continue if rule.tool not in ("*", tool): diff --git a/tests/test_guard.py b/tests/test_guard.py index 537e301..8c92701 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -2968,8 +2968,9 @@ def test_check_fail_open_still_honours_hard_policy_rules( monkeypatch: pytest.MonkeyPatch, ) -> None: # A crash in an unrelated classifier must not wave through a command a hard - # policy rule plainly names. - monkeypatch.setattr(guard, "_repo_root_for_action", _boom) + # policy rule plainly names. (The note rules run before the hard rules; + # since #460 the repo lookup runs after them, so it cannot pre-empt them.) + monkeypatch.setattr(guard, "_note_rules_verdict", _boom) verdict = guard.check_action({"tool": "Bash", "command": "sudo rm x", "session": "s420h"}) assert not verdict.allow assert verdict.rule_id == "sudo-use-fleet-sudo" @@ -5352,3 +5353,144 @@ def counting(text: str, *args: Any) -> None: assert guard._is_repo_sensitive_action(action) assert guard._is_commit_action(action) assert calls == [command] + + +# --- #460: filesystem errors on walked paths, and the judging deadline ------- + +_UNLOOKABLE_ACTIONS: list[dict[str, Any]] = [ + {"tool": "Bash", "command": "cd x; " * 2000 + "echo hi"}, + {"tool": "Bash", "command": "cd x; " * 2000 + "git commit -m x"}, + {"tool": "Bash", "command": "cd " + "a/" * 3000 + " && touch f"}, + {"tool": "Bash", "command": "git -C " + "a/" * 3000 + " commit -m x"}, + {"tool": "Write", "path": "/" + "a/" * 3000 + "f"}, + {"tool": "Write", "path": "/tmp/a\0b"}, +] + + +@pytest.mark.parametrize("action", _UNLOOKABLE_ACTIONS) +def test_paths_the_filesystem_cannot_look_up_do_not_raise( + action: dict[str, Any], tmp_path: Path +) -> None: + """#460: a walked cwd or a target too long to stat (ENAMETOOLONG) or + holding a NUL raised out of `decide()`. It degrades to an unknown repo.""" + verdict = guard.decide({**action, "session": "s460", "cwd": str(tmp_path)}) + assert verdict.rule_id != guard.DEADLINE_RULE + + +def test_a_long_cd_chain_is_walked_in_linear_time() -> None: + """#460: each `cd x` re-joined every segment before it (quadratic). Past + the longest path an OS can name, the directory is unknowable.""" + command = "cd x; " * 50_000 + "echo hi" + cold_shell_caches() + with _hard_time_limit(traced_bound(1.0)): + _sites, cwd, _local, _bodies = guard._shell_walk(command) + assert cwd is None + assert guard._chdir(Path("a"), "b") == Path("a/b") + + +def test_the_deadline_is_a_no_op_outside_a_scope() -> None: + """#460: the deadline only binds inside a judging scope; a nested scope + keeps the outer budget, and `at_least` gives the hard rules a floor.""" + from omind import deadline + + deadline.check() # no scope: never raises + with deadline.scope(0.0) as outer: + with deadline.scope(60.0) as inner: + assert inner is outer # the enclosing budget covers the action + with pytest.raises(deadline.DeadlineExceededError): + deadline.check() + with deadline.at_least(60.0): + deadline.check() # the hard rules' floor + with pytest.raises(deadline.DeadlineExceededError): + deadline.check() + deadline.check() + + +def test_the_judging_budget_fits_inside_the_hook_timeout() -> None: + """#460: budget plus the hard rules' floor leaves the interpreter start + and the adapter room inside the shortest OMI guard hook timeout.""" + from omind import deadline, provision + + worst = deadline.JUDGE_BUDGET_SECONDS + deadline.HARD_RULE_FLOOR_SECONDS + assert worst <= provision.OMI_GUARD_TIMEOUT - 3 + + +def _slow_subjects(monkeypatch: pytest.MonkeyPatch, seconds: float) -> None: + real = guard._local_shell_subjects + + def slow(text: str, *args: Any) -> None: + time.sleep(seconds) + real(text, *args) + + monkeypatch.setattr(guard, "_local_shell_subjects", slow) + + +def test_hard_rules_fail_closed_past_the_deadline(monkeypatch: pytest.MonkeyPatch) -> None: + """#460: a judge that runs past the deadline denies with the #445 reason + and logs `deadline-exceeded`, instead of the hook timing out (which + returns no verdict and skips every gate).""" + from omind import deadline + + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) + monkeypatch.setattr(deadline, "HARD_RULE_FLOOR_SECONDS", 0.05) + _slow_subjects(monkeypatch, 0.2) + command = "echo deadline-460-hard" + cold_shell_caches() + verdict = guard.decide({"tool": "Bash", "command": command, "session": "dl-hard"}) + assert not verdict.allow + assert verdict.rule_id == guard.DEADLINE_RULE + assert verdict.reason == f"omi-guard (hard): {guard.BUDGET_EXCEEDED_MESSAGE}" + events = [e for e in compliance.read_events() if e["kind"] == "deadline-exceeded"] + assert [(e["rule_id"], e["session"], e["outcome"]) for e in events] == [ + (guard.DEADLINE_RULE, "dl-hard", "deny") + ] + # A partial subject search is never cached: in time, it is judged in full. + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 60.0) + monkeypatch.setattr(deadline, "HARD_RULE_FLOOR_SECONDS", 60.0) + assert guard._hard_policy_verdict(command) is None + + +def test_soft_gates_fail_open_past_the_deadline(monkeypatch: pytest.MonkeyPatch) -> None: + """#460: once the hard rules have passed, an exhausted budget skips the + soft gates (here the consult gate, which would deny) and allows.""" + from omind import deadline + + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) + monkeypatch.setattr(deadline, "HARD_RULE_FLOOR_SECONDS", 30.0) + real = guard._repo_root_for_action + + def slow(action: dict[str, Any]) -> Path | None: + time.sleep(0.2) + return real(action) + + monkeypatch.setattr(guard, "_repo_root_for_action", slow) + action = {"tool": "Bash", "command": "echo deadline-460-soft", "session": "dl-soft"} + verdict = guard.check_action(action) + assert verdict.allow + events = [e for e in compliance.read_events() if e["kind"] == "deadline-exceeded"] + assert [(e["session"], e["outcome"], e["severity"]) for e in events] == [ + ("dl-soft", "fail-open", policy.SEVERITY_SOFT) + ] + # The hard rules still ran in full, so a real match is still denied. + sudo = guard.check_action({**action, "command": "sudo id"}) + assert not sudo.allow and sudo.rule_id == "sudo-use-fleet-sudo" + + +def test_a_slow_earlier_step_does_not_deny_a_plain_command( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """#460: the hard rules always get their floor, so a note rule's slow + network lookup spending the budget never turns `echo` into a deny.""" + from omind import deadline + + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) + monkeypatch.setattr(deadline, "HARD_RULE_FLOOR_SECONDS", 30.0) + + def slow_note_rules(action: dict[str, Any], omi_dir: Path | None) -> None: + time.sleep(0.2) + + monkeypatch.setattr(guard, "_note_rules_verdict", slow_note_rules) + base = {"tool": "Bash", "session": "dl-floor"} + assert guard.check_action({**base, "command": "echo hi"}).allow + sudo = guard.check_action({**base, "command": "sudo id"}) + assert not sudo.allow and sudo.rule_id == "sudo-use-fleet-sudo" diff --git a/uv.lock b/uv.lock index 8bafd05..d5b7972 100644 --- a/uv.lock +++ b/uv.lock @@ -2364,7 +2364,7 @@ wheels = [ [[package]] name = "omind" -version = "10.2.27" +version = "10.2.28" source = { editable = "." } dependencies = [ { name = "cryptography" }, From b7b7861db17268643b5d6ba42e1ccbe6165fbcb4 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Fri, 2 Oct 2026 20:16:56 -0500 Subject: [PATCH 2/4] fix(guard): fail closed past the judging deadline, bound each fact lookup (#460) EM review on #466: once the deadline fires anywhere in judging, decide() and check_action() deny with the explicit "command too large/complex to judge safely" reason and a deadline-exceeded event. Several gates after the hard rules are hard denies too (global config, capability questions, repo work, operator note-rule denies), and padding a command past the budget skipped all of them. - Every fact lookup (gh repo view, git ls-remote, git config ...) runs under its own timeout from deadline.lookup_timeout: at most 3 s, and never into the 3 s reserve left for judging. A slow lookup makes that fact unknown; it never spends the budget. - Handlers that failed open on any exception now re-raise the deadline: _note_rules_verdict, _git_dash_c_path, _local_code_texts, _rules_command_view, _bash_write_repo, the repo lookup in _repo_root_for_action, and CommandSite.write_repos. - _rules_command_view memoises resolution and the parent walk per command; _dir_repo walks parents lazily. `cd x; ` x 2000 through check_action with note rules: 8 s (deadline) -> about 1 s. - The steps after the verdict (budget re-arm, excerpt, retrieval suggestion) run outside the deadline; the hard-rule verdict is cached per check, so the fail-open re-check reuses it. - Removed the unused deadline.active(), deadline.expired() and Deadline.expired(). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011Zj1PCfsSSJgzLxi32jvde --- src/omind/deadline.py | 61 ++++++--- src/omind/guard.py | 287 ++++++++++++++++++++++++++-------------- src/omind/rules.py | 62 ++++----- tests/test_guard.py | 300 +++++++++++++++++++++++++++++++++++++++--- tests/test_rules.py | 4 +- 5 files changed, 542 insertions(+), 172 deletions(-) diff --git a/src/omind/deadline.py b/src/omind/deadline.py index d779710..004e8de 100644 --- a/src/omind/deadline.py +++ b/src/omind/deadline.py @@ -2,8 +2,15 @@ A hook the harness kills on its timeout returns no verdict at all, and a missing verdict skips every gate. So judging one action runs under a deadline -well inside the hook timeout, and the guard decides what an exhausted budget -means: the hard rules fail CLOSED, the soft gates fail open. +well inside the hook timeout, and an exhausted budget fails CLOSED: the guard +denies the action with an explicit "too large/complex to judge" reason. A +padded command must not slip past a hard gate by outrunning it. + +Ordinary commands never get there. Each fact lookup that waits on another +process or the network (``gh repo view``, ``git ls-remote``, ``git config``) +runs under its own timeout from :func:`lookup_timeout`, cut so that +:data:`LOOKUP_RESERVE_SECONDS` of the budget always remain for judging. A +lookup that times out makes that one fact unknown; it never spends the budget. The deadline is cooperative: the walk, the stage loops and the rule loops call :func:`check` at their loop points, and :func:`check` raises @@ -37,6 +44,14 @@ #: earlier step spent (see :func:`at_least`). HARD_RULE_FLOOR_SECONDS = 3.0 +#: Longest any one fact lookup (a ``gh``/``git`` subprocess) may take while +#: judging: well inside the budget. +LOOKUP_TIMEOUT_SECONDS = 3.0 + +#: Seconds of the budget fact lookups leave for judging itself, which takes +#: milliseconds on an ordinary command: lookups stop short of them. +LOOKUP_RESERVE_SECONDS = 3.0 + class DeadlineExceededError(Exception): """The active judging budget is spent (#460).""" @@ -50,26 +65,12 @@ class Deadline: def __init__(self, end: float) -> None: self.end = end - def expired(self) -> bool: - return time.monotonic() >= self.end - _ACTIVE: contextvars.ContextVar[Deadline | None] = contextvars.ContextVar( "omind_judge_deadline", default=None ) -def active() -> Deadline | None: - """The deadline judging currently runs under, if any.""" - return _ACTIVE.get() - - -def expired() -> bool: - """Whether a deadline is active and spent.""" - current = _ACTIVE.get() - return current is not None and current.expired() - - def check() -> None: """Raise :class:`DeadlineExceededError` when the active deadline is spent. A no-op outside a judging scope.""" @@ -78,6 +79,34 @@ def check() -> None: raise DeadlineExceededError +def lookup_timeout(cap: float) -> float: + """The timeout for one fact lookup whose own limit is ``cap`` seconds. + + Outside a judging scope, ``cap``. Inside one, at most + :data:`LOOKUP_TIMEOUT_SECONDS`, and never past the point where + :data:`LOOKUP_RESERVE_SECONDS` of the budget remain: a slow lookup then + times out, and only that fact is unknown. ``0.0`` when the reserve is + already reached; the caller treats the fact as unknown without asking.""" + current = _ACTIVE.get() + if current is None: + return cap + left = current.end - time.monotonic() - LOOKUP_RESERVE_SECONDS + return max(0.0, min(cap, LOOKUP_TIMEOUT_SECONDS, left)) + + +@contextlib.contextmanager +def suspended() -> Iterator[None]: + """Run the block with no deadline at all (:func:`check` a no-op). For the + steps after a verdict is reached (the gate's budget re-arm, a deny's + note excerpt, the retrieval suggestion): they have their own bounds, and + a deadline hit there is not a judging failure.""" + token = _ACTIVE.set(None) + try: + yield + finally: + _ACTIVE.reset(token) + + @contextlib.contextmanager def scope(seconds: float | None = None) -> Iterator[Deadline]: """Judge under a deadline ``seconds`` from now (default diff --git a/src/omind/guard.py b/src/omind/guard.py index aa28e55..a98e57a 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -40,7 +40,9 @@ import bisect import contextlib +import contextvars import functools +import itertools import json import os import re @@ -2266,6 +2268,8 @@ def _git_dash_c_path(command: str) -> Path | None: :func:`_rules_command_view`.""" try: sites, cwd, _local, _bodies = _shell_walk(command) + except deadline.DeadlineExceededError: + raise # never partial information: the guard denies (#460) except Exception: return None for site in sites: @@ -2395,11 +2399,20 @@ def _action_base_dir(action: dict[str, Any]) -> Path: return base -def _enclosing_repo(candidates: list[Path]) -> Path | None: - """The git worktree enclosing the first candidate that has one.""" +def _enclosing_repo( + candidates: list[Path], + memo: dict[Path, Path | None] | None = None, + resolved: dict[Path, tuple[Path, bool]] | None = None, +) -> Path | None: + """The git worktree enclosing the first candidate that has one. + + ``memo`` is :func:`_dir_repo`'s and ``resolved`` :func:`_resolve_dir`'s: + shared by every lookup of one command, a chain of ``cd`` steps resolves + and walks each directory once, not once per step (#460 review).""" + shared: dict[Path, Path | None] = {} if memo is None else memo for candidate in candidates: try: - cur = candidate.resolve() + cur = candidate.resolve() if resolved is None else _resolve_dir(candidate, resolved) except ValueError: # An embedded NUL (`cd /tm\0p`) names no directory; it raised # past every rule (#413 review 3). Fall back to the next candidate, @@ -2407,13 +2420,32 @@ def _enclosing_repo(candidates: list[Path]) -> Path | None: continue except OSError: cur = candidate.absolute() - for parent in (cur, *cur.parents): - deadline.check() - if _is_worktree_root(parent): - return parent + found = _dir_repo(cur, shared) + if found is not None: + return found return None +def _resolve_dir(path: Path, resolved: dict[Path, tuple[Path, bool]]) -> Path: + """``path.resolve()``, memoised in ``resolved`` with whether the result + exists (#460 review). Under a directory that does not exist, a child + resolves to that directory's resolution plus its name, as ``resolve()`` + itself does once a component cannot be looked up: a ``cd x; cd x; …`` + chain then resolves each step in one lookup instead of one per component + of an ever longer path. Raises as ``resolve()`` does.""" + if path in resolved: + return resolved[path][0] + above = resolved.get(path.parent) + if above is not None and not above[1] and path.name not in ("", ".."): + # Joined as text, as in `_chdir`: `/` would keep every segment. + result = (Path(os.path.join(str(above[0]), path.name)), False) + else: + found = path.resolve() + result = (found, os.path.lexists(found)) + resolved[path] = result + return result[0] + + def _is_worktree_root(path: Path) -> bool: """Whether ``path`` is the top of a git worktree. A real worktree has either a .git pointer file or a directory containing HEAD. Merely finding @@ -2435,7 +2467,9 @@ def _dir_repo(path: Path, memo: dict[Path, Path | None]) -> Path | None: shared by every target of one command, so each directory is checked once.""" visited: list[Path] = [] found: Path | None = None - for parent in (path, *path.parents): + # The parents lazily: listed up front, a deep directory built all of + # them before the memo could answer, once per `cd` step (#460 review). + for parent in itertools.chain((path,), path.parents): if parent in memo: found = memo[parent] break @@ -2470,10 +2504,14 @@ def _repo_root_for_action(action: dict[str, Any]) -> Path | None: # the shell's cwd, which misattributed `git -C fetch` (and # `git -C commit`) to the cwd repo (#147). Honor `-C` and # `cd` for git commands; a target outside any repo falls through to cwd. - with contextlib.suppress(Exception): + try: dash_c = _git_dash_c_path(_action_command(action)) if dash_c is not None: candidates.append(base / dash_c) + except deadline.DeadlineExceededError: + raise # never partial information: the guard denies (#460) + except Exception: + pass candidates.append(base) return _enclosing_repo(candidates) @@ -2494,10 +2532,12 @@ def _rules_command_view(action: dict[str, Any]) -> rules_mod.CommandView | None: base = _action_base_dir(action) sites, _cwd, local_text, _bodies = _shell_walk(command) repos: dict[Path | None, Path | None] = {} # 1,600 chained pushes share one cwd + walked: dict[Path, Path | None] = {} # each directory walked once (#460) + resolved: dict[Path, tuple[Path, bool]] = {} # and resolved once for site in sites: if site.cwd not in repos: cands = [base] if site.cwd is None else [base / site.cwd, base] - repos[site.cwd] = _enclosing_repo(cands) + repos[site.cwd] = _enclosing_repo(cands, walked, resolved) writers = _rules_write_finders(command, sites, base) return rules_mod.CommandView( local_text=local_text, @@ -2508,6 +2548,8 @@ def _rules_command_view(action: dict[str, Any]) -> rules_mod.CommandView | None: for site, writes in zip(sites, writers, strict=True) ), ) + except deadline.DeadlineExceededError: + raise # never partial information: the guard denies (#460) except Exception: return None @@ -2749,6 +2791,8 @@ def _local_code_texts(command: str) -> list[str]: fails partway, what it found is still classified.""" try: subjects = _hard_rule_subjects(command) + except deadline.DeadlineExceededError: + raise # never partial information: the guard denies (#460) except Exception: return [command] return [*subjects.code, *subjects.words, *subjects.runs] @@ -3096,6 +3140,8 @@ def _bash_write_repo(action: dict[str, Any]) -> Path | None: for site in _shell_walk(_without_comparisons(_action_command(action)))[0]: for found in _site_write_repos(site, base, memo, resolved): return found + except deadline.DeadlineExceededError: + raise # never partial information: the guard denies (#460) except Exception: return None return None @@ -4313,7 +4359,7 @@ def _hard_rule_subjects(command: str) -> _HardSubjects: def _subjects_of(command: str, *, expand: bool) -> _HardSubjects: """:func:`_hard_rule_subjects`, re-searching literal expansions when - ``expand``. Never raises.""" + ``expand``. Raises only :class:`deadline.DeadlineExceededError` (#460).""" try: sites, _cwd, _local, bodies = _shell_walk(command) except deadline.DeadlineExceededError: @@ -4456,11 +4502,12 @@ def _log_budget_exceeded(rule: policy.Rule, command: str, session: str) -> None: DEADLINE_RULE = "omi-guard-deadline" -def _log_deadline_exceeded( - command: str, session: str, *, outcome: str, severity: str, tool: str = "Bash" -) -> None: - """Record that judging one action ran out of time (#460): ``deny`` for - the hard rules, ``fail-open`` for the soft gates. Never raises.""" +def _deadline_deny(command: str, session: str, tool: str = "Bash") -> Verdict: + """The verdict when judging ``command`` ran out of time (#460): a deny, + with the #445 budget reason, logged as a ``deadline-exceeded`` event. + Judging fails CLOSED there wherever the deadline fires (note rules, hard + rules, soft gates): several later gates are hard denies too, and padding + a command until judging gives up must not get past any of them.""" with contextlib.suppress(Exception): compliance.log_event( compliance.KIND_DEADLINE_EXCEEDED, @@ -4468,15 +4515,9 @@ def _log_deadline_exceeded( tool=tool, command=command, rule_id=DEADLINE_RULE, - severity=severity, - outcome=outcome, + severity=policy.SEVERITY_HARD, + outcome="deny", ) - - -def _deadline_deny(command: str, session: str) -> Verdict: - """The hard rules' verdict when judging ``command`` ran out of time - (#460): they fail CLOSED, with the #445 budget reason.""" - _log_deadline_exceeded(command, session, outcome="deny", severity=policy.SEVERITY_HARD) return Verdict( allow=False, reason=f"omi-guard (hard): {BUDGET_EXCEEDED_MESSAGE}", @@ -4484,6 +4525,15 @@ def _deadline_deny(command: str, session: str) -> Verdict: ) +#: The hard-rule verdicts already reached in the current :func:`check_action`, +#: by ``(command, session)``: a re-decide, or :func:`_fail_open_verdict` after +#: a crash past them, reuses the verdict instead of judging (and spending a +#: second :data:`deadline.HARD_RULE_FLOOR_SECONDS`) again (#460 review). +_HARD_JUDGED: contextvars.ContextVar[dict[tuple[str, str], Verdict | None] | None] = ( + contextvars.ContextVar("omind_hard_judged", default=None) +) + + def _hard_policy_verdict(command: str, session: str = "") -> Verdict | None: """The deny for the first ``hard`` policy rule ``command`` matches, else None. @@ -4511,11 +4561,17 @@ def _hard_policy_verdict(command: str, session: str = "") -> Verdict | None: ``deadline-exceeded`` event: no verdict at all (a hook timeout) would skip every gate. """ + judged = _HARD_JUDGED.get() + if judged is not None and (command, session) in judged: + return judged[(command, session)] with deadline.at_least(deadline.HARD_RULE_FLOOR_SECONDS): try: - return _judge_hard_rules(command, session) + verdict = _judge_hard_rules(command, session) except deadline.DeadlineExceededError: - return _deadline_deny(command, session) + verdict = _deadline_deny(command, session) + if judged is not None: + judged[(command, session)] = verdict + return verdict def _judge_hard_rules(command: str, session: str) -> Verdict | None: @@ -4572,46 +4628,50 @@ def decide(action: dict[str, Any], *, git_rules_missing: bool = False) -> Verdic Judging runs under the action's wall-clock deadline (#460, see :mod:`omind.deadline`), so a verdict comes back before the hook times - out. The hard rules fail CLOSED when it runs out - (:func:`_hard_policy_verdict`); every gate after them fails OPEN: the - action is allowed and a ``deadline-exceeded`` event says so.""" + out. When it runs out anywhere, the action is denied with + :data:`DEADLINE_RULE` (:func:`_deadline_deny`): the gates after the hard + rules include hard denies too (global config, capability questions, + repo work). The hard rules keep a floor of + :data:`deadline.HARD_RULE_FLOOR_SECONDS`, and each fact lookup its own + timeout, so an ordinary command never gets there.""" session = str(action.get("session") or "") command = str(action.get("command") or "") with deadline.scope(): - # 1) Consulting OMI sets the per-turn sentinel and is always allowed. When - # the adapter knows what was consulted, record it (with target) so the - # verifier can judge relevance; otherwise just mark the gate consulted. - if action.get("is_omi_consult"): - target = str(action.get("consult_target") or "") - if target: - record_consult( - session, - kind=str(action.get("consult_kind") or "consult"), - target=target, - tool=str(action.get("tool") or ""), - ) - else: - mark_consulted(session) - return Verdict(allow=True) - - # 2) Hard blocks — every ``hard`` rule in the data-driven policy. - hard = _hard_policy_verdict(command, session) - if hard is not None: - return hard - try: - deadline.check() - return _soft_verdict(action, session, command, git_rules_missing) + return _decide(action, session, command, git_rules_missing) except deadline.DeadlineExceededError: - _log_deadline_exceeded( - command or _action_path(action), + return _deadline_deny( + command or _action_path(action), session, str(action.get("tool") or "") + ) + + +def _decide(action: dict[str, Any], session: str, command: str, git_rules_missing: bool) -> Verdict: + """:func:`decide` inside its deadline scope. Raises + :class:`deadline.DeadlineExceededError` when the budget runs out.""" + # 1) Consulting OMI sets the per-turn sentinel and is always allowed. When + # the adapter knows what was consulted, record it (with target) so the + # verifier can judge relevance; otherwise just mark the gate consulted. + if action.get("is_omi_consult"): + target = str(action.get("consult_target") or "") + if target: + record_consult( session, - outcome="fail-open", - severity=policy.SEVERITY_SOFT, + kind=str(action.get("consult_kind") or "consult"), + target=target, tool=str(action.get("tool") or ""), ) - return Verdict(allow=True) + else: + mark_consulted(session) + return Verdict(allow=True) + + # 2) Hard blocks — every ``hard`` rule in the data-driven policy. + hard = _hard_policy_verdict(command, session) + if hard is not None: + return hard + + deadline.check() + return _soft_verdict(action, session, command, git_rules_missing) def _soft_verdict( @@ -4619,7 +4679,7 @@ def _soft_verdict( ) -> Verdict: """:func:`decide` past the hard rules: the repo, capability, global-config and consult gates. Raises :class:`deadline.DeadlineExceededError` when the - judging budget runs out; :func:`decide` then fails open (#460).""" + judging budget runs out; :func:`decide` then denies (#460).""" repo = _repo_root_for_action(action) if repo is not None and _is_freshness_command(command): @@ -4725,7 +4785,9 @@ def _note_rules_verdict(action: dict[str, Any], omi_dir: Path | None) -> Verdict hit blocks with the rule's message (compliance-logged by the caller like any other hard deny); a ``warn`` or an unknown-visibility miss logs a decision event and falls through. Never raises — a broken rule table must - never brick the guard (fail-open like every other layer). + never brick the guard (fail-open like every other layer) — except + :class:`deadline.DeadlineExceededError`, which :func:`check_action` turns + into a deny (#460). """ if omi_dir is None: return None @@ -4758,6 +4820,8 @@ def _note_rules_verdict(action: dict[str, Any], omi_dir: Path | None) -> Verdict detail=(hit.detail or hit.rule.message)[:200], ) return None + except deadline.DeadlineExceededError: + raise # never partial information: the caller denies (#460) except Exception: return None @@ -4893,17 +4957,30 @@ def check_action(action: dict[str, Any], omi_dir: Path | None = None) -> Verdict waved through. The whole check, note rules included, runs under one judging deadline - (#460); see :func:`decide` for what an exhausted one means. + (#460), and an exhausted one denies, as in :func:`decide`. The steps after + the verdict (the gate's budget re-arm, the note excerpt, the retrieval + suggestion) run outside it: a deadline there is not a judging failure. """ - with deadline.scope(): - return _check_action(action, omi_dir) + token = _HARD_JUDGED.set({}) + try: + with deadline.scope(): + return _check_action(action, omi_dir) + finally: + _HARD_JUDGED.reset(token) def _check_action(action: dict[str, Any], omi_dir: Path | None) -> Verdict: """:func:`check_action` inside its deadline scope.""" verdict: Verdict | None = None try: - verdict = _note_rules_verdict(action, omi_dir) + try: + verdict = _note_rules_verdict(action, omi_dir) + except deadline.DeadlineExceededError: + verdict = _deadline_deny( + str(action.get("command") or "") or _action_path(action), + str(action.get("session") or ""), + str(action.get("tool") or ""), + ) if verdict is None: session = str(action.get("session") or "") # #358: once a turn has established the git-rules note is absent, the @@ -4934,49 +5011,55 @@ def _check_action(action: dict[str, Any], omi_dir: Path | None) -> Verdict: ) print(GIT_RULES_MISSING_MESSAGE, file=sys.stderr) verdict = decide(action, git_rules_missing=True) - if verdict.allow: - # #296: an allowed action still counts against the turn's budget, and at - # the budget the core may re-arm the gate around an unseen relevant note. - rearm = budget_verdict(action, omi_dir) - if rearm is not None: - verdict = rearm - if ( - not verdict.allow - and verdict.rule_id == "repo-work-read-git-rules" - and omi_dir is not None - ): - # #241: place the governing rule text adjacent to the action it blocks. - # The demand sentence stays first — the recall ceremony still runs and - # feeds consult telemetry — but the rule itself rides along, because an - # instruction next to the action wins attention that one injected 200 - # turns earlier has lost. - excerpt = _governing_excerpt(omi_dir, GIT_RULES_NOTE) - if excerpt: + # The verdict is reached: what follows (the budget re-arm, the deny's + # note excerpt and retrieval suggestion, the log) has its own bounds, + # and a deadline there is not a judging failure (#460 review). + with deadline.suspended(): + if verdict.allow: + # #296: an allowed action still counts against the turn's budget, and at + # the budget the core may re-arm the gate around an unseen relevant note. + rearm = budget_verdict(action, omi_dir) + if rearm is not None: + verdict = rearm + if ( + not verdict.allow + and verdict.rule_id == "repo-work-read-git-rules" + and omi_dir is not None + ): + # #241: place the governing rule text adjacent to the action it blocks. + # The demand sentence stays first — the recall ceremony still runs and + # feeds consult telemetry — but the rule itself rides along, because an + # instruction next to the action wins attention that one injected 200 + # turns earlier has lost. + excerpt = _governing_excerpt(omi_dir, GIT_RULES_NOTE) + if excerpt: + verdict = Verdict( + allow=False, + reason=( + f"{verdict.reason}\n\n--- Governing memory (excerpt) ---\n{excerpt}" + ), + rule_id=verdict.rule_id, + ) + if not verdict.allow and verdict.rule_id == "omi-gate" and omi_dir is not None: + from omind import retrieve + + session = str(action.get("session") or "") verdict = Verdict( allow=False, - reason=(f"{verdict.reason}\n\n--- Governing memory (excerpt) ---\n{excerpt}"), + reason=f"omi-gate: {retrieve.suggest_message(turn_task(session), omi_dir)}", rule_id=verdict.rule_id, ) - if not verdict.allow and verdict.rule_id == "omi-gate" and omi_dir is not None: - from omind import retrieve - - session = str(action.get("session") or "") - verdict = Verdict( - allow=False, - reason=f"omi-gate: {retrieve.suggest_message(turn_task(session), omi_dir)}", - rule_id=verdict.rule_id, - ) - if not verdict.allow and verdict.rule_id and not verdict.rule_id.startswith("omi-gate"): - compliance.log_event( - compliance.KIND_DECISION, - session=str(action.get("session") or ""), - tool=str(action.get("tool") or ""), - command=str(action.get("command") or ""), - rule_id=verdict.rule_id, - severity=policy.SEVERITY_HARD, - outcome="deny", - ) - return verdict + if not verdict.allow and verdict.rule_id and not verdict.rule_id.startswith("omi-gate"): + compliance.log_event( + compliance.KIND_DECISION, + session=str(action.get("session") or ""), + tool=str(action.get("tool") or ""), + command=str(action.get("command") or ""), + rule_id=verdict.rule_id, + severity=policy.SEVERITY_HARD, + outcome="deny", + ) + return verdict except Exception as exc: return _fail_open_verdict(action, exc, verdict) diff --git a/src/omind/rules.py b/src/omind/rules.py index 3cc1f16..e6dcd4d 100644 --- a/src/omind/rules.py +++ b/src/omind/rules.py @@ -244,6 +244,21 @@ def _visibility_cache_path() -> Path: return paths.state_dir() / "repo-visibility.json" +def _lookup( + args: list[str], cap: float, cwd: Path | None = None +) -> subprocess.CompletedProcess[str]: + """``subprocess.run`` for one fact lookup, under its own timeout (#460): + ``cap`` outside the guard, cut by :func:`deadline.lookup_timeout` while it + judges, so a slow ``gh``/``git`` leaves that fact unknown instead of + spending the judging budget. Raises :class:`subprocess.TimeoutExpired` + without starting the process when no lookup time is left; every caller + already treats that as an unknown fact.""" + timeout = deadline.lookup_timeout(cap) + if timeout <= 0: + raise subprocess.TimeoutExpired(args, 0) + return subprocess.run(args, cwd=cwd, capture_output=True, text=True, timeout=timeout) + + def _has_github_remote(repo: Path) -> bool: """True if ``repo`` has any git remote pointing at github.com. @@ -252,12 +267,7 @@ def _has_github_remote(repo: Path) -> bool: over SSH). The latter must not be logged as a failure — it is expected. """ try: - proc = subprocess.run( - ["git", "-C", str(repo), "remote", "-v"], - capture_output=True, - text=True, - timeout=5, - ) + proc = _lookup(["git", "-C", str(repo), "remote", "-v"], 5) except (OSError, subprocess.SubprocessError): return False if proc.returncode != 0: @@ -310,12 +320,8 @@ def _repo_visibility(repo: Path, *, now: datetime | None = None) -> str: except (OSError, ValueError, TypeError): cache = cache if isinstance(cache, dict) else {} try: - proc = subprocess.run( - ["gh", "repo", "view", "--json", "visibility", "-q", ".visibility"], - cwd=repo, - capture_output=True, - text=True, - timeout=10, + proc = _lookup( + ["gh", "repo", "view", "--json", "visibility", "-q", ".visibility"], 10, cwd=repo ) visibility = proc.stdout.strip().lower() if proc.returncode == 0 else "" except (OSError, subprocess.SubprocessError): @@ -353,12 +359,7 @@ def _repo_visibility(repo: Path, *, now: datetime | None = None) -> str: def _repo_name(repo: Path) -> str: try: - proc = subprocess.run( - ["git", "-C", str(repo), "remote", "get-url", "origin"], - capture_output=True, - text=True, - timeout=5, - ) + proc = _lookup(["git", "-C", str(repo), "remote", "get-url", "origin"], 5) url = proc.stdout.strip() except (OSError, subprocess.SubprocessError): return "" @@ -380,12 +381,7 @@ def _remote_has_commits(repo: Path) -> bool | None: keep granting the exemption after the first commit landed. """ try: - proc = subprocess.run( - ["git", "-C", str(repo), "ls-remote", "--heads", "origin"], - capture_output=True, - text=True, - timeout=10, - ) + proc = _lookup(["git", "-C", str(repo), "ls-remote", "--heads", "origin"], 10) except (OSError, subprocess.SubprocessError): _breadcrumb(f"rules_has_commits({repo})", "ls-remote failed") return None @@ -398,12 +394,7 @@ def _remote_has_commits(repo: Path) -> bool | None: def _repo_branch(repo: Path) -> str: try: - proc = subprocess.run( - ["git", "-C", str(repo), "rev-parse", "--abbrev-ref", "HEAD"], - capture_output=True, - text=True, - timeout=5, - ) + proc = _lookup(["git", "-C", str(repo), "rev-parse", "--abbrev-ref", "HEAD"], 5) return proc.stdout.strip() except (OSError, subprocess.SubprocessError): return "" @@ -415,12 +406,7 @@ def _git_out(repo: Path, *args: str, config: tuple[str, ...] = ()) -> str | None filtered by :func:`_cmdline_config`, so a read sees what the push will.""" flags = [f for kv in config for f in ("-c", kv)] try: - proc = subprocess.run( - ["git", *flags, "-C", str(repo), *args], - capture_output=True, - text=True, - timeout=5, - ) + proc = _lookup(["git", *flags, "-C", str(repo), *args], 5) except (OSError, subprocess.SubprocessError): return None return proc.stdout if proc.returncode == 0 else None @@ -807,6 +793,8 @@ def write_repos(self) -> tuple[Path, ...]: return () try: return tuple(self.writes()) + except deadline.DeadlineExceededError: + raise # never partial information: the guard denies (#460) except Exception: return () @@ -929,7 +917,7 @@ def evaluate( ``git -c k=v push``, but not ``git log --grep push``. Raises :class:`deadline.DeadlineExceededError` when the guard's judging - budget runs out mid-evaluation; the guard then fails open (#460). + budget runs out mid-evaluation; the guard then denies (#460). """ tool = str(action.get("tool") or "") command = str(action.get("command") or "") diff --git a/tests/test_guard.py b/tests/test_guard.py index 8c92701..d9d9eb9 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -5450,37 +5450,189 @@ def test_hard_rules_fail_closed_past_the_deadline(monkeypatch: pytest.MonkeyPatc assert guard._hard_policy_verdict(command) is None -def test_soft_gates_fail_open_past_the_deadline(monkeypatch: pytest.MonkeyPatch) -> None: - """#460: once the hard rules have passed, an exhausted budget skips the - soft gates (here the consult gate, which would deny) and allows.""" +def _deadline_events() -> list[tuple[str, str, str, str]]: + return [ + (e["rule_id"], e["session"], e["outcome"], e["severity"]) + for e in compliance.read_events() + if e["kind"] == "deadline-exceeded" + ] + + +def _assert_deadline_deny(verdict: guard.Verdict, session: str) -> None: + assert not verdict.allow + assert verdict.rule_id == guard.DEADLINE_RULE + assert verdict.reason == f"omi-guard (hard): {guard.BUDGET_EXCEEDED_MESSAGE}" + assert _deadline_events() == [(guard.DEADLINE_RULE, session, "deny", policy.SEVERITY_HARD)] + + +def test_soft_gates_fail_closed_past_the_deadline( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """#460 review: several gates after the hard rules are hard denies too. + A global-config write padded until judging passes the deadline is denied + with the deadline reason, not waved through the global-config gate.""" from omind import deadline + command = "cd x; " * 2000 + "echo '{}' > ~/.claude/settings.json" + action = {"tool": "Bash", "command": command, "session": "dl-soft", "cwd": str(tmp_path)} + guard.begin_turn("dl-soft", "What does the guard do?") + in_time = guard.decide(action) + assert not in_time.allow and in_time.rule_id == "global-config-explicit-auth" + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) monkeypatch.setattr(deadline, "HARD_RULE_FLOOR_SECONDS", 30.0) real = guard._repo_root_for_action def slow(action: dict[str, Any]) -> Path | None: - time.sleep(0.2) + time.sleep(0.2) # the padding's cost, past the budget return real(action) monkeypatch.setattr(guard, "_repo_root_for_action", slow) - action = {"tool": "Bash", "command": "echo deadline-460-soft", "session": "dl-soft"} - verdict = guard.check_action(action) - assert verdict.allow - events = [e for e in compliance.read_events() if e["kind"] == "deadline-exceeded"] - assert [(e["session"], e["outcome"], e["severity"]) for e in events] == [ - ("dl-soft", "fail-open", policy.SEVERITY_SOFT) - ] - # The hard rules still ran in full, so a real match is still denied. + _assert_deadline_deny(guard.check_action(action), "dl-soft") + # The hard rules kept their floor, so a real match still names its rule. sudo = guard.check_action({**action, "command": "sudo id"}) assert not sudo.allow and sudo.rule_id == "sudo-use-fleet-sudo" -def test_a_slow_earlier_step_does_not_deny_a_plain_command( +def test_note_rules_fail_closed_past_the_deadline( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """#460 review: `_note_rules_verdict` swallowed the deadline as "no rule + matched", so a padded command skipped every operator deny rule. It now + denies with the deadline reason and event, before the hard rules run.""" + from omind import deadline + + omi = tmp_path / "OMI" + omi.mkdir() + (omi / "Guard Rules.md").write_text( + '# Guard Rules\n\n```omind-rule\nid: no-deploy\ntool: Bash\nmatch: "*deploy*"\n' + 'action: deny\nmessage: "m"\n```\n', + encoding="utf-8", + ) + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) + real = guard._rules_command_view + + def slow(action: dict[str, Any]) -> Any: + time.sleep(0.2) + return real(action) + + monkeypatch.setattr(guard, "_rules_command_view", slow) + hard = [] + monkeypatch.setattr(guard, "_judge_hard_rules", lambda *a: hard.append(a)) + action = {"tool": "Bash", "command": "echo hi", "session": "dl-note", "cwd": str(tmp_path)} + _assert_deadline_deny(guard.check_action(action, omi), "dl-note") + assert hard == [] # denied in the note-rule phase itself + + +@pytest.mark.parametrize( + ("target", "patched"), + [ + (lambda: guard._git_dash_c_path("git status"), "_shell_walk"), + (lambda: guard._local_code_texts("git status"), "_hard_rule_subjects"), + (lambda: guard._rules_command_view({"command": "git status"}), "_shell_walk"), + (lambda: guard._repo_root_for_action({"command": "git status"}), "_git_dash_c_path"), + (lambda: guard._bash_write_repo({"command": "echo > f"}), "_shell_walk"), + (lambda: guard._note_rules_verdict({"command": "x"}, Path(".")), "_rules_command_view"), + ], +) +def test_handlers_never_turn_the_deadline_into_partial_information( + monkeypatch: pytest.MonkeyPatch, target: Callable[[], object], patched: str +) -> None: + """#460 review: every fail-open handler on the judging path re-raises the + deadline to `decide()`/`check_action()`, which deny, instead of answering + "no repo", "no rule", or "only the raw command".""" + from omind import deadline + + def expired(*_a: object, **_k: object) -> None: + raise deadline.DeadlineExceededError + + monkeypatch.setattr(guard, patched, expired) + with pytest.raises(deadline.DeadlineExceededError): + target() + + +def test_a_command_sites_write_repos_re_raise_the_deadline() -> None: + """#460 review: `CommandSite.write_repos` fails open to `()` on errors, + but not on the deadline.""" + from omind import deadline, rules + + def expired() -> tuple[Path, ...]: + raise deadline.DeadlineExceededError + + def broken() -> tuple[Path, ...]: + raise OSError("unreadable") + + site = rules.CommandSite(text="cat > f", repo=None, writes=expired) + with pytest.raises(deadline.DeadlineExceededError): + _ = site.write_repos + assert rules.CommandSite(text="cat > f", repo=None, writes=broken).write_repos == () + + +def test_a_slow_gh_lookup_leaves_only_that_fact_unknown( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """#460 review: a fact lookup that hangs times out on its own bound, well + inside the budget: visibility becomes unknown (the rule logs it and does + not fire) and the plain command is allowed, not denied by the deadline.""" + from omind import deadline, rules + + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 2.0) + monkeypatch.setattr(deadline, "LOOKUP_TIMEOUT_SECONDS", 0.2) + monkeypatch.setattr(deadline, "LOOKUP_RESERVE_SECONDS", 1.0) + repo = tmp_path / "repo" + (repo / ".git").mkdir(parents=True) + (repo / ".git" / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8") + omi = tmp_path / "OMI" + omi.mkdir() + (omi / "Guard Rules.md").write_text( + '# Guard Rules\n\n```omind-rule\nid: public-deploy\ntool: Bash\nmatch: "*deploy*"\n' + 'when:\n repo_visibility: public\naction: deny\nmessage: "m"\n```\n', + encoding="utf-8", + ) + timeouts: list[float] = [] + + def fake_run(args: list[str], **kwargs: Any) -> subprocess.CompletedProcess[str]: + if args[0] == "gh": + timeouts.append(kwargs["timeout"]) + time.sleep(kwargs["timeout"] + 0.05) # past its own timeout + raise subprocess.TimeoutExpired(args, kwargs["timeout"]) + # `git remote -v`: a GitHub remote, so the failed lookup is unknown. + return subprocess.CompletedProcess(args, 0, "origin\tgit@github.com:o/r.git (push)\n", "") + + monkeypatch.setattr(rules.subprocess, "run", fake_run) + guard.mark_consulted("dl-gh") + action = {"tool": "Bash", "command": "echo deploy", "session": "dl-gh", "cwd": str(repo)} + verdict = guard.check_action(action, omi) + assert verdict.allow, verdict + assert timeouts == [0.2] + assert _deadline_events() == [] + unknown = [e for e in compliance.read_events() if e["rule_id"] == "note-rule:public-deploy"] + assert [e["outcome"] for e in unknown] == ["unknown-visibility"] + + +def test_lookups_stop_short_of_the_judging_reserve() -> None: + """#460 review: inside a judging scope a lookup gets at most its bound and + never eats into the reserve left for judging; with no time left it is not + started at all (its fact is unknown).""" + from omind import deadline, rules + + assert deadline.lookup_timeout(10) == 10 # outside the guard: unchanged + with deadline.scope(60.0): + assert deadline.lookup_timeout(10) == deadline.LOOKUP_TIMEOUT_SECONDS + assert deadline.lookup_timeout(1) == 1 + with deadline.scope(deadline.LOOKUP_RESERVE_SECONDS): + assert deadline.lookup_timeout(10) == 0.0 + with pytest.raises(subprocess.TimeoutExpired): + rules._lookup(["git", "--version"], 5) + + +def test_the_hard_rules_keep_their_floor_after_a_slow_step( monkeypatch: pytest.MonkeyPatch, ) -> None: - """#460: the hard rules always get their floor, so a note rule's slow - network lookup spending the budget never turns `echo` into a deny.""" + """#460: a step that spends the budget before the hard rules (here the + note rules) cannot pre-empt them: they still name their own rule. What + comes after them is past the deadline, so a plain command is denied with + the deadline reason, never waved through.""" from omind import deadline monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) @@ -5491,6 +5643,122 @@ def slow_note_rules(action: dict[str, Any], omi_dir: Path | None) -> None: monkeypatch.setattr(guard, "_note_rules_verdict", slow_note_rules) base = {"tool": "Bash", "session": "dl-floor"} - assert guard.check_action({**base, "command": "echo hi"}).allow sudo = guard.check_action({**base, "command": "sudo id"}) assert not sudo.allow and sudo.rule_id == "sudo-use-fleet-sudo" + _assert_deadline_deny(guard.check_action({**base, "command": "echo hi"}), "dl-floor") + + +def test_a_crash_in_the_soft_gates_fails_open_with_the_hard_rules_rechecked( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """#460 review (coverage restored): the repo lookup crashing in the soft + gates fails open, after the hard rules; the fail-open re-check reuses + their verdict instead of judging (and opening a second floor) again.""" + monkeypatch.setattr(guard, "_repo_root_for_action", _boom) + real = guard._judge_hard_rules + judged: list[str] = [] + + def spy(command: str, session: str) -> guard.Verdict | None: + judged.append(command) + return real(command, session) + + monkeypatch.setattr(guard, "_judge_hard_rules", spy) + verdict = guard.check_action({"tool": "Bash", "command": "ls", "session": "s460c"}) + assert verdict.allow + assert judged == ["ls"] # judged once, re-checked from the cached verdict + error = [e for e in compliance.read_events() if e["rule_id"] == guard.GUARD_ERROR_RULE] + assert [(e["session"], e["outcome"]) for e in error] == [("s460c", "fail-open")] + sudo = guard.check_action({"tool": "Bash", "command": "sudo id", "session": "s460c"}) + assert not sudo.allow and sudo.rule_id == "sudo-use-fleet-sudo" + + +def test_the_steps_after_the_verdict_are_outside_the_deadline( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """#460 review: the budget re-arm, the excerpt and the retrieval + suggestion run after the verdict; a deadline there is neither a judging + failure (`guard-internal-error`) nor a deadline deny.""" + from omind import deadline + + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 0.05) + + def slow_budget(action: dict[str, Any], omi_dir: Path | None) -> None: + time.sleep(0.1) + deadline.check() + + monkeypatch.setattr(guard, "budget_verdict", slow_budget) + guard.mark_consulted("dl-tail") + verdict = guard.check_action({"tool": "Bash", "command": "ls", "session": "dl-tail"}) + assert verdict.allow + assert _deadline_events() == [] + assert not [e for e in compliance.read_events() if e["rule_id"] == guard.GUARD_ERROR_RULE] + + +def test_enclosing_repo_skips_a_path_holding_a_nul(tmp_path: Path) -> None: + """#460 review: a NUL in a `git -C` or `cd` path makes `resolve()` raise + `ValueError`. `harness.py` and `provision.py` call `decide()` bare, so it + must fall back to the next candidate, never raise.""" + (tmp_path / ".git").mkdir() + (tmp_path / ".git" / "HEAD").write_text("ref: refs/heads/main\n", encoding="utf-8") + nul = tmp_path / "a\0b" + assert guard._enclosing_repo([nul, tmp_path]) == tmp_path.resolve() + assert guard._enclosing_repo([nul, tmp_path], {}, {}) == tmp_path.resolve() + for command in ("git -C /tm\0p commit -m x", "cd /tm\0p && git push", "cd a\0b; git push"): + verdict = guard.decide({"tool": "Bash", "command": command, "cwd": str(tmp_path)}) + assert verdict.rule_id != guard.DEADLINE_RULE + + +def test_a_cd_chain_through_the_note_rules_is_linear(tmp_path: Path) -> None: + """#460 review: `_rules_command_view` resolved and walked every parent of + each distinct cwd of a `cd x; ` chain (quadratic: 8 s, then the deadline). + Resolution and the parent walk are memoised per command.""" + omi = tmp_path / "OMI" + omi.mkdir() + (omi / "Guard Rules.md").write_text( + '# Guard Rules\n\n```omind-rule\nid: every-bash\ntool: Bash\nmatch: "*"\n' + 'when:\n repo_visibility: public\naction: deny\nmessage: "m"\n```\n', + encoding="utf-8", + ) + action = {"tool": "Bash", "command": "cd x; " * 2000 + "echo hi", "cwd": str(tmp_path)} + cold_shell_caches() + with _hard_time_limit(traced_bound(2.5)): + verdict = guard.check_action({**action, "session": "dl-cd"}, omi) + assert verdict.rule_id != guard.DEADLINE_RULE + + +#: The shapes #460 names: each returns a verdict well inside the 8 s budget. +_ISSUE_460_SHAPES = { + "cd-chain": "cd x; " * 2000 + "echo hi", + "bash-c": "bash -c 'echo hi'; " * 100_000, + "heredoc-line": "cat " + "< None: + """#460 acceptance line 2: each shape, through `check_action` (note rules + included) and `decide()`, returns a real verdict well under the 8 s budget + (untraced: at most about 2.3 s, the `bash -c` flood). The budget is raised + here so a slow CI runner measures the shape instead of tripping it.""" + from omind import deadline + + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 600.0) + omi = tmp_path / "OMI" + omi.mkdir() + (omi / "Guard Rules.md").write_text( + '# Guard Rules\n\n```omind-rule\nid: every-bash\ntool: Bash\nmatch: "*"\n' + 'when:\n repo_visibility: public\naction: deny\nmessage: "m"\n```\n', + encoding="utf-8", + ) + action = {"tool": "Bash", "command": _ISSUE_460_SHAPES[shape], "cwd": str(tmp_path)} + for judge in ( + lambda: guard.check_action({**action, "session": "dl-shape"}, omi), + lambda: guard.decide({**action, "session": "dl-shape"}), + ): + cold_shell_caches() + with _hard_time_limit(traced_bound(5.0)): + verdict = judge() + assert verdict.rule_id != guard.DEADLINE_RULE + assert _deadline_events() == [] diff --git a/tests/test_rules.py b/tests/test_rules.py index a7569a1..091df37 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -1717,7 +1717,9 @@ def test_write_repo_lookup_failure_fails_open( def boom(*_a: object, **_k: object) -> None: raise RuntimeError("boom") - monkeypatch.setattr(guard, "_dir_repo", boom) + # `_write_target`, not `_dir_repo`: since #460 the cwd lookup shares the + # memoised parent walk, and this crash is in the write targets alone. + monkeypatch.setattr(guard, "_write_target", boom) command = f"echo x > {x.as_posix()}/f" action = {"tool": "Bash", "command": command, "cwd": outside.as_posix()} view = guard._rules_command_view(action) From c645613cf1331c0d64c13cbbcf1d73d451d6a6eb Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Fri, 2 Oct 2026 20:57:48 -0500 Subject: [PATCH 3/4] fix(guard): check the deadline in the stage split; timing tests independent of runner speed (#460) CI run 37085768401 (ubuntu, coverage on) overran the bash -c x 100,000 shape. The stage split (_program_stages_cached, one Python step per segment) and _stage_ends ran before the walk's first deadline.check(): about 0.4 s untraced, several seconds under coverage on a slow runner. Both loops now check the deadline. With a 0.3 s budget and 0.2 s floor under coverage, bash -c x 100,000 returns in 0.33 s (was 0.99 s); the longest stretch between checks is about 0.13 s untraced. Tests: - test_the_issue_460_shapes_always_return_inside_the_hook_margin: the issue's shapes at full size on a deliberately slowed judge (busy-wait in _basename and _chdir) must return within budget + floor + margin. It fails (20 s) without the new check points. - The "well inside the budget" test runs at the real budget, at sizes that hold on a CI runner under coverage; the cd-chain tests are scaled the same way. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011Zj1PCfsSSJgzLxi32jvde --- src/omind/guard.py | 2 + tests/test_guard.py | 102 +++++++++++++++++++++++++++++++++----------- 2 files changed, 80 insertions(+), 24 deletions(-) diff --git a/src/omind/guard.py b/src/omind/guard.py index a98e57a..3db8635 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -2094,6 +2094,7 @@ def _stage_ends(code: str, starts: list[int]) -> dict[int, int]: following = [*starts[1:], n] # built once: per stage it was quadratic (#445) r = 0 for idx, pos in enumerate(starts): + deadline.check() while real[r] <= pos: r += 1 # A split-off redirect target ends at the next stage of any kind. @@ -3432,6 +3433,7 @@ def _program_stages_cached( stages: list[tuple[str, tuple[str, ...], int, int]] = [] # An escaped `\;` (find's -exec terminator) is a word, not a separator (#419). for seg in _SEGMENT_RE.finditer(code): + deadline.check() # one per segment: a long chain stays inside the budget (#460) end = seg.end() while end < n and code[end].isspace(): end += 1 diff --git a/tests/test_guard.py b/tests/test_guard.py index d9d9eb9..7717988 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -5379,10 +5379,12 @@ def test_paths_the_filesystem_cannot_look_up_do_not_raise( def test_a_long_cd_chain_is_walked_in_linear_time() -> None: """#460: each `cd x` re-joined every segment before it (quadratic). Past - the longest path an OS can name, the directory is unknowable.""" - command = "cd x; " * 50_000 + "echo hi" + the longest path an OS can name, the directory is unknowable. (20,000 + steps: about 0.3 s untraced; quadratic, it took minutes. The bound holds + on a CI runner under coverage, about 7x slower.)""" + command = "cd x; " * 20_000 + "echo hi" cold_shell_caches() - with _hard_time_limit(traced_bound(1.0)): + with _hard_time_limit(traced_bound(1.5)): _sites, cwd, _local, _bodies = guard._shell_walk(command) assert cwd is None assert guard._chdir(Path("a"), "b") == Path("a/b") @@ -5708,10 +5710,7 @@ def test_enclosing_repo_skips_a_path_holding_a_nul(tmp_path: Path) -> None: assert verdict.rule_id != guard.DEADLINE_RULE -def test_a_cd_chain_through_the_note_rules_is_linear(tmp_path: Path) -> None: - """#460 review: `_rules_command_view` resolved and walked every parent of - each distinct cwd of a `cd x; ` chain (quadratic: 8 s, then the deadline). - Resolution and the parent walk are memoised per command.""" +def _public_every_bash_rule(tmp_path: Path) -> Path: omi = tmp_path / "OMI" omi.mkdir() (omi / "Guard Rules.md").write_text( @@ -5719,14 +5718,24 @@ def test_a_cd_chain_through_the_note_rules_is_linear(tmp_path: Path) -> None: 'when:\n repo_visibility: public\naction: deny\nmessage: "m"\n```\n', encoding="utf-8", ) - action = {"tool": "Bash", "command": "cd x; " * 2000 + "echo hi", "cwd": str(tmp_path)} + return omi + + +def test_a_cd_chain_through_the_note_rules_is_linear(tmp_path: Path) -> None: + """#460 review: `_rules_command_view` resolved and walked every parent of + each distinct cwd of a `cd x; ` chain (quadratic: 8 s at 2,000 steps, + then the deadline). Resolution and the parent walk are memoised per + command. 1,000 steps: about 0.3 s untraced, so the bound holds on a CI + runner under coverage.""" + omi = _public_every_bash_rule(tmp_path) + action = {"tool": "Bash", "command": "cd x; " * 1000 + "echo hi", "cwd": str(tmp_path)} cold_shell_caches() - with _hard_time_limit(traced_bound(2.5)): + with _hard_time_limit(traced_bound(1.5)): verdict = guard.check_action({**action, "session": "dl-cd"}, omi) assert verdict.rule_id != guard.DEADLINE_RULE -#: The shapes #460 names: each returns a verdict well inside the 8 s budget. +#: The shapes #460 names, at the size the issue gives. _ISSUE_460_SHAPES = { "cd-chain": "cd x; " * 2000 + "echo hi", "bash-c": "bash -c 'echo hi'; " * 100_000, @@ -5734,31 +5743,76 @@ def test_a_cd_chain_through_the_note_rules_is_linear(tmp_path: Path) -> None: } +def _slow_down(monkeypatch: pytest.MonkeyPatch, seconds: float) -> None: + """Make the walk's per-stage and per-`cd` helpers burn ``seconds`` per + call: a judge as slow as any machine, without guessing a runner's speed.""" + + def slowed(real: Callable[..., Any]) -> Callable[..., Any]: + def slow(*args: Any) -> Any: + stop = time.perf_counter() + seconds + while time.perf_counter() < stop: + pass + return real(*args) + + return slow + + monkeypatch.setattr(guard, "_basename", slowed(guard._basename)) + monkeypatch.setattr(guard, "_chdir", slowed(guard._chdir)) + + @pytest.mark.parametrize("shape", list(_ISSUE_460_SHAPES)) -def test_the_issue_460_shapes_are_judged_well_inside_the_budget( +def test_the_issue_460_shapes_always_return_inside_the_hook_margin( monkeypatch: pytest.MonkeyPatch, tmp_path: Path, shape: str ) -> None: - """#460 acceptance line 2: each shape, through `check_action` (note rules - included) and `decide()`, returns a real verdict well under the 8 s budget - (untraced: at most about 2.3 s, the `bash -c` flood). The budget is raised - here so a slow CI runner measures the shape instead of tripping it.""" + """#460 acceptance, independent of hardware: on a judge slowed far past + its budget, `check_action` and `decide()` still return within budget + + hard-rule floor + a small margin, with a real verdict or the deadline + deny. A stretch of judging with no `deadline.check()` (CI run + 37085768401: the stage split of 100,000 `bash -c` commands) fails this.""" from omind import deadline - monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", 600.0) - omi = tmp_path / "OMI" - omi.mkdir() - (omi / "Guard Rules.md").write_text( - '# Guard Rules\n\n```omind-rule\nid: every-bash\ntool: Bash\nmatch: "*"\n' - 'when:\n repo_visibility: public\naction: deny\nmessage: "m"\n```\n', - encoding="utf-8", - ) + budget, floor = 0.2, 0.2 + monkeypatch.setattr(deadline, "JUDGE_BUDGET_SECONDS", budget) + monkeypatch.setattr(deadline, "HARD_RULE_FLOOR_SECONDS", floor) + _slow_down(monkeypatch, 0.0001) + omi = _public_every_bash_rule(tmp_path) action = {"tool": "Bash", "command": _ISSUE_460_SHAPES[shape], "cwd": str(tmp_path)} for judge in ( lambda: guard.check_action({**action, "session": "dl-shape"}, omi), lambda: guard.decide({**action, "session": "dl-shape"}), ): cold_shell_caches() - with _hard_time_limit(traced_bound(5.0)): + started = time.perf_counter() + with _hard_time_limit(30.0): + verdict = judge() + elapsed = time.perf_counter() - started + assert elapsed <= budget + floor + traced_bound(0.5), elapsed + assert verdict.rule_id in (guard.DEADLINE_RULE, "omi-gate"), verdict + + +#: The same shapes at a size each judges well inside the 8 s budget even on a +#: CI runner under coverage (about 7x slower than a laptop untraced). +_ISSUE_460_SMALL_SHAPES = { + "cd-chain": "cd x; " * 500 + "echo hi", + "bash-c": "bash -c 'echo hi'; " * 10_000, + "heredoc-line": "cat " + "< None: + """#460 acceptance line 2, at the real budget: each shape, through + `check_action` (note rules included) and `decide()`, returns a real + verdict, never the deadline deny. Untraced, each takes at most about + 0.25 s; at the issue's own sizes, about 2.3 s (`bash -c` x 100,000).""" + omi = _public_every_bash_rule(tmp_path) + action = {"tool": "Bash", "command": _ISSUE_460_SMALL_SHAPES[shape], "cwd": str(tmp_path)} + for judge in ( + lambda: guard.check_action({**action, "session": "dl-shape"}, omi), + lambda: guard.decide({**action, "session": "dl-shape"}), + ): + cold_shell_caches() + with _hard_time_limit(traced_bound(1.5)): verdict = judge() assert verdict.rule_id != guard.DEADLINE_RULE assert _deadline_events() == [] From dbd9b063b7fee4e00b91239d9c4745cc6ec4cb6f Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Fri, 2 Oct 2026 21:09:55 -0500 Subject: [PATCH 4/4] test(guard): give the #455 find-actions verdict CI headroom (3.10 + coverage) The 100k find-actions verdict takes ~0.4 s untraced; CI's 3.10 coverage run is ~7x slower and crossed the 3 s traced bound. A quadratic regression still costs 10x+ more, so traced_bound(2.0) keeps the test meaningful. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_011Zj1PCfsSSJgzLxi32jvde --- tests/test_guard.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_guard.py b/tests/test_guard.py index 7717988..4da4084 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -3760,7 +3760,9 @@ def test_find_exec_and_expansion_searches_stay_linear(shape: tuple[str, str], co guard._EVAL_LITERAL_RE.sub("", command) guard._EXPANDED_NAME_RE.sub("", command) cold_shell_caches() - with _hard_time_limit(traced_bound(1.0)): + # The full verdict at 100k is ~0.4 s untraced; CI's 3.10 coverage run is + # ~7x slower. A quadratic regression costs 10x+ more, so 2 s still bites. + with _hard_time_limit(traced_bound(2.0)): guard._hard_policy_verdict(command)