You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The engine's core promise is that every degraded path resolves to deny, or to the configured default, and never fails open. That promise is currently spread across doc comments, individual tests, and the judgment of whoever wrote each seam. It is not written down in one place and it is not tested as a single property.
Two jobs here. First codify the invariants. Then build a fault injection suite that asserts them at every seam, rather than testing each seam's happy path and trusting the rest.
The top invariant: plugin panic, plugin error, plugin timeout, PDP error, PDP timeout, malformed config, and missing attribute all resolve to deny or the configured default.
Current state
Some of this is already right and should be preserved rather than rebuilt:
OnError defaults to Fail (plugin.rs:515).
The CEL resolver defaults to OnError::Deny, and membership_on_absent_key_denies_but_empty_set_evaluates pins missing-attribute behaviour.
Concurrent plugin panics are contained. run_branches catches them via JoinError::is_panic, surfaces BranchOutcome::Panicked, and routes through the plugin's on_error. The executor has table-driven tests over concurrent × {error, timeout, panic} × {fail, ignore, disable}.
The gap is that the containment is asymmetric. There is no catch_unwind in the executor, and serial, transform, audit, and ref phases run their plugins inline with .await. A panic in any of those unwinds out of execute() and out of invoke() to the caller.
That is not fail open, since no allow decision is produced. But it is uncontained: it bypasses on_error, skips the audit plugins that would have recorded it, and leaves the pipeline in an indeterminate state. The concurrent phase already proves what the contained behaviour should look like. The other four phases should either match it or the difference should be a written, deliberate decision.
Scope
Write the invariants down as a document, one list, each item stated as a testable claim.
Build a fault injection harness: a plugin and a PDP resolver that can be told to panic, return an error, or hang, on demand.
Drive the harness across every seam and phase, asserting the invariant holds for each cell.
Resolve the phase asymmetry, either by containing panics in all phases or by documenting why serial panics propagate.
This generalises the existing audit-seam panic containment into a reusable pattern instead of a per-seam judgment call.
Acceptance Criteria
An invariants document listing each non-negotiable as a testable claim, linked from CONTRIBUTING
A reusable fault injection plugin and PDP resolver supporting panic, error, and hang
A table-driven test enumerating {plugin, PDP} × {panic, error, timeout} with an asserted safe verdict per cell
The plugin axis covers every phase: serial, transform, audit, ref, concurrent, fire and forget
The PDP axis covers cedar, cel, and opa
Malformed config and missing attribute are covered as their own cells
Panic behaviour is identical across phases, or the difference is documented with a reason
Each cell asserts the resulting decision, not just that no allow was returned
Adding a new phase or dialect fails the test until a cell is added for it
Description
The engine's core promise is that every degraded path resolves to deny, or to the configured default, and never fails open. That promise is currently spread across doc comments, individual tests, and the judgment of whoever wrote each seam. It is not written down in one place and it is not tested as a single property.
Two jobs here. First codify the invariants. Then build a fault injection suite that asserts them at every seam, rather than testing each seam's happy path and trusting the rest.
The top invariant: plugin panic, plugin error, plugin timeout, PDP error, PDP timeout, malformed config, and missing attribute all resolve to deny or the configured default.
Current state
Some of this is already right and should be preserved rather than rebuilt:
OnErrordefaults toFail(plugin.rs:515).OnError::Deny, andmembership_on_absent_key_denies_but_empty_set_evaluatespins missing-attribute behaviour.run_branchescatches them viaJoinError::is_panic, surfacesBranchOutcome::Panicked, and routes through the plugin'son_error. The executor has table-driven tests over concurrent × {error, timeout, panic} × {fail, ignore, disable}.The gap is that the containment is asymmetric. There is no
catch_unwindin the executor, and serial, transform, audit, and ref phases run their plugins inline with.await. A panic in any of those unwinds out ofexecute()and out ofinvoke()to the caller.That is not fail open, since no allow decision is produced. But it is uncontained: it bypasses
on_error, skips the audit plugins that would have recorded it, and leaves the pipeline in an indeterminate state. The concurrent phase already proves what the contained behaviour should look like. The other four phases should either match it or the difference should be a written, deliberate decision.Scope
This generalises the existing audit-seam panic containment into a reusable pattern instead of a per-seam judgment call.
Acceptance Criteria