Skip to content

chore(cli-integ): make it possible to run on GitHub Actions#33175

Merged
mergify[bot] merged 6 commits intomainfrom
huijbers/integtest-changes-back
Jan 28, 2025
Merged

chore(cli-integ): make it possible to run on GitHub Actions#33175
mergify[bot] merged 6 commits intomainfrom
huijbers/integtest-changes-back

Conversation

@rix0rrr
Copy link
Copy Markdown
Contributor

@rix0rrr rix0rrr commented Jan 27, 2025

Migrate some changes back from the new testing repo. These changes are necessary to make the tests run on GitHub Actions.

If we keep them here, in the future we can do a cp -R on the test directory. If not, we'll have to do manual sorting on every copy over, which is annoying and easy to make mistakes in.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Migrate some changes back from the new testing repo. These changes are
necessary to make the tests run on GitHub Actions.

If we keep them here, in the future we can do a `cp -R` on the test
directory. If not, we'll have to do manual sorting on every copy
over, which is annoying and easy to make mistakes in.
@rix0rrr rix0rrr added the pr/do-not-merge This PR should not be merged at this time. label Jan 27, 2025
@rix0rrr rix0rrr requested a review from a team as a code owner January 27, 2025 11:12
@github-actions github-actions bot added the p2 label Jan 27, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team January 27, 2025 11:12
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jan 27, 2025
@rix0rrr rix0rrr changed the title chore: make integ tests run on GitHub Actions chore(cli-integ): make it possible to run on GitHub Actions Jan 27, 2025
}

export class AwsClients {
public static async default(output: NodeJS.WritableStream) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you add this back in? No tests are using it anymore.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.78%. Comparing base (8c13cf2) to head (d63265c).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #33175   +/-   ##
=======================================
  Coverage   80.78%   80.78%           
=======================================
  Files         232      232           
  Lines       14111    14111           
  Branches     2453     2453           
=======================================
  Hits        11400    11400           
  Misses       2431     2431           
  Partials      280      280           
Flag Coverage Δ
suite.unit 80.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
packages/aws-cdk 79.51% <ø> (ø)
packages/aws-cdk-lib/core 82.17% <ø> (ø)

}

child.kill('SIGINT');
child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`);

Check warning

Code scanning / CodeQL

Unsafe shell command constructed from library input

This string concatenation which depends on [library input](1) is later used in a [shell command](2). This string concatenation which depends on [library input](3) is later used in a [shell command](2). This string concatenation which depends on [library input](4) is later used in a [shell command](2).

Copilot Autofix

AI about 1 year ago

To fix the problem, we need to ensure that the user-controlled input is properly sanitized before being used in a shell command. The best way to achieve this is by using the shell-quote library to escape any special characters in the input. This will prevent shell injection vulnerabilities by ensuring that the input is treated as a literal string rather than being interpreted by the shell.

  1. Install the shell-quote library if it is not already installed.
  2. Import the shell-quote library in the file.
  3. Use the shellQuote.quote function to escape the command variable before including it in the shell command on line 290.
Suggested changeset 2
packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts b/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts
--- a/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts
+++ b/packages/@aws-cdk-testing/cli-integ/lib/with-sam.ts
@@ -4,2 +4,3 @@
 import axios from 'axios';
+import * as shellQuote from 'shell-quote';
 import { TestContext } from './integ-test';
@@ -289,3 +290,4 @@
   child.kill('SIGINT');
-  child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`);
+  const escapedCommand = shellQuote.quote([command]);
+  child_process.exec(`for pid in $(ps -ef | grep ${escapedCommand} | awk '{print $2}'); do kill -2 $pid; done`);
 }
EOF
@@ -4,2 +4,3 @@
import axios from 'axios';
import * as shellQuote from 'shell-quote';
import { TestContext } from './integ-test';
@@ -289,3 +290,4 @@
child.kill('SIGINT');
child_process.exec(`for pid in $(ps -ef | grep "${command}" | awk '{print $2}'); do kill -2 $pid; done`);
const escapedCommand = shellQuote.quote([command]);
child_process.exec(`for pid in $(ps -ef | grep ${escapedCommand} | awk '{print $2}'); do kill -2 $pid; done`);
}
packages/@aws-cdk-testing/cli-integ/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/packages/@aws-cdk-testing/cli-integ/package.json b/packages/@aws-cdk-testing/cli-integ/package.json
--- a/packages/@aws-cdk-testing/cli-integ/package.json
+++ b/packages/@aws-cdk-testing/cli-integ/package.json
@@ -69,3 +69,4 @@
     "yaml": "1.10.2",
-    "yargs": "^17.7.2"
+    "yargs": "^17.7.2",
+    "shell-quote": "^1.8.2"
   },
EOF
@@ -69,3 +69,4 @@
"yaml": "1.10.2",
"yargs": "^17.7.2"
"yargs": "^17.7.2",
"shell-quote": "^1.8.2"
},
This fix introduces these dependencies
Package Version Security advisories
shell-quote (npm) 1.8.2 None
Copilot is powered by AI and may make mistakes. Always verify output.
@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

➡️ PR build request submitted to test-main-pipeline ⬅️

A maintainer must now check the pipeline and add the pr-linter/cli-integ-tested label once the pipeline succeeds.

@rix0rrr rix0rrr added pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested and removed pr/do-not-merge This PR should not be merged at this time. labels Jan 28, 2025
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jan 28, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: d63265c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Jan 28, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit c8de5be into main Jan 28, 2025
1 of 2 checks passed
@mergify mergify bot deleted the huijbers/integtest-changes-back branch January 28, 2025 09:18
@github-actions
Copy link
Copy Markdown
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

contribution/core This is a PR that came from AWS. p2 pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants