-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: check integrity when loading bpmn-visualization
from CDN
#541
Merged
tbouffard
merged 24 commits into
master
from
refactor/use_resource_integrity_for_bpmn-visu
Sep 8, 2023
Merged
feat: check integrity when loading bpmn-visualization
from CDN
#541
tbouffard
merged 24 commits into
master
from
refactor/use_resource_integrity_for_bpmn-visu
Sep 8, 2023
Conversation
This file contains 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
This was referenced Sep 8, 2023
…or tests)" This reverts commit 5d4399d.
csouchet
approved these changes
Sep 8, 2023
tbouffard
changed the title
feat: use resource integrity when loading bpmn-visualization from CDN
feat: check integrity when loading Sep 8, 2023
bpmn-visualization
from CDN
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously, we load the
bpmn-visualization
IIFE bundle from thejsdelivr
CDN without integrity. This was not consistent with the rest of resources that we load from this CDN and this could lead to safety risks.The version of
bpmn-visualization
is updated automatically when a new release is available. Changing the version is easy, but the integrity computation requires more work. As a first implementation, we decided to not do it.Now, the integrity string is first computed from the bundle of the npm package (the package is downloaded by the GitHub workflow). Then, all pages using the bundle from the jsdelivr CDN are updated with the new version (as before) and with the new integrity string.
All pages have been updated to use the
bpmn-visualization
0.39.0 integrity value in order to benefit from the improvement without waiting for the new version.Live environment of examples
https://cdn.statically.io/gh/process-analytics/bpmn-visualization-examples/71365fa/examples/index.html
Tests
I tested the script with command locally on Ubuntu 20.04:
./scripts/update-lib-version.bash 0.39.0 sha384-E2ofmlxS5nRTDew9wflMEYwva2yN5z16RmzyDVzNy2IyEYqqOlqOmrLXkNZpXSOQ
--> no change./scripts/update-lib-version.bash 0.38.0 sha384-K2Sz6i2iSMe18FznqMMiRexBzPN/nlxor5+/M2L3FwJBlThg/jcYMvkjKvqRC5uQ
--> changes, the page correctly use the 0.38.0 version and the integrity is verifiedI cannot test on Windows or macOS, so please fix the syntax if you find issues. Remember that this script is almost never used locally but mainly in a GitHub workflow running on Ubuntu 22. So this is clearly not a priority to make it work on other OS.
Also tested with GitHub actions with a modified workflow that doesn't update the lib demo (not require for the test and make the workflow run easier to configure) - see 5d4399d:
Notes
I tried to validate the npm package checksum but I didn't find a solution to use the value returned by
npm view
Another way is to verify the signature of the package (see resource below).
I decided to not spend more time on this as part of this PR. If there is an integrity issue with the downloaded gzipped package, it is impossible to unarchive it. We currently don't validate that the file has not been unintentionally modified.
We can leave with it for now IMHO. This PR already improves the previous situation.
Resources