diff --git a/__tests__/server/plugins/reactHtml/index.spec.jsx b/__tests__/server/plugins/reactHtml/index.spec.jsx index b33e8e8b1..23c739c01 100644 --- a/__tests__/server/plugins/reactHtml/index.spec.jsx +++ b/__tests__/server/plugins/reactHtml/index.spec.jsx @@ -920,7 +920,8 @@ describe('reactHtml', () => { 'this-dep': { filename: 'this-dep.js', semanticRange: '^2.2.0', - integrity: '12345hash', + browserIntegrity: '12345hash-browser', + nodeIntegrity: '12345hash-node', version: '2.3.1', }, }, @@ -934,7 +935,7 @@ describe('reactHtml', () => { moduleMap, }) ).toMatchInlineSnapshot( - '""' + '""' ); }); @@ -944,7 +945,8 @@ describe('reactHtml', () => { 'this-dep': { filename: 'this-dep.js', semanticRange: '^2.2.0', - integrity: '12345hash', + browserIntegrity: '12345hash-browser', + nodeIntegrity: '12345hash-node', version: '2.3.1', }, }, @@ -952,7 +954,8 @@ describe('reactHtml', () => { 'this-dep': { filename: 'this-dep.js', semanticRange: '^2.2.0', - integrity: '12345hash', + browserIntegrity: '12345hash-browser', + nodeIntegrity: '12345hash-node', version: '2.3.1', }, }, @@ -968,7 +971,7 @@ describe('reactHtml', () => { moduleMap, }) ).toMatchInlineSnapshot( - '""' + '""' ); }); @@ -984,6 +987,30 @@ describe('reactHtml', () => { ); }); + it('includes integrity as "undefined" when not present', () => { + setRequiredExternalsRegistry({ + 'child-module-a': { + 'this-dep': { + filename: 'this-dep.js', + semanticRange: '^2.2.0', + version: '2.3.1', + }, + }, + }); + + const fallbackScripts = renderExternalFallbacks({ + clientInitialState, + moduleMap, + }); + expect(fallbackScripts).toMatchInlineSnapshot( + '""' + ); + + expect(console.warn).toHaveBeenCalledWith( + 'No SRI integrity hash found for script https://example.com/cdn/child-module-a/1.0.0/this-dep.browser.js. This is a security risk.' + ); + }); + it('handles trailing "/" in module baseUrl', () => { expect( renderExternalFallbacks({ @@ -998,20 +1025,7 @@ describe('reactHtml', () => { }, }) ).toMatchInlineSnapshot( - '""' - ); - }); - - it('supports legacy browsers', () => { - expect( - renderExternalFallbacks({ - clientInitialState, - moduleMap, - isDevelopmentEnv: true, - isLegacy: true, - }) - ).toMatchInlineSnapshot( - '""' + '""' ); }); }); @@ -1029,7 +1043,10 @@ describe('reactHtml', () => { sendHtml(request, reply); expect(request.log.error).toHaveBeenCalledTimes(1); - expect(request.log.error).toHaveBeenCalledWith('encountered an error serializing full client initial state', fullStateError); + expect(request.log.error).toHaveBeenCalledWith( + 'encountered an error serializing full client initial state', + fullStateError + ); expect(transit.toJSON).toHaveBeenCalledTimes(3); expect(reply.send).toHaveBeenCalledTimes(1); @@ -1061,8 +1078,14 @@ describe('reactHtml', () => { sendHtml(request, reply); expect(request.log.error).toHaveBeenCalledTimes(3); - expect(request.log.error).toHaveBeenCalledWith('encountered an error serializing full client initial state', fullStateError); - expect(request.log.error).toHaveBeenCalledWith('unable to build the most basic initial state for a client to startup', minimalStateError); + expect(request.log.error).toHaveBeenCalledWith( + 'encountered an error serializing full client initial state', + fullStateError + ); + expect(request.log.error).toHaveBeenCalledWith( + 'unable to build the most basic initial state for a client to startup', + minimalStateError + ); expect(transit.toJSON).toHaveBeenCalledTimes(4); expect(reply.send).toHaveBeenCalledTimes(1); @@ -1115,7 +1138,9 @@ describe('reactHtml', () => { jest.spyOn(console, 'error'); isRedirectUrlAllowed.mockImplementationOnce(() => false); checkStateForRedirectAndStatusCode(req, reply); - expect(util.format(...console.error.mock.calls[0])).toBe(`'${destination}' is not an allowed redirect URL`); + expect(util.format(...console.error.mock.calls[0])).toBe( + `'${destination}' is not an allowed redirect URL` + ); }); }); diff --git a/package-lock.json b/package-lock.json index bf2678f54..c6be40c35 100644 --- a/package-lock.json +++ b/package-lock.json @@ -41,7 +41,7 @@ "fastify-plugin": "^4.5.1", "flat": "^5.0.2", "helmet": "^7.0.0", - "holocron": "^1.8.2", + "holocron": "^1.9.1", "holocron-module-route": "^1.7.0", "immutable": "^4.1.0", "ip": "^1.1.8", @@ -14911,9 +14911,9 @@ } }, "node_modules/holocron": { - "version": "1.8.2", - "resolved": "https://registry.npmjs.org/holocron/-/holocron-1.8.2.tgz", - "integrity": "sha512-EQemR02NsNrRER1G0Hf6FUyd5RBKkMEKA/vEYn0JlSNKmVetkchEb4NlRF4SuqMq4po/DcHQdl0pfgLBw2u8nQ==", + "version": "1.9.1", + "resolved": "https://registry.npmjs.org/holocron/-/holocron-1.9.1.tgz", + "integrity": "sha512-SyjQ/qxTl/wWde24SPfDAR3/OYchNIdDK96gH6q362IHzUZznnfVHoEGoF/otJtoETUskG4ydjmRzV4Y9+sffg==", "dependencies": { "@americanexpress/vitruvius": "^2.0.0", "hoist-non-react-statics": "^3.3.0", diff --git a/package.json b/package.json index bad583e2c..f589adad3 100644 --- a/package.json +++ b/package.json @@ -112,7 +112,7 @@ "fastify-plugin": "^4.5.1", "flat": "^5.0.2", "helmet": "^7.0.0", - "holocron": "^1.8.2", + "holocron": "^1.9.1", "holocron-module-route": "^1.7.0", "immutable": "^4.1.0", "ip": "^1.1.8", diff --git a/src/server/plugins/reactHtml/index.jsx b/src/server/plugins/reactHtml/index.jsx index c4341fc14..b56e4b55a 100644 --- a/src/server/plugins/reactHtml/index.jsx +++ b/src/server/plugins/reactHtml/index.jsx @@ -76,6 +76,10 @@ function renderI18nScript(clientInitialState, appBundlesURLPrefix) { function renderScript({ src, integrity, isDevelopmentEnv, clientCacheRevision, }) { + if (!integrity && !isDevelopmentEnv) console.warn(`No SRI integrity hash found for script ${src}. This is a security risk.`); + // TODO: consider throwing an error in next major version. This is a breaking change. + // currently we rely on "undefined" to throw integrity error in the browser, this is + // results in poor DX, hard to find bugs. const additionalAttributes = isDevelopmentEnv ? '' : `integrity="${integrity}"`; const scriptSource = isDevelopmentEnv || !clientCacheRevision ? src @@ -110,7 +114,6 @@ export function renderExternalFallbacks({ clientInitialState, moduleMap, isDevelopmentEnv, - isLegacy, }) { const loadedModules = clientInitialState.getIn(['holocron', 'loaded'], iSet()).toArray(); const requiredFallbacks = loadedModules @@ -132,14 +135,13 @@ export function renderExternalFallbacks({ const { clientCacheRevision, modules } = moduleMap; return requiredFallbacks - .map(({ name, integrity, moduleName }) => { + .map(({ name, browserIntegrity, moduleName }) => { const { baseUrl } = modules[moduleName]; const endsWithSlash = baseUrl.endsWith('/'); const src = [ baseUrl, [ name, - isLegacy ? 'legacy' : '', 'browser', 'js', ].filter(Boolean).join('.'), @@ -147,7 +149,7 @@ export function renderExternalFallbacks({ return renderScript({ src, - integrity, + integrity: browserIntegrity, isDevelopmentEnv, clientCacheRevision, });