Conversation
There was a problem hiding this comment.
Pull request overview
Adds a Content Security Policy (CSP) to scratch.html, with allowed origins derived from deployment environment variables via webpack template parameters, and updates CI/CD workflow inputs to support multiple API origins (staging + test).
Changes:
- Load
.envduring webpack config evaluation and derive CSP-friendly origin strings (API + assets) for HTML template injection. - Add a CSP
<meta http-equiv="Content-Security-Policy">tosrc/scratch.html, including a dev-onlyunsafe-eval. - Extend deployment workflows to pass a
CSP_API_MULTIPLE_ORIGINSvalue (and adjust chunk syncing include patterns).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
webpack.config.js |
Loads env for build-time templating; computes CSP origins and passes them into scratch.html template. |
src/scratch.html |
Introduces a CSP meta tag with directives driven by env-derived origins and dev/prod differences. |
.github/workflows/deploy.yml |
Adds a workflow input and env var for CSP_API_MULTIPLE_ORIGINS; modifies S3 sync include patterns. |
.github/workflows/ci-cd.yml |
Supplies staging-specific csp_api_multiple_origins value to deployment workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| run: | | ||
| aws s3 sync ./build/ s3://${{ secrets.AWS_S3_BUCKET }}/${{ needs.setup-environment.outputs.deploy_dir }} --endpoint ${{ secrets.AWS_ENDPOINT }} --progress-frequency 5 | ||
| aws s3 sync ./build/chunks/ s3://${{ secrets.AWS_S3_BUCKET }}/chunks/ --endpoint ${{ secrets.AWS_ENDPOINT }} --exclude "*" --include "fetch-worker*" --progress-frequency 5 | ||
| aws s3 sync ./build/chunks/ s3://${{ secrets.AWS_S3_BUCKET }}/chunks/ --endpoint ${{ secrets.AWS_ENDPOINT }} --exclude "*" --include "fetch-worker*" --include "mediapipe/**" --progress-frequency 5 |
There was a problem hiding this comment.
On testing and staging, but NOT on localhost, there is javascript being pulled in from the following CDN when trying to use the face sensing extension
Whats happening in scratch
In scratch-editor there is an initial attempt to see if chunks exist for that extension
But a fallback is established if those chunks aren’t present locally
What we can do in editor-ui to get around this
It looks like the scratch-editor makes these available as part of the build - so they should exist in the dist folder for scratch-gui In editor-ui, we are already copying the chunks from scratch-gui via web pack so this will include the face-sensing code
So updating and adding this to our aws sync should get this working on all our environments - essentially we would then be hosting the code ourselves and the can request wouldn’t happen.
I’ve had to dig a little to understand the above - but I think this one liner should enable the face sensing extension without having to hit the CDN
There was a problem hiding this comment.
Great. Since we already have disabled some extensions that is also an option if this there are more problems with this extension or others.
|
I saw Scratch doesn't work for the build for this PR - https://staging-editor-static.raspberrypi.org/branches/1438_merge/web-component.html
|
I've had look at this - in our current Scratch GUI bundle/runtime, Adding |
zetter-rpf
left a comment
There was a problem hiding this comment.
Adding unsafe-eval for all environment for now - I don't think there is an easy work around for this at the moment
Even with unsafe-eval this is really valuable - it will help us see in testing if we are depending on content from other domains.
issue: 1257
I’ve built the CSP to try to tap into the env variables that we use for deployments.
I’ve also tried to do some due diligence on the testing staging and staging environments in order to anticipate any problems - I did find something that was a begin pulled in on those environments that I’ve added a workaround for (see the comment below)
Local dev CSP output
This has to feature 'unsafe-eval' to allow the dev server to work
Staging CSP output (take values from deploy-main in ci-cd.yml)
As the test environment uses editor-ui in staging that testing api needs to be listed on staging as well
Production (take values from deploy-tag in ci-cd.yml)
CSP violations for assets
Regarding current attempts to access images from
https://cdn.assets.scratch.mit.edu/- the set up above will result in CSP errors in the console.This should be short lived until we arrive at the solution for asset hosting - I don’t think this will stop anything else in the app working as a results. If we choose to host images on a different domain to the ones already stated we should be able to just add that domain to the img-src/media-src CSP.