[build] generate and ship cargo-free SBOM and license notices with Selenium Manager - #17812
Closed
titusfortner wants to merge 1 commit into
Closed
[build] generate and ship cargo-free SBOM and license notices with Selenium Manager#17812titusfortner wants to merge 1 commit into
titusfortner wants to merge 1 commit into
Conversation
selenium-ci
added
C-py
Contributor
PR Summary by QodoGenerate and bundle cargo-free SBOM + third-party notices for Selenium Manager
AI Description
Diagram
High-Level Assessment
Files changed (11)
|
Contributor
Code Review by Qodo
1. Aspect crashes on file labels
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+15
to
+27
| py_binary( | ||
| name = "sbom_generator", | ||
| srcs = ["generate_sbom.py"], | ||
| main = "generate_sbom.py", | ||
| visibility = ["//visibility:private"], | ||
| ) | ||
|
|
||
| py_binary( | ||
| name = "notice_generator", | ||
| srcs = ["generate_notice.py"], | ||
| main = "generate_notice.py", | ||
| visibility = ["//visibility:private"], | ||
| ) |
Contributor
There was a problem hiding this comment.
1. Sbom/notice generators untested 📘 Rule violation ☼ Reliability
New Python generators for selenium-manager.cdx.json and selenium-manager-THIRD-PARTY-NOTICES.txt are introduced without accompanying small/unit tests, increasing regression risk for license/SBOM artifact correctness. This conflicts with the requirement to cover new behavior with unit tests where feasible.
Agent Prompt
## Issue description
The newly-added SBOM and NOTICE generator scripts include non-trivial parsing/formatting logic but no corresponding small/unit tests were added in this PR.
## Issue Context
These scripts generate compliance artifacts (CycloneDX SBOM + THIRD-PARTY-NOTICES). Without unit tests, changes to Cargo.lock parsing, manifest parsing, dependency resolution, and license-text deduplication can silently break artifact correctness.
## Fix Focus Areas
- common/manager/BUILD.bazel[15-27]
- common/manager/generate_sbom.py[34-167]
- common/manager/generate_notice.py[43-118]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+82
to
+87
| for attr in ["compile_data", "data"]: | ||
| if not hasattr(ctx.rule.attr, attr): | ||
| continue | ||
| for dep in getattr(ctx.rule.attr, attr): | ||
| for file in dep.files.to_list(): | ||
| if not _is_repo_root(file): |
Contributor
There was a problem hiding this comment.
2. Aspect crashes on file labels 🐞 Bug ≡ Correctness
_collect() in crate_manifests_aspect unconditionally calls dep.files.to_list() for compile_data/data entries, which can fail when those attributes contain direct file labels (not targets with .files). This can break analysis/build of //common/manager:selenium-manager-sbom and //common/manager:selenium-manager-notice because //rust:selenium_manager uses compile_data with file labels.
Agent Prompt
## Issue description
The aspect helper `_collect(ctx)` assumes every element of `ctx.rule.attr.compile_data` / `ctx.rule.attr.data` has a `.files` field and calls `dep.files.to_list()`. When those attributes contain file labels (which can happen in this repo), the aspect can crash during analysis, preventing SBOM/NOTICE generation.
## Issue Context
In `//rust:selenium_manager`, `compile_data` is populated with direct file labels (e.g., markdown resources). The SBOM/NOTICE rules seed the aspect from `//rust:selenium-manager`, which depends on `//rust:selenium_manager`, so the aspect will evaluate `_collect()` on a rule with file labels in `compile_data`.
## Fix Focus Areas
- common/manager/crate_metadata.bzl[79-93]
### Suggested implementation direction
Update `_collect()` to handle both cases:
- If an attribute entry is a file-like object, treat it as a single file.
- If it is a target, iterate `target.files`.
For example (Starlark sketch):
```starlark
for dep in getattr(ctx.rule.attr, attr):
files = []
if hasattr(dep, "files"):
files = dep.files.to_list()
else:
# file label
files = [dep]
for file in files:
...
```
Alternatively, prefer `ctx.rule.files.<attr>` when present, to directly obtain `File` objects for file labels, and separately handle target entries if needed.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
titusfortner
marked this pull request as draft
July 22, 2026 17:26
Member
Author
|
Unfortunately, I think the right way to do this is actually to extend our current release assets hack rather than try to get it working cross platform with bazel. |
2 tasks
Member
Author
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
2 participants
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.
🔗 Related Issues
Fixes #17808
💥 What does this PR do?
selenium-manager.cdx.json— a CycloneDX SBOM inventorying the ~300 statically-linked Rust crates (name, version, checksum, dependency graph, SPDX license) for vulnerability and license scanning.selenium-manager-THIRD-PARTY-NOTICES.txt— the third-party attribution file reproducing each bundled crate's copyright notice and full license text, which MIT/BSD/ISC/Apache-2.0 require in binary distributions.🔧 Implementation Notes
Cargo.tomlmanifests andLICENSE/NOTICEfiles crate_universe has already vendored; the SBOM component graph comes fromrust/Cargo.lock.winapionly on Windows). A split transition seeds the aspect under every shipped target triple and unions the results, so both artifacts cover all platforms from a single build without cross-compiling — only the crates' source files are read.Cargo.lockgraph — industry-standard, where over-inclusion is safe and omission is a coverage gap. Crates not linked on any platform (build-only, test-only) resolve toNOASSERTION. Three crates declare an SPDX license but bundle no license file and are listed as such.Cargo.lock) rather than the compiled binary so is excluded.🤖 AI assistance
💡 Additional Considerations
.dist-info/sboms/(PEP 770) for wheels andMETA-INF/sbom/for jars — because the pinnedrules_pythonpy_wheelcannot write into.dist-info/and the Java move would disturb the manager binary's resource-loading path. Deferred as a follow-up (gated onrules_pythonPEP 770 support) so a generic scanner can auto-discover the SBOM.🔄 Types of changes