-
Notifications
You must be signed in to change notification settings - Fork 351
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
Fix code injection warnings in check-codescanning-config
internal Action
#2781
Conversation
`runner.temp` is not user-controlled but we replace it with `$RUNNER_TEMP` in any case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR fixes code injection warnings in the check-codescanning-config Action by switching to environment variables and safer temporary directory references.
- Replace user-supplied inline inputs with environment variables for improved security.
- Update temporary directory references to use $RUNNER_TEMP.
Reviewed Changes
File | Description |
---|---|
.github/actions/check-codescanning-config/action.yml | Updated to use environment variable EXPECTED_CONFIG_FILE_CONTENTS and $RUNNER_TEMP for file paths in commands and cleanup steps. |
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See copilot's comment.
Oh...now it looks like the tests need to be updated. I'm not sure if this is a semantic change or just some artifact of how the quoting has changed. |
Hrm.. looks like the |
0bbfcad
to
e12eb8d
Compare
Okay, figured it out... I had declared the environment variable in the wrong block 🤦 |
EXPECTED_CONFIG_FILE_CONTENTS
to escape any injected input$RUNNER_TEMP
rather than${{ runner.temp }}
for good measure, even thoughrunner.temp
should not be user-controlled input anyway.Merge / deployment checklist