Repository navigation
fix: close security scan findings and license checks - #140
Merged
Merged
Conversation
- run CI checks for main pushes and non-draft PRs regardless of branch name - exempt Dependabot by PR identity instead of spoofable message text - reject attribution and signoffs discarded by Git scissors Generated-by: Codex <noreply@openai.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- evaluate SPDX choices and obligations in Node, Python, and CycloneDX reports - reject disallowed conjunctions and malformed expressions with regression cases - remove manifest exclusions that emitted empty-match warnings Generated-by: Codex <noreply@openai.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The new commitlint condition would block generated Release Please pull requests.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Hardens CI and footer validation, corrects SPDX license-expression checks, and cleans Python packaging configuration.
Changes:
- Removes unsafe CI/commitlint bypasses.
- Ignores content after Git scissors markers.
- Adds SPDX/CycloneDX license parsing and regression coverage.
| File | Description |
|---|---|
packages/python-gitlint/MANIFEST.in |
Removes redundant exclusions. |
packages/node-commitlint/tests/rai-signed-off-by.test.ts |
Tests scissors handling. |
packages/node-commitlint/tests/rai-footer-exists.test.ts |
Tests attribution before/after scissors. |
packages/node-commitlint/tests/integration.test.ts |
Adds policy integration tests. |
packages/node-commitlint/src/rules/rai-signed-off-by.ts |
Excludes discarded scissors content. |
packages/node-commitlint/src/rules/rai-footer-exists.ts |
Excludes discarded scissors content. |
docs/architecture.md |
Documents Node/Python behavior. |
commitlint.config.js |
Removes message-based Dependabot bypass. |
.github/workflows/test-and-build.yml |
Reworks CI job conditions. |
.github/scripts/test_check_licenses.py |
Expands license-checker coverage. |
.github/scripts/check-licenses.py |
Parses SPDX and CycloneDX licenses. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: df5b954be1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- revert scissors-marker truncation in rai-footer-exists and rai-signed-off-by - diff lines under the marker can never match the anchored footer patterns - CI lints committed history, where git has already discarded scissors content - restore the documented gitlint input nuance and remove the scissors tests - recovers the 436-byte bundle growth flagged by bundle analysis Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- mark commitlint.config.js with a JSDoc @Satisfies UserConfig so rule tuples keep literal types - drop the stale ignores option the config no longer exports Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- drop job-level Dependabot exemption so commitlint runs on every non-draft PR - on Dependabot and Release Please PRs, skip only verified commits from that bot - lint every other commit on those PRs, so pushes onto a bot branch stay checked - release exemption requires github-actions[bot] author plus a release-please-- branch Generated-by: Codex <noreply@openai.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- resolve the JSON path argument and reject anything outside the cwd - closes Sonar pythonsecurity:S8707 path traversal on the CLI argument Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- license checker tests run in the security audit workflow without coverage reporting - unreported coverage on .github sources was failing the new-code coverage gate Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
anchildress1
commented
Oct 1, 2026
anchildress1
left a comment
anchildress1
left a comment
Owner
Author
There was a problem hiding this comment.
Self-review pass: 2 findings, both fixed locally and not yet pushed.
- map OSI Apache, BSD, ISC, MIT and PSF classifiers to allow-list ids before SPDX parsing - cyclonedx-py emits classifiers verbatim, so arrow and python-dateutil failed the audit - unknown classifiers such as GPL still fail closed; add 3 regression cases Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- forward commitlint.config.js ignores into lint() so a reintroduced bypass fails the test Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- call node_modules/.bin/commitlint so CI never fetches an unpinned package - clears Sonar githubactions:S6505 and S8543 on the commitlint step Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
Owner
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
- join multiple component license entries with AND instead of accepting any one - map trove classifiers per entry so multi-classifier components still resolve - add regression cases for MIT plus GPL-3.0 and dual BSD declarations Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- run the checker in a subprocess from a temp dir and assert outside paths exit 1 Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
- iterate git rev-list instead of the PR commits API, which caps at 250 entries - use the API only as an allowlist of verified bot commits so omissions fail closed Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
Contributor
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 8
Open (9)
AND/ORwith… · Newif:changes drop the previous!startsWith(github.head_ref, 'release-please--')gating,… · Newif:changes drop the previous!startsWith(github.head_ref, 'release-please--')gating,… · Newif:changes drop the previous!startsWith(github.head_ref, 'release-please--')gating,… · Newif:changes drop the previous!startsWith(github.head_ref, 'release-please--')gating,… · Newif:changes drop the previous!startsWith(github.head_ref, 'release-please--')gating,… · Newignoresvia a type cast, butcommitlint.config.jsno longer definesignores… · Newglobal-excludeentries means common unwanted artifacts (e.g.,*.pyc,… · Newjq. To reduce external tooling dependency and simplify… · New
- state the accepted SPDX operators, precedence, and fail-closed constructs Generated-by: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Ashley Childress <anchildress1@gmail.com>
|
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
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
commitlintbinary instead ofnpx(SonarS6505/S8543).ignoresentry fromcommitlint.config.js, since its message text could be forged; the config is now type-checked via JSDoc@satisfies.AND/ORpolicy checks in the license checker, including CycloneDX expressions; every declared CycloneDX license entry must be allowed, and PyPI trove classifiers map to allow-list ids.pythonsecurity:S8707)..github/**from Sonar coverage, since the license checker's tests run in the security audit workflow without coverage reporting.Validation
make ai-checkspassed: 49 Node and 56 Python tests, both at 100% coverage.python3 .github/scripts/test_check_licenses.pypassed all 31 cases, including a subprocess check that a path outside the working directory is rejected.actionlint .github/workflows/test-and-build.ymlpassed.origin/main..HEAD.github-actions[bot]and GitHub-verified.