Do not fail a combined (OR) constraint when a sibling operand errors

[arpitjain099]

combinedConstraint.Satisfied (an OR of sub-constraints) returns immediately on the first operand that returns an error:

for _, op := range c.OrOperands {
    satisfied, err := op.Satisfied(version)
    if err != nil {
        return false, fmt.Errorf("error evaluating constraint %s: %w", op, err)
    }
    if satisfied {
        return true, nil
    }
}
return false, nil

For OR semantics that is order-dependent: if operand A errors but operand B is satisfied, the constraint really is satisfied, yet this returns an error.

That is not contained locally. In grype/search/version_constraint.go, constraintFuncCriteria.MatchesVulnerability turns any constraint error into a dropped vulnerability (its own comment notes it "has the effect of dropping the vulnerability in the case an error occurs parsing a package or other version"). So an erroring operand ordered before a matching operand becomes a real false negative. A per-operand error is reachable when one OR branch carries a constraint-version that fails to parse (for example an rpm constraint with a non-numeric epoch) while another branch legitimately matches. CombineConstraints is used to build these OR constraints in the RHEL-EUS matcher.

Fix: accumulate operand errors with errors.Join and continue; return true as soon as any operand is satisfied, and only surface the joined error if nothing matched.

I updated the existing TestCombinedConstraint_Satisfied_WithErrors case that asserted the old abort-on-first-error behavior (an error from an earlier operand while a later operand is satisfied is now NoError). Ran go test ./grype/version/ locally (passes).

[kzantow]

I can understand the logic behind this change, but are there some cases that this is impactful? Do you have some real-world examples that this change fixes?

Generally speaking an error here being returned would mean that the constraint isn't satisfied, regardless if part of it is.

[arpitjain099]

Fair question, and honestly you're right to push on it: I don't have a real-world scan that triggers this today.

I went back through it after your comment. The only caller is buildPatchedVulnerabilityRecord in rhel_eus.go, and there every OR operand is the same rpm format built against the same package version (the advisory constraints plus the < fixVersion constraints from v.Format). Those constraint strings come from the curated DB rather than package or user input, so a per-operand error while a sibling still matches is theoretical, not something I can reproduce against current data.

The motivation was only the downstream behavior: constraintFuncCriteria.MatchesVulnerability turns any constraint error into a dropped vulnerability, so for an OR an error ordered before a match fails toward "not vulnerable" instead of "vulnerable", which is the less-safe direction for a scanner. But I agree that's a defensive/latent argument, not a demonstrated bug, and your "an error means not satisfied" reading is a reasonable design choice.

I'm happy to close this if you'd rather keep the fail-loud semantics. Thanks for taking the time to look.