From 3d7c3e1b8ae566cf825b2df12f97c46c67a02cfc Mon Sep 17 00:00:00 2001 From: Imran Siddique Date: Fri, 31 Jul 2026 16:36:29 -0700 Subject: [PATCH] refactor(engines): migrate codex and scheduled-agents onto the capture core Completes the migration begun for claude-code. All three engines now delegate hashing, component tree digests, the exclusion denylist, comparison shapes, observed and scope gating, and state read/write to the shared core, with the vendored fallback so a bare install still works. Test files are untouched in all three engines. That is the point of the exercise: behaviour is proven identical by not moving the contract. claude-code 59, codex 25, scheduled-agents 21, core 45. Four behaviours deliberately stayed local, each because unifying would have been silently wrong rather than merely different: codex _safe_hash rejects symlinks and the core's does not. Codex resolves instructions and policy from per-workspace paths, so a cloned repo could point a symlink at a file outside the tree and have its contents recorded as if they belonged to the workspace. The stricter version stays. codex _atomic_write writes 0o600, because the same helper writes the Ed25519 signing key. Rather than lose that, the core gained an optional mode parameter applied before the replace, so the file is never briefly readable at wider permissions. codex _save keeps sorted keys and a trailing newline. Switching to the core's format would rewrite every existing state file for no behavioural gain; the digest is over the mapping, not the bytes. scheduled-agents _sha_obj takes arbitrary objects, not just mappings: it is called on lists of allowed tools and MCP names. The core also gained the mode parameter noted above; the vendored copies are re-synced and the sync check passes. Not included: baseline sealing for codex and scheduled-agents. Both lack it and both would benefit, but that is a feature addition rather than a migration, and it touches their report layouts and hook messages. Kept separate so this PR is reviewable as a pure refactor. ruff clean at py39 across all three engines, the package and the scripts. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Imran Siddique --- .../_vendor/agentrust_capture_core/state.py | 12 +- .../src/agentrust_capture_core/state.py | 12 +- .../_vendor/agentrust_capture_core/state.py | 12 +- plugins/agentrust-codex/engine/capture.py | 201 +++++------------- .../_vendor/agentrust_capture_core/state.py | 12 +- scheduled-agents/engine/capture.py | 55 ++--- 6 files changed, 120 insertions(+), 184 deletions(-) diff --git a/claude-code/engine/_vendor/agentrust_capture_core/state.py b/claude-code/engine/_vendor/agentrust_capture_core/state.py index c5e87c9..5a0839d 100644 --- a/claude-code/engine/_vendor/agentrust_capture_core/state.py +++ b/claude-code/engine/_vendor/agentrust_capture_core/state.py @@ -28,12 +28,17 @@ class StatePaths: latest: Path -def atomic_write(path: Path, content: str) -> None: +def atomic_write(path: Path, content: str, *, mode: int | None = None) -> None: """Write via a temporary file and replace, so a crash cannot truncate state. A half-written baseline is worse than a missing one: the engine would treat it as corrupt on every future session, and a user who sees a broken check often enough stops reading it. + + ``mode`` is applied to the temporary file before the replace, so the file is + never briefly readable at wider permissions than intended. Callers that write + a private key pass ``0o600``. Best-effort, since not every filesystem carries + POSIX permissions. """ path.parent.mkdir(parents=True, exist_ok=True) handle, tmp_name = tempfile.mkstemp(dir=str(path.parent), prefix=path.name, suffix=".tmp") @@ -43,6 +48,11 @@ def atomic_write(path: Path, content: str) -> None: fh.write(content) fh.flush() os.fsync(fh.fileno()) + if mode is not None: + try: + os.chmod(tmp, mode) + except OSError: + pass os.replace(tmp, path) except BaseException: tmp.unlink(missing_ok=True) diff --git a/packages/agentrust-capture-core/src/agentrust_capture_core/state.py b/packages/agentrust-capture-core/src/agentrust_capture_core/state.py index c5e87c9..5a0839d 100644 --- a/packages/agentrust-capture-core/src/agentrust_capture_core/state.py +++ b/packages/agentrust-capture-core/src/agentrust_capture_core/state.py @@ -28,12 +28,17 @@ class StatePaths: latest: Path -def atomic_write(path: Path, content: str) -> None: +def atomic_write(path: Path, content: str, *, mode: int | None = None) -> None: """Write via a temporary file and replace, so a crash cannot truncate state. A half-written baseline is worse than a missing one: the engine would treat it as corrupt on every future session, and a user who sees a broken check often enough stops reading it. + + ``mode`` is applied to the temporary file before the replace, so the file is + never briefly readable at wider permissions than intended. Callers that write + a private key pass ``0o600``. Best-effort, since not every filesystem carries + POSIX permissions. """ path.parent.mkdir(parents=True, exist_ok=True) handle, tmp_name = tempfile.mkstemp(dir=str(path.parent), prefix=path.name, suffix=".tmp") @@ -43,6 +48,11 @@ def atomic_write(path: Path, content: str) -> None: fh.write(content) fh.flush() os.fsync(fh.fileno()) + if mode is not None: + try: + os.chmod(tmp, mode) + except OSError: + pass os.replace(tmp, path) except BaseException: tmp.unlink(missing_ok=True) diff --git a/plugins/agentrust-codex/engine/_vendor/agentrust_capture_core/state.py b/plugins/agentrust-codex/engine/_vendor/agentrust_capture_core/state.py index c5e87c9..5a0839d 100644 --- a/plugins/agentrust-codex/engine/_vendor/agentrust_capture_core/state.py +++ b/plugins/agentrust-codex/engine/_vendor/agentrust_capture_core/state.py @@ -28,12 +28,17 @@ class StatePaths: latest: Path -def atomic_write(path: Path, content: str) -> None: +def atomic_write(path: Path, content: str, *, mode: int | None = None) -> None: """Write via a temporary file and replace, so a crash cannot truncate state. A half-written baseline is worse than a missing one: the engine would treat it as corrupt on every future session, and a user who sees a broken check often enough stops reading it. + + ``mode`` is applied to the temporary file before the replace, so the file is + never briefly readable at wider permissions than intended. Callers that write + a private key pass ``0o600``. Best-effort, since not every filesystem carries + POSIX permissions. """ path.parent.mkdir(parents=True, exist_ok=True) handle, tmp_name = tempfile.mkstemp(dir=str(path.parent), prefix=path.name, suffix=".tmp") @@ -43,6 +48,11 @@ def atomic_write(path: Path, content: str) -> None: fh.write(content) fh.flush() os.fsync(fh.fileno()) + if mode is not None: + try: + os.chmod(tmp, mode) + except OSError: + pass os.replace(tmp, path) except BaseException: tmp.unlink(missing_ok=True) diff --git a/plugins/agentrust-codex/engine/capture.py b/plugins/agentrust-codex/engine/capture.py index 38af769..b197e5d 100644 --- a/plugins/agentrust-codex/engine/capture.py +++ b/plugins/agentrust-codex/engine/capture.py @@ -17,13 +17,21 @@ import re import socket import sys -import tempfile import time -import uuid from datetime import datetime, timedelta, timezone from pathlib import Path from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Set, Tuple +# Prefer the installed package; fall back to the pinned vendored copy. The hook +# runs before anything is installed, so the fallback is what makes drift detection +# work on a bare install. The copy is generated by scripts/sync_vendored_core.py +# and CI fails if it disagrees with the package. +try: + import agentrust_capture_core as core +except ImportError: # pragma: no cover - exercised by the bare-install path + sys.path.insert(0, str(Path(__file__).resolve().parent / "_vendor")) + import agentrust_capture_core as core + VERSION = "0.1.0" @@ -60,20 +68,21 @@ } -def _sha_bytes(value: bytes) -> str: - return "sha256:" + hashlib.sha256(value).hexdigest() - - -def _sha_file(path: Path) -> str: - return _sha_bytes(path.read_bytes()) - - -def _sha_mapping(value: Mapping[str, Any]) -> str: - encoded = json.dumps(value, sort_keys=True, separators=(",", ":")).encode("utf-8") - return _sha_bytes(encoded) +_sha_bytes = core.sha_bytes +_sha_file = core.sha_file +_sha_mapping = core.sha_mapping +_now_iso = core.now_iso +_uuid7 = core.uuid7 def _safe_hash(path: Path) -> Optional[str]: + """Digest a regular file, or None if it is missing, unreadable, or a symlink. + + Stricter than core.safe_sha_file, which does not consider symlinks. Kept + stricter here on purpose: Codex resolves instructions and policy from + per-workspace paths, so a cloned repo could point a symlink at a file outside + the tree and have its contents recorded as if they belonged to the workspace. + """ try: if path.is_file() and not path.is_symlink(): return _sha_file(path) @@ -82,20 +91,6 @@ def _safe_hash(path: Path) -> Optional[str]: return None -def _now_iso() -> str: - return ( - datetime.now(timezone.utc).isoformat(timespec="seconds").replace("+00:00", "Z") - ) - - -def _uuid7() -> str: - milliseconds = int(time.time() * 1000) - raw = bytearray(milliseconds.to_bytes(6, "big") + os.urandom(10)) - raw[6] = 0x70 | (raw[6] & 0x0F) - raw[8] = 0x80 | (raw[8] & 0x3F) - return str(uuid.UUID(bytes=bytes(raw))) - - def _slug(value: str) -> str: normalized = re.sub(r"[^a-z0-9._-]+", "-", value.lower()).strip("-") return normalized or "unknown" @@ -217,60 +212,16 @@ def _skill_roots(chain: Sequence[Path]) -> List[Tuple[str, Path]]: #: Controlled here rather than by a file inside the skill on purpose. A per-skill #: ignore file would let the thing being measured decide what gets measured, so a #: hostile skill could ship an ignore rule covering its own payload. -SKILL_EXCLUDE_DIRS = frozenset( - {"state", ".cache", "__pycache__", ".git", ".pytest_cache", "node_modules"} -) +SKILL_EXCLUDE_DIRS = core.EXCLUDE_DIRS #: File suffixes skipped for the same reason: run artifacts, not behaviour. -SKILL_EXCLUDE_SUFFIXES = frozenset({".log", ".tmp", ".pyc", ".pyo"}) +SKILL_EXCLUDE_SUFFIXES = core.EXCLUDE_SUFFIXES -def _skill_tree_digest(skill_dir: Path) -> Optional[str]: - """Hash every behavioural file in one skill directory. - - Covers the whole tree rather than SKILL.md alone. A skill is not just its - manifest: these directories carry scripts, tools and reference material that - decide what the skill actually does, so hashing only SKILL.md let a payload be - swapped into scripts/ while the report said nothing added, nothing subtracted. - - Relative paths are bound into the digest alongside contents so a rename or a - move is drift too, and symlinks are skipped so a link out of the tree cannot - drag unrelated content into the fingerprint. - """ - digest = hashlib.sha256() - try: - paths = sorted(skill_dir.rglob("*")) - except OSError: - return None - seen_any = False - for path in paths: - if path.is_symlink() or not path.is_file(): - continue - try: - relative = path.relative_to(skill_dir) - except ValueError: # pragma: no cover - rglob results are relative - continue - if SKILL_EXCLUDE_DIRS & set(relative.parts[:-1]): - continue - if path.suffix in SKILL_EXCLUDE_SUFFIXES: - continue - try: - content = path.read_bytes() - except OSError: - # An unreadable file is itself worth recording: bind its path so the - # file appearing or vanishing still moves the digest. - digest.update(relative.as_posix().encode("utf-8")) - digest.update(b"\0\0") - seen_any = True - continue - digest.update(relative.as_posix().encode("utf-8")) - digest.update(b"\0") - digest.update(content) - digest.update(b"\0") - seen_any = True - if not seen_any: - return None - return "sha256:" + digest.hexdigest() +#: Digest every behavioural file in one skill directory, or None when nothing +#: readable was found. See core.tree_digest for why the whole tree is covered +#: rather than SKILL.md alone, and for the symlink and exclusion behaviour. +_skill_tree_digest = core.tree_digest def _skill_fingerprints(chain: Sequence[Path]) -> Dict[str, str]: @@ -593,57 +544,24 @@ def snapshot(live: Optional[Mapping[str, Any]] = None) -> Dict[str, Any]: } -def _map_changes( - before: Mapping[str, str], - after: Mapping[str, str], - label: str, -) -> List[Dict[str, str]]: - changes: List[Dict[str, str]] = [] - before_keys, after_keys = set(before), set(after) - for name in sorted(after_keys - before_keys): - changes.append({"change": "added", "what": label, "detail": name}) - for name in sorted(before_keys - after_keys): - changes.append({"change": "removed", "what": label, "detail": name}) - for name in sorted(before_keys & after_keys): - if before[name] != after[name]: - changes.append({"change": "changed", "what": label, "detail": name}) - return changes - - -def _set_changes( - before: Iterable[str], - after: Iterable[str], - label: str, -) -> List[Dict[str, str]]: - changes: List[Dict[str, str]] = [] - before_set, after_set = set(before), set(after) - for name in sorted(after_set - before_set): - changes.append({"change": "added", "what": label, "detail": name}) - for name in sorted(before_set - after_set): - changes.append({"change": "removed", "what": label, "detail": name}) - return changes +_map_changes = core.diff_maps +_set_changes = core.diff_sets def diff(base: Mapping[str, Any], current: Mapping[str, Any]) -> List[Dict[str, str]]: - common = set(base.get("observed", [])) & set(current.get("observed", [])) + common = core.observed_categories(base, current) changes: List[Dict[str, str]] = [] # A baseline written before MEASUREMENT_SCOPE 2 holds skill digests over # SKILL.md alone, so comparing them against whole-directory digests would - # report every skill as changed. Drop skills from the comparison and say why. - base_scope = base.get("scope", 1) - if base_scope != MEASUREMENT_SCOPE: - changes.append( - { - "change": "changed", - "what": "measurement scope", - "detail": ( - "widened from %s to %s; skill digests now cover the whole skill " - "directory. Re-approve once to compare on the new scope." - % (base_scope, MEASUREMENT_SCOPE) - ), - } - ) + # report every skill as changed. Drop skills and say why. + scope = core.scope_change( + base, MEASUREMENT_SCOPE, + affected=["skills"], + reason="skill digests now cover the whole skill directory.", + ) + if scope is not None: + changes.append(scope) common.discard("skills") if "instructions" in common: @@ -834,34 +752,23 @@ def build_trace(current: Mapping[str, Any]) -> Dict[str, Any]: def _atomic_write(path: Path, content: str) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - temporary: Optional[str] = None - try: - with tempfile.NamedTemporaryFile( - mode="w", - encoding="utf-8", - dir=str(path.parent), - prefix=".%s." % path.name, - delete=False, - ) as handle: - handle.write(content) - handle.flush() - os.fsync(handle.fileno()) - temporary = handle.name - try: - os.chmod(temporary, 0o600) - except OSError: - pass - os.replace(temporary, path) - finally: - if temporary and os.path.exists(temporary): - try: - os.unlink(temporary) - except OSError: - pass + """Owner-only atomic write. + + 0o600 rather than the core default, because _save also writes the Ed25519 + signing key. The core applies the mode before the replace, so the file is never + briefly world-readable. + """ + core.atomic_write(path, content, mode=0o600) def _save(path: Path, value: Mapping[str, Any]) -> None: + """Serialise sorted with a trailing newline. + + Kept local rather than using core.save_state: this engine's state files are + sorted and newline-terminated, and switching the format would rewrite every + existing file for no behavioural gain. The digest is computed over the mapping + rather than the bytes, so the two formats are interchangeable in meaning. + """ _atomic_write(path, json.dumps(value, indent=2, sort_keys=True) + "\n") @@ -953,7 +860,7 @@ def sign_all( #: Shown wherever a category was not measured. An integrity report must not let #: "we did not check" read like "we checked and there is nothing", because a #: reader who cannot tell them apart will treat an absent measurement as a pass. -UNMEASURED = "not measured this run" +UNMEASURED = core.UNMEASURED def render_report( diff --git a/scheduled-agents/engine/_vendor/agentrust_capture_core/state.py b/scheduled-agents/engine/_vendor/agentrust_capture_core/state.py index c5e87c9..5a0839d 100644 --- a/scheduled-agents/engine/_vendor/agentrust_capture_core/state.py +++ b/scheduled-agents/engine/_vendor/agentrust_capture_core/state.py @@ -28,12 +28,17 @@ class StatePaths: latest: Path -def atomic_write(path: Path, content: str) -> None: +def atomic_write(path: Path, content: str, *, mode: int | None = None) -> None: """Write via a temporary file and replace, so a crash cannot truncate state. A half-written baseline is worse than a missing one: the engine would treat it as corrupt on every future session, and a user who sees a broken check often enough stops reading it. + + ``mode`` is applied to the temporary file before the replace, so the file is + never briefly readable at wider permissions than intended. Callers that write + a private key pass ``0o600``. Best-effort, since not every filesystem carries + POSIX permissions. """ path.parent.mkdir(parents=True, exist_ok=True) handle, tmp_name = tempfile.mkstemp(dir=str(path.parent), prefix=path.name, suffix=".tmp") @@ -43,6 +48,11 @@ def atomic_write(path: Path, content: str) -> None: fh.write(content) fh.flush() os.fsync(fh.fileno()) + if mode is not None: + try: + os.chmod(tmp, mode) + except OSError: + pass os.replace(tmp, path) except BaseException: tmp.unlink(missing_ok=True) diff --git a/scheduled-agents/engine/capture.py b/scheduled-agents/engine/capture.py index 0a55845..963a1af 100644 --- a/scheduled-agents/engine/capture.py +++ b/scheduled-agents/engine/capture.py @@ -38,16 +38,23 @@ from __future__ import annotations import argparse -import hashlib import json import os import stat import sys import time -import uuid -from datetime import datetime, timezone from pathlib import Path +# Prefer the installed package; fall back to the pinned vendored copy. The hook +# runs before anything is installed, so the fallback is what makes drift detection +# work on a bare install. The copy is generated by scripts/sync_vendored_core.py +# and CI fails if it disagrees with the package. +try: + import agentrust_capture_core as core +except ImportError: # pragma: no cover - exercised by the bare-install path + sys.path.insert(0, str(Path(__file__).resolve().parent / "_vendor")) + import agentrust_capture_core as core + CLAUDE_HOME = Path(os.path.expanduser("~")) / ".claude" STATE_DIR = CLAUDE_HOME / "agentrust" / "scheduled" BASELINE = STATE_DIR / "baseline.json" @@ -61,8 +68,8 @@ # --------------------------------------------------------------------------- # # hashing (never secrets) # --------------------------------------------------------------------------- # -def _sha_bytes(b: bytes) -> str: - return "sha256:" + hashlib.sha256(b).hexdigest() +_sha_bytes = core.sha_bytes +_now_iso = core.now_iso def _sha_text(s: str) -> str: @@ -70,20 +77,15 @@ def _sha_text(s: str) -> str: def _sha_obj(obj: object) -> str: - return _sha_bytes(json.dumps(obj, sort_keys=True).encode("utf-8")) - + """Digest an arbitrary object by sorted JSON. -def _now_iso() -> str: - return datetime.now(timezone.utc).isoformat(timespec="seconds").replace("+00:00", "Z") + Kept local rather than core.sha_mapping, which takes a mapping: this is also + called on lists of allowed tools and MCP names. Same canonicalisation. + """ + return _sha_bytes(json.dumps(obj, sort_keys=True).encode("utf-8")) -def _uuid7() -> str: - """RFC 9562 UUID v7 (time-ordered).""" - ms = int(time.time() * 1000) - b = bytearray(ms.to_bytes(6, "big") + os.urandom(10)) - b[6] = 0x70 | (b[6] & 0x0F) - b[8] = 0x80 | (b[8] & 0x3F) - return str(uuid.UUID(bytes=bytes(b))) +_uuid7 = core.uuid7 # --------------------------------------------------------------------------- # @@ -409,25 +411,12 @@ def render_report(cur: dict, changes: list[dict] | None, signed: bool) -> str: # --------------------------------------------------------------------------- # # state helpers # --------------------------------------------------------------------------- # -def _save(path: Path, obj: dict) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(json.dumps(obj, indent=2), encoding="utf-8") +#: Atomic via the core, so a crash mid-write cannot leave a truncated baseline +#: that reads as corrupt on every future session. +_save = core.save_state -def _load(path: Path) -> dict | None: - """Load a state file, or None if absent, unreadable, or corrupt. - - A truncated baseline (crash mid-write, disk full, racing sessions) must not - brick the hook forever. Treating corrupt state as absent lets the next run - re-establish it. - """ - if not path.is_file(): - return None - try: - data = json.loads(path.read_text(encoding="utf-8")) - except (json.JSONDecodeError, OSError): - return None - return data if isinstance(data, dict) else None +_load = core.load_state # --------------------------------------------------------------------------- #