From aed80017949be95bdebf0f0ba31ff41551869d41 Mon Sep 17 00:00:00 2001 From: Hinne Stolzenberg Date: Thu, 30 Jul 2026 16:08:45 +0200 Subject: [PATCH] fix: match forbidden licenses by family, not exact string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The check compared each forbidden entry against the found-license array with `jq 'index($license)'`, which is exact element equality. With the org's `FORBIDDEN_LICENSES="GPL;AGPL"` that blocked only a license literally named "GPL" — no real dependency is. Every actual SPDX identifier passed: GPL-3.0-only, AGPL-3.0, and any composite such as "Apache-2.0 AND GPL-2.0". Both validators had it. An entry now names a family: it matches an identifier that equals it or continues it after `-`, `.` or `+`, case-insensitively, after the SPDX expression is split on AND / OR / WITH and parentheses. Family, not substring, so "GPL" does not silently start blocking LGPL and AGPL — those are different licenses, and a project that listed GPL has not thereby decided about them. Callers wanting them list them. test/forbidden-matching_test.sh extracts the block from the shipped action.yml rather than restating it, so it cannot keep passing after the code it covers has changed. It fails 18 of its 28 cases against the exact-match implementation. --- .github/workflows/lint.yml | 13 ++++ go-license-validator/README.md | 18 ++++++ go-license-validator/action.yml | 23 ++++++- npm-license-validator/README.md | 18 ++++++ npm-license-validator/action.yml | 23 ++++++- test/forbidden-matching_test.sh | 104 +++++++++++++++++++++++++++++++ 6 files changed, 197 insertions(+), 2 deletions(-) create mode 100755 test/forbidden-matching_test.sh diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index f3644c9..1f70d14 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -68,6 +68,19 @@ jobs: done [[ "$failed" -eq 0 ]] + behaviour: + name: forbidden-license matching + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # Schema and shellcheck prove the run blocks parse; this proves the one + # that decides pass/fail actually decides it correctly. + - name: Forbidden-license matching + run: bash test/forbidden-matching_test.sh + workflow: name: actionlint runs-on: ubuntu-latest diff --git a/go-license-validator/README.md b/go-license-validator/README.md index 1d33fdc..f9795bf 100644 --- a/go-license-validator/README.md +++ b/go-license-validator/README.md @@ -34,6 +34,24 @@ The action runs through multiple focused steps: | `fail_on_forbidden` | Whether to fail the action when forbidden licenses are found | No | `true` | | `include_dev_dependencies` | Include development dependencies in the scan | No | `false` | + +### How `forbidden_licenses` is matched + +Each entry names a **license family**, matched case-insensitively against every +identifier in a dependency's SPDX expression: + +- `GPL` matches `GPL-3.0-only`, `GPL-2.0-or-later`, `GPL-3.0` — an entry matches + an identifier that equals it, or that continues it after `-`, `.` or `+`. +- `GPL` does **not** match `LGPL-3.0` or `AGPL-3.0`. Those are different + licenses that merely contain the same letters; list them separately if you + want them blocked (`GPL;LGPL;AGPL`). +- Composite expressions are split on `AND` / `OR` / `WITH` and parentheses, so + `Apache-2.0 AND GPL-2.0` is caught by `GPL`, as is + `GPL-2.0-or-later WITH GCC-exception-3.1`. + +An entry naming a full identifier (`GPL-3.0`) still works, and also matches the +modern `-only` / `-or-later` spellings that identifier predates. + ## Outputs | Output | Description | diff --git a/go-license-validator/action.yml b/go-license-validator/action.yml index 7a53b0d..5624e65 100644 --- a/go-license-validator/action.yml +++ b/go-license-validator/action.yml @@ -129,7 +129,28 @@ runs: forbidden=$(echo "$forbidden" | xargs) # Trim whitespace if [[ -n "$forbidden" ]]; then # Check if this forbidden license exists in our found licenses - if echo "$LICENSES_FOUND" | jq -e --arg license "$forbidden" 'index($license) != null' >/dev/null 2>&1; then + # A found value is an SPDX *expression*, not a bare identifier: + # it may combine identifiers with AND / OR / WITH and parens, so + # it is split into identifiers before matching. A forbidden entry + # names a family — "GPL" must catch "GPL-3.0-only" while leaving + # "LGPL-3.0" and "AGPL-3.0" alone, since those are different + # licenses that merely contain the same letters. Hence equality + # or the entry followed by an SPDX separator, never a bare + # substring. (Add "LGPL" explicitly if you want it blocked.) + if echo "$LICENSES_FOUND" | jq -e --arg forbidden "$forbidden" ' + def identifiers: + [ splits("[[:space:]()]+") ] + | map(select(. != "")) + | map(select(ascii_downcase as $t | ["and", "or", "with"] | index($t) | not)); + def blocked($entry): + (ascii_downcase) as $l + | ($entry | ascii_downcase) as $f + | $l == $f + or ($l | startswith($f + "-")) + or ($l | startswith($f + ".")) + or ($l | startswith($f + "+")); + any(.[]; identifiers | any(blocked($forbidden))) + ' >/dev/null 2>&1; then FORBIDDEN_FOUND=true if [[ -n "$FORBIDDEN_DETAILS" ]]; then FORBIDDEN_DETAILS="$FORBIDDEN_DETAILS, $forbidden" diff --git a/npm-license-validator/README.md b/npm-license-validator/README.md index a76dda8..e9c5a59 100644 --- a/npm-license-validator/README.md +++ b/npm-license-validator/README.md @@ -34,6 +34,24 @@ The action runs through multiple focused steps: | `fail_on_forbidden` | Whether to fail the action when forbidden licenses are found | No | `true` | | `include_dev_dependencies` | Include development dependencies in the scan | No | `false` | + +### How `forbidden_licenses` is matched + +Each entry names a **license family**, matched case-insensitively against every +identifier in a dependency's SPDX expression: + +- `GPL` matches `GPL-3.0-only`, `GPL-2.0-or-later`, `GPL-3.0` — an entry matches + an identifier that equals it, or that continues it after `-`, `.` or `+`. +- `GPL` does **not** match `LGPL-3.0` or `AGPL-3.0`. Those are different + licenses that merely contain the same letters; list them separately if you + want them blocked (`GPL;LGPL;AGPL`). +- Composite expressions are split on `AND` / `OR` / `WITH` and parentheses, so + `Apache-2.0 AND GPL-2.0` is caught by `GPL`, as is + `GPL-2.0-or-later WITH GCC-exception-3.1`. + +An entry naming a full identifier (`GPL-3.0`) still works, and also matches the +modern `-only` / `-or-later` spellings that identifier predates. + ## Outputs | Output | Description | diff --git a/npm-license-validator/action.yml b/npm-license-validator/action.yml index 9a391c7..61df7cc 100644 --- a/npm-license-validator/action.yml +++ b/npm-license-validator/action.yml @@ -132,7 +132,28 @@ runs: for forbidden in "${FORBIDDEN_ARRAY[@]}"; do forbidden=$(echo "$forbidden" | xargs) if [[ -n "$forbidden" ]]; then - if echo "$LICENSES_FOUND" | jq -e --arg license "$forbidden" 'index($license) != null' >/dev/null 2>&1; then + # A found value is an SPDX *expression*, not a bare identifier: + # it may combine identifiers with AND / OR / WITH and parens, so + # it is split into identifiers before matching. A forbidden entry + # names a family — "GPL" must catch "GPL-3.0-only" while leaving + # "LGPL-3.0" and "AGPL-3.0" alone, since those are different + # licenses that merely contain the same letters. Hence equality + # or the entry followed by an SPDX separator, never a bare + # substring. (Add "LGPL" explicitly if you want it blocked.) + if echo "$LICENSES_FOUND" | jq -e --arg forbidden "$forbidden" ' + def identifiers: + [ splits("[[:space:]()]+") ] + | map(select(. != "")) + | map(select(ascii_downcase as $t | ["and", "or", "with"] | index($t) | not)); + def blocked($entry): + (ascii_downcase) as $l + | ($entry | ascii_downcase) as $f + | $l == $f + or ($l | startswith($f + "-")) + or ($l | startswith($f + ".")) + or ($l | startswith($f + "+")); + any(.[]; identifiers | any(blocked($forbidden))) + ' >/dev/null 2>&1; then FORBIDDEN_FOUND=true if [[ -n "$FORBIDDEN_DETAILS" ]]; then FORBIDDEN_DETAILS="$FORBIDDEN_DETAILS, $forbidden" diff --git a/test/forbidden-matching_test.sh b/test/forbidden-matching_test.sh new file mode 100755 index 0000000..af07924 --- /dev/null +++ b/test/forbidden-matching_test.sh @@ -0,0 +1,104 @@ +#!/usr/bin/env bash +# Exercises the forbidden-license matching in both validators. +# +# The block under test is extracted from the shipped action.yml rather than +# restated here: a copy would keep passing after the action it claims to cover +# had changed. Extraction means this test fails loudly if the block is renamed +# or reshaped, which is the correct outcome — it can no longer prove anything. +set -euo pipefail + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +pass=0 +fail=0 + +# extract_block pulls the matching loop out of an action.yml and de-indents it +# to column 0 so it can be sourced as a standalone script. +extract_block() { + local action="$1" out="$2" + python3 - "$action" "$out" <<'PY' +import sys + +action, out = sys.argv[1], sys.argv[2] +s = open(action).read() +start = s.index(' if [[ "$LICENSES_FOUND" != "[]"') +end = s.index(' echo "forbidden_found=', start) +block = s[start:end] +block = "\n".join(l[8:] if l.startswith(" ") else l for l in block.split("\n")) +open(out, "w").write(block) +PY +} + +# run_case evaluates the extracted block for one (found, forbidden) pair and +# echoes the two outputs the action derives from it. +# +# The variables below look unused here because the block that reads and writes +# them is sourced at runtime, so shellcheck cannot see the use. +# shellcheck disable=SC2034 +run_case() { + local block="$1" + LICENSES_FOUND="$2" + FORBIDDEN_LICENSES="$3" + local FORBIDDEN_ARRAY FORBIDDEN_FOUND FORBIDDEN_DETAILS FORBIDDEN_LIST + IFS=';' read -ra FORBIDDEN_ARRAY <<< "$FORBIDDEN_LICENSES" + FORBIDDEN_FOUND=false + FORBIDDEN_DETAILS="" + FORBIDDEN_LIST="" + # shellcheck source=/dev/null + source "$block" + echo "$FORBIDDEN_FOUND|$FORBIDDEN_DETAILS" +} + +check() { # block desc found forbidden want_found want_details + local block="$1" desc="$2" got + got="$(run_case "$block" "$3" "$4")" + if [[ "$got" == "$5|$6" ]]; then + pass=$((pass + 1)) + echo " PASS $desc" + else + fail=$((fail + 1)) + echo " FAIL $desc -> got '$got', want '$5|$6'" + fi +} + +for action in go-license-validator npm-license-validator; do + echo "== $action" + block="$work/$action.sh" + mkdir -p "$(dirname "$block")" + extract_block "$repo_root/$action/action.yml" "$block" + + # SPDX identifiers carry a version suffix, so a family entry has to match the + # whole family. This is the case the exact-match implementation missed: with + # FORBIDDEN_LICENSES="GPL;AGPL" it blocked only a license literally named + # "GPL", which no real dependency is. + check "$block" "GPL-3.0-only is caught by GPL" '["GPL-3.0-only"]' 'GPL;AGPL' true 'GPL' + check "$block" "AGPL-3.0 is caught by AGPL" '["AGPL-3.0"]' 'GPL;AGPL' true 'AGPL' + + # Different licenses that merely contain the same letters must not be caught + # by a family entry — a project blocking GPL has not thereby blocked LGPL. + check "$block" "AGPL-3.0 is not caught by GPL" '["AGPL-3.0"]' 'GPL' false '' + check "$block" "LGPL is not caught by GPL" '["LGPL-3.0-or-later"]' 'GPL;AGPL' false '' + check "$block" "LGPL is caught when listed" '["LGPL-3.0-or-later"]' 'LGPL' true 'LGPL' + + # A found value is an SPDX expression; a forbidden identifier inside a + # composite is still present in the dependency tree. + check "$block" "composite AND is split" '["Apache-2.0 AND GPL-2.0"]' 'GPL' true 'GPL' + check "$block" "composite OR is split" '["BSD-3-Clause OR GPL-2.0-or-later"]' 'GPL' true 'GPL' + check "$block" "WITH exception matches the base" '["GPL-2.0-or-later WITH GCC-exception-3.1"]' 'GPL' true 'GPL' + + # The identifiers the README documents must keep working, including against + # the modern -only / -or-later spellings they predate. + check "$block" "exact documented id works" '["GPL-3.0"]' 'GPL-3.0' true 'GPL-3.0' + check "$block" "documented id matches -only form" '["GPL-3.0-only"]' 'GPL-3.0' true 'GPL-3.0' + + check "$block" "permissive set passes" '["MIT","Apache-2.0","BSD-3-Clause"]' 'GPL;AGPL' false '' + check "$block" "empty found list passes" '[]' 'GPL;AGPL' false '' + check "$block" "matching is case-insensitive" '["gpl-3.0-only"]' 'GPL' true 'GPL' + check "$block" "every matched family is reported" '["GPL-2.0","AGPL-3.0"]' 'GPL;AGPL' true 'GPL, AGPL' +done + +echo +echo "passed=$pass failed=$fail" +[[ "$fail" -eq 0 ]]