Skip to content

Commit

Permalink
fix: revert CSP header and script-src addition (#25445)
Browse files Browse the repository at this point in the history
* Revert "feat: Do not strip CSP headers from HTTPResponse (#24760)"

This reverts commit 0472bb9.

* run ci
  • Loading branch information
AtofStryker authored Jan 13, 2023
1 parent f71c30a commit 478407e
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 559 deletions.
17 changes: 0 additions & 17 deletions packages/driver/cypress/e2e/e2e/security.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,4 @@ describe('security', () => {
cy.visit('/fixtures/security.html')
cy.get('div').should('not.exist')
})

it('works even with content-security-policy script-src', () => {
// create report URL
cy.intercept('/csp-report', (req) => {
throw new Error(`/csp-report should not be reached:${ req.body}`)
})

// inject script-src on visit
cy.intercept('/fixtures/empty.html', (req) => {
req.continue((res) => {
res.headers['content-security-policy'] = `script-src http://not-here.net; report-uri /csp-report`
})
})

cy.visit('/fixtures/empty.html')
.wait(1000)
})
})
37 changes: 3 additions & 34 deletions packages/proxy/lib/http/response-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ import { URL } from 'url'
import { CookiesHelper } from './util/cookies'
import { doesTopNeedToBeSimulated } from './util/top-simulation'
import { toughCookieToAutomationCookie } from '@packages/server/lib/util/cookies'
import { generateCspDirectives, hasCspHeader, parseCspHeaders } from './util/csp-header'
import crypto from 'crypto'

