icestorm@0.0.0-20251212-59e4c98#6802
Conversation
|
Hello @UebelAndre, modules you maintain (icestorm) have been updated in this PR. |
There was a problem hiding this comment.
Code Review
This pull request adds a new version 0.0.0-20251212-59e4c98 for the icestorm module. However, it also changes the module's homepage, maintainers, and repository to point to a fork (MrAMS/icestorm) instead of the original upstream (YosysHQ/icestorm). This is a significant change with security implications, as it could be considered a takeover of the module name. My review flags this as a critical issue that requires manual intervention from the BCR maintainers, as per the repository's style guide. If this is a fork, it should be published under a new, distinct module name.
| "homepage": "https://github.com/MrAMS/icestorm", | ||
| "maintainers": [ | ||
| { | ||
| "email": "26427366+UebelAndre@users.noreply.github.com", | ||
| "github": "UebelAndre", | ||
| "github_user_id": 26427366, | ||
| "name": "UebelAndre" | ||
| "email": "", | ||
| "github": "MrAMS", | ||
| "name": "Qijia Yang", | ||
| "github_user_id": 25056812 | ||
| } | ||
| ], | ||
| "repository": [ | ||
| "github:YosysHQ/icestorm" | ||
| "github:MrAMS/icestorm" |
There was a problem hiding this comment.
This pull request modifies the repository, homepage, and maintainers for the existing icestorm module to point to a fork (MrAMS/icestorm) instead of the original upstream (YosysHQ/icestorm). This is a significant and potentially dangerous change that could be considered a module takeover.
Changing the source of an existing module can introduce a serious supply chain security risk, as users might unknowingly start using code from a different, potentially untrusted, source.
If this is a fork with custom changes, it should be published as a new, distinctly named module (e.g., icestorm-mrams-fork) to avoid confusion with the original. Modifying the source repository of an existing module is strongly discouraged.
As per the BCR style guide, this change requires manual review from the BCR maintainers. @bazelbuild/bcr-maintainers
References
- The style guide requires pinging BCR maintainers for significant changes to
maintainersorrepositoryfields inmetadata.json. Changing the upstream repository for an existing module is a significant change that has security implications. (link)
|
This PR contributes to the full implementation of the rules_hdl migration. Compared to the previous version, this update enhances the BCR smoke tests and resolves dependency issues. Additionally, I have configured a monthly CI workflow to automatically fetch and merge updates from the upstream repository. I look forward to any feedback from the reviewers. ❤️ |
UebelAndre
left a comment
There was a problem hiding this comment.
This thread must first be resolved: hdl/bazel_rules_hdl#428 (comment)
Release: https://github.com/MrAMS/icestorm/releases/tag/v0.0.0-20251212-59e4c98
Automated by Publish to BCR