Skip to content

fix: honor OR in SPDX license expressions - #601

Open
wagoodman wants to merge 2 commits into
mainfrom
fix/or-expression-semantics
Open

wagoodman wants to merge 2 commits into
mainfrom
fix/or-expression-semantics

Conversation

@wagoodman

@wagoodman wagoodman commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

grant flattened every SPDX expression into its individual licenses and denied the package if any one of them was denied, so OR behaved like AND. MIT OR GPL-3.0-only with allow: [MIT] was denied even though the consumer can pick MIT. Follow up to #500, which had the same shape (Apache-2.0 OR Apache-2.0 WITH LLVM-exception OR MIT).

Each declared expression is now kept as a tree and evaluated per SPDX 2.3 Annex D:

  • OR passes when any side passes, AND needs every side, WITH is a single license (allowing X never covers X WITH E)
  • separate license entries on one package still AND together
  • -or-later / + ranges apply on the declared side: GPL-2.0-or-later is allowed by allow: [GPL-3.0-only]. Only well formed ranges and deprecated aliases are range checked, so plain IDs and a contradictory GPL-2.0-only+ stay exact. Allow entries are never widened, so globs, ranges (GPL-2.0-or-later, GPL-2.0+, with or without an exception) and non-canonical spellings in allow still match literally
  • require-known-license only denies when no known alternative satisfies the expression
  • anything grant can't read the same way go-spdx does (glued operators, nesting past 100) falls back to the old behavior of requiring every license, so bad input never makes a result more permissive. An expression go-spdx rejects outright is kept as one license named by the expression itself, never by the SBOM's separate value field

grant does the AND/OR evaluation itself rather than handing whole expressions to go-spdx Satisfies, which is unsound on nested input in v2.7.0 (drops LicenseRef-only ORs under AND, can return true with a required license missing) and exponential on AND-of-ORs. Satisfies is only used one license at a time for range checks.

Behavior changes worth a release note line:

  • packages with OR expressions can flip from denied to allowed. This is the default, there's no flag
  • alternatives an OR didn't need are listed in neither the allowed nor the denied licenses for that package, and a failed expression only lists the licenses that made it fail ((MIT OR GPL-3.0-only) AND ISC is denied on ISC alone). Allowed findings in the JSON still show every declared license
  • the JSON summary.licenses allowed/denied counts follow those per-license decisions instead of counting every license on the package, so an allowed license on a denied package now counts as allowed
  • a license that isn't on the SPDX list inside an expression (LicenseRef-x) is now named by its own text instead of the whole declared value, so allow: [LicenseRef-x] matches it and the output shows the shorter name
  • duplicate name-only licenses on a package are merged into one, keeping the first entry's locations
  • a malformed spdxExpression used to be reported under the license's value, and was dropped when that value was a sha256: hash. It is now reported under the expression and denied unless allowed literally

No public API changes. Package, ConvertSyftPackage and ConvertSyftLicenses are unchanged (the expression trees live beside the packages during evaluation, not on them), and Policy.IsLicensePermitted is unchanged (its doc now says it's a pattern match only, without operators or ranges).

Fixes #157
Fixes #122
Partially addresses #205

@wagoodman wagoodman added the bug Something isn't working label Oct 2, 2026
@wagoodman
wagoodman force-pushed the fix/or-expression-semantics branch from 7af24e1 to d799120 Compare October 2, 2026 21:02
grant flattened every SPDX expression into its individual licenses and denied the package if any one of them was denied, so `OR` behaved like `AND`. `MIT OR GPL-3.0-only` with `allow: [MIT]` was denied even though the consumer can pick MIT. Follow up to #500, which had the same shape (`Apache-2.0 OR Apache-2.0 WITH LLVM-exception OR MIT`).

Each declared expression is now kept as a tree and evaluated per SPDX 2.3 Annex D:

- `OR` passes when any side passes, `AND` needs every side, `WITH` is a single license (allowing `X` never covers `X WITH E`)
- separate license entries on one package still AND together
- `-or-later` / `+` ranges apply on the declared side: `GPL-2.0-or-later` is allowed by `allow: [GPL-3.0-only]`. Only well formed ranges and deprecated aliases are range checked, so plain IDs and a contradictory `GPL-2.0-only+` stay exact. Allow entries are never widened, so globs, ranges (`GPL-2.0-or-later`, `GPL-2.0+`, with or without an exception) and non-canonical spellings in `allow` still match literally
- `require-known-license` only denies when no known alternative satisfies the expression
- anything grant can't read the same way go-spdx does (glued operators, nesting past 100) falls back to the old behavior of requiring every license, so bad input never makes a result more permissive. An expression go-spdx rejects outright is kept as one license named by the expression itself, never by the SBOM's separate `value` field

grant does the AND/OR evaluation itself rather than handing whole expressions to go-spdx `Satisfies`, which is unsound on nested input in v2.7.0 (drops LicenseRef-only ORs under AND, can return true with a required license missing) and exponential on AND-of-ORs. `Satisfies` is only used one license at a time for range checks.

Behavior changes worth a release note line:

- packages with `OR` expressions can flip from denied to allowed. This is the default, there's no flag
- alternatives an `OR` didn't need are listed in neither the allowed nor the denied licenses for that package, and a failed expression only lists the licenses that made it fail (`(MIT OR GPL-3.0-only) AND ISC` is denied on `ISC` alone). Allowed findings in the JSON still show every declared license
- the JSON `summary.licenses` allowed/denied counts follow those per-license decisions instead of counting every license on the package, so an allowed license on a denied package now counts as allowed
- a license that isn't on the SPDX list inside an expression (`LicenseRef-x`) is now named by its own text instead of the whole declared value, so `allow: [LicenseRef-x]` matches it and the output shows the shorter name
- duplicate name-only licenses on a package are merged into one, keeping the first entry's locations
- a malformed `spdxExpression` used to be reported under the license's `value`, and was dropped when that value was a `sha256:` hash. It is now reported under the expression and denied unless allowed literally

No public API changes. `Package`, `ConvertSyftPackage` and `ConvertSyftLicenses` are unchanged (the expression trees live beside the packages during evaluation, not on them), and `Policy.IsLicensePermitted` is unchanged (its doc now says it's a pattern match only, without operators or ranges).

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman
wagoodman force-pushed the fix/or-expression-semantics branch from d799120 to a8a1fe4 Compare October 2, 2026 21:09
@mxmehl

mxmehl commented Oct 5, 2026

Copy link
Copy Markdown

FWIW, I've recently summarised the failures of grant with more complex SPDX expressions including concrete test cases and expected outcomes. Perhaps worth a try to test with this PR? https://github.com/mxmehl/grant-policy-testcases

Adds the four expressions and policies from https://github.com/mxmehl/grant-policy-testcases as a CLI regression test: `OR`, `WITH`, `WITH` under `AND`, and nested `AND`/`OR`. Tests 1 and 4 failed before this PR, 2 and 3 already passed on main.

Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
@wagoodman
wagoodman marked this pull request as ready for review October 5, 2026 14:24
@elaine-mattos

Copy link
Copy Markdown

This is just GREAT! How long till merge? :D

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

3 participants