feat: Selective CSP header stripping from HTTPResponse

[pgoforth]

• Closes #1030

User facing changelog

• Cypress now allows selective stripping of Content-Security-Policy and Content-Security-Policy-Report-Only header directives via the stripCspDirectives config option.

Additional details

• add generated nonce to inline script injection
• append generated nonce policy value to each CSP header script-src-elem, script-src, and default-src directive if provided in original response
• remove mandatory content-security-policy and content-security-policy-report-only header stripping
• selectively strip problematic CSP directive frame-ancestors because it prevents Cypress from loading target into iframe
• allows for proviiding CSP directives to strip in the parseCspHeaders method
• add config option stripCspDirectives that permits selective stripping of individual CSP directives
• default value for the stripCspDirectives config option maintains existing CSP header stripping

Steps to test

1. Create a local HTTP server that responds to a request with CSP headers defined
2. Run a Cypress test that sends a request to the local server, and tests the response header, similar to the following:

const route = 'http://localhost:8080';
cy.request(route).as('route');
cy.get('@route').should(response => {
    expect(response).to.have.property('headers');
    expect(response.headers).to.have.property('content-security-policy');
    expect(response.headers['content-security-policy']).to.match(
        /script-src 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/
    );
});
cy.visit(route);

How has the user experience changed?

This change does not affect UI/UX

PR Tasks

• Have tests been added/updated?
• Has a PR for user-facing changes been opened in cypress-documentation?
  Documentation for stripCspDirectives
• Have API changes been updated in the type definitions?

[pgoforth]

I'd like to use this time while the PR is in draft to discuss potentially exposing a configuration option allowing the selective stripping of certain CSP directives, with the goal being to default strip to ALL directives (which maintains parity with current version of Cypress)

The only reason to consider this is to make this feature opt-in, with an eye to changing the default in a future release. I would most definitely need some guidance as to how to best implement any global config setting, but in the end, we may not even want to expose a config property at all. I'll leave it up for discussion.

EDIT
I decided it's in the best interest of all Cypress users to create a config flag called stripCspDirectives: boolean | string | string[] and default it to true

stripCspDirectives: true Would remove the CSP headers entirely and maintain parity with old versions of Cypress.

stripCspDirectives: false would strip ONLY functionality breaking directives...like frame-ancestors

stripCspDirectives: '<directive-name>' would strip ONLY the specific directive defined, potentially including functionality breaking directives...like frame-ancestors

stripCspDirectives: ['<directive-name>', '<directive-name>', ...] would strip ONLY the specific directives defined in the array, potentially including functionality breaking directives...like frame-ancestors

[cypress]

25 flaky tests on run #45566 ↗︎

0

27265

1307

0

25

Details:

feat: Selective CSP header directive stripping from HTTPResponse
Project: cypress	Commit: ee0269baaa
Status: Passed	Duration: 19:45 💡
Started: Apr 14, 2023 9:53 PM	Ended: Apr 14, 2023 10:12 PM

  e2e/origin/navigation.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test		Artifacts
delayed navigation > errors > redirects to an unexpected cross-origin		Output Video

  e2e/origin/user_agent_override.cy.ts • 1 flaky test • 5x-driver-electron

View Output Video

Test		Artifacts
user agent override > persists modified user agent after cy.go		Output Video

  cypress/cypress.cy.js • 3 flaky tests • 5x-driver-electron

View Output Video

Test		Artifacts
... > correctly returns currentRetry		Output Video
... > correctly returns currentRetry		Output Video
... > correctly returns currentRetry		Output Video

  commands/net_stubbing.cy.ts • 1 flaky test • 5x-driver-firefox

View Output Video

Test		Artifacts
network stubbing > intercepting request > can delay and throttle a StaticResponse		Output

  project-setup.cy.ts • 1 flaky test • launchpad-e2e

View Output Video

Test		Artifacts
Launchpad: Setup Project > Command for package managers > works with Yarn 3 Plug n Play		Output Screenshots Video

The first 5 flaky specs are shown, see all 15 specs in Cypress Cloud.

_{This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.}

[pgoforth]

Implementation for the config parameter is blocked by #21151. It seems that the server restart is either:

1. Not happening
2. Not getting the config updates

I would imagine that it's the later.

EDIT
Seems the restart never happens, which would certainly be the reason for #21151 because the config in the server context is static

[pgoforth]

Everything here works when you set the stripCspDirectives attribute in you cypres.config.js file...you just cannot set is on a per spec/run basis using Cy.config. This is the same fate as the blockHosts parameter in #21151.

This PR stands on it's own, and hopefully (since the parameter is setup the same way as blockHosts, when that issue is resolved, this one should be too.

[mschile]

Hi @pgoforth 👋, thanks for this contribution! I'm planning on bringing it up next week with the team so we can discuss it.

[pgoforth]

@AtofStryker Looks like the snapshots for the new system tests I wrote need updating, and I had to update a unit test. I do not know how to run the system tests and update the snapshots. I'm rebasing off develop, resolving the conflict, and pushing up the unit test fix. Hopefully you can get the snapshots updated and passing.

[AtofStryker]

@pgoforth taking a look and should have it updated soon

[AtofStryker]

@mschile since this isn't making it into today's release you may have to handle it for the next release

[AtofStryker]

@emilyrohrbough I think this is ready for another look. I needed to tweak the system tests a bit but I think they provide ample coverage. We also added sandbox and navigate-to to the always strip directive since sandbox seems to apply to all iframes, which breaks Cypress, and navigate-to is not implemented in any browser yet and could very like cause issues with app navigation since in some cases we interact with the document directly.

[AtofStryker]

@pgoforth we are working on getting a few additional reviewers for this PR on Monday. TLDR there is a test that is failing in CI on your branch, but we believe it is due to flake since the same commits on a different branch owned by cypress ICs seems to work fine. This should be going into develop next week barring anything that might be problematic that is caught in review.

[pgoforth]

@AtofStryker That all sounds great. I'm OOO next week, but will have my machine with me while I'm traveling. Let me know if there's anything I can assist with and will do my best to help out.

[cypress-bot]

Released in 12.15.0.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.15.0, please open a new issue.