Skip to content

Commit 639455e

Browse files
authored
fix(cli): logs entire environment (#623)
Right now the CLI logs the entire environment to debug logging. This is a lot of data, and potentially also includes tokens we'd rather don't end up in logging. Instead, just log our additions to the existing environment. --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 4eab621 commit 639455e

File tree

5 files changed

+42
-9
lines changed

5 files changed

+42
-9
lines changed

packages/@aws-cdk/cli-lib-alpha/lib/cli.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ export class AwsCdkCli implements IAwsCdkCli {
137137
...params.env,
138138
};
139139

140-
const cleanupContext = await writeContextToEnv(env, fullCtx);
140+
const cleanupContext = await writeContextToEnv(env, fullCtx, 'env-is-complete');
141141
try {
142142
return await withEnv(async() => createAssembly(await producer.produce(fullCtx)), env);
143143
} finally {

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/private/prepare-source.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,15 +179,21 @@ export class ExecutionEnvironment implements AsyncDisposable {
179179
* This *would* have returned an `IAsyncDisposable` but that requires messing
180180
* with TypeScript type definitions to use it in aws-cdk, so returning an
181181
* explicit cleanup function is easier.
182+
*
183+
* `completeness` indicates whether this `env` block represents the full `env`
184+
* that will be passed to a subprocess, or whether it will be mixed into
185+
* `process.env` later.
182186
*/
183-
export function writeContextToEnv(env: Env, context: Context) {
187+
export function writeContextToEnv(env: Env, context: Context, completeness: 'add-process-env-later' | 'env-is-complete') {
184188
let contextOverflowLocation = null;
185189

186190
// On Windows, all envvars together must fit in a 32k block (<https://devblogs.microsoft.com/oldnewthing/20100203-00>)
187191
// On Linux, a single entry may not exceed 131k; but we're treating it as all together because that's safe
188192
// and it's a single execution path for both platforms.
189193
const envVariableSizeLimit = os.platform() === 'win32' ? 32760 : 131072;
190-
const [smallContext, overflow] = splitBySize(context, spaceAvailableForContext(env, envVariableSizeLimit));
194+
195+
const completeEnv = { ...completeness === 'add-process-env-later' ? process.env : {}, ...env };
196+
const [smallContext, overflow] = splitBySize(context, spaceAvailableForContext(completeEnv, envVariableSizeLimit));
191197

192198
// Store the safe part in the environment variable
193199
env[cxapi.CONTEXT_ENV] = JSON.stringify(smallContext);

packages/@aws-cdk/toolkit-lib/lib/api/cloud-assembly/source-builder.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@ export abstract class CloudAssemblySourceBuilder {
342342
...synthParams.env,
343343
});
344344

345-
const cleanupContextTemp = writeContextToEnv(env, fullContext);
345+
const cleanupContextTemp = writeContextToEnv(env, fullContext, 'env-is-complete');
346346
using _cleanupEnv = (props.clobberEnv ?? true) ? temporarilyWriteEnv(env) : undefined;
347347
let assembly;
348348
try {
@@ -498,7 +498,7 @@ export abstract class CloudAssemblySourceBuilder {
498498
// Environment variables derived from settings
499499
...synthParams.env,
500500
});
501-
const cleanupTemp = writeContextToEnv(env, fullContext);
501+
const cleanupTemp = writeContextToEnv(env, fullContext, 'env-is-complete');
502502
try {
503503
await execInChildProcess(commandLine.join(' '), {
504504
eventPublisher: async (type, line) => {

packages/aws-cdk/lib/cxapp/exec.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,12 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config:
2929
await debugFn(format('context:', context));
3030

3131
const env: Record<string, string> = noUndefined({
32-
// Need to start with full env of `writeContextToEnv` will not be able to do the size
33-
// calculation correctly.
34-
...process.env,
3532
// Versioning, outdir, default account and region
3633
...await prepareDefaultEnvironment(aws, debugFn),
3734
// Environment variables derived from settings
3835
...params.env,
3936
});
37+
4038
const build = config.settings.get(['build']);
4139
if (build) {
4240
await exec(build);
@@ -73,6 +71,7 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config:
7371
}
7472

7573
await debugFn(`outdir: ${outdir}`);
74+
7675
env[cxapi.OUTDIR_ENV] = outdir;
7776

7877
// Acquire a lock on the output directory
@@ -84,7 +83,7 @@ export async function execProgram(aws: SdkProvider, ioHelper: IoHelper, config:
8483

8584
await debugFn(format('env:', env));
8685

87-
const cleanupTemp = writeContextToEnv(env, context);
86+
const cleanupTemp = writeContextToEnv(env, context, 'add-process-env-later');
8887
try {
8988
await exec(commandLine.join(' '));
9089

packages/aws-cdk/test/cxapp/exec.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,34 @@ test('the application set in --app is executed', async () => {
164164
await lock.release();
165165
});
166166

167+
test('execProgram will not log existing environment variables', async () => {
168+
const varName = 'SOME_BOGUS_VAR';
169+
170+
// GIVEN
171+
ioHost.level = 'debug';
172+
config.settings.set(['app'], 'cloud-executable');
173+
mockSpawn({
174+
commandLine: 'cloud-executable',
175+
sideEffect: () => writeOutputAssembly(),
176+
});
177+
178+
// WHEN
179+
process.env[varName] = 'hello';
180+
const { lock } = await execProgram(sdkProvider, ioHelper, config);
181+
await lock.release();
182+
delete process.env[varName];
183+
184+
// THEN
185+
const envMessage = (ioHost.notifySpy.mock.calls as Array<{ message: string }[]>)
186+
.map(([arg0]) => arg0)
187+
.find((arg0) => arg0.message.includes('env:'));
188+
189+
expect(envMessage).toBeDefined();
190+
expect(envMessage!.message).not.toContain(varName);
191+
192+
ioHost.level = 'info';
193+
});
194+
167195
test('the application set in --app is executed as-is if it contains a filename that does not exist', async () => {
168196
// GIVEN
169197
config.settings.set(['app'], 'does-not-exist');

0 commit comments

Comments
 (0)