From 3e8a9600ce4b6a5dea8b6798684a5096ae099ab8 Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Tue, 11 Jan 2022 14:48:48 +0200 Subject: [PATCH 1/5] middlewares: limit `process.env` to inferred usage Production middlewares will only expose env vars that are statically analyzable, as mentioned here: https://nextjs.org/docs/api-reference/next/server#how-do-i-access-environment-variables This creates some incompatibility with `next dev` and `next start`, where all `process.env` data is shared and can lead to unexpected behavior in runtime. This PR fixes it by limiting the data in `process.env` with the inferred env vars from the code usage. --- packages/next/server/base-server.ts | 1 + packages/next/server/require.ts | 3 +- packages/next/server/web/sandbox/context.ts | 20 ++++++-- packages/next/server/web/sandbox/sandbox.ts | 2 + .../index.test.ts | 50 +++++++++++++++++++ 5 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts diff --git a/packages/next/server/base-server.ts b/packages/next/server/base-server.ts index e0feebf06e5b589..10be06f4463a1a6 100644 --- a/packages/next/server/base-server.ts +++ b/packages/next/server/base-server.ts @@ -725,6 +725,7 @@ export default abstract class Server { result = await run({ name: middlewareInfo.name, paths: middlewareInfo.paths, + env: middlewareInfo.env, request: { headers: params.request.headers, method: params.request.method || 'GET', diff --git a/packages/next/server/require.ts b/packages/next/server/require.ts index a45a6619084109f..9c34b8538300fda 100644 --- a/packages/next/server/require.ts +++ b/packages/next/server/require.ts @@ -85,7 +85,7 @@ export function getMiddlewareInfo(params: { distDir: string page: string serverless: boolean -}): { name: string; paths: string[] } { +}): { name: string; paths: string[]; env: string[] } { const serverBuildPath = join( params.distDir, params.serverless && !params.dev ? SERVERLESS_DIRECTORY : SERVER_DIRECTORY @@ -112,5 +112,6 @@ export function getMiddlewareInfo(params: { return { name: pageInfo.name, paths: pageInfo.files.map((file) => join(params.distDir, file)), + env: pageInfo.env ?? [], } } diff --git a/packages/next/server/web/sandbox/context.ts b/packages/next/server/web/sandbox/context.ts index 880ab46f220dbd1..de3876ffae9480f 100644 --- a/packages/next/server/web/sandbox/context.ts +++ b/packages/next/server/web/sandbox/context.ts @@ -56,6 +56,7 @@ export function getModuleContext(options: { module: string onWarning: (warn: Error) => void useCache: boolean + env: string[] }) { let moduleCache = options.useCache ? caches.get(options.module) @@ -97,12 +98,13 @@ export function getModuleContext(options: { function createModuleContext(options: { onWarning: (warn: Error) => void module: string + env: string[] }) { const requireCache = new Map([ [require.resolve('next/dist/compiled/cookie'), { exports: cookie }], ]) - const context = createContext() + const context = createContext(options) requireDependencies({ requireCache: requireCache, @@ -171,7 +173,10 @@ function createModuleContext(options: { * Create a base context with all required globals for the runtime that * won't depend on any externally provided dependency. */ -function createContext() { +function createContext(options: { + /** Environment variables to be provided to the context */ + env: string[] +}) { const context: { [key: string]: unknown } = { _ENTRIES: {}, atob: polyfills.atob, @@ -196,7 +201,9 @@ function createContext() { crypto: new polyfills.Crypto(), File, FormData, - process: { env: { ...process.env } }, + process: { + env: buildEnvironmentVariablesFrom(options.env), + }, ReadableStream: polyfills.ReadableStream, setInterval, setTimeout, @@ -245,3 +252,10 @@ function createContext() { : undefined, }) } + +function buildEnvironmentVariablesFrom( + keys: string[] +): Record { + const pairs = keys.map((key) => [key, process.env[key]]) + return Object.fromEntries(pairs) +} diff --git a/packages/next/server/web/sandbox/sandbox.ts b/packages/next/server/web/sandbox/sandbox.ts index 7a98d4d1223432f..576e8be7580c6e5 100644 --- a/packages/next/server/web/sandbox/sandbox.ts +++ b/packages/next/server/web/sandbox/sandbox.ts @@ -3,6 +3,7 @@ import { getModuleContext } from './context' export async function run(params: { name: string + env: string[] onWarning: (warn: Error) => void paths: string[] request: RequestData @@ -12,6 +13,7 @@ export async function run(params: { module: params.name, onWarning: params.onWarning, useCache: params.useCache !== false, + env: params.env, }) for (const paramPath of params.paths) { diff --git a/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts b/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts new file mode 100644 index 000000000000000..f18fc371cf08ae0 --- /dev/null +++ b/test/production/middleware-environment-variables-in-node-server-reflect-the-usage-inference/index.test.ts @@ -0,0 +1,50 @@ +import { createNext } from 'e2e-utils' +import { NextInstance } from 'test/lib/next-modes/base' +import { renderViaHTTP } from 'next-test-utils' + +describe('middleware environment variables in node server reflect the usage inference', () => { + let next: NextInstance + + beforeAll(() => { + process.env.CAN_BE_INFERRED = 'can-be-inferred' + process.env.X_CUSTOM_HEADER = 'x-custom-header' + process.env.IGNORED_ENV_VAR = 'ignored-env-var' + }) + + beforeAll(async () => { + next = await createNext({ + files: { + 'pages/_middleware.js': ` + export default function middleware() { + return new Response(JSON.stringify({ + canBeInferred: process.env.CAN_BE_INFERRED, + rest: process.env + }), { + headers: { + 'Content-Type': 'application/json', + 'X-Custom-Header': process.env.X_CUSTOM_HEADER, + } + }) + } + `, + }, + dependencies: {}, + }) + }) + afterAll(() => next.destroy()) + + it('limits process.env to only contain env vars that are inferred from usage', async () => { + const html = await renderViaHTTP(next.url, '/test') + let parsed: any + expect(() => { + parsed = JSON.parse(html) + }).not.toThrow() + expect(parsed).toEqual({ + canBeInferred: 'can-be-inferred', + rest: { + CAN_BE_INFERRED: 'can-be-inferred', + X_CUSTOM_HEADER: 'x-custom-header', + }, + }) + }) +}) From 4dbec5d69f2bde3af54e5c92cecc2534adfa6682 Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Wed, 12 Jan 2022 11:39:40 +0200 Subject: [PATCH 2/5] make middleware plugin aware of environment variables in server components too --- packages/next/build/webpack/plugins/middleware-plugin.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index 9f0c53abe43d5cf..f61ce9442aca797 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -141,7 +141,10 @@ export default class MiddlewarePlugin { envPerRoute.clear() for (const [name, info] of compilation.entries) { - if (name.match(MIDDLEWARE_ROUTE)) { + if ( + name.match(MIDDLEWARE_ROUTE) || + info.options.runtime === 'middleware-ssr-runtime' + ) { const middlewareEntries = new Set() const env = new Set() From bdd9a1700a4d241faaf457301693dd88188162e5 Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Wed, 12 Jan 2022 11:40:48 +0200 Subject: [PATCH 3/5] use constant --- packages/next/build/webpack/plugins/middleware-plugin.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index f61ce9442aca797..d36f2cf167021fa 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -7,6 +7,7 @@ import { MIDDLEWARE_BUILD_MANIFEST, MIDDLEWARE_REACT_LOADABLE_MANIFEST, MIDDLEWARE_RUNTIME_WEBPACK, + MIDDLEWARE_SSR_RUNTIME_WEBPACK, } from '../../../shared/lib/constants' import { MIDDLEWARE_ROUTE } from '../../../lib/constants' import { nonNullable } from '../../../lib/non-nullable' @@ -142,8 +143,8 @@ export default class MiddlewarePlugin { for (const [name, info] of compilation.entries) { if ( - name.match(MIDDLEWARE_ROUTE) || - info.options.runtime === 'middleware-ssr-runtime' + info.options.runtime === MIDDLEWARE_SSR_RUNTIME_WEBPACK || + name.match(MIDDLEWARE_ROUTE) ) { const middlewareEntries = new Set() const env = new Set() From 9b7592344bacba0aec866725d3ab6787ddc66885 Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Wed, 12 Jan 2022 11:49:14 +0200 Subject: [PATCH 4/5] Check runtimes for both cases for consistency Co-authored-by: Tobias Koppers --- packages/next/build/webpack/plugins/middleware-plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index d36f2cf167021fa..7091e39a4efddb9 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -144,7 +144,7 @@ export default class MiddlewarePlugin { for (const [name, info] of compilation.entries) { if ( info.options.runtime === MIDDLEWARE_SSR_RUNTIME_WEBPACK || - name.match(MIDDLEWARE_ROUTE) + info.options.runtime === MIDDLEWARE_RUNTIME_WEBPACK ) { const middlewareEntries = new Set() const env = new Set() From 577a44cef5f700bbfd783167550cd8edb40d0ccb Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Wed, 12 Jan 2022 12:16:38 +0200 Subject: [PATCH 5/5] lint fix --- packages/next/build/webpack/plugins/middleware-plugin.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts index 7091e39a4efddb9..0cac58aa6e7264a 100644 --- a/packages/next/build/webpack/plugins/middleware-plugin.ts +++ b/packages/next/build/webpack/plugins/middleware-plugin.ts @@ -9,7 +9,6 @@ import { MIDDLEWARE_RUNTIME_WEBPACK, MIDDLEWARE_SSR_RUNTIME_WEBPACK, } from '../../../shared/lib/constants' -import { MIDDLEWARE_ROUTE } from '../../../lib/constants' import { nonNullable } from '../../../lib/non-nullable' const PLUGIN_NAME = 'MiddlewarePlugin'