ci: add license-check workflow - #217
Conversation
Adds a PR gate that fails when a dependency introduces a copyleft (GPL/AGPL/LGPL) license: - license-check/node (bpr): runs `license-checker` over the root production dependency tree. - license-check/rust (bpr): runs `cargo deny check licenses` against the Solana OFT crate, governed by a new contracts/solanaOFT/deny.toml allow-list. Both jobs carry the (bpr) suffix so they can be added to the default-branch required-status-checks ruleset.
b2df1f0 to
07ae9ff
Compare
07ae9ff to
dee3d5c
Compare
801e2fc to
80bdea6
Compare
|
|
||
| - name: Check licenses | ||
| working-directory: ${{ matrix.package }} | ||
| run: npx --yes license-checker --production --failOn "GPL;AGPL;LGPL" |
There was a problem hiding this comment.
is this the only rule that we allow ? also: i thought it was ok for tooling/testing (ie non-smart contract libraries), which I don't think we differentiate here
you can check an old work of mine here (in the makefile) if you're curious : https://github.com/zama-ai/protocol-apps/pull/14/changes
|
@claude review |
|
Claude finished @Seth-Schmidt's request. — View run |
There was a problem hiding this comment.
[AUTOMATED]
Code Review — ci: add license-check workflow
The workflow is well-structured and follows established repository conventions (SHA-pinned actions, least-privilege permissions, concurrency groups, persist-credentials: false, (bpr) naming). The shell logic for discovering affected packages in a monorepo is sound.
However, the license-checking step has a critical issue that defeats the purpose of the workflow.
Summary
| Category | Result |
|---|---|
| Guideline Compliance | ✅ Fully compliant with repo conventions |
| Error Handling | ✅ Adequate (set -euo pipefail, proper aggregate gate) |
| Security | ✅ Least-privilege, pinned actions, no credential exposure |
| Bug / Correctness |
| uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0 | ||
| with: | ||
| node-version: 24 | ||
|
|
There was a problem hiding this comment.
[AUTOMATED]
Bug: --failOn uses exact string matching — copyleft licenses will slip through
license-checker's --failOn compares the full license string against each entry using literal equality (source). The values "GPL", "AGPL", and "LGPL" do not match real-world SPDX identifiers like GPL-2.0-only, GPL-3.0-only, AGPL-3.0-only, LGPL-2.1-only, etc.
As a result, every copyleft-licensed package will silently pass this check, defeating the purpose of the workflow.
Options to fix:
-
Enumerate the specific SPDX identifiers (and common non-SPDX variants reported by npm):
npx --yes license-checker --production --failOn \ "GPL-1.0-only;GPL-1.0-or-later;GPL-2.0-only;GPL-2.0-or-later;GPL-3.0-only;GPL-3.0-or-later;AGPL-1.0-only;AGPL-1.0-or-later;AGPL-3.0-only;AGPL-3.0-or-later;LGPL-2.0-only;LGPL-2.0-or-later;LGPL-2.1-only;LGPL-2.1-or-later;LGPL-3.0-only;LGPL-3.0-or-later" -
Use
--onlyAllowinstead (allowlist approach —--onlyAllowdoes substring matching, so it's safer):npx --yes license-checker --production --onlyAllow \ "MIT;ISC;Apache-2.0;BSD-2-Clause;BSD-3-Clause;0BSD;CC0-1.0;Unlicense;CC-BY-3.0;CC-BY-4.0;Python-2.0;BlueOak-1.0.0" -
Switch to a fork with SPDX-aware matching, e.g.
license-checker-rseidelsohn(actively maintained) or@onebeyond/license-checker(enforces valid SPDX).
Confidence: 95/100
|
The review has been posted successfully. Here's a summary of the review: Review Summary for PR #217:
|
3a231d2 to
6845239
Compare
Summary
Adds a PR gate that fails when a package introduces a production dependency
under a disallowed license. Addresses compliance finding 3-1 (License Check).
Scope is what we distribute: contract bytecode and
scripts/source. Packages with known-but-pending orpeer-install-only exceptions are suppressed via exact
name@versionexclusionsin the Makefile.
Changes
Makefile—make check-licensesscripts/fhevm-cliandscripts/governance-proposal-builder.EXCLUDE_PACKAGES: LGPL Safe (cleared), LZBL LayerZero v2 packages (legalpending), BUSL peers not compiled into our Solidity (v1, Chainlink CCIP,
lz-v2-utilities)..github/workflows/license-check.ymllicense-check/check-licenses (bpr)package.json/ lockfiles — move deployed Solidity deps fromdevDependencies→dependenciesingovernance,token,solanaOFT,staking,safe, andscripts/fhevm-cliso--productionscans the realcompile-time tree. Lockfiles regenerated to match.