Block Certain Env Keys That Are Used Internally (vercel#6260)

Closes: vercel#6244 This will block the following keys: ``` NODE_.+ __.+ ``` There doesn't seem to be a way to simulate a failed build or else I'd add tests for it.
filoblu · Feb 15, 2019 · 1e5d090 · 1e5d090
1 parent d2ef344
commit 1e5d090
Show file tree

Hide file tree

Showing 12 changed files with 83 additions and 23 deletions.
diff --git a/errors/env-key-not-allowed.md b/errors/env-key-not-allowed.md
@@ -0,0 +1,9 @@
+# The key "<your key>" under "env" in next.config.js is not allowed.
+
+#### Why This Error Occurred
+
+Next.js configures internal variables for replacement itself. These start with `__` or `NODE_`, for this reason they are not allowed as values for `env` in `next.config.js`
+
+#### Possible Ways to Fix It
+
+Rename the specified key so that it does not start with `__` or `NODE_`.
diff --git a/packages/next-server/server/config.js b/packages/next-server/server/config.js
@@ -4,6 +4,7 @@ import {CONFIG_FILE} from 'next-server/constants'
 const targets = ['server', 'serverless']
 
 const defaultConfig = {
+  env: [],
   webpack: null,
   webpackDevMiddleware: null,
   poweredByHeader: true,

diff --git a/packages/next-server/server/next-server.ts b/packages/next-server/server/next-server.ts
@@ -249,7 +249,7 @@ export default class Server {
     }
 
     if (this.nextConfig.poweredByHeader) {
-      res.setHeader('X-Powered-By', 'Next.js ' + process.env.NEXT_VERSION)
+      res.setHeader('X-Powered-By', 'Next.js ' + process.env.__NEXT_VERSION)
     }
     return this.sendHTML(req, res, html)
   }

diff --git a/packages/next-server/taskfile-typescript.js b/packages/next-server/taskfile-typescript.js
@@ -35,7 +35,7 @@ try {
       }
 
       // update file's data
-      file.data = Buffer.from(result.outputText.replace(/process\.env\.NEXT_VERSION/, `"${require('./package.json').version}"`), 'utf8')
+      file.data = Buffer.from(result.outputText.replace(/process\.env\.__NEXT_VERSION/, `"${require('./package.json').version}"`), 'utf8')
     })
   }
 } catch (err) {

diff --git a/packages/next/bin/next.ts b/packages/next/bin/next.ts
@@ -37,7 +37,7 @@ const args = arg({
 // Version is inlined into the file using taskr build pipeline
 if (args['--version']) {
   // tslint:disable-next-line
-  console.log(`Next.js v${process.env.NEXT_VERSION}`)
+  console.log(`Next.js v${process.env.__NEXT_VERSION}`)
   process.exit(0)
 }
 

diff --git a/packages/next/build/webpack-config.js b/packages/next/build/webpack-config.js
@@ -291,18 +291,20 @@ export default async function getBaseWebpackConfig (dir, {dev = false, isServer
       dev && new CaseSensitivePathPlugin(), // Since on macOS the filesystem is case-insensitive this will make sure your path are case-sensitive
       !dev && new webpack.HashedModuleIdsPlugin(),
       // Removes server/client code by minifier
-      new webpack.DefinePlugin(Object.assign(
-        {},
-        config.env ? Object.keys(config.env)
-          .reduce((acc, key) => ({
+      new webpack.DefinePlugin({
+        ...(Object.keys(config.env).reduce((acc, key) => {
+          if (/^(?:NODE_.+)|(?:__.+)$/i.test(key)) {
+            throw new Error(`The key "${key}" under "env" in next.config.js is not allowed. https://err.sh/zeit/next.js/env-key-not-allowed`)
+          }
+
+          return {
             ...acc,
-            ...{ [`process.env.${key}`]: JSON.stringify(config.env[key]) }
-          }), {}) : {},
-        {
-          'process.crossOrigin': JSON.stringify(config.crossOrigin),
-          'process.browser': JSON.stringify(!isServer)
-        }
-      )),
+            [`process.env.${key}`]: JSON.stringify(config.env[key])
+          }
+        }, {})),
+        'process.crossOrigin': JSON.stringify(config.crossOrigin),
+        'process.browser': JSON.stringify(!isServer)
+      }),
       // This is used in client/dev-error-overlay/hot-dev-client.js to replace the dist directory
       !isServer && dev && new webpack.DefinePlugin({
         'process.env.__NEXT_DIST_DIR': JSON.stringify(distDir)

diff --git a/packages/next/client/on-demand-entries-client.js b/packages/next/client/on-demand-entries-client.js
@@ -20,7 +20,7 @@ export default async ({ assetPrefix }) => {
     }
 
     return new Promise(resolve => {
-      ws = new WebSocket(`${wsProtocol}://${hostname}:${process.env.NEXT_WS_PORT}${process.env.NEXT_WS_PROXY_PATH}`)
+      ws = new WebSocket(`${wsProtocol}://${hostname}:${process.env.__NEXT_WS_PORT}${process.env.__NEXT_WS_PROXY_PATH}`)
       ws.onopen = () => resolve()
       ws.onclose = () => {
         setTimeout(async () => {

diff --git a/packages/next/server/hot-reloader.js b/packages/next/server/hot-reloader.js
@@ -167,8 +167,8 @@ export default class HotReloader {
   addWsConfig (configs) {
     const { websocketProxyPath, websocketProxyPort } = this.config.onDemandEntries
     const opts = {
-      'process.env.NEXT_WS_PORT': websocketProxyPort || this.wsPort,
-      'process.env.NEXT_WS_PROXY_PATH': JSON.stringify(websocketProxyPath)
+      'process.env.__NEXT_WS_PORT': websocketProxyPort || this.wsPort,
+      'process.env.__NEXT_WS_PROXY_PATH': JSON.stringify(websocketProxyPath)
     }
     configs[0].plugins.push(new webpack.DefinePlugin(opts))
   }

diff --git a/packages/next/taskfile-typescript.js b/packages/next/taskfile-typescript.js
@@ -40,7 +40,7 @@ try {
       if (file.base === 'next-dev.js') result.outputText = result.outputText.replace('// REPLACE_NOOP_IMPORT', `import('./noop');`)
 
       // update file's data
-      file.data = Buffer.from(result.outputText.replace(/process\.env\.NEXT_VERSION/, `"${require('./package.json').version}"`), 'utf8')
+      file.data = Buffer.from(result.outputText.replace(/process\.env\.__NEXT_VERSION/, `"${require('./package.json').version}"`), 'utf8')
     })
   }
 } catch (err) {

diff --git a/test/integration/production-config/next.config.js b/test/integration/production-config/next.config.js
@@ -2,11 +2,19 @@ const withCSS = require('@zeit/next-css')
 const withSass = require('@zeit/next-sass')
 const path = require('path')
 module.exports = withCSS(withSass({
+  env: {
+    ...(process.env.ENABLE_ENV_FAIL_UNDERSCORE ? {
+      '__NEXT_MY_VAR': 'test'
+    } : {}),
+    ...(process.env.ENABLE_ENV_FAIL_NODE ? {
+      'NODE_ENV': 'abc'
+    } : {})
+  },
   onDemandEntries: {
     // Make sure entries are not getting disposed.
     maxInactiveAge: 1000 * 60 * 60
   },
-  webpack (config, {buildId}) {
+  webpack (config) {
     // When next-css is `npm link`ed we have to solve loaders from the project root
     const nextLocation = path.join(require.resolve('next/package.json'), '../')
     const nextCssNodeModulesLocation = path.join(

diff --git a/test/integration/production-config/test/index.test.js b/test/integration/production-config/test/index.test.js
@@ -5,18 +5,20 @@ import {
   nextServer,
   nextBuild,
   startApp,
-  stopApp
+  stopApp,
+  runNextCommand
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5
 
+const appDir = join(__dirname, '../')
+
 let appPort
 let server
 
 describe('Production Config Usage', () => {
   beforeAll(async () => {
-    const appDir = join(__dirname, '../')
     await nextBuild(appDir)
     const app = nextServer({
       dir: join(__dirname, '../'),
@@ -37,6 +39,34 @@ describe('Production Config Usage', () => {
     })
   })
 
+  describe('env', () => {
+    it('should fail with __ in env key', async () => {
+      const result = await runNextCommand(['build', appDir], {spawnOptions: {
+        env: {
+          ...process.env,
+          ENABLE_ENV_FAIL_UNDERSCORE: true
+        }
+      },
+      stdout: true,
+      stderr: true})
+
+      expect(result.stderr).toMatch(/The key "__NEXT_MY_VAR" under/)
+    })
+
+    it('should fail with NODE_ in env key', async () => {
+      const result = await runNextCommand(['build', appDir], {spawnOptions: {
+        env: {
+          ...process.env,
+          ENABLE_ENV_FAIL_NODE: true
+        }
+      },
+      stdout: true,
+      stderr: true})
+
+      expect(result.stderr).toMatch(/The key "NODE_ENV" under/)
+    })
+  })
+
   describe('with generateBuildId', () => {
     it('should add the custom buildid', async () => {
       const browser = await webdriver(appPort, '/')

diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js
@@ -69,7 +69,14 @@ export function runNextCommand (argv, options = {}) {
   const cwd = path.dirname(require.resolve('next/package'))
   return new Promise((resolve, reject) => {
     console.log(`Running command "next ${argv.join(' ')}"`)
-    const instance = spawn('node', ['dist/bin/next', ...argv], { cwd, stdio: options.stdout ? ['ignore', 'pipe', 'ignore'] : 'inherit' })
+    const instance = spawn('node', ['dist/bin/next', ...argv], { ...options.spawnOptions, cwd, stdio: ['ignore', 'pipe', 'pipe'] })
+
+    let stderrOutput = ''
+    if (options.stderr) {
+      instance.stderr.on('data', function (chunk) {
+        stderrOutput += chunk
+      })
+    }
 
     let stdoutOutput = ''
     if (options.stdout) {
@@ -80,11 +87,14 @@ export function runNextCommand (argv, options = {}) {
 
     instance.on('close', () => {
       resolve({
-        stdout: stdoutOutput
+        stdout: stdoutOutput,
+        stderr: stderrOutput
       })
     })
 
     instance.on('error', (err) => {
+      err.stdout = stdoutOutput
+      err.stderr = stderrOutput
       reject(err)
     })
   })