Skip to content
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
merged 24 commits into from
Sep 8, 2023

Conversation

tbouffard
Copy link
Member

@tbouffard tbouffard commented Sep 8, 2023

Previously, we load the bpmn-visualization IIFE bundle from the jsdelivr 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 verified

I 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

$ npm view bpmn-visualization@0.39.0 dist.integrity
sha512-VX974CN07dVJ8I0d6iWusH87ePZoyTn06me6pD8JCwlHlQOAlDne6zywAxuEDuYde2PmlTpXVUdUrLcUvzSt9A==

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

@tbouffard tbouffard marked this pull request as ready for review September 8, 2023 15:16
@tbouffard tbouffard requested a review from csouchet September 8, 2023 15:18
@tbouffard tbouffard added the enhancement New feature or request label Sep 8, 2023
@tbouffard tbouffard merged commit 18232c2 into master Sep 8, 2023
@tbouffard tbouffard deleted the refactor/use_resource_integrity_for_bpmn-visu branch September 8, 2023 15:26
@tbouffard tbouffard changed the title feat: use resource integrity when loading bpmn-visualization from CDN feat: check integrity when loading bpmn-visualization from CDN Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants