[build-utils][cli][go][node][ruby][static-build] Explicitly set PATH when copying env vars (#8532)

On Windows 10 and 11 machines, environment variables are not case sensitive. The system PATH is actually defined as `process.env.Path`, however Node.js kindly handles the case sensitivity and will automatically return the system path when specifying `process.env.PATH`.

When we clone the environment variables via `{ ...process.env }`, we lose the automatic resolving of `Path` to `PATH`. To fix this, we need to explicitly copy the `PATH`.

#### Tests

- [x] The code changed/added as part of this PR has been covered with tests
- [x] All tests pass locally with `yarn test-unit`

#### Code Review

- [ ] This PR has a concise title and thorough description useful to a reviewer
- [ ] Issue from task tracker has a link to this PR
This commit is contained in:
Chris Barber
2022-09-14 15:29:52 -05:00
committed by GitHub
parent 6498fd1aab
commit 7ddebb099d
9 changed files with 184 additions and 32 deletions

View File

@@ -0,0 +1,29 @@
import type { Env } from './types';
const { hasOwnProperty } = Object.prototype;
/**
* Clones zero or more objects into a single new object while ensuring that the
* `PATH` environment variable is defined when the `PATH` or `Path` environment
* variables are defined.
*
* @param {Object} [...envs] Objects and/or `process.env` to clone and merge
* @returns {Object} The new object
*/
export function cloneEnv(...envs: (Env | undefined)[]): Env {
return envs.reduce((obj: Env, env) => {
if (env === undefined || env === null) {
return obj;
}
// the system path is called `Path` on Windows and Node.js will
// automatically return the system path when accessing `PATH`,
// however we lose this proxied value when we destructure and
// thus we must explicitly copy it
if (hasOwnProperty.call(env, 'PATH') || hasOwnProperty.call(env, 'Path')) {
obj.PATH = env.PATH;
}
return Object.assign(obj, env);
}, {});
}

View File

@@ -11,6 +11,7 @@ import { NowBuildError } from '../errors';
import { Meta, PackageJson, NodeVersion, Config } from '../types'; import { Meta, PackageJson, NodeVersion, Config } from '../types';
import { getSupportedNodeVersion, getLatestNodeVersion } from './node-version'; import { getSupportedNodeVersion, getLatestNodeVersion } from './node-version';
import { readConfigFile } from './read-config-file'; import { readConfigFile } from './read-config-file';
import { cloneEnv } from '../clone-env';
// Only allow one `runNpmInstall()` invocation to run concurrently // Only allow one `runNpmInstall()` invocation to run concurrently
const runNpmInstallSema = new Sema(1); const runNpmInstallSema = new Sema(1);
@@ -217,7 +218,7 @@ export function getSpawnOptions(
nodeVersion: NodeVersion nodeVersion: NodeVersion
): SpawnOptions { ): SpawnOptions {
const opts = { const opts = {
env: { ...process.env }, env: cloneEnv(process.env),
}; };
if (!meta.isDev) { if (!meta.isDev) {
@@ -449,7 +450,7 @@ export async function runNpmInstall(
debug(`Installing to ${destPath}`); debug(`Installing to ${destPath}`);
const opts: SpawnOptionsExtended = { cwd: destPath, ...spawnOpts }; const opts: SpawnOptionsExtended = { cwd: destPath, ...spawnOpts };
const env = opts.env ? { ...opts.env } : { ...process.env }; const env = cloneEnv(opts.env || process.env);
delete env.NODE_ENV; delete env.NODE_ENV;
opts.env = getEnvForPackageManager({ opts.env = getEnvForPackageManager({
cliType, cliType,
@@ -591,10 +592,7 @@ export async function runPackageJsonScript(
cliType, cliType,
lockfileVersion, lockfileVersion,
nodeVersion: undefined, nodeVersion: undefined,
env: { env: cloneEnv(process.env, spawnOpts?.env),
...process.env,
...spawnOpts?.env,
},
}), }),
}; };

View File

@@ -41,6 +41,7 @@ import debug from './debug';
import getIgnoreFilter from './get-ignore-filter'; import getIgnoreFilter from './get-ignore-filter';
import { getPlatformEnv } from './get-platform-env'; import { getPlatformEnv } from './get-platform-env';
import { getPrefixedEnvVars } from './get-prefixed-env-vars'; import { getPrefixedEnvVars } from './get-prefixed-env-vars';
import { cloneEnv } from './clone-env';
export { export {
FileBlob, FileBlob,
@@ -84,6 +85,7 @@ export {
getLambdaOptionsFromFunction, getLambdaOptionsFromFunction,
scanParentDirs, scanParentDirs,
getIgnoreFilter, getIgnoreFilter,
cloneEnv,
}; };
export { EdgeFunction } from './edge-function'; export { EdgeFunction } from './edge-function';

View File

@@ -0,0 +1,120 @@
import { cloneEnv } from '../src';
it('should clone env with Path', () => {
expect(
cloneEnv(
new Proxy(
{
foo: 'bar',
Path: 'baz',
},
{
get(target: typeof process.env, prop: string) {
if (prop === 'PATH') {
return target.PATH ?? target.Path;
}
return target[prop];
},
}
)
)
).toEqual({
foo: 'bar',
Path: 'baz',
PATH: 'baz',
});
});
it('should clone env with PATH', () => {
expect(
cloneEnv({
foo: 'bar',
PATH: 'baz',
})
).toEqual({
foo: 'bar',
PATH: 'baz',
});
});
it('should clone and merge multiple env objects', () => {
// note: this also tests the last object doesn't overwrite `PATH` with
// `undefined`
expect(
cloneEnv(
{
foo: 'bar',
},
{
PATH: 'baz',
},
{
baz: 'wiz',
}
)
).toEqual({
foo: 'bar',
PATH: 'baz',
baz: 'wiz',
});
});
it('should clone the actual process.env object', () => {
expect(cloneEnv(process.env).PATH).toEqual(process.env.PATH);
});
it('should overwrite PATH with last value', () => {
expect(
cloneEnv(
new Proxy(
{
Path: 'foo',
},
{
get(target: typeof process.env, prop: string) {
if (prop === 'PATH') {
return target.PATH ?? target.Path;
}
return target[prop];
},
}
),
{
PATH: 'bar',
},
{
PATH: undefined,
}
)
).toEqual({
Path: 'foo',
PATH: undefined,
});
});
it('should handle process.env at any argument position', () => {
expect(
cloneEnv(
{
foo: 'bar',
},
new Proxy(
{
Path: 'baz',
},
{
get(target: typeof process.env, prop: string) {
if (prop === 'PATH') {
return target.PATH ?? target.Path;
}
return target[prop];
},
}
)
)
).toEqual({
foo: 'bar',
Path: 'baz',
PATH: 'baz',
});
});

View File

@@ -31,6 +31,7 @@ import {
} from '@vercel/routing-utils'; } from '@vercel/routing-utils';
import { import {
Builder, Builder,
cloneEnv,
Env, Env,
StartDevServerResult, StartDevServerResult,
FileFsRef, FileFsRef,
@@ -2222,7 +2223,8 @@ export default class DevServer {
const port = await getPort(); const port = await getPort();
const env: Env = { const env: Env = cloneEnv(
{
// Because of child process 'pipe' below, isTTY will be false. // Because of child process 'pipe' below, isTTY will be false.
// Most frameworks use `chalk`/`supports-color` so we enable it anyway. // Most frameworks use `chalk`/`supports-color` so we enable it anyway.
FORCE_COLOR: process.stdout.isTTY ? '1' : '0', FORCE_COLOR: process.stdout.isTTY ? '1' : '0',
@@ -2230,10 +2232,13 @@ export default class DevServer {
// browser window, since it will not be the port that `vc dev` // browser window, since it will not be the port that `vc dev`
// is listening on and thus will be missing Vercel features. // is listening on and thus will be missing Vercel features.
BROWSER: 'none', BROWSER: 'none',
...process.env, },
...this.envConfigs.allEnv, process.env,
this.envConfigs.allEnv,
{
PORT: `${port}`, PORT: `${port}`,
}; }
);
// This is necesary so that the dev command in the Project // This is necesary so that the dev command in the Project
// will work cross-platform (especially Windows). // will work cross-platform (especially Windows).

View File

@@ -27,6 +27,7 @@ import {
getWriteableDirectory, getWriteableDirectory,
shouldServe, shouldServe,
debug, debug,
cloneEnv,
} from '@vercel/build-utils'; } from '@vercel/build-utils';
const TMP = tmpdir(); const TMP = tmpdir();
@@ -694,11 +695,9 @@ Learn more: https://vercel.com/docs/runtimes#official-runtimes/go`
`vercel-dev-port-${Math.random().toString(32).substring(2)}` `vercel-dev-port-${Math.random().toString(32).substring(2)}`
); );
const env: typeof process.env = { const env = cloneEnv(process.env, meta.env, {
...process.env,
...meta.env,
VERCEL_DEV_PORT_FILE: portFile, VERCEL_DEV_PORT_FILE: portFile,
}; });
const tmpRelative = `.${sep}${entrypointDir}`; const tmpRelative = `.${sep}${entrypointDir}`;
const child = spawn('go', ['run', tmpRelative], { const child = spawn('go', ['run', tmpRelative], {

View File

@@ -36,6 +36,7 @@ import {
debug, debug,
isSymbolicLink, isSymbolicLink,
walkParentDirs, walkParentDirs,
cloneEnv,
} from '@vercel/build-utils'; } from '@vercel/build-utils';
import type { import type {
File, File,
@@ -525,15 +526,13 @@ export const startDevServer: StartDevServer = async opts => {
const child = fork(devServerPath, [], { const child = fork(devServerPath, [], {
cwd: workPath, cwd: workPath,
execArgv: [], execArgv: [],
env: { env: cloneEnv(process.env, meta.env, {
...process.env,
...meta.env,
VERCEL_DEV_ENTRYPOINT: entrypoint, VERCEL_DEV_ENTRYPOINT: entrypoint,
VERCEL_DEV_TSCONFIG: projectTsConfig || '', VERCEL_DEV_TSCONFIG: projectTsConfig || '',
VERCEL_DEV_IS_ESM: isEsm ? '1' : undefined, VERCEL_DEV_IS_ESM: isEsm ? '1' : undefined,
VERCEL_DEV_CONFIG: JSON.stringify(config), VERCEL_DEV_CONFIG: JSON.stringify(config),
VERCEL_DEV_BUILD_ENV: JSON.stringify(meta.buildEnv || {}), VERCEL_DEV_BUILD_ENV: JSON.stringify(meta.buildEnv || {}),
}, }),
}); });
const { pid } = child; const { pid } = child;

View File

@@ -16,6 +16,7 @@ import {
createLambda, createLambda,
debug, debug,
walkParentDirs, walkParentDirs,
cloneEnv,
} from '@vercel/build-utils'; } from '@vercel/build-utils';
import { installBundler } from './install-ruby'; import { installBundler } from './install-ruby';
@@ -63,12 +64,11 @@ async function bundleInstall(
['install', '--deployment', '--gemfile', gemfilePath, '--path', bundleDir], ['install', '--deployment', '--gemfile', gemfilePath, '--path', bundleDir],
{ {
stdio: 'pipe', stdio: 'pipe',
env: { env: cloneEnv(process.env, {
...process.env,
BUNDLE_SILENCE_ROOT_WARNING: '1', BUNDLE_SILENCE_ROOT_WARNING: '1',
BUNDLE_APP_CONFIG: bundleAppConfig, BUNDLE_APP_CONFIG: bundleAppConfig,
BUNDLE_JOBS: '4', BUNDLE_JOBS: '4',
}, }),
} }
); );
} }

View File

@@ -31,6 +31,7 @@ import {
debug, debug,
NowBuildError, NowBuildError,
scanParentDirs, scanParentDirs,
cloneEnv,
} from '@vercel/build-utils'; } from '@vercel/build-utils';
import type { Route, RouteWithSrc } from '@vercel/routing-utils'; import type { Route, RouteWithSrc } from '@vercel/routing-utils';
import * as BuildOutputV1 from './utils/build-output-v1'; import * as BuildOutputV1 from './utils/build-output-v1';
@@ -465,8 +466,7 @@ export const build: BuildV2 = async ({
debug('Detected Gemfile'); debug('Detected Gemfile');
printInstall(); printInstall();
const opts = { const opts = {
env: { env: cloneEnv(process.env, {
...process.env,
// See more: https://github.com/rubygems/rubygems/blob/a82d04856deba58be6b90f681a5e42a7c0f2baa7/bundler/lib/bundler/man/bundle-config.1.ronn // See more: https://github.com/rubygems/rubygems/blob/a82d04856deba58be6b90f681a5e42a7c0f2baa7/bundler/lib/bundler/man/bundle-config.1.ronn
BUNDLE_BIN: 'vendor/bin', BUNDLE_BIN: 'vendor/bin',
BUNDLE_CACHE_PATH: 'vendor/cache', BUNDLE_CACHE_PATH: 'vendor/cache',
@@ -476,7 +476,7 @@ export const build: BuildV2 = async ({
BUNDLE_SILENCE_ROOT_WARNING: '1', BUNDLE_SILENCE_ROOT_WARNING: '1',
BUNDLE_DISABLE_SHARED_GEMS: '1', BUNDLE_DISABLE_SHARED_GEMS: '1',
BUNDLE_DISABLE_VERSION_CHECK: '1', BUNDLE_DISABLE_VERSION_CHECK: '1',
}, }),
}; };
await runBundleInstall(workPath, [], opts, meta); await runBundleInstall(workPath, [], opts, meta);
isBundleInstall = true; isBundleInstall = true;