Repository navigation
feat(sbom): include package license metadata - #1208
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change records package license metadata during installation, resolves it from installed packages and fallback state, and adds license values to CycloneDX and SPDX SBOM output. Tests cover parsing, path layouts, fallback behavior, and SPDX validation. ChangesLicense metadata and SBOM generation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InstallState
participant LicenseCollector
participant PackageManifest
participant SBOMCommand
participant SBOMRenderers
InstallState->>LicenseCollector: Persist package license metadata
SBOMCommand->>LicenseCollector: Collect installed package metadata
LicenseCollector->>PackageManifest: Read manifest license fields
PackageManifest-->>LicenseCollector: Return license metadata
LicenseCollector-->>SBOMCommand: Return metadata map
SBOMCommand->>SBOMRenderers: Render CycloneDX or SPDX output
SBOMRenderers-->>SBOMCommand: Emit classified or validated license values
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Greptile SummaryThis PR adds installed-package license discovery and emits license metadata in CycloneDX and SPDX SBOMs.
Confidence Score: 4/5The PR is not yet safe to merge because legacy or incomplete license snapshots can still lose dependency licenses after a one-shot custom virtual-store setting is removed. The effective custom virtual-store base path is not persisted, so an SBOM generated later must use current settings when a package lacks a cached license; this reconstructs a nonexistent package path and emits missing or Files Needing Attention: crates/aube/src/commands/licenses.rs, crates/aube/src/state.rs Important Files Changed
Reviews (9): Last reviewed commit: "chore(deps): loosen spdx patch requireme..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Instruction counts
1 benchmark(s) above the 1% gate: Only instruction counts gate. Wall clock is shown for context — on identical hardware it moves 4-20% run to run. Measured by tak — instruction-counted CLI benchmarks, stored in this repository's git notes.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/aube/src/commands/licenses.rs (1)
344-354: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider preserving multiple legacy
licensesentries as an SPDX expression.
license_from_valuesonly reads the first element of the legacylicensesarray. A package declaring several licenses in that field loses all but the first. Joining the entries withOR(matching how SPDX expresses multiple licenses) would keep the full information instead of silently dropping entries.♻️ Proposed refactor to combine multiple legacy license entries
pub(super) fn license_from_values( license: Option<&serde_json::Value>, licenses: Option<&serde_json::Value>, ) -> Option<String> { license.and_then(extract_license).or_else(|| { licenses .and_then(|v| v.as_array()) - .and_then(|arr| arr.first()) - .and_then(extract_license) + .map(|arr| arr.iter().filter_map(extract_license).collect::<Vec<_>>()) + .filter(|names| !names.is_empty()) + .map(|names| names.join(" OR ")) }) }🤖 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 `@crates/aube/src/commands/licenses.rs` around lines 344 - 354, Update license_from_values to process every entry in the legacy licenses array rather than only arr.first(), extract each valid license, and combine multiple results into an SPDX OR expression while preserving the existing single-entry and primary license behavior.test/sbom.bats (1)
40-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the lockfile-fallback license path.
Both new tests exercise licenses read from installed
package.jsonmanifests. Neither test exercises the fallback to lockfile license metadata when the installed manifest is unavailable, which is one of the stated PR objectives. Add a test that removes or corrupts an installed package'spackage.jsonafteraube installand confirms the SBOM still emits a license sourced from lockfile data.🤖 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 `@test/sbom.bats` around lines 40 - 49, Add a test in test/sbom.bats covering the lockfile-license fallback: after aube install in the existing fixture setup, remove or corrupt an installed dependency’s package.json, run aube sbom, and assert the dependency still reports its expected lockfile-derived license. Keep the assertion focused on the affected component’s license metadata.
🤖 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 `@crates/aube/src/commands/licenses.rs`:
- Around line 344-354: Update license_from_values to process every entry in the
legacy licenses array rather than only arr.first(), extract each valid license,
and combine multiple results into an SPDX OR expression while preserving the
existing single-entry and primary license behavior.
In `@test/sbom.bats`:
- Around line 40-49: Add a test in test/sbom.bats covering the lockfile-license
fallback: after aube install in the existing fixture setup, remove or corrupt an
installed dependency’s package.json, run aube sbom, and assert the dependency
still reports its expected lockfile-derived license. Keep the assertion focused
on the affected component’s license metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 09f0a720-a857-48cc-a6b4-c1f0b87927cd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomlcrates/aube/Cargo.tomlcrates/aube/src/commands/licenses.rscrates/aube/src/commands/sbom.rstest/sbom.bats
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
Addressed the latest review feedback in 50a0911:
AI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@crates/aube/src/state.rs`:
- Around line 797-805: The fallback in read_state_package_licenses can combine a
stale default license sidecar with the active layout state. Make license lookup
use the same selected state directory as collect_installed_metadata, only
consulting the default sidecar when the default layout is selected; preserve the
empty-map fallback. Add a regression test covering an active legacy state with a
stale default license sidecar.
🪄 Autofix (Beta)
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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 540d2c0a-88a0-4d73-8fb4-18ca233f0ad3
📒 Files selected for processing (4)
crates/aube/src/commands/licenses.rscrates/aube/src/commands/sbom.rscrates/aube/src/state.rstest/sbom.bats
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/aube/src/commands/sbom.rs
- crates/aube/src/commands/licenses.rs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 57a673d. Configure here.
Summary
aube licensesinstalled-package discovery for SBOM generation, including isolated, hoisted, linked, and custom virtual-store layoutslicenseDeclaredfor valid expressions while leavinglicenseConcludedasNOASSERTIONRoot cause
aube sbomonly consumed the lockfile, whileaube licensesindependently read installed package manifests. Canonical aube/pnpm lockfiles do not preserve package licenses, so SBOM output omitted CycloneDX licenses and hardcoded SPDX license fields.Performance
The initial implementation raised SBOM instructions by 53.32%. Selective parsing, cached SPDX classification, and a fingerprinted license sidecar reduced that to 6.31%. Warm install is +0.68%, startup +0.68%, and tree +0.23%, all below the 1% gate. The remaining SBOM-only delta covers reading the compact sidecar, validating SPDX values, and serializing the new license objects.
Validation
cargo fmt --checkcargo clippy -p aube --all-targets -- -D warningscargo test -p aube commands::sbom::testscargo test -p aube commands::licenses::testsmise run test:bats test/sbom.batsCloses discussion #1204.
AI-assisted — Tool: Codex; model: openai/gpt-5; version: unavailable.
Summary by CodeRabbit
New Features
NOASSERTIONwhen unavailable.Bug Fixes