diff --git a/docs/pdp-access-path.md b/docs/pdp-access-path.md index a84fa90..99f373d 100644 --- a/docs/pdp-access-path.md +++ b/docs/pdp-access-path.md @@ -76,21 +76,40 @@ v1 shipped and was deployed with no `input.tenant` rule at all; a `rotate` under change is visible in the version string, and this engine refuses a v1 decision outright. See `docs/tenant-alignment.md`. -## Known gap: the digest join does not hold against a real request +## The digest join, and why it is not a digest comparison -Proved live on 2026-09-07 and **not yet fixed**; the engine currently rejects -every real allow, which is fail-closed and therefore safe to leave standing -while the rule is published. +Proved live on 2026-09-07, fixed the same day. -The evaluator enriches the request from its registry before hashing — `subject` -gains `attributes` and `tenant`, `resource` gains `tenant` — so -`binding.request_digest` is a digest of material we did not send and cannot -reproduce. `validate_decision_envelope` compares the binding against our -unenriched request and fails with *"decision binding does not match request"*. +The evaluator normalizes before hashing: the request tenant is copied onto +`subject` and `resource`, and a registry hit copies `type`, `tenant` and +selected `attributes` onto the refs. So `binding.request_digest` is over +material we did not send and cannot reproduce, and the validator's original +byte-equality check against our unenriched request rejected every real allow. -Which fields the evaluator may enrich is flex-auth's contract to publish, not -ours to infer. Guessing it would mean accepting a binding that differs from what -we proposed in some way we decided was benign — the same fail-open shape as a -guessed digest exclusion or a guessed vocabulary mapping. Raised with flex-auth; -staying fail-closed and naming the missing rule is the correct posture until -they answer. +flex-auth's `canonical-request-digest.md` already published the consumer rule, +and the fix follows it rather than inventing one: + +> A consumer that re-hashes the **original** unenriched request will not match a +> decision that turned on registry attributes. Compare structured `binding` +> fields to the proposed action, and treat `request_digest` as the evaluator's +> statement of what it hashed. To recompute independently, hash the same +> normalized tuple the binding carries. + +`validate_decision_envelope` now does exactly that: + +- **Structured correspondence.** Everything we proposed — tenant, action, + context, `subject.id`/`type`, `resource.id`/`type`/`system` and every + attribute we sent — must survive unchanged in the binding. +- **Enrichment only where the contract permits it.** A ref may gain `type`, + `tenant` and `attributes`; any other added field is refused. An enriched + `tenant` must be the request's tenant, so a cross-tenant binding cannot arrive + wearing our request's clothes. +- **Digest self-consistency.** `request_digest` is recomputed over the + normalized tuple the binding carries, per the contract's own instruction. It + is still checked — just for coherence of the binding rather than against + material we never sent. + +The lesson worth keeping: the replay fixtures could not catch this, because +`_request_from()` rebuilds the request from the binding, so the tests hashed the +evaluator's output and compared it to the evaluator's output. Only a real +request through this access path exposed it. diff --git a/docs/tenant-alignment.md b/docs/tenant-alignment.md index 0423d07..0fc2790 100644 --- a/docs/tenant-alignment.md +++ b/docs/tenant-alignment.md @@ -116,9 +116,9 @@ alone. port-forward` plus a bounded `TokenRequest` token; see `docs/pdp-access-path.md`. Adopted and enforced in `decision_check.require_supported_pdp_address`. -- The evaluator's **enrichment rule**: `binding` carries registry-enriched - `subject.attributes`, `subject.tenant` and `resource.tenant` that we did not - send, so the digest join fails against a real request. flex-auth's to publish. +- ~~The evaluator's enrichment rule~~ — **already published** in flex-auth's + `canonical-request-digest.md` ("Normalization"), and now implemented. See + `docs/pdp-access-path.md`. ## Hazard: the Service DNS name resolves here, to the wrong host diff --git a/src/secrets_engine/authorization.py b/src/secrets_engine/authorization.py index 70aa86a..9313209 100644 --- a/src/secrets_engine/authorization.py +++ b/src/secrets_engine/authorization.py @@ -321,6 +321,111 @@ def _check_approval_binding_digest( ) +#: Fields the evaluator's documented normalization may add to a ref, per +#: canonical-request-digest.md "Normalization": the request tenant is copied onto +#: subject and resource when they omit it, and a registry hit copies type, +#: tenant, and selected attributes onto the refs the digest sees. +_ENRICHABLE_REF_FIELDS = ("type", "tenant", "attributes") + + +def _require_ref_corresponds(bound: Any, sent: dict[str, Any], tenant: str, name: str) -> None: + """The bound ref must be our ref, plus only documented enrichment.""" + if not isinstance(bound, dict): + raise DecisionError(f"flex-auth decision binding {name} must be an object") + for key, value in sent.items(): + if key == "attributes": + continue + if bound.get(key) != value: + raise DecisionError( + f"flex-auth decision binding {name}.{key} does not match the " + "proposed action" + ) + for key in bound: + if key not in sent and key not in _ENRICHABLE_REF_FIELDS: + raise DecisionError( + f"flex-auth decision binding {name} carries unexpected field " + f"'{key}'; enrichment may add only {_ENRICHABLE_REF_FIELDS}" + ) + # Normalization rule 1: an enriched tenant is the REQUEST's tenant. A ref + # bearing some other tenant would be a cross-tenant binding wearing our + # request's clothes. + bound_tenant = bound.get("tenant") + if bound_tenant is not None and tenant and bound_tenant != tenant: + raise DecisionError( + f"flex-auth decision binding {name}.tenant '{bound_tenant}' is not " + f"the request tenant '{tenant}'" + ) + sent_attributes = sent.get("attributes") + if isinstance(sent_attributes, dict): + bound_attributes = bound.get("attributes") + if not isinstance(bound_attributes, dict): + raise DecisionError( + f"flex-auth decision binding {name}.attributes must be an object" + ) + # Every attribute we proposed must survive unchanged. The registry may + # add its own; it may not restate ours differently. stage, fields, + # policy_targets and auth_targets are security-bearing and are ours. + for key, value in sent_attributes.items(): + if bound_attributes.get(key) != value: + raise DecisionError( + f"flex-auth decision binding {name}.attributes.{key} does " + "not match the proposed action" + ) + + +def _require_binding_corresponds( + binding: dict[str, Any], expected: dict[str, Any] +) -> None: + """Compare structured binding fields to the proposed action. + + This is what canonical-request-digest.md tells a consumer to do, and why it + is not a digest comparison: "A consumer that re-hashes the original + unenriched request will not match a decision that turned on registry + attributes. Compare structured binding fields to the proposed action, and + treat request_digest as the evaluator's statement of what it hashed." + + The evaluator normalizes before hashing -- it copies the request tenant onto + subject and resource, and a registry hit copies type, tenant and selected + attributes onto the refs. Those additions are material we never sent and + cannot reproduce, so byte-equality against our own request is unsatisfiable + against every real decision. + + What must still hold, and is enforced here: everything we DID propose + survives unchanged, enrichment appears only where the contract permits it, + and an enriched tenant is our request's tenant rather than another one. + """ + tenant = str(expected.get("tenant", "") or "") + if tenant and binding.get("tenant") != tenant: + raise DecisionError("flex-auth decision binding tenant does not match request") + if binding.get("action") != expected["action"]: + raise DecisionError("flex-auth decision binding action does not match request") + if binding.get("context", {}) != expected.get("context", {}): + raise DecisionError("flex-auth decision binding context does not match request") + _require_ref_corresponds(binding.get("subject"), expected["subject"], tenant, "subject") + _require_ref_corresponds(binding.get("resource"), expected["resource"], tenant, "resource") + + +def binding_tuple(binding: dict[str, Any]) -> dict[str, Any]: + """The normalized tuple the binding carries, for independent re-hashing. + + canonical-request-digest.md: "To recompute independently, hash the same + normalized tuple the binding carries (tenant, subject, action, resource, + context)." + """ + tuple_: dict[str, Any] = {} + if binding.get("tenant"): + tuple_["tenant"] = binding["tenant"] + tuple_.update( + { + "subject": binding.get("subject"), + "action": binding.get("action"), + "resource": binding.get("resource"), + "context": binding.get("context", {}), + } + ) + return tuple_ + + def validate_decision_envelope( envelope: object, expected_request: object, @@ -361,37 +466,25 @@ def validate_decision_envelope( raise DecisionError("flex-auth decision request id does not match request") binding = _required_dict(envelope, "binding") - bound_request: dict[str, Any] = {} - if binding.get("tenant"): - bound_request["tenant"] = binding["tenant"] - bound_request.update( - { - "subject": binding.get("subject"), - "action": binding.get("action"), - "resource": binding.get("resource"), - "context": binding.get("context", {}), - } + _require_binding_corresponds(binding, expected) + # The digest is the evaluator's statement of what it hashed, so it is + # verified for self-consistency against the tuple the binding carries -- + # never against our unenriched request, which no real decision can match. + bound_tuple = binding_tuple(binding) + if binding.get("request_digest") != request_digest(bound_tuple): + raise DecisionError( + "flex-auth request digest does not match the binding it carries" + ) + _check_approval_binding_digest(binding, bound_tuple, expected_approval_binding_digest) + # The envelope's top-level refs are enriched too, so they get the same + # correspondence rule rather than byte-equality against what we sent. + _tenant = str(expected.get("tenant", "") or "") + _require_ref_corresponds( + _subject_ref(envelope.get("subject")), expected["subject"], _tenant, "subject" ) - expected_bound: dict[str, Any] = {} - if expected.get("tenant"): - expected_bound["tenant"] = expected["tenant"] - expected_bound.update( - { - "subject": expected["subject"], - "action": expected["action"], - "resource": expected["resource"], - "context": expected.get("context", {}), - } + _require_ref_corresponds( + _resource_ref(envelope.get("resource")), expected["resource"], _tenant, "resource" ) - if canonical_check_request(bound_request) != canonical_check_request(expected_bound): - raise DecisionError("flex-auth decision binding does not match request") - if binding.get("request_digest") != request_digest(expected): - raise DecisionError("flex-auth request digest does not match request") - _check_approval_binding_digest(binding, expected, expected_approval_binding_digest) - if _subject_ref(envelope.get("subject")) != expected["subject"]: - raise DecisionError("flex-auth decision subject does not match request") - if _resource_ref(envelope.get("resource")) != expected["resource"]: - raise DecisionError("flex-auth decision resource does not match request") current = (now or datetime.now(timezone.utc)).astimezone(timezone.utc) lifetime = _required_dict(envelope, "lifetime") diff --git a/tests/test_action_authorization.py b/tests/test_action_authorization.py index 49b4ae9..3427928 100644 --- a/tests/test_action_authorization.py +++ b/tests/test_action_authorization.py @@ -135,7 +135,7 @@ def test_state_hub_authority_is_no_longer_required(): [ (lambda d: d.update(effect="deny"), "effect is not allow"), (lambda d: d.update(effect="audit_only"), "effect is not allow"), - (lambda d: d["binding"].update(action="destroy"), "binding does not match"), + (lambda d: d["binding"].update(action="destroy"), "binding action does not match"), (lambda d: d["binding"].update(request_digest="sha256:" + "0" * 64), "request digest does not match"), (lambda d: d["provenance"].update(policy_package="other.package"), @@ -144,7 +144,15 @@ def test_state_hub_authority_is_no_longer_required(): "policy version is not accepted"), (lambda d: d.update(contract_version="flex-auth.decision-record.v2"), "contract version"), - (lambda d: d["subject"].update(id="user:mallory"), "subject does not match"), + (lambda d: d["subject"].update(id="user:mallory"), + "subject.id does not match"), + # Enrichment may add attributes; it may never restate what we sent. + (lambda d: d["binding"]["resource"]["attributes"].update(stage="build"), + "resource.attributes.stage does not match"), + (lambda d: d["binding"]["subject"].update(tenant="tenant:coulomb"), + "subject.tenant 'tenant:coulomb' is not the request tenant"), + (lambda d: d["binding"]["subject"].update(surprise="x"), + "carries unexpected field"), ], ) def test_invalid_envelopes_fail_closed(mutation, match): diff --git a/tests/test_live_decision_enrichment.py b/tests/test_live_decision_enrichment.py index bb79280..5d3cfb4 100644 --- a/tests/test_live_decision_enrichment.py +++ b/tests/test_live_decision_enrichment.py @@ -3,21 +3,28 @@ tests/test_decision_replay.py passes every digest assertion because ``_request_from()`` reconstructs the request out of ``envelope["binding"]`` -- which is the *enriched* form the evaluator hashed. That is a self-consistent -fake agreeing with itself, and it hid the gap below through three rounds of -digest work. +fake agreeing with itself, and it hid a real defect through three rounds of +digest work: the validator required byte-equality between the binding and our +unenriched request, which no real decision can satisfy. -This module builds the request the way the engine actually builds it and -compares it to a decision the engine actually received, so the gap is pinned -until flex-auth publishes the enrichment rule. +The rule was already published in flex-auth's canonical-request-digest.md +("Normalization"): the evaluator copies the request tenant onto subject and +resource, a registry hit copies type/tenant/selected attributes onto the refs, +and a consumer is told to compare structured binding fields to the proposed +action rather than re-hash its own request. This module builds the request the +way the engine actually builds it, compares it to a decision the engine actually +received, and proves the corrected check accepts it. """ from __future__ import annotations import json +from datetime import datetime, timedelta, timezone from pathlib import Path import pytest from secrets_engine.authorization import ( + binding_tuple, build_action_request, canonical_check_request, request_digest, @@ -105,19 +112,70 @@ def test_our_digest_cannot_match_a_real_binding(): assert canonical_check_request(bound) != canonical_check_request(sent) -def test_we_currently_reject_every_real_allow_and_that_is_fail_closed(): - """Pins today's behaviour honestly rather than asserting it is correct. +def test_the_live_allow_now_validates_under_the_documented_rule(): + """The real decision validates -- this is the fix, proved against the artifact. - Rejecting a valid allow is wrong, but it is wrong in the safe direction, so - it stays until flex-auth publishes which fields may be enriched. Guessing - the rule would mean accepting a binding that differs from our proposal in - some way we decided was benign -- the fail-open shape GH-DEC-2026-008 - rejected for vocabularies and FLEX-DEC-2026-007 rejected for digests. + canonical-request-digest.md "Normalization" is the published rule, and it + says a consumer must compare structured binding fields to the proposed + action rather than re-hash its own request. Under that rule this envelope, + which our previous byte-equality check rejected outright, is accepted. + + Only the lifetime is refreshed: the decision was issued on 2026-09-07 with a + bounded lifetime, and pinning a clock-dependent field is what the fixture + provenance forbids. """ - with pytest.raises(DecisionError, match="binding does not match request"): + env = _live() + now = datetime.now(timezone.utc) + env["lifetime"] = { + "kind": "bounded", + "not_before": (now - timedelta(minutes=1)).strftime("%Y-%m-%dT%H:%M:%SZ"), + "expires_at": (now + timedelta(minutes=10)).strftime("%Y-%m-%dT%H:%M:%SZ"), + } + result = validate_decision_envelope( + env, + _our_request(), + accepted_policy_packages={PACKAGE}, + accepted_policy_versions={VERSION}, + ) + assert result.action == "rotate" + assert result.subject_id == "secrets-engine" + assert result.decision_id == "decision:0f9c98f14545c42d" + + +def test_the_unrefreshed_live_decision_is_refused_on_lifetime(): + """It is a real expired allow, so it must be refused -- and for that reason. + + Reaching the lifetime check at all is the evidence that every binding check + before it now passes against a real decision. + """ + with pytest.raises(DecisionError, match="lifetime has expired"): validate_decision_envelope( _live(), _our_request(), accepted_policy_packages={PACKAGE}, accepted_policy_versions={VERSION}, ) + + +def test_the_digest_is_verified_against_the_binding_it_carries(): + """Independent recomputation, per the contract's own instruction. + + "To recompute independently, hash the same normalized tuple the binding + carries." So the digest is still checked -- it is simply checked for + self-consistency rather than against material we never sent. + """ + binding = _live()["binding"] + assert request_digest(binding_tuple(binding)) == binding["request_digest"] + + +def test_a_tampered_binding_still_fails_the_digest(): + """Self-consistency is a real check, not a formality.""" + env = _live() + env["binding"]["resource"]["attributes"]["stage"] = "build" + with pytest.raises(DecisionError, match="does not match"): + validate_decision_envelope( + env, + _our_request(), + accepted_policy_packages={PACKAGE}, + accepted_policy_versions={VERSION}, + ) diff --git a/workplans/SECRETS-WP-0009-glas-claude-native-delivery.md b/workplans/SECRETS-WP-0009-glas-claude-native-delivery.md index bf7fd5f..d591fee 100644 --- a/workplans/SECRETS-WP-0009-glas-claude-native-delivery.md +++ b/workplans/SECRETS-WP-0009-glas-claude-native-delivery.md @@ -96,6 +96,49 @@ authorization. No unsafe-demo switch and no human runtime token will be used to reach a prod lane. This task stays `wait`: activation is blocked on the owner access path, not on engine work or on approval shape. +Digest join corrected 2026-09-07. The live proof from 2026-09-06 (commit +`03c0569`) showed `validate_decision_envelope` rejecting every real allow. The +cause and the fix are both now settled, and the fix follows a published rule +rather than an inferred one. + +- **Cause.** The evaluator normalizes before hashing: the request tenant is + copied onto `subject` and `resource`, and a registry hit copies `type`, + `tenant` and selected `attributes` onto the refs. `binding.request_digest` is + therefore over material we never sent and cannot reproduce, so byte-equality + against our unenriched request is unsatisfiable, not merely mismatched. +- **The rule was already published.** flex-auth's `canonical-request-digest.md` + section "Normalization" states it, and instructs consumers to compare + structured `binding` fields to the proposed action and treat `request_digest` + as the evaluator's statement of what it hashed, recomputing independently over + the tuple the binding carries. Correction to the previous note: this repo + raised it with flex-auth as an unpublished gap and asked them to choose + between three shapes. It was already in their contract, and the answer was the + first of the three. Nothing was blocked on them. +- **Implemented.** `_require_binding_corresponds` enforces that everything we + proposed survives unchanged — tenant, action, context, `subject.id`/`type`, + `resource.id`/`type`/`system`, and every attribute we sent. Enrichment may add + only `type`, `tenant`, `attributes`; any other added field is refused, and an + enriched tenant must be the request tenant so a cross-tenant binding cannot + arrive wearing our request's clothes. `request_digest` is still checked, now + against `binding_tuple(binding)` for self-consistency. +- **Proved against the artifact.** `tests/test_live_decision_enrichment.py` + validates the real `decision:0f9c98f14545c42d` under the corrected rule, and + separately asserts the unrefreshed envelope is refused on lifetime — reaching + the lifetime check at all is the evidence every binding check now passes on a + real decision. Negative tests cover a restated `resource.attributes.stage`, a + foreign `subject.tenant`, and an unexpected enrichment field. +- **Why the fixtures could not catch it.** `_request_from()` rebuilds the + request out of the binding, i.e. the already-enriched form, so every digest + assertion hashed the evaluator's output and compared it to the evaluator's + output. The defect survived the excluded-fields fix, the + `approval_binding_digest` fix and the tenant fix because all three were tested + that way. Only a real request through the owner access path exposed it. + +Remaining before activation is now one external dependency, not two: +approval-engine must serve the claim endpoint so protocol step 1 can run and +`GH-DEC-2026-003` consume-before-OpenBao can be satisfied. The decision path +itself is proved end to end against the deployed pin. + Depends on SECRETS-WP-0007-T04 and SECRETS-WP-0008-T02/T06: canonical production authorization, successful consume and scoped service authority must exist. Current production exec refuses before OpenBao because durable access-engine