diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8de5dd3..138195f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,8 @@ jobs: python-version: "3.12" - name: Verify release allowlist and hashes run: python scripts/check_release_bundle.py + - name: Verify unified execute request contract + run: python scripts/check_execute_contract.py - name: Run setup tests run: python -m unittest discover -s keel-setup/tests -p 'test_*.py' - name: Run policy tests diff --git a/SHA256SUMS b/SHA256SUMS index 794fd1a..9a8b4fa 100644 --- a/SHA256SUMS +++ b/SHA256SUMS @@ -1,8 +1,8 @@ -740fe7b07095e90a37f141e2141856cd83bbe8021571078c331ffc3614cb1250 .github/workflows/ci.yml +c25cb1fbeee655efde2fa1b12af630e6cf5657a5717f2ce923affbbbf7008da5 .github/workflows/ci.yml 5d87b13a525cd88e59552924d2b70e1ac4f05408ed88c1f09f2ad073657d938c .gitignore 3c0871d9b30c3e9647426b5b756be7dbfc5eee8728e6f0e0c371385de7c5f8f7 LICENSE 5fa2ec165604ae3612566968097f69d354e1722ccd5e4272793eb533a8097d23 README.md -85941406521e6d6fffcbb079283c230e2a701504b0a3a86b9e20e148efdeb0e4 SOURCE.json +2bb3f3751259b6f45ef5a899491c156c2d73e953d08919af278f056a12cc98fe SOURCE.json 9eb38e845d150019366de6cf74cf5c8b7e7a321e795f13505fb3e0788b043594 keel-policy/SKILL.md 6b449605b6b4fe6eb84f5b12befb90a57b7841d2973bbf9341cf40866e7dd81a keel-policy/examples/README.md 9a13b533704c1a37734bd6c28fc202c7acdaa8335773d55790dac50a693f8b2b keel-policy/examples/stripe-refund-approval-enterprise.json @@ -13,16 +13,19 @@ 4be41a65667952df8105e627026a921b66fc4fef19761242efabe69df0bd47cf keel-policy/reference/policy-document.schema.json 83fd6d47ded91e6b159ca3f2be5c486eb05bad7f289a591654f17141b980de99 keel-policy/scripts/validate_enforceability_report.py 322b430367e859dae341e9c225be34c6c5272c481074e7b1ae2242db527e1cbe keel-policy/tests/test_validate_enforceability_report.py -ab845c8aacdfed484bb95990f85db2d46200d65184df5a5184e24607e38e758d keel-setup/SKILL.md +f2b711de6770931e5286759c94e77b97c17d35db8d3d189c22b2f5a5e7b9bd2e keel-setup/SKILL.md 2b9c2490e921e83b83372a09e422a30f0f7ee338a2809b8e07edbd381d9eb650 keel-setup/reference/coverage.schema.json 5e9c31ef991ec9c4fc2195b4907bacbd9383d47453c6a48a0da90fb158256541 keel-setup/reference/setup-state.schema.json +b0fe7edfc7b0780853c04968ca1caf8cb94731c38e8c56981987c8e334d5c951 keel-setup/reference/unified-execute-request.contract.json fa9e64c1fb3e6851098350d6a83c8cb185c9dea075082b12d7816b166a64f2ad keel-setup/scripts/inventory.py -640f0b4efa46a6e0382ac9dc3038bd5b79aaff2481c3fb59c5175595d84b7393 keel-setup/scripts/setup_state.py -62eea67d8892d8e5dd4552b663176c1ece9dfe636029837c7efc4e1321d8ebbd keel-setup/scripts/verify_execute.py +2786d8a6e89a36cced8af0452f2721ea4fb1b6c7b59a65e3f1ea0ac5f2837b78 keel-setup/scripts/setup_state.py +17dcf121577bd0d01d1912aabd59abb91e0c119ddccb20b3f500b662aedb3174 keel-setup/scripts/verify_execute.py +a891810ef4370ab3107627c972cfd66f18fa9554f5f2ff857312f0756c15a4f7 keel-setup/tests/test_execute_request_contract.py 3e952c3b7ebf382c39140a6a1a5e3bff9458732fe1a0ca9ea63b44206814bb80 keel-setup/tests/test_inventory.py -9d91e7ca7b0d368e3296178505f13e89c8b5f1e01771228a753256e09b6bf7f6 keel-setup/tests/test_setup_state.py -a0732181166dafc8ba7248b0698e4a5ee163f41f3779f7ea5042507747c91b00 keel-setup/tests/test_verify_execute.py -ef030ef0042f765d9d6d08795fb7570978431c4d11e6c939feea50fc9e80a0fe scripts/check_release_bundle.py +25148cb0368c3720b424573f38baf31e2baef2fb1aad9000f5f3eae3453ea374 keel-setup/tests/test_setup_state.py +2ef380a77b3eb9583e71fa99815233f7ffb6a9ab0986d82387853ea26b6117e8 keel-setup/tests/test_verify_execute.py +16b9e69e558e73eb7b32132b3382d5a7621573057fea02b101482fd9311c1bf3 scripts/check_execute_contract.py +4727d8c69971be67eeabf3662fdce88fb987c77fe9f88b507efa4a5d726ddd5e scripts/check_release_bundle.py e0195765e2b9a31938a455fc81f93b02127f7b769948607b19a821f2bd82b2d1 shared/CONSTITUTION.md 6510e9f92c2a8fa3c40a2ca99eca06e2acaad146101c9526e06bb53f18085421 shared/feedback-report.schema.json 92b11e2380bb588b2a00be8d5eae9665b4667747fd044278a65ec2a698de7889 shared/feedback-report.template.md diff --git a/SOURCE.json b/SOURCE.json index c8c6daf..9e74406 100644 --- a/SOURCE.json +++ b/SOURCE.json @@ -8,6 +8,7 @@ "included_files": [ "tools/public_surface.json" ], - "source_commit": "9204f54688d3a3c6e84a3cb7c6c54c3aafd35a09", + "source_commit": "ef6b92880f0729c336b3fad85e256135c91968da", + "source_merge_commit": "165c5f308bb339d8024f9fb1d66956e0940db2e7", "source_repository": "keelapi/keel-skills" } diff --git a/keel-setup/SKILL.md b/keel-setup/SKILL.md index 3ff2ad5..07d05fb 100644 --- a/keel-setup/SKILL.md +++ b/keel-setup/SKILL.md @@ -59,8 +59,9 @@ python3 keel-setup/scripts/setup_state.py --repo-root . --state .keel/setup-stat The helper validates the file against `reference/setup-state.schema.json`, refuses a file carrying a bearer value, a credential assignment, a known credential prefix, an over-long string, or a -mapping-authority field, and reports what the return loop is due to do. Exit `1` means the state was -refused or the path is not ignored by git. +mapping-authority field, increments once, and atomically persists the validated next state only after +git confirms the exact path is ignored. It reports what the return loop is due to do. Exit `1` means +the state was refused, the ignore status could not be established, or the atomic write failed. If the file is missing, invalid, or refused, say that continuity was lost, start at invocation 1, and do not infer prior success from it. The count is local workflow state, not Keel evidence: it records @@ -161,6 +162,10 @@ Run: python3 keel-setup/scripts/verify_execute.py --provider PROVIDER --allow-model ALLOWED --deny-model DENIED ``` +Retain the script's non-secret `request_id` and `permit_id` values for exact dashboard and Permit +matching. If either is null, report that exact correlation is unavailable; never infer it from a +nearby model name or timestamp. + The helper reads the key only from its environment. It generates an integer timestamp and a distinct nonce inside each request attempt, sends `input.messages`, and prints only bounded classification fields. Exit `0` means `allowed_completed` followed by `keel_denied`; exit `1` means requests completed diff --git a/keel-setup/reference/unified-execute-request.contract.json b/keel-setup/reference/unified-execute-request.contract.json new file mode 100644 index 0000000..24841a7 --- /dev/null +++ b/keel-setup/reference/unified-execute-request.contract.json @@ -0,0 +1,72 @@ +{ + "contract_schema_version": "keel.public_unified_execute_request_contract.v1", + "helper_request_keys": [ + "input", + "model", + "provider" + ], + "schema": { + "additionalProperties": false, + "properties": { + "action_verb": { + "default": "ai.generate", + "description": "Action verb this request executes. Defaults to ai.generate for backward compatibility.", + "enum": [ + "ai.generate", + "payment.execute" + ], + "title": "Action Verb", + "type": "string" + }, + "budget_envelope_id": { + "anyOf": [ + { + "format": "uuid", + "type": "string" + }, + { + "type": "null" + } + ], + "description": "Optional Keel budget envelope bound to this execution. When set, the execution permit reserves against the envelope before dispatch.", + "title": "Budget Envelope Id" + }, + "input": { + "additionalProperties": true, + "title": "Input", + "type": "object" + }, + "model": { + "maxLength": 128, + "minLength": 1, + "title": "Model", + "type": "string" + }, + "provider": { + "anyOf": [ + { + "maxLength": 64, + "minLength": 1, + "type": "string" + }, + { + "type": "null" + } + ], + "title": "Provider" + } + }, + "required": [ + "model", + "input" + ], + "title": "UnifiedExecuteRequest", + "type": "object" + }, + "schema_sha256": "67beb5283aa68267eeaaa52bfcc4e7d799fe61d5e9f194445e8230de44478213", + "source_artifact": "docs/public-artifacts/openapi.json", + "source_artifact_sha256": "40f9b7ef1ad7ab38409c6b97d36ec654afc94a40f98475b6cda06b331b355db9", + "source_commit": "4ce5f5def6258004861dfaeeef512e49afc87cfd", + "source_merge_commit": "c130f73d966fce6572aa5dc8ff48164b9e9dbf15", + "source_repository": "keelapi/keel-api" +} diff --git a/keel-setup/scripts/setup_state.py b/keel-setup/scripts/setup_state.py index bbd2dc6..26dcb05 100644 --- a/keel-setup/scripts/setup_state.py +++ b/keel-setup/scripts/setup_state.py @@ -12,11 +12,14 @@ from __future__ import annotations import argparse +import datetime as dt import json +import os import pathlib import re import subprocess import sys +import tempfile from typing import Any ROOT = pathlib.Path(__file__).resolve().parents[2] @@ -59,7 +62,9 @@ "carries a credential assignment", ), ( - re.compile(r"^(?:ks_|sk-|sk_|pk_|rk_|ghp_|gho_|xox[baprs]-)[A-Za-z0-9_\-]{8,}"), + re.compile( + r"(? dict[str, Any]: +def _utc_now() -> str: + return dt.datetime.now(dt.timezone.utc).isoformat(timespec="seconds").replace("+00:00", "Z") + + +def initial_state(*, updated_at: str | None = None) -> dict[str, Any]: return { "schema_version": SCHEMA_VERSION, "invocation_count": 1, "stage": "discovery", - "updated_at": None, + "updated_at": updated_at or _utc_now(), } @@ -193,34 +202,70 @@ def is_git_ignored(repo_root: pathlib.Path, relative: pathlib.PurePosixPath) -> return None +def write_state_atomically(state_path: pathlib.Path, state: dict[str, Any]) -> None: + """Persist validated continuity without ever exposing a partial JSON file.""" + + failures = validate_state(state) + if failures: + raise ValueError("refusing to persist invalid local setup state: " + "; ".join(failures)) + + state_path.parent.mkdir(parents=True, exist_ok=True) + temporary_path: pathlib.Path | None = None + try: + with tempfile.NamedTemporaryFile( + mode="w", + encoding="utf-8", + dir=state_path.parent, + prefix=f".{state_path.name}.", + suffix=".tmp", + delete=False, + ) as handle: + temporary_path = pathlib.Path(handle.name) + os.chmod(temporary_path, 0o600) + json.dump(state, handle, indent=2, sort_keys=True) + handle.write("\n") + handle.flush() + os.fsync(handle.fileno()) + os.replace(temporary_path, state_path) + temporary_path = None + finally: + if temporary_path is not None: + try: + temporary_path.unlink() + except FileNotFoundError: + pass + + def begin(state_path: pathlib.Path, repo_root: pathlib.Path) -> dict[str, Any]: - """Read the state once, increment the count once, and report what is due.""" + """Read, advance, and atomically persist one local invocation.""" + now = _utc_now() continuity = "resumed" continuity_reason: str | None = None problems: list[str] = [] try: raw = state_path.read_text(encoding="utf-8") except OSError: - state = initial_state() + state = initial_state(updated_at=now) continuity = "lost" continuity_reason = "no local state file was readable at this path" else: try: loaded = json.loads(raw) except json.JSONDecodeError as exc: - state = initial_state() + state = initial_state(updated_at=now) continuity = "lost" continuity_reason = f"local state file is not valid JSON: {exc}" else: problems = validate_state(loaded) if problems: - state = initial_state() + state = initial_state(updated_at=now) continuity = "lost" continuity_reason = "local state file was refused; see refusals" else: state = dict(loaded) state["invocation_count"] = int(state["invocation_count"]) + 1 + state["updated_at"] = now stage = state.get("stage") relative = state_path @@ -230,6 +275,20 @@ def begin(state_path: pathlib.Path, repo_root: pathlib.Path) -> dict[str, Any]: relative = state_path ignored = is_git_ignored(repo_root, pathlib.PurePosixPath(pathlib.PurePath(relative).as_posix())) + state_persisted = False + persistence_error: str | None = None + if ignored is True: + try: + write_state_atomically(state_path, state) + except (OSError, ValueError) as exc: + persistence_error = f"local state was not persisted: {exc}" + else: + state_persisted = True + elif ignored is False: + persistence_error = "local state path is not ignored by git" + else: + persistence_error = "could not establish that the local state path is ignored by git" + return { "schema_version": SCHEMA_VERSION, "continuity": continuity, @@ -244,6 +303,8 @@ def begin(state_path: pathlib.Path, repo_root: pathlib.Path) -> dict[str, Any]: state.get("last_maintenance_review_invocation"), ), "state_path_git_ignored": ignored, + "state_persisted": state_persisted, + "persistence_error": persistence_error, "evidence_level": "unresolved", "does_not_establish": list(DOES_NOT_ESTABLISH), } @@ -287,6 +348,9 @@ def main(argv: list[str] | None = None) -> int: file=sys.stderr, ) return 1 + if not report["state_persisted"]: + print(report["persistence_error"] or "local state was not persisted", file=sys.stderr) + return 1 return 0 diff --git a/keel-setup/scripts/verify_execute.py b/keel-setup/scripts/verify_execute.py index eaa1036..f814fbd 100644 --- a/keel-setup/scripts/verify_execute.py +++ b/keel-setup/scripts/verify_execute.py @@ -9,18 +9,22 @@ import argparse import json import os +import re import secrets import sys import time import urllib.error import urllib.parse import urllib.request +import uuid from typing import Any DEFAULT_BASE_URL = "https://api.keelapi.com" OUTPUT_FIELDS = ( "model", "expectation", + "request_id", + "permit_id", "http_status", "body_status", "governance_decision", @@ -29,6 +33,18 @@ "classification", ) +CORRELATION_HEADERS = { + "request_id": "X-Keel-Request-ID", + "permit_id": "X-Keel-Permit-ID", +} + +MAX_RESPONSE_BYTES = 64 * 1024 +_ULID_PATTERN = re.compile(r"[0-9A-HJKMNP-TV-Z]{26}", re.IGNORECASE) +_SAFE_ERROR_CODE_PATTERN = re.compile(r"[a-z][a-z0-9_.-]{0,127}") +_BODY_STATUSES = frozenset({"completed", "denied", "failed"}) +_GOVERNANCE_DECISIONS = frozenset({"allow", "deny"}) +_ERROR_STAGES = frozenset({"permit", "dispatch"}) + def _nonempty(value: str) -> str: value = value.strip() @@ -84,10 +100,10 @@ def classify(http_status: int | None, body: dict[str, Any] | None) -> dict[str, body = body or {} governance = body.get("governance") if isinstance(body.get("governance"), dict) else {} error = body.get("error") if isinstance(body.get("error"), dict) else {} - status = body.get("status") if isinstance(body.get("status"), str) else None - decision = governance.get("decision") if isinstance(governance.get("decision"), str) else None - stage = error.get("stage") if isinstance(error.get("stage"), str) else None - code = error.get("code") if isinstance(error.get("code"), str) else None + status = _allowlisted_scalar(body.get("status"), _BODY_STATUSES) + decision = _allowlisted_scalar(governance.get("decision"), _GOVERNANCE_DECISIONS) + stage = _allowlisted_scalar(error.get("stage"), _ERROR_STAGES) + code = _safe_error_code(error.get("code")) if http_status == 200 and status == "completed" and decision == "allow": result = "allowed_completed" @@ -117,6 +133,22 @@ def classify(http_status: int | None, body: dict[str, Any] | None) -> dict[str, } +def _allowlisted_scalar(value: Any, allowed: frozenset[str]) -> str | None: + """Return only a response scalar from a closed protocol vocabulary.""" + + if not isinstance(value, str) or value not in allowed: + return None + return value + + +def _safe_error_code(value: Any) -> str | None: + """Return one bounded protocol error code, never arbitrary response text.""" + + if not isinstance(value, str) or _SAFE_ERROR_CODE_PATTERN.fullmatch(value) is None: + return None + return value + + def execute_attempt( *, base_url: str, key: str, provider: str, model: str, expectation: str, timeout: float = 10.0 ) -> dict[str, Any]: @@ -127,7 +159,6 @@ def execute_attempt( { "provider": provider, "model": model, - "operation": "generate.text", "input": {"messages": [{"role": "user", "content": "Reply with OK."}]}, }, separators=(",", ":"), @@ -145,13 +176,16 @@ def execute_attempt( ) http_status: int | None raw: bytes + response_headers: Any = None try: with _open(request, timeout=timeout) as response: http_status = response.status - raw = response.read() + response_headers = response.headers + raw = _read_bounded(response) except urllib.error.HTTPError as exc: http_status = exc.code - raw = exc.read() + response_headers = exc.headers + raw = _read_bounded(exc) except (urllib.error.URLError, TimeoutError, OSError): http_status = None raw = b"" @@ -165,10 +199,44 @@ def execute_attempt( except (UnicodeDecodeError, json.JSONDecodeError): body = None result = classify(http_status, body) - result.update({"model": model, "expectation": expectation}) + correlation = { + field: _correlation_header(response_headers, field=field, name=header) + for field, header in CORRELATION_HEADERS.items() + } + result.update({"model": model, "expectation": expectation, **correlation}) return {field: result.get(field) for field in OUTPUT_FIELDS} +def _read_bounded(response: Any) -> bytes: + """Read one response body up to the verification helper's fixed ceiling.""" + + raw = response.read(MAX_RESPONSE_BYTES + 1) + if len(raw) > MAX_RESPONSE_BYTES: + return b"" + return raw + + +def _correlation_header(headers: Any, *, field: str, name: str) -> str | None: + """Return only a correlation value in the format emitted by ``/v1/execute``.""" + + if headers is None or not hasattr(headers, "get"): + return None + value = headers.get(name) + if not isinstance(value, str): + return None + value = value.strip() + if field == "request_id": + return value if _ULID_PATTERN.fullmatch(value) is not None else None + if field != "permit_id": + return None + try: + parsed = uuid.UUID(value) + except ValueError: + return None + canonical = str(parsed) + return canonical if value.lower() == canonical else None + + def redact_record(record: dict[str, Any], secret: str) -> dict[str, Any]: """Remove the environment secret even if an upstream field unexpectedly echoes it.""" redacted: dict[str, Any] = {} diff --git a/keel-setup/tests/test_execute_request_contract.py b/keel-setup/tests/test_execute_request_contract.py new file mode 100644 index 0000000..a7800fb --- /dev/null +++ b/keel-setup/tests/test_execute_request_contract.py @@ -0,0 +1,83 @@ +from __future__ import annotations + +import importlib.util +import json +import pathlib +import tempfile +import unittest + + +ROOT = pathlib.Path(__file__).resolve().parents[2] +CHECKER = ROOT / "scripts" / "check_execute_contract.py" +SPEC = importlib.util.spec_from_file_location("check_execute_contract", CHECKER) +check_execute_contract = importlib.util.module_from_spec(SPEC) +assert SPEC.loader +SPEC.loader.exec_module(check_execute_contract) + + +class ExecuteRequestContractTest(unittest.TestCase): + def test_shipped_helper_matches_the_pinned_api_schema(self): + check_execute_contract.validate() + + def test_extra_operation_field_is_refused_even_if_the_snapshot_is_rehashed(self): + source = check_execute_contract.HELPER.read_text(encoding="utf-8") + mutated = source.replace( + ' "provider": provider,\n', + ' "provider": provider,\n "operation": "generate.text",\n', + 1, + ) + self.assertNotEqual(source, mutated) + with tempfile.TemporaryDirectory() as directory: + helper = pathlib.Path(directory) / "verify_execute.py" + helper.write_text(mutated, encoding="utf-8") + with self.assertRaisesRegex(ValueError, "helper request keys changed"): + check_execute_contract.validate(helper_path=helper) + + def test_missing_required_input_is_refused(self): + source = check_execute_contract.HELPER.read_text(encoding="utf-8") + mutated = source.replace( + ' "input": {"messages": [{"role": "user", "content": "Reply with OK."}]},\n', + "", + 1, + ) + self.assertNotEqual(source, mutated) + with tempfile.TemporaryDirectory() as directory: + helper = pathlib.Path(directory) / "verify_execute.py" + helper.write_text(mutated, encoding="utf-8") + with self.assertRaisesRegex(ValueError, "helper request keys changed"): + check_execute_contract.validate(helper_path=helper) + + def test_snapshot_drift_from_a_supplied_openapi_is_refused(self): + contract = json.loads(check_execute_contract.CONTRACT.read_text(encoding="utf-8")) + openapi = { + "components": { + "schemas": { + "UnifiedExecuteRequest": contract["schema"], + } + } + } + with tempfile.TemporaryDirectory() as directory: + artifact = pathlib.Path(directory) / "openapi.json" + artifact.write_text(json.dumps(openapi), encoding="utf-8") + with self.assertRaisesRegex(ValueError, "OpenAPI digest differs"): + check_execute_contract.validate(openapi_path=artifact) + + def test_source_and_merge_provenance_are_exact_pins(self): + contract = json.loads(check_execute_contract.CONTRACT.read_text(encoding="utf-8")) + mutations = ( + ("source_commit", "0" * 40, "reviewed API source commit"), + ("source_merge_commit", "1" * 40, "merged API commit"), + ("source_artifact_sha256", "2" * 64, "merged OpenAPI digest"), + ) + for field, value, message in mutations: + with self.subTest(field=field), tempfile.TemporaryDirectory() as directory: + mutated = dict(contract) + mutated[field] = value + path = pathlib.Path(directory) / "contract.json" + path.write_text(json.dumps(mutated), encoding="utf-8") + with self.assertRaisesRegex(ValueError, message): + check_execute_contract.validate(contract_path=path) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/keel-setup/tests/test_setup_state.py b/keel-setup/tests/test_setup_state.py index 69ed5a9..3666fcd 100644 --- a/keel-setup/tests/test_setup_state.py +++ b/keel-setup/tests/test_setup_state.py @@ -8,6 +8,7 @@ import subprocess import tempfile import unittest +from unittest import mock SCRIPT = pathlib.Path(__file__).resolve().parents[1] / "scripts" / "setup_state.py" SPEC = importlib.util.spec_from_file_location("setup_state", SCRIPT) @@ -23,7 +24,7 @@ def _valid(**overrides): "stage": "integration_ready", "provider": "openai", "allowed_model": "gpt-4o-mini", - "denied_model": "gpt-4o", + "denied_model": "o4-mini", "updated_at": "2026-08-28T10:00:00Z", } state.update(overrides) @@ -63,7 +64,14 @@ def test_credential_assignment_is_refused(self): self.assertTrue(any("credential assignment" in item for item in failures), failures) def test_known_credential_prefix_is_refused(self): - for value in ("ks_live_abcdefgh", "sk-abcdefghijkl", "ghp_abcdefghijkl"): + for value in ( + "ks_live_abcdefgh", + "sk-abcdefghijkl", + "ghp_abcdefghijkl", + " ks_live_abcdefgh", + "credential value ks_live_abcdefgh", + "config/ks_live_abcdefgh", + ): with self.subTest(value=value): failures = setup_state.refusals(_valid(pinned_skill_ref=value)) self.assertTrue(any("credential prefix" in item for item in failures), failures) @@ -139,10 +147,14 @@ def _repo(self, directory: str) -> pathlib.Path: def test_missing_file_reports_lost_continuity_and_starts_at_one(self): with tempfile.TemporaryDirectory() as directory: root = self._repo(directory) - report = setup_state.begin(root / ".keel" / "setup-state.json", root) + path = root / ".keel" / "setup-state.json" + report = setup_state.begin(path, root) + stored = json.loads(path.read_text()) self.assertEqual(report["continuity"], "lost") self.assertEqual(report["invocation_count"], 1) self.assertEqual(report["due"], []) + self.assertTrue(report["state_persisted"]) + self.assertEqual(stored["invocation_count"], 1) self.assertIn("prior_run_success", report["does_not_establish"]) self.assertEqual(report["evidence_level"], "unresolved") @@ -152,9 +164,13 @@ def test_invalid_json_reports_lost_continuity_without_inferring_success(self): path = root / ".keel" / "setup-state.json" path.write_text("{not json") report = setup_state.begin(path, root) + stored = json.loads(path.read_text()) self.assertEqual(report["continuity"], "lost") self.assertEqual(report["invocation_count"], 1) self.assertEqual(report["stage"], "discovery") + self.assertEqual(setup_state.validate_state(stored), []) + self.assertEqual(stored["invocation_count"], 1) + self.assertEqual(stored["stage"], "discovery") def test_refused_file_reports_lost_continuity_and_the_reasons(self): with tempfile.TemporaryDirectory() as directory: @@ -162,9 +178,13 @@ def test_refused_file_reports_lost_continuity_and_the_reasons(self): path = root / ".keel" / "setup-state.json" path.write_text(json.dumps(_valid(pinned_skill_ref="Bearer ks_live_abcdefgh"))) report = setup_state.begin(path, root) + stored = json.loads(path.read_text()) self.assertEqual(report["continuity"], "lost") self.assertEqual(report["invocation_count"], 1) self.assertTrue(report["refusals"]) + self.assertEqual(setup_state.validate_state(stored), []) + self.assertEqual(stored["invocation_count"], 1) + self.assertEqual(stored["stage"], "discovery") def test_valid_file_increments_exactly_once(self): with tempfile.TemporaryDirectory() as directory: @@ -173,11 +193,23 @@ def test_valid_file_increments_exactly_once(self): path.write_text(json.dumps(_valid(invocation_count=4, stage="state_d_verified"))) report = setup_state.begin(path, root) again = setup_state.begin(path, root) + stored = json.loads(path.read_text()) self.assertEqual(report["continuity"], "resumed") self.assertEqual(report["invocation_count"], 5) self.assertIn("drift_audit", report["due"]) - # begin() never writes, so the count advances only when the caller stores it. - self.assertEqual(again["invocation_count"], 5) + self.assertEqual(again["invocation_count"], 6) + self.assertEqual(stored["invocation_count"], 6) + + def test_persisted_state_is_private_and_leaves_no_temporary_file(self): + with tempfile.TemporaryDirectory() as directory: + root = self._repo(directory) + path = root / ".keel" / "setup-state.json" + report = setup_state.begin(path, root) + mode = path.stat().st_mode & 0o777 + temporary_files = list(path.parent.glob(f".{path.name}.*.tmp")) + self.assertTrue(report["state_persisted"]) + self.assertEqual(mode, 0o600) + self.assertEqual(temporary_files, []) def test_state_f_stage_is_reported_unsupported_on_this_revision(self): with tempfile.TemporaryDirectory() as directory: @@ -195,10 +227,31 @@ def test_untracked_ignore_status_is_reported(self): path = root / ".keel" / "setup-state.json" path.write_text(json.dumps(_valid())) report = setup_state.begin(path, root) + stored = json.loads(path.read_text()) self.assertFalse(report["state_path_git_ignored"]) + self.assertFalse(report["state_persisted"]) + self.assertEqual(stored["invocation_count"], 3) class CommandLineTest(unittest.TestCase): + def test_main_persists_each_successful_invocation(self): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + (root / ".keel").mkdir() + (root / ".gitignore").write_text(".keel/\n") + subprocess.run(["git", "init", "-q", str(root)], check=True, capture_output=True) + path = root / ".keel" / "setup-state.json" + reports = [] + for _ in range(2): + with contextlib.redirect_stdout(io.StringIO()) as stdout: + code = setup_state.main(["--state", str(path), "--repo-root", str(root)]) + self.assertEqual(code, 0) + reports.append(json.loads(stdout.getvalue())) + stored = json.loads(path.read_text()) + self.assertEqual([item["invocation_count"] for item in reports], [1, 2]) + self.assertTrue(all(item["state_persisted"] for item in reports)) + self.assertEqual(stored["invocation_count"], 2) + def test_unignored_state_path_exits_nonzero(self): with tempfile.TemporaryDirectory() as directory: root = pathlib.Path(directory) @@ -210,6 +263,26 @@ def test_unignored_state_path_exits_nonzero(self): code = setup_state.main(["--state", str(path), "--repo-root", str(root)]) self.assertEqual(code, 1) + def test_write_failure_exits_nonzero_without_claiming_persistence(self): + with tempfile.TemporaryDirectory() as directory: + root = pathlib.Path(directory) + (root / ".keel").mkdir() + (root / ".gitignore").write_text(".keel/\n") + subprocess.run(["git", "init", "-q", str(root)], check=True, capture_output=True) + path = root / ".keel" / "setup-state.json" + with ( + mock.patch.object( + setup_state, + "write_state_atomically", + side_effect=OSError("simulated write failure"), + ), + contextlib.redirect_stdout(io.StringIO()) as stdout, + contextlib.redirect_stderr(io.StringIO()), + ): + code = setup_state.main(["--state", str(path), "--repo-root", str(root)]) + self.assertEqual(code, 1) + self.assertFalse(json.loads(stdout.getvalue())["state_persisted"]) + def test_validate_only_rejects_a_refused_file(self): with tempfile.TemporaryDirectory() as directory: path = pathlib.Path(directory) / "setup-state.json" diff --git a/keel-setup/tests/test_verify_execute.py b/keel-setup/tests/test_verify_execute.py index 9782daa..37cd0ad 100644 --- a/keel-setup/tests/test_verify_execute.py +++ b/keel-setup/tests/test_verify_execute.py @@ -12,6 +12,7 @@ import unittest import urllib.error import urllib.request +import uuid from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer from unittest import mock @@ -24,17 +25,26 @@ class _Handler(BaseHTTPRequestHandler): requests: list[dict] = [] - responses: list[tuple[int, object]] = [] + responses: list[tuple[int, object] | tuple[int, object, dict[str, str]]] = [] def do_POST(self): # noqa: N802 length = int(self.headers["Content-Length"]) body = json.loads(self.rfile.read(length)) type(self).requests.append({"path": self.path, "headers": dict(self.headers), "body": body}) - status, response = type(self).responses.pop(0) + queued = type(self).responses.pop(0) + status, response = queued[:2] + extra_headers = queued[2] if len(queued) == 3 else {} raw = response if isinstance(response, bytes) else json.dumps(response).encode() self.send_response(status) self.send_header("Content-Type", "application/json") self.send_header("Content-Length", str(len(raw))) + request_number = len(type(self).requests) + default_headers = { + "X-Keel-Request-ID": f"01K4{request_number:022d}", + "X-Keel-Permit-ID": str(uuid.UUID(int=request_number)), + } + for name, value in {**default_headers, **extra_headers}.items(): + self.send_header(name, value) self.end_headers() self.wfile.write(raw) @@ -96,6 +106,98 @@ def test_expected_pair_uses_unique_freshness_and_messages_without_leak(self): self.assertTrue(all(len(item) >= 16 for item in nonces)) self.assertTrue(all(item["headers"]["X-Keel-Timestamp"].isdigit() for item in _Handler.requests)) self.assertTrue(all("messages" in item["body"]["input"] and "text" not in item["body"]["input"] for item in _Handler.requests)) + self.assertTrue( + all(set(item["body"]) == {"provider", "model", "input"} for item in _Handler.requests) + ) + self.assertTrue(all("operation" not in item["body"] for item in _Handler.requests)) + records = [json.loads(line) for line in out.getvalue().splitlines()] + self.assertEqual( + [(item["request_id"], item["permit_id"]) for item in records], + [ + ("01K40000000000000000000001", "00000000-0000-0000-0000-000000000001"), + ("01K40000000000000000000002", "00000000-0000-0000-0000-000000000002"), + ], + ) + + def test_missing_or_oversized_correlation_headers_do_not_become_evidence(self): + self.assertIsNone( + verify_execute._correlation_header( + None, + field="request_id", + name="X-Keel-Request-ID", + ) + ) + self.assertIsNone( + verify_execute._correlation_header( + {"X-Keel-Request-ID": "x" * 257}, + field="request_id", + name="X-Keel-Request-ID", + ) + ) + + def test_split_secret_in_correlation_headers_is_not_printed(self): + sentinel = "ks_live_abcdefghijklmnop" + first, second = sentinel[:12], sentinel[12:] + allow = {"status": "completed", "governance": {"decision": "allow"}} + deny = { + "status": "denied", + "governance": {"decision": "deny"}, + "error": {"stage": "permit", "code": "policy.rule_denied"}, + } + injected = { + "X-Keel-Request-ID": first, + "X-Keel-Permit-ID": second, + } + with server([(200, allow, injected), (403, deny, injected)]) as base_url: + out, err = io.StringIO(), io.StringIO() + with ( + mock.patch.dict(os.environ, {"KEEL_API_KEY": sentinel}, clear=False), + contextlib.redirect_stdout(out), + contextlib.redirect_stderr(err), + ): + code = verify_execute.main( + [ + "--provider", + "test", + "--allow-model", + "small", + "--deny-model", + "large", + "--base-url", + base_url, + ] + ) + emitted = out.getvalue() + err.getvalue() + self.assertEqual(code, 0) + self.assertNotIn(first, emitted) + self.assertNotIn(second, emitted) + self.assertNotIn(sentinel, emitted) + + def test_response_body_over_the_fixed_limit_is_refused_without_printing_it(self): + marker = "response-content-must-not-print" + raw = (marker + "x" * verify_execute.MAX_RESPONSE_BYTES).encode() + with server([(502, raw)]) as base_url: + result = verify_execute.execute_attempt( + base_url=base_url, + key="redacted-test-value", + provider="test", + model="model", + expectation="allow", + ) + self.assertEqual(result["classification"], "malformed_response") + self.assertNotIn(marker, json.dumps(result)) + + def test_response_scalars_are_closed_or_bounded_protocol_values(self): + body = { + "status": "credential fragment", + "governance": {"decision": "credential fragment"}, + "error": {"stage": "credential fragment", "code": "UPPERCASE secret"}, + } + result = verify_execute.classify(500, body) + self.assertIsNone(result["body_status"]) + self.assertIsNone(result["governance_decision"]) + self.assertIsNone(result["error_stage"]) + self.assertIsNone(result["error_code"]) def test_provider_refusal_at_401_and_403_is_not_denial(self): for status in (401, 403): diff --git a/scripts/check_execute_contract.py b/scripts/check_execute_contract.py new file mode 100644 index 0000000..75cc198 --- /dev/null +++ b/scripts/check_execute_contract.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python3 +"""Verify the public helper against the pinned unified-execute request contract.""" + +from __future__ import annotations + +import argparse +import ast +import hashlib +import json +import pathlib +from typing import Any + + +ROOT = pathlib.Path(__file__).resolve().parents[1] +CONTRACT = ROOT / "keel-setup/reference/unified-execute-request.contract.json" +HELPER = ROOT / "keel-setup/scripts/verify_execute.py" +EXPECTED_CONTRACT_VERSION = "keel.public_unified_execute_request_contract.v1" +EXPECTED_API_REPOSITORY = "keelapi/keel-api" +EXPECTED_API_SOURCE_COMMIT = "4ce5f5def6258004861dfaeeef512e49afc87cfd" +EXPECTED_API_MERGE_COMMIT = "c130f73d966fce6572aa5dc8ff48164b9e9dbf15" +EXPECTED_API_OPENAPI_SHA256 = "40f9b7ef1ad7ab38409c6b97d36ec654afc94a40f98475b6cda06b331b355db9" + + +def _canonical_sha256(value: Any) -> str: + encoded = json.dumps( + value, + ensure_ascii=False, + separators=(",", ":"), + sort_keys=True, + ).encode("utf-8") + return hashlib.sha256(encoded).hexdigest() + + +def _file_sha256(path: pathlib.Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + +def _helper_request_keys(path: pathlib.Path) -> set[str]: + """Read the literal JSON payload keys from execute_attempt without importing it.""" + + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + functions = [ + node + for node in tree.body + if isinstance(node, ast.FunctionDef) and node.name == "execute_attempt" + ] + if len(functions) != 1: + raise ValueError("verify_execute.py must define exactly one execute_attempt") + + payloads: list[ast.Dict] = [] + for node in ast.walk(functions[0]): + if not isinstance(node, ast.Call) or not isinstance(node.func, ast.Attribute): + continue + if not ( + isinstance(node.func.value, ast.Name) + and node.func.value.id == "json" + and node.func.attr == "dumps" + ): + continue + if node.args and isinstance(node.args[0], ast.Dict): + payloads.append(node.args[0]) + if len(payloads) != 1: + raise ValueError("execute_attempt must JSON-encode exactly one literal request object") + + keys: set[str] = set() + for key in payloads[0].keys: + if not isinstance(key, ast.Constant) or not isinstance(key.value, str): + raise ValueError("execute request keys must be string literals") + keys.add(key.value) + return keys + + +def validate( + *, + contract_path: pathlib.Path = CONTRACT, + helper_path: pathlib.Path = HELPER, + openapi_path: pathlib.Path | None = None, +) -> None: + contract = json.loads(contract_path.read_text(encoding="utf-8")) + if contract.get("contract_schema_version") != EXPECTED_CONTRACT_VERSION: + raise ValueError("unknown unified-execute contract snapshot version") + if contract.get("source_repository") != EXPECTED_API_REPOSITORY: + raise ValueError("unified-execute contract must name keelapi/keel-api") + source_commit = contract.get("source_commit") + if source_commit != EXPECTED_API_SOURCE_COMMIT: + raise ValueError("unified-execute contract does not pin the reviewed API source commit") + source_merge_commit = contract.get("source_merge_commit") + if source_merge_commit != EXPECTED_API_MERGE_COMMIT: + raise ValueError("unified-execute contract does not pin the merged API commit") + if contract.get("source_artifact_sha256") != EXPECTED_API_OPENAPI_SHA256: + raise ValueError("unified-execute contract does not pin the merged OpenAPI digest") + + schema = contract.get("schema") + if not isinstance(schema, dict): + raise ValueError("unified-execute contract has no schema object") + if _canonical_sha256(schema) != contract.get("schema_sha256"): + raise ValueError("unified-execute schema digest mismatch") + if schema.get("additionalProperties") is not False: + raise ValueError("UnifiedExecuteRequest must continue to forbid extra fields") + + properties = schema.get("properties") + required = schema.get("required") + declared_helper_keys = contract.get("helper_request_keys") + if not isinstance(properties, dict) or not isinstance(required, list): + raise ValueError("unified-execute schema is missing properties or required") + if not isinstance(declared_helper_keys, list) or not all( + isinstance(item, str) for item in declared_helper_keys + ): + raise ValueError("helper_request_keys must be a list of strings") + + emitted = _helper_request_keys(helper_path) + declared = set(declared_helper_keys) + allowed = set(properties) + required_keys = set(required) + if emitted != declared: + raise ValueError( + f"helper request keys changed: emitted={sorted(emitted)} snapshot={sorted(declared)}" + ) + if not emitted <= allowed: + raise ValueError(f"helper emits fields forbidden by UnifiedExecuteRequest: {sorted(emitted - allowed)}") + if not required_keys <= emitted: + raise ValueError(f"helper omits required UnifiedExecuteRequest fields: {sorted(required_keys - emitted)}") + if "operation" in emitted: + raise ValueError("the unified execute helper must not send a top-level operation") + + if openapi_path is not None: + openapi = json.loads(openapi_path.read_text(encoding="utf-8")) + live_schema = openapi.get("components", {}).get("schemas", {}).get("UnifiedExecuteRequest") + if live_schema != schema: + raise ValueError("committed UnifiedExecuteRequest snapshot differs from supplied OpenAPI") + if _file_sha256(openapi_path) != contract.get("source_artifact_sha256"): + raise ValueError("supplied OpenAPI digest differs from the pinned API artifact") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument( + "--openapi", + type=pathlib.Path, + help="optional local keel-api OpenAPI artifact for cross-repository verification", + ) + args = parser.parse_args(argv) + validate(openapi_path=args.openapi) + print("PASS: verify_execute request matches pinned UnifiedExecuteRequest") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/check_release_bundle.py b/scripts/check_release_bundle.py index 2a71c2a..7310609 100644 --- a/scripts/check_release_bundle.py +++ b/scripts/check_release_bundle.py @@ -11,7 +11,8 @@ ROOT = Path(__file__).resolve().parents[1] -SOURCE_COMMIT = "9204f54688d3a3c6e84a3cb7c6c54c3aafd35a09" +SOURCE_COMMIT = "ef6b92880f0729c336b3fad85e256135c91968da" +SOURCE_MERGE_COMMIT = "165c5f308bb339d8024f9fb1d66956e0940db2e7" EXPECTED_FILES = { ".github/workflows/ci.yml", ".gitignore", @@ -32,13 +33,16 @@ "keel-setup/SKILL.md", "keel-setup/reference/coverage.schema.json", "keel-setup/reference/setup-state.schema.json", + "keel-setup/reference/unified-execute-request.contract.json", "keel-setup/scripts/inventory.py", "keel-setup/scripts/setup_state.py", "keel-setup/scripts/verify_execute.py", "keel-setup/tests/test_inventory.py", + "keel-setup/tests/test_execute_request_contract.py", "keel-setup/tests/test_setup_state.py", "keel-setup/tests/test_verify_execute.py", "scripts/check_release_bundle.py", + "scripts/check_execute_contract.py", "shared/CONSTITUTION.md", "shared/feedback-report.schema.json", "shared/feedback-report.template.md", @@ -116,6 +120,8 @@ def main() -> int: source = json.loads((ROOT / "SOURCE.json").read_text(encoding="utf-8")) if source.get("source_commit") != SOURCE_COMMIT: raise ValueError("SOURCE.json does not pin the reviewed source commit") + if source.get("source_merge_commit") != SOURCE_MERGE_COMMIT: + raise ValueError("SOURCE.json does not pin the merged source commit") if source.get("included_roots") != ["keel-policy", "keel-setup", "shared"]: raise ValueError("SOURCE.json included_roots changed") if source.get("included_files") != ["tools/public_surface.json"]: