Skip to content

Commit

Permalink
fix(bundling): prevent sensitive keys from being bundled
Browse files Browse the repository at this point in the history
  • Loading branch information
jaysoo committed Mar 20, 2024
1 parent a00f643 commit 5fd72ce
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 6 deletions.
28 changes: 28 additions & 0 deletions e2e/esbuild/src/esbuild.test.ts
Expand Up @@ -230,4 +230,32 @@ describe('EsBuild Plugin', () => {
const output = runCLI(`build ${myPkg}`);
expect(output).toContain('custom config loaded');
}, 120_000);

it('should bundle in non-sensitive NX_ environment variables', () => {
const myPkg = uniq('my-pkg');
runCLI(`generate @nx/js:lib ${myPkg} --bundler=esbuild`, {});

updateFile(
`libs/${myPkg}/src/index.ts`,
`
console.log(process.env['NX_CLOUD_ENCRYPTION_KEY']);
console.log(process.env['NX_CLOUD_ACCESS_TOKEN']);
console.log(process.env['NX_PUBLIC_TEST']);
`
);

runCLI(`build ${myPkg} --platform=browser`, {
env: {
NX_CLOUD_ENCRYPTION_KEY: 'secret',
NX_CLOUD_ACCESS_TOKEN: 'secret',
NX_PUBLIC_TEST: 'foobar',
},
});

const output = runCommand(`node dist/libs/${myPkg}/index.cjs`, {
failOnError: true,
});
expect(output).not.toMatch(/secret/);
expect(output).toMatch(/foobar/);
});
});
35 changes: 35 additions & 0 deletions e2e/storybook/src/storybook.test.ts
Expand Up @@ -2,11 +2,14 @@ import {
checkFilesExist,
cleanupProject,
killPorts,
listFiles,
newProject,
readFile,
runCLI,
runCommandUntil,
tmpProjPath,
uniq,
updateFile,
} from '@nx/e2e/utils';
import { writeFileSync } from 'fs';
import { createFileSync } from 'fs-extra';
Expand Down Expand Up @@ -106,5 +109,37 @@ describe('Storybook generators and executors for monorepos', () => {
runCLI(`run ${reactStorybookApp}:build-storybook --verbose`);
checkFilesExist(`${reactStorybookApp}/storybook-static/index.html`);
}, 300_000);

it('should bundle in non-sensitive NX_ environment variables', () => {
updateFile(
`${reactStorybookApp}/.storybook/main.ts`,
(content) => `
${content}
console.log(process.env);
`
);
runCLI(`run ${reactStorybookApp}:build-storybook --verbose`, {
env: {
NX_CLOUD_ENCRYPTION_KEY: 'secret',
NX_CLOUD_ACCESS_TOKEN: 'secret',
NX_PUBLIC_TEST: 'foobar',
},
});

// Check all output chunks for bundled environment variables
const outDir = `${reactStorybookApp}/storybook-static`;
const files = listFiles(outDir);
let nxPublicKeyFound = false;
for (const file of files) {
if (!file.endsWith('.js')) continue;
const content = readFile(`${outDir}/${file}`);
expect(content).not.toMatch(/NX_CLOUD_ENCRYPTION_KEY/);
expect(content).not.toMatch(/NX_CLOUD_ACCESS_TOKEN/);
if (content.match(/NX_PUBLIC_TEST/)) {
nxPublicKeyFound = true;
}
}
expect(nxPublicKeyFound).toBe(true);
}, 300_000);
});
});
31 changes: 30 additions & 1 deletion e2e/webpack/src/webpack.test.ts
@@ -1,7 +1,8 @@
import {
checkFilesExist,
cleanupProject,
listFiles,
newProject,
readFile,
rmDist,
runCLI,
runCommand,
Expand Down Expand Up @@ -159,4 +160,32 @@ describe('Webpack Plugin', () => {
let output = runCommand(`node dist/${appName}/main.js`);
expect(output).toMatch(/Hello/);
}, 500_000);

it('should bundle in non-sensitive NX_ environment variables', () => {
const appName = uniq('app');
runCLI(`generate @nx/web:app ${appName} --bundler webpack`);
updateFile(
`apps/${appName}/src/main.ts`,
`
console.log(process.env['NX_CLOUD_ENCRYPTION_KEY']);
console.log(process.env['NX_CLOUD_ACCESS_TOKEN']);
console.log(process.env['NX_PUBLIC_TEST']);
`
);

runCLI(`build ${appName}`, {
env: {
NX_CLOUD_ENCRYPTION_KEY: 'secret',
NX_CLOUD_ACCESS_TOKEN: 'secret',
NX_PUBLIC_TEST: 'foobar',
},
});

const mainFile = listFiles(`dist/apps/${appName}`).filter((f) =>
f.startsWith('main.')
);
const content = readFile(`dist/apps/${appName}/${mainFile}`);
expect(content).not.toMatch(/secret/);
expect(content).toMatch(/foobar/);
});
});
9 changes: 8 additions & 1 deletion packages/esbuild/src/utils/environment-variables.ts
@@ -1,8 +1,15 @@
// Prevent sensitive keys from being bundled when source code uses entire `process.env` object rather than individual keys (e.g. `process.env.NX_FOO`).
// TODO(v19): Only env vars prefixed with NX_PUBLIC should be bundled. This is a breaking change so we won't do it in v18.
const excludedKeys = ['NX_CLOUD_ACCESS_TOKEN', 'NX_CLOUD_ENCRYPTION_KEY'];

export function getClientEnvironment(): Record<string, string> {
const NX_APP = /^NX_/i;

return Object.keys(process.env)
.filter((key) => NX_APP.test(key) || key === 'NODE_ENV')
.filter(
(key) =>
!excludedKeys.includes(key) && (NX_APP.test(key) || key === 'NODE_ENV')
)
.reduce((env, key) => {
env[`process.env.${key}`] = JSON.stringify(process.env[key]);
return env;
Expand Down
6 changes: 5 additions & 1 deletion packages/next/plugins/with-nx.ts
Expand Up @@ -360,9 +360,13 @@ export function getNextConfig(
};
}

// Prevent sensitive keys from being bundled when source code uses entire `process.env` object rather than individual keys (e.g. `process.env.NX_FOO`).
// TODO(v19): BREAKING: Only support NEXT_PUBLIC_ env vars and ignore NX_ vars since this is a standard Next.js feature.
const excludedKeys = ['NX_CLOUD_ACCESS_TOKEN', 'NX_CLOUD_ENCRYPTION_KEY'];

function getNxEnvironmentVariables() {
return Object.keys(process.env)
.filter((env) => /^NX_/i.test(env))
.filter((env) => !excludedKeys.includes(env) && /^NX_/i.test(env))
.reduce((env, key) => {
env[key] = process.env[key];
return env;
Expand Down
10 changes: 9 additions & 1 deletion packages/react/plugins/storybook/index.ts
Expand Up @@ -18,6 +18,10 @@ import { mergePlugins } from './merge-plugins';
import { withReact } from '../with-react';
import { existsSync } from 'fs';

// Prevent sensitive keys from being bundled when source code uses entire `process.env` object rather than individual keys (e.g. `process.env.NX_FOO`).
// TODO(v19): BREAKING: Only env vars prefixed with NX_PUBLIC should be bundled. This is a breaking change so we won't do it in v18.
const excludedKeys = ['NX_CLOUD_ACCESS_TOKEN', 'NX_CLOUD_ENCRYPTION_KEY'];

// This is shamelessly taken from CRA and modified for NX use
// https://github.com/facebook/create-react-app/blob/4784997f0682e75eb32a897b4ffe34d735912e6c/packages/react-scripts/config/env.js#L71
function getClientEnvironment(mode) {
Expand All @@ -27,7 +31,11 @@ function getClientEnvironment(mode) {
const STORYBOOK_PREFIX = /^STORYBOOK_/i;

const raw = Object.keys(process.env)
.filter((key) => NX_PREFIX.test(key) || STORYBOOK_PREFIX.test(key))
.filter(
(key) =>
!excludedKeys.includes(key) &&
(NX_PREFIX.test(key) || STORYBOOK_PREFIX.test(key))
)
.reduce(
(env, key) => {
env[key] = process.env[key];
Expand Down
6 changes: 5 additions & 1 deletion packages/webpack/src/utils/get-client-environment.ts
@@ -1,10 +1,14 @@
// Prevent sensitive keys from being bundled when source code uses entire `process.env` object rather than individual keys (e.g. `process.env.NX_FOO`).
// TODO(v19): Only env vars prefixed with NX_PUBLIC should be bundled. This is a breaking change so we won't do it in v18.
const excludedKeys = ['NX_CLOUD_ACCESS_TOKEN', 'NX_CLOUD_ENCRYPTION_KEY'];

export function getClientEnvironment(mode?: string) {
// Grab NODE_ENV and NX_* environment variables and prepare them to be
// injected into the application via DefinePlugin in webpack configuration.
const NX_APP = /^NX_/i;

const raw = Object.keys(process.env)
.filter((key) => NX_APP.test(key))
.filter((key) => !excludedKeys.includes(key) && NX_APP.test(key))
.reduce(
(env, key) => {
env[key] = process.env[key];
Expand Down
Expand Up @@ -8,8 +8,12 @@ export function interpolateEnvironmentVariablesToIndex(

const NX_PREFIX = /^NX_/i;

// Prevent sensitive keys from being bundled when source code uses entire `process.env` object rather than individual keys (e.g. `process.env.NX_FOO`).
// TODO(v19): Only env vars prefixed with NX_PUBLIC should be bundled. This is a breaking change so we won't do it in v18.
const excludedKeys = ['NX_CLOUD_ACCESS_TOKEN', 'NX_CLOUD_ENCRYPTION_KEY'];

function isNxEnvironmentKey(x: string): boolean {
return NX_PREFIX.test(x);
return !excludedKeys.includes(x) && NX_PREFIX.test(x);
}

function getClientEnvironment(deployUrl: string) {
Expand Down

0 comments on commit 5fd72ce

Please sign in to comment.