Verify addon before install #112
Conversation
Summary of ChangesHello @vadimpiven, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly bolsters the supply chain security of the 'packages/node' module by integrating a robust cryptographic verification process into its postinstall script. It ensures that any precompiled binaries downloaded are authentic and originate from trusted GitHub Actions workflows, leveraging Sigstore and Rekor for provenance attestation. Additionally, it includes minor dependency updates and improvements to development tooling configurations. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Greptile OverviewGreptile SummaryThis PR enhances the postinstall script with cryptographic verification using Sigstore provenance attestations. The implementation ensures that downloaded precompiled binaries were built in the same GitHub Actions workflow run as the published npm package, preventing supply chain attacks. Key changes:
The verification process validates:
Confidence Score: 5/5
Important Files Changed
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant security enhancement by adding cryptographic verification of the downloaded binary using Sigstore. The implementation is thorough, checking both the npm package and the binary artifact attestations to ensure they originate from the same build workflow. My feedback includes a few suggestions to improve script robustness and configuration consistency.
| function getExtensionValue(cert, oid) { | ||
| const ext = cert.extension(oid); | ||
| if (!ext) return null; | ||
|
|
||
| // v2 extensions (1.3.6.1.4.1.57264.1.8+) are DER-encoded UTF8String | ||
| // v1 extensions store raw ASCII in the value | ||
| const subs = ext.valueObj.subs; | ||
| if (subs && subs.length > 0) { | ||
| return subs[0].value.toString("ascii"); | ||
| } | ||
| return ext.value.toString("ascii"); | ||
| } |
There was a problem hiding this comment.
According to the Fulcio OID documentation, newer extension values (like Issuer v2, Source Repository URI) are specified as UTF8String. The current implementation decodes them as ascii, which is incorrect and could lead to verification failures if the strings contain non-ASCII characters. The decoding should be changed to utf8 for these newer extensions to ensure correctness.
| function getExtensionValue(cert, oid) { | |
| const ext = cert.extension(oid); | |
| if (!ext) return null; | |
| // v2 extensions (1.3.6.1.4.1.57264.1.8+) are DER-encoded UTF8String | |
| // v1 extensions store raw ASCII in the value | |
| const subs = ext.valueObj.subs; | |
| if (subs && subs.length > 0) { | |
| return subs[0].value.toString("ascii"); | |
| } | |
| return ext.value.toString("ascii"); | |
| } | |
| function getExtensionValue(cert, oid) { | |
| const ext = cert.extension(oid); | |
| if (!ext) return null; | |
| // v2 extensions (1.3.6.1.4.1.57264.1.8+) are DER-encoded UTF8String | |
| // v1 extensions store raw ASCII in the value | |
| const subs = ext.valueObj.subs; | |
| if (subs && subs.length > 0) { | |
| return subs[0].value.toString("utf8"); | |
| } | |
| return ext.value.toString("ascii"); | |
| } |
| hide = true | ||
| depends = ["setup:rust"] | ||
| run = "cargo clippy -r --locked --fix --allow-dirty" | ||
| run = "cargo clippy --all-targets --fix --allow-dirty" |
There was a problem hiding this comment.
For consistency with the check:clippy task and to prevent unintended dependency updates, it's recommended to add the -r and --locked flags to this command. The fix command should operate on the same set of dependencies as the check command.
run = "cargo clippy --all-targets -r --locked --fix --allow-dirty"
| }; | ||
| // Extract expected repository from package.json | ||
| const repoUrl = typeof repository === "string" ? repository : (repository?.url ?? ""); | ||
| const repoMatch = repoUrl.match(/github\.com[/:]([^/]+\/[^/.]+)/); |
There was a problem hiding this comment.
The current regex for extracting the repository name from package.json might not handle all possible Git URL formats, such as the common SCP-like syntax (e.g., git@github.com:user/repo.git). Using a more flexible regex would make this security-sensitive script more robust against different URL formats.
| const repoMatch = repoUrl.match(/github\.com[/:]([^/]+\/[^/.]+)/); | |
| const repoMatch = repoUrl.match(/github\.com[/:]([^/]+\/[^/]+?)(\.git)?$/); |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
No description provided.