fix(mandate): fail closed on grant content the service cannot evaluate - #531
Merged
Conversation
A mandate is an authorization document, so an input this version does not
understand may be one that RESTRICTS the grant. Both places that could
happen resolved as "more authority", which is the wrong direction for an
authorization decision to fail in.
Unknown top-level fields. MandateDocument carried Pydantic's default
extra="ignore". The signature is computed over model_dump(), so a mandate
issued at a newer schema version lost its unknown fields before
canonicalization and failed as "one or more signatures are invalid",
indistinguishable from tampering. MandateDocument and IssueMandateRequest
now set extra="forbid": an unknown field is a parse error naming the field.
Unevaluated conditions. `conditions` is inside the signed grant and nothing
in this service evaluates it. An issuer could sign {"env": "staging"} and
/verify would answer valid: true in production, with no way for a caller to
tell that verdict from one over a mandate carrying no conditions at all.
/verify gains a sixth check, conditions_evaluable, which fails when
conditions are present and unevaluated. A caller holding its own evaluator
passes conditions_evaluated=true to assert it did that work; the flag is an
assertion, not a bypass.
The principle is standards-independent, and the same one argued publicly in
ocsf#1756: an unevaluable restriction cannot bound authority, so it must not
read as satisfied.
Breaking for external callers submitting populated conditions to /verify.
Nothing in this repository does: the smoke script sends an empty map, the
overspend demo sends none. The gateway's enforcement path is unaffected, it
calls /mandates/{id}/spend directly rather than /verify.
Seven tests cover both failure modes, the escape hatch, and that conditions
stay inside the signature so stripping them cannot buy a clean verdict.
Five of the seven fail against the previous code. Mandate suite 59 passed /
6 skipped, gateway mandate tests 17 passed, ruff check and format clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XyU3PRkgvMxgimkPBC5726
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Levaj2000
marked this pull request as ready for review
September 11, 2026 14:36
Levaj2000
added a commit
that referenced
this pull request
Sep 11, 2026
…532) PR #531 made /verify honest about conditions it could not evaluate. It did not make anything evaluate them. The gateway, which actually gates requests, never saw them at all: it draws against /mandates/{id}/spend and that response carried no conditions. An issuer could sign {"env": "prod"} and a staging agent would spend against the grant unimpeded. Where the evaluation lives, and why it is not the gateway. The check has to be atomic with the spend. Evaluating at the gateway means evaluating after the draw returns, by which point an accepted spend is already recorded against the budget of a request about to be denied. So the gateway sends the context it alone holds and the service that owns the grant applies its own terms before mutating state. The context is agent metadata plus the endpoint and method. Those two are reserved and win over a stored attribute of the same name: an agent whose metadata claims endpoint=/v1/safe must not satisfy a condition pinning the mandate there while calling something else. Conditions reuse common.policy.eval.evaluate_when, the same function behind a policy's `when` clause, so a mandate condition and a policy condition mean the same thing and fail the same way. A missing key or an unknown operator is a failed match, never a pass, so a caller that sends no context is denied on a conditioned mandate. That is the intended rollout direction. Ordering: after status, before currency and limit. A grant that does not cover the request is not a budget question, and asking how much is left on an inapplicable grant is the wrong question. Evidence: deny_reason gains mandate_conditions_unmet, and the per-condition results ride the audit row and the OCSF unmapped.mandate block as conditions_checked / conditions_failed, so a denial says not just "denied" but which field failed with expected against actual. One judgment call, made explicit in the test that pins it. A condition-failing settlement is refused rather than recorded, unlike an over-limit settlement. Incrementing spent_cents asserts this grant funded the spend, which is what the failed condition denies. Nothing is lost: the audit row and event log capture the attempt either way, only the budget counter stays untouched. Thirteen new tests. Mandate suite 68 passed / 6 skipped, gateway suite 142 passed, ruff check and format clean. Two pre-existing failures in this environment (a rate-limiter timing test and seven OCSF export cases) were confirmed identical on clean main and are unrelated. Claude-Session: https://claude.ai/code/session_01XyU3PRkgvMxgimkPBC5726 Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A mandate is an authorization document, so an input this version does not understand may be one that restricts the grant. Both places that could happen resolved as "more authority", which is the wrong direction for an authorization decision to fail in.
Two failure modes, one principle
Unknown top-level fields.
MandateDocumentcarried Pydantic's defaultextra="ignore", so an unrecognized field was silently dropped. The signature is computed overmodel_dump()(signing._build_signable_payload), so a mandate issued at a newer schema version lost its unknown fields before canonicalization and then failed with "one or more signatures are invalid" — indistinguishable from tampering, and the wrong thing to send an operator chasing.Unevaluated conditions. This is the live one.
conditionsis inside the signed grant and nothing in this service evaluates it. An issuer could sign{"env": "staging"}andPOST /api/v1/mandates/verifywould answer:{"valid": true, "checks": {"signatures_valid": true, "status_active": true, "not_expired": true, "within_spend_limit": true, "scope_sufficient": true}}in production, ignoring a restriction its issuer signed. A caller could not tell that verdict apart from one over a mandate carrying no conditions at all.
The change
MandateDocumentandIssueMandateRequestsetextra="forbid". An unknown field is a parse error naming the field rather than a silent widening of authority. On the issue request this also means a misspelledspend_limmitis rejected instead of quietly producing a mandate with no spend limit./verifygains a sixth check,conditions_evaluable, which fails when conditions are present and unevaluated.VerifyMandateRequestgainsconditions_evaluated: bool = False. A caller holding its own evaluator, a gateway with request context this service does not have, asserts it did that work. It is an assertion of responsibility, not a bypass.The principle is standards-independent, and the same one argued publicly in ocsf#1756: an unevaluable restriction cannot bound authority, so it must not read as satisfied. That argument was made outward before being adopted here; this closes the gap.
Blast radius, stated plainly
Breaking for external callers that submit mandates with populated
conditionsto/verify. They will now getvalid: falsewith an error naming the condition keys and telling them to evaluate and resubmit.Nothing in this repository is affected. The smoke script sends
"conditions": {}, the overspend demo sends none, and no test used the field.The gateway's live enforcement path is unaffected. It calls
/api/v1/mandates/{id}/spenddirectly, not/verify. Worth naming as a separate gap rather than quietly widening this PR:conditionsis unenforced on both paths. This change makes/verifyhonest about that. It does not make the gateway evaluate conditions, which is a real piece of work and a different change.Tests
Seven new tests in
mandate/tests/test_strict_unknown_fields.pycover both failure modes, the escape hatch, and that conditions stay inside the signature so an attacker cannot strip the field to buy a clean verdict.Red-then-green verified: five of the seven fail against the previous code, with the sixth check reported as
KeyError: 'conditions_evaluable'.ruff check/ruff format --checkThe 6 skips are the ML-DSA cases that need
liboqs, skipped on main as well.🤖 Generated with Claude Code
https://claude.ai/code/session_01XyU3PRkgvMxgimkPBC5726
Generated by Claude Code