fix(config): preserve full URLs for inferred CSP header values (#1636)

* fix(config): preserve full URLs for inferred CSP header values * docs: changeset * refactor(config): handle environment variables with empty values * refactor(config): omit empty fields from additional env variables * docs: update changelog
commercetools · Jul 19, 2020 · 96b3af7 · 96b3af7 · vercel · Jul 19, 2020
1 parent 1c7bac8
commit 96b3af7
Show file tree

Hide file tree

Showing 11 changed files with 86 additions and 56 deletions.
diff --git a/.changeset/lovely-pianos-thank.md b/.changeset/lovely-pianos-thank.md
@@ -0,0 +1,9 @@
+---
+'@commercetools-frontend/application-config': patch
+'@commercetools-frontend/mc-html-template': patch
+'playground': patch
+'@commercetools-website/custom-applications': patch
+---
+
+Fix parsing of application config to preserve full URLs when inferring CSP directives.
+Furthermore, every environment variable referenced within the application config that has an empty value will be parsed as-is and it will not be rejected. Additionally, the fields passed to the `additionalEnv` object that are empty will be removed from the resulting environment and `window.app`.
diff --git a/packages/application-config/package.json b/packages/application-config/package.json
@@ -47,7 +47,8 @@
     "ajv": "6.12.3",
     "core-js": "3.6.5",
     "cosmiconfig": "6.0.0",
-    "lodash": "4.17.19"
+    "lodash": "4.17.19",
+    "omit-empty-es": "1.0.3"
   },
   "devDependencies": {
     "@babel/plugin-transform-runtime": "7.10.4",

diff --git a/packages/application-config/src/process-config.ts b/packages/application-config/src/process-config.ts
@@ -4,6 +4,7 @@ import type { DeprecatedOptions } from './load-deprecated-config';
 
 // Loads the configuration file and parse the environment and header values.
 // Most of the resulting values are inferred from the config.
+import omitEmpty from 'omit-empty-es';
 import loadConfig from './load-config';
 import loadDeprecatedConfig from './load-deprecated-config';
 import validateConfig from './validate-config';
@@ -73,8 +74,8 @@ const processConfig = ({
     `Invalid application URL: "${appEnvConfig.url}"`
   );
   const cdnUrl = getOrThrow(
-    () => new URL(appEnvConfig.cdnUrl || appUrl.origin),
-    `Invalid application CDN URL: "${appEnvConfig.cdnUrl || appUrl.origin}"`
+    () => new URL(appEnvConfig.cdnUrl || appUrl.href),
+    `Invalid application CDN URL: "${appEnvConfig.cdnUrl || appUrl.href}"`
   );
   const mcApiUrl = getOrThrow(
     () =>
@@ -89,10 +90,10 @@ const processConfig = ({
 
   cachedConfig = {
     env: {
-      ...additionalAppEnv,
+      ...omitEmpty(additionalAppEnv),
       applicationId: appConfig.id,
       applicationName: appConfig.name,
-      cdnUrl: cdnUrl.origin,
+      cdnUrl: cdnUrl.href,
       env: appEnvKey,
       frontendHost: appUrl.host,
       location: appConfig.cloudIdentifier,
@@ -106,15 +107,15 @@ const processConfig = ({
         ...appConfig.headers?.csp,
         'connect-src': getUniqueValues(
           appConfig.headers?.csp['connect-src'],
-          [mcApiUrl.hostname].concat(isProd ? appUrl.hostname : [])
+          [mcApiUrl.origin].concat(isProd ? [appUrl.href] : [])
         ),
         'script-src': getUniqueValues(
           appConfig.headers?.csp['script-src'],
-          isProd ? [appUrl.hostname] : []
+          isProd ? [appUrl.href, cdnUrl.href] : []
         ),
         'style-src': getUniqueValues(
           appConfig.headers?.csp['style-src'],
-          isProd ? [appUrl.hostname] : []
+          isProd ? [appUrl.href, cdnUrl.href] : []
         ),
       },
     },

diff --git a/packages/application-config/src/substitute-env-variable-placeholders.ts b/packages/application-config/src/substitute-env-variable-placeholders.ts
@@ -25,9 +25,9 @@ const substituteEnvVariablePlaceholder = (
   meta: { processEnv: NodeJS.ProcessEnv } = { processEnv: process.env }
 ) => {
   const [, requestedEnvVar] = valueOfPlaceholder.split(':');
-  const valueOfEnv = meta.processEnv[requestedEnvVar];
+  const hasEnvField = meta.processEnv.hasOwnProperty(requestedEnvVar);
 
-  if (!valueOfEnv) {
+  if (!hasEnvField) {
     throw new Error(
       `Missing '${requestedEnvVar}' specified in config as 'env:${requestedEnvVar}'.`
     );
@@ -36,7 +36,7 @@ const substituteEnvVariablePlaceholder = (
   const escapedMatchedString = matchedString.replace(/[${}:]/g, '\\$&');
   return valueOfEnvConfig.replace(
     new RegExp(`(${escapedMatchedString})+`, 'g'),
-    valueOfEnv
+    meta.processEnv[requestedEnvVar] as string
   );
 };
 

diff --git a/packages/application-config/test/fixtures/config-full.json b/packages/application-config/test/fixtures/config-full.json
@@ -9,18 +9,16 @@
     }
   },
   "additionalEnv": {
-    "numberOfMovies": 4
+    "numberOfMovies": 4,
+    "empty": ""
   },
   "headers": {
     "csp": {
       "script-src": [
-        "cdn.avengers.app"
+        "https://track.avengers.app"
       ],
       "connect-src": [
-        "cdn.avengers.app"
-      ],
-      "style-src": [
-        "cdn.avengers.app"
+        "https://track.avengers.app"
       ]
     },
     "featurePolicies": {

diff --git a/packages/application-config/test/process-config.spec.js b/packages/application-config/test/process-config.spec.js
@@ -33,7 +33,7 @@ describe('processing a simple config', () => {
       env: {
         applicationId: undefined,
         applicationName: 'avengers-app',
-        cdnUrl: 'http://localhost:3001',
+        cdnUrl: 'http://localhost:3001/',
         env: 'test',
         frontendHost: 'localhost:3001',
         location: 'gcp-eu',
@@ -43,7 +43,7 @@ describe('processing a simple config', () => {
       },
       headers: {
         csp: {
-          'connect-src': ['mc-api.europe-west1.gcp.commercetools.com'],
+          'connect-src': ['https://mc-api.europe-west1.gcp.commercetools.com'],
           'script-src': [],
           'style-src': [],
         },
@@ -63,7 +63,7 @@ describe('processing a simple config', () => {
         env: {
           applicationId: undefined,
           applicationName: 'avengers-app',
-          cdnUrl: 'https://avengers.app',
+          cdnUrl: 'https://avengers.app/',
           env: 'production',
           frontendHost: 'avengers.app',
           location: 'gcp-eu',
@@ -74,11 +74,11 @@ describe('processing a simple config', () => {
         headers: {
           csp: {
             'connect-src': [
-              'mc-api.europe-west1.gcp.commercetools.com',
-              'avengers.app',
+              'https://mc-api.europe-west1.gcp.commercetools.com',
+              'https://avengers.app/',
             ],
-            'script-src': ['avengers.app'],
-            'style-src': ['avengers.app'],
+            'script-src': ['https://avengers.app/'],
+            'style-src': ['https://avengers.app/'],
           },
         },
       });
@@ -98,7 +98,7 @@ describe('processing a simple config', () => {
         env: {
           applicationId: undefined,
           applicationName: 'avengers-app',
-          cdnUrl: 'http://localhost:3001',
+          cdnUrl: 'http://localhost:3001/',
           env: 'development',
           frontendHost: 'localhost:3001',
           location: 'gcp-eu',
@@ -108,7 +108,9 @@ describe('processing a simple config', () => {
         },
         headers: {
           csp: {
-            'connect-src': ['mc-api.europe-west1.gcp.commercetools.com'],
+            'connect-src': [
+              'https://mc-api.europe-west1.gcp.commercetools.com',
+            ],
             'script-src': [],
             'style-src': [],
           },
@@ -128,7 +130,7 @@ describe('processing a full config', () => {
       env: {
         applicationId: undefined,
         applicationName: 'avengers-app',
-        cdnUrl: 'http://localhost:3001',
+        cdnUrl: 'http://localhost:3001/',
         env: 'test',
         frontendHost: 'localhost:3001',
         location: 'gcp-eu',
@@ -140,11 +142,11 @@ describe('processing a full config', () => {
       headers: {
         csp: {
           'connect-src': [
-            'cdn.avengers.app',
-            'mc-api.europe-west1.gcp.commercetools.com',
+            'https://track.avengers.app',
+            'https://mc-api.europe-west1.gcp.commercetools.com',
           ],
-          'script-src': ['cdn.avengers.app'],
-          'style-src': ['cdn.avengers.app'],
+          'script-src': ['https://track.avengers.app'],
+          'style-src': [],
         },
         featurePolicies: {
           microphone: 'none',
@@ -165,7 +167,7 @@ describe('processing a full config', () => {
         env: {
           applicationId: undefined,
           applicationName: 'avengers-app',
-          cdnUrl: 'https://cdn.avengers.app',
+          cdnUrl: 'https://cdn.avengers.app/',
           env: 'production',
           frontendHost: 'avengers.app',
           location: 'gcp-eu',
@@ -177,12 +179,16 @@ describe('processing a full config', () => {
         headers: {
           csp: {
             'connect-src': [
-              'cdn.avengers.app',
-              'mc-api.europe-west1.gcp.commercetools.com',
-              'avengers.app',
+              'https://track.avengers.app',
+              'https://mc-api.europe-west1.gcp.commercetools.com',
+              'https://avengers.app/',
             ],
-            'script-src': ['cdn.avengers.app', 'avengers.app'],
-            'style-src': ['cdn.avengers.app', 'avengers.app'],
+            'script-src': [
+              'https://track.avengers.app',
+              'https://avengers.app/',
+              'https://cdn.avengers.app/',
+            ],
+            'style-src': ['https://avengers.app/', 'https://cdn.avengers.app/'],
           },
           featurePolicies: {
             microphone: 'none',
@@ -205,7 +211,7 @@ describe('processing a full config', () => {
         env: {
           applicationId: undefined,
           applicationName: 'avengers-app',
-          cdnUrl: 'http://localhost:3001',
+          cdnUrl: 'http://localhost:3001/',
           env: 'development',
           frontendHost: 'localhost:3001',
           location: 'gcp-eu',
@@ -217,11 +223,11 @@ describe('processing a full config', () => {
         headers: {
           csp: {
             'connect-src': [
-              'cdn.avengers.app',
-              'mc-api.europe-west1.gcp.commercetools.com',
+              'https://track.avengers.app',
+              'https://mc-api.europe-west1.gcp.commercetools.com',
             ],
-            'script-src': ['cdn.avengers.app'],
-            'style-src': ['cdn.avengers.app'],
+            'script-src': ['https://track.avengers.app'],
+            'style-src': [],
           },
           featurePolicies: {
             microphone: 'none',
@@ -250,7 +256,7 @@ describe('processing a config with environment variable placeholders', () => {
       env: {
         applicationId: undefined,
         applicationName: 'avengers-app',
-        cdnUrl: 'http://localhost:3001',
+        cdnUrl: 'http://localhost:3001/',
         env: 'test',
         frontendHost: 'localhost:3001',
         location: 'gcp-eu',
@@ -260,7 +266,7 @@ describe('processing a config with environment variable placeholders', () => {
       },
       headers: {
         csp: {
-          'connect-src': ['mc-api.europe-west1.gcp.commercetools.com'],
+          'connect-src': ['https://mc-api.europe-west1.gcp.commercetools.com'],
           'script-src': [],
           'style-src': [],
         },
@@ -282,7 +288,7 @@ describe('processing a config with environment variable placeholders', () => {
         env: {
           applicationId: undefined,
           applicationName: 'avengers-app',
-          cdnUrl: 'https://avengers.app',
+          cdnUrl: 'https://avengers.app/',
           env: 'production',
           frontendHost: 'avengers.app',
           location: 'gcp-eu',
@@ -293,11 +299,11 @@ describe('processing a config with environment variable placeholders', () => {
         headers: {
           csp: {
             'connect-src': [
-              'mc-api.europe-west1.gcp.commercetools.com',
-              'avengers.app',
+              'https://mc-api.europe-west1.gcp.commercetools.com',
+              'https://avengers.app/',
             ],
-            'script-src': ['avengers.app'],
-            'style-src': ['avengers.app'],
+            'script-src': ['https://avengers.app/'],
+            'style-src': ['https://avengers.app/'],
           },
         },
       });
@@ -319,7 +325,7 @@ describe('processing a config with environment variable placeholders', () => {
         env: {
           applicationId: undefined,
           applicationName: 'avengers-app',
-          cdnUrl: 'http://localhost:3001',
+          cdnUrl: 'http://localhost:3001/',
           env: 'development',
           frontendHost: 'localhost:3001',
           location: 'gcp-eu',
@@ -329,7 +335,9 @@ describe('processing a config with environment variable placeholders', () => {
         },
         headers: {
           csp: {
-            'connect-src': ['mc-api.europe-west1.gcp.commercetools.com'],
+            'connect-src': [
+              'https://mc-api.europe-west1.gcp.commercetools.com',
+            ],
             'script-src': [],
             'style-src': [],
           },

diff --git a/packages/application-config/test/substitute-env-variable-placeholders.spec.js b/packages/application-config/test/substitute-env-variable-placeholders.spec.js
@@ -8,6 +8,7 @@ describe('providing a config with env variable placeholders', () => {
       CDN_REGION: 'eu',
       CDN_ID: 'cdn-id',
       STATUS: '200',
+      EMPTY: '',
     };
     const result = substituteEnvVariablePlaceholders({
       title: 'The title is ${env:TITLE}',
@@ -26,6 +27,7 @@ describe('providing a config with env variable placeholders', () => {
           },
         },
       },
+      empty: '${env:EMPTY}',
     });
     expect(result).toEqual({
       title: 'The title is Awesome',
@@ -44,6 +46,7 @@ describe('providing a config with env variable placeholders', () => {
           },
         },
       },
+      empty: '',
     });
   });
 });

diff --git a/packages/mc-html-template/src/compile-html.js b/packages/mc-html-template/src/compile-html.js
@@ -8,6 +8,8 @@ const replaceHtmlPlaceholders = require('./utils/replace-html-placeholders');
 const requiredOptions = ['publicAssetsPath'];
 const deprecatedOptions = ['envPath', 'cspPath', 'headersPath'];
 
+const trimTrailingSlash = (value) => value.replace(/\/$/, '');
+
 /**
  * Options:
  * - envPath (deprecated)
@@ -62,7 +64,9 @@ module.exports = async function compileHtml(options) {
   const randomQueryParam = Date.now();
   // Fetch `index.html.template` from remote CDN
   const remoteIndexHtmlResponse = await fetch(
-    `${applicationConfig.env.cdnUrl}/index.html.template?${randomQueryParam}`
+    `${trimTrailingSlash(
+      applicationConfig.env.cdnUrl
+    )}/index.html.template?${randomQueryParam}`
   );
   if (!remoteIndexHtmlResponse.ok) {
     const rawResponseError = await remoteIndexHtmlResponse.text();

diff --git a/packages/mc-html-template/src/utils/replace-html-placeholders.js b/packages/mc-html-template/src/utils/replace-html-placeholders.js
@@ -2,6 +2,8 @@ const htmlScripts = require('../load-html-scripts');
 const htmlStyles = require('../load-html-styles');
 const sanitizeAppEnvironment = require('./sanitize-app-environment');
 
+const trimTrailingSlash = (value) => value.replace(/\/$/, '');
+
 const getGtmTrackingScript = (gtmId) => {
   if (!gtmId) return '';
   const url = `https://www.googletagmanager.com/gtm.js?id=${gtmId}`;
@@ -16,10 +18,13 @@ const replaceHtmlPlaceholders = (indexHtmlContent, config) =>
       new RegExp('__CDN_URL__', 'g'),
       config.cdnUrl
         ? // Ensure there is a trailing slash
-          `${config.cdnUrl.replace(/\/$/, '')}/`
+          `${trimTrailingSlash(config.cdnUrl)}/`
         : ''
     )
-    .replace(new RegExp('__MC_API_URL__', 'g'), config.mcApiUrl)
+    .replace(
+      new RegExp('__MC_API_URL__', 'g'),
+      trimTrailingSlash(config.mcApiUrl)
+    )
     .replace(
       new RegExp('__APP_ENVIRONMENT__', 'g'),
       sanitizeAppEnvironment(config)