Skip to content

Align alphaNumericRegex() with the documented grouped SHA pattern #21838

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.

Sign up for GitHub

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

Open
Copilot wants to merge 8 commits into
base: main
Choose a base branch
from
+40 −5
Open

Align alphaNumericRegex() with the documented grouped SHA pattern #21838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ef1bde7
Widen pinned SHA regex to support SHA-256 (64-char hex) and add tests
Copilot May 12, 2026
48b1dad
Add change note for SHA-256 pinned actions support
Copilot May 12, 2026
0620d34
Update Bash alphaNumericRegex to match grouped quantified forms
Copilot May 12, 2026
562f415
Tidy Bash alphaNumericRegex comment spacing
Copilot May 12, 2026
2067113
Update expected test output
owen-mc May 12, 2026
f582680
Add change note for alphanumeric regex change
owen-mc May 12, 2026
ea29986
Fix non-US english by using "parentheses" instead of "brackets"
owen-mc May 12, 2026
b49b8ff
Give slightly more detail in change note
owen-mc May 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion actions/ql/examples/snippets/uses_pinned_sha.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@
import actions

from UsesStep uses
where uses.getVersion().regexpMatch("^[A-Fa-f0-9]{40}$")
where uses.getVersion().regexpMatch("^[A-Fa-f0-9]{40}([A-Fa-f0-9]{24})?$")
select uses, "This 'uses' step has a pinned SHA version."
4 changes: 4 additions & 0 deletions actions/ql/lib/change-notes/2026-05-12-improved-alphanumeric-regex.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The GitHub Actions analysis now recognizes more Bash regex checks that restrict a value to alphanumeric characters, include regexes like `^[0-9a-zA-Z]{40}([0-9a-zA-Z]{24})?$` which check for a sha1 or sha256 hash. This may reduce false positive results where command output is validated with grouped or optional alphanumeric patterns before being used.
19 changes: 17 additions & 2 deletions actions/ql/lib/codeql/actions/Bash.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -785,7 +785,22 @@ module Bash {

/**
* Holds if the given regex is used to match an alphanumeric string
* eg: `^[0-9a-zA-Z]{40}$`, `^[0-9]+$` or `^[a-zA-Z0-9_]+$`
* eg: `^[0-9a-zA-Z]{40}([0-9a-zA-Z]{24})?$`, `^[0-9]+$` or `^[a-zA-Z0-9_]+$`
*/
string alphaNumericRegex() { result = "^\\^\\[([09azAZ_-]+)\\](\\+|\\{\\d+\\})\\$$" }
string alphaNumericRegex() {
exists(string r1, string r2, string r3, string r4 |
// An alphanumeric character class
r1 = "\\[([09azAZ_-]+)\\]" and
// The same as above, followed by a quantifier like `+` or `{20}`
r2 = r1 + "(\\+|\\{\\d+\\})" and
// The same as above, possibly with parentheses around it
r3 = "\\(?" + r2 + "\\)?" and
Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel May 13, 2026

Choose a reason for hiding this comment

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

What if there is only an open or close parenthesis?

Copy link
Copy Markdown
Contributor

@owen-mc owen-mc May 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Wouldn't that make it a malformed regex? I guess you could have just a close parenthesis in a later one.

Do you think this would be better?

Suggested change
r3 = "\\(?" + r2 + "\\)?" and
r3 = "(" + r2 + "|\\(" + r2 + "\\)" and

If I do that I should probably include the \\?? at the end, just for the second option, since that should only come after parentheses (a ? directly after a quantifier it makes the quantifier lazy/non-greedy, rather than expressing optionality).

Copy link
Copy Markdown
Contributor

@michaelnebel michaelnebel May 13, 2026

Choose a reason for hiding this comment

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

Yes, then it would be malformed.
Maybe just keep the code as it is.

// The same as above, possibly with a `?` after it
r4 = r3 + "\\??"
|
// The same as above, repeated one or more times, and with `^` at the
// beginning and `$` at the end
result = "^\\^(" + r4 + ")+\\$$"
)
}
}
4 changes: 3 additions & 1 deletion actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import actions
import codeql.actions.security.UseOfUnversionedImmutableAction

bindingset[version]
private predicate isPinnedCommit(string version) { version.regexpMatch("^[A-Fa-f0-9]{40}$") }
private predicate isPinnedCommit(string version) {
version.regexpMatch("^[A-Fa-f0-9]{40}([A-Fa-f0-9]{24})?$")
}

bindingset[nwo]
private predicate isTrustedOwner(string nwo) {
Expand Down
4 changes: 4 additions & 0 deletions actions/ql/src/change-notes/2026-05-12-sha256-pinned-actions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `actions/unpinned-tag` query now recognizes 64-character SHA-256 commit hashes as properly pinned references, in addition to 40-character SHA-1 hashes.
6 changes: 6 additions & 0 deletions actions/ql/test/query-tests/Security/CWE-829/.github/workflows/unpinned_tags.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,9 @@ jobs:
- uses: foo/bar@25b062c917b0c75f8b47d8469aff6c94ffd89abb
- uses: docker://foo/bar@latest
- uses: docker://foo/bar@sha256:887a259a5a534f3c4f36cb02dca341673c6089431057242cdc931e9f133147e9
# SHA-256 pinned (64 hex chars) - should NOT be flagged
- uses: foo/bar@25b062c917b0c75f8b47d8469aff6c94ffd89abb25b062c917b0c75f8b47d84d
# SHA-1 pinned (40 hex chars) regression - should NOT be flagged
- uses: foo/bar@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2
# Invalid 50-char hex string - should be flagged
- uses: foo/bar@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5
1 change: 1 addition & 0 deletions actions/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@
| .github/workflows/test18.yml:37:21:37:63 | sonarsource/sonarcloud-github-action@master | Unpinned 3rd party Action 'Sonar' step $@ uses 'sonarsource/sonarcloud-github-action' with ref 'master', not a pinned commit hash | .github/workflows/test18.yml:36:15:40:58 | Uses Step | Uses Step |
| .github/workflows/unpinned_tags.yml:10:13:10:22 | foo/bar@v1 | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'v1', not a pinned commit hash | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Uses Step |
| .github/workflows/unpinned_tags.yml:12:13:12:35 | docker://foo/bar@latest | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'docker://foo/bar' with ref 'latest', not a pinned commit hash | .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step | Uses Step |
| .github/workflows/unpinned_tags.yml:19:13:19:70 | foo/bar@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5 | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5', not a pinned commit hash | .github/workflows/unpinned_tags.yml:19:7:19:71 | Uses Step | Uses Step |
5 changes: 4 additions & 1 deletion actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,10 @@ edges
| .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: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/unpinned_tags.yml:12:7:13:4 | Uses Step | .github/workflows/unpinned_tags.yml:13:7:15:4 | Uses Step |
| .github/workflows/unpinned_tags.yml:13:7:15:4 | Uses Step | .github/workflows/unpinned_tags.yml:15:7:17:4 | Uses Step |
| .github/workflows/unpinned_tags.yml:15:7:17:4 | Uses Step | .github/workflows/unpinned_tags.yml:17:7:19:4 | Uses Step |
| .github/workflows/unpinned_tags.yml:17:7:19:4 | Uses Step | .github/workflows/unpinned_tags.yml:19:7:19:71 | 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 |
Expand Down
Loading