🐛 llx: stop array contains/difference builtins panicking on a null argument#8320
Open
tas50 wants to merge 2 commits into
Open
🐛 llx: stop array contains/difference builtins panicking on a null argument#8320tas50 wants to merge 2 commits into
tas50 wants to merge 2 commits into
Conversation
…gument
`arrayContainsAll`, `arrayContainsNone`, and `arrayDifferenceV2` guarded the
receiver array against null but then did an unchecked `arg.Value.([]any)` on
the argument. When the argument resolves to a typed null array — e.g. a
`map[string][]T` key miss such as `pam.conf.services["su"]` on a host with no
`/etc/pam.d` (COS, Flatcar, Bottlerocket k8s nodes) — that assertion panics
with `interface conversion: interface {} is nil, not []interface {}`.
Because the executor runs blocks in goroutines the panic is unrecoverable and
crashes the entire scan rather than failing the single check. The dict
variants already use the safe comma-ok form; only the array variants were
affected.
Guard the argument the same way the receiver is guarded: a null argument
propagates as null. The compiled `… == []` wrapper then resolves the check to
a clean pass/fail instead of crashing.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![mondoo-code-review[bot]](https://avatars.githubusercontent.com/in/2834902?s=60&v=4){kind=small}
Contributor
Contributor
|
Fixes #8319 |
mondoo-linux-security's su-restriction check relies on `groups.containsAll(suRestrictedGroups)` being true when suRestrictedGroups is an empty (typed, non-null) []string. Add a regression test pinning that containsAll of an empty list is vacuously satisfied, distinct from the null-arg case which propagates null. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8319
Problem
A surge of client scan panics (
panic: interface conversion: interface {} is nil, not []interface {}) traced to the arraycontains*/differencebuiltins.arrayContainsAll,arrayContainsNone, andarrayDifferenceV2guard the receiver array against null but then perform an uncheckedarg.Value.([]any)on the argument. When the argument resolves to a typed null array — e.g. amap[string][]Tkey miss likepam.conf.services["su"]on hosts with no/etc/pam.d(container-optimized k8s node OSes: COS, Flatcar, Bottlerocket) — that type assertion panics.Because the MQL executor runs blocks in goroutines, the panic is unrecoverable by the caller and crashes the entire scan rather than failing the single check. The
dictvariants (dictContainsAll/dictContainsNone) already use the safe comma-ok form, so only the array variants were affected.Fix
Guard the argument the same way the receiver is already guarded: a null argument propagates as null (placed before the type check so an untyped null short-circuits cleanly too). The compiled
… == []wrapper then resolves the check to a clean pass/fail.After the fix,
realArray.containsAll(nullArg)returns null → the check resolves tofalse(fails cleanly) instead of crashing. Whether a pam-absent host should pass or fail the check remains a policy decision; the engine's responsibility here is simply to not panic.Testing
a = {a: [1,2]}; [1,2,3].containsAll(a['b'])(typed-null arg via map key miss); confirmed crash pre-fix, clean result post-fix.containsAll/containsNone/differencetoTestArray.go test ./llx/and the full./providers/core/resources/suite pass.