feat: Add support for partial evaluation#82
Conversation
|
@swenger - thank you for this PR. I will take a look when I can. That may be in a couple days. Can you share how you're using or planning to use this in your own application? |
|
Thanks @skuenzli!
Sure! We want to control access to data that's stored in a SQL database. Especially for queries returning more than one result, that means that at least some of the access control needs to happen in SQL (keeping only the rows the user is allowed to see). In order to keep the amount of work in the Currently, implementing this would require starting the cedar CLI in a new process, which isn't great for latency and scalability (besides not being very convenient API to code against). Does this sound reasonable? |
|
Thank you @swenger - your use case is great context and I understand it completely -- to the extent I need to ;) |
skuenzli
left a comment
There was a problem hiding this comment.
Thank you for proposing an implementation to support partial evaluation. Overall, I think is_authorized_partial uses a good approach.
I've added a several comments and requests.
If we can work through those, then I think we can land this change.
|
@swenger - thanks for the updates and your patience. I wasn't able to review the changes today, but will try to review it tomorrow. |
PR was branched from an older main and had transitive-dep churn unrelated to partial-eval. Restore the lock to main's state to keep the PR's dependency-change surface aligned with its scope. Cedar-policy and its family already resolve to 4.8.2 on main, so no manifest pin is required. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The partial-eval response populated id_annotations_by_reason from definitely_satisfied and all_residuals, but not from definitely_errored. Errored policies are in neither of the first two sets, so callers inspecting diagnostics.errors had no way to recover the @id label of a policy that errored at evaluation. Add a third lookup loop over errored_ids alongside the existing two, and a parallel test covering both satisfied and errored policies with @id annotations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Documents the remediation for the security review finding on is_authorized_partial: schema-based context type-checking is skipped for fields that remain unknown (notably, action-typed context shapes when action is unbound), so callers must re-run is_authorized once unknowns are bound rather than treating a residual or a partial Allow as a final decision. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Resolves CHANGELOG.md conflict by keeping both Unreleased entries in Keep a Changelog section order (Added, then Changed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No changes in performance of existing codepaths were expected. The existing benchmark suite passes, verifying that: |
skuenzli
left a comment
There was a problem hiding this comment.
Thanks for making the requested adjustments! I have also clarified and fixed a few things. I think this PR is ready to merge.
Notably, I restored the Cargo.lock from main to avoid picking up some dependency changes that were introduced early in your branch. The result is that there are no dependency changes except enabling cedar-policy 4.8.2's partial-eval feature.
Please review the full PR including my changes. Then either resolve or add/update conversations if you think we need to make more changes.
|
Thanks @skuenzli! I've made one minor change for you to review (see #82 (comment)) and left another question (#82 (comment)), but generally all looks good to me. Feel free to merge if you agree! I've updated my SQL generation code to use the latest version (with nontrivial residuals etc.) and it seems to work fine 👍 |
Grounds the is_authorized_partial docstring warning with a concrete test. When schema is provided but action is unknown, Cedar's action-specific context type check is not performed, and ill-typed context values (a string where the schema requires Boolean) pass through without a diagnostic. The test deliberately uses a policy that doesn't constrain action, so the result is Decision.Allow despite the unknown action — making the missing type check the actually-surprising part. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
That's terrific! Thank you for testing. |
…bling `test_partial_with_schema_unknown_action_skips_context_type_check_when_complete` inverted its scenario in the name. Rename to `test_partial_with_schema_known_action_enforces_context_type_check` so the two paired tests read as a clear input/outcome contrast: unknown action → skips context type check known action → enforces context type check Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
skuenzli
left a comment
There was a problem hiding this comment.
This addition looks great. Thank you for your contribution and patience iterating on the solution.
|
Thanks @skuenzli for the review and great collaboration! I think what we merged is indeed much better than what I started out with ❤️ |
|
@swenger - I'm so glad you feel that way. I think so too! "Simple" isn't easy! |
Summary
Adds
is_authorized_partial(request, policies, entities, schema=None), a Python binding for Cedar's partial-evaluation authorizer. Request fields that areNoneor absent are treated as unknowns; the authorizer returnsDecision.Allow/Decision.Denywhen the unknowns can't change the outcome, orDecision.NoDecisionplus residual policies (as Cedar JSON) for the caller to re-evaluate once unknowns are bound.PartialAuthzResultmirrorsAuthzResult's shape (decision/correlation_id/diagnostics/metrics) and addsmay_be_determining,must_be_determining,nontrivial_residuals,unknown_entitiesto diagnostics. There is no engine version change. cedarpy continues to use cedar-policy 4.8.2 with thepartial-evalCargo feature enabled.The docstring and CHANGELOG both warn that partial-eval results are not a final authorization decision: callers must re-run
is_authorizedonce unknowns are bound, since schema type-checking (including action-typed context shapes) is skipped while fields remain unknown.Fixes #28.
Test plan
make integration-testspassespytest tests/unit -v— full suite passes, including 27 new tests intests/unit/test_authorize_partial.pycovering: unknowns at each request slot, definitive Allow/Deny with unknowns, schema interactions, errored-policy diagnostics, residual JSON structure,@idannotation surfacing (incl. on errored policies), and three "progressive request filling" cases that exercise partial → complete request transitionsmake benchmark-compare— no regression on the post-merge benchmark gateis_authorized_partialcan be inspected and acted on by a caller