diff --git a/docs/secrets-engine-action-vocabulary.md b/docs/secrets-engine-action-vocabulary.md index 1aba2d2..ca3208a 100644 --- a/docs/secrets-engine-action-vocabulary.md +++ b/docs/secrets-engine-action-vocabulary.md @@ -78,14 +78,44 @@ it does not have. ## Dual control on `destroy` -`destroy` requires `context.approval` marked `approved` with at least two -distinct approver subject ids. Repeated entries from one subject count once. +`destroy` requires an **approval-claim** on `context.approval`, issued by +`approval-engine`, whose `valid_now` is `true`. That is the whole check. -flex-auth consumes that claim as an input claim and never mutates it +The shape is `approval-engine/schemas/approval_claim.schema.json`. +`valid_now` is a summary predicate — true only when the object is approved, +inside its validity window, and not consumed, superseded, revoked, or expired — +and **the distinct-approver threshold is folded into it**. A claim that failed +the threshold returns `reason_code: insufficient_approvers`. + +flex-auth consumes the claim as an input claim and never mutates it (`security-layer-model_v0.7` §9.4). Per `FLEX-DEC-2026-006` the approval fact is `approval-engine`'s step-1 artifact and the decision is step 2; the PEP -validates across both. This package checks that the claim *says* approved with -distinct approvers. It does **not** re-derive the claim's temporal validity, -signature, or supersession state — those belong to `approval-engine` to assert -and to the PEP to verify against the live claim. A PDP re-deriving them from a -caller-supplied blob would be inventing an authority it does not have. +validates across both. + +The package does **not** count approvers, re-derive validity/freshness/ +signature/supersession, or compare `binding.pdp_digest`. Counting approvers is +the duplication the split removes (`GH-DEC-2026-005`); the compensating property +is reconstructability at the issuer under §9.6, which is `approval-engine`'s. +The digest comparison is real and preferred but the request digest is computed +after policy evaluation, so a Rego rule cannot see it — it belongs in the PEP. + +### Correction, 2026-09-06 + +The first published rule required `context.approval.status == "approved"` and +counted `context.approval.approvals[].subject_id`. Neither field exists: the +claim's field is `state` (and `valid`, not `approved`, is the operative value), +and it carries no approver list. The rule was unsatisfiable — every live +`destroy` would have denied `dual_control_required` regardless of the approval. +It failed closed, so it was never a hole, but it was a policy written against an +invented shape. Corrected the same day. + +### Open item: no published binding mapping + +The claim's `binding.action` and `binding.target` use `approval-engine`'s +vocabulary (`secrets.kv.destroy`, `{"id": ..., "stage": ...}`), not this one +(`destroy`, `lane:...`). No mapping between them is published, so the package +makes no cross-check that the claim was approved for *this* action and target, +and must not invent one — a wrong mapping would silently accept a claim approved +for something else. Until a mapping exists that correspondence is the PEP's, via +`binding.pdp_digest`. Worth closing before `SECRETS-WP-0007-T04` makes `destroy` +reachable. diff --git a/examples/caring/action_authorization.json b/examples/caring/action_authorization.json index b32103c..28dac82 100644 --- a/examples/caring/action_authorization.json +++ b/examples/caring/action_authorization.json @@ -107,6 +107,11 @@ "mode": "standalone", "policy_package": "secrets-engine.catalog-lane.lifecycle", "policy_version": "v1", + "policy_package_digest": "sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "registry_snapshot_digest": "sha256:bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "input_claim_digests": { + "context": "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" + }, "decision_time": "2026-08-23T10:00:30Z", "policy_package_note": "Reserved coordinate, not yet published (FLEX-DEC-2026-005). Illustrative only." } diff --git a/examples/secrets-engine/README.md b/examples/secrets-engine/README.md index 82d8eb0..64b59e6 100644 --- a/examples/secrets-engine/README.md +++ b/examples/secrets-engine/README.md @@ -10,7 +10,7 @@ catalog-lane operations. Opened by `FLEX-DEC-2026-005`, carried by | `protected_system_manifest.yaml` | the `secret-catalog-lane` resource type and twelve actions | | `subject_manifest.yaml` | the single `secrets-engine` service identity | | `registry_snapshot.json` | loadable snapshot combining both manifests | -| `policy_fixtures.yaml` | 26 fixtures — 11 allows, dual control both ways, and every denial branch | +| `policy_fixtures.yaml` | 29 fixtures — 11 allows, dual control both ways, and every denial branch | | `check_request_*.json` | standalone requests for `POST /v1/check` | The action vocabulary is **secrets-engine's**, delivered under @@ -25,7 +25,7 @@ go run ./cmd/flex-auth validate -kind policy -file examples/secrets-engine/polic go run ./cmd/flex-auth load-registry -file examples/secrets-engine/registry_snapshot.json ``` -22 Rego tests and 26 fixtures. +25 Rego tests and 29 fixtures. ## Not yet deployed diff --git a/examples/secrets-engine/check_request_allow_destroy_dual_control.json b/examples/secrets-engine/check_request_allow_destroy_dual_control.json index 3664840..bff2ab3 100644 --- a/examples/secrets-engine/check_request_allow_destroy_dual_control.json +++ b/examples/secrets-engine/check_request_allow_destroy_dual_control.json @@ -19,15 +19,14 @@ }, "context": { "approval": { - "status": "approved", - "approvals": [ - { - "subject_id": "human:alice" - }, - { - "subject_id": "human:bob" - } - ] + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "valid", + "valid_now": true, + "consumed": false, + "reason_code": "ok" } } } diff --git a/examples/secrets-engine/policy_fixtures.yaml b/examples/secrets-engine/policy_fixtures.yaml index 49ae012..6b99675 100644 --- a/examples/secrets-engine/policy_fixtures.yaml +++ b/examples/secrets-engine/policy_fixtures.yaml @@ -338,15 +338,14 @@ }, "context": { "approval": { - "status": "approved", - "approvals": [ - { - "subject_id": "human:alice" - }, - { - "subject_id": "human:bob" - } - ] + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "valid", + "valid_now": true, + "consumed": false, + "reason_code": "ok" } } }, @@ -384,7 +383,7 @@ } }, { - "id": "fixture:secrets-engine-destroy-single-approver-deny", + "id": "fixture:secrets-engine-destroy-insufficient-approvers-deny", "request": { "id": "check:secrets-engine-destroy", "tenant": "tenant:platform", @@ -406,12 +405,14 @@ }, "context": { "approval": { - "status": "approved", - "approvals": [ - { - "subject_id": "human:alice" - } - ] + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "requested", + "valid_now": false, + "consumed": false, + "reason_code": "insufficient_approvers" } } }, @@ -421,7 +422,7 @@ } }, { - "id": "fixture:secrets-engine-destroy-repeated-approver-deny", + "id": "fixture:secrets-engine-destroy-consumed-claim-deny", "request": { "id": "check:secrets-engine-destroy", "tenant": "tenant:platform", @@ -443,15 +444,14 @@ }, "context": { "approval": { - "status": "approved", - "approvals": [ - { - "subject_id": "human:alice" - }, - { - "subject_id": "human:alice" - } - ] + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "consumed", + "valid_now": false, + "consumed": true, + "reason_code": "consumed" } } }, @@ -461,7 +461,7 @@ } }, { - "id": "fixture:secrets-engine-destroy-pending-claim-deny", + "id": "fixture:secrets-engine-destroy-revoked-claim-deny", "request": { "id": "check:secrets-engine-destroy", "tenant": "tenant:platform", @@ -483,15 +483,131 @@ }, "context": { "approval": { - "status": "pending", - "approvals": [ - { - "subject_id": "human:alice" - }, - { - "subject_id": "human:bob" - } - ] + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "revoked", + "valid_now": false, + "consumed": false, + "reason_code": "revoked" + } + } + }, + "expect": { + "effect": "deny", + "reason": "dual_control_required" + } + }, + { + "id": "fixture:secrets-engine-destroy-approved-not-valid-now-deny", + "request": { + "id": "check:secrets-engine-destroy", + "tenant": "tenant:platform", + "subject": { + "id": "secrets-engine", + "type": "service" + }, + "action": "destroy", + "resource": { + "id": "lane:glas-primary", + "type": "secret-catalog-lane", + "system": "secrets-engine", + "attributes": { + "stage": "prod", + "fields": [], + "policy_targets": [], + "auth_targets": [] + } + }, + "context": { + "approval": { + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "approved", + "valid_now": false, + "consumed": false, + "reason_code": "not_yet_valid" + } + } + }, + "expect": { + "effect": "deny", + "reason": "dual_control_required" + } + }, + { + "id": "fixture:secrets-engine-destroy-foreign-issuer-deny", + "request": { + "id": "check:secrets-engine-destroy", + "tenant": "tenant:platform", + "subject": { + "id": "secrets-engine", + "type": "service" + }, + "action": "destroy", + "resource": { + "id": "lane:glas-primary", + "type": "secret-catalog-lane", + "system": "secrets-engine", + "attributes": { + "stage": "prod", + "fields": [], + "policy_targets": [], + "auth_targets": [] + } + }, + "context": { + "approval": { + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "some-other-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "valid", + "valid_now": true, + "consumed": false, + "reason_code": "ok" + } + } + }, + "expect": { + "effect": "deny", + "reason": "dual_control_required" + } + }, + { + "id": "fixture:secrets-engine-destroy-wrong-kind-deny", + "request": { + "id": "check:secrets-engine-destroy", + "tenant": "tenant:platform", + "subject": { + "id": "secrets-engine", + "type": "service" + }, + "action": "destroy", + "resource": { + "id": "lane:glas-primary", + "type": "secret-catalog-lane", + "system": "secrets-engine", + "attributes": { + "stage": "prod", + "fields": [], + "policy_targets": [], + "auth_targets": [] + } + }, + "context": { + "approval": { + "schema_version": "0.1", + "kind": "action-authorization", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "valid", + "valid_now": true, + "consumed": false, + "reason_code": "ok" } } }, diff --git a/examples/secrets-engine/policy_package.md b/examples/secrets-engine/policy_package.md index 530238e..c60139d 100644 --- a/examples/secrets-engine/policy_package.md +++ b/examples/secrets-engine/policy_package.md @@ -79,7 +79,24 @@ wrongly. They are recorded in [`../../docs/secrets-engine-action-vocabulary.md`](../../docs/secrets-engine-action-vocabulary.md). The four that an inferred list would have gotten wrong are encoded here, not -assumed: +assumed — see "The four traps" below. + +## Correction, 2026-09-06 — the dual-control rule was written against an invented shape + +The first published version of this rule required `context.approval.status == +"approved"` and counted `context.approval.approvals[].subject_id`. **Neither +field exists.** The claim's field is `state` (whose `approved` is not the +operative value — `valid` is), and it carries no approver list at all. That rule +was unsatisfiable: every live `destroy` would have denied `dual_control_required` +no matter how good the approval was. + +It failed closed, so it was never a hole. It was, however, a policy written +against an invented shape rather than a published one — the same class of error +`approval-engine` had flagged one message earlier, committed while flagging it. +Recorded here rather than quietly rewritten. The rule now consumes +`valid_now` from the published claim; see "Dual control on `destroy`". + +## The four traps 1. **`revoke` is not an action.** The CLI verb `revoke` gates as `deactivate`, which is also reached from `lifecycle deactivate`. There is no `revoke` @@ -125,18 +142,50 @@ delivered secret's own lifetime is secrets-engine's to bound. ## Dual control on `destroy` `destroy` is the one action that requires more than a known caller. It requires -an **approval claim** on `context.approval`, marked `approved`, naming at least -two distinct approver subject ids. +an **approval-claim** on `context.approval`, issued by `approval-engine`, whose +`valid_now` is `true`. -flex-auth consumes that claim as an input claim and **never mutates it** -(`security-layer-model_v0.7` §9.4). Per `FLEX-DEC-2026-006` the approval fact -is `approval-engine`'s step-1 artifact and this decision is step 2; the PEP -validates across both. Concretely, this package checks that the claim *says* -approved with distinct approvers. It does **not** re-derive the claim's -temporal validity, its signature, or its supersession state — those are -`approval-engine`'s to assert and the PEP's to verify against the live claim. -A PDP that re-derived them from a caller-supplied blob would be inventing an -authority it does not have. +That is the whole check, and the shape is +`approval-engine/schemas/approval_claim.schema.json` — not a shape of +flex-auth's devising. `valid_now` is a summary predicate: true only when the +object is approved, inside its validity window, and not consumed, superseded, +revoked, or expired. **The distinct-approver threshold is folded into it**, and +`reason_code: insufficient_approvers` is how a claim that failed the threshold +comes back. + +flex-auth consumes the claim as an input claim and **never mutates it** +(`security-layer-model_v0.7` §9.4). Per `FLEX-DEC-2026-006` the approval fact is +`approval-engine`'s step-1 artifact and this decision is step 2; the PEP +validates across both. + +### What this package deliberately does not do + +- **It does not count approvers.** The claim carries no approver list, and + re-deriving the threshold here is exactly the duplication the step-1/step-2 + split removes (`GH-DEC-2026-005`). The compensating property is + reconstructability at the issuer under §9.6 — detection, not prevention — + and it is `approval-engine`'s, not ours. +- **It does not re-derive validity, freshness, signature, or supersession.** + Those are `approval-engine`'s to assert and the PEP's to verify against the + live claim. A PDP re-deriving them from a caller-supplied blob would be + inventing an authority it does not have. +- **It does not compare `binding.pdp_digest`.** That comparison is real and + preferred (`approval-engine/docs/approval-claim.md`), but the request digest + is computed by the engine *after* policy evaluation, so a Rego rule cannot + see it. It belongs in the PEP and in the engine, not here. + +### Open item: no published binding mapping + +The claim's `binding.action` and `binding.target` use `approval-engine`'s +vocabulary (`secrets.kv.destroy`, `{"id": "lane-openbao-root", "stage": "prod"}`), +not this package's (`destroy`, `lane:...`). There is no published mapping +between the two, so this package makes **no** cross-check that the claim was +approved for *this* action and target — and it must not invent one, because a +wrong mapping would silently accept a claim approved for something else. + +Until a mapping is published, that correspondence is the PEP's to enforce via +`binding.pdp_digest`. This is worth closing before `destroy` becomes reachable +under `SECRETS-WP-0007-T04`. ### Decision: one subject, so no `action_not_granted` branch @@ -208,14 +257,11 @@ allowed if { dual_control_satisfied } -approver_ids := {id | - some entry in input.context.approval.approvals - id := entry.subject_id -} - dual_control_satisfied if { - input.context.approval.status == "approved" - count(approver_ids) >= 2 + claim := input.context.approval + claim.kind == "approval-claim" + claim.issuer == "approval-engine" + claim.valid_now == true } default first_denial := "no_matching_rule" @@ -259,10 +305,20 @@ lane(action) := { "context": {} } -approved_destroy := object.union(lane("destroy"), {"context": {"approval": { - "status": "approved", - "approvals": [{"subject_id": "human:alice"}, {"subject_id": "human:bob"}] -}}}) +valid_claim := { + "schema_version": "0.1", + "kind": "approval-claim", + "issuer": "approval-engine", + "approval_id": "3d1c0a8e-6b7f-4c21-9a0e-1f2b3c4d5e6f", + "state": "valid", + "valid_now": true, + "consumed": false, + "reason_code": "ok" +} + +destroy_with(claim) := object.union(lane("destroy"), {"context": {"approval": claim}}) + +approved_destroy := destroy_with(valid_claim) test_apply_allowed if { catalog_lane.decision.effect == "allow" with input as lane("apply") @@ -316,33 +372,39 @@ test_destroy_without_claim_denied if { catalog_lane.decision.reason == "dual_control_required" with input as lane("destroy") } -test_destroy_single_approver_denied if { - catalog_lane.decision.reason == "dual_control_required" with input as object.union( - lane("destroy"), - {"context": {"approval": { - "status": "approved", - "approvals": [{"subject_id": "human:alice"}] - }}} +test_destroy_insufficient_approvers_denied if { + catalog_lane.decision.reason == "dual_control_required" with input as destroy_with( + object.union(valid_claim, {"state": "requested", "valid_now": false, "reason_code": "insufficient_approvers"}) ) } -test_destroy_repeated_approver_counts_once if { - catalog_lane.decision.reason == "dual_control_required" with input as object.union( - lane("destroy"), - {"context": {"approval": { - "status": "approved", - "approvals": [{"subject_id": "human:alice"}, {"subject_id": "human:alice"}] - }}} +test_destroy_consumed_claim_denied if { + catalog_lane.decision.reason == "dual_control_required" with input as destroy_with( + object.union(valid_claim, {"state": "consumed", "valid_now": false, "consumed": true, "reason_code": "consumed"}) ) } -test_destroy_unapproved_claim_denied if { - catalog_lane.decision.reason == "dual_control_required" with input as object.union( - lane("destroy"), - {"context": {"approval": { - "status": "pending", - "approvals": [{"subject_id": "human:alice"}, {"subject_id": "human:bob"}] - }}} +test_destroy_revoked_claim_denied if { + catalog_lane.decision.reason == "dual_control_required" with input as destroy_with( + object.union(valid_claim, {"state": "revoked", "valid_now": false, "reason_code": "revoked"}) + ) +} + +test_destroy_approved_but_not_valid_now_denied if { + catalog_lane.decision.reason == "dual_control_required" with input as destroy_with( + object.union(valid_claim, {"state": "approved", "valid_now": false, "reason_code": "not_yet_valid"}) + ) +} + +test_destroy_foreign_issuer_denied if { + catalog_lane.decision.reason == "dual_control_required" with input as destroy_with( + object.union(valid_claim, {"issuer": "some-other-engine"}) + ) +} + +test_destroy_wrong_kind_denied if { + catalog_lane.decision.reason == "dual_control_required" with input as destroy_with( + object.union(valid_claim, {"kind": "action-authorization"}) ) }