Skip to content

Commit

Permalink
chore: address a number of CodeQL errors (#27253)
Browse files Browse the repository at this point in the history
Reported by GitHub's code quality scanning tool.

Many of these are problems with regular expressions.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Sep 25, 2023
1 parent ab52bb5 commit 7d0f32f
Show file tree
Hide file tree
Showing 15 changed files with 31 additions and 15 deletions.
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/lib/cfn-init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,10 @@ function deepMerge(target?: Record<string, any>, src?: Record<string, any>) {
if (src == null) { return target; }

for (const [key, value] of Object.entries(src)) {
if (key === '__proto__' || key === 'constructor') {
continue;
}

if (Array.isArray(value)) {
if (target[key] && !Array.isArray(target[key])) {
throw new Error(`Trying to merge array [${value}] into a non-array '${target[key]}'`);
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-ec2/test/cfn-init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -660,7 +660,7 @@ function dontExpectLine(lines: string[], re: RegExp) {
}

function cmdArg(command: string, argument: string) {
return new RegExp(`${escapeRegex(command)}(\.exe)? .*${escapeRegex(argument)}`);
return new RegExp(`${escapeRegex(command)}(\\.exe)? .*${escapeRegex(argument)}`);
}

function escapeRegex(s: string) {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-ecs/lib/images/repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ContainerImage, ContainerImageConfig } from '../container-image';
* Regex pattern to check if it is an ECR image URL.
*
*/
const ECR_IMAGE_REGEX = /(^[a-zA-Z0-9][a-zA-Z0-9-_]*).dkr.ecr.([a-zA-Z0-9][a-zA-Z0-9-_]*).amazonaws.com(.cn)?\/.*/;
const ECR_IMAGE_REGEX = /(^[a-zA-Z0-9][a-zA-Z0-9-_]*)\.dkr\.ecr\.([a-zA-Z0-9][a-zA-Z0-9-_]*)\.amazonaws.com(\.cn)?\/.*/;

/**
* The properties for an image hosted in a public or private repository.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def helm_handler(event, context):

def get_oci_cmd(repository, version):
# Generates OCI command based on pattern. Public ECR vs Private ECR are treated differently.
private_ecr_pattern = 'oci://(?P<registry>\d+.dkr.ecr.(?P<region>[a-z0-9\-]+).amazonaws.com)*'
public_ecr_pattern = 'oci://(?P<registry>public.ecr.aws)*'
private_ecr_pattern = 'oci://(?P<registry>\d+\.dkr\.ecr\.(?P<region>[a-z0-9\-]+)\.amazonaws\.com)*'
public_ecr_pattern = 'oci://(?P<registry>public\.ecr\.aws)*'

private_registry = re.match(private_ecr_pattern, repository).groupdict()
public_registry = re.match(public_ecr_pattern, repository).groupdict()
Expand All @@ -115,7 +115,7 @@ def get_oci_cmd(repository, version):
elif public_registry['registry'] is not None:
logger.info("Found AWS public repository, will use default region as deployment")
region = os.environ.get('AWS_REGION', 'us-east-1')

if is_ecr_public_available(region):
cmnd = [
f"aws ecr-public get-login-password --region us-east-1 | " \
Expand All @@ -124,7 +124,7 @@ def get_oci_cmd(repository, version):
else:
# `aws ecr-public get-login-password` and `helm registry login` not required as ecr public is not available in current region
# see https://helm.sh/docs/helm/helm_registry_login/
cmnd = [f"helm pull {repository} --version {version} --untar"]
cmnd = [f"helm pull {repository} --version {version} --untar"]
else:
logger.error("OCI repository format not recognized, falling back to helm pull")
cmnd = [f"helm pull {repository} --version {version} --untar"]
Expand All @@ -144,7 +144,7 @@ def get_chart_from_oci(tmpdir, repository = None, version = None):
output = subprocess.check_output(cmnd, stderr=subprocess.STDOUT, cwd=tmpdir, shell=True)
logger.info(output)

# effectively returns "$tmpDir/$lastPartOfOCIUrl", because this is how helm pull saves OCI artifact.
# effectively returns "$tmpDir/$lastPartOfOCIUrl", because this is how helm pull saves OCI artifact.
# Eg. if we have oci://9999999999.dkr.ecr.us-east-1.amazonaws.com/foo/bar/pet-service repository, helm saves artifact under $tmpDir/pet-service
return os.path.join(tmpdir, repository.rpartition('/')[-1])
except subprocess.CalledProcessError as exc:
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/aws-events-targets/lib/log-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class CloudWatchLogGroup implements events.IRuleTarget {
if (typeof(resolvedTemplate) === 'string') {
// need to add the quotes back to the string so that we can parse the json
// '{"timestamp": <time>}' -> '{"timestamp": "<time>"}'
const quotedTemplate = resolvedTemplate.replace(new RegExp('(\<.*?\>)', 'g'), '"$1"');
const quotedTemplate = resolvedTemplate.replace(new RegExp('(<[^<>]*?>)', 'g'), '"$1"');
try {
const inputTemplate = JSON.parse(quotedTemplate);
const inputTemplateKeys = Object.keys(inputTemplate);
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/core/lib/cfn-mapping.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ export class CfnMapping extends CfnRefElement {
* Sets a value in the map based on the two keys.
*/
public setValue(key1: string, key2: string, value: any) {
if ([key1, key2].some(k => ['__proto__', 'constructor'].includes(k))) {
throw new Error('Cannot use \'__proto__\' or \'constructor\' as keys');
}

this.validateAlphanumeric(key2);

if (!(key1 in this.mapping)) {
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk-lib/core/lib/cfn-resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,10 @@ function deepMerge(target: any, ...sources: any[]) {
}

for (const key of Object.keys(source)) {
if (key === '__proto__' || key === 'constructor') {
continue;
}

const value = source[key];
if (typeof(value) === 'object' && value != null && !Array.isArray(value)) {
// if the value at the target is not an object, override it with an
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/core/lib/context-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ function extractProviderError(value: any): string | undefined {
* than that \ is going to lead to quoting hell when the keys are stored in JSON.
*/
function colonQuote(xs: string): string {
return xs.replace('$', '$$').replace(':', '$:');
return xs.replace(/\$/g, '$$').replace(/:/g, '$:');
}

function propsToArray(props: {[key: string]: any}, keyPrefix = ''): string[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,7 @@ const validationReport = (data: ValidationReportData[]) => {
expect.stringMatching(new RegExp(`Plugin: ${d.pluginName}`)),
expect.stringMatching(new RegExp(`Version: ${d.version ?? 'N/A'}`)),
expect.stringMatching(new RegExp(`Status: ${d.status}`)),
expect.stringMatching(new RegExp('\(Violations\)')),
expect.stringMatching(new RegExp('\\(Violations\\)')),
title,
...d.severity ? [expect.stringMatching(new RegExp(`Severity: ${d.severity}`))] : [],
expect.stringMatching(new RegExp(' Occurrences:')),
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/import.ts
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ export class ResourceImporter {
const defaultValue = typeof resourceProps[idProp] ?? '';

const prompt = [
promptPattern.replace(/%/, chalk.blue(idProp)),
promptPattern.replace(/%/g, chalk.blue(idProp)),
defaultValue
? `[${defaultValue}]`
: '(empty to skip)',
Expand Down
4 changes: 4 additions & 0 deletions packages/aws-cdk/lib/util/objects.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ export function deepSet(x: any, path: string[], value: any) {
export function deepMerge(...objects: Array<Obj<any> | undefined>) {
function mergeOne(target: Obj<any>, source: Obj<any>) {
for (const key of Object.keys(source)) {
if (key === '__proto__' || key === 'constructor') {
continue;
}

const value = source[key];

if (isObject(value)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/test/commands/migrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe('Migrate Function Tests', () => {

// Replaced stack file is referenced correctly in app file
const app = fs.readFileSync(path.join(workDir, 'GoodJava', 'src', 'main', 'java', 'com', 'myorg', 'GoodJavaApp.java'), 'utf8').split('\n');
expect(app.map(line => line.match('public class GoodJavaApp \{')).filter(line => line).length).toEqual(1);
expect(app.map(line => line.match('public class GoodJavaApp {')).filter(line => line).length).toEqual(1);
expect(app.map(line => line.match(/ new GoodJavaStack\(app, "GoodJava", StackProps.builder()/)).filter(line => line).length).toEqual(1);

// Replaced stack file is correctly generated
Expand Down
2 changes: 1 addition & 1 deletion packages/cdk-assets/lib/private/shell.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function hasAnyChars(...chars: string[]): (x: string) => boolean {
*/
function posixEscape(x: string) {
// Turn ' -> '"'"'
x = x.replace("'", "'\"'\"'");
x = x.replace(/'/g, "'\"'\"'");
return `'${x}'`;
}

Expand Down
2 changes: 1 addition & 1 deletion tools/@aws-cdk/cdk-build-tools/lib/os.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ function hasAnyChars(...chars: string[]): (x: string) => boolean {
*/
function posixEscape(x: string) {
// Turn ' -> '"'"'
x = x.replace("'", "'\"'\"'");
x = x.replace(/'/g, "'\"'\"'");
return `'${x}'`;
}

Expand Down
2 changes: 1 addition & 1 deletion tools/@aws-cdk/pkglint/lib/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1857,7 +1857,7 @@ function isIncludedInMonolith(pkg: PackageJson): boolean {
}

function beginEndRegex(label: string) {
return new RegExp(`(<\!--BEGIN ${label}-->)([\s\S]+)(<\!--END ${label}-->)`, 'm');
return new RegExp(`(<\!--BEGIN ${label}-->)([\\s\\S]+)(<\!--END ${label}-->)`, 'm');
}

function readIfExists(filename: string): string | undefined {
Expand Down

0 comments on commit 7d0f32f

Please sign in to comment.