Skip to content

TPE: Guard against stack overflow#2298

Merged
john-h-kastner-aws merged 1 commit intocedar-policy:mainfrom
luxas:tpe-stack-check
Apr 8, 2026
Merged

TPE: Guard against stack overflow#2298
john-h-kastner-aws merged 1 commit intocedar-policy:mainfrom
luxas:tpe-stack-check

Conversation

@luxas
Copy link
Copy Markdown
Member

@luxas luxas commented Apr 8, 2026

Description of changes

Broken out from #2162.
I noticed this mis-match in behavior between normal eval and partial eval, so adding the stack overflow guard to TPE as well.

Issue #, if available

Checklist for requesting a review

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

  • A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)

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

  • 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.

I believe this is not covered by cedar-spec?

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.

Signed-off-by: Lucas Käldström <lucas.kaldstrom@upbound.io>
@luxas luxas force-pushed the tpe-stack-check branch from e55f639 to d5b53d3 Compare April 8, 2026 12:51
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

Coverage Report

Head Commit: d5b53d31d8c13687742d2ff9d03a1d90e4f3cd25

Base Commit: 03d1ca290f0c27afbb138c6c69990ec801944c4b

Download the full coverage report.

Coverage of Added or Modified Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 75.00%

Status: FAILED ❌

Details
File Status Covered Coverage Missed Lines
cedar-policy-core/src/evaluator.rs 🟢 1/1 100.00%
cedar-policy-core/src/tpe/evaluator.rs 🔴 2/3 66.67% 68

Coverage of All Lines of Rust Code

Required coverage: 80.00%

Actual coverage: 87.65%

Status: PASSED ✅

Details
Package Status Covered Coverage Base Coverage
cedar-language-server 🟢 4722/5102 92.55% 92.55%
cedar-policy 🟡 4227/5353 78.97% 78.97%
cedar-policy-cli 🟡 1210/1614 74.97% 74.97%
cedar-policy-core 🟢 24010/27282 88.01% 88.01%
cedar-policy-formatter 🟢 914/1088 84.01% 84.01%
cedar-policy-symcc 🟢 6752/7262 92.98% 92.98%
cedar-wasm 🔴 0/28 0.00% 0.00%

@luxas
Copy link
Copy Markdown
Member Author

luxas commented Apr 8, 2026

I didn't see any unit tests for this in the real evaluator, so do we add something to both places, or just ignore the coverage check for the modified lines?

@john-h-kastner-aws
Copy link
Copy Markdown
Contributor

I didn't see any unit tests for this in the real evaluator, so do we add something to both places, or just ignore the coverage check for the modified lines?

We probably didn't have the coverage checks when we added the guard to the evaluator, but I'm fine with leaving it uncovered.

@luxas
Copy link
Copy Markdown
Member Author

luxas commented Apr 8, 2026

Cool, then we can merge this 👍 Does GitHub let us merge this even though one check is failing? 🤔

@john-h-kastner-aws john-h-kastner-aws merged commit 6316d16 into cedar-policy:main Apr 8, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants