fix: has() on non-container types returns error instead of false

[kodareef5]

Summary

has() applied to a non-container type (integer, string, boolean, null) previously returned false silently when EnableErrorOnBadPresenceTest was not set. This caused !has(x.field) to evaluate to true when x had an unexpected type, creating a fail-open condition in policy expressions that use presence guards on dynamically typed inputs.

This change splits the default case in refQualify (interpreter/attributes.go) so that:

• has() (presenceOnly=true): errors with missingKey on non-container types, preventing silent false returns
• Optional select ?. (presenceOnly=false): continues to return not-present, preserving optional.none() compatibility

The EnableErrorOnBadPresenceTest option continues to work as before when explicitly set.

Example

env, _ := cel.NewEnv(cel.Variable("request", cel.DynType))
ast, _ := env.Parse(`!has(request.field)`)
prg, _ := env.Program(ast)

// Before: silently returns true (fail-open)
// After: returns error (fail-closed)
out, _, _ := prg.Eval(map[string]any{"request": 42})

Test changes

One test expectation updated: has({'foo': optional.none()}.foo.value) now expects an error ("no such key: value") instead of false, since optional.none() is not a container type and has() should not silently return false on it.

All existing tests pass. Full suite green.

[TristonianJones]

This is the expected behavior for accesses within an optional value. If the test were for a null return value, then you'd expect a no such key error

[kodareef5]

Good catch, thank you. You're right — has(optional_none.value) returning false is correct optional semantics, not a type error.

I've updated the fix to narrow the scope. The default case in refQualify now checks whether the value is a *types.Optional before deciding:

• has() on *types.Optional (e.g. has(optional_none.value)) → returns false (preserves optional semantics, no change)
• has() on a primitive like types.Int (e.g. has(42.field)) → returns missingKey error (the fix)
• Optional select ?. on any non-container → returns not-present (preserves optional.none(), no change)

No test expectations changed. Full suite passes.

[TristonianJones]

Could you add a test case that demonstrates the behavior addressed by this change? The presenceOnly flag is just an optimization to indicate that the return value isn't necessary - only the presence bit.

[kodareef5]

Added test cases for has() on int, string, bool, and null via dyn():

{expr: `has(dyn(42).field)`, out: "no such key: field"},
{expr: `has(dyn('hello').field)`, out: "no such key: field"},
{expr: `has(dyn(true).field)`, out: "no such key: field"},
{expr: `has(dyn(null).field)`, out: "no such key: field"},

On the presenceOnly flag -- you're right that it's an optimization. In this context it serves as the only available signal that distinguishes has() (where testOnlyQualifier always passes presenceOnly=true) from ?. optional select (where applyQualifiers passes presenceOnly=false). The ?. path needs to keep returning not-present on non-container types to preserve optional.none() chaining, while has() on a raw primitive like dyn(42).field should error rather than silently returning false.

I initially tried dropping the presenceOnly check and erroring for both paths, but that breaks the ?. tests ({0: dyn(0)}[?0].?invalid expects optional.none(), not an error). Happy to restructure if you'd prefer a different approach, e.g. threading a separate flag through the qualifier chain.

[TristonianJones]

These cases error properly though when EnableErrorOnBadPresenceTest(true) is enabled, so I think that's why I'm not clear on what the issue is that you're trying to address.