ci: add third-party license notices - #2268
Open
Tantanz20020918 wants to merge 19 commits into
Open
Conversation
github-actions
Bot
added
the
size/L
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe repository adds deterministic third-party notice generation, local dependency-change enforcement, CI validation, pinned workflow tooling, and release artifact checks. It also includes the generated notices document and tests for parsing, safety, workflow contracts, and Git hook behavior. ChangesThird-party notice compliance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Preflight
participant LicenseCompliance
participant BuildSignNotarize
participant ReleaseArtifacts
Preflight->>LicenseCompliance: provide validated source commit
LicenseCompliance->>LicenseCompliance: run make check-third-party-notices
LicenseCompliance->>BuildSignNotarize: allow build after compliance succeeds
BuildSignNotarize->>ReleaseArtifacts: create release archives and npm package
ReleaseArtifacts->>BuildSignNotarize: verify THIRD_PARTY_NOTICES.md
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@4fe1315d8c877911899f40b66cc078f05fe4e334🧩 Skill updatenpx skills add larksuite/cli#ci/third-party-notices -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In @.githooks/pre-commit:
- Around line 8-9: Update the pre-commit condition to trigger notice
regeneration only when staged manifest changes affect dependency declarations,
rather than any edit to go.mod, go.sum, package.json, or package-lock.json.
Preserve the existing THIRD_PARTY_NOTICES.md exemption, and add a regression
test covering a non-dependency manifest edit that should not reject the commit.
In `@README.md`:
- Around line 321-324: Update the “Third-party dependency notices” section to
describe the repository hook as local enforcement: state that it blocks commits
when dependency metadata changes without a staged notice file, while CI
independently verifies notice freshness.
In `@scripts/third_party_notices.py`:
- Around line 188-200: Update _detect_license_ids and the supported-license
detection helpers to reject incomplete license text by requiring all mandatory
clauses for MIT, Apache-2.0, BSD, and ISC before adding an identifier. Preserve
valid complete-license detection, and add negative tests covering truncated MIT,
Apache, BSD, and ISC notices.
- Around line 236-244: Update _copyright_lines to exclude lines whose normalized
text starts with “copyright notice” or “copyright license” before collecting
attribution lines, while preserving existing © handling, deduplication, and the
“Not specified” fallback.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d891dbf0-1e37-455c-b8e3-3c502ff1325e
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
.githooks/pre-commit.github/workflows/ci.yml.github/workflows/pkg-pr-new.yml.github/workflows/release.yml.goreleaser.ymlMakefileREADME.mdTHIRD_PARTY_NOTICES.mdgo.modpackage.jsonscripts/build-pkg-pr-new.shscripts/ci-workflow.test.shscripts/pkg-pr-new-workflow.test.shscripts/release-workflow.test.shscripts/testdata/third_party_notices/mit-package/LICENSEscripts/testdata/third_party_notices/mit-package/package.jsonscripts/third_party_notices.pyscripts/third_party_notices.test.pyscripts/third_party_notices_hook.test.sh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2268 +/- ##
==========================================
+ Coverage 76.36% 76.47% +0.10%
==========================================
Files 1011 1013 +2
Lines 111269 112079 +810
==========================================
+ Hits 84970 85707 +737
- Misses 19815 19859 +44
- Partials 6484 6513 +29 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
Author
|
Implemented in
Validated with |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/third_party_notices_hook.test.sh (2)
50-53: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winStrengthen the index-only hook check.
The regex matches forbidden commands only at the start of a physical line. It misses forms such as
if python3 ...,env python3 ...,git -C ... add, and command substitutions. A forbidden hook change can pass this test. Use a shell-aware check or a strict allow-list for executable commands.🤖 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/third_party_notices_hook.test.sh` around lines 50 - 53, Strengthen the pre-commit validation in scripts/third_party_notices_hook.test.sh beyond the current line-start regex so it detects forbidden execution or staging commands in shell contexts such as conditionals, env invocations, git -C ... add, and command substitutions. Prefer a shell-aware parser or strict allow-list of permitted executable commands, while preserving the existing failure message and exit behavior.
35-49: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover
optionalDependenciesexplicitly.The hook compares both
dependenciesandoptionalDependencies, but these cases populate onlydependencies. Add an optional dependency to the rejection, acceptance, and metadata-only scenarios.🤖 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/third_party_notices_hook.test.sh` around lines 35 - 49, Update the test scenarios around the pre-commit hook to include an optionalDependencies entry alongside dependencies in the missing-notices rejection, notices-present acceptance, and metadata-only acceptance cases. Keep the existing expected commit outcomes unchanged while ensuring both dependency sections are covered.scripts/third_party_notices.test.py (2)
202-203: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the release environment for every invocation.
run.call_argschecks only the last call. An earlier release target could omitCGO_ENABLED=0and this test would still pass. Iterate overrun.call_args_listand verify the environment and target arguments for every invocation.🤖 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/third_party_notices.test.py` around lines 202 - 203, Update the test around the release invocation mock to iterate through run.call_args_list instead of checking only run.call_args. For every invocation, assert CGO_ENABLED is "0" and validate the corresponding release target arguments against notices.RELEASE_TARGETS.
47-61: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winUse a complete Apache-2.0 fixture.
APACHE_TEXTcontains the header and section headings, but not the license clauses. The accepted component test can therefore accept an incomplete Apache document while the truncated-license test intends to reject incomplete text. Use the complete license text, or separate a minimal detector fixture from the component-preservation fixture.🤖 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/third_party_notices.test.py` around lines 47 - 61, Update the APACHE_TEXT fixture used by the third-party notices tests to contain the complete Apache-2.0 license clauses, not just its header and section headings. Preserve the truncated-license test’s ability to reject incomplete text; if a minimal fixture is still needed for detection, separate it from the complete component-preservation fixture.
🤖 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.
Inline comments:
In `@scripts/third_party_notices_hook.test.sh`:
- Around line 31-33: Update the test setup around the go.mod change and the hook
validation to verify regenerated THIRD_PARTY_NOTICES.md output, rather than
treating a go directive-only change as metadata-only. Use the existing hook
invocation and notice-generation comparison path to assert that go.mod changes
producing different notices are rejected.
---
Nitpick comments:
In `@scripts/third_party_notices_hook.test.sh`:
- Around line 50-53: Strengthen the pre-commit validation in
scripts/third_party_notices_hook.test.sh beyond the current line-start regex so
it detects forbidden execution or staging commands in shell contexts such as
conditionals, env invocations, git -C ... add, and command substitutions. Prefer
a shell-aware parser or strict allow-list of permitted executable commands,
while preserving the existing failure message and exit behavior.
- Around line 35-49: Update the test scenarios around the pre-commit hook to
include an optionalDependencies entry alongside dependencies in the
missing-notices rejection, notices-present acceptance, and metadata-only
acceptance cases. Keep the existing expected commit outcomes unchanged while
ensuring both dependency sections are covered.
In `@scripts/third_party_notices.test.py`:
- Around line 202-203: Update the test around the release invocation mock to
iterate through run.call_args_list instead of checking only run.call_args. For
every invocation, assert CGO_ENABLED is "0" and validate the corresponding
release target arguments against notices.RELEASE_TARGETS.
- Around line 47-61: Update the APACHE_TEXT fixture used by the third-party
notices tests to contain the complete Apache-2.0 license clauses, not just its
header and section headings. Preserve the truncated-license test’s ability to
reject incomplete text; if a minimal fixture is still needed for detection,
separate it from the complete component-preservation fixture.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1affd966-82ae-435e-a736-7917e0d9a8a2
📒 Files selected for processing (7)
.githooks/pre-commitREADME.mdTHIRD_PARTY_NOTICES.mdscripts/testdata/third_party_notices/mit-package/LICENSEscripts/third_party_notices.pyscripts/third_party_notices.test.pyscripts/third_party_notices_hook.test.sh
🚧 Files skipped from review as they are similar to previous changes (5)
- README.md
- .githooks/pre-commit
- scripts/testdata/third_party_notices/mit-package/LICENSE
- THIRD_PARTY_NOTICES.md
- scripts/third_party_notices.py
This reverts commit bb07fa2.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add a deterministic third-party license notice document and require it to stay in sync with production Go and npm dependencies. The release and preview package workflows now validate that notices are current and included in published packages.
Changes
scripts/third_party_notices.py, tests, and a local Git hook for deterministic dependency noticesTHIRD_PARTY_NOTICES.mdto npm, preview, and GoReleaser package contentsmake check-third-party-noticesbefore signing or publicationTest Plan
python3 scripts/third_party_notices.test.pypassedbash scripts/third_party_notices_hook.test.shpassedbash scripts/pkg-pr-new-workflow.test.shandbash scripts/release-workflow.test.shpassedgit diff --check origin/main...HEADpassedmake unit-testwas not run for this publication reviewRelated Issues
N/A
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores