Skip to content

fix(license-compliance-audit): handle large blobs and canonical SPDX - #1089

Merged
potiuk merged 5 commits into
apache:mainfrom
sgoel2be24-cyber:codex/fix-license-compliance-audit
Aug 17, 2026
Merged

fix(license-compliance-audit): handle large blobs and canonical SPDX#1089
potiuk merged 5 commits into
apache:mainfrom
sgoel2be24-cyber:codex/fix-license-compliance-audit

Conversation

@sgoel2be24-cyber

Copy link
Copy Markdown
Contributor

Summary

  • Fetch source files through the GitHub contents API's raw media type so blobs larger than the inline-JSON limit can still be inspected.
  • Treat failed content fetches as uninspected coverage gaps instead of false MISSING-SPDX-HEADER findings, and report coverage counts explicitly.
  • Require the canonical Apache-2.0 SPDX identifier and keep the local /tmp/lca-missing-spdx.txt reference out of GitHub-scan reports.
  • Add a regression fixture covering both a large uninspected blob and the invalid Apache 2.0 token.

Type of change

  • Skill change (skills/<name>/) — eval fixtures updated below

Test plan

  • PATH="$PWD/.venv/bin:$PATH" prek run --all-files passes, including workspace pytest, ruff, mypy, Markdown, link, skill, and spec checks.
  • uv run --project tools/skill-and-tool-validator --group dev skill-and-tool-validate passes (only pre-existing advisory warnings in unrelated skills).
  • All 9 license-compliance-audit model eval cases pass.
  • A new behavior regression fixture is included for large-blob and canonical-SPDX handling.

RFC-AI-0004 compliance

  • Vendor neutrality — the scan continues to use the existing <upstream> placeholder and GitHub adapter path; no model vendor is introduced.
  • Write-access discipline — the audit remains read-only and does not add autonomous outbound actions.

Linked issues

Closes #944

Notes for reviewers

The raw-media request is used instead of decoding the JSON content field because GitHub returns encoding: "none" for blobs above the inline-content threshold. A failed raw fetch is deliberately surfaced as an audit coverage gap rather than converted into a compliance finding.

Generative AI disclosure

Codex was used while preparing this change. The contributor reviewed the diff and test results before submission. The commit carries a Generated-by: OpenAI Codex (GPT-5.6) trailer and no AI Co-Authored-By: trailer.

ssvcool252-web and others added 5 commits August 17, 2026 14:05
Generated-by: OpenAI Codex (GPT-5.6)
Dropping case-insensitivity alongside whitespace normalisation went one step
too far. The SPDX specification requires case-insensitive identifier
matching — 2.3 Annex D says identifiers "should be matched in a
case-insensitive manner. MIT, Mit and mIt should all be treated as the same
identifier", and 3.x makes expressions case-insensitive throughout — so a
header reading `apache-2.0` would have been reported WRONG-SPDX-HEADER at
medium severity. That is a new false-positive class in a compliance audit,
the same kind of defect this change set out to remove.

Keep the punctuation and internal-whitespace strictness, which is the real
fix: `Apache 2.0` is not a valid SPDX identifier and the previous wording
wrongly treated it as equivalent to `Apache-2.0`.

The eval fixture needed no change; case-5 exercises only the space form.

Co-authored-by: Jarek Potiuk <potiuk@apache.org>
Generated-by: Claude Code (Opus 5)

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with one fixup pushed. The core of this is a real bug fix and the
diagnosis was right.

The large-blob bug was genuine, and worse than it looks

The old recipe was:

gh api repos/<upstream>/contents/<file_path> \
  --jq '.content' | base64 --decode | head -10 | grep "SPDX-License-Identifier"

For a blob over about 1 MiB the contents API returns encoding: "none" with no
inline content, so the decode produces nothing, grep matches nothing, and the
file is recorded as MISSING-SPDX-HEADER. That is a false compliance finding
against a file that is perfectly compliant
— the worst failure mode an audit
skill has, because it sends someone to "fix" something that is not broken and
quietly erodes trust in every other finding in the report.

The principle you wrote into the skill is exactly the right one, and worth
quoting back:

An unavailable API response is never evidence that the source file lacks an
SPDX header.

Tracking those files as uninspected and keeping them out of the
missing/wrong counts is the correct fix, and case-5 pins it properly:
uninspected_spdx_count: 1 alongside missing_spdx_count: 0. Adding
uninspected_spdx_count to the output spec, with the explicit note that it must
not roll into finding_count, is what makes the distinction enforceable rather
than aspirational.

Scoping the /tmp/lca-missing-spdx.txt citation to local-checkout scans is also
a genuine correctness fix — a GitHub-API scan never writes that file, so the
report was pointing at a path that does not exist.

The fixup: SPDX matching stays case-insensitive

The one thing I changed. This PR replaced the old rule with "Do not normalise
punctuation, internal whitespace, or case". The whitespace half is right and was
the actual bug — the previous text claimed Apache-2.0 and Apache 2.0 are
equivalent, and Apache 2.0 is not a valid SPDX identifier at all. But dropping
case-insensitivity contradicts the SPDX specification:

SPDX 2.3, Annex D: license identifiers "should be matched in a
case-insensitive manner. MIT, Mit and mIt should all be treated as the same
identifier and referring to the same license."

SPDX 3.x goes further and makes expressions case-insensitive throughout,
operators included. So a header reading apache-2.0 would have been classified
WRONG-SPDX-HEADER at medium severity — a new false-positive class, which is
the same category of defect this PR set out to remove. Ironic enough to be worth
naming rather than silently fixing.

The fixup restores case-insensitivity with the spec citation, and keeps your
punctuation and internal-whitespace strictness intact. Your eval fixture needed
no change at all: case-5 exercises only the space form, so it was already
testing the correct behaviour.

Sources:
SPDX 2.3 Annex D,
SPDX 3.0.1 Annex B.

Nice first contribution — finding a false-positive path in a compliance audit is
a more valuable catch than the diff size suggests.


This review was drafted by an AI-assisted tool and
confirmed by a Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Magpie handles maintainer review:
CONTRIBUTING.md.

@potiuk
potiuk merged commit 99c983b into apache:main Aug 17, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix three correctness gaps in the license-compliance-audit skill

3 participants