fix(sri): add integrity attributes with native html plugin#7133
fix(sri): add integrity attributes with native html plugin#7133chenjiahan merged 1 commit intomainfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello @chenjiahan, 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 addresses an issue where Subresource Integrity (SRI) attributes were not being correctly applied when Rsbuild was configured to use its native HTML plugin. The changes involve adjusting the SRI plugin's configuration logic and adding new end-to-end tests to confirm that SRI attributes are now properly generated for enhanced security across different build and development setups. 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
|
There was a problem hiding this comment.
Code Review
The pull request successfully implements support for Subresource Integrity (SRI) attributes when using the native HTML plugin. The changes in sri.ts correctly introduce conditional logic to apply the htmlPlugin option to the SubresourceIntegrityPlugin only when the JavaScript-based HTML implementation is active. The new E2E tests adequately verify the generation of SRI attributes for both script and link tags in build and development environments, ensuring the functionality works as expected.
There was a problem hiding this comment.
Pull request overview
This PR adds support for generating Subresource Integrity (SRI) attributes when using Rspack's native HTML plugin implementation. Previously, the SRI plugin only configured the htmlPlugin option for the JavaScript implementation, which prevented SRI attributes from being generated when using the native implementation.
Changes:
- Modified the SRI plugin to conditionally set the
htmlPluginoption only for the JS implementation - Added E2E tests to verify SRI attribute generation with the native HTML plugin
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/plugins/sri.ts | Refactored to conditionally set htmlPlugin option only when using JS implementation, allowing native plugin to work automatically |
| e2e/cases/security/sri-native-html-plugin/rsbuild.config.ts | Test configuration enabling SRI with native HTML implementation |
| e2e/cases/security/sri-native-html-plugin/src/index.js | Test entry point importing CSS and setting HTML content |
| e2e/cases/security/sri-native-html-plugin/src/index.css | Test CSS file to verify integrity attributes on link tags |
| e2e/cases/security/sri-native-html-plugin/index.test.ts | E2E tests verifying SRI attributes in both build and dev modes with native HTML plugin |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /<script crossorigin defer integrity="sha384-[A-Za-z0-9+/=]+"/, | ||
| ); | ||
| expect(html).toMatch( | ||
| /link crossorigin href="\/static\/css\/index\.\w{8}\.css" integrity="sha384-[A-Za-z0-9+/=]+"/, |
There was a problem hiding this comment.
The regex pattern is missing the opening < character before link. This will cause the test to match any substring containing "link crossorigin" instead of properly matching an HTML link tag. The pattern should be /<link crossorigin href=... to match the opening of a link element.
| /link crossorigin href="\/static\/css\/index\.\w{8}\.css" integrity="sha384-[A-Za-z0-9+/=]+"/, | |
| /<link crossorigin href="\/static\/css\/index\.\w{8}\.css" integrity="sha384-[A-Za-z0-9+/=]+"/, |
| expect(html).toMatch( | ||
| /<script crossorigin defer integrity="sha384-[A-Za-z0-9+/=]+"/, |
There was a problem hiding this comment.
The regex pattern for the script tag does not include the src attribute. While this pattern will match script tags with integrity attributes, it's too permissive and doesn't verify that the script tag is actually loading a JavaScript file. Consider adding the src attribute pattern to be consistent with the sri-basic test and ensure the pattern matches complete script tags like: /<script crossorigin defer src="\/static\/js\/index\.\w{8}\.js" integrity="sha384-[A-Za-z0-9+/=]+"/
Summary
Checklist