Skip to content

Fix docker SHA false positive #18785

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

Merged
merged 9 commits into from
Feb 14, 2025
Merged

Conversation

martincostello
Copy link
Contributor

Fix false positives for pinned Docker container images.

Resolves #18782.

Fix false positives for pinned Docker container images.
Fix various issues with the query.
@Copilot Copilot AI review requested due to automatic review settings February 14, 2025 13:01
@martincostello martincostello requested a review from a team as a code owner February 14, 2025 13:01
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 3 changed files in this pull request and generated no comments.

Files not reviewed (2)
  • actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql: Language not supported
  • actions/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected: Language not supported

@github-actions github-actions bot added the Actions Analysis of GitHub Actions label Feb 14, 2025
martincostello and others added 2 commits February 14, 2025 13:27
Add change note.
Apply code review suggestion.

Co-Authored-By: Taus <1104778+tausbn@users.noreply.github.com>
Makes for a neater diff.
Forgot to move the `and`.
Copy link
Collaborator

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

This looks good - thank you for the contribution! Minor suggestions, and then I'll kick off an internal CI run.

Co-authored-by: Aditya Sharad <6874315+adityasharad@users.noreply.github.com>
@adityasharad
Copy link
Collaborator

Ah the check failure is from autoformatting.
If you're using the CodeQL extension for VS Code, could you please run Format Document from the Command Palette, and if you're running the CodeQL CLI locally, could you please run codeql query format actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql, and commit the changes?

(If you are doing neither and just editing these directly in the UI, then we can make that change too.)

Fix lint warning.
@martincostello
Copy link
Contributor Author

I can't see what test(s) are failing, but the test I amended is passing for me locally.

@adityasharad
Copy link
Collaborator

Looks like a test for a different query, but that was sharing test data and so is picking up the new test code changes introduced in this PR. Apologies -- for Actions it looks like we don't have a clean separation of our test cases between queries. All other checks look good.

Try applying this to actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected (and since you can run tests locally, rerunning the test actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.qlref will let you validate it's working).

@@ -299,7 +299,9 @@
 | .github/workflows/test.yml:14:9:25:6 | Run Step | .github/workflows/test.yml:25:9:33:6 | Run Step |
 | .github/workflows/test.yml:25:9:33:6 | Run Step | .github/workflows/test.yml:33:9:37:34 | Run Step |
 | .github/workflows/unpinned_tags.yml:9:7:10:4 | Uses Step | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step |
-| .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | .github/workflows/unpinned_tags.yml:11:7:11:61 | Uses Step |
+| .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | .github/workflows/unpinned_tags.yml:11:7:12:4 | Uses Step |
+| .github/workflows/unpinned_tags.yml:11:7:12:4 | Uses Step | .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step |
+| .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step | .github/workflows/unpinned_tags.yml:13:7:13:101 | Uses Step |
 | .github/workflows/untrusted_checkout2.yml:7:9:14:6 | Run Step: pr_number | .github/workflows/untrusted_checkout2.yml:14:9:19:72 | Run Step |
 | .github/workflows/untrusted_checkout3.yml:11:9:12:6 | Uses Step | .github/workflows/untrusted_checkout3.yml:12:9:13:6 | Uses Step |
 | .github/workflows/untrusted_checkout3.yml:12:9:13:6 | Uses Step | .github/actions/dangerous-git-checkout/action.yml:6:7:11:4 | Uses Step |

Fix failing test.
@martincostello
Copy link
Contributor Author

Thanks for your help @adityasharad - looks like it's all passing now.

Copy link
Collaborator

@adityasharad adityasharad left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks again for taking the time to learn the CodeQL tools and help us improve this new analysis.

@adityasharad adityasharad merged commit 5f4871d into github:main Feb 14, 2025
11 checks passed
@martincostello martincostello deleted the gh-18782 branch February 14, 2025 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Actions Analysis of GitHub Actions documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive for GitHub action using a Docker image pinned by sha256
3 participants