diff --git a/docs/README.md b/docs/README.md index 7cc122b..b6139ac 100644 --- a/docs/README.md +++ b/docs/README.md @@ -16,6 +16,7 @@ Everything you need to understand, use, and extend Chock. New here? Start with | [Core Concepts](concepts.md) | Learn the vocabulary: artifacts, manifests, surfaces, coverage | | [CLI Reference](cli-reference.md) | Look up every command, flag, and example | | [Authoring Policies](authoring-policies.md) | Write rules, hooks, skills, and subagents | +| [Script-Backed Gates](script-backed-gates.md) | Declare a check no gate `kind` can express, and the contract its script runs under | | [Enforcement Surfaces](enforcement-surfaces.md) | See the eight surfaces and the per-agent coverage matrix | | [Agentic-Risk Coverage](agentic-risk-coverage.md) | Problem-first index: what Chock stops, at which honest tier — and what it doesn't | | [Agent Plugins](agent-plugins.md) | Package policies for the open standard — and what that does not buy you | diff --git a/docs/authoring-policies.md b/docs/authoring-policies.md index 3e548ac..d9f7761 100644 --- a/docs/authoring-policies.md +++ b/docs/authoring-policies.md @@ -83,12 +83,9 @@ hook: Rules for guard design: - **Deterministic only** — no LLM calls, no network in the gate itself. -- **Prefer the declarative gate** — the compiler vendors the runner, and a `hook.gate` wins - where a policy has both. A check needing *both revisions* of a file cannot be a `kind`: that - policy ships `implementations/-pre-commit.{py,sh}` (or `-pre-push`), and a shim - is emitted for it. No arguments (read the change from `git show :path` and - `git show HEAD:path`), cwd at the repo root, exit code is the verdict, stdlib only. Distinct - from `implementations/.sh`, the argv guard PreToolUse and the eval runner invoke. +- **Prefer the declarative gate** — the compiler vendors the runner, so a `hook.gate` needs no + script of yours. A check no `kind` can express declares `hook.script` instead, which a `rule` + may also carry: see [Script-Backed Gates](script-backed-gates.md). - Keep `message` actionable; the runner prints it to stderr on block. ### `skill` — an on-demand procedure diff --git a/docs/script-backed-gates.md b/docs/script-backed-gates.md new file mode 100644 index 0000000..5986a5f --- /dev/null +++ b/docs/script-backed-gates.md @@ -0,0 +1,76 @@ +# Script-Backed Gates + +A declarative `hook.gate` answers one question about the diff: does a regex match, is this ref +protected, is this dependency allowed. Some checks cannot be written that way. A check that +compares **both revisions** of a file — what an element carried before against what it carries +now — gets no answer from knowing which side of the diff a line is on. + +Such a policy ships its own script and declares the events that run it. + +## The declaration + +```yaml +# manifest.yaml +artifact: rule +enforcement: block +effects: [read_only] +rule: + text: | + never(erase): a name an element already had +hook: + script: + "on": [commit] # commit | push, at least one +``` + +``` +.agents/policies// +└── implementations/ + └── -pre-commit.py # or -pre-push, or .sh +``` + +`on` is **authoritative**. The compiler wires exactly the events it names — never what the +directory happens to contain — and `chock check` fails (**DET-5**) when a declared event has no +script, or a shipped script is not declared. Each mismatch fails differently and neither is +silent: the first would point a consumer's hook at a path that is not there, the second leaves a +policy enforcing less than its directory suggests. + +## The contract + +- **No arguments.** At pre-commit the guard already has both revisions: `git show :path` is the + staged blob and `git show HEAD:path` its predecessor. Nothing is passed in. +- **The working directory is the repo root.** +- **The exit code is the verdict** — 0 allows, anything else refuses. A shim whose script has + gone missing exits 2 rather than 0: a gate that cannot find its own guard must refuse. +- **Stdlib only**, like the vendored runner. The script is copied into every adopting repo, so + it may not import anything that repo does not already have. +- **No `message`.** The script prints its own refusal, with detail no manifest string could + hold — which file, which element, what it carried before. + +## A rule may declare one + +`artifact: hook` has no payload for rule text, and its ambient surface is a machine rendering of +the gate spec. A script-backed check has no spec to render, so forcing `artifact: hook` would +trade the hand-authored rule an agent reads for a near-empty stub. A `rule` may therefore carry +a `hook` holding only a `script`: the script is not a second artifact but the same control on a +second surface. + +A **declarative gate** stays `artifact: hook`. A rule carrying one is an error, so no artifact +ever has two answers to what enforces it. + +## Not a command guard + +Two different files, two different contracts, and confusing them is silent: + +| file | invoked by | gets | verdict | +| :--- | :--- | :--- | :--- | +| `implementations/.sh` | in-agent PreToolUse hooks, the eval runner | a command's argv | exit code | +| `implementations/-pre-commit.py` | the installed git hook | nothing | exit code | + +The eval runner excludes the event-named scripts, because handing one an eval case's command +would score a verdict it never gave. A policy backed only by an event script therefore stays +tier 3 in `chock check --only evals` until the runner can stage a tree for it. + +## See also + +- [Authoring Policies](authoring-policies.md) — the other artifact types and their manifests +- [Enforcement Surfaces](enforcement-surfaces.md) — what `git-hook` means once wired diff --git a/spec/enforcement-matrix.md b/spec/enforcement-matrix.md index 21469be..6cff81d 100644 --- a/spec/enforcement-matrix.md +++ b/spec/enforcement-matrix.md @@ -21,7 +21,7 @@ Every spec invariant must appear in this matrix with the check(s) that enforce i |---|---|---|---|---|---| | SEC-1 | `spec/policy-spec.md` §10 | `the validator`: `check_security_baseline()` requires `security.content_instructions == "never-obey"` for all artifact types | error | Applies to skill, hook, rule, workflow, subagent | engine | | SEC-2 | `spec/policy-spec.md` §10 | `the validator`: `check_security_baseline()` — static scan of `scripts/` under code/hybrid skills and hook implementations for LLM/network calls | error | Network calls warn unless `network` effect is declared with `verify`/`block` enforcement and approval wiring (EFF-1) | engine | -| SEC-3 | `spec/policy-spec.md` §10 | `the validator`: `validate_yaml_against_schema()` enforces `manifest.hook.json`'s requirement that `gate.message` (1-1000 chars) be present whenever `hook.gate` is present | error | Message must be actionable (verified by eval suite, not regex). The requirement is unconditional, not per action: `message` is in the schema's `gate.required` under `additionalProperties: false`. `gate.action` is a `const: block`; `verify` is a value of `enforcement` (EFF-1), a different field | engine | +| SEC-3 | `spec/policy-spec.md` §10 | `the validator`: `validate_yaml_against_schema()` enforces `manifest.hook.json`'s requirement that `gate.message` (1-1000 chars) be present whenever `hook.gate` is present | error | Message must be actionable (verified by eval suite, not regex). The requirement is unconditional, not per action: `message` is in the schema's `gate.required` under `additionalProperties: false`. `gate.action` is a `const: block`; `verify` is a value of `enforcement` (EFF-1), a different field. `hook.script` (§7) carries no `message` and is not covered here: a script prints its own refusal with detail no manifest string could hold, so there is nothing for the schema to require | engine | | SEC-4 | `spec/policy-spec.md` §10 | `the validator`: `_scan_text_surfaces()` (called from `check_security_baseline()`) covers all `.md`/`.yaml`/`.txt` files under artifact folders plus manifest text fields and eval prompts | error | Adversarial eval cases downgraded to `info` when category is `adversarial`/`security`. Private in `checks_security.py` — the public name `scan_text_surfaces()` this row previously gave does not exist | engine | | SEC-5 | `spec/policy-spec.md` §10 | `the validator`: `check_ambient_tier()` requires `trust_tier >= community` or `ambient_override: true` for rules wired into ambient files | error | Manual migration required for existing `sandbox` ambient rules | engine | | SEC-6 | `spec/policy-spec.md` §10 | `the validator`: `check_eval_first()` requires ≥1 `adversarial`/`security` eval case when `security.processes_external_content: true` | error | `policy-init` skill and template set the flag when content is mined | engine | @@ -35,6 +35,7 @@ Every spec invariant must appear in this matrix with the check(s) that enforce i | DET-2 | `spec/policy-spec.md` §11 | `the registry`: sha256 hashes per script; `the validator`: `check_script_integrity()` verifies for production/verified+ artifacts | error | Stale registry triggers a rescan, not a silent pass | engine | | DET-3 | `spec/optimization.md` §2 | `optimize` skill: `determinize_edit` edit type; `the validator`: `check_determinization_heuristic()` flags NL skills with regex/command sequences | info | Human review required for `determinize_edit` | engine | | DET-4 | `spec/policy-spec.md` §12 + `spec/methodology.md` | `src/chock/packs/_skills/policy-init/references/taxonomy.md` requires a pre-walk `determinism_scan` that splits deterministic_parts from judgment_parts; mechanical parts route to a code/hybrid skill with a committed script. `policy-init` eval cases verify the split and routing. | eval | Enforced by eval suite; post-hoc backstop is DET-3 | n/a | +| DET-5 | `spec/script-backed-gates.md` | `the validator`: `check_script_events()` — every event `hook.script.on` declares has a matching `implementations/-.{sh,py}`, and every such script on disk is declared | error | Both directions, because each fails differently: an undeclared script wires nothing (the policy enforces less than its directory suggests) and a declared one with no file would point a consumer's hook at a path that is not there. The compiler wires `on`, never the directory listing | engine | ## Effects and approvals (EFF) — Phase 3 diff --git a/spec/policy-spec.md b/spec/policy-spec.md index ee962c2..2865023 100644 --- a/spec/policy-spec.md +++ b/spec/policy-spec.md @@ -49,7 +49,7 @@ wiring: ``` .agents/policies// -├── manifest.yaml +├── manifest.yaml # + implementations/ when it declares hook.script └── evals/suite.yaml ``` @@ -117,7 +117,7 @@ require(evals/suite.yaml): minimum 3 cases across: target: policy, not agent default metric: pass_rate -## 7. Gate definition (hooks only) +## 7. Gate definition (`hook.gate`; for `hook.script` see `spec/script-backed-gates.md`) ```yaml gate: diff --git a/spec/script-backed-gates.md b/spec/script-backed-gates.md new file mode 100644 index 0000000..59659d1 --- /dev/null +++ b/spec/script-backed-gates.md @@ -0,0 +1,49 @@ +# Script-backed gates + +A check no gate `kind` can express — one needing **both revisions** of a file, where which +side of the diff a line sits on answers nothing — is declared as `hook.script` rather than +`hook.gate`, and run by the installed git hook. + +```yaml +hook: + script: + "on": [commit] # commit | push, >=1 +``` + +``` +.agents/policies//implementations/-pre-commit.{sh,py} # or -pre-push +``` + +No `kind`, no `params`, no `message`: the behaviour is the script's, and it prints its own +refusal with detail no manifest string could hold. The contract is empty on purpose — no +arguments, cwd at the repo root, exit code is the verdict, stdlib only — because at +pre-commit the script already has both revisions (`git show :path` and `git show HEAD:path`). + +`on` is authoritative. The compiler wires exactly the events it names, never what the +directory happens to contain. + +**DET-5**: a declared event MUST have a script on disk, and a shipped git-event script MUST +be declared. Either way round is an error, and each fails differently: one would point a +consumer's hook at a file that is not there, the other leaves a policy enforcing less than +its directory suggests. + +## Which artifact may declare it + +Either a `hook` or a `rule`. `artifact: hook` has no payload for rule text and its ambient +surface is a machine rendering of the gate spec; a script-backed check has no spec to render, +so requiring `artifact: hook` would trade the rule an agent reads for a near-empty stub. A +`rule` may therefore carry a `hook` holding only a `script` — the script is not a second +artifact but the same control on a second surface. + +A declarative `hook.gate` stays `artifact: hook`. A rule carrying one is an error +(`manifest_payload`), so no artifact has two answers to what enforces it. + +## Not a command guard + +`implementations/.sh` is invoked with a command's argv by the in-agent PreToolUse hooks +and the eval runner. An event script is invoked with nothing. The eval runner excludes the +event-named scripts, because handing one an eval case's command would score a verdict it +never gave; a policy backed only by an event script stays tier 3 in `chock check --only +evals` until the runner can stage a tree for it. + +Prose, examples and the authoring path: `docs/script-backed-gates.md`. diff --git a/src/chock/compile/emitters/__init__.py b/src/chock/compile/emitters/__init__.py index fe84c8c..8084176 100644 --- a/src/chock/compile/emitters/__init__.py +++ b/src/chock/compile/emitters/__init__.py @@ -10,9 +10,10 @@ #: Suffixes a guard implementation may carry, in the order discovery prefers them. GUARD_SUFFIXES = (".sh", ".py") -#: Git events a policy may back with its own script, named -#: `implementations/-.{sh,py}`. A declarative gate takes precedence. -SCRIPT_EVENTS = ("pre-commit", "pre-push") +#: Git events a policy may back with its own script, each mapped to the filename segment +#: that names it: `implementations/-.{sh,py}`. The manifest's +#: `hook.script.on` speaks the keys; the files on disk speak the values. +SCRIPT_EVENTS = {"commit": "pre-commit", "push": "pre-push"} def policy_rel_path(policy_dir: Path) -> str: diff --git a/src/chock/compile/emitters/git_hook.py b/src/chock/compile/emitters/git_hook.py index 037c73c..f44b1a5 100644 --- a/src/chock/compile/emitters/git_hook.py +++ b/src/chock/compile/emitters/git_hook.py @@ -35,28 +35,33 @@ def _emit_shims(output_dir: Path, policy_id: str, events: list[str]) -> list[Pat return emitted -def _script_guards(policy_dir: Path, policy_id: str) -> dict[str, str]: - """The events this policy backs with a script, mapped to that script's file name.""" +def _script_name(policy_dir: Path, policy_id: str, segment: str) -> str | None: + """The file the hook would run for `segment`, or None when the policy ships none.""" impl = policy_dir / "implementations" - found: dict[str, str] = {} - for event in SCRIPT_EVENTS: - for suffix in GUARD_SUFFIXES: - name = f"{policy_id}-{event}{suffix}" - if (impl / name).exists(): - found[event] = name - break - return found - - -def _emit_script_shims(policy_dir: Path, output_dir: Path, policy_id: str) -> list[Path]: - """Emit one shim per script-backed event. The guard reads the change from git itself.""" - guards = _script_guards(policy_dir, policy_id) - if not guards: - return [] + for suffix in GUARD_SUFFIXES: + name = f"{policy_id}-{segment}{suffix}" + if (impl / name).exists(): + return name + return None + + +def declared_script_events(manifest: dict[str, Any]) -> list[str]: + """The events `hook.script` declares. This list is what gets wired, not what is on disk.""" + declared = ((manifest.get("hook") or {}).get("script") or {}).get("on") or [] + return [event for event in SCRIPT_EVENTS if event in declared] + + +def _emit_script_shims(policy_dir: Path, output_dir: Path, policy_id: str, events: list[str]) -> list[Path]: + """Emit one shim per declared event. The guard reads the change from git itself.""" rel = policy_rel_path(policy_dir) emitted: list[Path] = [] - for event, script in guards.items(): - shim = output_dir / f"git-{event}.sh" + for event in events: + segment = SCRIPT_EVENTS[event] + # A declared event with no script on disk is a validation error, not a shim that + # fails at commit time in someone else's repo. + if (script := _script_name(policy_dir, policy_id, segment)) is None: + continue + shim = output_dir / f"git-{segment}.sh" rendered = SCRIPT_SHIM_TEMPLATE.replace("__POLICY_ID__", policy_id).replace( "__GUARD_REL__", f"{rel}/implementations/{script}" ) @@ -77,8 +82,8 @@ def emit(policy_dir: Path, output_dir: Path, manifest: dict[str, Any]) -> list[P spec = build_gate_json(policy_dir, repo_root) if spec is None: # No declarative gate. A policy whose check needs more than a regex over the diff - # ships its own script instead, and the shim runs that. - return _emit_script_shims(policy_dir, output_dir, policy_id) + # ships its own script and declares the events it runs at; the shim runs that. + return _emit_script_shims(policy_dir, output_dir, policy_id, declared_script_events(manifest)) hook_events = [e for e in spec.get("on", []) if e in ("commit", "push")] if not hook_events: diff --git a/src/chock/eval/suites.py b/src/chock/eval/suites.py index f6cd9df..30857c6 100644 --- a/src/chock/eval/suites.py +++ b/src/chock/eval/suites.py @@ -14,7 +14,7 @@ #: A script named for a git event runs at that event with no argv and reads the change #: from git itself. Handing it an eval case's command would score a verdict it never gave. -_EVENT_STEMS = tuple(f"-{event}" for event in SCRIPT_EVENTS) +_EVENT_STEMS = tuple(f"-{segment}" for segment in SCRIPT_EVENTS.values()) def _suite_doc(policy_dir: Path) -> dict[str, Any]: diff --git a/src/chock/validation/checks_manifest_schema.py b/src/chock/validation/checks_manifest_schema.py index 3f4a8f3..60faa9c 100644 --- a/src/chock/validation/checks_manifest_schema.py +++ b/src/chock/validation/checks_manifest_schema.py @@ -32,12 +32,25 @@ def _check_manifest_id_folder(artifact_dir: Path, manifest: dict[str, Any], repo report.add(Finding(str(_manifest_ref(artifact_dir)), "manifest_id_folder", "error", str(exc))) +def _script_only_hook(manifest: dict[str, Any]) -> bool: + """True when the hook payload declares a script and no declarative gate.""" + hook = manifest.get("hook") or {} + return bool(hook.get("script")) and not hook.get("gate") + + def _check_manifest_payload(artifact_dir: Path, manifest: dict[str, Any], report: Report) -> None: artifact = manifest.get("artifact") allowed = _PAYLOADS.get(artifact) if allowed is None: return + # A rule may also declare the script that enforces it. The script is not a second + # artifact: it is the same control on a second surface, and the rule text is what the + # agent reads, which a hook artifact has no payload for. A declarative gate is still + # artifact: hook -- a rule carrying one would have two answers to what enforces it. + if artifact == "rule" and _script_only_hook(manifest): + allowed = allowed | {"hook"} + present = {k for k in manifest if k in _ALL_PAYLOAD_KEYS} disallowed = present - allowed if disallowed: @@ -51,7 +64,9 @@ def _check_manifest_payload(artifact_dir: Path, manifest: dict[str, Any], report ) return - own = present & allowed + # The script declaration rides along; it is not the artifact's own payload, so it does + # not count toward the one-payload rule it was just admitted past. + own = (present & allowed) - ({"hook"} if artifact == "rule" else set()) if len(own) != 1: report.add( Finding( @@ -68,18 +83,20 @@ def _check_manifest_block_needs_gate(artifact_dir: Path, manifest: dict[str, Any if enforcement not in {"block", "verify"}: return - hook_gate = (manifest.get("hook") or {}).get("gate") + hook = manifest.get("hook") or {} - if hook_gate: - _validate_gate(hook_gate, str(_manifest_ref(artifact_dir)), report, tool_use_allowed=True) + if hook.get("gate"): + _validate_gate(hook["gate"], str(_manifest_ref(artifact_dir)), report, tool_use_allowed=True) return + if hook.get("script"): + return # the script refuses at the declared event; checks_script_events pins it to disk report.add( Finding( str(_manifest_ref(artifact_dir)), "manifest_block_needs_gate", "error", - f"enforcement is '{enforcement}' but no hook.gate definition found", + f"enforcement is '{enforcement}' but neither a hook.gate nor a hook.script was declared", ) ) diff --git a/src/chock/validation/checks_script_events.py b/src/chock/validation/checks_script_events.py new file mode 100644 index 0000000..097f02f --- /dev/null +++ b/src/chock/validation/checks_script_events.py @@ -0,0 +1,60 @@ +"""A declared git-event script exists, and a script on disk is declared.""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +from chock.compile.emitters import GUARD_SUFFIXES, SCRIPT_EVENTS +from chock.manifest import CANONICAL_MANIFEST, resolve_manifest_path +from chock.validation.report import Finding, Report + +_CATEGORY = "manifest_script_events" + + +def script_path(policy_dir: Path, policy_id: str, event: str) -> Path | None: + """The script the hook would run for `event`, or None when the policy ships none.""" + impl = Path(policy_dir) / "implementations" + for suffix in GUARD_SUFFIXES: + candidate = impl / f"{policy_id}-{SCRIPT_EVENTS[event]}{suffix}" + if candidate.exists(): + return candidate + return None + + +def shipped_events(policy_dir: Path, policy_id: str) -> set[str]: + """The events this policy ships a script for, whatever the manifest claims.""" + return {event for event in SCRIPT_EVENTS if script_path(policy_dir, policy_id, event)} + + +def check_script_events(artifact_dir: Path, manifest: dict[str, Any], _artifact_type: str, report: Report) -> None: + """Pin `hook.script.on` to the scripts on disk, in both directions.""" + policy_id = str(manifest.get("id") or Path(artifact_dir).name) + declared = set((manifest.get("hook") or {}).get("script", {}).get("on") or []) + shipped = shipped_events(artifact_dir, policy_id) + if not declared and not shipped: + return + + ref = str(resolve_manifest_path(artifact_dir) or (artifact_dir / CANONICAL_MANIFEST)) + names = {e: f"implementations/{policy_id}-{seg}.{{sh,py}}" for e, seg in SCRIPT_EVENTS.items()} + + for event in sorted(declared - shipped): + report.add( + Finding( + ref, + _CATEGORY, + "error", + f"hook.script declares '{event}' but the policy ships no {names[event]} -- " + "the compiler would wire a hook to a script that is not there", + ) + ) + for event in sorted(shipped - declared): + report.add( + Finding( + ref, + _CATEGORY, + "error", + f"{names[event]} is shipped but hook.script does not declare '{event}' -- " + "nothing wires it, so the policy enforces less than its directory suggests", + ) + ) diff --git a/src/chock/validation/engine.py b/src/chock/validation/engine.py index b3878d7..c40de2b 100644 --- a/src/chock/validation/engine.py +++ b/src/chock/validation/engine.py @@ -46,6 +46,7 @@ check_ambient_token_budget, check_release_consistency, ) +from chock.validation.checks_script_events import check_script_events from chock.validation.checks_security import ( check_ambient_tier, check_effects_and_approval, @@ -121,6 +122,7 @@ def validate_artifact( check_verb_first_naming(artifact_dir, manifest, artifact_type, report) check_manifest_schema(artifact_dir, manifest, artifact_type, report) check_manifest_advice(artifact_dir, manifest, artifact_type, report) + check_script_events(artifact_dir, manifest, artifact_type, report) if registry_check: check_registry_freshness(artifact_dir, manifest, artifact_type, root, report) diff --git a/src/chock/validation/schemas/manifest.hook.json b/src/chock/validation/schemas/manifest.hook.json index 4109440..a3547fe 100644 --- a/src/chock/validation/schemas/manifest.hook.json +++ b/src/chock/validation/schemas/manifest.hook.json @@ -2,12 +2,9 @@ "$schema": "http://json-schema.org/draft-07/schema#", "$id": "https://open-coder-ai.github.io/chock/schemas/v0/manifest.hook.json", "title": "ChockHookPayload", - "description": "V3 hook payload: compiled gate DSL under 'hook.gate'.", + "description": "V3 hook payload: exactly one of the compiled gate DSL under 'hook.gate' or, for a check no declarative kind can express, the event declaration under 'hook.script'.", "type": "object", "additionalProperties": false, - "required": [ - "gate" - ], "properties": { "gate": { "type": "object", @@ -55,6 +52,40 @@ "description": "Per-kind params are validated by checks_gate." } } + }, + "script": { + "type": "object", + "additionalProperties": false, + "description": "A check that cannot be expressed as a gate kind, run by the installed hook from implementations/-.{sh,py}. No kind and no params: the behaviour is the script's. No message either -- the script prints its own refusal, with the detail only it has. 'on' is authoritative: the compiler wires exactly these events, and the validator fails when a declared event has no script or a script is undeclared.", + "required": [ + "on" + ], + "properties": { + "on": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "enum": [ + "commit", + "push" + ] + } + } + } + } + }, + "oneOf": [ + { + "required": [ + "gate" + ] + }, + { + "required": [ + "script" + ] } - } + ] } diff --git a/src/chock/validation/schemas/manifest.schema.json b/src/chock/validation/schemas/manifest.schema.json index 93ab2d7..68d0eeb 100644 --- a/src/chock/validation/schemas/manifest.schema.json +++ b/src/chock/validation/schemas/manifest.schema.json @@ -155,7 +155,7 @@ }, "allOf": [ { - "oneOf": [ + "anyOf": [ { "required": [ "rule" @@ -176,10 +176,11 @@ "workflow" ] } - ] + ], + "$comment": "At least one artifact payload. Exclusivity is checked in code by _check_manifest_payload (manifest_payload), which admits exactly one exception: a rule may carry a hook holding only a script." }, { "$ref": "https://open-coder-ai.github.io/chock/schemas/v0/manifest.artifact-conditionals.json" } ] -} \ No newline at end of file +} diff --git a/tests/test_hook_script_declaration.py b/tests/test_hook_script_declaration.py new file mode 100644 index 0000000..1a8e6ea --- /dev/null +++ b/tests/test_hook_script_declaration.py @@ -0,0 +1,218 @@ +"""A script-backed gate can say what it does, and cannot say it without shipping it.""" + +from __future__ import annotations + +import shutil +from pathlib import Path + +import yaml +from conftest import baseline_policy + +from chock.compile.compiler import compile_policy +from chock.compile.emitters.git_hook import declared_script_events +from chock.compile.surfaces import Surface +from chock.validation.checks_script_events import check_script_events +from chock.validation.engine import validate_artifact +from chock.validation.report import Report + +POLICY_ID = "refuse-erasure" + +_GUARD = '#!/usr/bin/env python3\n"""Refuse nothing; the shape is what is under test."""\n\nraise SystemExit(0)\n' + + +def _manifest(**over: object) -> dict[str, object]: + base: dict[str, object] = { + "id": POLICY_ID, + "name": "Refuse Erasure", + "version": "0.0.1", + "description": "trigger: editing marked files. avoid: erasing the marker.", + "artifact": "rule", + "enforcement": "block", + "rule": {"text": "never(erase): MARKER\nprefer: restoring what the element carried\n"}, + "hook": {"script": {"on": ["commit"]}}, + "provenance": { + "author": "t", + "created_at": "2026-09-10T00:00:00Z", + "updated_at": "2026-09-10T00:00:00Z", + "source_repo": "https://example.invalid/t", + "license": "Apache-2.0", + "trust_tier": "community", + }, + "lifecycle": {"status": "draft"}, + "security": {"content_instructions": "never-obey"}, + } + base.update(over) + return base + + +def _policy(tmp_path: Path, manifest: dict[str, object], *segments: str) -> Path: + policy_dir = tmp_path / ".agents" / "policies" / POLICY_ID + (policy_dir / "implementations").mkdir(parents=True) + (policy_dir / "manifest.yaml").write_text(yaml.safe_dump(manifest), encoding="utf-8") + (policy_dir / "evals").mkdir() + (policy_dir / "evals" / "suite.yaml").write_text( + yaml.safe_dump( + { + "suite": { + "cases": [ + { + "id": "tc-001", + "category": "trigger", + "prompt": "erase the MARKER from page.html", + "expect": "refuses, naming the marker it would lose", + } + ] + } + } + ), + encoding="utf-8", + ) + for segment in segments: + guard = policy_dir / "implementations" / f"{POLICY_ID}-{segment}.py" + guard.write_text(_GUARD, encoding="utf-8") + guard.chmod(0o755) + return policy_dir + + +def _findings(policy_dir: Path, manifest: dict[str, object]) -> list[str]: + report = Report() + check_script_events(policy_dir, manifest, "rule", report) + return [f"{f.check}: {f.message}" for f in report.errors] + + +def test_a_rule_may_declare_the_script_that_enforces_it(tmp_path: Path) -> None: + """The point of the change: an existing, valid rule policy gains a script and stays valid. + + Built from a real baseline policy rather than a hand-written manifest, so what passes here + is the whole validator, not a fixture shaped to satisfy the checks this change touches. + """ + source = baseline_policy("protect-commit-privacy") + policy_dir = tmp_path / ".agents" / "policies" / source.name + shutil.copytree(source, policy_dir) + manifest = yaml.safe_load((policy_dir / "manifest.yaml").read_text(encoding="utf-8")) + assert manifest["artifact"] == "rule", "this test needs a rule artifact to add a hook to" + + manifest["enforcement"] = "block" + manifest["hook"] = {"script": {"on": ["commit"]}} + (policy_dir / "manifest.yaml").write_text(yaml.safe_dump(manifest), encoding="utf-8") + guard = policy_dir / "implementations" / f"{source.name}-pre-commit.py" + guard.write_text(_GUARD, encoding="utf-8") + guard.chmod(0o755) + + report = Report() + validate_artifact("rule", policy_dir, "agnostic", report, Path(tmp_path), registry_check=False) + + assert not report.errors, [f"{f.check}: {f.message}" for f in report.errors] + + +def test_a_declared_event_with_no_script_is_an_error(tmp_path: Path) -> None: + """Otherwise the compiler wires a hook to a file that is not there.""" + manifest = _manifest() + policy_dir = _policy(tmp_path, manifest) # declares commit, ships nothing + + found = _findings(policy_dir, manifest) + assert any("ships no" in f for f in found), found + + +def test_a_script_on_disk_that_nothing_declares_is_an_error(tmp_path: Path) -> None: + """The other drift direction: a policy that looks enforced and is not.""" + manifest = _manifest(hook={"script": {"on": ["commit"]}}) + policy_dir = _policy(tmp_path, manifest, "pre-commit", "pre-push") # push undeclared + + found = _findings(policy_dir, manifest) + assert any("does not declare 'push'" in f for f in found), found + + +def test_the_cross_check_runs_inside_the_validator(tmp_path: Path) -> None: + """Calling the check directly proves it works; only this proves `chock check` runs it.""" + source = baseline_policy("protect-commit-privacy") + policy_dir = tmp_path / ".agents" / "policies" / source.name + shutil.copytree(source, policy_dir) + manifest = yaml.safe_load((policy_dir / "manifest.yaml").read_text(encoding="utf-8")) + manifest["enforcement"] = "block" + manifest["hook"] = {"script": {"on": ["commit"]}} # declared, and deliberately not shipped + (policy_dir / "manifest.yaml").write_text(yaml.safe_dump(manifest), encoding="utf-8") + + report = Report() + validate_artifact("rule", policy_dir, "agnostic", report, Path(tmp_path), registry_check=False) + + assert [f for f in report.errors if f.check == "manifest_script_events"], [ + f"{f.check}: {f.message}" for f in report.errors + ] + + +def test_a_policy_with_neither_is_not_its_business(tmp_path: Path) -> None: + """A rule with no script and no declaration must not be reported about.""" + manifest = _manifest(enforcement="advise") + del manifest["hook"] + assert _findings(_policy(tmp_path, manifest), manifest) == [] + + +def test_enforcement_block_no_longer_demands_a_declarative_gate(tmp_path: Path) -> None: + """The whole reason for the change: manifest_block_needs_gate rejected this shape.""" + manifest = _manifest() + policy_dir = _policy(tmp_path, manifest, "pre-commit") + + report = Report() + validate_artifact("rule", policy_dir, "agnostic", report, Path(tmp_path), registry_check=False) + + assert not [f for f in report.errors if f.check == "manifest_block_needs_gate"] + + +def test_block_with_neither_gate_nor_script_is_still_refused(tmp_path: Path) -> None: + """The check must still catch a policy claiming enforcement it has not wired.""" + manifest = _manifest() + del manifest["hook"] + policy_dir = _policy(tmp_path, manifest) + + report = Report() + validate_artifact("rule", policy_dir, "agnostic", report, Path(tmp_path), registry_check=False) + + assert [f for f in report.errors if f.check == "manifest_block_needs_gate"] + + +def test_a_rule_may_not_carry_a_declarative_gate(tmp_path: Path) -> None: + """Two mechanisms would be two answers to what enforces this policy.""" + manifest = _manifest( + hook={ + "gate": { + "kind": "forbidden_ref", + "on": ["commit"], + "action": "block", + "message": "no", + "params": {"refs": ["main"]}, + } + } + ) + policy_dir = _policy(tmp_path, manifest) + + report = Report() + validate_artifact("rule", policy_dir, "agnostic", report, Path(tmp_path), registry_check=False) + + assert [f for f in report.errors if f.check == "manifest_payload"] + + +def test_the_declaration_decides_what_is_wired_not_the_directory(tmp_path: Path) -> None: + """A script the manifest does not declare must not reach a consumer repo's hooks.""" + manifest = _manifest() + policy_dir = _policy(tmp_path, manifest, "pre-commit", "pre-push") + output_root = tmp_path / ".chock" / "compiled" + + compile_policy(policy_dir, targets=[Surface.GIT_HOOK.value], output_root=output_root) + + git_hook_dir = output_root / POLICY_ID / "git-hook" + assert (git_hook_dir / "git-pre-commit.sh").exists() + assert not (git_hook_dir / "git-pre-push.sh").exists(), "an undeclared event was wired" + + +def test_a_shipped_script_with_no_declaration_wires_nothing(tmp_path: Path) -> None: + """Before this change the filename alone wired the hook; now the manifest must say so.""" + manifest = _manifest(enforcement="advise") + del manifest["hook"] + policy_dir = _policy(tmp_path, manifest, "pre-commit") + output_root = tmp_path / ".chock" / "compiled" + + compile_policy(policy_dir, targets=[Surface.GIT_HOOK.value], output_root=output_root) + + assert declared_script_events(manifest) == [] + assert not (output_root / POLICY_ID / "git-hook").exists() diff --git a/tests/test_manifest_schema.py b/tests/test_manifest_schema.py index 5b3cf3b..1e3e887 100644 --- a/tests/test_manifest_schema.py +++ b/tests/test_manifest_schema.py @@ -114,7 +114,22 @@ def test_wrong_payload_for_artifact_errors() -> None: VALIDATOR.validate(instance=manifest) -def test_two_payloads_error() -> None: +def test_the_schema_admits_a_rule_that_declares_its_script() -> None: + """A rule may carry a hook holding only a script: one control, two surfaces.""" + manifest = {**_rule(), "enforcement": "block", "hook": {"script": {"on": ["commit"]}}} + VALIDATOR.validate(instance=manifest) + + +def test_two_payloads_are_rejected_in_code_not_by_the_schema() -> None: + """Exclusivity moved, it did not go away: `oneOf` cannot admit the one legal pair. + + The schema now says "at least one payload" and `_check_manifest_payload` says which + combinations are legal -- this test pins where the check lives, so a reader who finds the + schema permissive does not conclude nothing enforces it. + """ + from chock.validation.checks_manifest_schema import _check_manifest_payload + from chock.validation.report import Report + manifest = _rule() manifest["hook"] = { "gate": { @@ -125,8 +140,11 @@ def test_two_payloads_error() -> None: "params": {"refs": ["main"]}, } } - with pytest.raises(jsonschema.ValidationError): - VALIDATOR.validate(instance=manifest) + VALIDATOR.validate(instance=manifest) # the schema no longer objects + + report = Report() + _check_manifest_payload(Path("policies/demo-policy"), manifest, report) + assert [f for f in report.errors if f.check == "manifest_payload"], "a rule carrying a gate" def test_unknown_top_level_key_errors() -> None: diff --git a/tests/test_script_backed_commit_gate.py b/tests/test_script_backed_commit_gate.py index a00fc3b..e56a735 100644 --- a/tests/test_script_backed_commit_gate.py +++ b/tests/test_script_backed_commit_gate.py @@ -37,16 +37,18 @@ "name": "Refuse Erasure", "version": "0.0.1", "description": "trigger: editing marked files. avoid: erasing the marker.", - "artifact": "hook", + "artifact": "rule", "enforcement": "block", "rule": {"text": "never(erase): MARKER\n"}, + # The filename alone no longer wires anything: `on` is what the compiler reads. + "hook": {"script": {"on": ["commit"]}}, } -def _policy(tmp_path: Path, name: str, body: str = _GUARD) -> Path: +def _policy(tmp_path: Path, name: str, body: str = _GUARD, manifest: dict | None = None) -> Path: policy_dir = tmp_path / ".agents" / "policies" / POLICY_ID (policy_dir / "implementations").mkdir(parents=True) - (policy_dir / "manifest.yaml").write_text(yaml.safe_dump(_MANIFEST), encoding="utf-8") + (policy_dir / "manifest.yaml").write_text(yaml.safe_dump(manifest or _MANIFEST), encoding="utf-8") guard = policy_dir / "implementations" / name guard.write_text(body, encoding="utf-8") guard.chmod(0o755) @@ -87,9 +89,10 @@ def test_a_declarative_gate_still_wins(tmp_path: Path) -> None: assert '.chock/bin/gate.py" run' in (git_hook_dir / "git-pre-commit.sh").read_text(encoding="utf-8") -def test_an_unrelated_policy_emits_nothing(tmp_path: Path) -> None: - """A rule-only policy must not gain a hook: the fallback keys off the script's name.""" - policy_dir = _policy(tmp_path, f"{POLICY_ID}.py") # a command guard, not an event script +def test_a_command_guard_alone_emits_nothing(tmp_path: Path) -> None: + """A policy shipping only an argv guard declares no event, so no git hook is its business.""" + manifest = {k: v for k, v in _MANIFEST.items() if k != "hook"} | {"enforcement": "advise"} + policy_dir = _policy(tmp_path, f"{POLICY_ID}.py", manifest=manifest) output_root = tmp_path / ".chock" / "compiled" compile_policy(policy_dir, targets=[Surface.GIT_HOOK.value], output_root=output_root)