Add functions for inspecting TPE determining policies by john-h-kastner-aws · Pull Request #2401 · cedar-policy/cedar

john-h-kastner-aws · 2026-06-10T20:55:23Z

Adds TpeResponse::reason, TpeResponse::satisfied_permits, and TpeResponse::satisfied_forbids to the public TPE API to enable callers to find determining policies for partial authorization responses.

Description of changes

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

A breaking change requiring a major version bump to cedar-policy (e.g., changes to the signature of an existing API).
A backwards-compatible change requiring a minor version bump to cedar-policy (e.g., addition of a new API).
A bug fix or other functionality change requiring a patch to cedar-policy.
A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)
A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).
Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that cedar-spec (choose one, and delete the other options):

Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.
Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in cedar-spec, and how you have tested that your updates are correct.)
Requires updates, but I do not plan to make them in the near future. (Make sure that your changes are hidden behind a feature flag to mark them as experimental.)
I'm not sure how my change impacts cedar-spec. (Post your PR anyways, and we'll discuss in the comments.)

I confirm that docs.cedarpolicy.com (choose one, and delete the other options):

Does not require updates because my change does not impact the Cedar language specification.
Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in cedar-docs. PRs should be targeted at a staging-X.Y branch, not main.)
I'm not sure how my change impacts the documentation. (Post your PR anyways, and we'll discuss in the comments.)

…peResponse::satisfied_forbids` Signed-off-by: John Kastner <jkastner@amazon.com>

Signed-off-by: John Kastner <jkastner@amazon.com>

Copilot

Pull request overview

This PR extends the (experimental) TPE public API to let callers inspect which policies were satisfied/determining for a partial-authorization outcome, and adjusts cedar-policy-core TPE internals to expose policy IDs by reference.

Changes:

Add TpeResponse::reason, TpeResponse::satisfied_permits, and TpeResponse::satisfied_forbids to inspect determining/satisfied policies for partial decisions.
Change cedar-policy-core::tpe::ResidualPolicy::get_policy_id to return &PolicyID and update internal logic/tests accordingly.
Add/update tests and a changelog entry for the new API surface.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

File	Description
cedar-policy/src/api/tpe.rs	Adds new `TpeResponse` inspection methods and corresponding tests.
cedar-policy/CHANGELOG.md	Documents the new TPE inspection API (needs alignment with actual changes).
cedar-policy-core/src/tpe/response.rs	Changes `ResidualPolicy::get_policy_id` to borrow the ID and updates set construction.
cedar-policy-core/src/tpe.rs	Updates tests to accommodate the borrowed policy ID accessor.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

luxas

Thanks, this is a good addition to the API surface 👍

What the Kubernetes integration in particular is interested in, is understanding that when given decision=Deny, did that originate from a matched deny policy or just no matching policies? In the latter case, one could treat that as a NoOpinion mode, and let other subsequent authorizers (if many) have their say.

luxas · 2026-06-11T13:49:05Z

+    /// These are a subset of the determining policies for any subsequent
+    /// concrete reauthorization.


I'm not sure I understand the for any subsequent concrete reauthorization part of the comment, the returned policies already evaluated to true, so there is no need for reauthorization?

The idea is that if you were to do concrete authorization, there might be more determining policies that aren't represented here. If you want to take some action based on which policies apply to a request, this might be important.

expanded comments substantially. let me know if it makes sense

Maybe determining policies for any subsequent concrete reauthorization could be reworded as determining policies of an concrete authorization request with a complete Entity store? I guess that's what you mean?

Signed-off-by: John Kastner <jkastner@amazon.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

luxas

Thanks, I like it 👍
Should we make the error change in this or another PR? It could be done in another.

luxas · 2026-06-15T17:28:19Z

+    /// These are a subset of the determining policies for any subsequent
+    /// concrete reauthorization.


Maybe determining policies for any subsequent concrete reauthorization could be reworded as determining policies of an concrete authorization request with a complete Entity store? I guess that's what you mean?

Signed-off-by: John Kastner <jkastner@amazon.com>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

github-actions · 2026-06-17T13:41:22Z

Coverage Report

Head Commit: ce2ef49e8fa0379bc475e255b52c69de4c4335fa

Base Commit: 19aea85bd58786eda7db8dc36c21f3b4afd1f8f5

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 100.00%

Status: PASSED ✅

Details

File	Status	Covered	Coverage	Missed Lines
cedar-policy-core/src/tpe/response.rs	🟢	41/41	100.00%
cedar-policy/src/api/tpe.rs	🟢	43/43	100.00%

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.91%

Status: PASSED ✅

Details

Package	Status	Covered	Coverage	Base Coverage
cedar-language-server	🟢	4722/5102	92.55%	--
cedar-policy	🟢	4573/5687	80.41%	--
cedar-policy-cli	🟡	1257/1649	76.23%	--
cedar-policy-core	🟢	24599/27905	88.15%	--
cedar-policy-formatter	🟢	914/1088	84.01%	--
cedar-policy-symcc	🟢	6790/7287	93.18%	--
cedar-wasm	🔴	0/28	0.00%	--

luxas

Approving with some small renames that could be followed up upon if we like.
Thanks, this should give folks quite a lot of data to play with to deepen the understanding of the decision.

luxas · 2026-06-18T20:27:37Z

@@ -130,28 +136,32 @@ impl<'a> Response<'a> {
            match rp.get_effect() {
                Effect::Forbid => {
                    if r.is_true() {


nit: feels like this could be a good candidate to have an enum / match statement for in the future.

We could write this as a match on the residual, but I don't think I see how it would make the code any nicer

luxas · 2026-06-18T20:30:01Z

            .map(|id| self.residuals.get(id).unwrap())
    }

+    /// Get trivially erroring permit residual policies


Suggested change

/// Get trivially erroring permit residual policies

/// Get permit policies that errored during evaluation

nit: not sure I understand the word trivially in this context, maybe just drop it?

luxas · 2026-06-18T20:30:11Z

            .map(|id| self.residuals.get(id).unwrap())
    }

+    /// Get trivially erroring forbid residual policies


Suggested change

/// Get trivially erroring forbid residual policies

/// Get forbid policies that errored during evaluation

luxas · 2026-06-18T20:33:38Z

+    /// Get the forbid policies that did not reach a concrete value or error for the partial request.
+    ///
+    /// This function only returns the `PolicyId`s for residual policies.
+    /// To access the residual policy conditions, use [`TpeResponse::nontrivial_residual_policies`].


this reminds me that we should probably rename TpeResponse::nontrivial_residual_policies to just TpeResponse::residual_policies now, as we've anyways factored out the "trivial" ones to their own constructors and thus don't need that trivial/nontrivial distinction. That'd then be consistent with calling the other getters here "just" residual_forbids/permits

I need to think more about the naming here. In some places "residual" refers to any output of partial evaluation (even if it reaches a concrete value). In other place "residual" specifically means a policy which didn't reach a concrete value.

luxas · 2026-06-18T20:36:23Z

+    /// must return a concrete `Deny`.
+    pub fn true_forbids(&self) -> impl Iterator<Item = &PolicyId> {
+        self.0
+            .satisfied_forbids()


seems like one missing getter rename

luxas · 2026-06-18T20:36:48Z

+    ///   policy.
+    pub fn true_permits(&self) -> impl Iterator<Item = &PolicyId> {
+        self.0
+            .satisfied_permits()


one missing getter rename, or don't we want to update all of these at the same time?

Add TpeResponse::decision, TpeResponse::satisfied_permits, and `T…

