-
Notifications
You must be signed in to change notification settings - Fork 0
Core 1345 support skipping image build and testing when only nomad or nomad pack change made skip test #137
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
This comment ensures that the correct Slack channel is notified after the team/project label See this comment for details. |
|
|
||
| - name: Get changed files | ||
| id: changed | ||
| uses: tj-actions/changed-files@v45 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
Uses Step: changed
| "Dockerfile" | ||
| ) | ||
| echo "🔍 Changed files:" | ||
| echo "${{ steps.changed.outputs.all_changed_files }}" |
Check warning
Code scanning / CodeQL
Code injection Medium
${ steps.changed.outputs.all_changed_files }
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix this code injection vulnerability, we should avoid placing ${{ steps.changed.outputs.all_changed_files }} directly inside shell commands. Instead, we should pass the value as an environment variable and reference it using native shell syntax ($ALL_CHANGED_FILES). Specifically, in the affected step, set up an env mapping:
env:
ALL_CHANGED_FILES: ${{ steps.changed.outputs.all_changed_files }}and then refer to $ALL_CHANGED_FILES in the shell script. This change should be made in the step named "Check if only irrelevant files changed," covering all uses of steps.changed.outputs.all_changed_files inside the run block.
The affected lines are:
- 100:
echo "${{ steps.changed.outputs.all_changed_files }}" - 123:
done <<< "$(echo "${{ steps.changed.outputs.all_changed_files }}" | tr ',' '\n')"
These should both refer to the environment variable using the shell-native form:
- On line 100, change to
echo "$ALL_CHANGED_FILES" - On line 123, change to
done <<< "$(echo "$ALL_CHANGED_FILES" | tr ',' '\n')"
We will also add the appropriate env mapping in the step definition.
-
Copy modified lines R83-R84 -
Copy modified line R102 -
Copy modified line R125
| @@ -80,6 +80,8 @@ | ||
|
|
||
| - name: Check if only irrelevant files changed | ||
| id: check | ||
| env: | ||
| ALL_CHANGED_FILES: ${{ steps.changed.outputs.all_changed_files }} | ||
| run: | | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
| @@ -97,7 +99,7 @@ | ||
| "Dockerfile" | ||
| ) | ||
| echo "🔍 Changed files:" | ||
| echo "${{ steps.changed.outputs.all_changed_files }}" | ||
| echo "$ALL_CHANGED_FILES" | ||
| echo | ||
|
|
||
| ONLY_IRRELEVANT=true | ||
| @@ -120,7 +122,7 @@ | ||
| ONLY_IRRELEVANT=false | ||
| break | ||
| fi | ||
| done <<< "$(echo "${{ steps.changed.outputs.all_changed_files }}" | tr ',' '\n')" | ||
| done <<< "$(echo "$ALL_CHANGED_FILES" | tr ',' '\n')" | ||
|
|
||
| echo "🔎 Result: only_irrelevant=$ONLY_IRRELEVANT" | ||
| echo "only_irrelevant=$ONLY_IRRELEVANT" >> "$GITHUB_OUTPUT" |
| ONLY_IRRELEVANT=false | ||
| break | ||
| fi | ||
| done <<< "$(echo "${{ steps.changed.outputs.all_changed_files }}" | tr ',' '\n')" |
Check warning
Code scanning / CodeQL
Code injection Medium
${ steps.changed.outputs.all_changed_files }
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the issue, we need to avoid interpolating the untrusted input using ${{ ... }} directly in the shell context inside the run: block. The best practice is to pass the value to the shell via an intermediate environment variable (set via env:), and then reference it using $VARIABLE.
Steps to fix:
- In the affected step (
Check if only irrelevant files changed), underenv:, add a variable likeALL_CHANGED_FILES: ${{ steps.changed.outputs.all_changed_files }}. - In the shell script, replace direct usage of
${{ steps.changed.outputs.all_changed_files }}with$ALL_CHANGED_FILES. - Specifically, the line:
should become:done <<< "$(echo "${{ steps.changed.outputs.all_changed_files }}" | tr ',' '\n')"done <<< "$(echo "$ALL_CHANGED_FILES" | tr ',' '\n')" - Also, any other echo or usage of the same variable via
${{ ... }}should be updated to$ALL_CHANGED_FILESfor consistency and safety.
No additional libraries are needed. You only need to add the environment variable definition and reference it using the shell’s $ syntax inside the script.
-
Copy modified lines R83-R84 -
Copy modified line R102 -
Copy modified line R125
| @@ -80,6 +80,8 @@ | ||
|
|
||
| - name: Check if only irrelevant files changed | ||
| id: check | ||
| env: | ||
| ALL_CHANGED_FILES: ${{ steps.changed.outputs.all_changed_files }} | ||
| run: | | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
| @@ -97,7 +99,7 @@ | ||
| "Dockerfile" | ||
| ) | ||
| echo "🔍 Changed files:" | ||
| echo "${{ steps.changed.outputs.all_changed_files }}" | ||
| echo "$ALL_CHANGED_FILES" | ||
| echo | ||
|
|
||
| ONLY_IRRELEVANT=true | ||
| @@ -120,7 +122,7 @@ | ||
| ONLY_IRRELEVANT=false | ||
| break | ||
| fi | ||
| done <<< "$(echo "${{ steps.changed.outputs.all_changed_files }}" | tr ',' '\n')" | ||
| done <<< "$(echo "$ALL_CHANGED_FILES" | tr ',' '\n')" | ||
|
|
||
| echo "🔎 Result: only_irrelevant=$ONLY_IRRELEVANT" | ||
| echo "only_irrelevant=$ONLY_IRRELEVANT" >> "$GITHUB_OUTPUT" |
https://remerge.atlassian.net/browse/CORE-1345