test(Cedar PDP): fail-closed-on-error override has zero test coverage

[jordigilh]

Summary

The Cedar PDP's fail-closed-on-runtime-error override — the security guarantee stated in the module's own doc comment ("if ANY policy errored, we return Deny regardless of what decision() says — an untrusted decision is worse than a closed gate") — has zero test coverage. No existing test drives Cedar into a runtime evaluation error, so nothing verifies the override actually fires instead of trusting Cedar's own response.decision().

Evidence

builtins/pdps/cedar-direct/src/decision.rs at upstream/main (6579993faaf5a18f0e811cde674e9eb893ee9522):

• Module doc (lines 26-33) states the guarantee explicitly:

// # Fail-closed on evaluation errors
//
// Cedar's `Response::diagnostics().errors()` lists policies that errored
// during runtime evaluation (e.g. type errors in a `when` clause that
// only manifest with certain entity data). If ANY policy errored, we
// return Deny regardless of what `decision()` says — an untrusted
// decision is worse than a closed gate. The error messages flow into
// the Deny reason so operators see why.

• The override itself lives in translate(), lines 78-94:

    if !errors.is_empty() {
        let reason = format!(
            "Cedar evaluation produced errors (fail-closed): {}",
            errors.join("; ")
        );
        let rule_source = firing_policies
            .first()
            .cloned()
            .unwrap_or_else(|| "cedar.evaluation_error".to_owned());
        return PdpDecision {
            decision: Decision::Deny {
                reason: Some(reason),
                rule_source,
            },
            diagnostics: firing_policies,
        };
    }

• Confirmed via lcov DA: line-hit records: every line in this block reports 0 hits — no test in the crate's suite constructs a Cedar evaluation that produces a runtime error via diagnostics().errors().

Risk

This is the fail-closed guarantee for the Cedar PDP path — the last line of defense against trusting a partially-failed Cedar evaluation. Because it is untested, a future refactor of translate() (e.g. reordering the errors.is_empty() check relative to the decision() match, or a Cedar upgrade that changes how/when diagnostics().errors() is populated) could silently remove or invert the override with no test failing to catch it. In an access-control decision engine, a regression here means Cedar evaluation errors could resolve to Allow instead of Deny.

Suggested fix

Add a test that:

1. Constructs a Cedar policy whose when clause type-errors at evaluation time against the entities supplied (e.g. comparing an attribute that is absent/wrong-typed for the entity in the request context, so cedar_policy records an evaluation error in diagnostics().errors() for that policy).
2. Calls translate() with the resulting Response and PolicySet.
3. Asserts PdpDecision.decision is Decision::Deny with a reason containing "fail-closed" — even in a construction where the underlying response.decision() would otherwise read Allow (i.e. at least one other policy in the set legitimately permits the request).

Severity: Critical

[mkoushni]

/assign

[mkoushni]

Cedar can still Allow when one policy in the set blows up at eval time, as long as another permit actually fired. We already override that to Deny, but nothing tests it, so the override can vanish and the suite stays green.

Fix is one test through evaluate(): an unconditional permit next to a when on principal.department (we never put that attribute on the principal). Cedar errors, still Allows on the permit, and we have to Deny with a reason that says fail-closed and names the error.