6e677ed

…peResponse::satisfied_forbids` Signed-off-by: John Kastner <jkastner@amazon.com>

john-h-kastner-aws requested a review from Copilot June 10, 2026 20:55

Copilot started reviewing on behalf of john-h-kastner-aws June 10, 2026 20:55 View session

fmt

8197a5a

Signed-off-by: John Kastner <jkastner@amazon.com>

Copilot AI reviewed Jun 10, 2026

View reviewed changes

This comment was marked as outdated.

Sign in to view

Apply suggestions from code review

0276383

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

john-h-kastner-aws requested review from luxas and victornicolet June 10, 2026 21:24

This comment was marked as outdated.

Sign in to view

victornicolet reviewed Jun 11, 2026

View reviewed changes

Comment thread cedar-policy/src/api/tpe.rs Outdated

luxas reviewed Jun 11, 2026

View reviewed changes

Expand doc comments

2da907c

Signed-off-by: John Kastner <jkastner@amazon.com>

This comment was marked as outdated.

Sign in to view

john-h-kastner-aws requested a review from Copilot June 15, 2026 13:57

Copilot started reviewing on behalf of john-h-kastner-aws June 15, 2026 13:57 View session

Copilot AI reviewed Jun 15, 2026

View reviewed changes

Comment thread cedar-policy-core/src/tpe/response.rs

Comment thread cedar-policy/src/api/tpe.rs

Comment thread cedar-policy/CHANGELOG.md Outdated

Potential fix for pull request finding

a1b7f39

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

This comment was marked as outdated.

Sign in to view

luxas reviewed Jun 15, 2026

View reviewed changes

john-h-kastner-aws added 2 commits June 16, 2026 21:43

Expand API furhter

85ce1f9

Signed-off-by: John Kastner <jkastner@amazon.com>

tweak

a1b0e6a

Signed-off-by: John Kastner <jkastner@amazon.com>

john-h-kastner-aws requested a review from Copilot June 16, 2026 21:53

Copilot started reviewing on behalf of john-h-kastner-aws June 16, 2026 21:53 View session

fmt

feccd1a

Signed-off-by: John Kastner <jkastner@amazon.com>

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread cedar-policy/src/api/tpe.rs Outdated

Comment thread cedar-policy/CHANGELOG.md Outdated

Apply suggestions from code review

ce2ef49

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

luxas approved these changes Jun 18, 2026

View reviewed changes

victornicolet approved these changes Jun 19, 2026

View reviewed changes

john-h-kastner-aws merged commit ce1cacd into main Jun 19, 2026
34 checks passed

john-h-kastner-aws mentioned this pull request Jun 22, 2026

Update TPE DRT for API changes cedar-policy/cedar-spec#973

Merged

john-h-kastner-aws deleted the tpe-det-policies branch June 30, 2026 18:51

		/// These are a subset of the determining policies for any subsequent
		/// concrete reauthorization.

	/// Get trivially erroring permit residual policies
	/// Get permit policies that errored during evaluation

	/// Get trivially erroring forbid residual policies
	/// Get forbid policies that errored during evaluation

Uh oh!

Conversation

john-h-kastner-aws commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Issue #, if available

Checklist for requesting a review

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

luxas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

luxas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 17, 2026

Coverage Report

Coverage of Added or Modified Lines of Rust Code

Coverage of All Lines of Rust Code

Uh oh!

luxas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

john-h-kastner-aws commented Jun 10, 2026 •

edited

Loading