-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
fix: revert CSP header and script-src addition #25445
Conversation
Test summaryRun details
View run in Cypress Dashboard ➡️ Flakiness
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
Not that this fixes the other problems with |
I think the overwriting here is a bug. It looks like |
* develop: (45 commits) fix: re-enable CYPRESS_INTERNAL_VITE_DEV development (#25364) fix: add skip domain injection description (#25463) fix: revert CSP header and script-src addition (#25445) chore: Update v8 snapshot cache (#25401) feat: Do not strip CSP headers from HTTPResponse (#24760) fix: keep spaces in formatted output in test runner (#24687) fix: Restrict dependency versions to known supported ranges (#25380) chore: Update v8 snapshot cache (#25370) feat: experimental skip domain injection (#25307) chore: support vite v4 for component testing (#25365) feat: Use JSX/TSX in generated spec filenames (#25318) docs(angular): Properties that are spied upon have to be defined within `componentProperties` instead of on root level. (#25359) chore: remove lint-changed from scripts/docs (#25308) chore: bump to 12.3.0 [skip ci] (#25355) fix: make NODE_ENV "production" for prod builds of launchpad (#25320) fix: .contains() should only return one element at all times (#25250) feat: add currentRetry to Cypress API (#25297) chore: release @cypress/webpack-dev-server-v3.2.2 chore: release create-cypress-tests-v2.0.1 fix: change wording for spec creation (#25271) ...
User facing changelog
N/A. Reverts feature.
Additional details
After merging #24760, the development team noticed that there were regressions in some of the authentication providers that were previously supported. This is mainly due to no longer stripping CSP headers, as well as potentially overwriting them.
In the current state, #24760 overwrites current CSP headers. This is an issue with Salesforce domains for example, where
Content-Security-Policy=upgrade-insecure-requests
becomesContent-Security-Policy=script-src 'nonce-<NONCE>'
. Since script-src itself is then required for even inline scripts, unsafe keywords would likely be required. The desired behavior here would likely be to not modify this header in this case sincescript-src
is not present, and if it is present, we would need these nonces to likely be added to each inline script, probably overwriting the previous nonce. Addingscript-src
to injected html files is likely more involved than previously thought.Another issue that is now present with this feature is CSP headers that are not compatible with Cypress. For instance, Auth0 provides
Content-Security-Policy='frame-ancestors 'none'
, which has the same effect asX-Frame-Options='deny'
. This means that the site cannot load in an iframe, which is inherently incompatible with CypressIn the near future, we need to investigate what CSP policies might be a problem with Cypress, as well as how to support script nonce injection with Cypress if the CSP policy is present. Is is likely a good idea once these are vetted and discovered that we add some system tests that verify the rewrite behavior is correct and that sites load properly when visited.
Steps to test
N/A, revert
How has the user experience changed?
PR Tasks
cypress-documentation
?type definitions
?