Conversation
Contributor
|
@msukkari your pull request is missing a changelog! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Contributor
License Audit❌ Audit failed to produce results. Check the workflow logs for details. |
Contributor
WalkthroughIntroduces a GitHub Actions workflow that audits open source software licenses in project dependencies. The workflow fetches license information from npm, summarizes results, uses Claude to categorize and validate licenses, and posts detailed audit results as PR comments. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant YL as Yarn Lock
participant NPM as npm Registry
participant CLA as Claude API
participant PR as PR Comment
GHA->>YL: Parse yarn.lock
GHA->>NPM: Fetch licenses for packages (batch, concurrency=10)
NPM-->>GHA: License data + overrides applied
GHA->>GHA: Summarize licenses by type
GHA->>CLA: Send license data for validation & categorization
CLA->>CLA: Identify unresolved & copyleft licenses
CLA-->>GHA: license-audit-result.json
GHA->>GHA: Validate results (unresolved/copyleft thresholds)
GHA->>PR: Post audit summary with structured table
PR-->>GHA: Comment created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
scripts/fetchLicenses.mjs (1)
99-99:⚠️ Potential issue | 🟡 MinorIncomplete string replacement for scoped packages.
The
.replace("%40", "@")only replaces the first occurrence. While scoped packages typically have only one@at the start (e.g.,@scope/package), usingreplaceAllor a regex with the global flag is more robust and clearer in intent.✏️ Suggested fix
- const url = `${NPM_REGISTRY}/${encodeURIComponent(name).replace("%40", "@")}/${version}`; + const url = `${NPM_REGISTRY}/${encodeURIComponent(name).replaceAll("%40", "@")}/${version}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/fetchLicenses.mjs` at line 99, The URL construction in scripts/fetchLicenses.mjs uses encodeURIComponent(name).replace("%40", "@") which only replaces the first encoded '@' and can miss additional occurrences; update the replacement to be global (e.g., use replaceAll("%40","@") or replace(/%40/g, "@")) when building the const url that uses NPM_REGISTRY so all encoded '@' sequences in the package name are properly restored.
🧹 Nitpick comments (5)
.github/workflows/license-audit.yml (2)
43-47: Consider adding a timeout for the Claude action.The Claude audit step could potentially run for an extended period when processing large dependency lists. Consider whether a timeout would be appropriate to prevent runaway CI costs.
You can add a
timeout-minutesat the step level:- name: Audit licenses with Claude timeout-minutes: 15 uses: anthropics/claude-code-action@v1🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/license-audit.yml around lines 43 - 47, The "Audit licenses with Claude" GitHub Actions step (uses: anthropics/claude-code-action@v1) lacks a timeout and may run indefinitely; add a step-level timeout by inserting a timeout-minutes setting (e.g., timeout-minutes: 15) into that step to cap runtime and prevent runaway CI costs, keeping the step name "Audit licenses with Claude" and the existing uses/with configuration unchanged.
125-129: Add error handling for malformed JSON in validation step.If
license-audit-result.jsonexists but contains invalid JSON or is missing expected fields, thenode -ecommands will throw exceptions with potentially unclear error messages.🛡️ Suggested defensive approach
- STATUS=$(node -e "const r = require('./license-audit-result.json'); console.log(r.status)") - UNRESOLVED=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary.unresolvedCount)") - STRONG=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary.strongCopyleftCount)") - WEAK=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary.weakCopyleftCount)") - RESOLVED=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary.resolvedCount)") + STATUS=$(node -e "const r = require('./license-audit-result.json'); console.log(r.status ?? 'UNKNOWN')" 2>/dev/null || echo "UNKNOWN") + UNRESOLVED=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary?.unresolvedCount ?? 0)" 2>/dev/null || echo "0") + STRONG=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary?.strongCopyleftCount ?? 0)" 2>/dev/null || echo "0") + WEAK=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary?.weakCopyleftCount ?? 0)" 2>/dev/null || echo "0") + RESOLVED=$(node -e "const r = require('./license-audit-result.json'); console.log(r.summary?.resolvedCount ?? 0)" 2>/dev/null || echo "0")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/license-audit.yml around lines 125 - 129, The current shell step assigns STATUS, UNRESOLVED, STRONG, WEAK, and RESOLVED by running inline node -e scripts that assume license-audit-result.json is valid; instead replace each node -e invocation with a small defensive snippet that reads the file, wraps JSON.parse in try/catch, verifies the expected fields (status, summary.unresolvedCount, summary.strongCopyleftCount, summary.weakCopyleftCount, summary.resolvedCount), and on parse/validation error prints a clear sentinel (e.g. "INVALID_JSON" for STATUS and "0" for counts) and exits non‑zero or returns defaults; update the assignments for STATUS, UNRESOLVED, STRONG, WEAK, and RESOLVED to use that snippet so malformed JSON or missing fields are handled gracefully.scripts/npmLicenseMap.json (1)
1-1: Add trailing newline for POSIX compliance.The file is missing a newline at the end, which is a common convention for text files.
✏️ Suggested fix
-{} +{} +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/npmLicenseMap.json` at line 1, The JSON file npmLicenseMap.json currently contains "{}" without a trailing newline; update npmLicenseMap.json by adding a single newline character at EOF so the file ends with a newline (POSIX-compliant), ensuring the existing content "{}" remains unchanged except for the added newline.scripts/fetchLicenses.mjs (1)
31-33: Consider edge cases indeepEqualimplementation.Using
JSON.stringifyfor deep equality has known limitations: it's sensitive to key ordering and doesn't handleundefinedvalues or circular references. This is acceptable for simple license object comparisons, but worth documenting or using a library if the license map grows complex.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/fetchLicenses.mjs` around lines 31 - 33, The current deepEqual function (deepEqual) uses JSON.stringify which fails on key ordering, undefined values, and circular refs; replace it with a robust deep equality check (e.g., import and use lodash/isEqual or deep-equal) and update calls to use that function, or if you intentionally want the simple behavior, add a clear comment above deepEqual explaining these limitations and when it is safe to use; reference the deepEqual function name so you can locate and replace or annotate it.scripts/summarizeLicenses.mjs (1)
30-31: Add error handling for JSON parsing.If the input file contains malformed JSON,
JSON.parsewill throw an exception with a generic error. Consider wrapping this in a try-catch for a more user-friendly error message.🛡️ Suggested fix
- const data = JSON.parse(fs.readFileSync(inputPath, "utf-8")); + let data; + try { + data = JSON.parse(fs.readFileSync(inputPath, "utf-8")); + } catch (err) { + console.error(`Failed to parse ${inputFile}: ${err.message}`); + process.exit(1); + } const packages = data.packages || [];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/summarizeLicenses.mjs` around lines 30 - 31, Wrap the JSON.parse(fs.readFileSync(inputPath, "utf-8")) call in a try-catch to handle malformed JSON: read the file into a string, then try JSON.parse and assign to data, and on error log a clear, user-facing message that includes inputPath and the error.message (use the same variable names data, packages, inputPath) and exit with a non-zero code; ensure packages = data.packages || [] still runs only after successful parsing so downstream code is not executed on parse failure.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/fetchLicenses.mjs`:
- Line 22: The current construction of LICENSE_MAP_PATH uses
path.join(path.dirname(new URL(import.meta.url).pathname), ...) which breaks on
Windows; import fileURLToPath from 'url' and replace the dirname/new URL usage
with path.dirname(fileURLToPath(import.meta.url)) so LICENSE_MAP_PATH is built
from a proper filesystem path; update any top-level imports to include
fileURLToPath and change the expression that computes the directory for
LICENSE_MAP_PATH accordingly.
---
Duplicate comments:
In `@scripts/fetchLicenses.mjs`:
- Line 99: The URL construction in scripts/fetchLicenses.mjs uses
encodeURIComponent(name).replace("%40", "@") which only replaces the first
encoded '@' and can miss additional occurrences; update the replacement to be
global (e.g., use replaceAll("%40","@") or replace(/%40/g, "@")) when building
the const url that uses NPM_REGISTRY so all encoded '@' sequences in the package
name are properly restored.
---
Nitpick comments:
In @.github/workflows/license-audit.yml:
- Around line 43-47: The "Audit licenses with Claude" GitHub Actions step (uses:
anthropics/claude-code-action@v1) lacks a timeout and may run indefinitely; add
a step-level timeout by inserting a timeout-minutes setting (e.g.,
timeout-minutes: 15) into that step to cap runtime and prevent runaway CI costs,
keeping the step name "Audit licenses with Claude" and the existing uses/with
configuration unchanged.
- Around line 125-129: The current shell step assigns STATUS, UNRESOLVED,
STRONG, WEAK, and RESOLVED by running inline node -e scripts that assume
license-audit-result.json is valid; instead replace each node -e invocation with
a small defensive snippet that reads the file, wraps JSON.parse in try/catch,
verifies the expected fields (status, summary.unresolvedCount,
summary.strongCopyleftCount, summary.weakCopyleftCount, summary.resolvedCount),
and on parse/validation error prints a clear sentinel (e.g. "INVALID_JSON" for
STATUS and "0" for counts) and exits non‑zero or returns defaults; update the
assignments for STATUS, UNRESOLVED, STRONG, WEAK, and RESOLVED to use that
snippet so malformed JSON or missing fields are handled gracefully.
In `@scripts/fetchLicenses.mjs`:
- Around line 31-33: The current deepEqual function (deepEqual) uses
JSON.stringify which fails on key ordering, undefined values, and circular refs;
replace it with a robust deep equality check (e.g., import and use
lodash/isEqual or deep-equal) and update calls to use that function, or if you
intentionally want the simple behavior, add a clear comment above deepEqual
explaining these limitations and when it is safe to use; reference the deepEqual
function name so you can locate and replace or annotate it.
In `@scripts/npmLicenseMap.json`:
- Line 1: The JSON file npmLicenseMap.json currently contains "{}" without a
trailing newline; update npmLicenseMap.json by adding a single newline character
at EOF so the file ends with a newline (POSIX-compliant), ensuring the existing
content "{}" remains unchanged except for the added newline.
In `@scripts/summarizeLicenses.mjs`:
- Around line 30-31: Wrap the JSON.parse(fs.readFileSync(inputPath, "utf-8"))
call in a try-catch to handle malformed JSON: read the file into a string, then
try JSON.parse and assign to data, and on error log a clear, user-facing message
that includes inputPath and the error.message (use the same variable names data,
packages, inputPath) and exit with a non-zero code; ensure packages =
data.packages || [] still runs only after successful parsing so downstream code
is not executed on parse failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b7afb64-3fbf-46ba-ba86-a09664a7870a
📒 Files selected for processing (5)
.github/workflows/license-audit.yml.gitignorescripts/fetchLicenses.mjsscripts/npmLicenseMap.jsonscripts/summarizeLicenses.mjs
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 by CodeRabbit
New Features
Chores