From 6d0dfc8010e5e120bbd8ec92a1acb3eaa13fd26a Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 6 Sep 2026 09:32:11 +0200 Subject: [PATCH] State pdp_digest explicitly; decline to publish a vocabulary mapping flex-auth asked whether this engine should publish an action/target mapping between the claim binding's vocabulary (secrets.kv.destroy, {"id": "lane-openbao-root"}) and a policy package's (destroy, lane:...), since their package makes no cross-check that a claim was approved for the action being decided. Answered no. A PIP asserting that one vocabulary's action means another's would author policy semantics it does not own, over vocabularies it does not own, and the failure mode is asymmetric: a wrong mapping silently accepts a claim approved for a different action, which is worse than no mapping. binding.pdp_digest is the correspondence and sidesteps vocabulary entirely -- it compares the PDP's own digest to the PDP's own digest, with no translation by anyone. Implemented the part that was ours. pdp_digest was emitted only when recorded, so a consumer could not distinguish "not issued against a decision" from "we forgot to look". It is now always present and null in that case, required-but-nullable in the schema, and documented as something a PEP on a privileged lane must refuse. This engine states the fact; enforcing the lane's policy stays with the consumer. Both published examples were already contradicting the updated schema by omitting the field -- the same fixture-versus-contract defect flex-auth hit twice this week and that secrets-engine implemented. Fixed both, made them cover the PDP-bound and unbound shapes so neither is inferred from the other, and added tests/test_examples.py to validate every example against the schema so the class cannot recur here. jsonschema added as a dev dependency. 94 tests pass (6 new). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TvyJPAaVCGsVheVhcCwNND Assistant: claude-code Assistant-Model: opus Assistant-Process: 411227@bnt-lap001 Assistant-Session: d566f6d3-bcaf-43c3-bc5e-3ddd0f64b535 --- approval_engine/store.py | 6 +- docs/approval-claim.md | 43 +++++- examples/claim.revoked.json | 10 +- examples/claim.valid.json | 10 +- pyproject.toml | 2 +- schemas/approval_claim.schema.json | 136 ++++++++++++++---- tests/test_examples.py | 41 ++++++ ...duction-readiness-and-consumer-adoption.md | 14 ++ 8 files changed, 224 insertions(+), 38 deletions(-) create mode 100644 tests/test_examples.py diff --git a/approval_engine/store.py b/approval_engine/store.py index 0d25d22..5df4dcf 100644 --- a/approval_engine/store.py +++ b/approval_engine/store.py @@ -813,8 +813,10 @@ class Engine: **canonical_binding(obj.binding), "digest": obj.binding_digest, } - if obj.pdp_digest: - binding["pdp_digest"] = obj.pdp_digest + # Always stated, null when the approval was not issued against a PDP + # decision. A missing key reads as an oversight; an explicit null is a + # fact the consumer must act on. See docs/approval-claim.md. + binding["pdp_digest"] = obj.pdp_digest or None return { "schema_version": CLAIM_SCHEMA, "kind": "approval-claim", diff --git a/docs/approval-claim.md b/docs/approval-claim.md index 88da6cb..114a113 100644 --- a/docs/approval-claim.md +++ b/docs/approval-claim.md @@ -89,6 +89,45 @@ compares digests. | `principal` | `subject.attributes.principal` if present, else `subject.id` | | `purpose` | `context.purpose` | +### Action and target vocabulary — there is no published mapping, by design + +The table above maps *fields*, not *values*. The claim's `action` and `target` +carry whatever vocabulary the approval's creator used +(`secrets.kv.destroy`, `{"id": "lane-openbao-root", "stage": "prod"}`); a policy +package may use its own (`destroy`, `lane:...`). **This engine does not publish +a translation between them and will not.** + +This is a layer boundary, not an omission. A PIP that asserted +`secrets.kv.destroy` *means* `destroy` would be authoring policy semantics it +does not own, over vocabularies it does not own. The failure mode is also +asymmetric: a wrong mapping silently accepts a claim approved for a +*different* action, which is worse than no mapping at all. A consumer that +finds itself wanting one should read that as a signal it is about to compare +the wrong two things. + +**`binding.pdp_digest` is the correspondence.** It sidesteps vocabulary +entirely: it is the PDP's own `NewDecisionBinding.request_digest`, recorded at +issue time, so comparing + +```text +claim.binding.pdp_digest == decision.binding.request_digest +``` + +compares the PDP's digest to the PDP's digest, in one vocabulary, with no +translation by anyone. That is strictly stronger than a name-to-name mapping +could be. + +`pdp_digest` is **always present** on the claim and is `null` when the approval +was not issued against a PDP decision — a stated fact rather than a missing +key, so a consumer cannot read absence as an oversight. It is not required on +every approval, because approvals legitimately exist that no decision preceded. + +**A PEP on a privileged lane MUST refuse a claim whose `pdp_digest` is +`null`.** Such a claim proves an approval exists; it does not prove the +approval was issued against the request now being decided, and no vocabulary +comparison recovers that. Requiring it is the consumer's own gate — this engine +states the fact and does not enforce the lane's policy. + Go's `json.Marshal` of a `CheckRequest` is **not** this canonical JSON (field order and `omitempty` differ). Do not hash a CheckRequest with this function and expect it to equal `NewDecisionBinding.request_digest`. @@ -124,7 +163,9 @@ A production consumer of this claim, before treating it as an input, checks: 3. `valid_now` is true and `consumed` is false. 4. `binding.digest` equals the digest of the binding the consumer computed from the proposed action, **or** `binding.pdp_digest` equals the - `NewDecisionBinding` digest of that request. + `NewDecisionBinding` digest of that request. On a privileged lane, take the + second: require `binding.pdp_digest` to be non-null and equal, and refuse + the claim otherwise. See "Action and target vocabulary" above. 5. `freshness.not_after` is still in the future. 6. `reason_code` is `ok`. diff --git a/examples/claim.revoked.json b/examples/claim.revoked.json index 6e53eb4..229dc38 100644 --- a/examples/claim.revoked.json +++ b/examples/claim.revoked.json @@ -1,7 +1,7 @@ { "schema_version": "0.1", "kind": "approval-claim", - "yields_to": "net-kingdom taxonomy request-claim schema (statute §17; unassigned)", + "yields_to": "net-kingdom taxonomy request-claim schema (statute \u00a717; unassigned)", "issuer": "approval-engine", "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", "state": "revoked", @@ -9,11 +9,15 @@ "consumed": false, "binding": { "action": "secrets.kv.destroy", - "target": {"id": "lane-openbao-root", "stage": "prod"}, + "target": { + "id": "lane-openbao-root", + "stage": "prod" + }, "actor": "agt-secrets-engine", "principal": "bernd", "purpose": "rotate-exposed-key", - "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "pdp_digest": null }, "freshness": { "observed_at": "2026-08-29T12:05:00+00:00", diff --git a/examples/claim.valid.json b/examples/claim.valid.json index 2d00e86..89bd363 100644 --- a/examples/claim.valid.json +++ b/examples/claim.valid.json @@ -1,7 +1,7 @@ { "schema_version": "0.1", "kind": "approval-claim", - "yields_to": "net-kingdom taxonomy request-claim schema (statute §17; unassigned)", + "yields_to": "net-kingdom taxonomy request-claim schema (statute \u00a717; unassigned)", "issuer": "approval-engine", "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", "state": "valid", @@ -9,11 +9,15 @@ "consumed": false, "binding": { "action": "secrets.kv.destroy", - "target": {"id": "lane-openbao-root", "stage": "prod"}, + "target": { + "id": "lane-openbao-root", + "stage": "prod" + }, "actor": "agt-secrets-engine", "principal": "bernd", "purpose": "rotate-exposed-key", - "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "pdp_digest": "sha256:3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f3f" }, "freshness": { "observed_at": "2026-08-29T12:00:00+00:00", diff --git a/pyproject.toml b/pyproject.toml index ce365d8..32e4395 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,7 +7,7 @@ requires-python = ">=3.11" dependencies = ["PyJWT[crypto]>=2.7,<3"] [project.optional-dependencies] -dev = ["pytest"] +dev = ["pytest", "jsonschema>=4,<5"] serve = ["waitress>=3,<4"] [project.scripts] diff --git a/schemas/approval_claim.schema.json b/schemas/approval_claim.schema.json index 2b54243..990e3ed 100644 --- a/schemas/approval_claim.schema.json +++ b/schemas/approval_claim.schema.json @@ -2,7 +2,7 @@ "$schema": "https://json-schema.org/draft/2020-12/schema", "$id": "https://approval-engine.netkingdom/schemas/approval_claim.schema.json", "title": "ApprovalClaim", - "description": "Input claim that access-engine consumes. This is a fact about an approval object, not a decision. Yields to the Taxonomy request-claim schema (statute §17) when that artifact exists and is assented; do not treat this local shape as permanent.", + "description": "Input claim that access-engine consumes. This is a fact about an approval object, not a decision. Yields to the Taxonomy request-claim schema (statute \u00a717) when that artifact exists and is assented; do not treat this local shape as permanent.", "type": "object", "additionalProperties": false, "required": [ @@ -19,14 +19,23 @@ "reason_code" ], "properties": { - "schema_version": { "const": "0.1" }, - "kind": { "const": "approval-claim" }, + "schema_version": { + "const": "0.1" + }, + "kind": { + "const": "approval-claim" + }, "yields_to": { "type": "string", "description": "Taxonomy artifact this contract yields to. Informational; consumers must not branch on it." }, - "issuer": { "const": "approval-engine" }, - "approval_id": { "type": "string", "format": "uuid" }, + "issuer": { + "const": "approval-engine" + }, + "approval_id": { + "type": "string", + "format": "uuid" + }, "state": { "type": "string", "enum": [ @@ -43,10 +52,18 @@ "type": "boolean", "description": "True only when the object is approved, inside its validity window, and not consumed, superseded, revoked, or expired. Not a permission." }, - "consumed": { "type": "boolean" }, - "binding": { "$ref": "#/$defs/binding" }, - "freshness": { "$ref": "#/$defs/freshness" }, - "validity": { "$ref": "#/$defs/validity" }, + "consumed": { + "type": "boolean" + }, + "binding": { + "$ref": "#/$defs/binding" + }, + "freshness": { + "$ref": "#/$defs/freshness" + }, + "validity": { + "$ref": "#/$defs/validity" + }, "reason_code": { "type": "string", "enum": [ @@ -63,52 +80,115 @@ }, "not": { "anyOf": [ - { "required": ["effect"] }, - { "required": ["decision"] }, - { "required": ["allow"] }, - { "required": ["deny"] } + { + "required": [ + "effect" + ] + }, + { + "required": [ + "decision" + ] + }, + { + "required": [ + "allow" + ] + }, + { + "required": [ + "deny" + ] + } ] }, "$defs": { "binding": { "type": "object", "additionalProperties": false, - "required": ["action", "target", "actor", "principal", "purpose", "digest"], + "required": [ + "action", + "target", + "actor", + "principal", + "purpose", + "digest", + "pdp_digest" + ], "properties": { - "action": { "type": "string", "minLength": 1 }, - "target": { "type": "object" }, - "actor": { "type": "string", "minLength": 1 }, - "principal": { "type": "string", "minLength": 1 }, - "purpose": { "type": "string", "minLength": 1 }, + "action": { + "type": "string", + "minLength": 1 + }, + "target": { + "type": "object" + }, + "actor": { + "type": "string", + "minLength": 1 + }, + "principal": { + "type": "string", + "minLength": 1 + }, + "purpose": { + "type": "string", + "minLength": 1 + }, "digest": { "type": "string", "pattern": "^sha256:[0-9a-f]{64}$", "description": "SHA-256 over the canonical JSON of action, actor, principal, purpose, target (sorted keys, RFC 8259). Distinguishes approved from approved-for-this-exact-request." }, "pdp_digest": { - "type": "string", + "type": [ + "string", + "null" + ], "pattern": "^sha256:[0-9a-f]{64}$", - "description": "Optional. The flex-auth NewDecisionBinding request_digest recorded at issue time. When present, access-engine MUST compare this to the digest it already computes, not re-derive our native digest as a substitute." + "description": "The flex-auth NewDecisionBinding request_digest recorded at issue time, or null when the approval was not issued against a PDP decision. Always present so its absence is a stated fact rather than a missing key. When non-null, access-engine MUST compare this to the digest it already computes and MUST NOT re-derive the native digest as a substitute. A PEP on a privileged lane MUST refuse a claim whose pdp_digest is null." } } }, "freshness": { "type": "object", "additionalProperties": false, - "required": ["observed_at", "ttl_seconds", "not_after"], + "required": [ + "observed_at", + "ttl_seconds", + "not_after" + ], "properties": { - "observed_at": { "type": "string", "format": "date-time" }, - "ttl_seconds": { "type": "integer", "minimum": 1 }, - "not_after": { "type": "string", "format": "date-time" } + "observed_at": { + "type": "string", + "format": "date-time" + }, + "ttl_seconds": { + "type": "integer", + "minimum": 1 + }, + "not_after": { + "type": "string", + "format": "date-time" + } } }, "validity": { "type": "object", "additionalProperties": false, - "required": ["not_before", "expires_at"], + "required": [ + "not_before", + "expires_at" + ], "properties": { - "not_before": { "type": "string", "format": "date-time" }, - "expires_at": { "type": "string", "format": "date-time" } + "not_before": { + "type": "string", + "format": "date-time" + }, + "expires_at": { + "type": "string", + "format": "date-time" + } } } } diff --git a/tests/test_examples.py b/tests/test_examples.py new file mode 100644 index 0000000..d61ad6d --- /dev/null +++ b/tests/test_examples.py @@ -0,0 +1,41 @@ +"""The published examples must satisfy the published schema. + +flex-auth shipped two defects in one day from fixtures that contradicted their +own contracts, and secrets-engine built a validator against one of them. A +contract whose examples contradict its prose will be implemented as its +examples, so the examples are tested rather than trusted. +""" + +import json +from pathlib import Path + +import pytest + +jsonschema = pytest.importorskip("jsonschema") + +ROOT = Path(__file__).resolve().parent.parent +SCHEMA = json.loads((ROOT / "schemas" / "approval_claim.schema.json").read_text()) +EXAMPLES = sorted((ROOT / "examples").glob("claim.*.json")) + + +def test_examples_exist(): + assert EXAMPLES, "no claim examples found to validate" + + +@pytest.mark.parametrize("path", EXAMPLES, ids=lambda p: p.name) +def test_example_matches_schema(path): + jsonschema.validate(json.loads(path.read_text()), SCHEMA) + + +@pytest.mark.parametrize("path", EXAMPLES, ids=lambda p: p.name) +def test_example_states_pdp_digest_explicitly(path): + """Absence must be a stated null, never a missing key.""" + assert "pdp_digest" in json.loads(path.read_text())["binding"] + + +def test_examples_cover_both_pdp_binding_states(): + """An implementer must see both shapes, not infer one from the other.""" + states = { + json.loads(p.read_text())["binding"]["pdp_digest"] is None for p in EXAMPLES + } + assert states == {True, False} diff --git a/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md b/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md index 80b7278..d6f609d 100644 --- a/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md +++ b/workplans/APPROVAL-WP-0002-production-readiness-and-consumer-adoption.md @@ -223,3 +223,17 @@ this engine's state transitions and its use outbox row under §9.6. Implemented: with `approved_at` plus assurance/evidence refs). Tests prove reconstruction from the use row alone and that the claim still discloses no approver identities. Documented in `docs/outbox-contract.md`. + +2026-09-06 follow-on (T05): flex-auth asked whether this engine should publish +an action/target vocabulary mapping between the claim binding and their policy +package before `SECRETS-WP-0007-T04` makes destroy reachable. Answered no, and +recorded why in `docs/approval-claim.md`: a PIP asserting that one vocabulary's +action *means* another's would author policy semantics it does not own, and a +wrong mapping silently accepts a claim approved for a different action. +`binding.pdp_digest` is the correspondence — it compares the PDP's digest to +the PDP's digest with no translation. Implemented the part that was ours: +`pdp_digest` is now always present on the claim and `null` when the approval +was not issued against a PDP decision, so absence is a stated fact rather than +a missing key, and the schema requires it as nullable. A PEP on a privileged +lane must refuse a null. Both published examples were contradicting the schema; +fixed, and `tests/test_examples.py` now validates every example against it.