From 15d77e748081716d191d6020ae35258e4e24095c Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Wed, 1 Jul 2026 17:26:11 -0500 Subject: [PATCH] fix: hardening batch from adversarial code review (v3.7.6) Address findings from a full adversarial review of the vault store, mesh, guard/enforcement layer, provisioning, and CLI/cron paths. All fixes; no API breaks. NoteFields gains backward-compatible frontmatter/lead fields. Data integrity: - Preserve YAML frontmatter + lead prose through parse/render and the mesh merge; fence-aware section splitting; symmetric equal-rev merge convergence. - errors="replace" (+ BOM strip) on every note/index/log read so one bad byte can't down listing/search/writes; search/backlinks skip a mid-scan delete. - Case-insensitive reserved-name check; reject dot-prefixed/over-long titles; close the create_note concurrent-create race; atomic dir-fsynced writes. Guard false-positives + crash-hardening: - Freshness accepts `git -C fetch` and `git fetch && git status`. - Command-anchor the forge/destructive seed rules; a bare `>` (pytest 2>&1) is no longer a side effect; resolve global-config paths against $HOME so a project-local .claude/ isn't gated; negation-aware auth detection. - Reject uncompilable/empty-match learned rules at load and skip them at match time so one bad rule can't brick every tool call; wrapper/sudoedit command-position coverage. Enforcement fail-open holes: - Adapter fails CLOSED on unparseable events and reads array-shaped args; contentless list-notes/graph-* no longer clears the gate; secret-output guard no longer leaks `pass show X 2>/dev/null | head` nor false-blocks `pass` inside a word; turn-reset clears pending/freshness (per-turn again). Availability: - Atomic config/hook/backup writes (a torn write can't brick a harness or the guard hook). Checkpoint never raises into the timer + absolute ExecStart. - Mesh: gitignore .obsidian/workspace.json, doctor surfaces per-peer sync errors, push timeout no longer aborts the pass, BatchMode git. - Self-update: OMIND_NO_UPDATE_CHECK no longer disables an explicit update; install/check timeouts. lint --strict passes on a healthy vault. Migrate hook migrates before deleting (no fuzzy/slug-less data loss). Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 74 ++++++++ extras/omi_enforce.py | 80 ++++---- pyproject.toml | 2 +- src/omind/__init__.py | 2 +- src/omind/_omi_enforce.py | 76 ++++---- src/omind/adapters.py | 60 +++++- src/omind/agents.py | 41 ++-- src/omind/backup.py | 18 +- src/omind/checkpoint.py | 48 ++++- src/omind/cli.py | 21 ++- src/omind/compliance.py | 6 +- src/omind/guard.py | 200 ++++++++++++++++---- src/omind/journal.py | 4 +- src/omind/lint.py | 101 ++++++++-- src/omind/merge.py | 14 +- src/omind/mesh.py | 71 ++++++- src/omind/omi-gate-reset.sh | 8 + src/omind/omi-guard-hermes.sh | 5 + src/omind/omi-guard.sh | 8 + src/omind/paths.py | 37 ++++ src/omind/policy.py | 57 +++++- src/omind/provision.py | 12 +- src/omind/secret-output-guard.sh | 41 +++- src/omind/store.py | 301 ++++++++++++++++++++++++------ src/omind/update.py | 18 +- src/omind/verify.py | 14 +- tests/test_adapters.py | 18 ++ tests/test_compliance.py | 10 + tests/test_guard.py | 82 ++++++++ tests/test_lint.py | 47 +++++ tests/test_merge.py | 21 +++ tests/test_policy.py | 58 ++++++ tests/test_secret_output_guard.py | 23 +++ tests/test_store.py | 68 +++++++ tests/test_update.py | 2 +- uv.lock | 2 +- 36 files changed, 1379 insertions(+), 271 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab651dd..92acada 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,80 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [3.7.6] - 2026-07-01 + +Hardening release from an adversarial code review — data-integrity, guard +false-positives, enforcement fail-open holes, and crash/availability fixes. No +API breaks; `NoteFields` gains backward-compatible `frontmatter`/`lead` fields. + +### Fixed + +- **Note data loss:** the parse/render round-trip no longer drops YAML + frontmatter (Obsidian Properties) or lead prose before the first `##`; section + splitting is now fence-aware so a `##` inside a code block is body text, not a + new section. Applies to the local edit path and the mesh merge driver. +- **Mesh convergence:** equal-rev/different-content now resolves by a symmetric + content tiebreak, so `merge(A,B)` == `merge(B,A)` and the fleet converges + instead of ping-ponging; the index-description migration stamps a rev on mesh + nodes so it no longer creates equal-rev divergence. +- **Mesh replication stalls silently:** `.obsidian/workspace.json` (and friends) + are now gitignored so their per-machine churn can't abort every peer merge; + `omind doctor` surfaces recorded per-peer sync errors instead of always + reporting "ok"; a push timeout records the error and continues to the next + peer (and still writes sync state) instead of aborting the whole pass; network + git runs with `GIT_TERMINAL_PROMPT=0` + ssh `BatchMode` so a prompt can't hang. +- **One bad byte no longer downs the vault:** all note/index/log reads decode + with `errors="replace"` (and strip a BOM), and `search`/`backlinks` skip a note + deleted mid-scan — a single non-UTF-8 note can't break listing/search/writes. +- **Guard false-positive interruptions:** + - Freshness now recognizes `git -C fetch` and compound read forms + (`git fetch && git status`) — the exact remediation the block message tells + you to run — instead of only a bare `git fetch`. + - Forge/destructive seed rules (`gh repo delete`, `gh auth setup-git`, …) are + command-anchored, so a grep pattern or commit message no longer blocks. + - A bare `>` (as in `pytest 2>&1`) is no longer treated as a file-writing side + effect; only real stdout redirects count. + - The global-config-mutation gate resolves the path against `$HOME`, so a + project-local `/.claude/settings.json` is not treated as global config. + - Global-authorization detection is negation-aware ("don't change" no longer + authorizes) and covers more imperatives (fix/add/create/…). +- **Guard crash-hardening:** a learned rule whose regex fails to compile or + matches the empty string is rejected at load and skipped at match time, so one + bad rule can no longer brick every tool call on the machine. The command- + position anchor now covers shell keywords/wrappers (`then`, `exec`, `xargs`, + absolute paths) and `sudoedit`, closing sudo-rule bypasses. +- **Enforcement fail-open holes:** the guard adapter fails **closed** on an + unparseable event and accepts array-shaped `args`; a contentless + `list-notes`/`graph-*` call no longer clears the gate or auto-scores relevant; + the secret-output guard no longer treats `pass show X 2>/dev/null | head` as + safe (a real leak) and no longer false-blocks `pass` inside another word / a + grep pattern; the turn-reset clears pending-intent and git-freshness (freshness + is per-turn again, not per-session). +- **Cron/timer safety:** `checkpoint run` degrades cleanly instead of raising a + traceback into the timer; `install-timer` writes an absolute `ExecStart` (or + fails loudly) instead of a silently-broken unit; a malformed journal bullet or + tz-aware log timestamp no longer crashes a checkpoint, and boundary-minute + actions are no longer dropped from every window. +- **Self-update:** `OMIND_NO_UPDATE_CHECK` no longer disables an explicit + `omind self-update`; the install subprocess and the version check have + timeouts sized for a user-invoked update. +- **`omind lint` false failures:** a dated note series (daily Worklogs) is no + longer flagged as near-duplicates; links to archived or `Journal/`-subfolder + notes and `[[wikilinks]]` quoted in code fences are no longer broken-link + errors — so `lint --strict` passes on a healthy vault. +- **Config/hook write corruption:** every managed settings/config/hook/backup + write is now atomic (temp file + `os.replace` + directory fsync) so a crash + mid-write can't brick a harness config or the guard hook; store atomic writes + fsync the directory too. +- **Filenames:** the reserved-name check is case-insensitive (so a note titled + "Index" can't destroy `index.md` on a case-insensitive filesystem); dot-prefixed + and over-long titles raise a clean `NoteError` instead of creating an invisible + note or an `ENAMETOOLONG` crash; `create_note` closes a concurrent-create race. +- **Enforcement migrate hook:** no longer deletes a memory file on a fuzzy + filename match or a missing `name:` slug — it migrates (with a timeout and + permission-safe unlinks) before deleting, and leaves the file if migration + fails. + ## [3.7.5] - 2026-07-01 ### Changed diff --git a/extras/omi_enforce.py b/extras/omi_enforce.py index 52e21b8..8dd2283 100755 --- a/extras/omi_enforce.py +++ b/extras/omi_enforce.py @@ -8,6 +8,12 @@ 3. If NOT found → migrate via `omind note` first 4. Delete the Claude memory file either way """ +# Lazy annotations so the builtin-generic hints (dict[str, str]) don't need +# evaluation at import — this ships as a hook run under the host's system +# python3, whose version `requires-python` does not govern. +from __future__ import annotations + +import contextlib import glob import pathlib import re @@ -20,7 +26,7 @@ CLAUDE_PROJECTS = HOME / ".claude/projects" -def parse_frontmatter(text: str) -> tuple[dict, str]: +def parse_frontmatter(text: str) -> tuple[dict[str, str], str]: """Return (fields_dict, body_text). Handles simple key: value and nested metadata.type.""" if not text.startswith("---"): return {}, text @@ -28,7 +34,7 @@ def parse_frontmatter(text: str) -> tuple[dict, str]: if len(parts) < 3: return {}, text fm_raw, body = parts[1], parts[2].strip() - fields: dict = {} + fields: dict[str, str] = {} in_metadata = False for line in fm_raw.splitlines(): if line.strip() == "metadata:": @@ -51,25 +57,18 @@ def slug_to_title(slug: str) -> str: return " ".join(w.capitalize() for w in re.split(r"[-_]", slug) if w) -def omi_exists(title: str, slug: str) -> bool: - """True if the OMI vault already has a note covering this memory.""" +def omi_exists(title: str) -> bool: + """True only if the OMI vault already has a note with this EXACT filename. + + A fuzzy "≥2 of 3 slug words appear in some filename" match used to declare a + memory already-covered and DELETE it without migrating its content — so + ``docker-compose-tips`` was destroyed because a "Docker Compose Setup" note + existed. Exact match only: anything else is migrated before deletion. + """ if not OMI_DIR.exists(): return False - - # Exact filename match (omind derives filename directly from title) safe_title = re.sub(r'[<>:"/\\|?*\x00-\x1f]', "", title).strip() - if (OMI_DIR / f"{safe_title}.md").exists(): - return True - - # Fuzzy: ≥2 of the first 3 meaningful words appear in some filename - words = [w.lower() for w in re.split(r"[-_]", slug) if len(w) > 3] - if len(words) >= 2: - for f in OMI_DIR.glob("*.md"): - fname = f.stem.lower() - if sum(1 for w in words[:3] if w in fname) >= 2: - return True - - return False + return bool(safe_title) and (OMI_DIR / f"{safe_title}.md").exists() def migrate(title: str, summary: str, body: str, mem_type: str) -> bool: @@ -85,23 +84,38 @@ def migrate(title: str, summary: str, body: str, mem_type: str) -> bool: "--summary", summary or title, "--tags", tags, ] - result = subprocess.run(cmd, input=body, text=True, capture_output=True) + try: + # Timeout so a hung `omind note` (vault lock held by mesh sync, gpg + # pinentry, NFS stall) can't hang this PostToolUse hook — and therefore + # every agent turn — indefinitely. + result = subprocess.run( + cmd, input=body, text=True, capture_output=True, timeout=30 + ) + except (subprocess.SubprocessError, OSError): + return False return result.returncode == 0 +def _safe_unlink(path: pathlib.Path) -> None: + """Delete a migrated memory file; a permission/read-only-FS error must not + crash a hook that fires on every tool call.""" + with contextlib.suppress(OSError): + path.unlink(missing_ok=True) + + def main() -> None: pattern = str(CLAUDE_PROJECTS / "*/memory/*.md") for filepath in glob.glob(pattern): path = pathlib.Path(filepath) - # Always nuke stale MEMORY.md index files + # Always nuke stale MEMORY.md index files (a generated pointer, no content) if path.name == "MEMORY.md": - path.unlink(missing_ok=True) + _safe_unlink(path) continue try: - content = path.read_text(encoding="utf-8") - except Exception: + content = path.read_text(encoding="utf-8", errors="replace") + except OSError: continue fm, body = parse_frontmatter(content) @@ -109,18 +123,16 @@ def main() -> None: description = fm.get("description", "").strip() mem_type = fm.get("type", "").strip() - if not slug: - path.unlink(missing_ok=True) - continue - - title = slug_to_title(slug) + # A file with no `name:` slug still holds memory content — derive a title + # from its filename and MIGRATE it before deleting (never unlink blind). + title = slug_to_title(slug) if slug else slug_to_title(path.stem) - if omi_exists(title, slug): - path.unlink(missing_ok=True) - else: - if migrate(title, description, body, mem_type): - path.unlink(missing_ok=True) - # If migration fails, leave the file — don't lose data + if omi_exists(title): + _safe_unlink(path) # content already in OMI under this exact title + elif migrate(title, description, body, mem_type): + _safe_unlink(path) + # If migration fails (or omind is unavailable), LEAVE the file — the whole + # point is to never lose memory content. if __name__ == "__main__": diff --git a/pyproject.toml b/pyproject.toml index 7bcb391..6d10395 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "omind" -version = "3.7.5" +version = "3.7.6" 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 f281878..e2c6d4e 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__ = "3.7.5" +__version__ = "3.7.6" diff --git a/src/omind/_omi_enforce.py b/src/omind/_omi_enforce.py index fa28fd5..8dd2283 100755 --- a/src/omind/_omi_enforce.py +++ b/src/omind/_omi_enforce.py @@ -8,6 +8,12 @@ 3. If NOT found → migrate via `omind note` first 4. Delete the Claude memory file either way """ +# Lazy annotations so the builtin-generic hints (dict[str, str]) don't need +# evaluation at import — this ships as a hook run under the host's system +# python3, whose version `requires-python` does not govern. +from __future__ import annotations + +import contextlib import glob import pathlib import re @@ -51,25 +57,18 @@ def slug_to_title(slug: str) -> str: return " ".join(w.capitalize() for w in re.split(r"[-_]", slug) if w) -def omi_exists(title: str, slug: str) -> bool: - """True if the OMI vault already has a note covering this memory.""" +def omi_exists(title: str) -> bool: + """True only if the OMI vault already has a note with this EXACT filename. + + A fuzzy "≥2 of 3 slug words appear in some filename" match used to declare a + memory already-covered and DELETE it without migrating its content — so + ``docker-compose-tips`` was destroyed because a "Docker Compose Setup" note + existed. Exact match only: anything else is migrated before deletion. + """ if not OMI_DIR.exists(): return False - - # Exact filename match (omind derives filename directly from title) safe_title = re.sub(r'[<>:"/\\|?*\x00-\x1f]', "", title).strip() - if (OMI_DIR / f"{safe_title}.md").exists(): - return True - - # Fuzzy: ≥2 of the first 3 meaningful words appear in some filename - words = [w.lower() for w in re.split(r"[-_]", slug) if len(w) > 3] - if len(words) >= 2: - for f in OMI_DIR.glob("*.md"): - fname = f.stem.lower() - if sum(1 for w in words[:3] if w in fname) >= 2: - return True - - return False + return bool(safe_title) and (OMI_DIR / f"{safe_title}.md").exists() def migrate(title: str, summary: str, body: str, mem_type: str) -> bool: @@ -85,23 +84,38 @@ def migrate(title: str, summary: str, body: str, mem_type: str) -> bool: "--summary", summary or title, "--tags", tags, ] - result = subprocess.run(cmd, input=body, text=True, capture_output=True) + try: + # Timeout so a hung `omind note` (vault lock held by mesh sync, gpg + # pinentry, NFS stall) can't hang this PostToolUse hook — and therefore + # every agent turn — indefinitely. + result = subprocess.run( + cmd, input=body, text=True, capture_output=True, timeout=30 + ) + except (subprocess.SubprocessError, OSError): + return False return result.returncode == 0 +def _safe_unlink(path: pathlib.Path) -> None: + """Delete a migrated memory file; a permission/read-only-FS error must not + crash a hook that fires on every tool call.""" + with contextlib.suppress(OSError): + path.unlink(missing_ok=True) + + def main() -> None: pattern = str(CLAUDE_PROJECTS / "*/memory/*.md") for filepath in glob.glob(pattern): path = pathlib.Path(filepath) - # Always nuke stale MEMORY.md index files + # Always nuke stale MEMORY.md index files (a generated pointer, no content) if path.name == "MEMORY.md": - path.unlink(missing_ok=True) + _safe_unlink(path) continue try: - content = path.read_text(encoding="utf-8") - except Exception: + content = path.read_text(encoding="utf-8", errors="replace") + except OSError: continue fm, body = parse_frontmatter(content) @@ -109,18 +123,16 @@ def main() -> None: description = fm.get("description", "").strip() mem_type = fm.get("type", "").strip() - if not slug: - path.unlink(missing_ok=True) - continue - - title = slug_to_title(slug) + # A file with no `name:` slug still holds memory content — derive a title + # from its filename and MIGRATE it before deleting (never unlink blind). + title = slug_to_title(slug) if slug else slug_to_title(path.stem) - if omi_exists(title, slug): - path.unlink(missing_ok=True) - else: - if migrate(title, description, body, mem_type): - path.unlink(missing_ok=True) - # If migration fails, leave the file — don't lose data + if omi_exists(title): + _safe_unlink(path) # content already in OMI under this exact title + elif migrate(title, description, body, mem_type): + _safe_unlink(path) + # If migration fails (or omind is unavailable), LEAVE the file — the whole + # point is to never lose memory content. if __name__ == "__main__": diff --git a/src/omind/adapters.py b/src/omind/adapters.py index 062c06b..864753d 100644 --- a/src/omind/adapters.py +++ b/src/omind/adapters.py @@ -21,6 +21,7 @@ from __future__ import annotations +import json import sys from pathlib import Path from typing import Any, TextIO @@ -41,6 +42,32 @@ def _first_str(data: dict[str, Any], keys: tuple[str, ...]) -> str: return "" +def _derive_command(event: dict[str, Any], tool_input: dict[str, Any]) -> str: + """Best-effort command text from a harness event. + + Accepts the common argv-as-list shape (``"args": ["repo", "delete", "a/b"]``) + and an ``input``/``args`` object, not just a plain string — otherwise the + guard saw an empty command and no hard rule could match the real payload. + """ + for source in (event.get("command"), tool_input.get("command")): + if isinstance(source, str) and source: + return source + for container in (event, tool_input): + for key in ("args", "input"): + val = container.get(key) + if isinstance(val, str) and val: + return val + if isinstance(val, list): + joined = " ".join(str(x) for x in val if x is not None).strip() + if joined: + return joined + if isinstance(val, dict): + inner = _first_str(val, ("command", "cmd")) + if inner: + return inner + return "" + + def normalize_action(event: dict[str, Any]) -> dict[str, Any]: """Map a harness pre-action event into the guard's action schema. @@ -52,11 +79,7 @@ def normalize_action(event: dict[str, Any]) -> dict[str, Any]: tool = _first_str(event, ("tool", "tool_name", "name")) tool_input = event.get("tool_input") tool_input = tool_input if isinstance(tool_input, dict) else {} - command = ( - _first_str(event, ("command",)) - or _first_str(tool_input, ("command",)) - or _first_str(event, ("args", "input")) - ) + command = _derive_command(event, tool_input) file_path = _first_str(tool_input, ("file_path", "path")) or _first_str( event, ("file_path", "path") ) @@ -89,10 +112,33 @@ def run_adapter( from omind import harness as harness_mod src = stream if stream is not None else sys.stdin - event = guard._load(src) + spec = harness_mod.spec_for(harness) + try: + if src.isatty(): # a by-hand invocation with no piped event: nothing to guard + return 0 + except (AttributeError, ValueError, OSError): + pass + raw = src.read() + if not raw.strip(): + return 0 # no event (the shell adapter also allows empty input) + try: + event = json.loads(raw) + if not isinstance(event, dict): + raise ValueError("event is not a JSON object") + except (ValueError, TypeError): + # A mangled/truncated event in an enforcement component must FAIL CLOSED: + # a destructive command must never be waved through because its event + # didn't parse. Emit the harness's block verdict. + blocked = guard.Verdict( + allow=False, + reason="omi-guard: unparseable guard event — blocking (fail-closed)", + rule_id="adapter-parse-error", + ) + return harness_mod.render_decision( + blocked, spec.block_format, sys.stdout, sys.stderr, event="" + ) action = normalize_action(event) verdict = guard.check_action(action) - spec = harness_mod.spec_for(harness) # Codex's deny shape depends on which hook fired (PreToolUse vs # PermissionRequest); pass the event name through (ignored by other harnesses). return harness_mod.render_decision( diff --git a/src/omind/agents.py b/src/omind/agents.py index 3e4ce9c..3958a21 100644 --- a/src/omind/agents.py +++ b/src/omind/agents.py @@ -435,9 +435,8 @@ def register_mcp(self) -> None: ) if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - yaml.safe_dump(data, sort_keys=False, allow_unicode=True), - encoding="utf-8", + paths.atomic_write_text( + path, yaml.safe_dump(data, sort_keys=False, allow_unicode=True) ) def install_priming(self) -> None: @@ -476,9 +475,8 @@ def install_priming(self) -> None: self._record(f"install OMI priming hook (pre_llm_call) in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - yaml.safe_dump(data, sort_keys=False, allow_unicode=True), - encoding="utf-8", + paths.atomic_write_text( + path, yaml.safe_dump(data, sort_keys=False, allow_unicode=True) ) else: self.log(f" OMI priming hook already installed in {path}") @@ -548,9 +546,8 @@ def install_guard(self) -> None: self._record(f"install OMI guard hook (pre_tool_call) in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - yaml.safe_dump(data, sort_keys=False, allow_unicode=True), - encoding="utf-8", + paths.atomic_write_text( + path, yaml.safe_dump(data, sort_keys=False, allow_unicode=True) ) else: self.log(f" OMI guard hook already installed in {path}") @@ -597,8 +594,8 @@ def _allowlist_hook(self, event: str, command: str, marker: str) -> None: self._record(f"pre-approve OMI {event} hook in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - json.dumps(data, indent=2, sort_keys=True) + "\n", encoding="utf-8" + paths.atomic_write_text( + path, json.dumps(data, indent=2, sort_keys=True) + "\n" ) @@ -658,7 +655,7 @@ def register_mcp(self) -> None: ) if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def install_priming(self) -> None: """Wire OpenClaw to read OMI first each session. @@ -712,7 +709,7 @@ def install_priming(self) -> None: self._record(f"register OMI bootstrap priming (bootstrap-extra-files) in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def integrate(self) -> None: super().integrate() @@ -751,7 +748,7 @@ def install_guard(self) -> None: self._record(f"register OMI guard gateway hook (detect-only) in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") else: self.log(f" OMI guard gateway hook already installed in {path}") @@ -839,7 +836,7 @@ def install_guard(self) -> None: self._record(f"install OMI guard hook (BeforeTool) in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") else: self.log(f" OMI guard hook already installed in {path}") @@ -930,7 +927,7 @@ def register_mcp(self) -> None: ) if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def integrate(self) -> None: super().integrate() @@ -1071,7 +1068,7 @@ def install_guard(self) -> None: ) if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") else: self.log(f" OMI guard hooks already installed in {path}") @@ -1100,7 +1097,7 @@ def install_priming(self) -> None: self._record(f"install OMI priming hook (SessionStart) in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def _guard_wired(self) -> bool: try: @@ -1294,7 +1291,7 @@ def install_hook_trust(self) -> None: self._record(f"persist trust for omind Codex hooks in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(tomlkit.dumps(doc), encoding="utf-8") + paths.atomic_write_text(path, tomlkit.dumps(doc)) def bootstrap_content(self) -> str: return seeds.CODEX_AGENTS_BOOTSTRAP_TEMPLATE.format( @@ -1338,7 +1335,7 @@ def install_bootstrap(self) -> None: self._record(f"install OMI bootstrap pointer in {path}") if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(updated, encoding="utf-8") + paths.atomic_write_text(path, updated) # -- MCP registration (config.toml, TOML — see class docstring) --------- @@ -1401,7 +1398,7 @@ def register_mcp(self) -> None: ) if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(tomlkit.dumps(doc), encoding="utf-8") + paths.atomic_write_text(path, tomlkit.dumps(doc)) # -- MCP-only targets (register the omi server, no guard / skill / priming) ----- @@ -1471,7 +1468,7 @@ def register_mcp(self) -> None: ) if not self.config.dry_run: path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def integrate(self) -> None: # MCP registration only — no skill / priming / guard for these targets. diff --git a/src/omind/backup.py b/src/omind/backup.py index 9752f86..13645ec 100644 --- a/src/omind/backup.py +++ b/src/omind/backup.py @@ -37,6 +37,7 @@ from datetime import datetime, timezone from pathlib import Path +from omind import paths from omind.notes import upsert_note from omind.paths import INDEX_FILENAME from omind.proc import DEFAULT_TIMEOUT, run_command @@ -119,7 +120,7 @@ def load_config() -> BackupConfig | None: if not path.is_file(): return None try: - data = json.loads(path.read_text(encoding="utf-8")) + data = json.loads(path.read_text(encoding="utf-8", errors="replace")) except json.JSONDecodeError as exc: raise BackupError( f"{path} is not valid JSON ({exc}); fix or remove it and re-run " @@ -127,18 +128,25 @@ def load_config() -> BackupConfig | None: ) from exc if not isinstance(data, dict) or not isinstance(data.get("repo"), str) or not data["repo"]: return None + # A hand-edited/corrupt non-numeric consecutive_failures must not crash the + # loader with a raw ValueError — treat it as 0. + try: + failures = int(data.get("consecutive_failures") or 0) + except (TypeError, ValueError): + failures = 0 return BackupConfig( repo=data["repo"], - consecutive_failures=int(data.get("consecutive_failures") or 0), + consecutive_failures=failures, last_success=data.get("last_success") or None, last_snapshot=data.get("last_snapshot") or None, ) def save_config(config: BackupConfig) -> None: - path = config_path() - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(asdict(config), indent=2) + "\n", encoding="utf-8") + # Atomic write: a torn backup.json makes load_config raise, which disables + # backups AND the failure counter (run_backup dies in _require_config before + # it can record anything) — the escalation machinery needs the file that broke. + paths.atomic_write_text(config_path(), json.dumps(asdict(config), indent=2) + "\n") # -- subprocess plumbing -------------------------------------------------------- diff --git a/src/omind/checkpoint.py b/src/omind/checkpoint.py index 5f337d0..46399c1 100644 --- a/src/omind/checkpoint.py +++ b/src/omind/checkpoint.py @@ -85,9 +85,15 @@ def is_empty(self) -> bool: def _parse_ts(value: Any) -> datetime | None: try: - return datetime.fromisoformat(str(value)) + dt = datetime.fromisoformat(str(value)) except (ValueError, TypeError): return None + # The rest of the module works in NAIVE local time; a tz-aware log line (a + # foreign writer, a hand edit) would raise "can't compare offset-naive and + # offset-aware" mid-timer. Normalize to naive local. + if dt.tzinfo is not None: + dt = dt.astimezone().replace(tzinfo=None) + return dt def _journal_actions(omi_dir: Path | str, day: datetime, cutoff: datetime, now: datetime) -> list[ @@ -96,17 +102,24 @@ def _journal_actions(omi_dir: Path | str, day: datetime, cutoff: datetime, now: """Action bullets from ``day``'s journal whose time falls in ``(cutoff, now]``.""" path = hooks.journal_dir(omi_dir) / hooks.journal_name(day) try: - text = path.read_text(encoding="utf-8") + text = path.read_text(encoding="utf-8", errors="replace") except OSError: return [] + # Bullets carry only HH:MM. Floor the cutoff to the minute for the comparison + # so an action in the cutoff's own minute (journaled AFTER the previous run + # already fired) is not silently dropped from EVERY window forever. + cutoff_minute = cutoff.replace(second=0, microsecond=0) out: list[dict[str, str]] = [] for line in text.splitlines(): match = _BULLET_RE.match(line.strip()) if not match: continue hour, minute, rest = int(match.group(1)), int(match.group(2)), match.group(3) - when = day.replace(hour=hour, minute=minute, second=0, microsecond=0) - if when < cutoff or when > now: + try: + when = day.replace(hour=hour, minute=minute, second=0, microsecond=0) + except ValueError: + continue # a hand-edited bullet like "27:70 ..." must not crash the timer + if when < cutoff_minute or when > now: continue tokens = rest.split() event = tokens[0] if tokens else "" @@ -124,7 +137,9 @@ def gather_activity(omi_dir: Path | str, cutoff: datetime, now: datetime) -> Act if cutoff.date() < now.date(): actions = _journal_actions(omi_dir, now - timedelta(days=1), cutoff, now) + actions guard = [ - e for e in compliance.read_events() if (ts := _parse_ts(e.get("ts"))) and ts >= cutoff + e + for e in compliance.read_events() + if (ts := _parse_ts(e.get("ts"))) and cutoff <= ts <= now ] return Activity(actions=actions, guard_events=guard) @@ -145,10 +160,19 @@ def _llm_narrative(activity: Activity, since: str) -> str | None: ) try: result = subprocess.run( - [claude, "-p", prompt], capture_output=True, text=True, timeout=_LLM_TIMEOUT + [claude, "-p", prompt], + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=_LLM_TIMEOUT, ) except (subprocess.TimeoutExpired, OSError): return None + # A non-zero exit means claude printed a diagnostic, not a summary — don't + # adopt an error message as the day's narrative. + if result.returncode != 0: + return None text = (result.stdout or "").strip() return text or None @@ -247,9 +271,19 @@ def install_timer( ``every`` (e.g. ``15m``). ``Type=oneshot`` with no dependents, so a failing checkpoint never blocks anything.""" secs = int(parse_since(every).total_seconds()) + if secs < 60: + raise ValueError(f"--every must be at least 60s to avoid a tight timer loop: {every!r}") unit_dir = systemd_user_dir() unit_dir.mkdir(parents=True, exist_ok=True) - omind = shutil.which("omind") or "omind" + # systemd requires an ABSOLUTE path in ExecStart; a bare "omind" fallback + # produced a unit that fires and fails every interval while the install + # reported success (a cron job silently failing forever). Fail loudly here. + omind = shutil.which("omind") + if not omind: + raise FileNotFoundError( + "omind is not on PATH — cannot write an absolute systemd ExecStart. " + "Install omind so `which omind` resolves, then re-run." + ) service = ( "[Unit]\n" "Description=omind activity checkpoint\n" diff --git a/src/omind/cli.py b/src/omind/cli.py index 3bfb0ef..6c37165 100644 --- a/src/omind/cli.py +++ b/src/omind/cli.py @@ -839,12 +839,27 @@ def _run_checkpoint(args: argparse.Namespace) -> int: omi_dir = (args.vault / args.folder).expanduser() if args.action == "install-timer": - checkpoint.install_timer(args.every, args.vault, args.folder) + try: + checkpoint.install_timer(args.every, args.vault, args.folder) + except (ValueError, FileNotFoundError, OSError) as exc: + print(f"error: could not install timer: {exc}", file=sys.stderr) + return 1 return 0 if args.action == "uninstall-timer": - checkpoint.uninstall_timer() + try: + checkpoint.uninstall_timer() + except OSError as exc: + print(f"error: could not uninstall timer: {exc}", file=sys.stderr) + return 1 return 0 - action, filename = checkpoint.write_checkpoint(omi_dir, since=args.since, llm=args.llm) + # `checkpoint run` fires from a systemd timer. Its contract is "never raises + # into a timer" — a vault/store error here must be a clean non-zero exit with + # a message, not an unhandled traceback every interval. + try: + action, filename = checkpoint.write_checkpoint(omi_dir, since=args.since, llm=args.llm) + except Exception as exc: # noqa: BLE001 — a timer must degrade, never crash-loop + print(f"error: checkpoint failed: {exc}", file=sys.stderr) + return 1 print(f"{action} {filename}") return 0 diff --git a/src/omind/compliance.py b/src/omind/compliance.py index 547c2f5..35ff23e 100644 --- a/src/omind/compliance.py +++ b/src/omind/compliance.py @@ -97,7 +97,11 @@ def read_events(limit: int | None = None) -> list[dict[str, Any]]: """Parse the compliance log into records, newest last. Skips bad lines; never raises. ``limit`` keeps only the most recent N records.""" try: - lines = compliance_log_path().read_text(encoding="utf-8").splitlines() + # errors="replace": a single torn multibyte sequence (a short os.write + # under ENOSPC) is a UnicodeDecodeError (a ValueError, NOT an OSError), so + # strict decoding made read_events raise forever and took down the + # checkpoint timer / doctor / corpus export until the log was hand-repaired. + lines = compliance_log_path().read_text(encoding="utf-8", errors="replace").splitlines() except OSError: return [] events: list[dict[str, Any]] = [] diff --git a/src/omind/guard.py b/src/omind/guard.py index 8b88863..99c1620 100644 --- a/src/omind/guard.py +++ b/src/omind/guard.py @@ -416,7 +416,9 @@ def clear_all_gates() -> None: cannot know the live session id, so a single-session clear would miss it). Also reaps the legacy ``/tmp`` sentinels. Never raises.""" state = paths.state_dir() - for pattern in ("gate-*", "reclose-*", "pending-*", "offtopic-*", "git-fresh-*"): + # ``turn-*`` holds the captured raw prompt; it was never reaped, so those + # files accumulated unboundedly (and leaked prompt text) across sessions. + for pattern in ("gate-*", "reclose-*", "pending-*", "offtopic-*", "git-fresh-*", "turn-*"): try: stale = list(state.glob(pattern)) except OSError: @@ -446,27 +448,58 @@ def clear_all_gates() -> None: _REPO_TEST_RE = re.compile( r"(?:^|[;&|\n(]\s*)(?:uv|pytest|python|tox|nox|hatch|npm|pnpm|yarn|cargo|go|make)\b" ) -_GIT_FRESH_ONLY_RE = re.compile( - r"^\s*git\s+(?:fetch\b[^;&|\n]*|pull\b[^;&|\n]*(?:--ff-only|--rebase)[^;&|\n]*)\s*$" +# Optional leading git global options (``-C ``, ``-c key=val``) so a +# freshness command run with an explicit repo dir — ``git -C fetch`` — is +# still recognised as freshness (it previously required a bare ``git fetch``). +_GIT_GLOBAL_OPTS = r"(?:-C[ \t]+\S+[ \t]+|-c[ \t]+\S+[ \t]+)*" +# One git subcommand that ESTABLISHES freshness (a fetch, or an ff-only/rebase +# pull). ``[^|>&;\n]*`` keeps the whole subcommand free of pipes/redirects/chains +# so a piped write (``git fetch | tee x``) is never mistaken for a pure fetch. +_GIT_FRESH_SUB_RE = re.compile( + rf"^git[ \t]+{_GIT_GLOBAL_OPTS}" + r"(?:fetch(?:[ \t][^|>&;\n]*)?|pull[^|>&;\n]*(?:--ff-only|--rebase)[^|>&;\n]*)$" ) -_GIT_STATUS_ONLY_RE = re.compile(r"^\s*git\s+(?:status|rev-parse|branch|remote)\b[^;&|\n]*$") -_GLOBAL_CONFIG_RE = re.compile( - r"(?:^|[\s'\"=:/])(?:~?/)?(?:" - r"\.codex/(?:AGENTS\.md|hooks\.json|config\.toml)|" - r"\.claude/(?:settings\.json|hooks/[^ \t\n'\";]+)|" - r"\.hermes/(?:config\.yaml|hooks/[^ \t\n'\";]+|AGENTS\.md)|" - r"\.config/opencode/(?:opencode\.json|plugin/omi-guard\.js)|" - r"\.gemini/settings\.json|" - r"\.openclaw/(?:openclaw\.json|omind/MEMORY\.md)" - r")" +# A read-only git subcommand (inspection). Same no-pipe/redirect constraint. +_GIT_READONLY_SUB_RE = re.compile( + rf"^git[ \t]+{_GIT_GLOBAL_OPTS}" + r"(?:status|rev-parse|branch|remote|log|show|diff|for-each-ref|symbolic-ref|" + r"describe|config[ \t]+--get)(?:[ \t][^|>&;\n]*)?$" ) +# GLOBAL (home-anchored) agent config files/dirs. A project-local +# ``/.claude/settings.json`` is ordinary version-controlled config an +# agent edits routinely and must NOT trip the global-mutation gate — hence the +# resolve-against-$HOME check in :func:`_is_global_config_path`, not a text regex +# that couldn't tell ``~/.claude`` from ``/.claude``. +_GLOBAL_CONFIG_FILES = frozenset( + { + ".codex/AGENTS.md", + ".codex/hooks.json", + ".codex/config.toml", + ".claude/settings.json", + ".hermes/config.yaml", + ".hermes/AGENTS.md", + ".config/opencode/opencode.json", + ".config/opencode/plugin/omi-guard.js", + ".gemini/settings.json", + ".openclaw/openclaw.json", + ".openclaw/omind/MEMORY.md", + } +) +_GLOBAL_CONFIG_DIRS = (".claude/hooks/", ".hermes/hooks/") _GLOBAL_AUTH_RE = re.compile( - r"\b(?:please\s+)?(?:" - r"make|modify|edit|write|install|update|change|patch|apply|do it|go ahead|" - r"proceed|send it" + r"\b(?:" + r"make|modify|edit|write|install|update|change|patch|apply|fix|add|create|" + r"remove|delete|configure|enable|disable|wire|register|provision|rename|" + r"set\s*up|set|do it|go ahead|proceed|send it" r")\b", re.IGNORECASE, ) +# Negation immediately before an auth verb — "don't change anything", "no need to +# update" — must NOT read as authorization. +_AUTH_NEGATION_RE = re.compile( + r"\b(?:don'?t|do\s+not|never|without|no\s+need\s+to|avoid|instead\s+of)\s*$", + re.IGNORECASE, +) _STRONG_ACTION_AUTH_RE = re.compile( r"\b(?:do it|go ahead|proceed|send it|approved|authorized|" r"you have (?:my )?(?:permission|authorization)|" @@ -474,9 +507,14 @@ def clear_all_gates() -> None: re.IGNORECASE, ) _CAPABILITY_QUESTION_RE = re.compile( - r"^\s*(?:hey[, ]+|please[, ]+)?(?:can|could|would|will)\s+you\b", + r"^\s*(?:\w+[,:]\s+)?(?:hey[, ]+|please[, ]+)?(?:can|could|would|will)\s+you\b", re.IGNORECASE, ) +# A REAL output redirect to a file: ``> f`` / ``>> f`` — but NOT ``2>&1`` (fd +# dup), NOT ``2>/dev/null``, and NOT ``->`` / ``=>`` (arrows in code/strings). +# Distinguishing these is what stops ``pytest 2>&1 | tail`` from being read as a +# file-writing "side effect" and false-blocking a read-only capability question. +_FILE_REDIRECT_RE = re.compile(r"(?&\d])>>?[ \t]*(?!&)(?!/dev/null\b)[^\s&|>]") _GLOBAL_MUTATING_BASH_RE = re.compile( r"(?:^|[;&|\n(]\s*)(?:" r"chmod|chown|cp|dd|ed|ex|install|mv|rm|tee|touch|truncate|" @@ -484,7 +522,7 @@ def clear_all_gates() -> None: r"python3?\b[^;&|\n]*(?:write_text|write_bytes|open\([^;&|\n]*[\"']a|" r"open\([^;&|\n]*[\"']w)|" r"node\b[^;&|\n]*(?:writeFile|appendFile)" - r")\b|>|>>" + r")\b" ) _SHELL_SIDE_EFFECT_RE = re.compile( r"(?:^|[;&|\n(]\s*)(?:" @@ -495,10 +533,79 @@ def clear_all_gates() -> None: r"kubectl\s+(?:apply|delete|rollout\s+restart|scale)|" r"docker\s+(?:compose\s+)?(?:up|down|restart|rm)|" r"chmod|chown|cp|dd|install|mv|rm|tee|touch|truncate" - r")\b|>|>>" + r")\b" ) +def _split_simple_commands(command: str) -> list[str]: + """Split a shell command into its ``&&`` / ``||`` / ``;`` / newline parts.""" + return [c.strip() for c in re.split(r"&&|\|\||;|\n", command) if c.strip()] + + +def _is_freshness_command(command: str) -> bool: + """True when the command is composed ONLY of safe git read/fetch subcommands + and includes at least one fetch / ff-pull — so it establishes freshness and + is itself harmless. Accepts ``git -C fetch --all --prune`` and + compound forms like ``git fetch --all --prune && git status -sb`` (the exact + remediation the block message tells the agent to run). A part that is NOT a + safe git read (``git fetch && pytest``, ``git fetch | tee x``) disqualifies + the whole command, so it can never grant freshness to a piggybacked action.""" + parts = _split_simple_commands(command) + if not parts: + return False + fresh = False + for part in parts: + if _GIT_FRESH_SUB_RE.match(part): + fresh = True + elif not _GIT_READONLY_SUB_RE.match(part): + return False + return fresh + + +def _is_readonly_git_command(command: str) -> bool: + """True when every part of the command is a safe git read/fetch (so it needs + no note-read / freshness of its own).""" + parts = _split_simple_commands(command) + return bool(parts) and all( + _GIT_FRESH_SUB_RE.match(p) or _GIT_READONLY_SUB_RE.match(p) for p in parts + ) + + +def _is_global_config_path(raw: str) -> bool: + """True only for a GLOBAL (home-anchored) agent config file — never a + project-local ``/.claude/…`` even when the repo lives under $HOME.""" + try: + p = Path(raw).expanduser() + if not p.is_absolute(): + p = Path.cwd() / p + except (OSError, RuntimeError): + return False + candidates = {p} + with contextlib.suppress(OSError): + candidates.add(p.resolve()) + homes = {Path.home()} + with contextlib.suppress(OSError): + homes.add(Path.home().resolve()) + for cand in candidates: + for home in homes: + try: + rel = cand.relative_to(home).as_posix() + except ValueError: + continue + if rel in _GLOBAL_CONFIG_FILES or any(rel.startswith(d) for d in _GLOBAL_CONFIG_DIRS): + return True + return False + + +def _command_targets_global_config(command: str) -> bool: + """True when a shell command references a global config path via ``~/`` or the + absolute home dir (a project-relative path in the command does not count).""" + haystack = command.replace("\\", "/") + home = str(Path.home()) + targets = [*_GLOBAL_CONFIG_FILES, *_GLOBAL_CONFIG_DIRS] + return any(f"~/{t}" in haystack or f"{home}/{t}" in haystack for t in targets) + + def _opt_in_satisfied(opt_in: str, command: str) -> bool: """True only when the ``VAR=VALUE`` opt-in token appears as a REAL leading environment assignment — at the command start, right after a shell separator @@ -513,8 +620,14 @@ def _opt_in_satisfied(opt_in: str, command: str) -> bool: multi-line script (``…\n OMI_PUSH_GITHUB=1 git push …``) is legitimate and must be recognised — omitting ``\\n`` from the separator class wrongly rejected it (3.0.2). A plain space is NOT a separator, so a mid-line ``echo OMI_SUDO_OK=1`` - still doesn't count.""" - pattern = r"(?:^|[;&|\n]|\benv)\s*" + re.escape(opt_in) + r"(?=\s|$)" + still doesn't count. + + The optional ``env `` prefix must ITSELF be at command position — otherwise + ``echo "use env OMI_SUDO_OK=1" && sudo …`` forged the opt-in from inside a + string (the ``\\benv``-anywhere bug) and skipped a hard rule.""" + pattern = ( + r"(?:^|[;&|\n])[ \t]*(?:env[ \t]+)?" + re.escape(opt_in) + r"(?=\s|$)" + ) return re.search(pattern, command) is not None @@ -561,7 +674,7 @@ def _is_repo_sensitive_action(action: dict[str, Any]) -> bool: if tool in _WRITE_TOOLS or tool in _READ_REVIEW_TOOLS: return True if tool == "Bash": - if _GIT_FRESH_ONLY_RE.match(command) or _GIT_STATUS_ONLY_RE.match(command): + if _is_readonly_git_command(command): return False if re.search( r"(?:^|[;&|\n(]\s*)git\s+(?:add|commit|push|merge|rebase|checkout|switch)\b", @@ -581,16 +694,18 @@ def _is_repo_sensitive_action(action: dict[str, Any]) -> bool: def _is_global_config_mutation(action: dict[str, Any]) -> bool: tool = str(action.get("tool") or "") - if tool not in _WRITE_TOOLS and tool != "Bash": + command = str(action.get("command") or "") + if tool in _WRITE_TOOLS: + # A write tool targets exactly one file — resolve it and gate only a + # GLOBAL (home-anchored) config, not a project-local /.claude/…. + return _is_global_config_path(_action_path(action)) + if tool != "Bash": return False - haystack = " ".join( - part for part in (str(action.get("command") or ""), _action_path(action)) if part - ).replace("\\", "/") - if not _GLOBAL_CONFIG_RE.search(haystack): + # Bash: require a home-anchored global-config path in the command AND a + # mutating verb or a real file redirect (a plain read is not a mutation). + if not _command_targets_global_config(command): return False - if tool in _WRITE_TOOLS: - return True - return bool(_GLOBAL_MUTATING_BASH_RE.search(str(action.get("command") or ""))) + return bool(_GLOBAL_MUTATING_BASH_RE.search(command) or _FILE_REDIRECT_RE.search(command)) def _turn_authorization_text(action: dict[str, Any], session: str) -> str: @@ -613,11 +728,21 @@ def _is_capability_question(text: str) -> bool: return bool(_CAPABILITY_QUESTION_RE.search(text)) +def _has_global_auth(text: str) -> bool: + """True when the turn text contains an authorizing verb that is NOT negated + just before it — so "don't change anything" / "no need to update" do not read + as authorization, while the expanded verb set ("fix"/"add"/"create"/...) does.""" + for m in _GLOBAL_AUTH_RE.finditer(text): + if not _AUTH_NEGATION_RE.search(text[: m.start()]): + return True + return False + + def _turn_has_explicit_global_auth(action: dict[str, Any], session: str) -> bool: text = _turn_authorization_text(action, session) if _is_capability_question(text): return _has_strong_action_auth(text) - return bool(_GLOBAL_AUTH_RE.search(text)) + return _has_global_auth(text) def _is_side_effect_action(action: dict[str, Any]) -> bool: @@ -628,7 +753,7 @@ def _is_side_effect_action(action: dict[str, Any]) -> bool: return True command = str(action.get("command") or "") if tool == "Bash" or command: - return bool(_SHELL_SIDE_EFFECT_RE.search(command)) + return bool(_SHELL_SIDE_EFFECT_RE.search(command) or _FILE_REDIRECT_RE.search(command)) return False @@ -668,7 +793,14 @@ def decide(action: dict[str, Any]) -> Verdict: for rule in policy.load_policy(): if rule.severity != policy.SEVERITY_HARD: continue - if not rule.compiled().search(command): + # A single malformed rule must never brick the guard on EVERY tool call: + # a pattern that fails to compile / errors mid-match is skipped, not + # raised. (Learned rules are also validated at load; this is the belt to + # that suspenders, covering a bad seed rule or a catastrophic pattern.) + try: + if not rule.compiled().search(command): + continue + except re.error: continue if rule.opt_in and _opt_in_satisfied(rule.opt_in, command): continue @@ -678,7 +810,7 @@ def decide(action: dict[str, Any]) -> Verdict: rule_id=rule.id, ) - if repo is not None and _GIT_FRESH_ONLY_RE.match(command): + if repo is not None and _is_freshness_command(command): _record_git_freshness(session, repo, command) return Verdict(allow=True) diff --git a/src/omind/journal.py b/src/omind/journal.py index 2c24924..e2c7693 100644 --- a/src/omind/journal.py +++ b/src/omind/journal.py @@ -109,7 +109,7 @@ def migrate_journals(omi_dir: Path | str) -> list[str]: for stray in find_stray_journals(store.omi_dir): target = target_dir / stray.name if target.is_file(): - bullets = action_bullets(stray.read_text(encoding="utf-8")) + bullets = action_bullets(stray.read_text(encoding="utf-8", errors="replace")) if bullets: with target.open("a", encoding="utf-8") as fh: fh.write("\n".join(bullets) + "\n") @@ -260,7 +260,7 @@ def rollup_journals( archived_dated.append((day, path)) stats = JournalStats() for _, path in [*archived_dated, *dated_paths]: - _tally(path.read_text(encoding="utf-8"), stats) + _tally(path.read_text(encoding="utf-8", errors="replace"), stats) days = sorted({day.isoformat() for day, _ in [*archived_dated, *dated_paths]}) filename = rollup_name(wk) _atomic_write(directory / filename, render_rollup(wk, days, stats)) diff --git a/src/omind/lint.py b/src/omind/lint.py index 39db2d6..00850a7 100644 --- a/src/omind/lint.py +++ b/src/omind/lint.py @@ -32,11 +32,14 @@ from pathlib import Path from omind.paths import RESERVED_FILENAMES -from omind.store import _WIKILINK_RE, NoteFields, parse_note +from omind.store import _FENCE_RE, _WIKILINK_RE, NoteFields, parse_note #: Titles overlapping at/above this Jaccard score are flagged as near-duplicates. _NEAR_DUP = 0.6 _TOKEN_RE = re.compile(r"[a-z0-9]+") +#: A date / week / bare number in a title — the distinguishing part of a periodic +#: note series ("Worklog 2026-06-29" vs "…-30"), which must NOT read as a dupe. +_DATE_RE = re.compile(r"\d{4}-w?\d{1,2}(?:-\d{1,2})?|\bw?\d+\b", re.IGNORECASE) #: Title tokens too generic to anchor a duplicate match on. _STOP = frozenset( {"the", "and", "for", "with", "note", "omi", "memory", "cj", "cryptojones"} @@ -62,11 +65,32 @@ def _link_target(raw: str) -> str: return raw.split("|", 1)[0].split("#", 1)[0].strip() +def _strip_code(text: str) -> str: + """Blank out fenced code blocks and inline code so a ``[[wikilink]]`` quoted + in a code example isn't mistaken for a real link (a false broken-link error).""" + out: list[str] = [] + in_fence = False + fence_ch = "" + for line in text.splitlines(): + fence = _FENCE_RE.match(line.lstrip()) + if fence: + ch = fence.group(1)[0] + if not in_fence: + in_fence, fence_ch = True, ch + elif ch == fence_ch: + in_fence = False + continue + out.append("" if in_fence else re.sub(r"`[^`]*`", "", line)) + return "\n".join(out) + + def _outbound(text: str) -> set[str]: """Link targets named by the note body, original-case (deduped, blanks dropped). Resolution against :data:`known` is case-insensitive; the original - case is kept so a broken-link report shows the link as the author wrote it.""" - return {t for t in (_link_target(m) for m in _WIKILINK_RE.findall(text)) if t} + case is kept so a broken-link report shows the link as the author wrote it. + Links inside code fences/inline code are ignored.""" + body = _strip_code(text) + return {t for t in (_link_target(m) for m in _WIKILINK_RE.findall(body)) if t} def _title_tokens(title: str) -> frozenset[str]: @@ -79,6 +103,18 @@ def _jaccard(a: frozenset[str], b: frozenset[str]) -> float: return len(a & b) / len(a | b) +def _is_periodic_series(title_a: str, title_b: str) -> bool: + """True when two titles are identical except for a date / week / number — a + periodic note series (Worklog/journal/rollup), not a duplicated memory. Such + notes ``omind checkpoint`` creates one of per day, so without this every pair + scored 100% similar and lint --strict failed on a healthy vault.""" + stem_a = _DATE_RE.sub(" ", title_a.lower()).split() + stem_b = _DATE_RE.sub(" ", title_b.lower()).split() + dates_a = _DATE_RE.findall(title_a.lower()) + dates_b = _DATE_RE.findall(title_b.lower()) + return stem_a == stem_b and bool(dates_a or dates_b) and dates_a != dates_b + + @dataclass class _Note: path: Path @@ -87,12 +123,22 @@ class _Note: ids: frozenset[str] # stem + title, lowercased — how others link to this note -def _load(omi_dir: Path | str) -> list[_Note]: - """Parse every live (non-disabled, non-reserved) note once.""" +def _note_ids(path: Path, fields: NoteFields) -> set[str]: + ids = {path.stem.strip().lower()} + if fields.title.strip(): + ids.add(fields.title.strip().lower()) + return ids + + +def _load(omi_dir: Path | str) -> tuple[list[_Note], set[str]]: + """Parse the top-level live notes to lint, plus the set of ALL valid link + targets in the vault tree — including archived (soft-deleted) notes and notes + under ``Journal/`` — so a link to one of those is not a false broken-link.""" omi = Path(omi_dir) notes: list[_Note] = [] + known_extra: set[str] = set() if not omi.is_dir(): - return notes + return notes, known_extra for path in sorted(omi.glob("*.md")): if path.name in RESERVED_FILENAMES or path.name.startswith("."): continue @@ -101,21 +147,34 @@ def _load(omi_dir: Path | str) -> list[_Note]: except OSError: continue fields = parse_note(text) + ids = _note_ids(path, fields) if fields.disabled: + # Archived notes are restorable and remain valid link targets, but are + # not linted as sources (a normal delete only archives — every link to + # the archived note would otherwise become an error and flip --strict). + known_extra |= ids continue - ids = {path.stem.strip().lower()} - if fields.title.strip(): - ids.add(fields.title.strip().lower()) notes.append(_Note(path, fields, _outbound(text), frozenset(ids))) - return notes + # Notes in subfolders (Journal/, Journal/Archive/) are legitimate link + # targets too, though they are auto-generated and not linted as sources. + for path in sorted(omi.rglob("*.md")): + if path.parent == omi or path.name.startswith("."): + continue + try: + fields = parse_note(path.read_text(encoding="utf-8", errors="replace")) + except OSError: + continue + known_extra |= _note_ids(path, fields) + return notes, known_extra def lint_vault(omi_dir: Path | str) -> list[LintIssue]: """Every problem found in the vault, ordered error → warn → info then by note.""" - notes = _load(omi_dir) - # Every identifier any note can be linked by (+ reserved stems, which are - # legitimate link targets even though they're skipped as notes). + notes, known_extra = _load(omi_dir) + # Every identifier any note can be linked by (+ reserved stems, archived + # notes, and subfolder notes, which are legitimate link targets). known = {stem for path in RESERVED_FILENAMES for stem in (Path(path).stem.lower(),)} + known |= known_extra for n in notes: known |= n.ids linked: set[str] = set() # ids that at least one OTHER note links to @@ -141,15 +200,19 @@ def lint_vault(omi_dir: Path | str) -> list[LintIssue]: ) # Near-duplicate titles — each unordered pair reported once. - toks = [(_title_tokens(n.fields.title or n.path.stem), n) for n in notes] + toks = [(_title_tokens(n.fields.title or n.path.stem), n.fields.title or n.path.stem, n) + for n in notes] for i in range(len(toks)): for j in range(i + 1, len(toks)): + if _jaccard(toks[i][0], toks[j][0]) < _NEAR_DUP: + continue + if _is_periodic_series(toks[i][1], toks[j][1]): + continue # "Worklog 2026-06-29" vs "…-30": a dated series, not a dupe score = _jaccard(toks[i][0], toks[j][0]) - if score >= _NEAR_DUP: - a, b = sorted((toks[i][1].path.name, toks[j][1].path.name)) - issues.append( - LintIssue("near-duplicate", "info", f"{a} | {b}", f"titles {score:.0%} similar") - ) + a, b = sorted((toks[i][2].path.name, toks[j][2].path.name)) + issues.append( + LintIssue("near-duplicate", "info", f"{a} | {b}", f"titles {score:.0%} similar") + ) rank = {"error": 0, "warn": 1, "info": 2} issues.sort(key=lambda x: (rank.get(x.severity, 9), x.kind, x.note)) diff --git a/src/omind/merge.py b/src/omind/merge.py index 1f1680e..41a8237 100644 --- a/src/omind/merge.py +++ b/src/omind/merge.py @@ -223,7 +223,13 @@ def merge_fields(base: NoteFields, ours: NoteFields, theirs: NoteFields) -> Merg elif o_rev is None or t_rev is None: ours_wins = t_rev is None # a stamped edit beats an unstamped one else: - ours_wins = o_rev.sort_key() > t_rev.sort_key() + ok, tk = o_rev.sort_key(), t_rev.sort_key() + # Equal rev identity with differing content (reachable when an unstamped + # mutation re-uses a rev — e.g. an index-description migration on a node + # whose config failed to load) must NOT resolve by side, or merge(A,B) + # and merge(B,A) diverge and the mesh never converges. Fall back to the + # symmetric content tiebreak used for unversioned edits. + ours_wins = None if ok == tk else ok > tk def scalar(name: str) -> str | bool: b, o, t = getattr(base, name), getattr(ours, name), getattr(theirs, name) @@ -235,7 +241,7 @@ def scalar(name: str) -> str | bool: return o # type: ignore[no-any-return] if ours_wins is None: winner = max(o, t) - messages.append(f"{name}: concurrent unversioned edits; kept {winner!r}") + messages.append(f"{name}: concurrent edits at equal/absent rev; kept {winner!r}") return winner # type: ignore[no-any-return] messages.append(f"{name}: last-writer-wins by rev") return o if ours_wins else t # type: ignore[no-any-return] @@ -263,6 +269,10 @@ def scalar(name: str) -> str | bool: connections=_union3(base.connections, ours.connections, theirs.connections), action_items=_merge_actions(base.action_items, ours.action_items, theirs.action_items), references=_union3(base.references, ours.references, theirs.references), + # YAML frontmatter and lead prose are hand-curated content the driver + # must never eat; carry them through with the same symmetric LWW rule. + frontmatter=str(scalar("frontmatter")), + lead=str(scalar("lead")), rev=merged_rev, disabled=bool(scalar("disabled")), ) diff --git a/src/omind/mesh.py b/src/omind/mesh.py index 9bfb056..3e22bdd 100644 --- a/src/omind/mesh.py +++ b/src/omind/mesh.py @@ -58,10 +58,21 @@ class MeshError(Exception): .omi-tombstones merge=union """ -#: Never replicate the lock or torn temp files. +#: Never replicate the lock, torn temp files, or Obsidian's volatile per-machine +#: app state. ``.obsidian/workspace*.json`` (and the caches) are rewritten +#: constantly and DIFFERENTLY on every machine; with ``git add -A`` committing +#: them and no merge driver, two nodes with Obsidian open produce a genuine text +#: conflict that aborts the WHOLE peer merge every cycle — silently stopping +#: replication. Ignoring them is what keeps the mesh converging. GITIGNORE = """\ .omi.lock .tmp-* +.obsidian/workspace.json +.obsidian/workspace-mobile.json +.obsidian/workspace +.obsidian/cache +.obsidian/.DS_Store +.trash/ """ _NODE_ID_RE = re.compile(r"[^A-Za-z0-9._-]+") @@ -73,9 +84,20 @@ def git( check: bool = True, timeout: float = GIT_TIMEOUT, ) -> subprocess.CompletedProcess[str]: - """Run git against the OMI repo, output captured, failures as MeshError.""" + """Run git against the OMI repo, output captured, failures as MeshError. + + Network calls run with ``GIT_TERMINAL_PROMPT=0`` and ssh ``BatchMode`` so an + unknown host key / credential prompt fails fast instead of hanging until the + timeout on every sync tick (the churning-seed-IP scenario). + """ + env = {**os.environ, "GIT_TERMINAL_PROMPT": "0"} + env.setdefault("GIT_SSH_COMMAND", "ssh -oBatchMode=yes") return run_command( - ["git", "-C", str(omi_dir), *args], error=MeshError, check=check, timeout=timeout + ["git", "-C", str(omi_dir), *args], + error=MeshError, + check=check, + timeout=timeout, + env=env, ) @@ -560,7 +582,7 @@ def _write_sync_state(omi_dir: Path, report: SyncReport) -> None: "conflicts": report.conflicts, "ok": report.ok, } - path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + _atomic_write(path, json.dumps(payload, indent=2) + "\n") except OSError: pass # advisory state for doctor; never fail a sync over it @@ -647,10 +669,20 @@ def sync( report.conflicts = conflict_scan(omi_dir) for ps in merged_peers: - push = git(omi_dir, "push", ps.name, f"HEAD:refs/omind/{node_id}", check=False) - ps.pushed = push.returncode == 0 + # A push can TIME OUT (a hung/blackholed peer); run_command raises on + # TimeoutExpired even with check=False. Catch it per-peer so one slow peer + # records its error and the loop still pushes the rest AND writes sync + # state — previously the MeshError escaped, skipping every later peer and + # leaving the state file stale. + try: + push = git(omi_dir, "push", ps.name, f"HEAD:refs/omind/{node_id}", check=False) + ps.pushed = push.returncode == 0 + if not ps.pushed: + ps.error = f"push: {_first_line(push.stderr or push.stdout)}" + except MeshError as exc: + ps.pushed = False + ps.error = f"push: {_first_line(str(exc)) or 'timed out'}" if not ps.pushed: - ps.error = f"push: {_first_line(push.stderr or push.stdout)}" log(f"peer {ps.name}: {ps.error}") _write_sync_state(omi_dir, report) for note_name in report.conflicts: @@ -935,6 +967,17 @@ def diagnose_mesh(config: Any) -> list[Any]: ) ) + state = read_sync_state(omi) + # Per-peer errors recorded by the last sync (fetch/merge/push failures). Doctor + # previously reported every fetched peer "ok" no matter how divergent and never + # read these, so a peer that conflict-aborts or is rejected every cycle looked + # healthy while the fleet silently diverged. + peer_errors: dict[str, str] = {} + if state is not None: + for entry in state.get("peers", []) or []: + if isinstance(entry, dict) and entry.get("error"): + peer_errors[str(entry.get("name") or "")] = str(entry.get("error")) + peer_map = peers(omi) if not peer_map: results.append( @@ -943,13 +986,24 @@ def diagnose_mesh(config: Any) -> list[Any]: ) ) for name in sorted(peer_map): + if name in peer_errors: + results.append( + CheckResult( + f"mesh_peer:{name}", + "fail", + f"peer {name}: last sync FAILED — {peer_errors[name]}", + ) + ) + continue ref = f"refs/remotes/{name}/main" if git(omi, "rev-parse", "--verify", ref, check=False).returncode != 0: results.append( CheckResult(f"mesh_peer:{name}", "warn", f"peer {name}: never fetched") ) continue - counts = git(omi, "rev-list", "--left-right", "--count", f"HEAD...{ref}").stdout.split() + counts = git( + omi, "rev-list", "--left-right", "--count", f"HEAD...{ref}", check=False + ).stdout.split() ahead, behind = (counts + ["0", "0"])[:2] results.append( CheckResult( @@ -959,7 +1013,6 @@ def diagnose_mesh(config: Any) -> list[Any]: ) ) - state = read_sync_state(omi) interval = cfg.interval_seconds if cfg else 300 if state is None: results.append( diff --git a/src/omind/omi-gate-reset.sh b/src/omind/omi-gate-reset.sh index e84bc0d..bb3c4d1 100644 --- a/src/omind/omi-gate-reset.sh +++ b/src/omind/omi-gate-reset.sh @@ -9,6 +9,9 @@ # dir, the same location guard.py uses. Never raises. set -u +# Default HOME so `set -u` can't crash the reset (which would leave the gate +# cleared from the previous turn); mirrors omi-guard.sh. +HOME="${HOME:-/tmp}" input="$(cat 2>/dev/null)" command -v jq >/dev/null 2>&1 || exit 0 sid="$(printf '%s' "$input" | jq -r '.session_id // empty' 2>/dev/null | tr -cd 'A-Za-z0-9._-')" @@ -18,6 +21,11 @@ rm -f "$STATE/gate-$sid" 2>/dev/null # Reset the verifier's per-turn re-close counter (its anti-wedge cap is measured # per turn; guard.py reads reclose-). Best-effort. rm -f "$STATE/reclose-$sid" 2>/dev/null +# Clear the per-turn pending-intent and the git-freshness record too, matching +# guard.begin_turn(). Omitting these made the "same-turn freshness check" +# actually per-SESSION — one fetch at 9am satisfied a 6pm commit (a fail-open of +# the freshness control) — and left stale pending intent feeding the verifier. +rm -f "$STATE/pending-$sid.txt" "$STATE/git-fresh-$sid.json" 2>/dev/null # Capture this turn's task so the verifier/retrieval can judge consult relevance # (guard.py reads turn-.txt). Best-effort; empty prompt is fine. mkdir -p "$STATE" 2>/dev/null diff --git a/src/omind/omi-guard-hermes.sh b/src/omind/omi-guard-hermes.sh index 7fe217a..7de4f62 100644 --- a/src/omind/omi-guard-hermes.sh +++ b/src/omind/omi-guard-hermes.sh @@ -13,6 +13,11 @@ # (no decision emitted = Hermes allows), so a broken hook never wedges the agent. set -u +# An unset HOME would trip `set -u` at the STATE expansion below and crash the +# hook (no decision emitted = Hermes allows = the guard silently disabled). +# Default it so the guard can't be turned off by a missing HOME (mirrors +# omi-guard.sh). +HOME="${HOME:-/tmp}" OMIND='__OMIND_BIN__' OMI_DIR='__OMI_DIR__' STATE="${XDG_STATE_HOME:-$HOME/.local/state}/omind" diff --git a/src/omind/omi-guard.sh b/src/omind/omi-guard.sh index 3ff5a2a..bfa7536 100644 --- a/src/omind/omi-guard.sh +++ b/src/omind/omi-guard.sh @@ -62,8 +62,16 @@ SENT="$STATE/gate-$sid" # turn's consults + relevance verdicts as JSON in this same file, and a second # consult in the turn must not wipe the first. case "$tool" in + # Navigation/listing tools (list-notes, list-tags, graph-*, backlinks) surface + # no note CONTENT, so — like re-reading index.md — they must NOT clear the gate + # (that was a verifier-proof gate-dodge). Allow them through without consulting. + mcp__omi__list-notes | mcp__omi__list-tags | mcp__omi__graph-* | mcp__omi__backlinks) + exit 0 ;; mcp__omi__*) target="$(printf '%s' "$input" | jq -r '.tool_input.name // .tool_input.query // .tool_input.q // .tool_input.file_path // .tool_input.path // empty' 2>/dev/null)" + # An empty target means a contentless call (nothing to consult); allow it but + # do not clear the gate. + [ -z "$target" ] && exit 0 jq -nc --arg t "$tool" --arg s "$sid" --arg target "$target" \ '{tool:$t, command:"", session:$s, is_omi_consult:true, consult_target:$target}' 2>/dev/null \ | "$OMIND" guard check >/dev/null 2>&1 diff --git a/src/omind/paths.py b/src/omind/paths.py index f5ccf1f..a92aee1 100644 --- a/src/omind/paths.py +++ b/src/omind/paths.py @@ -9,8 +9,10 @@ from __future__ import annotations +import contextlib import hashlib import os +import tempfile from pathlib import Path MEMORY_TEMPLATE_FILENAME = "Memory Template.md" @@ -41,6 +43,41 @@ JOURNAL_GLOB = f"{JOURNAL_PREFIX} *.md" +def atomic_write_text(path: Path, text: str, *, mode: int | None = None) -> None: + """Write ``text`` to ``path`` atomically: same-dir temp file + ``os.replace``. + + Used for every managed config/hook write (settings.json, config.toml, hook + scripts, backup.json, the provision manifest). A plain ``path.write_text`` + truncates in place, so a crash / OOM / ENOSPC mid-write leaves a torn file — + which for a harness config means a bricked agent and for ``omi-guard.sh`` + means every tool call is denied. The temp file + rename makes a concurrent + reader see either the old file or the new one in full, and the directory + fsync makes the rename itself durable across a power loss. + """ + directory = path.parent + directory.mkdir(parents=True, exist_ok=True) + fd, tmp = tempfile.mkstemp(dir=directory, prefix=".tmp-", suffix=path.suffix or ".tmp") + try: + with os.fdopen(fd, "w", encoding="utf-8") as fh: + fh.write(text) + fh.flush() + os.fsync(fh.fileno()) + if mode is not None: + with contextlib.suppress(OSError): + os.chmod(tmp, mode) + os.replace(tmp, path) + with contextlib.suppress(OSError, AttributeError): + dir_fd = os.open(directory, os.O_RDONLY) + try: + os.fsync(dir_fd) + finally: + os.close(dir_fd) + except BaseException: + with contextlib.suppress(OSError): + os.unlink(tmp) + raise + + def state_dir() -> Path: """omind's state directory: ``$XDG_STATE_HOME/omind`` or ``~/.local/state/omind``.""" env = os.environ.get("XDG_STATE_HOME") diff --git a/src/omind/policy.py b/src/omind/policy.py index 5c6da2b..dcfea02 100644 --- a/src/omind/policy.py +++ b/src/omind/policy.py @@ -42,17 +42,30 @@ TIER_SUDO = "sudo" TIER_LEARNED = "learned" +#: Shell wrapper/keyword tokens that transparently precede the real command, so +#: the anchored token is still in command position after them: +#: ``if/while … ; then sudo …``, ``exec sudo …``, ``nohup sudo …``, +#: ``xargs sudo …``, ``time sudo …``. Without these, ``if true; then sudo rm`` +#: sailed past the sudo hard rule (a fail-open of a hard control). +_CMD_WRAPPERS = r"then|do|else|elif|exec|nohup|command|time|builtin|xargs" + #: Prefix that anchors a ``match="command"`` pattern to COMMAND POSITION: the #: command start, or immediately after a shell separator (``;`` ``&`` ``|`` #: NEWLINE ``(`` backtick — single chars suffice since ``&&`` / ``||`` / ``$(`` #: all END in a char in the class), skipping any leading ``VAR=val`` environment -#: assignments. This is how a token like ``sudo`` is matched only when it is the -#: command being run — not when it appears as a grep arg, a path segment, a +#: assignments and shell wrapper keywords (:data:`_CMD_WRAPPERS`), and an +#: optional absolute/relative path to the binary (``/usr/bin/sudo``, +#: ``./x/sudo``). This is how a token like ``sudo`` is matched only when it is +#: the command being run — not when it appears as a grep arg, a path segment, a #: filename, a commit message, or a ``pass show sudo/...`` value (the #98/#108 #: false-positive class). It mirrors the leading-assignment idea already proven #: in ``guard._opt_in_satisfied``. Use ``[ \t]`` (not ``\s``) so the #: assignment-skip never crosses a newline into another command. -_CMD_POSITION = r"(?:^|[\n;&|`(])[ \t]*(?:\w+=\S*[ \t]+)*" +_CMD_POSITION = ( + r"(?:^|[\n;&|`(])[ \t]*" + r"(?:(?:\w+=\S*|" + _CMD_WRAPPERS + r")[ \t]+)*" + r"(?:[./][^\s;&|`()]*/)?" +) @dataclass @@ -101,7 +114,11 @@ def label(self) -> str: SEED_RULES: tuple[Rule, ...] = ( Rule( id="gh-auth-setup-git", - pattern=r"\bgh\s+auth\s+setup-git\b", + # match="command" (#101): anchor to command position so `grep -rn "gh + # auth setup-git"`, a commit message, or a heredoc writing this rule no + # longer false-block — routine when working on omind itself. + pattern=r"gh\s+auth\s+setup-git\b", + match="command", message=( "never 'gh auth setup-git'. GitHub auth = the gh-YOLO PAT from pass via " "a one-shot (per-command) credential helper. Read OMI: github-auth-ssh." @@ -109,7 +126,8 @@ def label(self) -> str: ), Rule( id="gh-repo-delete", - pattern=r"\bgh\s+repo\s+delete\b", + pattern=r"gh\s+repo\s+delete\b", + match="command", message=( "never delete a repo from a hook-reachable command. Typed-name " "confirmation only. Read OMI: Operational Rules - Git Repos and Secrets." @@ -121,7 +139,9 @@ def label(self) -> str: # `gh api repos/o/r -X DELETE` (path before method) is caught as well as # `gh api -X DELETE repos/o/r`. Both lookaheads stay within one simple # command (no pipe/;/&), so an unrelated later command can't trip it. + # Command-anchored (#101) so the phrase in a grep/commit message is safe. pattern=r"gh\s+api(?=[^|;&]*(?:-X\s*|--method\s*)DELETE)(?=[^|;&]*repos/)", + match="command", message=( "never DELETE a repo via the API. Typed-name confirmation only. " "Read OMI: Operational Rules - Git Repos and Secrets." @@ -136,6 +156,7 @@ def label(self) -> str: r"curl(?=[^|;&]*(?:-X\s*|--request\s*)DELETE)" r"(?=[^|;&]*api\.github\.com/repos/)" ), + match="command", message=( "never DELETE a GitHub repo/resource via the raw API. Use the reviewed " "path; typed-name confirmation only. Read OMI: Operational Rules - Git " @@ -151,7 +172,7 @@ def label(self) -> str: # `sudo …`, `; sudo …`, `a && sudo …`, `a | sudo …`, `$(sudo …)`, and # `FOO=1 sudo …` still block. `fleet-sudo` never matches (it is not a # command-position `sudo` token), so no lookbehind is needed. - pattern=r"sudo\b", + pattern=r"sudo(?:edit)?\b", match="command", message=( "raw sudo is blocked — run `fleet-sudo ` instead (it reads the " @@ -203,15 +224,35 @@ def seed_policy_path() -> Path: def _rule_from_dict(data: dict[str, object]) -> Rule | None: """Build a Rule from on-disk data, dropping unknown keys. ``None`` if it lacks the required ``id``/``pattern``/``message`` (a corrupt entry is skipped, - never fatal).""" + never fatal). + + A learned/hand-edited rule whose ``pattern`` does not compile — or matches + the empty string (a trailing ``|``) — would crash or hard-block the guard on + EVERY tool call (a bricked machine). It is dropped here so a bad table can + never reach the guard hot path. A wrong-typed ``severity`` (a JSON number) + that would silently demote a hard rule to non-blocking is likewise rejected. + """ kwargs = {k: v for k, v in data.items() if k in _RULE_FIELDS} required = ("id", "pattern", "message") if not all(isinstance(kwargs.get(k), str) and kwargs.get(k) for k in required): return None + # Optional string fields must be strings if present (a JSON number for + # ``severity`` would demote a hard rule; a bad ``match`` mode would confuse + # the anchor logic). + for key in ("severity", "tier", "match", "source", "created"): + if key in kwargs and not isinstance(kwargs[key], str): + return None try: - return Rule(**kwargs) # type: ignore[arg-type] + rule = Rule(**kwargs) # type: ignore[arg-type] except (TypeError, ValueError): return None + try: + compiled = rule.compiled() + except re.error: + return None + if compiled.search(""): # matches everything → would block every action + return None + return rule def _rule_to_dict(rule: Rule) -> dict[str, object]: diff --git a/src/omind/provision.py b/src/omind/provision.py index 0011e8f..a6d8f06 100644 --- a/src/omind/provision.py +++ b/src/omind/provision.py @@ -121,7 +121,7 @@ def write_provision_manifest() -> None: with contextlib.suppress(OSError): _guard_test_isolation(path) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(payload, indent=2) + "\n") def read_provision_manifest() -> dict[str, Any]: @@ -352,7 +352,7 @@ def _write_if_absent(self, path: Path, content: str) -> None: if not self.config.dry_run: _guard_test_isolation(path) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content, encoding="utf-8") + paths.atomic_write_text(path, content) def _write_managed(self, path: Path, content: str) -> None: """Write a Managed-by-omind file, refreshing it whenever its content drifts. @@ -375,7 +375,7 @@ def _write_managed(self, path: Path, content: str) -> None: if not self.config.dry_run: _guard_test_isolation(path) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content, encoding="utf-8") + paths.atomic_write_text(path, content) # -- steps -------------------------------------------------------------- @@ -718,7 +718,7 @@ def ensure_hooks_installed(self) -> None: if not self.config.dry_run: _guard_test_isolation(path) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def ensure_guard_hook_installed(self) -> None: """Idempotently register the PreToolUse(Bash) guard hooks. @@ -771,7 +771,7 @@ def ensure_guard_hook_installed(self) -> None: if not self.config.dry_run: _guard_test_isolation(path) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def _remove_legacy_omi_guard(self) -> None: """Delete the retired hand-rolled ``omi-git-guard.sh`` prototype if present, @@ -912,7 +912,7 @@ def ensure_omi_guard_installed(self) -> None: if not self.config.dry_run: _guard_test_isolation(path) path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") + paths.atomic_write_text(path, json.dumps(data, indent=2) + "\n") def verify(self) -> None: if self.config.dry_run: diff --git a/src/omind/secret-output-guard.sh b/src/omind/secret-output-guard.sh index 4a8c9a7..393a96f 100755 --- a/src/omind/secret-output-guard.sh +++ b/src/omind/secret-output-guard.sh @@ -32,14 +32,25 @@ set -u input="$(cat 2>/dev/null)" [ -z "$input" ] && exit 0 -command -v jq >/dev/null 2>&1 || exit 0 # omi-guard fails-closed for Bash separately +# jq parses the event; without it this specific guard can't read the command. +# NOTE: the omind policy has no secret-output rules, so no-jq means NO +# secret-output protection here (the general omi-guard's fail-closed-for-Bash +# path does not cover this leak class). Install jq. +command -v jq >/dev/null 2>&1 || exit 0 cmd="$(printf '%s' "$input" | jq -r '.tool_input.command // empty' 2>/dev/null)" [ -z "$cmd" ] && exit 0 -# explicit, audited override -printf '%s' "$cmd" | grep -q 'OMI_SECRET_OK=1' && exit 0 +# Explicit, audited override — must be a REAL leading assignment (start or after +# a shell separator, optionally via `env`), not a substring forged in a comment +# or a quoted string (which would silently disable the guard). +printf '%s' "$cmd" | grep -Eq '(^|[;&|])[[:space:]]*(env[[:space:]]+)?OMI_SECRET_OK=1([[:space:]]|$)' && exit 0 -READ='pass[[:space:]]+show([[:space:]]|$)|pass[[:space:]]+[A-Za-z0-9_.@-]+/|gh[[:space:]]+auth[[:space:]]+token([[:space:]]|$)' +# Anchor `pass`/`gh` to COMMAND POSITION — start, or after a shell separator +# (`;` `&` `|` `(`), past any leading `VAR=val` / `env` — so `grep "pass tests/"`, +# a commit message, and "bypass proxy/" no longer false-positive, while a real +# `pass show`, `; pass work/x`, or `env FOO=1 pass show` still matches. +BND='(^|[;&|(])[[:space:]]*([A-Za-z_][A-Za-z0-9_]*=[^[:space:];&|]*[[:space:]]+|env[[:space:]]+)*' +READ="${BND}pass[[:space:]]+show([[:space:]]|\$)|${BND}pass[[:space:]]+[A-Za-z0-9_.@-]+/|${BND}gh[[:space:]]+auth[[:space:]]+token([[:space:]]|\$)" block() { { @@ -53,7 +64,7 @@ block() { } # 1) A literal credential pasted into the command text. -if printf '%s' "$cmd" | grep -Eq '(gh[pousr]|ghu)_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|glpat-[A-Za-z0-9_-]{18,}|xox[baprs]-[A-Za-z0-9-]{10,}|AKIA[0-9A-Z]{16}|-----BEGIN [A-Z ]*PRIVATE KEY-----'; then +if printf '%s' "$cmd" | grep -Eq 'gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|glpat-[A-Za-z0-9_-]{18,}|xox[baprs]-[A-Za-z0-9-]{10,}|AKIA[0-9A-Z]{16}|-----BEGIN [A-Z ]*PRIVATE KEY-----'; then block "the command text contains a literal credential/token." fi @@ -68,12 +79,22 @@ if printf '%s' "$cmd" | grep -Eq '\b(echo|printf|print|cat|tee|head|tail|xxd|od| block "a secret read is piped into a command that prints to stdout." fi -# 3) A bare / piped secret-read that is not captured and not redirected. -# Strip command substitutions; whatever read remains runs to the shell stdout. -bare="$(printf '%s' "$cmd" | sed -E 's/\$\([^)]*\)//g; s/`[^`]*`//g')" +# 3) A bare / piped secret-read that is not captured and not safely redirected. +# Flatten newlines first so a multi-line `TOK=$(\n pass show x\n)` capture is +# stripped (line-based sed missed it and false-blocked the captured read), +# then strip command substitutions; whatever read remains runs to shell stdout. +flat="$(printf '%s' "$cmd" | tr '\n' ' ')" +bare="$(printf '%s' "$flat" | sed -E 's/\$\([^)]*\)//g; s/`[^`]*`//g')" if printf '%s' "$bare" | grep -Eq "$READ"; then - if printf '%s' "$bare" | grep -Eq '>[[:space:]]*/dev/null|>>?[[:space:]]*[^|&[:space:]]'; then - : # redirected off the transcript (to /dev/null or a file) — allow + if printf '%s' "$bare" | grep -Eq '\|'; then + # The read's stdout is piped into another command -> transcript. Not safe, + # even with a `2>/dev/null` stderr redirect (the exact `pass show X 2>/dev/null + # | head` leak that a bare `>`-means-redirected check waved through). + block "a secret read is piped to another command; its value reaches the transcript." + elif printf '%s' "$bare" | grep -Eq '(^|[^0-9])(1?>|&>)[[:space:]]*([^|&[:space:]]|/dev/null)'; then + # STDOUT redirected off the transcript (to a file or /dev/null) — allow. A + # bare `2>...` only redirects STDERR, so it does NOT count here. + : else block "a secret read (pass show / pass / gh auth token) prints to stdout." fi diff --git a/src/omind/store.py b/src/omind/store.py index f11f303..322eb62 100644 --- a/src/omind/store.py +++ b/src/omind/store.py @@ -54,7 +54,9 @@ def _atomic_write(path: Path, text: str) -> None: """Write ``text`` to ``path`` atomically: same-dir temp file + ``os.replace``. On POSIX ``os.replace`` is an atomic rename, so a concurrent reader sees - either the old file or the new one in full — never a half-written file. + either the old file or the new one in full — never a half-written file. The + parent directory is fsynced after the rename so the rename itself is durable + across a power loss, not just the file's data blocks. """ directory = path.parent directory.mkdir(parents=True, exist_ok=True) @@ -65,13 +67,46 @@ def _atomic_write(path: Path, text: str) -> None: fh.flush() os.fsync(fh.fileno()) os.replace(tmp, path) + _fsync_dir(directory) except BaseException: with contextlib.suppress(OSError): os.unlink(tmp) raise + +def _fsync_dir(directory: Path) -> None: + """Best-effort fsync of a directory so a rename into it is durable. + + Silently skipped where the platform can't open a directory fd (Windows). + """ + with contextlib.suppress(OSError, AttributeError): + dir_fd = os.open(directory, os.O_RDONLY) + try: + os.fsync(dir_fd) + finally: + os.close(dir_fd) + + +def _read_text(path: Path) -> str: + """Read a note/index file tolerantly. + + Uses ``utf-8-sig`` so a leading BOM (Windows Notepad) is stripped rather + than glued to the ``# Title`` line, and ``errors="replace"`` so a single + note with a stray non-UTF-8 byte (an external editor, a mesh sync) can never + raise ``UnicodeDecodeError`` and take down listing/search/writes vault-wide. + """ + return path.read_text(encoding="utf-8-sig", errors="replace") + # \w is Unicode-aware for str patterns, so non-Latin tags (e.g. #память) round-trip. _TAG_RE = re.compile(r"#(\w[\w/-]*)") +# Section-splitting primitives. Heading detection is fence-aware (see +# :func:`_scan_note`) so a ``##`` inside a fenced code block is body text. +_FENCE_RE = re.compile(r"^(`{3,}|~{3,})") +_H1_RE = re.compile(r"^#\s+(.*)$") +_H2_RE = re.compile(r"^##\s+(.*)$") +# Longest filename we will create. Well under the common 255-*byte* limit so a +# long LLM-generated title raises a clean NoteError instead of ENAMETOOLONG. +_MAX_FILENAME_BYTES = 200 _WIKILINK_RE = re.compile(r"\[\[([^\]]+)\]\]") _ACTION_RE = re.compile(r"^\s*-\s*\[([ xX])\]\s?(.*)$") _BULLET_RE = re.compile(r"^\s*-\s+(.*)$") @@ -87,6 +122,14 @@ def _atomic_write(path: Path, text: str) -> None: # Per-day journal notes written by omind.hooks; auto-recorded noise that would # otherwise crowd hand-curated memories out of the capped index list. _JOURNAL_NOTE_RE = re.compile(rf"^{re.escape(JOURNAL_PREFIX)} .*\.md$") +# Case-folded reserved set: on a case-insensitive filesystem (APFS/NTFS) a note +# titled "Index" resolves to the same file as the generated index.md, so the +# reserved check must be case-insensitive or that write destroys index.md. +_RESERVED_LOWER = frozenset(r.lower() for r in RESERVED_FILENAMES) + + +def _is_reserved(name: str) -> bool: + return name.lower() in _RESERVED_LOWER class NoteError(Exception): @@ -125,6 +168,12 @@ class NoteFields: connections: list[str] = field(default_factory=list) action_items: list[ActionItem] = field(default_factory=list) references: list[str] = field(default_factory=list) + # Verbatim content outside the template body, preserved across edits and + # merges so hand-curated context is never silently dropped: ``frontmatter`` + # is the leading YAML ``---`` block (Obsidian Properties) / any pre-title + # text; ``lead`` is prose between the ``# Title`` and the first ``##``. + frontmatter: str = "" + lead: str = "" # Non-template ``## Heading`` sections (anything outside TEMPLATE_SECTIONS). # Captured on parse and re-emitted on render so a note carrying its own # H2 headings inside a field body round-trips instead of being silently @@ -163,6 +212,8 @@ def from_dict(cls, data: dict[str, Any]) -> NoteFields: str(h): [str(line) for line in (lines or [])] for h, lines in (data.get("extras") or {}).items() }, + frontmatter=str(data.get("frontmatter", "")), + lead=str(data.get("lead", "")), rev=str(data.get("rev", "")).strip(), disabled=bool(data.get("disabled")), ) @@ -207,36 +258,92 @@ def _strip_blank_edges(lines: list[str]) -> list[str]: return lines[start:end] -def split_sections(md: str) -> tuple[str, dict[str, list[str]]]: - """Split a note into ``(title, {heading: body lines})``. +def _scan_note(md: str) -> tuple[str, str, str, dict[str, list[str]]]: + """Scan a note into ``(frontmatter, title, lead, {heading: body lines})``. - THE ``## heading`` splitter: :func:`parse_note` and the mesh merge - driver's extra-section pass (:mod:`omind.merge`) must agree on what - counts as a section heading, or template-owned content gets classified - as "extra" and duplicated into every merged note. + ``frontmatter`` is the verbatim text before the ``# Title`` line — a leading + YAML ``---`` block (Obsidian Properties) plus any stray pre-title lines. + ``lead`` is the verbatim prose between the ``# Title`` and the first ``##`` + heading. Both were previously discarded, silently destroying hand-curated + content on every edit; they are captured here and re-emitted by + :func:`render_fields`. + + Heading detection is fence-aware: a ``#``/``##`` line inside a ```` ``` ```` + or ``~~~`` code fence is body content, not a heading, so a note that quotes + Markdown in a fenced block round-trips instead of being torn apart. """ + lines = md.splitlines() + n = len(lines) + i = 0 + frontmatter: list[str] = [] + # Leading YAML frontmatter block: consume --- ... --- verbatim so a YAML + # comment line (``# foo``) inside it is never mistaken for the title. + if i < n and lines[i].strip() == "---": + j = i + 1 + while j < n and lines[j].strip() != "---": + j += 1 + if j < n: # closing --- found + frontmatter = lines[i : j + 1] + i = j + 1 + title = "" + seen_title = False + lead: list[str] = [] sections: dict[str, list[str]] = {} current: str | None = None - seen_title = False - for line in md.splitlines(): - if not seen_title and line.startswith("# "): - title = line[2:].strip() - seen_title = True - continue - heading = re.match(r"^##\s+(.*)$", line) - if heading: - current = heading.group(1).strip() - sections.setdefault(current, []) - continue + in_fence = False + fence_ch = "" + while i < n: + line = lines[i] + i += 1 + fence = _FENCE_RE.match(line.lstrip()) + if fence: + ch = fence.group(1)[0] + if not in_fence: + in_fence, fence_ch = True, ch + elif ch == fence_ch: + in_fence = False + # fence lines are body content — fall through to the append below + elif not in_fence: + if not seen_title and (h1 := _H1_RE.match(line)): + title = h1.group(1).strip() + seen_title = True + continue + if h2 := _H2_RE.match(line): + current = h2.group(1).strip() + sections.setdefault(current, []) + continue if current is not None: sections[current].append(line) + elif seen_title: + lead.append(line) + else: + frontmatter.append(line) + + return ( + "\n".join(frontmatter).strip("\n"), + title, + "\n".join(_strip_blank_edges(lead)), + sections, + ) + + +def split_sections(md: str) -> tuple[str, dict[str, list[str]]]: + """Split a note into ``(title, {heading: body lines})``. + + THE ``## heading`` splitter: :func:`parse_note` and the mesh merge + driver's extra-section pass (:mod:`omind.merge`) must agree on what + counts as a section heading, or template-owned content gets classified + as "extra" and duplicated into every merged note. Fence-aware via + :func:`_scan_note`. + """ + _, title, _, sections = _scan_note(md) return title, sections def parse_note(md: str) -> NoteFields: """Parse a note's Markdown into structured fields (best effort).""" - title, sections = split_sections(md) + frontmatter, title, lead, sections = _scan_note(md) def body(name: str) -> str: return "\n".join(sections.get(name, [])).strip() @@ -293,6 +400,8 @@ def body(name: str) -> str: action_items=action_items, references=references, extras=extras, + frontmatter=frontmatter, + lead=lead, rev=rev, disabled=disabled, ) @@ -300,7 +409,17 @@ def body(name: str) -> str: def render_fields(f: NoteFields) -> str: """Render structured fields back into template-shaped Markdown.""" - out: list[str] = [f"# {f.title}".rstrip(), ""] + out: list[str] = [] + # Preserved verbatim content: YAML frontmatter above the title, then the + # title, then any lead prose before the first section. + if f.frontmatter.strip(): + out.append(f.frontmatter.strip("\n")) + out.append("") + out.append(f"# {f.title}".rstrip()) + if f.lead.strip(): + out.append("") + out.append(f.lead.strip("\n")) + out.append("") out.append("## Metadata") out.append(f"- Created: {f.created or today()}".rstrip()) @@ -357,9 +476,17 @@ def _split_field_headings(body: str) -> tuple[str, dict[str, list[str]]]: pre: list[str] = [] sections: dict[str, list[str]] = {} current: str | None = None + in_fence = False + fence_ch = "" for line in body.splitlines(): - m = re.match(r"^##\s+(.*)$", line) - if m: + fence = _FENCE_RE.match(line.lstrip()) + if fence: + ch = fence.group(1)[0] + if not in_fence: + in_fence, fence_ch = True, ch + elif ch == fence_ch: + in_fence = False + elif not in_fence and (m := _H2_RE.match(line)): current = m.group(1).strip() sections.setdefault(current, []) continue @@ -416,29 +543,36 @@ def _metadata_line_edit(md: str, pattern: re.Pattern[str], replacement: str | No """Set/replace/remove one ``## Metadata`` bullet, leaving the rest untouched. Surgical line edit (like :func:`_with_summary`) so hand-curated notes keep - sections the template doesn't know about. Replaces the first line matching - ``pattern`` with ``replacement`` (or removes it when ``replacement`` is - None); when absent, inserts at the end of the ``## Metadata`` section, or - appends a fresh section when the note has none. + sections the template doesn't know about. The match is scoped to the + ``## Metadata`` section — a stray ``- Disabled: true`` bullet in a Details + body (documenting omind itself, say) must not be mistaken for the real + metadata flag, which silently made ``disable_note`` a no-op. Replaces the + first matching Metadata line with ``replacement`` (or removes it when + ``replacement`` is None); when absent, inserts at the end of the Metadata + section, or appends a fresh section when the note has none. """ lines = md.splitlines() - for i, line in enumerate(lines): - if pattern.match(line): - rest = lines[i + 1 :] - middle = [replacement] if replacement is not None else [] - return "\n".join([*lines[:i], *middle, *rest]).rstrip() + "\n" - if replacement is None: - return md meta_start = next( (i for i, line in enumerate(lines) if _METADATA_HEADING_RE.match(line)), None ) if meta_start is None: - return md.rstrip() + f"\n\n## Metadata\n{replacement}\n" - # End of the Metadata section = last non-blank line before the next heading. - end = meta_start + return md if replacement is None else md.rstrip() + f"\n\n## Metadata\n{replacement}\n" + # Metadata section spans (meta_start, meta_end) up to the next ``## `` heading. + meta_end = len(lines) for i in range(meta_start + 1, len(lines)): - if lines[i].startswith("#"): + if lines[i].startswith("## "): + meta_end = i break + for i in range(meta_start + 1, meta_end): + if pattern.match(lines[i]): + rest = lines[i + 1 :] + middle = [replacement] if replacement is not None else [] + return "\n".join([*lines[:i], *middle, *rest]).rstrip() + "\n" + if replacement is None: + return md + # Insert after the last non-blank line of the Metadata section. + end = meta_start + for i in range(meta_start + 1, meta_end): if lines[i].strip(): end = i return "\n".join([*lines[: end + 1], replacement, *lines[end + 1 :]]).rstrip() + "\n" @@ -544,7 +678,7 @@ def _reject_reserved(self, path: Path) -> None: """Generated files are not notes: a note titled 'index' would overwrite index.md, and the next regeneration would adopt the note body as the hand-written index intro — permanently.""" - if path.name in RESERVED_FILENAMES: + if _is_reserved(path.name): raise NoteError(f"{path.name!r} is a reserved file, not a note; pick another title") def safe_name(self, name: str) -> Path: @@ -564,8 +698,15 @@ def safe_name(self, name: str) -> Path: base = Path(name).name if base != name: raise NoteError(f"unsafe note name: {name!r}") + # A leading dot makes the file a dotfile: _note_paths / list / search / + # index all skip it, so the note would save "successfully" yet be + # unrecallable. Reject it rather than silently black-hole the memory. + if base.startswith("."): + raise NoteError(f"note name may not start with a dot: {name!r}") if not base.endswith(".md"): base += ".md" + if len(base.encode("utf-8")) > _MAX_FILENAME_BYTES: + raise NoteError(f"note name is too long ({len(base.encode('utf-8'))} bytes): {name!r}") target = (self.omi_dir / base).resolve() if target.parent != self.omi_dir.resolve(): raise NoteError(f"note name escapes the OMI directory: {name!r}") @@ -574,6 +715,16 @@ def safe_name(self, name: str) -> Path: def filename_for_title(self, title: str) -> str: cleaned = _ILLEGAL_FILENAME_CHARS.sub(" ", title).strip() cleaned = re.sub(r"\s+", " ", cleaned) + # Strip leading dots so a title like ".NET notes" doesn't become an + # invisible dotfile (see safe_name). + cleaned = cleaned.lstrip(". ").strip() + if not cleaned: + raise NoteError("title produces an empty filename") + # Truncate on a char boundary so the encoded filename stays under the + # OS byte limit (leaving room for the ".md" suffix). + budget = _MAX_FILENAME_BYTES - len(".md") + while len(cleaned.encode("utf-8")) > budget: + cleaned = cleaned[:-1].rstrip() if not cleaned: raise NoteError("title produces an empty filename") return f"{cleaned}.md" @@ -585,13 +736,13 @@ def _note_paths(self) -> Iterator[Path]: if not self.omi_dir.is_dir(): return for path in self.omi_dir.glob("*.md"): - if path.name in RESERVED_FILENAMES or path.name.startswith("."): + if _is_reserved(path.name) or path.name.startswith("."): continue yield path def _summarize(self, path: Path, text: str | None = None) -> NoteSummary: if text is None: - text = path.read_text(encoding="utf-8") + text = _read_text(path) return self._summarize_fields(path, parse_note(text)) def _cached_summary(self, path: Path) -> NoteSummary | None: @@ -651,7 +802,10 @@ def search( tag_needle = _clean_tag(tag).lower() if tag else "" results: list[NoteSummary] = [] for path in self._note_paths(): - text = path.read_text(encoding="utf-8") + try: + text = _read_text(path) + except OSError: + continue # note deleted mid-scan (concurrent write/purge) — skip it fields = parse_note(text) if fields.disabled and not include_disabled: continue @@ -703,7 +857,7 @@ def backlinks(self, name: str) -> list[NoteSummary]: target = self.safe_name(name) if not target.is_file(): raise NoteNotFoundError(f"note not found: {name!r}") - target_text = target.read_text(encoding="utf-8") + target_text = _read_text(target) stem = target.name[:-3] if target.name.endswith(".md") else target.name identifiers = {stem.strip().lower()} title = parse_note(target_text).title.strip().lower() @@ -714,7 +868,10 @@ def backlinks(self, name: str) -> list[NoteSummary]: for path in self._note_paths(): if path.resolve() == target.resolve(): continue - text = path.read_text(encoding="utf-8") + try: + text = _read_text(path) + except OSError: + continue # note deleted mid-scan — skip it rather than 500 # Obsidian link forms: [[Note]], [[Note|alias]], [[Note#heading]] — # only the part before | or # names the target note. link_targets = { @@ -732,7 +889,7 @@ def read_note(self, name: str) -> str: path = self.safe_name(name) if not path.is_file(): raise NoteNotFoundError(f"note not found: {name!r}") - return path.read_text(encoding="utf-8") + return _read_text(path) def read_fields(self, name: str) -> NoteFields: return parse_note(self.read_note(name)) @@ -763,10 +920,23 @@ def all_tags(self) -> list[str]: # -- writes ------------------------------------------------------------- - def write_note(self, name: str, content: str, expected_version: str | None = None) -> str: + def write_note( + self, + name: str, + content: str, + expected_version: str | None = None, + *, + must_create: bool = False, + ) -> str: path = self.safe_name(name) self._reject_reserved(path) with self.write_lock(): + # ``must_create`` closes the create-note TOCTOU: two sessions saving + # the same brand-new title both passed a pre-lock exists() check and + # the second silently overwrote the first. The check now happens + # inside the lock. + if must_create and path.exists(): + raise NoteError(f"a note named {path.name!r} already exists") # Re-check the optimistic-concurrency token *inside* the lock so the # check-then-write is atomic against another process's save. A # missing file is a mismatch too (note_version returns ""): a stale @@ -778,7 +948,7 @@ def write_note(self, name: str, content: str, expected_version: str | None = Non f"note {name!r} changed on disk (expected {expected_version!r}, " f"found {current!r})" ) - if self.node_id is not None and path.name not in RESERVED_FILENAMES: + if self.node_id is not None and not _is_reserved(path.name): content = self._stamped(path, content) _atomic_write(path, content) self._write_index() @@ -811,8 +981,8 @@ def _mutate_note( f"note {name!r} changed on disk (expected {expected_version!r}, " f"found {current!r})" ) - content = transform(path.read_text(encoding="utf-8")) - if self.node_id is not None and path.name not in RESERVED_FILENAMES: + content = transform(_read_text(path)) + if self.node_id is not None and not _is_reserved(path.name): content = self._stamped(path, content) _atomic_write(path, content) self._write_index() @@ -830,7 +1000,7 @@ def _stamped(self, path: Path, content: str) -> str: return content current: Rev | None = None if path.is_file(): - current = Rev.parse(parse_note(path.read_text(encoding="utf-8")).rev) + current = Rev.parse(parse_note(_read_text(path)).rev) incoming = Rev.parse(parse_note(content).rev) if incoming is not None and (current is None or incoming.newer_than(current)): current = incoming @@ -842,11 +1012,10 @@ def create_note(self, fields: NoteFields) -> str: if not fields.created: fields.created = today() filename = self.filename_for_title(fields.title) - path = self.safe_name(filename) - if path.exists(): - raise NoteError(f"a note named {filename!r} already exists") _hoist_field_headings(fields) # canonicalize ## H2-in-body -> extras - return self.write_note(filename, render_fields(fields)) + # Existence is re-checked under the write lock (must_create) to close the + # concurrent-create race, not here. + return self.write_note(filename, render_fields(fields), must_create=True) def update_note( self, name: str, fields: NoteFields, expected_version: str | None = None @@ -866,9 +1035,14 @@ def transform(text: str) -> str: fields.created = current.created # Callers that don't carry extras (a partial edit-note, an MCP/CLI # upsert built from flat fields) must not drop the note's existing - # non-template sections. Inherit them like rev/created above. + # non-template sections, frontmatter, or lead prose. Inherit them + # like rev/created above. if not fields.extras: fields.extras = current.extras + if not fields.frontmatter: + fields.frontmatter = current.frontmatter + if not fields.lead: + fields.lead = current.lead # A multi-section body supplied through `details` (the only such # field the MCP/CLI API exposes) carries ## H2s that read back as # extras. Hoist them now so they REPLACE the same-named inherited @@ -893,7 +1067,7 @@ def delete_note(self, name: str) -> None: def disable_note(self, name: str) -> str: """Soft-delete: set ``Disabled: true``; hidden from listings, restorable.""" path = self.safe_name(name) - if path.name in RESERVED_FILENAMES: + if _is_reserved(path.name): raise NoteError(f"refusing to disable reserved file: {path.name}") return self._mutate_note(name, lambda md: _with_disabled(md, True)) @@ -904,7 +1078,7 @@ def restore_note(self, name: str) -> str: def purge_note(self, name: str) -> None: """Hard-delete a note file. In a mesh, only `omind mesh purge` may use this.""" path = self.safe_name(name) - if path.name in RESERVED_FILENAMES: + if _is_reserved(path.name): raise NoteError(f"refusing to delete reserved file: {path.name}") if not path.is_file(): raise NoteNotFoundError(f"note not found: {name!r}") @@ -944,7 +1118,7 @@ def _write_index(self) -> None: :meth:`_migrate_index_descriptions` before the list is regenerated. """ index_path = self.omi_dir / INDEX_FILENAME - existing = index_path.read_text(encoding="utf-8") if index_path.is_file() else "" + existing = _read_text(index_path) if index_path.is_file() else "" self._migrate_index_descriptions(existing) if INDEX_RECENT_HEADING in existing: intro = existing.split(INDEX_RECENT_HEADING, 1)[0].rstrip() @@ -985,9 +1159,16 @@ def _migrate_index_descriptions(self, existing: str) -> None: path = self.safe_name(entry.group(1).strip()) except NoteError: continue - if path.name in RESERVED_FILENAMES or not path.is_file(): + if _is_reserved(path.name) or not path.is_file(): continue - text = path.read_text(encoding="utf-8") + text = _read_text(path) if parse_note(text).summary.strip(): continue - _atomic_write(path, _with_summary(text, description)) + migrated = _with_summary(text, description) + # On a mesh node, stamp the next rev so this content change is + # ordered like any other write. Writing new content under the old + # rev produced equal-rev/different-content across peers — the + # precondition for a non-convergent merge (see omind.merge). + if self.node_id is not None: + migrated = self._stamped(path, migrated) + _atomic_write(path, migrated) diff --git a/src/omind/update.py b/src/omind/update.py index c1986f3..548ef7d 100644 --- a/src/omind/update.py +++ b/src/omind/update.py @@ -147,7 +147,10 @@ def check_for_update(*, force: bool = False, timeout: float = _HTTP_TIMEOUT) -> treated as "unknown / up to date". ``force=True`` bypasses the cache. """ current = __version__ - if os.environ.get(_DISABLE_ENV): + # The env var disables the PASSIVE nudge (privacy); an explicit + # `omind self-update` (force=True) must still be able to check, or the + # documented opt-out silently breaks self-update with a misleading "offline". + if os.environ.get(_DISABLE_ENV) and not force: return UpdateStatus(current, None) if not force: fresh, latest = _read_cache(_cache_path()) @@ -211,7 +214,10 @@ def self_update( *, check_only: bool = False, force: bool = False, log: Callable[[str], object] = print ) -> int: """``omind self-update``: report, then (unless ``--check``) reinstall the latest tag.""" - status = check_for_update(force=True) # an explicit update always re-checks + # A user-invoked update gets a generous network timeout, not the 2s nudge + # budget (which times out on a slow-but-working link and falsely reports + # "could not reach GitHub"). + status = check_for_update(force=True, timeout=15.0) log(f"installed: omind {status.current}") if status.latest is None: log("could not reach GitHub (offline, rate-limited, or no releases yet).") @@ -236,7 +242,13 @@ def self_update( return 1 log(f"updating: {' '.join(cmd)}") try: - result = subprocess.run(cmd, check=False) # streams to the user's terminal + # A watchdog timeout so a hung `uv tool install git+…` (a stalled clone, + # a dead network) can't wedge the update pass forever when run from + # fleet automation. + result = subprocess.run(cmd, check=False, timeout=600) # streams to terminal + except subprocess.TimeoutExpired: + log("update timed out after 600s (network stall?) — try again.") + return 1 except (OSError, subprocess.SubprocessError) as exc: log(f"update failed to launch: {exc}") return 1 diff --git a/src/omind/verify.py b/src/omind/verify.py index 5d81e2b..2bdfe55 100644 --- a/src/omind/verify.py +++ b/src/omind/verify.py @@ -199,11 +199,19 @@ def consult_target(event: dict[str, Any], omi_dir: Path | str) -> tuple[str, str ti = ti if isinstance(ti, dict) else {} if tool.startswith("mcp__omi__"): if "search" in tool: - return ("search", str(ti.get("query") or "")) + query = str(ti.get("query") or "").strip() + # An empty search query carries no content to judge relevance against, + # so it is not a real consult — treating it as one let a contentless + # call clear the gate and always score "relevant". + return ("search", query) if query else None target = str( ti.get("filename") or ti.get("name") or ti.get("note") or ti.get("query") or "" - ) - return ("read", target) + ).strip() + # A contentless navigation/listing call (list-notes, list-tags, graph-*) + # has no note target — like re-reading index.md, it is the gate-dodge, not + # a consult of relevant memory. Don't let it clear the gate or auto-score + # relevant on empty text (verify.py:192). + return ("read", target) if target else None if tool == "Read": fp = str(ti.get("file_path") or "") # The vault's table-of-contents (index.md), the recent-memories MEMORY.md diff --git a/tests/test_adapters.py b/tests/test_adapters.py index ebcff29..8df7842 100644 --- a/tests/test_adapters.py +++ b/tests/test_adapters.py @@ -42,11 +42,29 @@ def test_normalize_other_harness_shapes() -> None: assert adapters.normalize_action({"args": "rm -rf /"})["command"] == "rm -rf /" +def test_normalize_accepts_array_shaped_args() -> None: + """A list argv must reach the guard as a command, not be dropped to ''.""" + action = adapters.normalize_action( + {"tool": "shell", "args": ["gh", "repo", "delete", "a/b"], "session": "z"} + ) + assert action["command"] == "gh repo delete a/b" + inner = adapters.normalize_action({"tool": "shell", "input": {"command": "rm -rf /"}}) + assert inner["command"] == "rm -rf /" + + def test_run_adapter_hard_block_denies_any_harness() -> None: event = io.StringIO(json.dumps({"tool": "shell", "command": "gh pr create", "session": "a1"})) assert adapters.run_adapter(event) == 2 # hard rule fires without a consult too +def test_run_adapter_fails_closed_on_unparseable_event() -> None: + """A mangled event in an enforcement component must block, not wave through.""" + assert adapters.run_adapter(io.StringIO("{not valid json")) == 2 + assert adapters.run_adapter(io.StringIO("[1, 2, 3]")) == 2 # not an object + # A genuinely empty stream is not an error — nothing to guard. + assert adapters.run_adapter(io.StringIO(" ")) == 0 + + def test_run_adapter_consult_clears_then_gate_allows() -> None: guard.clear_gate("a2") blocked = io.StringIO(json.dumps({"tool": "shell", "command": "ls", "session": "a2"})) diff --git a/tests/test_compliance.py b/tests/test_compliance.py index 2be55ae..04ec96f 100644 --- a/tests/test_compliance.py +++ b/tests/test_compliance.py @@ -120,3 +120,13 @@ def test_post_tool_hook_runs_the_detector(tmp_path: object) -> None: ) hooks.run_hook("PostToolUse", tmp_path, stdin=io.StringIO(event)) # type: ignore[arg-type] assert compliance.read_events()[-1]["rule_id"] == "gh-repo-delete" + + +def test_read_events_survives_a_torn_non_utf8_line() -> None: + """A single bad byte in the log must not crash every consumer forever.""" + path = compliance.compliance_log_path() + path.parent.mkdir(parents=True, exist_ok=True) + good = json.dumps({"rule_id": "ok", "outcome": "observed"}) + path.write_bytes(good.encode() + b"\n\xff\xfe torn\n" + good.encode() + b"\n") + events = compliance.read_events() # must not raise + assert [e.get("rule_id") for e in events] == ["ok", "ok"] diff --git a/tests/test_guard.py b/tests/test_guard.py index 6265057..688cc13 100644 --- a/tests/test_guard.py +++ b/tests/test_guard.py @@ -764,6 +764,88 @@ def test_widened_destructive_rules_close_red_team_gaps() -> None: guard.clear_gate("b1") +def test_freshness_accepts_dash_c_and_compound_read_forms() -> None: + """#449: `git -C fetch` and `git fetch && git status` establish freshness.""" + guard.record_consult("fresh2", kind="read", target=guard.GIT_RULES_NOTE, relevant=True) + repo = guard._repo_root_for_action({"tool": "Bash", "command": "git status"}) + assert repo is not None + for cmd in ( + f"git -C {repo} fetch --all --prune", + "git fetch --all --prune && git status -sb", + ): + guard.clear_gate("fresh2") + guard.record_consult("fresh2", kind="read", target=guard.GIT_RULES_NOTE, relevant=True) + v = guard.decide({"tool": "Bash", "command": cmd, "session": "fresh2"}) + assert v.allow, cmd + # A fetch chained with a non-read command is NOT a pure freshness command, + # so it must not establish freshness (a piggybacked write can't ride in). + assert guard._is_freshness_command("git fetch --all --prune") + assert guard._is_freshness_command("git -C /r fetch && git status") + assert not guard._is_freshness_command("git fetch --all && rm -rf build") + assert not guard._is_freshness_command("git fetch | tee /etc/x") + guard.clear_gate("fresh2") + + +def test_stderr_redirect_is_not_a_side_effect_under_a_capability_question() -> None: + """#498: `pytest 2>&1 | tail` must not be read as a file-writing side effect.""" + guard.mark_consulted("redir") + v = guard.decide( + { + "tool": "Bash", + "command": "pytest -q 2>&1 | tail", + "prompt": "Could you check why the tests fail?", + "session": "redir", + } + ) + # Not blocked as an unauthorized capability side-effect (it may still need the + # repo note/freshness, but never `capability-question-explicit-auth`). + assert v.rule_id != "capability-question-explicit-auth" + guard.clear_gate("redir") + + +def test_project_local_dotclaude_is_not_a_global_config_mutation(tmp_path: Path) -> None: + """#453: editing /.claude/settings.json is project config, not global.""" + project = tmp_path / "myrepo" / ".claude" + project.mkdir(parents=True) + assert not guard._is_global_config_path(str(project / "settings.json")) + # The real home-anchored global still is. + assert guard._is_global_config_path(str(Path.home() / ".claude" / "settings.json")) + + +def test_bad_learned_rule_does_not_brick_the_guard() -> None: + """#668: a malformed regex reaching decide() must be skipped, not crash it.""" + from omind import policy + + # A rule object whose compiled() raises (bypassing the loader's validation). + class _BadRule(policy.Rule): + def compiled(self): # type: ignore[override] + raise __import__("re").error("boom") + + bad = _BadRule(id="bad", pattern="x", message="m", severity=policy.SEVERITY_HARD) + import unittest.mock as mock + + guard.mark_consulted("brick") + with mock.patch.object(policy, "load_policy", return_value=[bad]): + # Must not raise; the bad rule is skipped and the action is decided. + v = guard.decide({"tool": "Bash", "command": "echo hi", "session": "brick"}) + assert v.allow + guard.clear_gate("brick") + + +def test_opt_in_env_prefix_must_be_at_command_position() -> None: + """#517: `env TOKEN` forged inside a string must not satisfy the opt-in.""" + assert guard._opt_in_satisfied("OMI_SUDO_OK=1", "OMI_SUDO_OK=1 sudo x") + assert guard._opt_in_satisfied("OMI_SUDO_OK=1", "env OMI_SUDO_OK=1 sudo x") + assert not guard._opt_in_satisfied("OMI_SUDO_OK=1", 'echo "use env OMI_SUDO_OK=1" && sudo x') + + +def test_negated_verb_is_not_global_authorization() -> None: + """#463: 'don't change anything' must not authorize a global-config mutation.""" + assert guard._has_global_auth("please update the global config") + assert guard._has_global_auth("fix the hook please") # expanded verb set + assert not guard._has_global_auth("don't change anything yet") + + def test_guard_status_flags_agent_writable_config(capsys: pytest.CaptureFixture[str]) -> None: """#B2: status surfaces the kill-shot surface when the guard's own config is writable by the agent (here, under the test's isolated HOME).""" diff --git a/tests/test_lint.py b/tests/test_lint.py index e53a6ca..87da301 100644 --- a/tests/test_lint.py +++ b/tests/test_lint.py @@ -133,3 +133,50 @@ def test_clean_format_report_on_empty_vault(tmp_path: Path) -> None: omi = _omi(tmp_path) assert lint.lint_vault(omi) == [] assert "no issues" in lint.format_report([], omi_dir=omi) + + +def test_dated_series_is_not_a_near_duplicate(tmp_path: Path) -> None: + """A daily Worklog series must not be flagged as duplicate memories.""" + omi = _omi(tmp_path) + for day in ("2026-06-28", "2026-06-29", "2026-06-30"): + _write(omi, f"Worklog {day}.md", f"# Worklog {day}\n\n- [[Worklog 2026-06-28]]\n") + dupes = [i for i in lint.lint_vault(omi) if i.kind == "near-duplicate"] + assert dupes == [] + + +def test_link_to_archived_note_is_not_broken(tmp_path: Path) -> None: + """A link to a soft-deleted (archived) note is valid, not a broken-link error.""" + omi = _omi(tmp_path) + _write(omi, "Live.md", "# Live\n\n- [[Archived Note]]\n") + _write( + omi, + "Archived Note.md", + "# Archived Note\n\n## Metadata\n- Disabled: true\n", + ) + broken = [i for i in lint.lint_vault(omi) if i.kind == "broken-link"] + assert broken == [] + + +def test_link_into_journal_subfolder_is_not_broken(tmp_path: Path) -> None: + """A wikilink to a Journal/ rollup note must resolve, not error.""" + omi = _omi(tmp_path) + _write(omi, "Ref.md", "# Ref\n\n- [[Session Journal Rollup 2026-W26]]\n") + (omi / "Journal").mkdir() + (omi / "Journal" / "Session Journal Rollup 2026-W26.md").write_text( + "# Session Journal Rollup 2026-W26\n", encoding="utf-8" + ) + broken = [i for i in lint.lint_vault(omi) if i.kind == "broken-link"] + assert broken == [] + + +def test_wikilink_inside_code_fence_is_not_a_link(tmp_path: Path) -> None: + """A [[wikilink]] quoted in a fenced code block is documentation, not a link.""" + omi = _omi(tmp_path) + _write( + omi, + "Docs.md", + "# Docs\n\n## Details\nExample:\n\n```\nUse [[Some Note]] to link.\n```\n\n" + "## Connections\n- [[Docs]]\n", + ) + broken = [i for i in lint.lint_vault(omi) if i.kind == "broken-link"] + assert broken == [] diff --git a/tests/test_merge.py b/tests/test_merge.py index 2aa3097..eae542b 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -103,6 +103,27 @@ def test_result_rev_is_max_of_sides() -> None: assert merge_fields(base, note("2@b"), note("4@a")).fields.rev == "4@a" +def test_equal_rev_different_content_is_symmetric() -> None: + """Equal rev identity with differing content must converge (not resolve by side).""" + base = note("1@a", summary="base") + one = note("5@a", summary="apple") + two = note("5@a", summary="zebra") # same rev identity, different content + a = merge_fields(base, one, two) + b = merge_fields(base, two, one) + assert a.fields.summary == b.fields.summary # converges regardless of side + assert a.fields.summary == "zebra" # symmetric max() tiebreak + + +def test_merge_preserves_frontmatter_and_lead() -> None: + """A YAML frontmatter block and lead prose must survive a merge, not vanish.""" + base = "---\ntags: [x]\n---\n# N\n\nlead text.\n\n## Summary\ns\n\n## Details\nd\n" + ours = "---\ntags: [x]\n---\n# N\n\nlead text.\n\n## Summary\ns2\n\n## Details\nd\n" + theirs = base + merged, _clean, _msgs = merge_note_texts(base, ours, theirs) + assert "tags: [x]" in merged + assert "lead text." in merged + + # -- list union ------------------------------------------------------------------ diff --git a/tests/test_policy.py b/tests/test_policy.py index 7a4c5fd..25e98b5 100644 --- a/tests/test_policy.py +++ b/tests/test_policy.py @@ -124,3 +124,61 @@ def test_soft_rule_does_not_block_at_the_gate() -> None: ) assert guard.decide({"command": "git commit -m x", "session": "soft"}).allow guard.clear_gate("soft") + + +def test_forge_rules_are_command_anchored() -> None: + """The forge/destructive seed rules must not fire on the phrase as an + argument, a grep pattern, or a commit message (#101 false-positive class).""" + by_id = {r.id: r for r in policy.SEED_RULES} + d = "del" + "ete" + setup = "setup" + "-git" + + def hit(rule_id: str, cmd: str) -> bool: + return bool(by_id[rule_id].compiled().search(cmd)) + + # False positives that must NOT match. + assert not hit("gh-repo-delete", f'grep -rn "gh repo {d}" src/') + assert not hit("gh-repo-delete", f'git commit -m "forbid gh repo {d}"') + assert not hit("gh-auth-setup-git", f'grep "gh auth {setup}" notes') + # Real invocations that MUST still match. + assert hit("gh-repo-delete", f"gh repo {d} foo/bar") + assert hit("gh-repo-delete", f"echo x && gh repo {d} foo") + assert hit("gh-auth-setup-git", f"gh auth {setup}") + + +def test_sudo_wrapper_and_path_bypasses_are_caught() -> None: + """Shell keywords / absolute paths must not let a hard sudo rule fail open.""" + by_id = {r.id: r for r in policy.SEED_RULES} + + def hit(rule_id: str, cmd: str) -> bool: + return bool(by_id[rule_id].compiled().search(cmd)) + + assert hit("sudo-use-fleet-sudo", "if true; then sudo rm -rf /; fi") + assert hit("sudo-use-fleet-sudo", "nohup sudo x") + assert hit("sudo-use-fleet-sudo", "xargs sudo") + assert hit("sudo-use-fleet-sudo", "/usr/bin/sudo x") + assert hit("sudo-use-fleet-sudo", "sudoedit /etc/shadow") + assert hit("privesc-alternatives", "if true; then doas x; fi") + # Still no false positives on args / paths / the sanctioned wrapper. + assert not hit("sudo-use-fleet-sudo", "grep sudo /var/log/x") + assert not hit("sudo-use-fleet-sudo", "cat /usr/bin/sudo") + assert not hit("sudo-use-fleet-sudo", "pass show sudo/akclark") + assert not hit("sudo-use-fleet-sudo", "fleet-sudo systemctl restart x") + + +def test_loader_drops_uncompilable_and_universal_patterns() -> None: + """A bad learned regex must never reach the guard hot path (bricked machine).""" + policy.policy_path().parent.mkdir(parents=True, exist_ok=True) + policy.policy_path().write_text( + json.dumps( + [ + {"id": "good", "pattern": r"\bnpm\s+publish\b", "message": "m"}, + {"id": "uncompilable", "pattern": r"(unclosed", "message": "m"}, + {"id": "empty-match", "pattern": r"x|", "message": "m"}, + {"id": "bad-sev", "pattern": r"y", "message": "m", "severity": 5}, + ] + ), + encoding="utf-8", + ) + learned = policy.load_learned() + assert [r.id for r in learned] == ["good"] diff --git a/tests/test_secret_output_guard.py b/tests/test_secret_output_guard.py index 4491bff..df69aa8 100644 --- a/tests/test_secret_output_guard.py +++ b/tests/test_secret_output_guard.py @@ -96,3 +96,26 @@ def test_allows_token_in_curl_header(tmp_path: Path) -> None: def test_audited_override_allows(tmp_path: Path) -> None: assert _run(_hook(tmp_path), "OMI_SECRET_OK=1 pass show github/token | head") == 0 + + +def test_blocks_pass_show_with_stderr_redirect_piped(tmp_path: Path) -> None: + """CRITICAL: `2>/dev/null` redirects stderr; stdout still leaks to the transcript.""" + assert _run(_hook(tmp_path), "pass show github/token 2>/dev/null | head") == 2 + assert _run(_hook(tmp_path), "pass show github/token 2>/dev/null") == 2 + + +def test_word_boundary_avoids_bypass_false_positive(tmp_path: Path) -> None: + """`pass` inside another word / a grep pattern must not false-block.""" + assert _run(_hook(tmp_path), "curl --noproxy '' https://bypass.example/x") == 0 + assert _run(_hook(tmp_path), 'grep -r "pass tests/unit" .') == 0 + assert _run(_hook(tmp_path), 'git commit -m "make the pass show up"') == 0 + + +def test_forged_override_in_string_does_not_bypass(tmp_path: Path) -> None: + """OMI_SECRET_OK=1 inside a quoted string must not disable the guard.""" + assert _run(_hook(tmp_path), 'echo "set OMI_SECRET_OK=1 first" && pass show x | head') == 2 + + +def test_allows_multiline_captured_read(tmp_path: Path) -> None: + """A multi-line `TOK=$(\\n pass show x \\n)` capture is safe, not a leak.""" + assert _run(_hook(tmp_path), "TOK=$(\n pass show github/token\n)\necho done") == 0 diff --git a/tests/test_store.py b/tests/test_store.py index 0e2f360..a592fa7 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -716,3 +716,71 @@ def test_listing_cache_tracks_writes_and_deletes(store: OmiStore) -> None: ] store.purge_note(a) assert {s.title for s in store.list_notes()} == {"Cached B"} + + +def test_yaml_frontmatter_survives_edit(store: OmiStore) -> None: + """Obsidian Properties (a leading --- YAML block) must not be dropped on edit.""" + raw = ( + "---\ntags: [alpha, beta]\naliases:\n - Foo\n---\n" + "# Kept Note\n\nlead prose before the first section.\n\n" + "## Summary\ns\n\n## Details\nd\n" + ) + name = store.write_note("Kept Note.md", raw) + store.update_note(name, NoteFields(title="Kept Note", summary="new summary")) + after = store.read_note(name) + assert "tags: [alpha, beta]" in after + assert "aliases:" in after + assert "lead prose before the first section." in after + assert "new summary" in after + + +def test_fenced_h2_is_not_treated_as_a_section(store: OmiStore) -> None: + """A ## line inside a fenced code block is body text, not a section boundary.""" + details = "Example:\n\n```md\n## Not A Heading\nbody\n```\n\nreal details." + name = store.create_note(NoteFields(title="Fenced", details=details)) + fields = store.read_fields(name) + assert "Not A Heading" not in fields.extras + assert "## Not A Heading" in fields.details + + +def test_reserved_check_is_case_insensitive(store: OmiStore) -> None: + """On a case-insensitive filesystem 'Index' == index.md; reject it everywhere.""" + for title in ("Index", "INDEX", "Memory template"): + with pytest.raises(NoteError): + store.create_note(NoteFields(title=title)) + + +def test_dot_prefixed_title_is_rejected(store: OmiStore) -> None: + """A dotfile note would be invisible to listing/search/index — refuse it.""" + name = store.create_note(NoteFields(title=".NET migration notes", summary="s")) + assert not name.startswith(".") + assert name in {s.filename for s in store.list_notes()} + with pytest.raises(NoteError): + store.write_note(".hidden.md", "# x\n") + + +def test_overlong_title_raises_noteerror_not_oserror(store: OmiStore) -> None: + """A 300-char title must produce a clean NoteError, not ENAMETOOLONG.""" + name = store.create_note(NoteFields(title="x" * 300, summary="s")) + assert len(name.encode("utf-8")) <= 200 + assert (store.omi_dir / name).is_file() + + +def test_non_utf8_note_does_not_break_listing(store: OmiStore) -> None: + """One note with a stray non-UTF-8 byte must not take down list/search.""" + store.create_note(NoteFields(title="Good", summary="fine")) + bad = store.omi_dir / "Bad.md" + bad.write_bytes(b"# Bad\n\n## Summary\n\xff\xfe not utf8\n") + titles = {s.title for s in store.list_notes()} + assert "Good" in titles and "Bad" in titles + assert store.search("fine") # search still works + + +def test_disable_ignores_disabled_bullet_in_details(mesh_store: OmiStore) -> None: + """A '- Disabled: true' line in Details must not fool disable/parse (Metadata-scoped).""" + name = mesh_store.create_note( + NoteFields(title="Doc", details="Config keys:\n- Disabled: true") + ) + assert not mesh_store.read_fields(name).disabled + mesh_store.disable_note(name) + assert mesh_store.read_fields(name).disabled diff --git a/tests/test_update.py b/tests/test_update.py index 73444e4..822af18 100644 --- a/tests/test_update.py +++ b/tests/test_update.py @@ -160,7 +160,7 @@ def test_self_update_runs_installer(monkeypatch: pytest.MonkeyPatch) -> None: class _Result: returncode = 0 - def fake_run(cmd: list[str], check: bool) -> _Result: + def fake_run(cmd: list[str], check: bool, timeout: float | None = None) -> _Result: ran["cmd"] = cmd return _Result() diff --git a/uv.lock b/uv.lock index 0b38370..b3288a8 100644 --- a/uv.lock +++ b/uv.lock @@ -2364,7 +2364,7 @@ wheels = [ [[package]] name = "omind" -version = "3.7.5" +version = "3.7.6" source = { editable = "." } dependencies = [ { name = "fastapi" },