Skip to content

Commit 869b109

Browse files
kaizenccgithub-actions
andauthored
chore(cli): allow valid enum values in telemetry data (#711)
Enum values are defined as `choices` in the `cli-type-registry.json` file. If the choices array exists, and if the value we get is included as a choice, then we allow that raw value to be recorded in telemetry because it is not free text. Invalid values passed in an enum property are not recorded. In practice, the CLI fails before telemetry starts with ``` Invalid values: Argument: require-approval, Given: "neveasdfr", Choices: "never", "any-change", "broadening" ``` --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license --------- Signed-off-by: github-actions <github-actions@github.com> Co-authored-by: github-actions <github-actions@github.com>
1 parent d40bf52 commit 869b109

File tree

2 files changed

+35
-3
lines changed

2 files changed

+35
-3
lines changed

packages/aws-cdk/lib/cli/telemetry/sanitation.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export function sanitizeCommandLineArguments(argv: any): { path: string[]; param
3434
if (argv[argName] === undefined || (!globalOptions.includes(argName) && !commandOptions.includes(argName))) {
3535
continue;
3636
}
37-
if (isNumberOrBoolean(argv[argName])) {
37+
if (isNumberOrBoolean(argv[argName]) || isKnownEnumValue(argName, argv[argName], command, config)) {
3838
parameters[argName] = argv[argName];
3939
} else {
4040
parameters[argName] = '<redacted>';
@@ -71,6 +71,15 @@ function isNumberOrBoolean(value: any): boolean {
7171
return typeof value === 'number' || isBoolean(value);
7272
}
7373

74+
function isKnownEnumValue(name: string, value: any, command: string, config: any): boolean {
75+
const propertyDefiniton = config.globalOptions[name] ?? config.commands[command]?.options[name];
76+
if (propertyDefiniton.type === 'string') {
77+
// Even if the property has choices, only record if the value is a valid choice
78+
return propertyDefiniton.choices?.includes(value);
79+
}
80+
return false;
81+
}
82+
7483
function isFeatureFlag(flag: string): flag is FeatureFlag {
7584
return Object.values(FeatureFlag).includes(flag as FeatureFlag);
7685
}

packages/aws-cdk/test/cli/telemetry/sanitation.test.ts

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,35 @@ describe(sanitizeCommandLineArguments, () => {
103103
const argv = {
104104
_: ['deploy'],
105105
STACKS: ['MyStack'],
106-
['require-approval']: 'broadening',
107106
['build-exclude']: ['something'],
108107
};
109108
expect(sanitizeCommandLineArguments(argv)).toEqual({
110109
path: ['deploy', '$STACKS_1'],
111-
parameters: { 'require-approval': '<redacted>', 'build-exclude': '<redacted>' },
110+
parameters: { 'build-exclude': '<redacted>' },
111+
});
112+
});
113+
114+
test('enum options are allowed', () => {
115+
const argv = {
116+
_: ['deploy'],
117+
STACKS: ['MyStack'],
118+
['require-approval']: 'broadening',
119+
};
120+
expect(sanitizeCommandLineArguments(argv)).toEqual({
121+
path: ['deploy', '$STACKS_1'],
122+
parameters: { 'require-approval': 'broadening' },
123+
});
124+
});
125+
126+
test('invalid enum options are redacted', () => {
127+
const argv = {
128+
_: ['deploy'],
129+
STACKS: ['MyStack'],
130+
['require-approval']: 'invalid',
131+
};
132+
expect(sanitizeCommandLineArguments(argv)).toEqual({
133+
path: ['deploy', '$STACKS_1'],
134+
parameters: { 'require-approval': '<redacted>' },
112135
});
113136
});
114137
});

0 commit comments

Comments
 (0)