Skip to content

Commit

Permalink
middlewares: limit process.env to inferred usage (#33186)
Browse files Browse the repository at this point in the history
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. I believe the test speaks for itself 🕺 

<!--
## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
-->
  • Loading branch information
Schniz committed Jan 12, 2022
1 parent 029f2e3 commit e695004
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 6 deletions.
7 changes: 5 additions & 2 deletions packages/next/build/webpack/plugins/middleware-plugin.ts
Expand Up @@ -7,8 +7,8 @@ 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'

const PLUGIN_NAME = 'MiddlewarePlugin'
Expand Down Expand Up @@ -141,7 +141,10 @@ export default class MiddlewarePlugin {
envPerRoute.clear()

for (const [name, info] of compilation.entries) {
if (name.match(MIDDLEWARE_ROUTE)) {
if (
info.options.runtime === MIDDLEWARE_SSR_RUNTIME_WEBPACK ||
info.options.runtime === MIDDLEWARE_RUNTIME_WEBPACK
) {
const middlewareEntries = new Set<webpack5.Module>()
const env = new Set<string>()

Expand Down
1 change: 1 addition & 0 deletions packages/next/server/base-server.ts
Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion packages/next/server/require.ts
Expand Up @@ -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
Expand All @@ -112,5 +112,6 @@ export function getMiddlewareInfo(params: {
return {
name: pageInfo.name,
paths: pageInfo.files.map((file) => join(params.distDir, file)),
env: pageInfo.env ?? [],
}
}
20 changes: 17 additions & 3 deletions packages/next/server/web/sandbox/context.ts
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -245,3 +252,10 @@ function createContext() {
: undefined,
})
}

function buildEnvironmentVariablesFrom(
keys: string[]
): Record<string, string | undefined> {
const pairs = keys.map((key) => [key, process.env[key]])
return Object.fromEntries(pairs)
}
2 changes: 2 additions & 0 deletions packages/next/server/web/sandbox/sandbox.ts
Expand Up @@ -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
Expand All @@ -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) {
Expand Down
@@ -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',
},
})
})
})

0 comments on commit e695004

Please sign in to comment.