diff --git a/.agents/hooks/rules/related_test_extensions.json b/.agents/hooks/rules/related_test_extensions.json index 05b2d9a0..c72d6ee4 100644 --- a/.agents/hooks/rules/related_test_extensions.json +++ b/.agents/hooks/rules/related_test_extensions.json @@ -4,6 +4,7 @@ "tests/agents/hooks/test_check_selection.py" ], "agents/hooks/rules/secret_path_policy.json": [ + "tests/agents/hooks/test_secret_path_policy.py", "tests/agents/codex/test_hook_adapter_cli.py", "tests/agents/hooks/test_verification_rule.py" ], @@ -36,6 +37,7 @@ ], "agents/claude/settings.json": [ "tests/agents/claude/test_settings.py", + "tests/agents/hooks/test_sandbox_escape.py", "tests/agents/hooks/test_allowed_command.py", "tests/agents/hooks/test_command_cli.py", "tests/agents/test_notification.py" @@ -50,6 +52,7 @@ "tests/nix/configuration.bats" ], "agents/hooks.json": [ + "tests/agents/hooks/test_sandbox_escape.py", "tests/agents/test_notification.py", "tests/agents/hooks/test_verification_rule.py", "tests/agents/test_python_runtime.py", @@ -331,5 +334,14 @@ "agents/pi/hooks/hook_adapter.py": [ "tests/agents/pi/test_pi_hook_adapter.py", "tests/agents/test_hook_generation.py" + ], + "agents/hooks/lib/provider_adapter.py": [ + "tests/agents/test_provider_adapters.py", + "tests/agents/hooks/test_hook_boundary.py", + "tests/agents/codex/test_hook_adapter_cli.py", + "tests/agents/devin/test_hook_adapter.py", + "tests/agents/hermes/test_hermes_hook_adapter.py", + "tests/agents/pi/test_pi_hook_adapter.py", + "tests/agents/test_python_runtime.py" ] } diff --git a/agents/AGENTS.md b/agents/AGENTS.md index 9e7313b7..4ac71166 100644 --- a/agents/AGENTS.md +++ b/agents/AGENTS.md @@ -26,7 +26,8 @@ Existing project style takes precedence over these rules. - Keep classes and modules single-purpose, cohesive, loosely coupled, and minimally public. - Name classes by responsibility; order methods public-to-private; use DTOs at component boundaries. - Keep domain-specific enums and exception classes near their owning class; use one exception class per domain failure concept unless it is shared across modules. -- Keep functions small and single-purpose: prefer 0-3 arguments, one abstraction level, 2-4 lines when practical, short variable lifetimes, guard clauses, and at most one indentation level; split only when the extracted name clarifies intent. +- Keep functions single-purpose at one abstraction level: prefer 0-3 arguments, short variable lifetimes, and guard clauses over nested conditionals; keep nesting to about two levels. +- Extract a function when its name states intent the inline code does not; do not split merely to shorten a function. - Split duplicated logic, control structures, mixed responsibilities, and command/query behavior into named functions or objects. - Name one concept with one word; name command functions for their side effects and query functions for the value they return. - Let name length match scope size; include units, trust/safety attributes, and boolean prefixes where they clarify meaning. @@ -38,3 +39,5 @@ Existing project style takes precedence over these rules. - Prefer Collection Objects for domain collections with invariants; do not expose mutable raw collections - Use Entities only when stable identity matters across state changes - Use classification objects/enums when categories or state transitions have domain rules +- Parse raw input into constrained types once at the boundary; do not re-validate the same value downstream +- Make invalid states unrepresentable where the language allows: return expected failures as values (Result or an equivalent) and handle sum types exhaustively. Types do not replace tests for business rules, ordering, side effects, or integration diff --git a/agents/claude/settings.json b/agents/claude/settings.json index 241ad126..466b2ae6 100644 --- a/agents/claude/settings.json +++ b/agents/claude/settings.json @@ -10,7 +10,10 @@ "GIT_CONFIG_VALUE_0": "false" }, "permissions": { - "defaultMode": "auto" + "defaultMode": "auto", + "ask": [ + "Bash(dangerouslyDisableSandbox:true)" + ] }, "model": "fable", "preferredNotifChannel": "auto", @@ -48,7 +51,10 @@ "allowUnsandboxedCommands": true, "filesystem": { "allowWrite": [ - "$HOME/.cache/nix" + "~/.cache/nix", + "~/.cache/uv", + "~/Library/Caches/dprint", + "~/.local/state/agent-harness" ] }, "network": { @@ -57,8 +63,10 @@ ] }, "excludedCommands": [ - "docker", - "watchman" + "docker *", + "gh *", + "herdr *", + "watchman *" ] }, "effortLevel": "xhigh", diff --git a/agents/codex/config.toml b/agents/codex/config.toml index d8e1bdec..516aa75a 100644 --- a/agents/codex/config.toml +++ b/agents/codex/config.toml @@ -44,9 +44,9 @@ enabled = false [features] hooks = true -remote_connections = true external_migration = true [features.multi_agent_v2] +enabled = true min_wait_timeout_ms = 120000 default_wait_timeout_ms = 120000 diff --git a/agents/codex/hooks/hook_adapter.py b/agents/codex/hooks/hook_adapter.py index 6de6b9af..e50cc4e9 100755 --- a/agents/codex/hooks/hook_adapter.py +++ b/agents/codex/hooks/hook_adapter.py @@ -1,152 +1,25 @@ #!/usr/bin/env -S python3 -IB """Translate Codex hook payloads at the boundary to shared hook implementations.""" -import json -import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() -COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name == ".codex" else "hooks") +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. +COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax +import provider_adapter -EXTENSIONS = lint_format.EXTENSIONS - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def translated(payload: dict, tool: str, values: dict) -> dict: - return { - "tool_name": tool, - "tool_input": values, - "session_id": payload.get("session_id") or "", - "cwd": payload.get("cwd") or "", - "provider": "codex", - } - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using Codex's PostToolUse JSON contract.""" - lint_format.emit(lint_format.diagnostics(payload)) - return 0 - - -def harness(payload: dict) -> int: - values = payload.get("tool_input") or {} - body = hook_input.patch_body(values, payload.get("tool_name")) - for name in patch_input.paths(body): - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", translated(payload, "apply_patch", {"file_path": absolute}), shared_directory() - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = payload.get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, payload.get("tool_name"))), - ] - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=payload, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = payload.get("tool_input") or {} - if kind == "shell": - command = translated(payload, "Bash", {"command": values.get("command") or values.get("cmd") or ""}) - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - if arguments[0] == "prompt": - return guard_file.check("prompt", payload, shared_directory()) - return guard_file.check( - "write", - translated( - payload, - "Edit", - {"content": patch_input.added_text(hook_input.patch_body(values, payload.get("tool_name")))}, - ), - shared_directory(), - ) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or ( - kind in {"paths", "content"} - and len(rest) == 1 - and rest[0] in ({"command", "patch"} if kind == "paths" else {"prompt", "apply-patch"}) - ) +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="codex", + content_modes=("prompt", "apply-patch"), + cwd_fallback=provider_adapter.no_cwd, ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py ", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if payload.get("cwd"): - os.chdir(payload["cwd"]) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/command_permissions.json b/agents/command_permissions.json index 746eaa29..b7a7db36 100644 --- a/agents/command_permissions.json +++ b/agents/command_permissions.json @@ -1019,32 +1019,27 @@ { "decision": "allow", "prefix": ["docker", "ps"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "images"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "logs"], - "justification": "Allow read-only container inspection and image builds." - }, - { - "decision": "allow", - "prefix": ["docker", "build"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "compose", "ps"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", "prefix": ["docker", "compose", "logs"], - "justification": "Allow read-only container inspection and image builds." + "justification": "Allow read-only container inspection." }, { "decision": "allow", @@ -1328,7 +1323,12 @@ }, { "decision": "ask", - "prefix": ["herdr", "pane", "send"], + "prefix": ["herdr", "pane", "send-text"], + "justification": "Ask before controlling other panes; their commands run outside this policy." + }, + { + "decision": "ask", + "prefix": ["herdr", "pane", "send-keys"], "justification": "Ask before controlling other panes; their commands run outside this policy." }, { @@ -1348,7 +1348,12 @@ }, { "decision": "ask", - "prefix": ["herdr", "agent", "send"], + "prefix": ["herdr", "agent", "send-keys"], + "justification": "Ask before controlling other panes; their commands run outside this policy." + }, + { + "decision": "ask", + "prefix": ["herdr", "agent", "prompt"], "justification": "Ask before controlling other panes; their commands run outside this policy." }, { @@ -1381,6 +1386,11 @@ "prefix": ["esa", "post", "update"], "justification": "Ask before publishing or triggering work that is visible to others." }, + { + "decision": "ask", + "prefix": ["docker", "build"], + "justification": "Ask before building images; build steps run arbitrary commands outside the sandbox." + }, { "decision": "ask", "prefix": ["docker", "run"], diff --git a/agents/devin/hooks/hook_adapter.py b/agents/devin/hooks/hook_adapter.py old mode 100644 new mode 100755 index 8a245a98..f0440b2c --- a/agents/devin/hooks/hook_adapter.py +++ b/agents/devin/hooks/hook_adapter.py @@ -1,224 +1,49 @@ #!/usr/bin/env -S python3 -IB """Translate Devin hook payloads at the boundary to shared hook implementations.""" -import json import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax -import verification_session - -EXTENSIONS = lint_format.EXTENSIONS - -CANONICAL_TOOLS = { - "exec": "Bash", - "read": "Read", - "write": "Write", - "edit": "Edit", - "multi_edit": "MultiEdit", - "notebook_edit": "NotebookEdit", - "run_subagent": "Task", - "webfetch": "WebFetch", -} - -NATIVE_EVENTS = { - "session-start": "SessionStart", - "pre-tool-use": "PreToolUse", - "stop": "Stop", - "session-end": "SessionEnd", -} - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def normalized(payload: dict) -> dict: - """Preserve the provider event while supplying the fields shared checks require.""" - result = dict(payload) - result.setdefault("cwd", os.environ.get("DEVIN_PROJECT_DIR") or "") - result["provider"] = "devin" - if canonical := CANONICAL_TOOLS.get(result.get("tool_name") or ""): - result["tool_name"] = canonical - return result - - -def devin_result(event: str, result: dict) -> dict: - """Convert the shared Claude-shaped decision into Devin's native output contract.""" - specific = result.get("hookSpecificOutput") - if isinstance(specific, dict) and specific.get("permissionDecision") == "deny": - reason = specific.get("permissionDecisionReason") or "Blocked by hook policy" - return {"decision": "block", "reason": reason} - if message := result.get("systemMessage"): - return { - "hookSpecificOutput": { - "hookEventName": NATIVE_EVENTS.get(event, event), - "additionalContext": message, - } - } - return result - - -def native(event: str, payload: dict) -> int: - normalized_payload = normalized(payload) - try: - result = verification_session.dispatch("devin", event, normalized_payload) - except (OSError, ValueError, TypeError, KeyError) as error: - result = verification_session.failure(event, normalized_payload, error) - print(json.dumps(devin_result(event, result), ensure_ascii=True, separators=(",", ":"))) - return 0 - - -def audit(kind: str, payload: dict) -> int: - context = normalized(payload) - entry = audit_log.record(kind, context, "devin") - cwd = context.get("cwd") or "." - audit_log.append_record(entry, cwd=Path(cwd)) - return 0 - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using Devin's PostToolUse JSON contract.""" - lint_format.emit(lint_format.diagnostics(normalized(payload))) - return 0 - - -def harness(payload: dict) -> int: - values = payload.get("tool_input") or {} - candidates = [values.get("file_path") or values.get("path") or ""] - candidates += patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))) - for name in candidates: - if not name: - continue - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", - normalized(payload) | {"tool_name": "apply_patch", "tool_input": {"file_path": absolute}}, - shared_directory(), - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = payload.get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))), - ] - context = normalized(payload) - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=context, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def content(mode: str, payload: dict) -> int: - if mode == "apply-patch": - values = payload.get("tool_input") or {} - body = hook_input.patch_body(values, normalized(payload).get("tool_name") or "apply_patch") - return guard_file.check( - "write", - normalized(payload) | {"tool_name": "Edit", "tool_input": {"content": patch_input.added_text(body)}}, - shared_directory(), - ) - return guard_file.check(mode, normalized(payload), shared_directory()) - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = payload.get("tool_input") or {} - if kind == "native": - return native(arguments[0], payload) - if kind == "audit": - return audit(arguments[0], payload) - if kind == "shell": - command = normalized(payload) | { - "tool_name": "Bash", - "tool_input": {"command": values.get("command") or values.get("cmd") or ""}, - } - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - return content(arguments[0], payload) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind == "native" and len(rest) == 1 and rest[0] in NATIVE_EVENTS) - or (kind == "audit" and len(rest) == 1 and rest[0] in {"tool", "compaction"}) - or (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or (kind == "paths" and len(rest) == 1 and rest[0] in {"command", "patch"}) - or (kind == "content" and len(rest) == 1 and rest[0] in {"prompt", "read", "write", "apply-patch"}) +import provider_adapter + + +def project_directory() -> str: + """Devin omits cwd from hook input and exports the project directory instead.""" + return os.environ.get("DEVIN_PROJECT_DIR") or "" + + +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="devin", + tools={ + "exec": "Bash", + "read": "Read", + "write": "Write", + "edit": "Edit", + "multi_edit": "MultiEdit", + "notebook_edit": "NotebookEdit", + "run_subagent": "Task", + "webfetch": "WebFetch", + }, + events={ + "session-start": "SessionStart", + "pre-tool-use": "PreToolUse", + "stop": "Stop", + "session-end": "SessionEnd", + }, + compaction_events={"SessionStart": "SessionStart", "PostCompaction": "PostCompaction"}, + formatter=provider_adapter.additional_context, + cwd_fallback=project_directory, ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py ", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if not isinstance(payload, dict): - payload = {} - cwd = payload.get("cwd") or os.environ.get("DEVIN_PROJECT_DIR") - if cwd: - os.chdir(cwd) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/hermes/hooks/hook_adapter.py b/agents/hermes/hooks/hook_adapter.py index 030f67be..51bef562 100755 --- a/agents/hermes/hooks/hook_adapter.py +++ b/agents/hermes/hooks/hook_adapter.py @@ -1,232 +1,44 @@ #!/usr/bin/env -S python3 -IB """Translate Hermes hook payloads at the boundary to shared hook implementations.""" -import json -import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_dispatch -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax -import verification_session - -CANONICAL_TOOLS = { - "terminal": "Bash", - "exec_command": "Bash", - "shell": "Bash", - "read_file": "Read", - "write_file": "Write", - "edit_file": "Edit", - "patch": "Edit", - "apply_patch": "apply_patch", - "web_search": "WebSearch", - "fetch_url": "WebFetch", -} - -NATIVE_EVENTS = { - "session-start": "on_session_start", - "pre-tool-use": "pre_tool_call", - "stop": "pre_verify", - "session-end": "on_session_end", -} - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def manifest_path() -> Path: - return Path(os.environ.get("AGENT_HARNESS_HERMES_MANIFEST", str(Path.home() / ".hermes/hooks.json"))) - - -def normalized(payload: dict) -> dict: - """Preserve the provider event while supplying the fields shared checks require.""" - result = dict(payload) - values = result.get("tool_input") - if not isinstance(values, dict): - values = result.get("args") - result["tool_input"] = dict(values) if isinstance(values, dict) else {} - result["tool_input"].setdefault("file_path", result["tool_input"].get("path") or "") - result.setdefault("cwd", os.getcwd()) - result["provider"] = "hermes" - if canonical := CANONICAL_TOOLS.get(result.get("tool_name") or ""): - result["tool_name"] = canonical - return result - - -def hermes_result(event: str, result: dict) -> dict: - """Convert the shared Claude-shaped decision into Hermes's blocking contract.""" - specific = result.get("hookSpecificOutput") - if isinstance(specific, dict) and specific.get("permissionDecision") == "deny": - reason = specific.get("permissionDecisionReason") or "Blocked by hook policy" - return {"decision": "block", "reason": reason} - if message := result.get("systemMessage"): - return {"systemMessage": message} - return result - - -def native(event: str, payload: dict) -> int: - normalized_payload = normalized(payload) - try: - result = verification_session.dispatch("hermes", event, normalized_payload) - except (OSError, ValueError, TypeError, KeyError) as error: - result = verification_session.failure(event, normalized_payload, error) - print(json.dumps(hermes_result(event, result), ensure_ascii=True, separators=(",", ":"))) - return 0 - - -def audit(kind: str, payload: dict) -> int: - context = normalized(payload) - entry = audit_log.record(kind, context, "hermes") - cwd = context.get("cwd") or "." - audit_log.append_record(entry, cwd=Path(cwd)) - return 0 - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using the post-tool JSON contract.""" - lint_format.emit(lint_format.diagnostics(normalized(payload))) - return 0 - - -def harness(payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - candidates = [values.get("file_path") or values.get("path") or ""] - candidates += patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))) - for name in candidates: - if not name: - continue - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", - normalized(payload) | {"tool_name": "apply_patch", "tool_input": {"file_path": absolute}}, - shared_directory(), - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = normalized(payload).get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))), - ] - context = normalized(payload) - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=context, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def content(mode: str, payload: dict) -> int: - if mode == "apply-patch": - values = normalized(payload).get("tool_input") or {} - body = hook_input.patch_body(values, normalized(payload).get("tool_name") or "apply_patch") - return guard_file.check( - "write", - normalized(payload) | {"tool_name": "Edit", "tool_input": {"content": patch_input.added_text(body)}}, - shared_directory(), - ) - return guard_file.check(mode, normalized(payload), shared_directory()) - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - if kind == "native": - return native(arguments[0], payload) - if kind == "audit": - return audit(arguments[0], payload) - if kind == "dispatch": - return hook_dispatch.run(manifest_path(), arguments[0], payload) - if kind == "shell": - command = normalized(payload) | { - "tool_name": "Bash", - "tool_input": {"command": values.get("command") or values.get("cmd") or ""}, - } - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - return content(arguments[0], payload) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind == "native" and len(rest) == 1 and rest[0] in NATIVE_EVENTS) - or (kind == "audit" and len(rest) == 1 and rest[0] in {"tool", "compaction"}) - or (kind == "dispatch" and len(rest) == 1) - or (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or (kind == "paths" and len(rest) == 1 and rest[0] in {"command", "patch"}) - or (kind == "content" and len(rest) == 1 and rest[0] in {"prompt", "read", "write", "apply-patch"}) +import provider_adapter + +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="hermes", + tools={ + "terminal": "Bash", + "exec_command": "Bash", + "shell": "Bash", + "read_file": "Read", + "write_file": "Write", + "edit_file": "Edit", + "patch": "Edit", + "apply_patch": "apply_patch", + "web_search": "WebSearch", + "fetch_url": "WebFetch", + }, + events={ + "session-start": "on_session_start", + "pre-tool-use": "pre_tool_call", + "stop": "pre_verify", + "session-end": "on_session_end", + }, + compaction_events={"on_session_start": "SessionStart"}, + manifest_env="AGENT_HARNESS_HERMES_MANIFEST", + manifest_default=".hermes/hooks.json", ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py |shell allowed/forbidden/git/commit|lint|harness" - "|paths command/patch|content prompt/read/write/apply-patch>", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if not isinstance(payload, dict): - payload = {} - cwd = payload.get("cwd") - if isinstance(cwd, str) and cwd: - os.chdir(cwd) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/hooks.json b/agents/hooks.json index 3163b6f6..cd958a84 100644 --- a/agents/hooks.json +++ b/agents/hooks.json @@ -48,7 +48,7 @@ ] }, { - "matcher": "Read", + "matcher": "Read|Grep", "hooks": [ { "command": "\"$HOME/.claude/hooks/audit_log.py\" tool claude", @@ -61,7 +61,7 @@ ] }, { - "matcher": "Write|Edit", + "matcher": "Write|Edit|NotebookEdit", "hooks": [ { "command": "\"$HOME/.claude/hooks/audit_log.py\" tool claude", @@ -80,7 +80,7 @@ ], "PostToolUse": [ { - "matcher": "Write|Edit", + "matcher": "Write|Edit|NotebookEdit", "hooks": [ { "command": "\"$HOME/.claude/hooks/lint_format.py\"", diff --git a/agents/hooks/audit_log.py b/agents/hooks/audit_log.py index 6992c5c8..9f1a5e9a 100755 --- a/agents/hooks/audit_log.py +++ b/agents/hooks/audit_log.py @@ -17,6 +17,7 @@ from session_store import file_lock, private_directory, worktree_id +import hook_input import patch_input @@ -89,8 +90,7 @@ def _targets(tool: str, values: dict, cwd: Path) -> list[str]: if isinstance(command, str): names = patch_input.paths(command) elif tool in FILE_TOOLS: - file_path = values.get("file_path") or values.get("path") - names = (file_path,) if isinstance(file_path, str) else () + names = (name,) if (name := hook_input.file_path(values)) else () edits = values.get("edits") if isinstance(edits, list): names += tuple( diff --git a/agents/hooks/guard_file.py b/agents/hooks/guard_file.py index a0267e9b..5778f137 100755 --- a/agents/hooks/guard_file.py +++ b/agents/hooks/guard_file.py @@ -14,6 +14,7 @@ import audit_log from command_policy import regex_matches +import hook_input def policy_path(environment: str, name: str, directory: Path) -> Path: @@ -97,8 +98,9 @@ def content_text(mode: str, payload: dict) -> str: if mode == "prompt": return payload.get("prompt") or "" if mode == "write": - return (values.get("content") or "") + (values.get("new_string") or "") - path = values.get("file_path") or "" + return "".join(values.get(key) or "" for key in ("content", "new_string", "new_source")) + # Grep may name a directory; only a single file target can be scanned before the tool runs. + path = hook_input.file_path(values) if not path or not Path(path).is_file(): return "" with Path(path).open("rb") as stream: @@ -141,7 +143,7 @@ def scan_content(mode: str, payload: dict, directory: Path) -> None: tool = ( "UserPromptSubmit" if mode == "prompt" else "Read" if mode == "read" else payload.get("tool_name") or "Write" ) - summary = "" if mode == "prompt" else values.get("file_path") or "" + summary = "" if mode == "prompt" else hook_input.file_path(values) audit_log.blocked( tool, summary, @@ -177,9 +179,7 @@ def check(mode: str, payload: dict, directory: Path = ROOT) -> int: return 0 values = payload.get("tool_input") or {} tool = payload.get("tool_name") or tool - value = ( - values.get("command") or "" if mode == "commit" else values.get("file_path") or values.get("path") or "" - ) + value = values.get("command") or "" if mode == "commit" else hook_input.file_path(values) if not value: return 0 reason = ( diff --git a/agents/hooks/lib/check_selection.py b/agents/hooks/lib/check_selection.py index 606b82a1..c1ea0045 100644 --- a/agents/hooks/lib/check_selection.py +++ b/agents/hooks/lib/check_selection.py @@ -193,6 +193,8 @@ def language_plan(root: Path, rules: dict, changed: tuple[str, ...]) -> tuple[li affected = changed_language(rule, changed) or triggered if not affected and not explicit[language]: continue + if not triggered and not explicit[language] and delegated(rule, changed, covered, language): + continue markers = rule.get("project_markers", ()) if language == "javascript_typescript": markers = ("package.json",) @@ -248,6 +250,13 @@ def language_plan(root: Path, rules: dict, changed: tuple[str, ...]) -> tuple[li return invocations, errors +def delegated(rule: dict, changed: tuple[str, ...], covered: dict[str, set[str]], language: str) -> bool: + """Whether every changed source of a language is explicitly verified by another language's tests.""" + sources = {path for path in changed if any(path.endswith(extension) for extension in rule["source_extensions"])} + others = set().union(*(files for name, files in covered.items() if name != language)) + return bool(sources) and sources <= others + + def matching_targets(root: Path, language: str, rule: dict, changed: tuple[str, ...]) -> tuple[set[str], set[str]]: """Return each language's matched tests and the sources that resolve to none.""" if language == "rust": diff --git a/agents/hooks/lib/hook_input.py b/agents/hooks/lib/hook_input.py index 21a6c9fe..6727d008 100644 --- a/agents/hooks/lib/hook_input.py +++ b/agents/hooks/lib/hook_input.py @@ -29,6 +29,12 @@ ".sty": "tex", } EDIT_TOOLS = frozenset({"Write", "Edit", "MultiEdit", "NotebookEdit", "apply_patch"}) +FILE_FIELDS = ("file_path", "path", "notebook_path") + + +def file_path(values: dict) -> str: + """Return the single file a tool names, whichever provider field carries it.""" + return next((value for key in FILE_FIELDS if isinstance(value := values.get(key), str) and value), "") def _cwd(payload: dict) -> Path: @@ -74,9 +80,8 @@ def _input_names(payload: dict) -> tuple[str, ...]: if tool and tool not in EDIT_TOOLS: return () names = list(patch_input.paths(patch_body(values, tool))) - file_path = values.get("file_path") or values.get("path") - if isinstance(file_path, str): - names.append(file_path) + if name := file_path(values): + names.append(name) edits = values.get("edits") if isinstance(edits, list): names.extend( diff --git a/agents/hooks/lib/provider_adapter.py b/agents/hooks/lib/provider_adapter.py new file mode 100644 index 00000000..70b0b953 --- /dev/null +++ b/agents/hooks/lib/provider_adapter.py @@ -0,0 +1,309 @@ +"""Translate provider hook payloads at the boundary to shared hook implementations. + +Each provider adapter declares only a Profile of provider facts. Payload normalization, +guard dispatch, and CLI validation live here so the providers cannot drift apart. +""" + +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field +import json +import os +from pathlib import Path +import sys + +import audit_log +import guard_command +import guard_file +import guard_git +import hook_dispatch +import hook_input +import lint_format +import patch_input +import secret_path_policy +import shell_syntax +import verification_session + + +SECRET_PATH_RULE = "guard_secret_paths.sh" +SHELL_MODES = ("allowed", "forbidden", "git", "commit") +PATH_MODES = ("command", "patch") +AUDIT_KINDS = ("tool", "compaction") +CONTENT_MODES = ("prompt", "read", "write", "apply-patch") +NO_ARGUMENT: tuple[str, ...] = () +ANY_ARGUMENT = None +Formatter = Callable[[str, dict], dict] + + +def denial(result: dict) -> dict | None: + """Return the block decision for a shared Claude-shaped denial, if the result is one.""" + specific = result.get("hookSpecificOutput") + if not isinstance(specific, dict) or specific.get("permissionDecision") != "deny": + return None + return {"decision": "block", "reason": specific.get("permissionDecisionReason") or "Blocked by hook policy"} + + +def system_message(event: str, result: dict) -> dict: + """Format a shared result for providers that surface a top-level systemMessage.""" + message = result.get("systemMessage") + return denial(result) or ({"systemMessage": message} if message else result) + + +def additional_context(event: str, result: dict) -> dict: + """Format a shared result for providers that surface messages as additionalContext.""" + message = result.get("systemMessage") + context = {"hookSpecificOutput": {"hookEventName": event, "additionalContext": message}} + return denial(result) or (context if message else result) + + +def no_cwd() -> str: + return "" + + +@dataclass(frozen=True) +class Profile: + """Provider facts the shared adapter needs; every behavior derived from them is shared. + + `events` maps verification events to native event names and enables `native`. + `compaction_events` maps native event names to shared audit event names and enables `audit`. + `manifest_env` names the manifest override and enables `dispatch`. + """ + + name: str + tools: Mapping[str, str] = field(default_factory=dict) + events: Mapping[str, str] = field(default_factory=dict) + compaction_events: Mapping[str, str] = field(default_factory=dict) + formatter: Formatter = system_message + content_modes: tuple[str, ...] = CONTENT_MODES + cwd_fallback: Callable[[], str] = os.getcwd + manifest_env: str = "" + manifest_default: str = "" + + +class Adapter: + """Run one provider's hook commands against the shared hook implementations.""" + + def __init__(self, profile: Profile) -> None: + self.profile = profile + + def main(self, arguments: list[str]) -> int: + kind, rest = (arguments[0], arguments[1:]) if arguments else ("", []) + if not self._accepts(kind, rest): + print(f"Usage: hook_adapter.py <{self._usage()}>", file=sys.stderr) + return 1 + try: + return self.dispatch(kind, rest, self._payload()) + except (OSError, ValueError, TypeError, AttributeError) as error: + print(f"BLOCKED: {error}", file=sys.stderr) + return 2 + + def dispatch(self, kind: str, arguments: list[str], payload: dict) -> int: + if kind == "lint": + return self.lint(payload) + if kind == "harness": + return self.harness(payload) + commands = { + "native": self.native, + "audit": self.audit, + "dispatch": self.fan_out, + "shell": self.shell, + "paths": self.check_paths, + "content": self.content, + } + return commands[kind](arguments[0], payload) + + def normalized(self, payload: dict) -> dict: + """Copy the provider payload, adding the shared fields and canonical tool name.""" + result = dict(payload) + result["tool_input"] = tool_input(payload) + result.setdefault("cwd", self.profile.cwd_fallback()) + result.setdefault("prompt", payload.get("text") or "") + result["provider"] = self.profile.name + if canonical := self.profile.tools.get(payload.get("tool_name") or ""): + result["tool_name"] = canonical + return result + + def translated(self, payload: dict, tool: str, values: dict) -> dict: + """Forward only the fields a shared guard reads, under the shared tool name.""" + return { + "tool_name": tool, + "tool_input": values, + "session_id": payload.get("session_id") or "", + "cwd": payload.get("cwd") or self.profile.cwd_fallback(), + "provider": self.profile.name, + } + + def native(self, event: str, payload: dict) -> int: + context = self.normalized(payload) + try: + result = verification_session.dispatch(self.profile.name, event, context) + except (OSError, ValueError, TypeError, KeyError) as error: + result = verification_session.failure(event, context, error) + output = self.profile.formatter(self.profile.events[event], result) + print(json.dumps(output, ensure_ascii=True, separators=(",", ":"))) + return 0 + + def audit(self, kind: str, payload: dict) -> int: + context = self.normalized(payload) + if kind == "compaction": + context = self._compaction_context(context) + if context is not None: + audit_log.append_record( + audit_log.record(kind, context, self.profile.name), cwd=Path(context["cwd"] or ".") + ) + return 0 + + def fan_out(self, event: str, payload: dict) -> int: + return hook_dispatch.run(self._manifest(), event, payload) + + def shell(self, mode: str, payload: dict) -> int: + values = {"command": command_text(self.normalized(payload)["tool_input"])} + command = self.translated(payload, "Bash", values) + if mode == "git": + return guard_git.check(command) + if mode == "commit": + return guard_file.check("commit", command, shared_directory()) + return guard_command.check(mode, command, shared_directory()) + + def lint(self, payload: dict) -> int: + """Report shared quality diagnostics using the PostToolUse JSON contract.""" + lint_format.emit(lint_format.diagnostics(self.normalized(payload))) + return 0 + + def harness(self, payload: dict) -> int: + for name in edit_targets(self.normalized(payload)): + absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) + values = {"file_path": absolute} + if status := guard_file.check( + "harness", self.translated(payload, "apply_patch", values), shared_directory() + ): + return status + return 0 + + def check_paths(self, mode: str, payload: dict) -> int: + rules = secret_path_policy.load(secret_policy_path()) + context = self.normalized(payload) + candidates = edit_targets(context) if mode == "patch" else shell_words(command_text(context["tool_input"])) + for value in candidates: + if rule := secret_path_policy.blocked_rule(value, rules): + deny_secret_path(mode, value, rule, context) + return 0 + + def content(self, mode: str, payload: dict) -> int: + context = self.normalized(payload) + if mode != "apply-patch": + return guard_file.check(mode, context, shared_directory()) + body = hook_input.patch_body(context["tool_input"], context.get("tool_name") or "apply_patch") + values = {"content": patch_input.added_text(body)} + return guard_file.check("write", self.translated(payload, "Edit", values), shared_directory()) + + def _compaction_context(self, context: dict) -> dict | None: + """Name a native compaction event as shared audit expects, or drop an unknown event.""" + event = self.profile.compaction_events.get(context.get("hook_event_name") or "") + return None if event is None else context | {"hook_event_name": event} + + def _manifest(self) -> Path: + default = Path.home() / self.profile.manifest_default + return Path(os.environ.get(self.profile.manifest_env, str(default))) + + def _payload(self) -> dict: + payload = json.load(sys.stdin) + if not isinstance(payload, dict): + raise TypeError("hook payload must be an object") + cwd = payload.get("cwd") or self.profile.cwd_fallback() + if isinstance(cwd, str) and cwd: + os.chdir(cwd) + return payload + + def _accepts(self, kind: str, rest: list[str]) -> bool: + commands = self._commands() + if kind not in commands or any(value in {"-h", "--help"} for value in rest): + return False + choices = commands[kind] + if choices == NO_ARGUMENT: + return not rest + return len(rest) == 1 and (choices is ANY_ARGUMENT or rest[0] in choices) + + def _commands(self) -> dict[str, tuple[str, ...] | None]: + """Map each accepted command to its argument choices.""" + commands: dict[str, tuple[str, ...] | None] = {} + if self.profile.events: + commands["native"] = tuple(self.profile.events) + if self.profile.compaction_events: + commands["audit"] = AUDIT_KINDS + if self.profile.manifest_env: + commands["dispatch"] = ANY_ARGUMENT + return commands | { + "shell": SHELL_MODES, + "lint": NO_ARGUMENT, + "harness": NO_ARGUMENT, + "paths": PATH_MODES, + "content": self.profile.content_modes, + } + + def _usage(self) -> str: + return "|".join(usage_entry(kind, choices) for kind, choices in self._commands().items()) + + +def shared_directory() -> Path: + return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" + + +def secret_policy_path() -> Path: + default = shared_directory() / "rules/secret_path_policy.json" + return Path(os.environ.get("AGENT_SECRET_PATH_POLICY", str(default))) + + +def tool_input(payload: dict) -> object: + """Copy the tool arguments; a non-object is kept so the consuming check rejects it.""" + values = payload.get("tool_input") + if values is None: + # Hermes carries tool arguments in `args`. + values = payload.get("args") + if not isinstance(values, dict): + return {} if values is None else values + result = dict(values) + if name := hook_input.file_path(values): + result["file_path"] = name + return result + + +def command_text(values: dict) -> str: + return values.get("command") or values.get("cmd") or "" + + +def edit_targets(context: dict) -> list[str]: + values = context["tool_input"] + body = hook_input.patch_body(values, context.get("tool_name")) + return [name for name in (hook_input.file_path(values), *patch_input.paths(body)) if name] + + +def shell_words(command: str) -> list[str]: + """Return every argument and redirection word, plus the values of assignment words.""" + try: + commands = shell_syntax.parse(command) + except shell_syntax.UnsupportedSyntax as error: + raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error + words = [word for item in commands for word in (*item.arguments, *item.redirections)] + return words + [word.partition("=")[2] for word in words if "=" in word] + + +def deny_secret_path(mode: str, value: str, rule: dict, context: dict) -> None: + command = mode == "command" + audit_log.blocked( + "Bash" if command else "apply_patch", + command_text(context["tool_input"]) if command else value, + f"{rule['reason']}: {value}", + SECRET_PATH_RULE, + context.get("session_id") or "", + payload=context, + targets=() if command else (value,), + ) + raise ValueError( + f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" + ) + + +def usage_entry(kind: str, choices: tuple[str, ...] | None) -> str: + if choices is ANY_ARGUMENT: + return f"{kind} " + return f"{kind} {'/'.join(choices)}" if choices else kind diff --git a/agents/hooks/rules/allowed_commands.json b/agents/hooks/rules/allowed_commands.json index ed7589d4..23b3289f 100644 --- a/agents/hooks/rules/allowed_commands.json +++ b/agents/hooks/rules/allowed_commands.json @@ -144,7 +144,6 @@ "^docker ps( [^;&|<>$`]+)?$", "^docker images( [^;&|<>$`]+)?$", "^docker logs( [^;&|<>$`]+)?$", - "^docker build( [^;&|<>$`]+)?$", "^docker compose ps( [^;&|<>$`]+)?$", "^docker compose logs( [^;&|<>$`]+)?$", "^kubectl get( [^;&|<>$`]+)?$", diff --git a/agents/hooks/rules/secret_path_policy.json b/agents/hooks/rules/secret_path_policy.json index a67dadc8..8ab7b1ff 100644 --- a/agents/hooks/rules/secret_path_policy.json +++ b/agents/hooks/rules/secret_path_policy.json @@ -2,20 +2,55 @@ "version": 1, "rules": [ { - "pattern": ".env*", + "pattern": ".env", "access": ["read", "write"], "reason": "Environment files may contain credentials." }, { - "pattern": "**/*key*", + "pattern": ".env.*", + "access": ["read", "write"], + "reason": "Environment files may contain credentials." + }, + { + "pattern": "**/*.key", + "access": ["read"], + "reason": "Key files may contain private key material." + }, + { + "pattern": "**/*_key", "access": ["read"], "reason": "Key-named files may contain credentials." }, { - "pattern": "**/*token*", + "pattern": "**/*key*.json", + "access": ["read"], + "reason": "Key-named JSON files may contain credentials." + }, + { + "pattern": "**/*.pem", + "access": ["read"], + "reason": "PEM files may contain private key material." + }, + { + "pattern": "**/*.p12", + "access": ["read"], + "reason": "PKCS#12 bundles contain private key material." + }, + { + "pattern": "**/*.token", + "access": ["read"], + "reason": "Token files may contain credentials." + }, + { + "pattern": "**/*_token", "access": ["read"], "reason": "Token-named files may contain credentials." }, + { + "pattern": "**/*token*.json", + "access": ["read"], + "reason": "Token-named JSON files may contain credentials." + }, { "pattern": "**/.netrc", "access": ["read"], diff --git a/agents/manifest.json b/agents/manifest.json index 4c86629f..4fa53ee7 100644 --- a/agents/manifest.json +++ b/agents/manifest.json @@ -2,6 +2,7 @@ "version": 1, "runtime_commands": [ "bash", + "git", "jq", "python3", "shfmt", diff --git a/agents/pi/hooks/hook_adapter.py b/agents/pi/hooks/hook_adapter.py index 7e74d9bd..3bd66887 100755 --- a/agents/pi/hooks/hook_adapter.py +++ b/agents/pi/hooks/hook_adapter.py @@ -1,229 +1,42 @@ #!/usr/bin/env -S python3 -IB """Translate pi hook payloads at the boundary to shared hook implementations.""" -import json -import os from pathlib import Path import sys SCRIPT = Path(__file__).resolve() +# Deployed adapters live in ~/./hooks beside ~/.claude/hooks; the source tree keeps agents/hooks. COMMON = SCRIPT.parents[2] / (".claude/hooks" if SCRIPT.parent.parent.name.startswith(".") else "hooks") sys.path.insert(0, str(COMMON)) sys.path.insert(0, str(COMMON / "lib")) -import audit_log -import guard_command -import guard_git -import guard_file -import hook_dispatch -import hook_input -import lint_format -import patch_input -import secret_path_policy -import shell_syntax -import verification_session - -CANONICAL_TOOLS = { - "bash": "Bash", - "read": "Read", - "write": "Write", - "edit": "Edit", - "multi_edit": "MultiEdit", - "notebook_edit": "NotebookEdit", - "apply_patch": "apply_patch", - "webfetch": "WebFetch", - "web_search": "WebSearch", -} - -NATIVE_EVENTS = { - "session-start": "session_start", - "pre-tool-use": "tool_call", - "session-end": "session_shutdown", -} - - -def shared_directory() -> Path: - return Path(os.environ.get("AGENT_HARNESS_ROOT", str(Path.home()))) / ".claude/hooks" - - -def manifest_path() -> Path: - return Path(os.environ.get("AGENT_HARNESS_PI_MANIFEST", str(Path.home() / ".pi/agent/hooks.json"))) - - -def normalized(payload: dict) -> dict: - """Preserve the provider event while supplying the fields shared checks require.""" - result = dict(payload) - values = result.get("tool_input") - result["tool_input"] = dict(values) if isinstance(values, dict) else {} - result["tool_input"].setdefault("file_path", result["tool_input"].get("path") or "") - result.setdefault("cwd", os.getcwd()) - result.setdefault("prompt", result.get("text") or "") - result["provider"] = "pi" - if canonical := CANONICAL_TOOLS.get(result.get("tool_name") or ""): - result["tool_name"] = canonical - return result - - -def pi_result(event: str, result: dict) -> dict: - """Convert the shared Claude-shaped decision into pi's blocking contract.""" - specific = result.get("hookSpecificOutput") - if isinstance(specific, dict) and specific.get("permissionDecision") == "deny": - reason = specific.get("permissionDecisionReason") or "Blocked by hook policy" - return {"decision": "block", "reason": reason} - if message := result.get("systemMessage"): - return {"systemMessage": message} - return result - - -def native(event: str, payload: dict) -> int: - normalized_payload = normalized(payload) - try: - result = verification_session.dispatch("pi", event, normalized_payload) - except (OSError, ValueError, TypeError, KeyError) as error: - result = verification_session.failure(event, normalized_payload, error) - print(json.dumps(pi_result(event, result), ensure_ascii=True, separators=(",", ":"))) - return 0 - - -def audit(kind: str, payload: dict) -> int: - context = normalized(payload) - entry = audit_log.record(kind, context, "pi") - cwd = context.get("cwd") or "." - audit_log.append_record(entry, cwd=Path(cwd)) - return 0 - - -def lint(payload: dict) -> int: - """Report shared quality diagnostics using the post-tool JSON contract.""" - lint_format.emit(lint_format.diagnostics(normalized(payload))) - return 0 - - -def harness(payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - candidates = [values.get("file_path") or values.get("path") or ""] - candidates += patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))) - for name in candidates: - if not name: - continue - absolute = name if name.startswith(("/", "~/")) else str(Path.cwd() / name) - if status := guard_file.check( - "harness", - normalized(payload) | {"tool_name": "apply_patch", "tool_input": {"file_path": absolute}}, - shared_directory(), - ): - return status - return 0 - - -def check_paths(mode: str, payload: dict) -> int: - policy = Path( - os.environ.get("AGENT_SECRET_PATH_POLICY", str(shared_directory() / "rules/secret_path_policy.json")) - ) - rules = secret_path_policy.load(policy) - values = normalized(payload).get("tool_input") or {} - command = values.get("command") or values.get("cmd") or "" - if mode == "command": - try: - commands = shell_syntax.parse(command) - except shell_syntax.UnsupportedSyntax as error: - raise ValueError(f"{error}\n\nCommand: {shell_syntax.command_preview(command)}") from error - candidates = [word for item in commands for word in (*item.arguments, *item.redirections)] - candidates += [word.partition("=")[2] for word in candidates if "=" in word] - else: - candidates = [ - values.get("file_path") or values.get("path") or "", - *patch_input.paths(hook_input.patch_body(values, normalized(payload).get("tool_name"))), - ] - context = normalized(payload) - for value in candidates: - if rule := secret_path_policy.blocked_rule(value, rules): - audit_log.blocked( - "Bash" if mode == "command" else "apply_patch", - command if mode == "command" else value, - f"{rule['reason']}: {value}", - "adapt_guard_secret_paths.sh", - payload.get("session_id") or "", - payload=context, - targets=() if mode == "command" else (value,), - ) - raise ValueError( - f"secret path policy matched.\n\nPath: {value}\nPattern: {rule['pattern']}\n\nWhy:\n {rule['reason']}" - ) - return 0 - - -def content(mode: str, payload: dict) -> int: - if mode == "apply-patch": - values = normalized(payload).get("tool_input") or {} - body = hook_input.patch_body(values, normalized(payload).get("tool_name") or "apply_patch") - return guard_file.check( - "write", - normalized(payload) | {"tool_name": "Edit", "tool_input": {"content": patch_input.added_text(body)}}, - shared_directory(), - ) - return guard_file.check(mode, normalized(payload), shared_directory()) - - -def dispatch(kind: str, arguments: list[str], payload: dict) -> int: - values = normalized(payload).get("tool_input") or {} - if kind == "native": - return native(arguments[0], payload) - if kind == "audit": - return audit(arguments[0], payload) - if kind == "dispatch": - return hook_dispatch.run(manifest_path(), arguments[0], payload) - if kind == "shell": - command = normalized(payload) | { - "tool_name": "Bash", - "tool_input": {"command": values.get("command") or values.get("cmd") or ""}, - } - if arguments[0] in {"allowed", "forbidden"}: - return guard_command.check(arguments[0], command, shared_directory()) - if arguments[0] == "git": - return guard_git.check(command) - return guard_file.check("commit", command, shared_directory()) - if kind == "lint": - return lint(payload) - if kind == "harness": - return harness(payload) - if kind == "paths": - return check_paths(arguments[0], payload) - return content(arguments[0], payload) - - -def main(arguments: list[str]) -> int: - kind = arguments[0] if arguments else "" - rest = arguments[1:] - valid = ( - (kind == "native" and len(rest) == 1 and rest[0] in NATIVE_EVENTS) - or (kind == "audit" and len(rest) == 1 and rest[0] in {"tool", "compaction"}) - or (kind == "dispatch" and len(rest) == 1) - or (kind in {"lint", "harness"} and not rest) - or (kind == "shell" and len(rest) == 1 and rest[0] in {"allowed", "forbidden", "git", "commit"}) - or (kind == "paths" and len(rest) == 1 and rest[0] in {"command", "patch"}) - or (kind == "content" and len(rest) == 1 and rest[0] in {"prompt", "read", "write", "apply-patch"}) +import provider_adapter + +ADAPTER = provider_adapter.Adapter( + provider_adapter.Profile( + name="pi", + tools={ + "bash": "Bash", + "read": "Read", + "write": "Write", + "edit": "Edit", + "multi_edit": "MultiEdit", + "notebook_edit": "NotebookEdit", + "apply_patch": "apply_patch", + "webfetch": "WebFetch", + "web_search": "WebSearch", + }, + events={ + "session-start": "session_start", + "pre-tool-use": "tool_call", + "session-end": "session_shutdown", + }, + compaction_events={"session_before_compact": "PreCompact", "session_compact": "PostCompaction"}, + manifest_env="AGENT_HARNESS_PI_MANIFEST", + manifest_default=".pi/agent/hooks.json", ) - if not valid or any(value in {"-h", "--help"} for value in rest): - print( - "Usage: hook_adapter.py |shell allowed/forbidden/git/commit|lint|harness" - "|paths command/patch|content prompt/read/write/apply-patch>", - file=sys.stderr, - ) - return 1 - try: - payload = json.load(sys.stdin) - if not isinstance(payload, dict): - payload = {} - cwd = payload.get("cwd") - if isinstance(cwd, str) and cwd: - os.chdir(cwd) - return dispatch(kind, rest, payload) - except (OSError, ValueError, TypeError, AttributeError) as error: - print(f"BLOCKED: {error}", file=sys.stderr) - return 2 +) if __name__ == "__main__": - sys.exit(main(sys.argv[1:])) + sys.exit(ADAPTER.main(sys.argv[1:])) diff --git a/agents/skill_rendering.json b/agents/skill_rendering.json index 28099766..d77bf315 100644 --- a/agents/skill_rendering.json +++ b/agents/skill_rendering.json @@ -26,18 +26,6 @@ "allow_implicit_invocation": false } } - }, - "project-setup": { - "claude": { - "frontmatter": { - "disable-model-invocation": true - } - }, - "codex": { - "openai": { - "allow_implicit_invocation": false - } - } } } } diff --git a/agents/skills/adr/SKILL.md b/agents/skills/adr/SKILL.md index ca4dd7c1..f80f4a31 100644 --- a/agents/skills/adr/SKILL.md +++ b/agents/skills/adr/SKILL.md @@ -1,8 +1,10 @@ --- name: adr description: > - Use to decide whether a broad design decision needs an Architecture Decision Record (ADR), - or to create, review, update, supersede, or deprecate ADRs. + Use to create, review, update, supersede, or deprecate ADRs under docs/adr/, or when a change + selects a library, framework, database, protocol, hosting, or tool among real alternatives; + picks an architectural pattern; sets a durable constraint not obvious from code; or reverses + a previous decision. Not for local implementation details understandable from the code. --- # Architecture Decision Records @@ -13,22 +15,9 @@ Use this together with `tsdd` when the work involves both executable behavior an ## Core Rule -When a change introduces or reverses a broad project decision, create or update an ADR under `docs/adr/` as part of the same task. - -When Decision Test identifies a broad decision requiring an ADR, record its rationale before -finishing the work rather than leaving it as an optional follow-up. - -## When To Load - -Load this skill before making or reviewing changes involving: - -- Architecture Decision Records, decision records, ADRs, or `docs/adr/`. -- Library, framework, database, protocol, hosting, or tooling selection where real alternatives exist. -- Architectural patterns such as server-rendered vs SPA, sessions vs tokens, monolith vs services, sync vs async, polling vs events, or generated vs handwritten artifacts. -- Durable constraints that are not obvious from code, such as "no runtime dependencies", "Nix owns this configuration", or "requirement intent remains independent of executable test evidence". -- Reversing, superseding, deprecating, or intentionally rejecting a previous decision. - -Do not load it for a local implementation detail that can be understood from the code in under a minute. +When a change introduces or reverses a broad project decision that the Decision Test identifies, +create or update an ADR under `docs/adr/` in the same task, before finishing the work rather than +as an optional follow-up. ## Decision Test @@ -155,23 +144,8 @@ Use the longer template only when a Y-Statement would be too compressed: ## Workflow -For decision-relevant implementation tasks: - -1. Load `tsdd` and any language-specific skill required by the code. -2. Establish the executable behavior with tests when the change has behavior. -3. Inspect existing ADRs before choosing or recording the decision. -4. Implement the code change. -5. Create or supersede the ADR before finishing. -6. Confirm the relevant verification results. Follow `tsdd`'s Automatic Verification policy for execution needs and reuse of successful hook evidence. -7. Report the ADR path and the verification result. - -For documentation-only decision tasks: - -1. Inspect existing ADRs. -2. Decide whether this is a new decision, a duplicate, or a supersession. -3. Create or update only the necessary ADR files. -4. Confirm the repository's markdown or generated-artifact check results when available. Follow `tsdd`'s Automatic Verification policy for execution needs and reuse of successful hook evidence. -5. Report the ADR path. +After [Repository Inspection](#repository-inspection), create or supersede only the necessary ADR +files before finishing, then report the ADR path. ## Non-Goals diff --git a/agents/skills/bash-style/SKILL.md b/agents/skills/bash-style/SKILL.md index c0ebe3d4..870c889d 100644 --- a/agents/skills/bash-style/SKILL.md +++ b/agents/skills/bash-style/SKILL.md @@ -38,11 +38,11 @@ set -x ## usage Function -Every script must define a `usage` function that prints documentation to stderr and exits with failure. Use heredoc + redirect inside the function body: +Every script must define a `usage` function that prints documentation. Use a heredoc inside the function body: ```sh function usage() { - cat <&2 + cat <&2` and `exit 1`. ## Constants Declare constants with `readonly`. Names use `SCREAMING_SNAKE_CASE`: ```sh -readonly INPUT_DIR="../data/input" +readonly INPUT_DIR="$SCRIPT_DIR/../data/input" readonly MAX_RETRY=3 ``` @@ -88,8 +88,6 @@ Always quote the right-hand side — values may contain spaces or special charac | Variables | `snake_case` | `input_file="..."` | | Functions | `snake_case` | `function parse_args()` | | Local variables | `_snake_case` (leading underscore) | `local _tmp_dir` | -| Files | `snake_case` | `lint_format.sh` | -| Directories | `kebab-case` | `claude-scripts/` | Do not start names with a digit. diff --git a/agents/skills/bash-style/references/bats_testing.md b/agents/skills/bash-style/references/bats_testing.md index 0f0a2dab..9a13d97d 100644 --- a/agents/skills/bash-style/references/bats_testing.md +++ b/agents/skills/bash-style/references/bats_testing.md @@ -1,6 +1,6 @@ # Testing with bats -Test shell scripts with [bats](https://github.com/bats-core/bats-core) (Bash Automated Testing System). Available tools: `bats`, `shellcheck`, `shfmt`. +Test shell scripts with [bats](https://github.com/bats-core/bats-core) (Bash Automated Testing System). ### Directory Layout @@ -37,7 +37,7 @@ setup() { @test "descriptive lowercase sentence" { run bash "$SCRIPT" --help - [ "$status" -eq 1 ] + [ "$status" -eq 0 ] [[ "$output" == *"Usage:"* ]] } ``` diff --git a/agents/skills/bash-style/references/script_template.md b/agents/skills/bash-style/references/script_template.md index 86024ad4..97a1b0f7 100644 --- a/agents/skills/bash-style/references/script_template.md +++ b/agents/skills/bash-style/references/script_template.md @@ -6,7 +6,7 @@ set -euCo pipefail # Resolve the working directory according to the task; see the parent skill. function usage() { - cat <&2 + cat <&2 + exit 1 + ;; + esac + local _input="$1" echo "processing: $_input" diff --git a/agents/skills/domain-modeling/SKILL.md b/agents/skills/domain-modeling/SKILL.md index 59cac1e5..0afcaaa5 100644 --- a/agents/skills/domain-modeling/SKILL.md +++ b/agents/skills/domain-modeling/SKILL.md @@ -52,7 +52,8 @@ Use `source-of-truth` when durable placement, duplication, or conflicting author judgment. Reuse existing authoritative material; do not automatically create `CONTEXT.md` or a context map when the first term is clarified. -When a glossary is warranted, follow its existing format. Keep definitions concise but sufficient +Add a short glossary only when code cannot communicate the distinctions to every relevant +audience. When a glossary is warranted, follow its existing format. Keep definitions concise but sufficient to distinguish concepts, identify the applicable context, and note easily confused terms where useful. Include domain-specific meanings, not a general programming tutorial. Do not copy implementation details or behavioral specifications into the glossary. diff --git a/agents/skills/explain-visually/scripts/verify_page.py b/agents/skills/explain-visually/scripts/verify_page.py index 4bcb8a42..5ed97167 100644 --- a/agents/skills/explain-visually/scripts/verify_page.py +++ b/agents/skills/explain-visually/scripts/verify_page.py @@ -34,6 +34,11 @@ r"(?:\bfrom\s+|\bimport\s*\(\s*|\bimport\s+|\bfetch\s*\(\s*)[\"']([^\"']+)[\"']", re.IGNORECASE, ) +SRCSET_URL_PATTERN = re.compile(r"[\s,]*(\S*[^\s,])(?:,+|\s[^,]*,?|$)") +REFRESH_URL_PATTERN = re.compile( + r"^\s*[\d.]*\s*[;,]?\s*(?:url\s*=\s*)?([\"']?)(.*?)\1\s*$", + re.IGNORECASE | re.DOTALL, +) BROWSER_COMMANDS = ( "google-chrome", "google-chrome-stable", @@ -49,14 +54,23 @@ RESOURCE_ATTRIBUTES = { "audio": ("src",), "base": ("href",), + "embed": ("src",), "iframe": ("src",), + "image": ("href", "xlink:href"), "img": ("src", "srcset"), "link": ("href",), "object": ("data",), "script": ("src",), "source": ("src", "srcset"), + "track": ("src",), + "use": ("href", "xlink:href"), "video": ("poster", "src"), } +# Tags that load a resource only when a qualifying attribute has the given value. +CONDITIONAL_RESOURCE_ATTRIBUTES = { + "input": ("type", "image", "src"), + "meta": ("http-equiv", "refresh", "content"), +} @dataclass(frozen=True, slots=True) @@ -103,9 +117,9 @@ def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None content = attributes.get("content") if name is not None and content is not None: self.metadata[name] = content.strip() - for attribute in RESOURCE_ATTRIBUTES.get(tag, ()): + for attribute in _resource_attributes(tag, attributes): value = attributes.get(attribute) - if value is not None and (attribute == "srcset" or _loads_external_resource(value)): + if value is not None and any(map(_is_external_reference, _referenced_urls(attribute, value))): self.external_resources.append(f"{tag}[{attribute}]={value}") style = attributes.get("style") if style is not None: @@ -193,8 +207,22 @@ def viewport_height(dom: str) -> tuple[int, str | None]: return bounded_height, None -def _loads_external_resource(value: str) -> bool: - return _is_external_reference(value) +def _resource_attributes(tag: str, attributes: dict[str, str | None]) -> tuple[str, ...]: + condition = CONDITIONAL_RESOURCE_ATTRIBUTES.get(tag) + if condition is None: + return RESOURCE_ATTRIBUTES.get(tag, ()) + qualifier, expected_value, resource_attribute = condition + is_loading = (attributes.get(qualifier) or "").strip().lower() == expected_value + return (resource_attribute,) if is_loading else () + + +def _referenced_urls(attribute: str, value: str) -> tuple[str, ...]: + if attribute == "srcset": + return tuple(SRCSET_URL_PATTERN.findall(value)) + if attribute == "content": + match = REFRESH_URL_PATTERN.match(value) + return (match.group(2),) if match is not None else (value,) + return (value,) def _external_css_resources(styles: Iterable[str]) -> tuple[str, ...]: diff --git a/agents/skills/git-workflow/SKILL.md b/agents/skills/git-workflow/SKILL.md index 6658fe93..a885adc2 100644 --- a/agents/skills/git-workflow/SKILL.md +++ b/agents/skills/git-workflow/SKILL.md @@ -19,9 +19,8 @@ number and plan URL from issue-workflow and associate them with the branch or PR - Inspect Git state before edits: current branch, `git status --porcelain=v1`, and recent commit style when commit messages will be written. - Never overwrite, reset, clean, or discard user changes unless the user explicitly asked for that exact destructive action. - Do not push directly to the default / protected branch, and rewrite published history only with the leased force push described in [Delivery](references/delivery.md). Ordinary implementation and PR-creation requests do not authorize merging; merge only when explicitly requested and permitted by repository and runtime rules. -- Complete requested implementation, relevant verification, and fixes for problems caused by the change. Do not stop at the first implementation or first Green when required work remains. - Commit, push, and create pull requests only within the user's request or an explicit standing authorization. Permission to execute a command is not authorization to perform that action for the task. Do not repeat confirmation for steps already covered by the authorized workflow; runtime approval requirements still apply. -- When commits are authorized, group them by reviewable intent. Multiple TSDD cycles may belong to one commit; do not force a commit for every cycle. Both TSDD paths end Green. +- When commits are authorized, group them by reviewable intent. Multiple TSDD cycles may belong to one commit; do not force a commit for every cycle. - Keep branch names and commit subjects aligned with the primary intent of the change, not with filenames. Review-only requests authorize inspection and findings, not edits or delivery. A request to fix a bug diff --git a/agents/skills/git-workflow/references/delivery.md b/agents/skills/git-workflow/references/delivery.md index fbe02319..d9799d99 100644 --- a/agents/skills/git-workflow/references/delivery.md +++ b/agents/skills/git-workflow/references/delivery.md @@ -10,7 +10,7 @@ For implementation tasks: - `git fetch origin` - optionally `git rebase origin/` before the first push when the feature branch should be refreshed onto the latest base - `git push -u origin ` - - `gh pr create -f --base ` + - `gh pr create --base --title "" --body-file <file>`, with a [pull request body](#pull-request-body) 6. Stop before merge; merging is a human decision unless the user explicitly asks for it. When the user asks, `gh pr merge` prompts for approval, and that prompt is the authorization step. For explicit PR requests: @@ -18,7 +18,8 @@ For explicit PR requests: 1. If the user asked for a stacked or red-green pull request, follow [Stacked pull requests](stacked_prs.md) instead of steps 2 and 3. 2. Confirm the target base branch if it is not obvious from the repo default. 3. Follow [Local Verification Before Delivery](#local-verification-before-delivery), then push the feature branch with `git push -u origin <branch>`. -4. Create a normal PR with `gh pr create -f --base <base>`. +4. Create a normal PR with `gh pr create --base <base> --title "<title>" --body-file <file>`, with a + [pull request body](#pull-request-body). For rebase and force-push: @@ -27,6 +28,20 @@ For rebase and force-push: - Interactive rebase is outside the default workflow. - After rebasing a branch this task already pushed, publish it with `git push --force-with-lease --force-if-includes origin <branch>`. The lease refuses to overwrite commits that were pushed elsewhere, and `--force-if-includes` refuses when a background fetch hid them. Never use bare `--force`, `+refspec` pushes, or any force push to the default branch. If the lease is rejected, stop and ask the user. +## Pull Request Body + +Write the title as a Conventional Commits subject stating the primary intent, and keep the body +short. Do not use `-f`/`--fill`; commit messages do not carry this content. The body must include: + +- A few lines on what actually changed. +- `Closes #N` only when the PR fully resolves the Issue; otherwise `Refs #N`. +- A link to the plan comment, when one exists. +- Verification evidence: what ran and its result, or what could not run. +- Material departures from the plan, or none. + +For what the PR body owns relative to the Issue and plan, follow +[Information Placement](../../source-of-truth/SKILL.md#information-placement). + ## Local Verification Before Delivery 1. Complete required local verification before delivery without waiting for Stop. diff --git a/agents/skills/git-workflow/references/stacked_prs.md b/agents/skills/git-workflow/references/stacked_prs.md index 425da34e..07f44eba 100644 --- a/agents/skills/git-workflow/references/stacked_prs.md +++ b/agents/skills/git-workflow/references/stacked_prs.md @@ -29,7 +29,8 @@ before publishing, as `tsdd` requires for Red. 4. `gh stack add fix/<subject>` creates layer 2 on top of layer 1 and checks it out. 5. Remove the marker and its comment, implement the fix, and commit as `fix(<scope>): ...`. 6. `gh stack submit --auto` pushes both branches and opens two linked draft pull requests. Set - each title and body with `gh pr edit`, then mark them ready with `gh pr ready`. + each title and body with `gh pr edit` following [Pull request body](delivery.md#pull-request-body), + then mark them ready with `gh pr ready`. 7. Report the stack with `gh stack view --short`. Rules: diff --git a/agents/skills/github-actions-style/SKILL.md b/agents/skills/github-actions-style/SKILL.md index 71d8614e..185a917a 100644 --- a/agents/skills/github-actions-style/SKILL.md +++ b/agents/skills/github-actions-style/SKILL.md @@ -7,7 +7,7 @@ description: > # GitHub Actions Coding Conventions -Claude already knows the basic structure of GitHub Actions (events, workflows, jobs, steps). Focus on the following patterns that are easy to overlook but important for security and reliability. +These patterns are easy to overlook but important for security and reliability. ## 1. Permissions — Whitelist Approach @@ -27,28 +27,27 @@ Common permission keys: `contents`, `issues`, `pull-requests`, `packages`, `id-t ## 2. Version Pinning -Tags like `@v4` can be moved or deleted by the action author — the same tag may point to different code over time. Pin to an exact version tag or commit SHA. +Tags such as `@v4` or `@v4.2.1` can be moved or deleted by the action author, so the same tag may +point to different code over time. Pin every action to a full commit SHA and record the version in +a comment for readability and update tooling. ```yaml # Avoid — tag can move - uses: actions/checkout@v4 -# Recommended — exact version tag -- uses: actions/checkout@v4.2.1 - -# High security — immutable SHA (add the tag as a comment for readability) +# Recommended — immutable SHA with the version as a comment - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.1 ``` ## 3. Script Injection Prevention -GitHub context values like PR titles, issue bodies, and branch names come from untrusted external sources. Interpolating them directly into `run:` allows an attacker to inject arbitrary shell commands. Always pass them through `env:` instead. +GitHub context values like PR titles, issue bodies, and branch names come from untrusted external sources. Interpolating them directly into `run:` allows an attacker to inject arbitrary shell commands. Always pass them through `env:` instead, and quote the variable in the script. This is not escaping: the value never becomes part of the script text. ```yaml # Dangerous — PR title injected directly into shell - run: echo "${{ github.event.pull_request.title }}" -# Safe — value passed as environment variable (shell-escaped automatically) +# Safe — the shell reads the value from a variable; it is never interpolated into the script - env: TITLE: ${{ github.event.pull_request.title }} run: echo "$TITLE" diff --git a/agents/skills/nix-dotfiles/SKILL.md b/agents/skills/nix-dotfiles/SKILL.md index 4af304c0..d0be0027 100644 --- a/agents/skills/nix-dotfiles/SKILL.md +++ b/agents/skills/nix-dotfiles/SKILL.md @@ -130,36 +130,11 @@ and Atuin modules disable generated Zsh integration for this reason. ## Dotfile Symlinks -### Editable symlinks (mkOutOfStoreSymlink) - -```nix -let - link = path: config.lib.file.mkOutOfStoreSymlink "${dotfilesDir}/${path}"; -in -{ - home.file = { - ".zshrc".source = link "zsh/.zshrc"; - ".config/nvim".source = link "nvim"; # directory symlink - # ... etc - }; -} -``` - -Read `home.file` and `xdg.configFile` in `nix/home/default.nix` for each target's source. Entries -using `link` point at editable working-tree files. Agent deployment follows the managed-source -lifecycle above. - -Use `mkOutOfStoreSymlink` for files edited frequently — changes apply immediately without rebuild. - -### Nix-generated files - -```nix -home.file.".config/zsh/nix-plugins.zsh".text = '' - source ${pkgs.zsh-autosuggestions}/share/zsh-autosuggestions/zsh-autosuggestions.zsh -''; -``` - -Use `.text` when the file content depends on Nix store paths (e.g. plugin source paths that change on every nixpkgs update). +Read `home.file` and `xdg.configFile` in `nix/home/default.nix` for each target's source. +Entries using the `link` helper (`mkOutOfStoreSymlink`) point at editable working-tree files; +use it for frequently edited files, since changes apply without a rebuild. Use a generated +`.text` file only when its content depends on Nix store paths. Agent deployment follows the +managed-source lifecycle above. ## Activation Hooks @@ -189,37 +164,6 @@ that require the primary user's context. Preserve those execution boundaries whe ## Homebrew (via nix-homebrew) -```nix -nix-homebrew = { - enable = true; - user = username; - autoMigrate = true; -}; - -homebrew = { - enable = true; - onActivation = { - autoUpdate = true; - upgrade = true; - cleanup = "uninstall"; # remove unlisted casks/brews on rebuild - }; - casks = [ ... ]; # GUI apps - taps = [ ... ]; # third-party repos - brews = [ ... ]; # formulae not in nixpkgs -}; -``` - -`cleanup = "uninstall"` means any cask or brew removed from the list will be uninstalled on the next `darwin-rebuild switch`. This keeps the machine declarative. - -## Key Patterns - -| Pattern | Usage | -| --------------------------------------------- | ----------------------------------------------------------------- | -| `with pkgs;` | Avoids repeating `pkgs.` in `home.packages` list | -| `unstable.xxx` | Package from `nixpkgs-unstable` (passed via `extraSpecialArgs`) | -| `input.packages.${system}.default` | Package from a third-party flake input | -| `link "path"` | Helper for `mkOutOfStoreSymlink` (defined in `let` block) | -| `${pkgs.xxx}` in `.text` | Embeds Nix store paths into generated files | -| `enableZshIntegration = false` | Keep shell initialization in the linked `.zshrc` where configured | -| `\|\| true` in activation | Prevents non-fatal errors from aborting rebuild | -| `homebrew.onActivation.cleanup = "uninstall"` | Declarative cask management | +Read `nix-homebrew` and `homebrew` in `nix/darwin/default.nix` for the current settings and +lists. `onActivation.cleanup = "uninstall"` uninstalls any cask or brew removed from those lists +on the next `darwin-rebuild switch`, so remove an app by deleting its entry. diff --git a/agents/skills/project-setup/SKILL.md b/agents/skills/project-setup/SKILL.md index b0fc10de..f4bad797 100644 --- a/agents/skills/project-setup/SKILL.md +++ b/agents/skills/project-setup/SKILL.md @@ -1,8 +1,10 @@ --- name: project-setup description: > - Set up per-project Nix development environments and create or configure GitHub repositories - using this user's templates and repo commands. For requested setup, not routine code edits. + Use only when the user requests creating a GitHub repository, applying standard GitHub + settings to one, or setting up a per-project Nix devShell or development environment, using + this user's templates and repo commands. Not for ordinary code edits or other work in + existing projects. --- # Project Setup diff --git a/agents/skills/python-style/references/code_style.md b/agents/skills/python-style/references/code_style.md index 6ca5d44a..d653a6f6 100644 --- a/agents/skills/python-style/references/code_style.md +++ b/agents/skills/python-style/references/code_style.md @@ -7,19 +7,9 @@ - Don't use `global`, `nonlocal` (not explicit enough) - Use built-in generics (e.g., `tuple`, `list`, `dict`) instead of `typing.Tuple`, `typing.List`, `typing.Dict` -## Strings +## Formatting -### Quote Usage - -- String contains `'` → use `"` -- String contains `"` → use `'` -- f-strings (variable substitution) → use `"` -- `raise` statements → use `"` (normal sentences use `'`) - -### Operators - -- Separate operators and operands by one space -- When using 2+ operators, omit spaces around `*`, `/`, `//`, `%`, `**` (higher precedence) +Let `ruff format` decide quotes and operator spacing; do not hand-format against it. ## Logging @@ -108,17 +98,17 @@ class MyClass: # Function level -def function_name(arg1, arg2): +def function_name(arg1: int, arg2: str) -> bool: """Function functionality summary. (Detailed function functionality.) Args: - arg1 (type): Argument description - arg2 (type): Argument description + arg1: Argument description + arg2: Argument description Returns/Yields: - type: Return value description + Return value description Raises: ErrorType: Error description diff --git a/agents/skills/python-style/references/tooling.md b/agents/skills/python-style/references/tooling.md index 750a6ac7..0a9a40ea 100644 --- a/agents/skills/python-style/references/tooling.md +++ b/agents/skills/python-style/references/tooling.md @@ -4,7 +4,7 @@ - Use only `uv` for package management, don't use `pip` - Install dependencies using `uv sync` -- Run tools using `uv run {tool}` +- Run verification tools using `uv run --frozen {tool}` so checks never rewrite `uv.lock` - Baseline tooling dependencies belong in template-defined dependency groups, not ad hoc commands - Add project-specific dependencies using `uv add {package}` - Add project-specific tool dependencies using `uv add --group <group> {package}` @@ -28,7 +28,7 @@ ## Static Type Checking - ty should already be provided by the project's `typecheck` dependency group -- Run type checks using `uv run ty check` -- Check specific paths using `uv run ty check src tests` +- Run type checks using `uv run --frozen ty check` +- Check specific paths using `uv run --frozen ty check src tests` - Use `uv run ty server` only for editor or language-server integration - Do not introduce another type checker unless the existing project is already standardized on it diff --git a/agents/skills/rust-style/references/code_style.md b/agents/skills/rust-style/references/code_style.md index e4007291..f297f260 100644 --- a/agents/skills/rust-style/references/code_style.md +++ b/agents/skills/rust-style/references/code_style.md @@ -10,13 +10,6 @@ ## Naming Conventions -- Modules and files: `snake_case`. -- Functions, methods, and variables: `snake_case`. -- Types, traits, and enum variants: `UpperCamelCase`. -- Constants and statics: `SCREAMING_SNAKE_CASE`. -- Generic type parameters: short `UpperCamelCase` names such as `T`, `E`, `P`. -- Lifetimes: short lowercase names such as `'a`; use descriptive names only when they clarify an unusual relationship. -- Cargo feature names: `kebab-case`. - Avoid `manager`, `helper`, `util`, and `common` unless the name is already established locally. ### Module Naming @@ -36,7 +29,6 @@ - Avoid `as` aliases unless they remove ambiguity or follow a local convention. - Prefer `crate::` for crate-local imports and `super::` for parent-module test imports. - Keep trait imports close to the code that needs method resolution. -- Do not use glob imports in production modules except for explicit prelude modules. ## Whitespace and Line Breaks diff --git a/agents/skills/rust-style/references/tooling.md b/agents/skills/rust-style/references/tooling.md index 0ca6e92e..c055326f 100644 --- a/agents/skills/rust-style/references/tooling.md +++ b/agents/skills/rust-style/references/tooling.md @@ -43,7 +43,6 @@ tests/ - Let `rustfmt` define formatting. Do not hand-format around rustfmt. - Keep files focused and cohesive. -- Prefer modules of roughly 200-500 lines. Split when multiple responsibilities appear. - Keep public items before private helpers when it improves scanning. - Use `mod.rs` only when the existing project already uses that style; otherwise prefer `module_name.rs` plus `module_name/child.rs`. - Put one top-level concept per file when the concept has real behavior. diff --git a/agents/skills/rust-style/references/types.md b/agents/skills/rust-style/references/types.md index 2799b953..e3825a12 100644 --- a/agents/skills/rust-style/references/types.md +++ b/agents/skills/rust-style/references/types.md @@ -24,11 +24,17 @@ Value Object pattern: #[derive(Debug, Clone, PartialEq, Eq)] pub struct SkillName(String); +#[derive(Debug, thiserror::Error)] +pub enum SkillNameError { + #[error("skill name must not be empty")] + Empty, +} + impl SkillName { - pub fn parse(value: impl Into<String>) -> anyhow::Result<Self> { + pub fn parse(value: impl Into<String>) -> Result<Self, SkillNameError> { let value = value.into(); if value.is_empty() { - anyhow::bail!("skill name must not be empty"); + return Err(SkillNameError::Empty); } Ok(Self(value)) } diff --git a/agents/skills/source-of-truth/SKILL.md b/agents/skills/source-of-truth/SKILL.md index d4fb64e3..a088e019 100644 --- a/agents/skills/source-of-truth/SKILL.md +++ b/agents/skills/source-of-truth/SKILL.md @@ -48,11 +48,14 @@ rationale, API reference, user guidance, migration guidance, or an operational r ## Information Placement -- **Issue body** → this task's purpose, scope, acceptance criteria, and requirement sources. -- **Plan comment** → this task's implementation approach, major steps, and important assumptions. - A plan is not the authority for requirements; existing formal specifications and user - requirements retain their authority under this skill's conflict policy. -- **PR body** → actual changes, verification results, and material departures from the plan. +- **Issue body** → this task's requirements; its contents follow + [Issue Resolution](../issue-workflow/SKILL.md#issue-resolution). +- **Plan comment** → this task's implementation approach; its contents follow + [Plan Comments](../issue-workflow/SKILL.md#plan-comments). A plan is not the authority for + requirements; existing formal specifications and user requirements retain their authority under + this skill's conflict policy. +- **PR body** → actual changes, verification results, and material departures from the plan; + its format follows [Pull Request Body](../git-workflow/references/delivery.md#pull-request-body). Link these artifacts to each other instead of manually maintaining the same details in all three. - **Requirement purpose, source, scope, or accepted risk** → user request, issue or reproduced defect, public interface, external standard, product record, or behavior intentionally preserved @@ -79,8 +82,8 @@ rationale, API reference, user guidance, migration guidance, or an operational r - **Agent navigation** → the repository-defined entry map described under Agent Entry Maps. Domain vocabulary is cross-cutting rather than a separate document by default. Within a bounded -context, use the same terms in code, types, tests, diagrams, and prose. Add a short glossary only -when code cannot communicate the distinctions to every relevant audience. +context, use the same terms in code, types, tests, diagrams, and prose. Glossary rules belong to +[domain-modeling](../domain-modeling/SKILL.md#preserve-useful-knowledge). ## Duplication Test diff --git a/agents/skills/tsdd/SKILL.md b/agents/skills/tsdd/SKILL.md index 3fc8cf80..ecd5af9b 100644 --- a/agents/skills/tsdd/SKILL.md +++ b/agents/skills/tsdd/SKILL.md @@ -95,7 +95,8 @@ merely because there is more than one assertion or example row. unclear names, mixed responsibilities, and unnecessary coupling. Refactor when a concrete improvement is warranted, then re-run the relevant gate. Do not manufacture edits or speculative abstractions merely to perform a Refactor step. -7. **Finish in a coherent verified state.** VCS recording and delivery are governed by the +7. **Finish in a coherent verified state.** Continue with the next slice while required work + remains; the first Green does not end the task. VCS recording and delivery are governed by the `git-workflow` skill. ### Work without a contract gap @@ -174,31 +175,7 @@ Direct execution of narrow tests for intermediate decisions is allowed. Report e and known coverage limits. Concrete CLI syntax and delivery ordering belong to [Git Delivery](../git-workflow/references/delivery.md#local-verification-before-delivery). -## Type-Driven Design - -Types carry compile-time invariants and constraints. They are not a second requirements source; -they make invalid states unrepresentable. - -### Patterns - -- **Value Objects** wrap primitives with domain constraints. Construct through a validating - boundary and keep fields immutable. -- **Branded or newtypes** prevent mixing structurally identical but semantically distinct values. -- **Result or Either types** force callers to handle expected success and failure paths. -- **Exhaustive sum types** make unhandled state-machine cases visible to the compiler. -- **Parse, do not validate repeatedly.** Turn raw boundary input into a constrained type once. - -Tests still cover behavior that types cannot express, including business rules, ordering, side -effects, integration, concurrency, and external-system interactions. - ## Enforcement Mechanisms -This section applies only when the user asks to establish or revise TSDD's development workflow. -Ordinary feature work uses existing mechanisms; it does not require creating hooks, wrappers, or PR -templates. Consider the following mechanisms to make the intended path observable: - -- A workflow or wrapper that supports selecting Red or Green-baseline paths without manufacturing - Red. Determining whether a real contract gap exists still requires judgment. -- Hooks and CI gates for tests, type checks, lint, formatting, and risk-specific evaluation. -- A PR template that records the requirement source, behavior slice, Red or baseline evidence, - final Green evidence, review needs, and ADR impact. +Only when the user asks to establish or revise TSDD's development workflow, read +[Enforcement mechanisms](references/enforcement.md). diff --git a/agents/skills/tsdd/references/enforcement.md b/agents/skills/tsdd/references/enforcement.md new file mode 100644 index 00000000..d9f261b8 --- /dev/null +++ b/agents/skills/tsdd/references/enforcement.md @@ -0,0 +1,11 @@ +# Enforcement Mechanisms + +This reference applies only when the user asks to establish or revise TSDD's development workflow. +Ordinary feature work uses existing mechanisms; it does not require creating hooks, wrappers, or PR +templates. Consider the following mechanisms to make the intended path observable: + +- A workflow or wrapper that supports selecting Red or Green-baseline paths without manufacturing + Red. Determining whether a real contract gap exists still requires judgment. +- Hooks and CI gates for tests, type checks, lint, formatting, and risk-specific evaluation. +- A PR template that records the requirement source, behavior slice, Red or baseline evidence, + final Green evidence, review needs, and ADR impact. diff --git a/flake.lock b/flake.lock index cf05ff07..0cf58552 100644 --- a/flake.lock +++ b/flake.lock @@ -10,11 +10,11 @@ ] }, "locked": { - "lastModified": 1789749985, - "narHash": "sha256-KbCFtPbooIHdevHWPRG+vQ8ChTCsBjiIPnM9pagJUfM=", + "lastModified": 1790580263, + "narHash": "sha256-n5/aBgN7YdnLRx7bcgW+HXLxKyN5EblBA8fFNVNTA68=", "owner": "furedea", "repo": "agent-harness", - "rev": "67fc439e817368d365338d879091d314f0467ced", + "rev": "9d9a8a31257487aa2cd474812c13ae821cc98a63", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index bc2163cb..922f82c5 100644 --- a/flake.nix +++ b/flake.nix @@ -217,9 +217,10 @@ in { default = shellPkgs.mkShell { - packages = with shellPkgs; [ - commitlint - lefthook + packages = [ + shellPkgs.commitlint + # lefthook before 2.1.6 runs hooks under a pty, which agent sandboxes deny. + nixpkgs-unstable.legacyPackages.${sys}.lefthook ]; }; } diff --git a/nix/checks.nix b/nix/checks.nix index aff5b0f1..de2faa84 100644 --- a/nix/checks.nix +++ b/nix/checks.nix @@ -110,6 +110,16 @@ in "devin-cli" "codex" ]; + no-unguarded-agent-packages = !(builtins.elem "opencode" homeNames); + # lefthook before 2.1.6 runs hook commands under a pty, which agent sandboxes deny. + sandbox-compatible-git-hooks = + let + lefthookVersions = versions "lefthook" home.home.packages; + in + lefthookVersions != [ ] + && builtins.all (version: lib.versionAtLeast version "2.1.6") lefthookVersions; + commit-message-alias-avoids-removed-codex-flag = + !(lib.hasInfix "--full-auto" home.programs.git.settings.alias.cc); shared-agent-input = inputs ? llm-agents && !(inputs ? nix-claude-code) diff --git a/nix/home/default.nix b/nix/home/default.nix index dc62bdfd..35829cd4 100644 --- a/nix/home/default.nix +++ b/nix/home/default.nix @@ -197,7 +197,7 @@ in autocorrect commitlint dprint - lefthook + unstable.lefthook ls-lint # prettierd: used for markdown because dprint-plugin-markdown hardcodes 2-space list indent. # tabWidth:4 in ~/.prettierrc gives 4-space list nesting to match Obsidian. @@ -217,7 +217,6 @@ in devinCliPackage herdrCompatibleCodex hermesAgentPackage - unstable.opencode unstable.pi-coding-agent # Nix tooling @@ -338,7 +337,7 @@ in receive.fsckObjects = true; status.short = true; status.branch = true; - alias.cc = "!f() { tmpf=$(mktemp) && codex exec --full-auto -o \"$tmpf\" 'Review the staged diff and generate a Conventional Commits message. Output ONLY the commit message, nothing else.' && git commit -F \"$tmpf\"; rm -f \"$tmpf\"; }; f"; + alias.cc = "!f() { tmpf=$(mktemp) && codex exec -s read-only -o \"$tmpf\" 'Review the staged diff and generate a Conventional Commits message. Output ONLY the commit message, nothing else.' && git commit -F \"$tmpf\"; rm -f \"$tmpf\"; }; f"; alias.wtd = "wt -D"; }; }; diff --git a/pyproject.toml b/pyproject.toml index 2571a5b7..7caadbef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,6 +29,7 @@ known_first_party = [ "lib", "lint_format", "patch_input", + "provider_adapter", "secret_path_policy", "shell_syntax", "session_gate", diff --git a/tests/agents/claude/test_settings.py b/tests/agents/claude/test_settings.py index b856f0ee..d5b98acb 100644 --- a/tests/agents/claude/test_settings.py +++ b/tests/agents/claude/test_settings.py @@ -12,3 +12,29 @@ def test_agent_git_commands_run_without_the_sandboxed_fsmonitor_daemon() -> None environment[f"GIT_CONFIG_KEY_{index}"]: environment[f"GIT_CONFIG_VALUE_{index}"] for index in range(count) } assert configured["core.fsmonitor"] == "false" + + +def test_commands_run_sandboxed_by_default() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert sandbox["enabled"] is True + + +def test_verification_and_agent_tools_work_without_escaping_the_sandbox() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert { + "~/.cache/nix", + "~/.cache/uv", + "~/Library/Caches/dprint", + "~/.local/state/agent-harness", + } <= set(sandbox["filesystem"]["allowWrite"]) + assert {"gh *", "herdr *"} <= set(sandbox["excludedCommands"]) + + +def test_excluded_commands_match_invocations_with_arguments() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert [command for command in sandbox["excludedCommands"] if not command.endswith(" *")] == [] + + +def test_sandbox_paths_use_the_home_prefix_the_sandbox_expands() -> None: + sandbox = json.loads((REPO_ROOT / "agents/claude/settings.json").read_text())["sandbox"] + assert [path for path in sandbox["filesystem"]["allowWrite"] if "$" in path] == [] diff --git a/tests/agents/hooks/test_check_selection.py b/tests/agents/hooks/test_check_selection.py index 7fab4048..6b68de97 100644 --- a/tests/agents/hooks/test_check_selection.py +++ b/tests/agents/hooks/test_check_selection.py @@ -56,6 +56,26 @@ def test_javascript_without_a_test_command_is_not_success(tmp_path: Path) -> Non selection.language_plan(tmp_path, rules, ("src/file.ts",)) +def test_sources_mapped_to_another_language_run_only_that_language(tmp_path: Path) -> None: + rules = project(tmp_path, {"src/assets/**": ["src/bridges.rs", "tests/artifacts.rs"]}) + (tmp_path / "package.json").write_text(json.dumps({"devDependencies": {"@openai/codex": "1.0.0"}})) + touch_all(tmp_path, ["Cargo.toml", "src/bridges.rs", "tests/artifacts.rs"]) + commands, errors = selection.language_plan(tmp_path, rules, ("src/assets/pi/hook_bridge.ts",)) + assert not errors + assert [item.arguments for item in commands] == [ + ("cargo", "test", "bridges", "--quiet"), + ("cargo", "test", "--test", "artifacts", "--quiet"), + ] + + +def test_unmapped_sources_beside_mapped_ones_still_require_their_runner(tmp_path: Path) -> None: + rules = project(tmp_path, {"src/assets/**": ["tests/artifacts.rs"]}) + (tmp_path / "package.json").write_text("{}") + touch_all(tmp_path, ["Cargo.toml", "tests/artifacts.rs"]) + with pytest.raises(selection.UnknownTestCommand): + selection.language_plan(tmp_path, rules, ("src/assets/bridge.ts", "src/app.ts")) + + PYTEST = ("uv", "run", "--frozen", "pytest", "--no-header", "-q") diff --git a/tests/agents/hooks/test_hook_boundary.py b/tests/agents/hooks/test_hook_boundary.py index 520e5961..1ed72acb 100644 --- a/tests/agents/hooks/test_hook_boundary.py +++ b/tests/agents/hooks/test_hook_boundary.py @@ -14,16 +14,17 @@ from tests.runtime import REPO_ROOT, load_script_module hook_adapter = load_script_module("agents/codex/hooks/hook_adapter.py", "codex_hook_adapter") +shared = hook_adapter.provider_adapter files = load_script_module("agents/hooks/guard_file.py", "guard_file") audit = load_script_module("agents/hooks/audit_log.py", "audit_log") def test_patch_paths_include_moves_and_deletions_once() -> None: patch = "*** Add File: a.py\n*** Update File: a.py\n*** Delete File: old.py\n*** Move to: new.py" - assert hook_adapter.patch_input.paths(patch) == ("a.py", "new.py", "old.py") + assert shared.patch_input.paths(patch) == ("a.py", "new.py", "old.py") -hook_input = hook_adapter.lint_format.hook_input +hook_input = shared.lint_format.hook_input def test_patch_body_requires_tool_or_format_evidence() -> None: @@ -59,7 +60,7 @@ def test_shell_commands_are_not_patch_targets() -> None: def test_scanner_receives_only_added_lines() -> None: - assert hook_adapter.patch_input.added_text("--- old\n+++ new\n context\n-old\n+new\n+more") == "new\nmore" + assert shared.patch_input.added_text("--- old\n+++ new\n context\n-old\n+new\n+more") == "new\nmore" @pytest.mark.parametrize( @@ -88,11 +89,13 @@ def test_scanner_receives_only_added_lines() -> None: ], ) def test_formatter_routes_supported_extensions(suffix: str, kind: str) -> None: - assert hook_adapter.EXTENSIONS[Path("file" + suffix).suffix] == kind + assert shared.lint_format.EXTENSIONS[Path("file" + suffix).suffix] == kind def test_translated_command_preserves_session_and_provider_context() -> None: - assert hook_adapter.translated({"session_id": "s", "untrusted": "ignored"}, "Bash", {"command": "git status"}) == { + assert hook_adapter.ADAPTER.translated( + {"session_id": "s", "untrusted": "ignored"}, "Bash", {"command": "git status"} + ) == { "tool_name": "Bash", "tool_input": {"command": "git status"}, "session_id": "s", @@ -193,11 +196,11 @@ def test_blocked_audit_rows_preserve_schema_and_append(tmp_path: Path, mocker: M ], ) def test_ast_command_boundaries_preserve_literals(source: str, expected: list[str]) -> None: - assert [command.raw for command in hook_adapter.shell_syntax.parse(source)] == expected + assert [command.raw for command in shared.shell_syntax.parse(source)] == expected def test_redirect_paths_are_inspected_separately_from_command_arguments() -> None: - command = hook_adapter.shell_syntax.parse("printf x > .env")[0] + command = shared.shell_syntax.parse("printf x > .env")[0] assert command.arguments == ("printf", "x") assert command.redirections == (".env",) @@ -209,16 +212,16 @@ def test_shell_translation_calls_shared_policy( ) -> None: monkeypatch.setenv("AGENT_HARNESS_ROOT", str(tmp_path)) target = ( - hook_adapter.guard_command + shared.guard_command if mode in {"allowed", "forbidden"} - else hook_adapter.guard_git + else shared.guard_git if mode == "git" - else hook_adapter.guard_file + else shared.guard_file ) check = Mock(return_value=2) monkeypatch.setattr(target, "check", check) payload = {"tool_input": {field: "git status"}, "session_id": "session-one"} - assert hook_adapter.dispatch("shell", [mode], payload) == 2 + assert hook_adapter.ADAPTER.dispatch("shell", [mode], payload) == 2 translated = { "tool_name": "Bash", "tool_input": {"command": "git status"}, @@ -235,17 +238,17 @@ def test_shell_translation_calls_shared_policy( def test_adapter_applies_payload_cwd_before_enforcement(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: monkeypatch.chdir(tmp_path.parent) payload = {"cwd": str(tmp_path), "tool_input": {"cmd": "git status"}} - monkeypatch.setattr(hook_adapter.sys, "stdin", io.StringIO(json.dumps(payload))) + monkeypatch.setattr(shared.sys, "stdin", io.StringIO(json.dumps(payload))) observed = [] - monkeypatch.setattr(hook_adapter.guard_git, "check", lambda _: observed.append(Path.cwd()) or 0) - assert hook_adapter.main(["shell", "git"]) == 0 + monkeypatch.setattr(shared.guard_git, "check", lambda _: observed.append(Path.cwd()) or 0) + assert hook_adapter.ADAPTER.main(["shell", "git"]) == 0 assert observed == [tmp_path] @pytest.mark.parametrize("raw", ["{", "null", "[]", '{"tool_input":true}']) def test_adapter_rejects_invalid_input(monkeypatch: pytest.MonkeyPatch, raw: str) -> None: - monkeypatch.setattr(hook_adapter.sys, "stdin", io.StringIO(raw)) - assert hook_adapter.main(["shell", "git"]) == 2 + monkeypatch.setattr(shared.sys, "stdin", io.StringIO(raw)) + assert hook_adapter.ADAPTER.main(["shell", "git"]) == 2 def test_lint_only_checks_existing_supported_patch_files(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: @@ -253,20 +256,20 @@ def test_lint_only_checks_existing_supported_patch_files(monkeypatch: pytest.Mon (tmp_path / "source.py").touch() (tmp_path / "unknown.bin").touch() check = Mock(return_value=()) - monkeypatch.setattr(hook_adapter.lint_format, "check_file", check) + monkeypatch.setattr(shared.lint_format, "check_file", check) patch = "\n".join(f"*** Update File: {name}" for name in ("source.py", "unknown.bin", "missing.py")) - assert hook_adapter.lint({"tool_input": {"command": patch}}) == 0 + assert hook_adapter.ADAPTER.lint({"tool_input": {"command": patch}}) == 0 check.assert_called_once_with("py", tmp_path / "source.py") @pytest.mark.parametrize( "module,mode,identifier", [ - (hook_adapter.guard_command, "allowed", "guard_allowed_commands.sh"), - (hook_adapter.guard_command, "forbidden", "guard_forbidden_commands.sh"), - (hook_adapter.guard_git, "", "guard_dangerous_git.sh"), - (hook_adapter.guard_file, "commit", "guard_secret_commit.sh"), - (hook_adapter.guard_file, "harness", "guard_harness_files.sh"), + (shared.guard_command, "allowed", "guard_allowed_commands.sh"), + (shared.guard_command, "forbidden", "guard_forbidden_commands.sh"), + (shared.guard_git, "", "guard_dangerous_git.sh"), + (shared.guard_file, "commit", "guard_secret_commit.sh"), + (shared.guard_file, "harness", "guard_harness_files.sh"), ], ) def test_malformed_guard_input_remains_auditable( diff --git a/tests/agents/hooks/test_sandbox_escape.py b/tests/agents/hooks/test_sandbox_escape.py new file mode 100644 index 00000000..82d6ad23 --- /dev/null +++ b/tests/agents/hooks/test_sandbox_escape.py @@ -0,0 +1,31 @@ +"""Leaving the sandbox is possible only through a human approval, even in auto mode.""" + +import json +from pathlib import Path + +import pytest + +from tests.runtime import CliRunner + + +ESCAPE_RULE = "Bash(dangerouslyDisableSandbox:true)" + + +@pytest.fixture(scope="module") +def generated_settings(agent_harness: CliRunner, tmp_path_factory: pytest.TempPathFactory) -> dict: + path = tmp_path_factory.mktemp("claude-sandbox") / "settings.json" + agent_harness("generate-claude-settings", "--output", str(path)) + return json.loads(Path(path).read_text()) + + +@pytest.mark.integration +def test_sandbox_escape_is_available(generated_settings: dict) -> None: + assert generated_settings["sandbox"]["enabled"] is True + assert generated_settings["sandbox"]["allowUnsandboxedCommands"] is True + + +@pytest.mark.integration +def test_every_sandbox_escape_asks_the_user(generated_settings: dict) -> None: + permissions = generated_settings["permissions"] + assert ESCAPE_RULE in permissions["ask"] + assert ESCAPE_RULE not in permissions.get("allow", []) diff --git a/tests/agents/hooks/test_secret_path_policy.py b/tests/agents/hooks/test_secret_path_policy.py new file mode 100644 index 00000000..4e9fb310 --- /dev/null +++ b/tests/agents/hooks/test_secret_path_policy.py @@ -0,0 +1,45 @@ +"""Secret path rules deny credential-shaped files without denying ordinary source code.""" + +import pytest + +from tests.runtime import REPO_ROOT, load_script_module + + +policy = load_script_module("agents/hooks/lib/secret_path_policy.py", "secret_path_policy") +RULES = policy.load(REPO_ROOT / "agents/hooks/rules/secret_path_policy.json") + + +@pytest.mark.parametrize( + "path", + [ + ".env", + ".env.local", + "config/github.token", + "config/api_token", + "auth/token.json", + "certs/server.key", + "certs/server.pem", + "certs/client.p12", + "keys/api_key.json", + "home/.netrc", + "credentials.json", + "id_ed25519", + ], +) +def test_credential_paths_are_denied(path: str) -> None: + assert policy.blocked_rule(path, RULES) is not None + + +@pytest.mark.parametrize( + "path", + [ + ".venv/lib/python3.14/site-packages/pygments/token.py", + ".envrc", + "src/tokenizer.rs", + "src/keyboard.ts", + "lib/monkey_patch.py", + "docs/token_design.md", + ], +) +def test_source_code_paths_are_readable(path: str) -> None: + assert policy.blocked_rule(path, RULES) is None diff --git a/tests/agents/hooks/test_tool_target_fields.py b/tests/agents/hooks/test_tool_target_fields.py new file mode 100644 index 00000000..4b9a2b53 --- /dev/null +++ b/tests/agents/hooks/test_tool_target_fields.py @@ -0,0 +1,69 @@ +"""NotebookEdit and Grep payload fields reach the shared file guards and lint targets.""" + +import json +from pathlib import Path + +import pytest + +from tests.runtime import CliRunner, load_script_module + + +SCRIPT = "agents/hooks/guard_file.py" +MARKER = "fixture-sensitive-marker" +hook_input = load_script_module("agents/hooks/lib/hook_input.py", "hook_input") +audit = load_script_module("agents/hooks/audit_log.py", "audit_log") + + +@pytest.fixture(autouse=True) +def policies(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + protected = tmp_path / "protected.json" + protected.write_text(json.dumps({"version": 1, "paths": ["~/.claude/settings.json"]})) + patterns = tmp_path / "patterns.json" + patterns.write_text(json.dumps({"fixture": {"pattern": MARKER, "message": "Fixture detected"}})) + monkeypatch.setenv("AGENT_PROTECTED_PATH_POLICY", str(protected)) + monkeypatch.setenv("AGENT_SECRET_CONTENT_PATTERNS", str(patterns)) + + +def notebook_edit(path: str, source: str = "x = 1") -> dict: + return {"tool_name": "NotebookEdit", "tool_input": {"notebook_path": path, "new_source": source}} + + +def test_notebook_edit_on_a_harness_file_is_blocked(run_cli: CliRunner) -> None: + result = run_cli(SCRIPT, "harness", payload=notebook_edit("~/.claude/settings.json")) + assert result.returncode == 2 + assert "agent harness boundary" in result.stderr + + +@pytest.mark.parametrize("source,blocked", [(MARKER, True), ("x = 1", False)]) +def test_notebook_edit_source_is_scanned(run_cli: CliRunner, source: str, blocked: bool) -> None: + result = run_cli(SCRIPT, "write", payload=notebook_edit("analysis.ipynb", source)) + assert result.returncode == 0 + if blocked: + assert json.loads(result.stdout)["hookSpecificOutput"]["permissionDecision"] == "deny" + else: + assert result.stdout == "" + + +@pytest.mark.parametrize("target,blocked", [("secret.txt", True), (".", False)]) +def test_grep_scans_a_single_file_target_only( + run_cli: CliRunner, isolated_project: Path, target: str, blocked: bool +) -> None: + (isolated_project / "secret.txt").write_text(MARKER + "\n") + payload = {"tool_name": "Grep", "tool_input": {"pattern": "x", "path": str(isolated_project / target)}} + result = run_cli(SCRIPT, "read", payload=payload) + assert result.returncode == 0 + if blocked: + assert json.loads(result.stdout)["hookSpecificOutput"]["permissionDecision"] == "deny" + else: + assert result.stdout == "" + + +def test_notebook_edit_target_reaches_lint(tmp_path: Path) -> None: + payload = notebook_edit("analysis.ipynb") | {"cwd": str(tmp_path)} + assert hook_input.target_paths(payload) == (tmp_path.resolve() / "analysis.ipynb",) + + +def test_notebook_edit_target_is_audited(tmp_path: Path) -> None: + entry = audit.record("tool", notebook_edit("analysis.ipynb") | {"cwd": str(tmp_path)}) + assert entry is not None + assert entry["targets"] == ["analysis.ipynb"] diff --git a/tests/agents/skills/explain-visually/test_verify_page.py b/tests/agents/skills/explain-visually/test_verify_page.py index 83e769d7..c1c77151 100644 --- a/tests/agents/skills/explain-visually/test_verify_page.py +++ b/tests/agents/skills/explain-visually/test_verify_page.py @@ -46,6 +46,18 @@ def test_validate_html_accepts_self_contained_page_with_external_source_link() - '<style>.diagram { background: url("https://cdn.example.com/bg.png"); }</style>', '<style>.diagram { background: url("bg.png"); }</style>', '<script type="module">import value from "./value.js";</script>', + '<svg><image href="https://cdn.example.com/diagram.png"/></svg>', + '<svg><image xlink:href="diagram.png"/></svg>', + '<svg><use href="https://cdn.example.com/icons.svg#arrow"/></svg>', + '<svg><use xlink:href="icons.svg#arrow"/></svg>', + '<embed src="https://cdn.example.com/diagram.pdf">', + '<video><track src="captions.vtt"></video>', + '<input type="image" src="https://cdn.example.com/button.png" alt="submit">', + '<input type="IMAGE" src="button.png" alt="submit">', + '<meta http-equiv="refresh" content="0;url=https://example.com/elsewhere">', + '<meta http-equiv="Refresh" content="0; URL=\'next.html\'">', + '<img srcset="data:image/png;base64,iVBORw0KGgo= 1x, https://cdn.example.com/diagram@2x.png 2x">', + '<picture><source srcset="diagram.webp"></picture>', ], ) def test_validate_html_rejects_externally_loaded_resources(resource: str) -> None: @@ -73,6 +85,24 @@ def test_validate_html_allows_embedded_resources_and_source_examples() -> None: assert verify_page.validate_html(html) == () +@pytest.mark.parametrize( + "resource", + [ + '<img srcset="data:image/png;base64,iVBORw0KGgo= 1x, data:image/png;base64,AAAA 2x" alt="diagram">', + '<picture><source srcset="data:image/webp;base64,UklGRg=="></picture>', + '<svg><defs><path id="arrow" d="M0 0"/></defs><use href="#arrow"/><use xlink:href="#arrow"/></svg>', + '<svg><image href="data:image/png;base64,iVBORw0KGgo="/></svg>', + '<input type="text" src="ignored.png">', + '<meta http-equiv="refresh" content="30">', + '<meta http-equiv="content-type" content="text/html; charset=utf-8">', + ], +) +def test_validate_html_allows_embedded_or_non_loading_references(resource: str) -> None: + html = VALID_HTML.replace("<main>", f"<main>{resource}") + + assert verify_page.validate_html(html) == () + + def test_validate_html_rejects_unresolved_template_tokens() -> None: errors = verify_page.validate_html(VALID_HTML.replace("Example explanation", "{{TITLE}}")) diff --git a/tests/agents/test_command_permission_sync.py b/tests/agents/test_command_permission_sync.py index ef636e21..a0e5edb4 100644 --- a/tests/agents/test_command_permission_sync.py +++ b/tests/agents/test_command_permission_sync.py @@ -119,6 +119,27 @@ def test_obsolete_nonfrozen_and_transient_prefix_is_absent(prefix: list[str]) -> assert all(rule["prefix"] != prefix for rule in POLICY["rules"]) +@pytest.mark.parametrize("prefix", [("docker", "build"), ("docker", "run"), ("docker", "compose", "up")]) +def test_container_execution_requires_approval(prefix: tuple[str, ...]) -> None: + assert prefix in ASK + assert prefix not in ALLOW + + +@pytest.mark.parametrize( + "prefix", + [ + ("herdr", "pane", "run"), + ("herdr", "pane", "send-text"), + ("herdr", "pane", "send-keys"), + ("herdr", "agent", "start"), + ("herdr", "agent", "prompt"), + ("herdr", "agent", "send-keys"), + ], +) +def test_driving_other_terminal_panes_requires_approval(prefix: tuple[str, ...]) -> None: + assert prefix in ASK + + def test_justifications_are_provider_neutral() -> None: provider_specific = [rule for rule in POLICY["rules"] if re.search(r"\b(Codex|Claude)\b", rule["justification"])] assert provider_specific == [] diff --git a/tests/agents/test_entry_points.py b/tests/agents/test_entry_points.py new file mode 100644 index 00000000..d75f2642 --- /dev/null +++ b/tests/agents/test_entry_points.py @@ -0,0 +1,33 @@ +"""Source entry points that providers execute directly are runnable scripts.""" + +import os +import subprocess + +import pytest + +from tests.runtime import REPO_ROOT + + +ENTRY_POINTS = ( + "agents/codex/hooks/hook_adapter.py", + "agents/devin/hooks/hook_adapter.py", + "agents/hermes/hooks/hook_adapter.py", + "agents/pi/hooks/hook_adapter.py", + "agents/claude/statusline/statusline.py", +) + + +@pytest.mark.parametrize("name", ENTRY_POINTS) +def test_entry_point_is_an_executable_script(name: str) -> None: + path = REPO_ROOT / name + assert os.access(path, os.X_OK), name + assert path.read_text().startswith("#!/usr/bin/env -S python3"), name + + +@pytest.mark.parametrize("name", ENTRY_POINTS) +def test_entry_point_is_executable_in_the_git_index(name: str) -> None: + # Nix builds the rendered harness from the Git tree, so the index mode is what ships. + result = subprocess.run( + ["git", "ls-files", "--stage", "--", name], cwd=REPO_ROOT, capture_output=True, text=True, check=True + ) + assert result.stdout.startswith("100755 "), result.stdout or name diff --git a/tests/agents/test_hook_generation.py b/tests/agents/test_hook_generation.py index 9da76065..66b3bdde 100644 --- a/tests/agents/test_hook_generation.py +++ b/tests/agents/test_hook_generation.py @@ -15,13 +15,29 @@ def test_generated_claude_lint_hook_uses_the_dispatcher_without_if(agent_harness path = tmp_path / "settings.json" agent_harness("generate-claude-settings", "--output", str(path)) settings = json.loads(path.read_text()) - groups = [group for group in settings["hooks"]["PostToolUse"] if group.get("matcher") == "Write|Edit"] + groups = [group for group in settings["hooks"]["PostToolUse"] if group.get("matcher") == "Write|Edit|NotebookEdit"] lint_hooks = [hook for group in groups for hook in group["hooks"] if "lint_format.py" in hook["command"]] assert len(lint_hooks) == 1 assert lint_hooks[0]["command"].endswith('lint_format.py"') assert "if" not in lint_hooks[0] +@pytest.mark.parametrize( + "matcher,mode", + [("Write|Edit|NotebookEdit", "harness"), ("Write|Edit|NotebookEdit", "write"), ("Read|Grep", "read")], +) +def test_generated_claude_file_guards_cover_every_file_tool( + agent_harness: CliRunner, tmp_path: Path, matcher: str, mode: str +) -> None: + path = tmp_path / "settings.json" + agent_harness("generate-claude-settings", "--output", str(path)) + groups = [ + group for group in json.loads(path.read_text())["hooks"]["PreToolUse"] if group.get("matcher") == matcher + ] + commands = [hook["command"] for group in groups for hook in group["hooks"]] + assert any(command.endswith(f'guard_file.py" {mode}') for command in commands) + + def test_generated_codex_lint_hook_matches_native_apply_patch_aliases( agent_harness: CliRunner, tmp_path: Path ) -> None: diff --git a/tests/agents/test_provider_adapters.py b/tests/agents/test_provider_adapters.py new file mode 100644 index 00000000..0fd66117 --- /dev/null +++ b/tests/agents/test_provider_adapters.py @@ -0,0 +1,128 @@ +"""Behavior every provider hook adapter shares, exercised through each real entry point.""" + +import json +from pathlib import Path +import shutil + +import pytest + +from tests.runtime import CliRunner, REPO_ROOT, load_script_module + + +PROVIDERS = ("codex", "devin", "hermes", "pi") + + +def script(provider: str) -> str: + return f"agents/{provider}/hooks/hook_adapter.py" + + +@pytest.fixture +def harness(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + home = tmp_path / "home" + root = tmp_path / "harness" + home.mkdir() + rules = root / ".claude/hooks/rules" + rules.mkdir(parents=True) + paths = [f"~/.{provider}/hooks/hook_adapter.py" for provider in PROVIDERS] + (rules / "protected_paths.json").write_text(json.dumps({"version": 1, "paths": paths})) + shutil.copyfile(REPO_ROOT / "agents/hooks/rules/secret_path_policy.json", rules / "secret_path_policy.json") + monkeypatch.setenv("HOME", str(home)) + monkeypatch.setenv("AGENT_HARNESS_ROOT", str(root)) + for name in ("AGENT_PROTECTED_PATH_POLICY", "AGENT_SECRET_PATH_POLICY", "DEVIN_PROJECT_DIR"): + monkeypatch.delenv(name, raising=False) + return root + + +@pytest.fixture +def state(isolated_project: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + # Audit logging refuses a state directory inside the working tree it describes. + path = isolated_project.parent / (isolated_project.name + "-state") + monkeypatch.setenv("XDG_STATE_HOME", str(path)) + return path + + +def audit_rows(state: Path) -> list[dict]: + logs = sorted((state / "agent-harness/audit").rglob("*.jsonl")) + return [json.loads(line) for path in logs for line in path.read_text().splitlines()] + + +@pytest.mark.parametrize("provider", PROVIDERS) +@pytest.mark.parametrize( + "case", + [("command", {"command": "cat .env"}), ("patch", {"file_path": "~/.ssh/config"})], +) +def test_secret_path_denial_is_audited_for_the_provider( + run_cli: CliRunner, harness: Path, state: Path, provider: str, case: tuple[str, dict] +) -> None: + mode, values = case + payload = {"session_id": "adapter-session", "tool_input": values} + result = run_cli(script(provider), "paths", mode, payload=payload) + assert result.returncode == 2 + [row] = audit_rows(state) + assert row["status"] == "blocked" + assert row["provider"] == provider + assert row["rule"] == "guard_secret_paths.sh" + + +@pytest.mark.parametrize("provider", ["devin", "pi"]) +def test_notebook_edits_are_checked_against_the_harness_boundary( + run_cli: CliRunner, harness: Path, provider: str +) -> None: + values = {"notebook_path": f"~/.{provider}/hooks/hook_adapter.py", "new_source": "x = 1"} + result = run_cli(script(provider), "harness", payload={"tool_name": "notebook_edit", "tool_input": values}) + assert result.returncode == 2 + assert "agent harness boundary" in result.stderr + + +SESSION_START = {"devin": "SessionStart", "hermes": "on_session_start"} + + +@pytest.mark.parametrize("provider", sorted(SESSION_START)) +@pytest.mark.parametrize("source,recorded", [("startup", False), ("", False), ("compact", True), ("resume", True)]) +def test_session_start_compaction_is_recorded_only_for_compact_or_resume( + run_cli: CliRunner, state: Path, provider: str, source: str, recorded: bool +) -> None: + payload = {"hook_event_name": SESSION_START[provider], "session_id": "adapter-session", "source": source} + result = run_cli(script(provider), "audit", "compaction", payload=payload) + assert result.returncode == 0 + rows = audit_rows(state) + assert len(rows) == int(recorded) + if recorded: + assert rows[0]["event"] == "SessionStart" + assert rows[0]["source"] == source + assert rows[0]["provider"] == provider + + +@pytest.mark.parametrize( + "provider,event,expected", + [ + ("devin", "PostCompaction", "PostCompaction"), + ("pi", "session_before_compact", "PreCompact"), + ("pi", "session_compact", "PostCompaction"), + ("devin", "", None), + ], +) +def test_native_compaction_events_are_recorded_under_the_shared_name( + run_cli: CliRunner, state: Path, provider: str, event: str, expected: str | None +) -> None: + payload = {"hook_event_name": event, "session_id": "adapter-session"} + result = run_cli(script(provider), "audit", "compaction", payload=payload) + assert result.returncode == 0 + assert [row["event"] for row in audit_rows(state)] == ([expected] if expected else []) + + +@pytest.mark.parametrize("provider", PROVIDERS) +def test_normalization_leaves_the_provider_payload_untouched(provider: str) -> None: + adapter = load_script_module(script(provider), f"{provider}_hook_adapter_under_test").ADAPTER + payload = {"tool_name": "write", "tool_input": {"path": "notes.txt"}, "args": {"path": "other.txt"}} + original = json.loads(json.dumps(payload)) + adapter.normalized(payload) + assert payload == original + + +@pytest.mark.parametrize("provider", PROVIDERS) +@pytest.mark.parametrize("raw", ["{", "null", "[]"]) +def test_non_object_payloads_are_rejected(run_cli: CliRunner, harness: Path, provider: str, raw: str) -> None: + result = run_cli(script(provider), "shell", "git", payload=raw) + assert result.returncode == 2 + assert "BLOCKED" in result.stderr diff --git a/tests/agents/test_python_runtime.py b/tests/agents/test_python_runtime.py index c161471e..652b4e23 100644 --- a/tests/agents/test_python_runtime.py +++ b/tests/agents/test_python_runtime.py @@ -10,46 +10,67 @@ from tests.runtime import CliRunner -@pytest.mark.integration -def test_rendered_hooks_load_adjacent_modules(agent_harness: CliRunner, isolated_project: Path) -> None: +ADAPTERS = (".codex/hooks", ".devin/hooks", ".hermes/hooks", ".pi/hooks") + + +@pytest.fixture +def rendered(agent_harness: CliRunner, isolated_project: Path) -> Path: prefix = isolated_project / "rendered" agent_harness("install", "--prefix", str(prefix), "--runtime-root", str(prefix)) - for name in ( - ".claude/hooks/guard_command.py", - ".claude/hooks/lint_format.py", - ".claude/hooks/lib/shell_syntax.py", - ".codex/hooks/hook_adapter.py", - ".claude/statusline/statusline.py", - ): - assert (prefix / name).is_file(), name - environment = os.environ | {"AGENT_HARNESS_ROOT": str(prefix)} + return prefix + + +@pytest.fixture +def environment(rendered: Path) -> dict[str, str]: + result = os.environ | {"AGENT_HARNESS_ROOT": str(rendered)} for name in ("AGENT_COMMAND_PERMISSIONS", "AGENT_ALLOWED_COMMAND_RULES", "AGENT_FORBIDDEN_COMMAND_RULES"): - environment.pop(name, None) - lint = prefix / ".claude/hooks/lint_format.py" - result = subprocess.run( - [str(lint), "py"], - input=json.dumps({"tool_input": {}}), + result.pop(name, None) + return result + + +def run( + executable: Path, arguments: list[str], payload: dict, environment: dict[str, str] +) -> subprocess.CompletedProcess: + return subprocess.run( + [str(executable), *arguments], + input=json.dumps(payload), env=environment, text=True, capture_output=True, timeout=10, check=False, ) + + +@pytest.mark.integration +def test_rendered_hooks_load_adjacent_modules(rendered: Path, environment: dict[str, str]) -> None: + for name in ( + ".claude/hooks/guard_command.py", + ".claude/hooks/lint_format.py", + ".claude/hooks/lib/shell_syntax.py", + ".claude/hooks/lib/provider_adapter.py", + ".claude/statusline/statusline.py", + *(f"{directory}/hook_adapter.py" for directory in ADAPTERS), + ): + assert (rendered / name).is_file(), name + result = run(rendered / ".claude/hooks/lint_format.py", ["py"], {"tool_input": {}}, environment) assert result.returncode == 0 assert result.stdout == result.stderr == "" - executable = prefix / ".codex/hooks/hook_adapter.py" - for arguments, payload, status, message in ( + + +@pytest.mark.integration +@pytest.mark.parametrize("directory", ADAPTERS) +@pytest.mark.parametrize( + "case", + [ (["shell", "allowed"], {"tool_input": {"cmd": "git status"}}, 0, ""), (["paths", "patch"], {"tool_input": {"file_path": ".env.local"}}, 2, "secret path policy matched"), - ): - result = subprocess.run( - [str(executable), *arguments], - input=json.dumps(payload), - env=environment, - text=True, - capture_output=True, - timeout=10, - check=False, - ) - assert result.returncode == status, result.stderr - assert message in result.stderr + ], +) +def test_rendered_provider_adapters_run_shared_guards( + rendered: Path, environment: dict[str, str], directory: str, case: tuple[list[str], dict, int, str] +) -> None: + arguments, payload, status, message = case + result = run(rendered / directory / "hook_adapter.py", arguments, payload, environment) + assert result.returncode == status, result.stderr + assert message in result.stderr diff --git a/tests/agents/test_workflow_skill_layout.py b/tests/agents/test_workflow_skill_layout.py index 1cdd9c03..482fa1fc 100644 --- a/tests/agents/test_workflow_skill_layout.py +++ b/tests/agents/test_workflow_skill_layout.py @@ -1,5 +1,10 @@ -"""Rendering and link integrity, not claims about model compliance with prose.""" +"""Skill source contracts: metadata, link integrity, and handoff reachability. +These tests check rendering and link integrity, not claims about model compliance with prose. +""" + +from collections import Counter +from collections.abc import Iterator import json from pathlib import Path import re @@ -10,7 +15,15 @@ SKILLS = REPO_ROOT / "agents/skills" -WORKFLOW_SKILLS = ("issue-workflow", "git-workflow", "source-of-truth", "tsdd") +SKILL_NAMES = sorted(path.name for path in SKILLS.iterdir() if path.is_dir()) +SKILL_RENDERING = REPO_ROOT / "agents/skill_rendering.json" +FENCED_CODE_PATTERN = re.compile(r"^(```|~~~).*?^\1\s*$", re.MULTILINE | re.DOTALL) +INLINE_CODE_PATTERN = re.compile(r"`[^`\n]*`") +LINK_PATTERN = re.compile(r"\[[^\]]*\]\(<?([^)\s>]+)>?(?:\s+\"[^\"]*\")?\)") +HEADING_PATTERN = re.compile(r"^#{1,6}\s+(.+?)\s*#*\s*$", re.MULTILINE) +FRONTMATTER_PATTERN = re.compile(r"\A---\n(.*?)\n---\n", re.DOTALL) +FRONTMATTER_KEY_PATTERN = re.compile(r"^([A-Za-z][\w-]*):[ \t]*(.*)$") +SKILL_HANDOFF_PATTERN = re.compile(r"^\.\./([^/]+)/SKILL\.md$") @pytest.mark.integration @@ -28,64 +41,137 @@ def test_issue_skill_renders_with_implicit_invocation(agent_harness: CliRunner, metadata = output / "issue-workflow/agents/openai.yaml" if metadata.exists(): assert not re.search(r"allow_implicit_invocation:\s*false", metadata.read_text()) - config = json.loads((REPO_ROOT / "agents/skill_rendering.json").read_text()) - settings = config["skills"].get("issue-workflow", {}) - assert settings.get("claude", {}).get("frontmatter", {}).get("disable-model-invocation") is not True - assert settings.get("codex", {}).get("openai", {}).get("allow_implicit_invocation") is not False - - -def test_workflow_local_links_resolve_and_retired_issue_reference_is_absent() -> None: - assert not (SKILLS / "git-workflow/references/issues.md").exists() - for name in WORKFLOW_SKILLS: - for path in (SKILLS / name).rglob("*.md"): - text = path.read_text() - for destination in re.findall(r"\[[^\]]+\]\(([^)]+)\)", text): - filename = destination.split("#", 1)[0] - if not filename or "://" in filename: - continue - assert (path.parent / filename).is_file(), (path, destination) - assert not filename.endswith("/issues.md"), (path, destination) - - -CHANGED_SKILLS = ("herdr-delegation", "tsdd", "github-actions-style") + assert not _is_model_invocation_disabled("issue-workflow") @pytest.mark.integration @pytest.mark.parametrize("provider", ["claude", "codex"]) -@pytest.mark.parametrize("skill", CHANGED_SKILLS) -def test_changed_skills_render_with_valid_frontmatter( - agent_harness: CliRunner, tmp_path: Path, provider: str, skill: str +def test_every_skill_renders_with_its_name_and_description( + agent_harness: CliRunner, tmp_path: Path, provider: str ) -> None: output = tmp_path / provider agent_harness("generate-skills", "--provider", provider, "--output", str(output)) - text = (output / skill / "SKILL.md").read_text() - frontmatter = text.split("---", 2)[1] - assert re.search(rf"^name:\s*{re.escape(skill)}\s*$", frontmatter, re.MULTILINE) - assert re.search(r"^description:\s*\S", frontmatter, re.MULTILINE) + for skill in SKILL_NAMES: + frontmatter = _frontmatter((output / skill / "SKILL.md").read_text()) + assert frontmatter.get("name") == skill, skill + assert frontmatter.get("description"), skill -@pytest.mark.parametrize("skill", CHANGED_SKILLS) -def test_changed_skill_links_resolve(skill: str) -> None: - for path in (SKILLS / skill).rglob("*.md"): - for destination in re.findall(r"\[[^\]]+\]\(([^)]+)\)", path.read_text()): - filename = destination.split("#", 1)[0] - if not filename or "://" in filename: - continue - assert (path.parent / filename).is_file(), (path, destination) +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_frontmatter_names_its_directory_and_describes_it(skill: str) -> None: + frontmatter = _frontmatter((SKILLS / skill / "SKILL.md").read_text()) + assert frontmatter.get("name") == skill + assert frontmatter.get("description") -@pytest.mark.integration -def test_generated_github_actions_skill_drops_only_the_blacksmith_section( - agent_harness: CliRunner, tmp_path: Path -) -> None: - output = tmp_path / "claude" - agent_harness("generate-skills", "--provider", "claude", "--output", str(output)) - rendered = (output / "github-actions-style/SKILL.md").read_text() - assert "blacksmith" not in rendered.lower() - assert "blacksmith" not in (SKILLS / "github-actions-style/SKILL.md").read_text().lower() - assert "## 7. Cache Design" in rendered + +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_relative_links_resolve_to_existing_files_and_headings(skill: str) -> None: + broken_links = [ + (str(path.relative_to(SKILLS)), destination) + for path, destination in _relative_links(SKILLS / skill) + if not _link_resolves(path, destination) + ] + + assert broken_links == [] + + +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_handoff_links_name_existing_skills(skill: str) -> None: + missing_skills = sorted(target for target in _linked_skills(SKILLS / skill) if target not in SKILL_NAMES) + + assert missing_skills == [] + + +@pytest.mark.parametrize("skill", SKILL_NAMES) +def test_skill_handoff_targets_allow_model_invocation(skill: str) -> None: + disabled_targets = sorted( + target + for target in _handoff_targets(SKILLS / skill) + if target != skill and _is_model_invocation_disabled(target) + ) + + assert disabled_targets == [] + + +def _frontmatter(text: str) -> dict[str, str]: + match = FRONTMATTER_PATTERN.match(text) + assert match is not None, "SKILL.md must start with YAML frontmatter" + fields: dict[str, list[str]] = {} + current_key: str | None = None + for line in match.group(1).splitlines(): + key_match = FRONTMATTER_KEY_PATTERN.match(line) + if key_match is not None: + current_key = key_match.group(1) + fields[current_key] = [key_match.group(2)] + elif current_key is not None and line[:1].isspace(): + fields[current_key].append(line) + return {key: _scalar(parts) for key, parts in fields.items()} + + +def _scalar(parts: list[str]) -> str: + head, *continuation = parts + words = continuation if head.strip() in {">", "|", ">-", "|-"} else [head, *continuation] + return " ".join(word.strip() for word in words).strip().strip("\"'") + + +def _relative_links(skill_directory: Path) -> Iterator[tuple[Path, str]]: + for path in sorted(skill_directory.rglob("*.md")): + for destination in LINK_PATTERN.findall(_prose(path.read_text())): + if not re.match(r"^[a-z][a-z0-9+.-]*:", destination, re.IGNORECASE): + yield path, destination + + +def _link_resolves(source: Path, destination: str) -> bool: + filename, _, anchor = destination.partition("#") + target = (source.parent / filename).resolve() if filename else source + if not target.is_file(): + return False + return not anchor or target.suffix != ".md" or anchor in _heading_anchors(target) + + +def _heading_anchors(path: Path) -> set[str]: + counts: Counter[str] = Counter() + anchors: set[str] = set() + for heading in HEADING_PATTERN.findall(_prose(path.read_text(), keep_inline_code=True)): + slug = _github_slug(heading) + anchors.add(f"{slug}-{counts[slug]}" if counts[slug] else slug) + counts[slug] += 1 + return anchors + + +def _github_slug(heading: str) -> str: + return re.sub(r"[^\w\- ]", "", heading.strip().lower()).replace(" ", "-") + + +def _handoff_targets(skill_directory: Path) -> set[str]: + return _linked_skills(skill_directory) | _skills_named_in_inline_code(skill_directory) + + +def _linked_skills(skill_directory: Path) -> set[str]: + return { + match.group(1) + for _, destination in _relative_links(skill_directory) + if (match := SKILL_HANDOFF_PATTERN.match(destination.partition("#")[0])) is not None + } + + +def _skills_named_in_inline_code(skill_directory: Path) -> set[str]: + return { + code.strip("`") + for path in skill_directory.rglob("*.md") + for code in INLINE_CODE_PATTERN.findall(_prose(path.read_text(), keep_inline_code=True)) + if code.strip("`") in SKILL_NAMES + } + + +def _is_model_invocation_disabled(skill: str) -> bool: + settings = json.loads(SKILL_RENDERING.read_text())["skills"].get(skill, {}) + claude_disabled = settings.get("claude", {}).get("frontmatter", {}).get("disable-model-invocation") is True + codex_disabled = settings.get("codex", {}).get("openai", {}).get("allow_implicit_invocation") is False + return claude_disabled or codex_disabled -def test_no_review_or_ponytail_skill_was_added() -> None: - names = {path.name for path in SKILLS.iterdir() if path.is_dir()} - assert not any("review" in name or "ponytail" in name for name in names) +def _prose(text: str, *, keep_inline_code: bool = False) -> str: + without_fences = FENCED_CODE_PATTERN.sub("", text) + return without_fences if keep_inline_code else INLINE_CODE_PATTERN.sub("", without_fences)