From f4080b5f929d0df719fed29930728be594c4efb8 Mon Sep 17 00:00:00 2001 From: Bill Napier Date: Mon, 31 Mar 2025 19:30:45 +0000 Subject: [PATCH 1/6] Add semgrep tests for pinned actions. This will look at all workflow files and look at: ``` uses: org/repo@v5 ``` and encourage users to instead pin those to git commits. --- .../actions-need-pinned-commits.md | 27 +++++++++++++++++++ .../actions/actions_need_pinned_commits.yaml | 18 +++++++++++++ .../actions_need_pinned_commits.test.yaml | 17 ++++++++++++ 3 files changed, 62 insertions(+) create mode 100644 docs/semgrep-rules/actions-need-pinned-commits.md create mode 100644 semgrep-rules/actions/actions_need_pinned_commits.yaml create mode 100644 semgrep-tests/actions/actions_need_pinned_commits.test.yaml diff --git a/docs/semgrep-rules/actions-need-pinned-commits.md b/docs/semgrep-rules/actions-need-pinned-commits.md new file mode 100644 index 0000000..40ffb13 --- /dev/null +++ b/docs/semgrep-rules/actions-need-pinned-commits.md @@ -0,0 +1,27 @@ +# actions-need-pinned-commits + +For actions that look like: + +``` +uses: actions/checkout@v4 +``` + +GitHub uses the underlying git label v4 to fetch the action to run. As seen in the [tj-actions/changed-files](https://semgrep.dev/blog/2025/popular-github-action-tj-actionschanged-files-is-compromised/) vulnerabilty, these lables are not immutable and trivially changeable. So what you think is a nice stable safe version, an attacker has changed behind your back to something nefarious. + +We are strongly encouraging use to use the full git commit hash instead to prevent this type of attacks. + +## Ratchet + +[Ratchet](https://github.com/sethvargo/ratchet) provides an easy way to do this. + +You can pin all your workflow files like this: + +``` +ratchet pin .github/workflows/* +``` + +And upgrade them (as needed, under your control and not someone elses): + +``` +ratchet upgrade .github/workflows/action_to_upgrade.yml +``` diff --git a/semgrep-rules/actions/actions_need_pinned_commits.yaml b/semgrep-rules/actions/actions_need_pinned_commits.yaml new file mode 100644 index 0000000..fd69de1 --- /dev/null +++ b/semgrep-rules/actions/actions_need_pinned_commits.yaml @@ -0,0 +1,18 @@ +rules: + - id: actions-need-pinned-commits + languages: + - yaml + severity: WARNING + message: 'Referencing an action to run by git tag is risky, due to the mutability of git tags. Prefer + to use full git SHAs instead. ' + metadata: + category: best-practice + technology: + - github-actions + patterns: + - pattern-either: + - patterns: + - pattern-inside: "{steps: ...}" + # Match all uses patterns that don't contain the full SHA1 hash. Yes, short hashes exist but suffer from a similar (but slightly harder) attack vector by purposely crafting a colliding SHA. + - pattern: "uses: ..." + - pattern-not-regex: ".*@[0-9A-Fa-f]{40}" diff --git a/semgrep-tests/actions/actions_need_pinned_commits.test.yaml b/semgrep-tests/actions/actions_need_pinned_commits.test.yaml new file mode 100644 index 0000000..b9737f8 --- /dev/null +++ b/semgrep-tests/actions/actions_need_pinned_commits.test.yaml @@ -0,0 +1,17 @@ +name: 'Test Actions Needing Pinned Commits' +on: + pull_request: +jobs: + do-stuff: + steps: + - name: 'Step 1' + # ruleid: actions-need-pinned-commits + uses: actions/checkout@v4 + - name: 'Step 2' + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # ratchet:actions/checkout@v4 + - name: 'Step 3' + # ruleid: actions-need-pinned-commits + uses: 'actions/checkout@11bd719' + - name: 'Step 4' + # ruleid: actions-need-pinned-commits + uses: 'actions/checkout@my_git_label' From 2145ea17912c0fb31b7709ef90797787295a843e Mon Sep 17 00:00:00 2001 From: Bill Napier Date: Mon, 31 Mar 2025 20:16:55 +0000 Subject: [PATCH 2/6] Fix mdformat. --- docs/semgrep-rules/actions-need-pinned-commits.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/docs/semgrep-rules/actions-need-pinned-commits.md b/docs/semgrep-rules/actions-need-pinned-commits.md index 40ffb13..7915ec5 100644 --- a/docs/semgrep-rules/actions-need-pinned-commits.md +++ b/docs/semgrep-rules/actions-need-pinned-commits.md @@ -6,9 +6,13 @@ For actions that look like: uses: actions/checkout@v4 ``` -GitHub uses the underlying git label v4 to fetch the action to run. As seen in the [tj-actions/changed-files](https://semgrep.dev/blog/2025/popular-github-action-tj-actionschanged-files-is-compromised/) vulnerabilty, these lables are not immutable and trivially changeable. So what you think is a nice stable safe version, an attacker has changed behind your back to something nefarious. +GitHub uses the underlying git label v4 to fetch the action to run. As seen in the +[tj-actions/changed-files](https://semgrep.dev/blog/2025/popular-github-action-tj-actionschanged-files-is-compromised/) +vulnerabilty, these lables are not immutable and trivially changeable. So what you think is a nice +stable safe version, an attacker has changed behind your back to something nefarious. -We are strongly encouraging use to use the full git commit hash instead to prevent this type of attacks. +We are strongly encouraging use to use the full git commit hash instead to prevent this type of +attacks. ## Ratchet From e1be33ab3f8a05739dc0556a0953c2a740c72be3 Mon Sep 17 00:00:00 2001 From: Bill Napier Date: Mon, 31 Mar 2025 20:18:43 +0000 Subject: [PATCH 3/6] Use ratchet to pin all our action usage references. --- .github/workflows/markdown_format.yml | 2 +- .github/workflows/publish_docs.yml | 6 +++--- .github/workflows/publish_docs_preview.yml | 6 +++--- .github/workflows/yaml_format.yml | 2 +- tools/ratchet | 2 ++ 5 files changed, 10 insertions(+), 8 deletions(-) create mode 100755 tools/ratchet diff --git a/.github/workflows/markdown_format.yml b/.github/workflows/markdown_format.yml index afd17b6..1d6d96e 100644 --- a/.github/workflows/markdown_format.yml +++ b/.github/workflows/markdown_format.yml @@ -10,6 +10,6 @@ jobs: runs-on: 'ubuntu-latest' steps: - name: 'Checkout Code' - uses: 'actions/checkout@v4' + uses: 'actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683' # ratchet:actions/checkout@v4 - name: 'Check Markdown Format' run: 'tools/mdformat --check --wrap 100 .' diff --git a/.github/workflows/publish_docs.yml b/.github/workflows/publish_docs.yml index c446cd1..70217c9 100644 --- a/.github/workflows/publish_docs.yml +++ b/.github/workflows/publish_docs.yml @@ -15,16 +15,16 @@ jobs: name: "Build and Deploy Docs" runs-on: 'ubuntu-latest' steps: - - uses: 'actions/checkout@v4' + - uses: 'actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683' # ratchet:actions/checkout@v4 - name: 'Generate HTML from Markdown' - uses: 'ldeluigi/markdown-docs@latest' + uses: 'ldeluigi/markdown-docs@e51f9a21070db6a21dfbe27ac92f065823e006a1' # ratchet:ldeluigi/markdown-docs@latest with: src: 'docs' dst: 'generated-pages' - name: 'Install rsync' run: 'apt-get update && apt-get install -y rsync' - name: 'Deploy Docs' - uses: JamesIves/github-pages-deploy-action@v4 + uses: JamesIves/github-pages-deploy-action@6c2d9db40f9296374acc17b90404b6e8864128c8 # ratchet:JamesIves/github-pages-deploy-action@v4 with: folder: generated-pages force: false diff --git a/.github/workflows/publish_docs_preview.yml b/.github/workflows/publish_docs_preview.yml index 13d6e3a..3fc9556 100644 --- a/.github/workflows/publish_docs_preview.yml +++ b/.github/workflows/publish_docs_preview.yml @@ -14,13 +14,13 @@ jobs: name: "Build PR Preview Docs" runs-on: 'ubuntu-latest' steps: - - uses: 'actions/checkout@v4' + - uses: 'actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683' # ratchet:actions/checkout@v4 - name: 'Generate HTML from Markdown' - uses: 'ldeluigi/markdown-docs@latest' + uses: 'ldeluigi/markdown-docs@e51f9a21070db6a21dfbe27ac92f065823e006a1' # ratchet:ldeluigi/markdown-docs@latest with: src: 'docs' dst: 'generated-pages' - name: 'Deploy GitHub Pages Preview' - uses: rossjrw/pr-preview-action@v1 + uses: rossjrw/pr-preview-action@df22037db54ab6ee34d3c1e2b8810ac040a530c6 # ratchet:rossjrw/pr-preview-action@v1 with: source-dir: './generated-pages/' diff --git a/.github/workflows/yaml_format.yml b/.github/workflows/yaml_format.yml index efc5017..7021be4 100644 --- a/.github/workflows/yaml_format.yml +++ b/.github/workflows/yaml_format.yml @@ -11,6 +11,6 @@ jobs: runs-on: 'ubuntu-latest' steps: - name: 'Checkout Code' - uses: 'actions/checkout@v4' + uses: 'actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683' # ratchet:actions/checkout@v4 - name: 'Check Yaml Format' run: 'tools/yamlfmt --lint .' diff --git a/tools/ratchet b/tools/ratchet new file mode 100755 index 0000000..4bc8de7 --- /dev/null +++ b/tools/ratchet @@ -0,0 +1,2 @@ +#!/bin/sh +docker run -it --rm -v "${PWD}:${PWD}" -w "${PWD}" ghcr.io/sethvargo/ratchet:latest $* \ No newline at end of file From 74634debacde7ebc4dca7f6c7566ff977bd9aec4 Mon Sep 17 00:00:00 2001 From: Bill Napier Date: Mon, 31 Mar 2025 20:21:38 +0000 Subject: [PATCH 4/6] Fix message to also include link to docs. --- semgrep-rules/actions/actions_need_pinned_commits.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semgrep-rules/actions/actions_need_pinned_commits.yaml b/semgrep-rules/actions/actions_need_pinned_commits.yaml index fd69de1..5f24f5e 100644 --- a/semgrep-rules/actions/actions_need_pinned_commits.yaml +++ b/semgrep-rules/actions/actions_need_pinned_commits.yaml @@ -4,7 +4,7 @@ rules: - yaml severity: WARNING message: 'Referencing an action to run by git tag is risky, due to the mutability of git tags. Prefer - to use full git SHAs instead. ' + to use full git SHAs instead. More information: https://google.github.io/github-team/semgrep-rules/actions-need-pinned-commits.html' metadata: category: best-practice technology: From 185ddcfa020e24a7cb65afa57836ce6df06ee80a Mon Sep 17 00:00:00 2001 From: Bill Napier Date: Mon, 31 Mar 2025 20:24:27 +0000 Subject: [PATCH 5/6] Add additional test case. --- semgrep-rules/actions/actions_need_pinned_commits.yaml | 2 +- semgrep-tests/actions/actions_need_pinned_commits.test.yaml | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/semgrep-rules/actions/actions_need_pinned_commits.yaml b/semgrep-rules/actions/actions_need_pinned_commits.yaml index 5f24f5e..bacec3a 100644 --- a/semgrep-rules/actions/actions_need_pinned_commits.yaml +++ b/semgrep-rules/actions/actions_need_pinned_commits.yaml @@ -15,4 +15,4 @@ rules: - pattern-inside: "{steps: ...}" # Match all uses patterns that don't contain the full SHA1 hash. Yes, short hashes exist but suffer from a similar (but slightly harder) attack vector by purposely crafting a colliding SHA. - pattern: "uses: ..." - - pattern-not-regex: ".*@[0-9A-Fa-f]{40}" + - pattern-not-regex: ".*@[0-9A-Fa-f]{40}.*" diff --git a/semgrep-tests/actions/actions_need_pinned_commits.test.yaml b/semgrep-tests/actions/actions_need_pinned_commits.test.yaml index b9737f8..9978546 100644 --- a/semgrep-tests/actions/actions_need_pinned_commits.test.yaml +++ b/semgrep-tests/actions/actions_need_pinned_commits.test.yaml @@ -8,7 +8,9 @@ jobs: # ruleid: actions-need-pinned-commits uses: actions/checkout@v4 - name: 'Step 2' - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # ratchet:actions/checkout@v4 + uses: 'actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683' + - name: 'Step 2.5' + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # ratchet:actions/checkout@v4 - name: 'Step 3' # ruleid: actions-need-pinned-commits uses: 'actions/checkout@11bd719' From 5cf2956dba62e52f98facc7f781cf2d1ffe20002 Mon Sep 17 00:00:00 2001 From: Bill Napier Date: Mon, 31 Mar 2025 20:25:46 +0000 Subject: [PATCH 6/6] Fix yaml format. --- semgrep-tests/actions/actions_need_pinned_commits.test.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/semgrep-tests/actions/actions_need_pinned_commits.test.yaml b/semgrep-tests/actions/actions_need_pinned_commits.test.yaml index 9978546..2349d21 100644 --- a/semgrep-tests/actions/actions_need_pinned_commits.test.yaml +++ b/semgrep-tests/actions/actions_need_pinned_commits.test.yaml @@ -10,7 +10,7 @@ jobs: - name: 'Step 2' uses: 'actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683' - name: 'Step 2.5' - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # ratchet:actions/checkout@v4 + uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # ratchet:actions/checkout@v4 - name: 'Step 3' # ruleid: actions-need-pinned-commits uses: 'actions/checkout@11bd719'