chore(compliance): regenerate THIRD_PARTY_LICENSES and automate its refresh#5787
chore(compliance): regenerate THIRD_PARTY_LICENSES and automate its refresh#5787devantler wants to merge 3 commits into
Conversation
…efresh The inventory was last regenerated 2025-05-13 and the consolidation script was never checked in, so compliance metadata silently drifted on every dependency bump. Adds scripts/gen-third-party-licenses (deterministic, covers both Go modules, fails on unverified Unknown-license deps), a `make licenses` target, and a CI self-heal job that regenerates the inventory via the existing auto-commit patch flow on module-graph changes. Fixes #5716 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📜 Recent review details⏰ Context from checks skipped due to timeout. (12)
📝 WalkthroughWalkthroughThis PR adds a Go-based generator for ChangesLicense inventory generation and CI automation
Estimated code review effort: 4 (Complex) | ~45 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
✅MegaLinter analysis: Success✅ Linters with no issuesactionlint, bash-exec, git_diff, hadolint, jscpd, jsonlint, lychee, markdown-table-formatter, markdownlint, prettier, prettier, shellcheck, shfmt, stylelint, syft, trivy-sbom, trufflehog, v8r, v8r, yamllint Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts
|
|
I believe Dependency Review is doing the same thing as this license action and related tooling. Please verify, and if I am correct we should be able to remove it in favor of the GHAS native Dependency Review action and it's license checks. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
scripts/gen-third-party-licenses/verified_unknown.go (2)
23-91: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse keyed struct literals for
verificationto prevent silent field swaps.All
verification{...}entries use positional literals ({apache2, urlAlibabaCR}). Since both fields arestring, an accidental swap oflicense/urlorder compiles fine but silently mislabels a dependency's license — the exact class of error this manual-verification mechanism exists to prevent.override(lines 97-100) already uses keyed literals; applying the same convention here removes the risk entirely.♻️ Proposed fix (representative example)
- "github.com/alibabacloud-go/cr-20160607/client": {apache2, urlAlibabaCR}, + "github.com/alibabacloud-go/cr-20160607/client": {license: apache2, url: urlAlibabaCR},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen-third-party-licenses/verified_unknown.go` around lines 23 - 91, The verifiedUnknown() map in verified_unknown.go uses positional verification literals, which can silently swap the license and url string fields; update all verification entries in this function to use keyed struct literals for the verification type, matching the safer style already used in override(), so the license and URL are explicitly assigned and harder to mislabel.
42-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated per-package entries could be generated from a shared license/url pair.
deitch/magic(3 entries),in-toto-golang(5 entries), andsegmentio/asm(9 entries) each repeat the same{license, url}pair across many map keys. A small helper that expands one(license, url, pkg...)tuple into multiple map entries would cut the boilerplate and reduce the chance of a typo in one of the repeated pairs.♻️ Illustrative helper
func addAll(m map[string]verification, license, url string, pkgs ...string) { for _, pkg := range pkgs { m[pkg] = verification{license: license, url: url} } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen-third-party-licenses/verified_unknown.go` around lines 42 - 90, The license map in verified_unknown.go has many repeated package entries with the same license/url pair, so replace the boilerplate in the map initialization with a shared helper that expands one license/url and a list of package paths into entries. Update the existing setup around the package groups for deitch/magic, in-toto-golang, and segmentio/asm to use that helper, keeping the same verification values while reducing duplication and typo risk.scripts/gen-third-party-licenses/main.go (2)
57-63: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo timeout on external
go-licenses/go listsubprocess calls.
runis invoked withcontext.Background()(line 58), and that unbounded context flows into everyexec.CommandContextcall (go-licenses csvat line 116,go listat line 251). If either binary hangs (e.g., resolving remote license data or a corrupted module cache), the generator — and any CI job invoking it — will block indefinitely with no self-recovery. Wrapping the top-level context with a bounded timeout is cheap insurance for a CI-invoked tool.🔒️ Proposed fix
+ "time" ) func main() { - err := run(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + + err := run(ctx)Worth confirming whether
go-licenses csvperforms any network calls in this codebase's usage (vs. purely local module-cache analysis) to gauge how likely a hang actually is.Also applies to: 116-127, 251-253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen-third-party-licenses/main.go` around lines 57 - 63, The generator entrypoint uses an unbounded context, so `run` can hang forever while `exec.CommandContext` waits on `go-licenses csv` or `go list`. Update `main` to create a top-level context with a timeout and pass that into `run`, and make sure the timeout is applied consistently through the existing `run`, `go-licenses`, and `go list` execution paths so the tool fails fast instead of blocking indefinitely.
52-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
dependency.moduleactually holds a package import path, not a Go module path.Evidenced by
modJSONCanonicalizer/modExternalTypes(verified_unknown.go lines 13-16) carrying sub-package suffixes, andgo-licenses csvclassifying at package granularity. The field/comment naming (module, "representative module", etc.) may mislead future maintainers into assuming module-level granularity. Consider renaming topkg/importPathfor clarity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen-third-party-licenses/main.go` around lines 52 - 55, The dependency struct’s module field is misleading because it stores a package import path, not a Go module path. Update the naming in dependency and all related references in gen-third-party-licenses/main.go (including any comments or variables like “representative module”) to use a clearer identifier such as pkg or importPath, so the granularity is obvious. Make sure any logic that reads or writes this field still reflects package-level classification used by go-licenses csv and the modJSONCanonicalizer/modExternalTypes flow.scripts/gen-third-party-licenses/format.go (1)
129-150: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
writeUnknownSectionsilently degrades if called with unverified deps.
entry := verified[module](line 148) doesn't check the map-lookupok; if a module reaches this function without having passedcheckUnknownsfirst, it silently renders blankVerified: ()text instead of failing loudly. Today this is safe becauserun()always callscheckUnknownsbeforerender()(main.go lines 80-85), but the invariant is only enforced by call-order convention, not locally. A defensive check (or at least a doc comment onrendernoting the precondition) would make the contract explicit and catch future misuse (e.g., a future caller invokingrenderdirectly, as tests already do).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/gen-third-party-licenses/format.go` around lines 129 - 150, writeUnknownSection currently assumes every module exists in verifiedUnknown() and will print blank Verified text if that contract is broken. Update writeUnknownSection to check the map lookup result for each module and fail fast or otherwise handle missing entries explicitly, so unverified deps cannot render silently; if you prefer to keep the behavior, add a clear precondition note on render explaining that checkUnknowns must run first..github/workflows/ci.yaml (1)
262-271: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider adding
.github/workflows/ci.yamlto thelicensesfilter.The sibling
desktopfilter (lines 251-261) explicitly includes.github/workflows/ci.yamlwith a comment explaining why go.mod/go.sum-adjacent CI logic changes must retrigger the check. The newlicensesfilter doesn't include it, so edits to theverify-licensesjob logic itself (e.g., a bug fix to the generator invocation or patch handling) won't retrigger this job unless bundled with an actual go.mod/go.sum/generator change.♻️ Suggested addition
licenses: # THIRD_PARTY_LICENSES tracks BOTH module graphs; anything that # changes either graph (or the generator itself) can drift the # inventory, so regenerate-and-self-heal on those paths (`#5716`). - 'go.mod' - 'go.sum' - 'desktop/go.mod' - 'desktop/go.sum' - 'scripts/gen-third-party-licenses/**' - 'THIRD_PARTY_LICENSES' + - '.github/workflows/ci.yaml'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yaml around lines 262 - 271, The licenses path filter in the CI workflow is missing the workflow file itself, so changes to the verify-licenses job logic won’t retrigger the check. Update the `licenses` filter in `ci.yaml` to include `.github/workflows/ci.yaml`, mirroring the sibling `desktop` filter’s coverage, so edits to the license verification workflow self-heal properly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/ci.yaml:
- Around line 262-271: The licenses path filter in the CI workflow is missing
the workflow file itself, so changes to the verify-licenses job logic won’t
retrigger the check. Update the `licenses` filter in `ci.yaml` to include
`.github/workflows/ci.yaml`, mirroring the sibling `desktop` filter’s coverage,
so edits to the license verification workflow self-heal properly.
In `@scripts/gen-third-party-licenses/format.go`:
- Around line 129-150: writeUnknownSection currently assumes every module exists
in verifiedUnknown() and will print blank Verified text if that contract is
broken. Update writeUnknownSection to check the map lookup result for each
module and fail fast or otherwise handle missing entries explicitly, so
unverified deps cannot render silently; if you prefer to keep the behavior, add
a clear precondition note on render explaining that checkUnknowns must run
first.
In `@scripts/gen-third-party-licenses/main.go`:
- Around line 57-63: The generator entrypoint uses an unbounded context, so
`run` can hang forever while `exec.CommandContext` waits on `go-licenses csv` or
`go list`. Update `main` to create a top-level context with a timeout and pass
that into `run`, and make sure the timeout is applied consistently through the
existing `run`, `go-licenses`, and `go list` execution paths so the tool fails
fast instead of blocking indefinitely.
- Around line 52-55: The dependency struct’s module field is misleading because
it stores a package import path, not a Go module path. Update the naming in
dependency and all related references in gen-third-party-licenses/main.go
(including any comments or variables like “representative module”) to use a
clearer identifier such as pkg or importPath, so the granularity is obvious.
Make sure any logic that reads or writes this field still reflects package-level
classification used by go-licenses csv and the
modJSONCanonicalizer/modExternalTypes flow.
In `@scripts/gen-third-party-licenses/verified_unknown.go`:
- Around line 23-91: The verifiedUnknown() map in verified_unknown.go uses
positional verification literals, which can silently swap the license and url
string fields; update all verification entries in this function to use keyed
struct literals for the verification type, matching the safer style already used
in override(), so the license and URL are explicitly assigned and harder to
mislabel.
- Around line 42-90: The license map in verified_unknown.go has many repeated
package entries with the same license/url pair, so replace the boilerplate in
the map initialization with a shared helper that expands one license/url and a
list of package paths into entries. Update the existing setup around the package
groups for deitch/magic, in-toto-golang, and segmentio/asm to use that helper,
keeping the same verification values while reducing duplication and typo risk.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 24a54d0a-761d-4657-8132-7a1fdb844c29
📒 Files selected for processing (8)
.github/workflows/ci.yamlAGENTS.mdMakefileTHIRD_PARTY_LICENSESscripts/gen-third-party-licenses/format.goscripts/gen-third-party-licenses/main.goscripts/gen-third-party-licenses/main_test.goscripts/gen-third-party-licenses/verified_unknown.go
📜 Review details
⏰ Context from checks skipped due to timeout. (11)
- GitHub Check: ⛵ Operator Chart E2E
- GitHub Check: 🏠 Home Isolation Guard
- GitHub Check: 🏗️ Build KSail Binary
- GitHub Check: 🧹 Lint - golangci-lint
- GitHub Check: 📊 Code Coverage
- GitHub Check: 🧪 Test
- GitHub Check: 🔍 Dead Code Analysis
- GitHub Check: 🏗️ Build
- GitHub Check: 🛡️ Vulnerability Scan
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (go)
🧰 Additional context used
🪛 LanguageTool
AGENTS.md
[uncategorized] ~382-~382: The official name of this software platform is spelled with a capital “H”.
Context: ...icenses/verified_unknown.go). See also .github/instructions/`. **Shared machine / auto...
(GITHUB)
🔇 Additional comments (10)
Makefile (1)
6-6: LGTM!Also applies to: 28-30
AGENTS.md (1)
379-382: LGTM!THIRD_PARTY_LICENSES (3)
13-42: LGTM! Provenance header and license summary match the generator'swriteHeaderoutput, and totals sum correctly (845).
108-133: LGTM! Remaining module-list churn, the BSD-0-Clause/XZ section additions, BSD-2-Clause-FreeBSD removal, CC0-1.0 entity-encoding, and the rewritten Unknown-verified section are all consistent with the generator logic andverified_unknown.gocontents shown in the provided context.Also applies to: 143-156, 180-193, 218-225, 268-276, 283-288, 296-302, 324-330, 380-386, 392-396, 404-409, 640-652, 676-688, 708-712, 731-742, 792-800, 883-886, 900-904, 916-922, 929-944, 975-980, 1108-1114, 1166-1170, 1178-1186, 1218-1234, 1240-1244, 1675-1712, 1728-1734, 1772-1798
610-631: 🎯 Functional CorrectnessNo issue with the BSD-0-Clause attribution. The emitted text matches the bundled LICENSE for
github.com/mikelolasagasti/xz.> Likely an incorrect or invalid review comment.scripts/gen-third-party-licenses/verified_unknown.go (1)
77-81: 🔒 Security & PrivacyConfirm the risk-acceptance for the unlicensed
loft-sh/external-typesdependency is still current.This entry documents that the package publishes no license at all and is included on a "risk-accepted" basis pending an upstream license request. Since this bypasses the normal license-verification bar (there is no actual license to verify), worth confirming the linked upstream issue is still tracked/open and that shipping this unlicensed code is an accepted position for the project, not an oversight carried over from before this PR.
scripts/gen-third-party-licenses/main_test.go (1)
1-143: LGTM!.github/workflows/ci.yaml (3)
135-135: LGTM!Also applies to: 513-518, 1444-1444, 1472-1472
1303-1310: 🗄️ Data Integrity & IntegrationVerify ordering vs.
verify-desktop-tidywhen both jobs produce drift.
verify-licensesdepends only onchangesand runs in parallel withverify-desktop-tidy. If a PR triggers both desktopgo.sumtidy drift and license drift simultaneously, this job regeneratesTHIRD_PARTY_LICENSESagainst the pre-tidy module graph.auto-committhen applies both patches together (lines 513-518, 537-542), but the license patch won't reflect the post-tidy dependency set — full convergence may require an extra CI cycle after the auto-commit push rather than resolving in one pass.This is a narrow edge case and self-heals eventually, but worth confirming whether
verify-licensesshould depend onverify-desktop-tidy(or rungo mod tidybefore regenerating) to avoid a stale license patch.
1311-1366: LGTM!
Code Coverage OverviewLanguages: Go Go / code-coverage/goThe overall coverage in the branch remains at 65%, unchanged from the branch. Show a code coverage summary of the most impacted files.
Updated |
Keyed verification literals + shared per-repository verifyAll entries, a run-level subprocess timeout, an explicit import-path doc on dependency.module, a fail-loud marker for the (guarded) unverified- Unknown render path, and ci.yaml in the licenses change filter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verified against the dependency-review-action docs — you're right about the gate, but the action can't cover this PR's artifact, so the split I've shipped is: What Dependency Review does cover: an allow/deny license gate on PR-introduced dependency changes — the same role as CI's existing What it can't cover (why this PR stays):
So: this PR resolves the inventory gap (#5716); #5788 executes your direction for the gate — native Dependency Review in, custom |
CodeRabbit review-body nitpicks drained in b2ce8eb: keyed |
|
We do not need the third party license file, as the found dependency are submitted via dependency submission from GHAS. So I belive this functionality can be fully removed! |
Closing per your direction above: with the inventory covered by GHAS dependency submission, the notice file — and with it this PR's generator, |
* ci: replace custom license tooling with GHAS Dependency Review Retire the go-licenses license-check job in favor of the org's shared dependency-review reusable workflow (deny-list ports go-licenses' forbidden category 1:1), and remove the stale THIRD_PARTY_LICENSES notice file — dependency inventory is covered by GHAS dependency submission (maintainer direction on #5787). Fixes #5788 Closes #5716 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: drop redundant in-CI dependency-review job The DependencyReview ruleset (id 17213449) already requires reusable-workflows/dependency-review.yaml@main on every PR, so a second in-CI invocation is redundant (maintainer direction on #5789). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: devantler <devantler@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Why
The third-party license inventory had not been regenerated for over a year, so the compliance metadata shipped with every release names removed dependencies and misses new ones — and nothing prevented it drifting further.
What
Regenerates the inventory (now covering both Go modules), checks in the previously-missing generator so regeneration is reproducible, and adds a CI self-heal that refreshes the file automatically on any dependency change. A new dependency that ships no license file now fails CI until its license is manually verified and recorded.
Fixes #5716