Add typechecker DRT target comparing the typed expression (#840) - #995
Add typechecker DRT target comparing the typed expression (#840)#995repowazdogz-droid wants to merge 10 commits into
Conversation
victornicolet
left a comment
There was a problem hiding this comment.
Thank you for the contribution!
Can you confirm you were able to run the target, and didn't find encounter any failures?
| @@ -0,0 +1,11 @@ | |||
| # Typed-expression DRT notes (issue #840) | |||
There was a problem hiding this comment.
I think the notes, including the other files in this repository, should be ommitted from this PR.
| -/ | ||
|
|
||
| import CedarFFI.Main | ||
| import CedarFFI.TypedExprDRT |
There was a problem hiding this comment.
nit: perhaps a better name can be found, the FFI is not specific to the DRT.
| /// blocks, so it can no longer fail a run. It exists to measure how often a | ||
| /// divergence class appears across generated input before deciding whether | ||
| /// that class is a defect or a declared difference. Leave it unset in CI. | ||
| fn survey_mode() -> bool { |
There was a problem hiding this comment.
Do we need this survey mode in the final PR? Was it used to test the new target?
|
Yes, it runs at scale. 40,000 executions against the current corpus of 5,379 inputs, of which 34,832 reached the comparison and produced 101,401 (policy, environment) pairs. I would rather not answer that as "no failures", because that is not what happened. Seven phantom-divergence classes came out of building and running this, every one of them the harness being wrong rather than either typechecker. The seventh only surfaced once I removed the survey switch: After those fixes, The first is mine and I will fix it. The second is yours to decide and I have deliberately left it alone. Declaring it a known difference is what makes the target green and lets it assert. Declaring it also answers the scope question this PR raises, and that answer belongs to you rather than to me. So I would rather ask than assume: should the I have not implemented either and will follow your answer. Your other three points are done locally, pending that answer:
|
f59daff to
4b95703
Compare
…olicy#840) Compares the TypedExpr each validator produces, not just whether both agree that validation passed. Rust side is Typechecker::typecheck_by_single_request_env; Lean side is a new typecheckPolicyTyped FFI entry point over Validation.typecheckPolicy. Both sides are rendered independently into a neutral Node type with the type annotation carried as a compared field. The Lean tree is not decoded into the Rust AST and neither side is projected through an untyped expression: the annotation is the object under test here, so any representation without an annotation slot would erase it. Two phantom-divergence classes found and fixed while building this. Both would have manufactured findings: 1. cedar-policy-core enables serde_json's preserve_order, so object key order is insertion order. Lean emits entity types as {"path":..,"id":..} and the natural Rust construction order is the reverse, which reported a mismatch on every entity-typed node. Fixed by sorted rendering in canonical_scalar. Guarded by a regression test that was checked to fail when the fix is removed. 2. Lean's derived ToJson does not emit constructor fields in declaration order ("and" comes back as ty, b, a). Positional child comparison reported a mismatch on every binary node. Fixed by canonical child ordering on both sides. Controls are two-halved: each asserts the clean pair produces zero divergences and the tampered pair produces exactly one classified divergence, in the same run. drt-840-notes/probe_full.lean and probe_like.lean dump Lean's derived ToJson for every TypedExpr constructor and CedarType shape; the renderer was written against that output rather than against a guess at the encoding. 13/13 tests pass, including an end-to-end run against the real Lean backend. That run makes no assertion that the two typecheckers agree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Adds fuzz_targets/typed-expression-drt.rs and the input plumbing that turns one generated schema and policy set into the two views the comparison needs. Running it found four more harness defects that three hand-written policies could not, all of which would have been reported as findings about the validators: 1. Round-tripping a policy set through `to_string` and `parse_policyset` is not id-preserving: the parser assigns fresh policy0, policy1, ... and every pair then failed to align. Now parsed one policy at a time under its original id. 2. `Display for PolicyID` escapes via `escape_debug`, so an id holding a control character rendered as \0 on the Rust side and as a raw NUL on the Lean side. Now taken via AsRef. 3. `Display for EntityUID` renders the eid through `Eid::escaped()` while Lean emits it raw, so the action component of the environment key had the same problem. Now built field by field. 4. Template-linked policies were skipped on the Rust side but still returned by Lean, leaving pairs that were never compared reported as unmatched environments. Now dropped from both sides. After these, 40000 fuzz executions complete with zero harness problems. The target asserts on findings by default so it sits on the failure path. CEDAR_TYPED_EXPR_SURVEY counts and prints them instead, which weakens the target to an observer and is for measuring a divergence class before deciding what it is. Unset in CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Rust's typechecker folds a statically-true policy condition to Lit(true) while Lean keeps the scope-constraint conjunction. That was landing in SHAPE_MISMATCH alongside genuine constructor disagreements and, at 45536 occurrences, burying them. It now has its own bucket. The folded side is a leaf, so there is no subtree to descend into and nothing is recovered by continuing; what is recovered is the type, since both sides still annotate the whole condition. Those annotations are compared and reported, and a folded pair whose types DISAGREE stays a finding rather than being reconciled. A non-boolean literal opposite a compound expression is still a shape mismatch, which the control checks. Survey mode now prints the pair count on every execution, not only on executions that produced a finding, so the tally has a denominator. Documents the cause of the var-against-lit result: Lean's typecheckPolicy calls substituteAction before typechecking; Rust has no equivalent on this path. Left as a finding rather than normalised away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
The PR draft, reply drafts and the raw 6304-line survey tally were session working files and do not belong in the repository. The probes and run script stay: the Rust renderer was written against the probe output, so they are how the encoding gets re-checked when the Lean TypedExpr changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
Four changes, all from preparing the branch for review.
The Rust side rendered entity eids through `Eid::escaped()`, a display
convention that turns U+0007 into the six characters `\u{7}`, which serde
then escapes a second time. Lean emits the raw character and escapes it
once. Every entity literal holding a control character therefore mismatched
in the harness rather than in either typechecker. This is a seventh
phantom-divergence class, and it has the same cause as the environment-key
fix at a site that fix did not reach. Two tests cover it: one asserting the
escaping alone is never reported as a divergence, one asserting a genuinely
different eid is still caught, so the fix widened nothing.
Removed the `CEDAR_TYPED_EXPR_SURVEY` environment variable. It switched the
target from asserting to counting, which weakens the target and does not
belong in the repository. The measured rate stays in the PR description.
Renamed `CedarFFI/TypedExprDRT.lean` to `CedarFFI/TypedExpr.lean` and
updated the import in `CedarFFI.lean`, matching the existing module naming
rather than naming the module after the fuzz target that consumes it.
Dropped the `drt-840-notes/` probe files.
Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
4b95703 to
c3938c3
Compare
`PolicyCheck::Irrelevant` means the Rust typechecker found the policy always false for an environment, from the scope or from a condition it folded to false. For some of those (policy, env) pairs Lean's `typecheckPolicy` returns an error. Neither side then offers a typed expression and both decline for the same reason, so reporting it as an `OutcomeMismatch` claimed a disagreement that is not there. Those pairs now land in `StaticallyFalsePolicy`, which `is_declared_difference` covers. They are still recorded, counted and printed; they are just not findings. This is a declaration about what the two implementations mean, not a repair of either, so it is written into the module's declared-normalisations list rather than applied silently. Measured over the 5,379-input corpus, 26,745 compared (policy, env) pairs, same denominator before and after: CONSTANT_FOLDED_CONDITION 15,565 -> 15,565 SHAPE_MISMATCH 2,676 -> 2,676 OUTCOME_MISMATCH 1,007 -> 0 STATICALLY_FALSE_POLICY 0 -> 1,007 Exactly the 1,007 pairs change bucket and nothing else moves, so no comparison coverage is traded for the reclassification. The obvious alternative, mapping `Irrelevant` to `None` so both sides land in the agree-on-rejection arm, was tried first and is wrong. Lean returns an expression for 7,969 of the 8,976 `Irrelevant` pairs and errors on only 1,007, so that mapping takes OUTCOME_MISMATCH from 3.77% to 29.80% and stops comparing 7,969 pairs whose trees both sides produce. Three tests, all bound to the production predicates rather than re-deciding the rule: * the declared case, end to end against the real Lean backend, from a corpus input reproduced verbatim; * the guards against it widening, which is that a Rust success against a Lean error is still an outcome divergence, that the outcome bucket is still a finding in both directions, and that a statically-false pair Lean does type is still compared as a tree; * the `OutcomeMismatch` arm driven through `run_typed_expr_drt` itself. The corpus has no pair where one side typechecks a policy the other rejects, so that arm is otherwise never reached end to end. The test forces it by giving the Rust and Lean views different policies under one id, which the driver already documents as the caller's responsibility. It shows the routing works, not that the two typecheckers disagree on real input. The first two fail if `Irrelevant` is remapped to `None`. Signed-off-by: wazdogz <233830950+repowazdogz-droid@users.noreply.github.com>
|
The branch is at I said I would map Over the 5,379-input corpus, 26,745 compared (policy, env) pairs, the cross-tab of the Rust
There are 8,976 The 1031/1031 figure I quoted earlier does not support the change and I should not have offered it as evidence. Under the old mapping What is on the branch instead is to treat Rust-
Exactly the 1,007 pairs change bucket and nothing else moves, so the reclassification costs no comparison coverage. Worth being precise about what that zero is: those pairs are not repaired, they are relabelled, and Three tests cover it, all bound to the production predicates rather than restating the rule in the test, since a test that re-implemented the mapping would have passed against the refuted version.
The Two corrections to locations I gave earlier. The mapping is in Your other three points are done: |
Closes #840.
The existing validator DRT compares whether Rust and Lean agree that validation
passed. This adds a target that compares the
TypedExpreach side produces, so adisagreement about the annotated AST is visible even when both sides reach the same
verdict.
Rust side is
Typechecker::typecheck_by_single_request_env. Lean side is a newtypecheckPolicyTypedexport overValidation.typecheckPolicy. Results are alignedby policy id and by a
(principal, action, resource)environment key that both sidesproduce independently.
How the comparison works
Each side is rendered independently into a small neutral node type, with the type
annotation carried as a compared field. The Lean tree is not decoded into the Rust
AST, and neither side is projected through an untyped expression. The annotation is
the object under test here, so any representation without an annotation slot would
erase exactly what the target exists to compare, and a decoder would have to hardcode
the
TypeagainstCedarTypecorrespondence that is under test. This is discussedin the issue thread.
Declared normalisations, each with a test:
LikeandIsare AST variants; Lean models both asunaryApp. The Rust sideis folded to the Lean shape. A matching case agrees and a differing pattern is still
caught.
a divergence while a duplicated or missing key still is.
SlotandUnknownhave no Lean counterpart and are reported as unsupported ratherthan as a shape mismatch.
Lit(true)and Lean keeps as aconjunction gets its own bucket. The folded side is a leaf, so the types the two
sides assign to the whole condition are compared and reported instead. A folded pair
whose types disagree stays a finding.
Six phantom-divergence classes found while building this
Each of these made the harness report a difference that was an artifact of the two
encodings rather than anything about the validators. They are in their own commit.
cedar-policy-coreenables serde_json'spreserve_order, so object key order isinsertion order. Lean emits entity types as
{"path": .., "id": ..}and the naturalRust construction order is the reverse, so every entity-typed node mismatched.
ToJsondoes not emit constructor fields in declaration order(
andcomes back asty, b, a), so positional child comparison mismatched on everybinary node.
to_stringandparse_policysetis notid-preserving: the parser assigns fresh
policy0, policy1, ...and every pair thenfailed to align.
Display for PolicyIDescapes viaescape_debugwhile Lean emits raw, so idsholding control characters never matched.
Display for EntityUIDrenders the eid throughEid::escaped(), giving the actioncomponent of the environment key the same problem.
leaving pairs that were never compared reported as unmatched environments.
The guard for the first was checked by removing the fix and confirming the test fails
with the predicted output, so it is not a test that passes for the wrong reason.
What the target establishes, and what it does not
Controls are two-halved: each asserts that the clean pair produces zero divergences
and that the tampered pair produces exactly one classified divergence, in the same
run, so a control that catches its plant has also shown the correct case passing. 14
unit tests plus an end-to-end run against the real Lean backend.
No claim is made that the two typecheckers agree. The end-to-end test asserts only
that the harness ran and produced no harness problems. Asserting agreement would turn
a real disagreement into a broken test, and the point of the target is to report
disagreements rather than to encode an expected answer. Agreement is also bounded by
the shared Cedar specification: if the spec is wrong, both sides can agree and both be
wrong.
Over 40000 fuzz executions the target completes with zero harness problems, covering
71865 (policy, environment) pairs. Of those pairs, 10.65% carry the
Var(Action)against literal result described below.
That percentage needs reading carefully and I would rather state the limit than let it
be taken for more than it is. libFuzzer is coverage-guided, so the executions are not
independent draws, and the generated schemas skew small. 10.65% describes this
generator's output under this harness. It is not a prevalence estimate for real Cedar
policies, and nothing here supports treating it as one.
One result worth a maintainer decision
Validation.typecheckPolicysubstitutes the concrete action EUID before typechecking:let expr := substituteAction env.reqty.action policy.toExprtypecheck_by_single_request_envtypecheckst.condition()directly and has noequivalent, so it keeps
Var(Action)where Lean has the literal.This is not offered as a defect in either implementation. The substitution is
deliberate and documented on the Lean side, and both sides reach the same validation
verdict, which is why a pass/fail comparison never surfaced it. It is a scope question:
whether the Rust typed expression is meant to mirror the specification's substitution
is yours to decide, and the target reports the difference rather than normalising it
away so that the decision stays with you.
Running it
CEDAR_TYPED_EXPR_SURVEYcounts and prints findings instead of asserting on them.That downgrades the target to an observer and should stay unset in CI; it exists for
measuring how often a class appears before deciding what it is.