interface ResponseMiddlewareProps {
/**
Expand Down Expand Up @@ -313,36 +311,6 @@ const SetInjectionLevel: ResponseMiddleware = function () {
// We set the header here only for proxied requests that have scripts injected that set the domain.
// Other proxied requests are ignored.
this.res.setHeader('Origin-Agent-Cluster', '?0')

// Only patch the headers that are being supplied by the response
const incomingCspHeaders = ['content-security-policy', 'content-security-policy-report-only']
.filter((headerName) => hasCspHeader(this.incomingRes.headers, headerName))

if (incomingCspHeaders.length) {
// In order to allow the injected script to run on sites with a CSP header
// we must add a generated `nonce` into the response headers
const nonce = crypto.randomBytes(16).toString('base64')

this.res.injectionNonce = nonce

// Since CSP headers are not cumulative, the nonce policy must be set on each CSP header individually
const mapPolicies = (policy: Map<string, string[]>) => {
const cspScriptSrc = policy.get('script-src') || []

policy.set('script-src', [...cspScriptSrc, `'nonce-${nonce}'`])

return generateCspDirectives(policy)
}

// Iterate through each CSP header
incomingCspHeaders.forEach((headerName) => {
// Map the nonce on each CSP header
const modifiedCspHeaders = parseCspHeaders(this.incomingRes.headers, headerName).map(mapPolicies)

// To replicate original response CSP headers, we must apply all header values as an array
this.res.setHeader(headerName, modifiedCspHeaders)
})
}
}

this.res.wantsSecurityRemoved = (this.config.modifyObstructiveCode || this.config.experimentalModifyObstructiveThirdPartyCode) &&
Expand Down Expand Up @@ -388,6 +356,8 @@ const OmitProblematicHeaders: ResponseMiddleware = function () {
'x-frame-options',
'content-length',
'transfer-encoding',
'content-security-policy',
'content-security-policy-report-only',
'connection',
])

Expand Down Expand Up @@ -570,7 +540,6 @@ const MaybeInjectHtml: ResponseMiddleware = function () {

const decodedBody = iconv.decode(body, nodeCharset)
const injectedBody = await rewriter.html(decodedBody, {
cspNonce: this.res.injectionNonce,
domainName: cors.getDomainNameFromUrl(this.req.proxiedUrl),
wantsInjection: this.res.wantsInjection,
wantsSecurityRemoved: this.res.wantsSecurityRemoved,
Expand Down Expand Up @@ -644,8 +613,8 @@ export default {
AttachPlainTextStreamFn,
InterceptResponse,
PatchExpressSetHeader,
OmitProblematicHeaders, // Since we might modify CSP headers, this middleware needs to come BEFORE SetInjectionLevel
SetInjectionLevel,
OmitProblematicHeaders,
MaybePreventCaching,
MaybeStripDocumentDomainFeaturePolicy,
MaybeCopyCookiesFromIncomingRes,
Expand Down
58 changes: 0 additions & 58 deletions packages/proxy/lib/http/util/csp-header.ts

This file was deleted.

19 changes: 4 additions & 15 deletions packages/proxy/lib/http/util/inject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { getRunnerInjectionContents, getRunnerCrossOriginInjectionContents } fro
import type { AutomationCookie } from '@packages/server/lib/automation/cookies'

interface InjectionOpts {
cspNonce?: string
shouldInjectDocumentDomain: boolean
}
interface FullCrossOriginOpts {
Expand All @@ -13,7 +12,6 @@ interface FullCrossOriginOpts {
}

export function partial (domain, options: InjectionOpts) {
const { cspNonce } = options
let documentDomainInjection = `document.domain = '${domain}';`

if (!options.shouldInjectDocumentDomain) {
Expand All @@ -23,17 +21,13 @@ export function partial (domain, options: InjectionOpts) {
// With useDefaultDocumentDomain=true we continue to inject an empty script tag in order to be consistent with our other forms of injection.
// This is also diagnostic in nature is it will allow us to debug easily to make sure injection is still occurring.
return oneLine`
<script type='text/javascript'${
cspNonce ? ` nonce="${cspNonce}"` : ''
}>
<script type='text/javascript'>
${documentDomainInjection}
</script>
`
}

export function full (domain, options: InjectionOpts) {
const { cspNonce } = options

return getRunnerInjectionContents().then((contents) => {
let documentDomainInjection = `document.domain = '${domain}';`

Expand All @@ -42,9 +36,7 @@ export function full (domain, options: InjectionOpts) {
}

return oneLine`
<script type='text/javascript'${
cspNonce ? ` nonce="${cspNonce}"` : ''
}>
<script type='text/javascript'>
${documentDomainInjection}
${contents}
Expand All @@ -55,7 +47,6 @@ export function full (domain, options: InjectionOpts) {

export async function fullCrossOrigin (domain, options: InjectionOpts & FullCrossOriginOpts) {
const contents = await getRunnerCrossOriginInjectionContents()
const { cspNonce, ...crossOriginOptions } = options

let documentDomainInjection = `document.domain = '${domain}';`

Expand All @@ -64,14 +55,12 @@ export async function fullCrossOrigin (domain, options: InjectionOpts & FullCros
}

return oneLine`
<script type='text/javascript'${
cspNonce ? ` nonce="${cspNonce}"` : ''
}>
<script type='text/javascript'>
${documentDomainInjection}
(function (cypressConfig) {
${contents}
}(${JSON.stringify(crossOriginOptions)}));
}(${JSON.stringify(options)}));
</script>
`
}
5 changes: 0 additions & 5 deletions packages/proxy/lib/http/util/rewriter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export type SecurityOpts = {
}

export type InjectionOpts = {
cspNonce?: string
domainName: string
wantsInjection: CypressWantsInjection
wantsSecurityRemoved: any
Expand All @@ -33,7 +32,6 @@ function getRewriter (useAstSourceRewriting: boolean) {

function getHtmlToInject (opts: InjectionOpts & SecurityOpts) {
const {
cspNonce,
domainName,
wantsInjection,
modifyObstructiveThirdPartyCode,
Expand All @@ -46,11 +44,9 @@ function getHtmlToInject (opts: InjectionOpts & SecurityOpts) {
case 'full':
return inject.full(domainName, {
shouldInjectDocumentDomain,
cspNonce,
})
case 'fullCrossOrigin':
return inject.fullCrossOrigin(domainName, {
cspNonce,
modifyObstructiveThirdPartyCode,
modifyObstructiveCode,
simulatedCookies,
Expand All @@ -59,7 +55,6 @@ function getHtmlToInject (opts: InjectionOpts & SecurityOpts) {
case 'partial':
return inject.partial(domainName, {
shouldInjectDocumentDomain,
cspNonce,
})
default:
return
Expand Down
1 change: 0 additions & 1 deletion packages/proxy/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export type CypressWantsInjection = 'full' | 'fullCrossOrigin' | 'partial' | fal
* An outgoing response to an incoming request to the Cypress web server.
*/
export type CypressOutgoingResponse = Response & {
injectionNonce?: string
isInitial: null | boolean
wantsInjection: CypressWantsInjection
wantsSecurityRemoved: null | boolean
Expand Down
109 changes: 0 additions & 109 deletions packages/proxy/test/integration/net-stubbing.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,27 +72,9 @@ context('network stubbing', () => {

destinationApp.get('/', (req, res) => res.send('it worked'))

destinationApp.get('/csp-header', (req, res) => {
const headerName = req.query.headerName

res.setHeader('content-type', 'text/html')
res.setHeader(headerName, 'fake-directive fake-value')
res.send('<foo>bar</foo>')
})

destinationApp.get('/csp-header-multiple', (req, res) => {
const headerName = req.query.headerName

res.setHeader('content-type', 'text/html')
res.setHeader(headerName, ['default \'self\'', 'script-src \'self\' localhost'])
res.send('<foo>bar</foo>')
})

server = allowDestroy(destinationApp.listen(() => {
destinationPort = server.address().port
remoteStates.set(`http://localhost:${destinationPort}`)
remoteStates.set(`http://localhost:${destinationPort}/csp-header`)
remoteStates.set(`http://localhost:${destinationPort}/csp-header-multiple`)
done()
}))
})
Expand Down Expand Up @@ -303,95 +285,4 @@ context('network stubbing', () => {
expect(sendContentLength).to.eq(receivedContentLength)
expect(sendContentLength).to.eq(realContentLength)
})

describe('CSP Headers', () => {
// Loop through valid CSP header names can verify that we handle them
[
'content-security-policy',
'Content-Security-Policy',
'content-security-policy-report-only',
'Content-Security-Policy-Report-Only',
].forEach((headerName) => {
describe(`${headerName}`, () => {
it('does not add CSP header if injecting JS and original response had no CSP header', () => {
netStubbingState.routes.push({
id: '1',
routeMatcher: {
url: '*',
},
hasInterceptor: false,
staticResponse: {
body: '<foo>bar</foo>',
},
getFixture: async () => {},
matches: 1,
})

return supertest(app)
.get(`/http://localhost:${destinationPort}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
expect(res.headers[headerName.toLowerCase()]).to.be.undefined
})
})

it('does not modify CSP header if not injecting JS and original response had CSP header', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.then((res) => {
expect(res.headers[headerName.toLowerCase()]).to.equal('fake-directive fake-value')
})
})

it('modifies CSP header if injecting JS and original response had CSP header', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName.toLowerCase()]).to.match(/^fake-directive fake-value; script-src 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/)
})
})

it('modifies CSP header if injecting JS and original response had multiple CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header-multiple?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName.toLowerCase()]).to.match(/default 'self'; script-src 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/)
expect(res.headers[headerName.toLowerCase()]).to.match(/script-src 'self' localhost 'nonce-[^-A-Za-z0-9+/=]|=[^=]|={3,}'/)
})
})

if (headerName !== headerName.toLowerCase()) {
// Do not add a non-lowercase version of a CSP header, because most-restrictive is used
it('removes non-lowercase CSP header to avoid conflicts on unmodified CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
})
})

it('removes non-lowercase CSP header to avoid conflicts on modified CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
})
})

it('removes non-lowercase CSP header to avoid conflicts on multiple CSP headers', () => {
return supertest(app)
.get(`/http://localhost:${destinationPort}/csp-header-multiple?headerName=${headerName}`)
.set('Accept', 'text/html,application/xhtml+xml')
.then((res) => {
expect(res.headers[headerName]).to.be.undefined
})
})
}
})
})
})
})
Loading

5 comments on commit 478407e

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 478407e Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.4.0/linux-arm64/develop-478407ec38b305cb5a55add1f089fbaf1920ceec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 478407e Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.4.0/linux-x64/develop-478407ec38b305cb5a55add1f089fbaf1920ceec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 478407e Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.4.0/darwin-x64/develop-478407ec38b305cb5a55add1f089fbaf1920ceec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 478407e Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.4.0/win32-x64/develop-478407ec38b305cb5a55add1f089fbaf1920ceec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on 478407e Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circle has built the darwin arm64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.4.0/darwin-arm64/develop-478407ec38b305cb5a55add1f089fbaf1920ceec/cypress.tgz

Please sign in to comment.