Merge pull request #1530 from SalesforceCommerceCloud/wjh/v2-fix-csp

@W-14446652@ v2 Change CSP enforcement to an opt-in middleware.

packages/pwa-kit-runtime/CHANGELOG.md

@@ -1,43 +1,62 @@
+## v2.8.1 (Nov 08, 2023)
+
+- Revert mandatory enforcement of Content-Security-Policy headers. Provide middleware as an opt-in replacement. [#1530](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1530)
+
+```js
+// your-project/app/ssr.js
+import {defaultPwaKitSecurityHeaders} from '@salesforce/pwa-kit-runtime/utils/middleware'
+const {handler} = runtime.createHandler(options, (app) => {
+    app.use(defaultPwaKitSecurityHeaders)
+    // ...
+}
+```
+
 ## v2.8.0 (Nov 03, 2023)
--   Move Content-Security-Policy logic into pwa-kit-runtime [#1491](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1491)
+
+- Move Content-Security-Policy logic into pwa-kit-runtime [#1491](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1491)
+
 ## v2.7.4 (Aug 28, 2023)
+
 ## v2.7.3 (Jun 20, 2023)
--   Support Node 18 and NPM 9. [#1265](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1265)
+
+- Support Node 18 and NPM 9. [#1265](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1265)
+
 ## v2.7.2 (May 29, 2023)
+
 ## v2.7.1 (May 11, 2023)
 
--   Add optional parameter to override configuration folder used in `getConfig` [#1049](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1049)
--   Moved the MRT reference app to the SDKs, so that we can verify eg. Node support [#966](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/966)
+- Add optional parameter to override configuration folder used in `getConfig` [#1049](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/1049)
+- Moved the MRT reference app to the SDKs, so that we can verify eg. Node support [#966](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/966)
 
 ## v2.7.0 (Mar 03, 2023)
 
--   Support Node 16 [#965](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/965)
+- Support Node 16 [#965](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/965)
 
 ## v2.6.0 (Jan 25, 2023)
 
--   Security package updates
+- Security package updates
 
 ## v2.5.0 (Jan 05, 2023)
 
--   Logging cid from res header isntead of req in local development [#821](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/821)
--   Replace morgan stream to use console.log [#847](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/847)
+- Logging cid from res header isntead of req in local development [#821](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/821)
+- Replace morgan stream to use console.log [#847](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/847)
 
 ## v2.4.0 (Dec 01, 2022)
 
 ## v2.3.0 (Oct 27, 2022)
 
--   Performance: Skip retries when flushing CloudWatch metrics, prioritize returning a response instead. [720](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/720)
--   Add Correlation ID to SCAPI requests. [#728](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/728)
+- Performance: Skip retries when flushing CloudWatch metrics, prioritize returning a response instead. [720](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/720)
+- Add Correlation ID to SCAPI requests. [#728](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/728)
 
 ## v2.2.0 (Aug 25, 2022)
 
 ## v2.1.0 (Jul 05, 2022)
 
 ## v2.0.0 (May 16, 2022)
 
--   Drop node 12 support for [#589](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/589)
--   Improve test coverage [#550](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/550)
--   Make the createApp API idiomatic for Express, fix service-worker loading. [#536](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/536)
--   Add environment specific configuration support via `getConfig`. [#447](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/447)
--   Remove legacy remote proxy, which allowed remote environments to use proxy configs in package.json [#425](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/425)
--   Remove default `body-parser` middleware from express server. [#444](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/444)
+- Drop node 12 support for [#589](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/589)
+- Improve test coverage [#550](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/550)
+- Make the createApp API idiomatic for Express, fix service-worker loading. [#536](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/536)
+- Add environment specific configuration support via `getConfig`. [#447](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/447)
+- Remove legacy remote proxy, which allowed remote environments to use proxy configs in package.json [#425](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/425)
+- Remove default `body-parser` middleware from express server. [#444](https://github.com/SalesforceCommerceCloud/pwa-kit/pull/444)

packages/pwa-kit-runtime/src/ssr/server/build-remote-server.js

@@ -11,9 +11,7 @@ import {
     X_MOBIFY_QUERYSTRING,
     SET_COOKIE,
     CACHE_CONTROL,
-    NO_CACHE,
-    CONTENT_SECURITY_POLICY,
-    STRICT_TRANSPORT_SECURITY
+    NO_CACHE
 } from './constants'
 import {
     catchAndLog,
@@ -598,7 +596,6 @@ export const RemoteServerFactory = {
 
         // Apply the SSR middleware to any subsequent routes that we expect users
         // to add in their projects, like in any regular Express app.
-        app.use(enforceSecurityHeaders) // Must be AFTER prepNonProxyRequest, as they both modify setHeader.
         app.use(ssrMiddleware)
         app.use(errorHandlerMiddleware)
 
@@ -919,105 +916,6 @@ export const RemoteServerFactory = {
     }
 }
 
-/**
- * Patches `res.setHeader` to ensure that the Content-Security-Policy header always includes the
- * directives required for PWA Kit to work.
- * @param {express.Request} req Express request object
- * @param {express.Response} res Express response object
- * @param {express.NextFunction} next Express next callback
- */
-export const enforceSecurityHeaders = (req, res, next) => {
-    /** CSP-compatible origin for Runtime Admin. */
-    // localhost doesn't include a protocol because different browsers behave differently :\
-    const runtimeAdmin = isRemote() ? 'https://runtime.commercecloud.com' : 'localhost:*'
-    /**
-     * Map of directive names/values that are required for PWA Kit to work. Array values will be
-     * merged with user-provided values; boolean values will replace user-provided values.
-     * @type Object.<string, string[] | boolean>
-     */
-    const directives = {
-        'connect-src': ["'self'", runtimeAdmin],
-        'frame-ancestors': [runtimeAdmin],
-        'img-src': ["'self'", 'data:'],
-        'script-src': ["'self'", "'unsafe-eval'", runtimeAdmin],
-        // Always upgrade insecure requests when deployed, never upgrade on local dev server
-        'upgrade-insecure-requests': isRemote()
-    }
-
-    const setHeader = res.setHeader
-    res.setHeader = (name, value) => {
-        let modifiedValue = value
-        switch (name?.toLowerCase()) {
-            case CONTENT_SECURITY_POLICY: {
-                // If multiple Content-Security-Policy headers are provided, then the most restrictive
-                // option is chosen for each directive. Therefore, we must modify *all* directives to
-                // ensure that our required directives will work as expected.
-                // Ref: https://w3c.github.io/webappsec-csp/#multiple-policies
-                modifiedValue = Array.isArray(value)
-                    ? value.map((item) => modifyDirectives(item, directives))
-                    : modifyDirectives(value, directives)
-                break
-            }
-            case STRICT_TRANSPORT_SECURITY: {
-                // Block setting this header on local development server - it will break things!
-                if (!isRemote()) return
-                break
-            }
-            default: {
-                break
-            }
-        }
-        return setHeader.call(res, name, modifiedValue)
-    }
-    // Provide an initial CSP (or patch the existing header)
-    res.setHeader(CONTENT_SECURITY_POLICY, res.getHeader(CONTENT_SECURITY_POLICY) ?? '')
-    // Provide an initial value for HSTS, if not already set - use default from `helmet`
-    if (!res.hasHeader(STRICT_TRANSPORT_SECURITY)) {
-        res.setHeader(STRICT_TRANSPORT_SECURITY, 'max-age=15552000; includeSubDomains')
-    }
-    next()
-}
-
-/**
- * Updates the given Content-Security-Policy header to include all directives required by PWA Kit.
- * @param {string} original Original Content-Security-Policy header
- * @returns {string} Modified Content-Security-Policy header
- * @private
- */
-const modifyDirectives = (original, required) => {
-    const directives = original
-        .trim()
-        .split(';')
-        .reduce((acc, directive) => {
-            const text = directive.trim()
-            if (text) {
-                const [name, ...values] = text.split(/ +/)
-                acc[name] = values
-            }
-            return acc
-        }, {})
-
-    // Add missing required CSP directives
-    for (const [name, value] of Object.entries(required)) {
-        if (value === true) {
-            // Boolean directive (required) - overwrite original value
-            directives[name] = []
-        } else if (value === false) {
-            // Boolean directive (disabled) - delete original value
-            delete directives[name]
-        } else {
-            // Regular string[] directive - merge values
-            // Wrapping with `[...new Set(array)]` removes duplicate entries
-            directives[name] = [...new Set([...(directives[name] ?? []), ...value])]
-        }
-    }
-
-    // Re-construct header string
-    return Object.entries(directives)
-        .map(([name, values]) => [name, ...values].join(' '))
-        .join(';')
-}
-
 /**
  * ExpressJS middleware that processes any non-proxy request passing
  * through the Express app.

packages/pwa-kit-runtime/src/ssr/server/build-remote-server.test.js

@@ -4,8 +4,7 @@
  * SPDX-License-Identifier: BSD-3-Clause
  * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
-import {enforceSecurityHeaders, once} from './build-remote-server'
-import {CONTENT_SECURITY_POLICY as CSP, STRICT_TRANSPORT_SECURITY} from './constants'
+import {once} from './build-remote-server'
 
 describe('the once function', () => {
     test('should prevent a function being called more than once', () => {
@@ -19,114 +18,3 @@ describe('the once function', () => {
         expect(v1).toBe(v2) // The exact same instance
     })
 })
-
-describe('Content-Security-Policy enforcement', () => {
-    let res
-
-    /** Sets the correct values for `isRemote()` to return true */
-    const mockProduction = () => {
-        process.env.AWS_LAMBDA_FUNCTION_NAME = 'testEnforceSecurityHeaders'
-    }
-    /**
-     * Helper to make expected CSP more readable. Asserts that the actual CSP header contains each
-     * of the expected directives.
-     * @param {string[]} expected Array of expected CSP directives
-     */
-    const expectDirectives = (expected) => {
-        const actual = res.getHeader(CSP).split(';')
-        expect(actual).toEqual(expect.arrayContaining(expected))
-    }
-
-    beforeEach(() => {
-        const headers = {}
-        res = {
-            hasHeader: (key) => Object.prototype.hasOwnProperty.call(headers, key),
-            getHeader: (key) => headers[key],
-            setHeader: (key, val) => (headers[key] = val)
-        }
-    })
-    // Revert state detected by `isRemote()`
-    afterEach(() => delete process.env.AWS_LAMBDA_FUNCTION_NAME)
-
-    test('adds required directives for development', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, '')
-        expectDirectives([
-            "connect-src 'self' localhost:*",
-            'frame-ancestors localhost:*',
-            "img-src 'self' data:",
-            "script-src 'self' 'unsafe-eval' localhost:*"
-        ])
-    })
-    test('adds required directives for production', () => {
-        mockProduction()
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, '')
-        expectDirectives([
-            "connect-src 'self' https://runtime.commercecloud.com",
-            'frame-ancestors https://runtime.commercecloud.com',
-            "img-src 'self' data:",
-            "script-src 'self' 'unsafe-eval' https://runtime.commercecloud.com",
-            'upgrade-insecure-requests'
-        ])
-    })
-    test('merges with existing CSP directives', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, "connect-src test:* ; script-src 'unsafe-eval' test:*")
-        expectDirectives([
-            "connect-src test:* 'self' localhost:*",
-            "script-src 'unsafe-eval' test:* 'self' localhost:*"
-        ])
-    })
-    test('allows other CSP directives', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, 'fake-directive test:*')
-        expectDirectives(['fake-directive test:*'])
-    })
-    test('enforces upgrade-insecure-requests disabled on development', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, 'upgrade-insecure-requests')
-        expect(res.getHeader(CSP)).not.toContain('upgrade-insecure-requests')
-    })
-    test('enforces upgrade-insecure-requests enabled on production', () => {
-        mockProduction()
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, 'connect-src localhost:*')
-        expectDirectives(['upgrade-insecure-requests'])
-    })
-    test('adds directives even if setHeader is never called', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        expectDirectives(["img-src 'self' data:"])
-    })
-    test('handles multiple CSP headers', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(CSP, ['connect-src first.header', 'script-src second.header'])
-        const headers = res.getHeader(CSP)
-        expect(headers).toHaveLength(2)
-        expect(headers[0]).toContain('connect-src first.header')
-        expect(headers[1]).toContain('script-src second.header')
-    })
-    test('does not modify unrelated headers', () => {
-        const header = 'Contentious-Secret-Police'
-        const value = 'connect-src unmodified fake directive'
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(header, value)
-        expect(res.getHeader(header)).toBe(value)
-    })
-    test('blocks Strict-Transport-Security header in development', () => {
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(STRICT_TRANSPORT_SECURITY, 'max-age=12345')
-        expect(res.hasHeader(STRICT_TRANSPORT_SECURITY)).toBe(false)
-    })
-    test('allows Strict-Transport-Security header in production', () => {
-        mockProduction()
-        enforceSecurityHeaders({}, res, () => {})
-        res.setHeader(STRICT_TRANSPORT_SECURITY, 'max-age=12345')
-        expect(res.getHeader(STRICT_TRANSPORT_SECURITY)).toBe('max-age=12345')
-    })
-    test('provides default value for Strict-Transport-Security header in production', () => {
-        mockProduction()
-        enforceSecurityHeaders({}, res, () => {})
-        expect(res.getHeader(STRICT_TRANSPORT_SECURITY)).toBe('max-age=15552000; includeSubDomains')
-    })
-})

packages/pwa-kit-runtime/src/utils/middleware/index.js

@@ -0,0 +1,7 @@
+/*
+ * Copyright (c) 2023, Salesforce, Inc.
+ * All rights reserved.
+ * SPDX-License-Identifier: BSD-3-Clause
+ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause
+ */
+export * from './security'