From 58240fbb2bad5abfed5427931101a3201f810ab5 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Tue, 31 Mar 2026 15:38:26 -0700 Subject: [PATCH 1/6] Actions: Add four experimental queries inspired by Zizmor New experimental security queries for GitHub Actions: - actions/secrets-inherit (CWE-200): Flags \`secrets: inherit\` on reusable workflow calls. This passes all org/repo secrets to the callee, violating least privilege. - actions/hardcoded-container-credentials (CWE-798): Flags hardcoded passwords in \`container.credentials\` and \`services.*.credentials\`. - actions/unsound-contains (CWE-183): Flags \`contains()\` with a string-literal first argument. This does substring matching, not array membership, so \`contains('refs/heads/main', github.ref)\` also matches a branch named \`mai\`. - actions/spoofable-actor-check (CWE-290): Flags \`github.actor\` or \`github.triggering_actor\` compared to a \`[bot]\` name on externally triggerable events. \`github.actor\` refers to the last actor on the context, not the triggering actor, so an attacker can spoof it. Includes .md help files and tests for each query. --- .../Security/CWE-183/UnsoundContains.md | 42 +++++++++++++++ .../Security/CWE-183/UnsoundContains.ql | 41 +++++++++++++++ .../Security/CWE-200/SecretsInherit.md | 33 ++++++++++++ .../Security/CWE-200/SecretsInherit.ql | 26 ++++++++++ .../Security/CWE-290/SpoofableActorCheck.md | 51 ++++++++++++++++++ .../Security/CWE-290/SpoofableActorCheck.ql | 50 ++++++++++++++++++ .../CWE-798/HardcodedContainerCredentials.md | 44 ++++++++++++++++ .../CWE-798/HardcodedContainerCredentials.ql | 47 +++++++++++++++++ .../.github/workflows/unsound-contains.yml | 26 ++++++++++ .../Security/CWE-183/UnsoundContains.expected | 2 + .../Security/CWE-183/UnsoundContains.qlref | 1 + .../.github/workflows/secrets-inherit.yml | 25 +++++++++ .../Security/CWE-200/SecretsInherit.expected | 1 + .../Security/CWE-200/SecretsInherit.qlref | 1 + .../.github/workflows/bot-conditions.yml | 42 +++++++++++++++ .../CWE-290/SpoofableActorCheck.expected | 3 ++ .../CWE-290/SpoofableActorCheck.qlref | 1 + .../workflows/hardcoded-credentials.yml | 52 +++++++++++++++++++ .../HardcodedContainerCredentials.expected | 2 + .../HardcodedContainerCredentials.qlref | 1 + 20 files changed, 491 insertions(+) create mode 100644 actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md create mode 100644 actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql create mode 100644 actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md create mode 100644 actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql create mode 100644 actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md create mode 100644 actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.ql create mode 100644 actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.md create mode 100644 actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql create mode 100644 actions/ql/test/query-tests/Security/CWE-183/.github/workflows/unsound-contains.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.expected create mode 100644 actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.qlref create mode 100644 actions/ql/test/query-tests/Security/CWE-200/.github/workflows/secrets-inherit.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.expected create mode 100644 actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.qlref create mode 100644 actions/ql/test/query-tests/Security/CWE-290/.github/workflows/bot-conditions.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.expected create mode 100644 actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.qlref create mode 100644 actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.expected create mode 100644 actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.qlref diff --git a/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md b/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md new file mode 100644 index 000000000000..4731bc5e8ae8 --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md @@ -0,0 +1,42 @@ +## Overview + +The GitHub Actions `contains()` function behaves differently depending on the type of its first argument. When the first argument is a string, `contains()` performs a substring match rather than an exact membership check. This can be bypassed by an attacker who crafts a value that happens to be a substring of the target string. + +For example, the condition `contains('refs/heads/main refs/heads/develop', github.ref)` would also match a branch named `mai` or `evelop`, because these are substrings of the target string. + +## Recommendation + +Use `fromJSON()` to pass an array as the first argument to `contains()`, which performs an exact array membership check: + +```yaml +if: contains(fromJSON('["refs/heads/main", "refs/heads/develop"]'), github.ref) +``` + +Alternatively, use explicit equality checks combined with logical OR: + +```yaml +if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop' +``` + +## Example + +### Incorrect Usage + +```yaml +steps: + - run: terraform apply + if: contains('refs/heads/main refs/heads/develop', github.ref) +``` + +### Correct Usage + +```yaml +steps: + - run: terraform apply + if: contains(fromJSON('["refs/heads/main", "refs/heads/develop"]'), github.ref) +``` + +## References + +- GitHub Docs: [contains() function](https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#contains). +- Zizmor: [unsound-contains](https://docs.zizmor.sh/audits/#unsound-contains). diff --git a/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql b/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql new file mode 100644 index 000000000000..d1b7323434be --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql @@ -0,0 +1,41 @@ +/** + * @name Unsound use of contains() for membership check + * @description Using `contains()` with a string literal as the first argument performs a substring + * match instead of an array membership check, which can be bypassed. + * @kind problem + * @precision high + * @security-severity 5.0 + * @problem.severity warning + * @id actions/unsound-contains + * @tags actions + * security + * experimental + * external/cwe/cwe-183 + */ + +import actions + +/** + * Holds if `expr` is an expression within an `if:` condition that uses `contains()` + * with a string literal as the first argument, performing a substring match + * instead of an array membership check. + */ +predicate isUnsoundContains(Expression expr) { + exists(If ifNode | + ifNode.getConditionExpr() = expr and + // Match contains() with a string-literal first argument + // This catches: contains('refs/heads/main refs/heads/develop', github.ref) + // But NOT: contains(fromJSON('["refs/heads/main"]'), github.ref) + // And NOT: contains(github.event.issue.labels.*.name, 'bug') + ( + expr.getExpression().regexpMatch("(?i)contains\\s*\\(\\s*'[^']*'\\s*,.*") + or + expr.getExpression().regexpMatch("(?i)contains\\s*\\(\\s*\"[^\"]*\"\\s*,.*") + ) + ) +} + +from Expression expr +where isUnsoundContains(expr) +select expr, + "The `contains()` call performs a substring match which can be bypassed. Use `fromJSON()` to create an array for a proper membership check." diff --git a/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md new file mode 100644 index 000000000000..cea692d58993 --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md @@ -0,0 +1,33 @@ +## Overview + +When calling a reusable workflow with `secrets: inherit`, all organization, repository, and environment secrets from the parent workflow are forwarded to the called workflow. This violates the principle of least privilege and increases the impact of a potential vulnerability in the reusable workflow. + +## Recommendation + +Instead of using `secrets: inherit`, explicitly pass only the secrets that the reusable workflow actually needs via a `secrets:` block. + +## Example + +### Incorrect Usage + +```yaml +jobs: + call-workflow: + uses: org/repo/.github/workflows/reusable.yml@main + secrets: inherit +``` + +### Correct Usage + +```yaml +jobs: + call-workflow: + uses: org/repo/.github/workflows/reusable.yml@main + secrets: + API_KEY: ${{ secrets.API_KEY }} +``` + +## References + +- GitHub Docs: [Passing secrets to reusable workflows](https://docs.github.com/en/actions/sharing-automations/reusing-workflows#passing-inputs-and-secrets-to-a-reusable-workflow). +- Zizmor: [secrets-inherit](https://docs.zizmor.sh/audits/#secrets-inherit). diff --git a/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql new file mode 100644 index 000000000000..84e6e40c3c42 --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql @@ -0,0 +1,26 @@ +/** + * @name Secrets inherited by reusable workflow + * @description Using `secrets: inherit` passes all parent secrets to a reusable workflow, + * violating the principle of least privilege. + * @kind problem + * @precision high + * @security-severity 5.0 + * @problem.severity warning + * @id actions/secrets-inherit + * @tags actions + * security + * experimental + * external/cwe/cwe-200 + */ + +import actions +private import codeql.actions.ast.internal.Yaml +private import codeql.actions.ast.internal.Ast + +from ExternalJob job, YamlScalar secretsNode +where + secretsNode = job.(ExternalJobImpl).getNode().lookup("secrets") and + secretsNode.getValue() = "inherit" +select secretsNode, + "All parent secrets are unconditionally inherited by the reusable workflow $@. Pass only the secrets that are needed.", + job.(Uses).getCalleeNode(), job.(Uses).getCallee() diff --git a/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md b/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md new file mode 100644 index 000000000000..d0e6739853eb --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md @@ -0,0 +1,51 @@ +## Overview + +Many workflows use `github.actor` or `github.triggering_actor` to check if a specific bot (such as Dependabot or Renovate) triggered the workflow, and then bypass security checks or perform privileged actions. However, `github.actor` refers to the last actor to perform an "action" on the triggering context, not necessarily the actor that actually caused the trigger. + +An attacker can exploit this by creating a pull request where the HEAD commit has `github.actor == 'dependabot[bot]'` but the rest of the branch history contains attacker-controlled code, bypassing the actor check. + +## Recommendation + +Instead of checking `github.actor`, use a context that refers to the actor who created the triggering event. For `pull_request_target` workflows, use `github.event.pull_request.user.login`. For `issue_comment` workflows, use `github.event.comment.user.login`. + +More generally, consider whether a bot-bypass check is the right approach. GitHub's documentation recommends not using `pull_request_target` for auto-merge workflows. + +## Example + +### Incorrect Usage + +```yaml +on: pull_request_target + +jobs: + automerge: + runs-on: ubuntu-latest + if: github.actor == 'dependabot[bot]' + steps: + - run: gh pr merge --auto --merge "$PR_URL" + env: + PR_URL: ${{ github.event.pull_request.html_url }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} +``` + +### Correct Usage + +```yaml +on: pull_request_target + +jobs: + automerge: + runs-on: ubuntu-latest + if: github.event.pull_request.user.login == 'dependabot[bot]' + steps: + - run: gh pr merge --auto --merge "$PR_URL" + env: + PR_URL: ${{ github.event.pull_request.html_url }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} +``` + +## References + +- Synacktiv: [GitHub Actions exploitations: Dependabot](https://www.synacktiv.com/publications/github-actions-exploitation-dependabot). +- GitHub Docs: [Automating Dependabot with GitHub Actions](https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions). +- Zizmor: [bot-conditions](https://docs.zizmor.sh/audits/#bot-conditions). diff --git a/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.ql b/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.ql new file mode 100644 index 000000000000..fb4d55e5ae15 --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.ql @@ -0,0 +1,50 @@ +/** + * @name Spoofable actor check used as security control + * @description Checking `github.actor` or `github.triggering_actor` against a bot name + * is spoofable and should not be used as a security control. + * @kind problem + * @precision high + * @security-severity 7.0 + * @problem.severity warning + * @id actions/spoofable-actor-check + * @tags actions + * security + * experimental + * external/cwe/cwe-290 + */ + +import actions + +/** + * Holds if `ifNode` contains a spoofable bot actor check. + * + * Matches conditions like: + * `github.actor == 'dependabot[bot]'` + * `github.triggering_actor == 'renovate[bot]'` + * `'dependabot[bot]' == github.actor` + * + * These are spoofable because `github.actor` refers to the last actor + * to act on the triggering context, not necessarily the actor that + * caused the trigger. + */ +predicate isSpoofableBotCheck(If ifNode) { + exists(string cond | + cond = normalizeExpr(ifNode.getCondition()) and + ( + // github.actor == 'something[bot]' or github.triggering_actor == 'something[bot]' + cond.regexpMatch("(?s).*\\bgithub\\.(actor|triggering_actor)\\s*==\\s*'[^']*\\[bot\\][^']*'.*") + or + // reversed: 'something[bot]' == github.actor + cond.regexpMatch("(?s).*'[^']*\\[bot\\][^']*'\\s*==\\s*github\\.(actor|triggering_actor)\\b.*") + ) + ) +} + +from If ifNode, Event event +where + isSpoofableBotCheck(ifNode) and + event = ifNode.getATriggerEvent() and + event.isExternallyTriggerable() +select ifNode, + "This condition checks `github.actor` against a bot name, which is spoofable on $@ triggers. Use `github.event.pull_request.user.login` or similar non-spoofable context instead.", + event, event.getName() diff --git a/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.md b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.md new file mode 100644 index 000000000000..d789ba336b4c --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.md @@ -0,0 +1,44 @@ +## Overview + +Hardcoding credentials (passwords) in GitHub Actions workflow `container` or `services` configurations embeds secrets directly in the repository source code. Anyone with read access to the repository can see these credentials. + +## Recommendation + +Use [encrypted secrets](https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions) instead of hardcoded credentials. + +## Example + +### Incorrect Usage + +```yaml +jobs: + test: + runs-on: ubuntu-latest + container: + image: registry.example.com/app + credentials: + username: user + password: hackme + steps: + - run: echo 'hello' +``` + +### Correct Usage + +```yaml +jobs: + test: + runs-on: ubuntu-latest + container: + image: registry.example.com/app + credentials: + username: ${{ secrets.REGISTRY_USERNAME }} + password: ${{ secrets.REGISTRY_PASSWORD }} + steps: + - run: echo 'hello' +``` + +## References + +- GitHub Docs: [Using encrypted secrets in a workflow](https://docs.github.com/en/actions/security-for-github-actions/security-guides/using-secrets-in-github-actions). +- Zizmor: [hardcoded-container-credentials](https://docs.zizmor.sh/audits/#hardcoded-container-credentials). diff --git a/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql new file mode 100644 index 000000000000..f1dfdb779df1 --- /dev/null +++ b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql @@ -0,0 +1,47 @@ +/** + * @name Hardcoded credentials in container configuration + * @description Hardcoding credentials in workflow container or service configurations + * exposes secrets in the repository source code. + * @kind problem + * @precision high + * @security-severity 9.0 + * @problem.severity error + * @id actions/hardcoded-container-credentials + * @tags actions + * security + * experimental + * external/cwe/cwe-798 + */ + +import actions +private import codeql.actions.ast.internal.Yaml +private import codeql.actions.ast.internal.Ast + +/** + * Gets a `credentials.password` scalar node from a container or service mapping within a job. + */ +YamlScalar getAHardcodedPassword(LocalJobImpl job, string context) { + exists(YamlMapping creds | + // Job-level container credentials + creds = + job.getNode().lookup("container").(YamlMapping).lookup("credentials").(YamlMapping) and + context = "container" + or + // Service-level container credentials + exists(YamlMapping service | + service = job.getNode().lookup("services").(YamlMapping).lookup(_).(YamlMapping) and + creds = service.lookup("credentials").(YamlMapping) and + context = "service" + ) + | + result = creds.lookup("password").(YamlScalar) and + // Not a ${{ }} expression reference (e.g. ${{ secrets.PASSWORD }}) + not result.getValue().regexpMatch("\\$\\{\\{.*\\}\\}") + ) +} + +from LocalJob job, YamlScalar password, string context +where password = getAHardcodedPassword(job, context) +select password, + "Hardcoded password in " + context + " credentials for job $@. Use an encrypted secret instead.", + job, job.(Job).getId() diff --git a/actions/ql/test/query-tests/Security/CWE-183/.github/workflows/unsound-contains.yml b/actions/ql/test/query-tests/Security/CWE-183/.github/workflows/unsound-contains.yml new file mode 100644 index 000000000000..b647d3cfbcb1 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-183/.github/workflows/unsound-contains.yml @@ -0,0 +1,26 @@ +name: unsound-contains-test +on: push + +jobs: + deploy: + runs-on: ubuntu-latest + steps: + # Positive: contains() with space-separated string (substring match) + - run: terraform apply + if: contains('refs/heads/main refs/heads/develop', github.ref) + + # Positive: contains() with double-quoted string + - run: terraform apply + if: contains("refs/heads/main", github.ref) + + # Negative: contains() with fromJSON (proper array membership) + - run: terraform apply + if: contains(fromJSON('["refs/heads/main", "refs/heads/develop"]'), github.ref) + + # Negative: contains() with context as first arg (array membership on labels) + - run: echo "has bug label" + if: contains(github.event.issue.labels.*.name, 'bug') + + # Negative: no contains() at all + - run: terraform apply + if: github.ref == 'refs/heads/main' diff --git a/actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.expected b/actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.expected new file mode 100644 index 000000000000..b25e90bd59a2 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.expected @@ -0,0 +1,2 @@ +| .github/workflows/unsound-contains.yml:10:15:10:72 | contains('refs/heads/main refs/heads/develop', github.ref) | The `contains()` call performs a substring match which can be bypassed. Use `fromJSON()` to create an array for a proper membership check. | +| .github/workflows/unsound-contains.yml:14:15:14:53 | contains("refs/heads/main", github.ref) | The `contains()` call performs a substring match which can be bypassed. Use `fromJSON()` to create an array for a proper membership check. | diff --git a/actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.qlref b/actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.qlref new file mode 100644 index 000000000000..3e742fa505da --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-183/UnsoundContains.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-183/UnsoundContains.ql diff --git a/actions/ql/test/query-tests/Security/CWE-200/.github/workflows/secrets-inherit.yml b/actions/ql/test/query-tests/Security/CWE-200/.github/workflows/secrets-inherit.yml new file mode 100644 index 000000000000..7294c7379191 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-200/.github/workflows/secrets-inherit.yml @@ -0,0 +1,25 @@ +name: secrets-inherit-test +on: + workflow_dispatch: + +jobs: + # Positive: secrets: inherit on reusable workflow call + inherit-all: + uses: org/repo/.github/workflows/reusable.yml@main + secrets: inherit + + # Negative: explicit secrets + explicit-secrets: + uses: org/repo/.github/workflows/reusable.yml@main + secrets: + API_KEY: ${{ secrets.API_KEY }} + + # Negative: no secrets key + no-secrets: + uses: org/repo/.github/workflows/reusable.yml@main + + # Negative: normal local job (not a reusable workflow call) + local-job: + runs-on: ubuntu-latest + steps: + - run: echo "hello" diff --git a/actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.expected b/actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.expected new file mode 100644 index 000000000000..877ed6f9844a --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.expected @@ -0,0 +1 @@ +| .github/workflows/secrets-inherit.yml:9:14:9:20 | inherit | All parent secrets are unconditionally inherited by the reusable workflow $@. Pass only the secrets that are needed. | .github/workflows/secrets-inherit.yml:8:11:8:54 | org/repo/.github/workflows/reusable.yml@main | org/repo/.github/workflows/reusable.yml | diff --git a/actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.qlref b/actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.qlref new file mode 100644 index 000000000000..28a7799068c0 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-200/SecretsInherit.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-200/SecretsInherit.ql diff --git a/actions/ql/test/query-tests/Security/CWE-290/.github/workflows/bot-conditions.yml b/actions/ql/test/query-tests/Security/CWE-290/.github/workflows/bot-conditions.yml new file mode 100644 index 000000000000..1e1420909e27 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-290/.github/workflows/bot-conditions.yml @@ -0,0 +1,42 @@ +name: bot-conditions-test +on: + pull_request_target: + +jobs: + # Positive: spoofable github.actor check on externally triggerable event + spoofable-actor: + runs-on: ubuntu-latest + if: github.actor == 'dependabot[bot]' + steps: + - run: gh pr merge --auto --merge "$PR_URL" + env: + PR_URL: ${{ github.event.pull_request.html_url }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + # Positive: spoofable github.triggering_actor check + spoofable-triggering: + runs-on: ubuntu-latest + if: github.triggering_actor == 'renovate[bot]' + steps: + - run: echo "auto-approve" + + # Positive: reversed operand order + spoofable-reversed: + runs-on: ubuntu-latest + if: ${{ 'dependabot[bot]' == github.actor }} + steps: + - run: echo "reversed" + + # Negative: non-spoofable context (event-level user) + safe-check: + runs-on: ubuntu-latest + if: github.event.pull_request.user.login == 'dependabot[bot]' + steps: + - run: echo "safe" + + # Negative: non-bot actor check + non-bot: + runs-on: ubuntu-latest + if: github.actor == 'admin-user' + steps: + - run: echo "admin" diff --git a/actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.expected b/actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.expected new file mode 100644 index 000000000000..524d1a68a806 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.expected @@ -0,0 +1,3 @@ +| .github/workflows/bot-conditions.yml:9:9:9:41 | github. ... t[bot]' | This condition checks `github.actor` against a bot name, which is spoofable on $@ triggers. Use `github.event.pull_request.user.login` or similar non-spoofable context instead. | .github/workflows/bot-conditions.yml:3:3:3:21 | pull_request_target | pull_request_target | +| .github/workflows/bot-conditions.yml:19:9:19:50 | github. ... e[bot]' | This condition checks `github.actor` against a bot name, which is spoofable on $@ triggers. Use `github.event.pull_request.user.login` or similar non-spoofable context instead. | .github/workflows/bot-conditions.yml:3:3:3:21 | pull_request_target | pull_request_target | +| .github/workflows/bot-conditions.yml:26:9:26:48 | ${{ 'de ... ctor }} | This condition checks `github.actor` against a bot name, which is spoofable on $@ triggers. Use `github.event.pull_request.user.login` or similar non-spoofable context instead. | .github/workflows/bot-conditions.yml:3:3:3:21 | pull_request_target | pull_request_target | diff --git a/actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.qlref b/actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.qlref new file mode 100644 index 000000000000..e36431a8a66b --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-290/SpoofableActorCheck.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-290/SpoofableActorCheck.ql diff --git a/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml b/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml new file mode 100644 index 000000000000..46183ec5ef92 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml @@ -0,0 +1,52 @@ +name: hardcoded-credentials-test +on: + push: + +jobs: + # Positive: hardcoded container password + hardcoded-container: + runs-on: ubuntu-latest + container: + image: registry.example.com/app + credentials: + username: user + password: hackme + steps: + - run: echo "hello" + + # Positive: hardcoded service password + hardcoded-service: + runs-on: ubuntu-latest + services: + mydb: + image: registry.example.com/db + credentials: + username: dbuser + password: dbpass123 + steps: + - run: echo "hello" + + # Negative: secrets reference for container + secret-container: + runs-on: ubuntu-latest + container: + image: registry.example.com/app + credentials: + username: ${{ secrets.REGISTRY_USERNAME }} + password: ${{ secrets.REGISTRY_PASSWORD }} + steps: + - run: echo "hello" + + # Negative: no credentials block + no-creds: + runs-on: ubuntu-latest + container: + image: ubuntu:latest + steps: + - run: echo "hello" + + # Negative: no container at all + plain-job: + runs-on: ubuntu-latest + steps: + - run: echo "hello" diff --git a/actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.expected b/actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.expected new file mode 100644 index 000000000000..c5e94deabbe3 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.expected @@ -0,0 +1,2 @@ +| .github/workflows/hardcoded-credentials.yml:13:19:13:24 | hackme | Hardcoded password in container credentials for job $@. Use an encrypted secret instead. | .github/workflows/hardcoded-credentials.yml:8:5:18:2 | Job: hardcoded-container | hardcoded-container | +| .github/workflows/hardcoded-credentials.yml:25:21:25:29 | dbpass123 | Hardcoded password in service credentials for job $@. Use an encrypted secret instead. | .github/workflows/hardcoded-credentials.yml:19:5:30:2 | Job: hardcoded-service | hardcoded-service | diff --git a/actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.qlref b/actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.qlref new file mode 100644 index 000000000000..eb86369545ea --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-798/HardcodedContainerCredentials.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-798/HardcodedContainerCredentials.ql From ba801869f984f84d88f2047d3bdc870ee4fa76ba Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Wed, 1 Apr 2026 09:15:34 -0700 Subject: [PATCH 2/6] Fix redundant casts and clarify actor-check wording --- .../experimental/Security/CWE-290/SpoofableActorCheck.md | 2 +- .../Security/CWE-798/HardcodedContainerCredentials.ql | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md b/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md index d0e6739853eb..ed386f3971c9 100644 --- a/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md +++ b/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.md @@ -2,7 +2,7 @@ Many workflows use `github.actor` or `github.triggering_actor` to check if a specific bot (such as Dependabot or Renovate) triggered the workflow, and then bypass security checks or perform privileged actions. However, `github.actor` refers to the last actor to perform an "action" on the triggering context, not necessarily the actor that actually caused the trigger. -An attacker can exploit this by creating a pull request where the HEAD commit has `github.actor == 'dependabot[bot]'` but the rest of the branch history contains attacker-controlled code, bypassing the actor check. +An attacker can exploit this by creating a pull request where the workflow run's `github.actor` is `'dependabot[bot]'` (for example, because Dependabot was the latest actor on the PR), but the branch contains attacker-controlled code, bypassing the actor check. ## Recommendation diff --git a/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql index f1dfdb779df1..4cf67d1383a1 100644 --- a/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql +++ b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql @@ -24,17 +24,17 @@ YamlScalar getAHardcodedPassword(LocalJobImpl job, string context) { exists(YamlMapping creds | // Job-level container credentials creds = - job.getNode().lookup("container").(YamlMapping).lookup("credentials").(YamlMapping) and + job.getNode().lookup("container").(YamlMapping).lookup("credentials") and context = "container" or // Service-level container credentials exists(YamlMapping service | - service = job.getNode().lookup("services").(YamlMapping).lookup(_).(YamlMapping) and - creds = service.lookup("credentials").(YamlMapping) and + service = job.getNode().lookup("services").(YamlMapping).lookup(_) and + creds = service.lookup("credentials") and context = "service" ) | - result = creds.lookup("password").(YamlScalar) and + result = creds.lookup("password") and // Not a ${{ }} expression reference (e.g. ${{ secrets.PASSWORD }}) not result.getValue().regexpMatch("\\$\\{\\{.*\\}\\}") ) From 0d18758c4ff7cdcc5d2cf234bea6df91745c737c Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Wed, 1 Apr 2026 10:15:17 -0700 Subject: [PATCH 3/6] Add new experimental queries to not_included_in_qls.expected --- .../query-suite/not_included_in_qls.expected | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/actions/ql/integration-tests/query-suite/not_included_in_qls.expected b/actions/ql/integration-tests/query-suite/not_included_in_qls.expected index 6ed0a557462b..2742dc8febcd 100644 --- a/actions/ql/integration-tests/query-suite/not_included_in_qls.expected +++ b/actions/ql/integration-tests/query-suite/not_included_in_qls.expected @@ -10,8 +10,12 @@ ql/actions/ql/src/experimental/Security/CWE-078/CommandInjectionCritical.ql ql/actions/ql/src/experimental/Security/CWE-078/CommandInjectionMedium.ql ql/actions/ql/src/experimental/Security/CWE-088/ArgumentInjectionCritical.ql ql/actions/ql/src/experimental/Security/CWE-088/ArgumentInjectionMedium.ql +ql/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.ql ql/actions/ql/src/experimental/Security/CWE-200/SecretExfiltration.ql +ql/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql ql/actions/ql/src/experimental/Security/CWE-284/CodeExecutionOnSelfHostedRunner.ql +ql/actions/ql/src/experimental/Security/CWE-290/SpoofableActorCheck.ql +ql/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql ql/actions/ql/src/experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql ql/actions/ql/src/experimental/Security/CWE-829/UnversionedImmutableAction.ql ql/actions/ql/src/experimental/Security/CWE-918/RequestForgery.ql From 0f515fa6f02ea014f023bec5e561d39f328b4e22 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Wed, 1 Apr 2026 10:16:27 -0700 Subject: [PATCH 4/6] Fix ql-format for HardcodedContainerCredentials --- .../Security/CWE-798/HardcodedContainerCredentials.ql | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql index 4cf67d1383a1..6395fa560ec2 100644 --- a/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql +++ b/actions/ql/src/experimental/Security/CWE-798/HardcodedContainerCredentials.ql @@ -23,8 +23,7 @@ private import codeql.actions.ast.internal.Ast YamlScalar getAHardcodedPassword(LocalJobImpl job, string context) { exists(YamlMapping creds | // Job-level container credentials - creds = - job.getNode().lookup("container").(YamlMapping).lookup("credentials") and + creds = job.getNode().lookup("container").(YamlMapping).lookup("credentials") and context = "container" or // Service-level container credentials From 78c3aba948f4e26cb421a4edf7ad60a5a9180efd Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Wed, 1 Apr 2026 10:25:31 -0700 Subject: [PATCH 5/6] Address Copilot review: fix ref wording, add service secrets negative test --- .../experimental/Security/CWE-183/UnsoundContains.md | 2 +- .../.github/workflows/hardcoded-credentials.yml | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md b/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md index 4731bc5e8ae8..91fe91b4a7e7 100644 --- a/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md +++ b/actions/ql/src/experimental/Security/CWE-183/UnsoundContains.md @@ -2,7 +2,7 @@ The GitHub Actions `contains()` function behaves differently depending on the type of its first argument. When the first argument is a string, `contains()` performs a substring match rather than an exact membership check. This can be bypassed by an attacker who crafts a value that happens to be a substring of the target string. -For example, the condition `contains('refs/heads/main refs/heads/develop', github.ref)` would also match a branch named `mai` or `evelop`, because these are substrings of the target string. +For example, the condition `contains('refs/heads/main refs/heads/develop', github.ref)` would also match `github.ref` values like `refs/heads/mai` or `refs/heads/evelop`, because these are substrings of the target string. ## Recommendation diff --git a/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml b/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml index 46183ec5ef92..ecd47f173fc6 100644 --- a/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml +++ b/actions/ql/test/query-tests/Security/CWE-798/.github/workflows/hardcoded-credentials.yml @@ -37,6 +37,18 @@ jobs: steps: - run: echo "hello" + # Negative: secrets reference for service + secret-service: + runs-on: ubuntu-latest + services: + mydb: + image: registry.example.com/db + credentials: + username: ${{ secrets.DB_USERNAME }} + password: ${{ secrets.DB_PASSWORD }} + steps: + - run: echo "hello" + # Negative: no credentials block no-creds: runs-on: ubuntu-latest From b71f88ab95338f568515f931d098a614413e5594 Mon Sep 17 00:00:00 2001 From: Jamie Magee Date: Thu, 2 Apr 2026 15:08:58 -0700 Subject: [PATCH 6/6] secrets-inherit: soften to best-practice recommendation --- .../experimental/Security/CWE-200/SecretsInherit.md | 4 ++-- .../experimental/Security/CWE-200/SecretsInherit.ql | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md index cea692d58993..2cb178deb43b 100644 --- a/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md +++ b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.md @@ -1,10 +1,10 @@ ## Overview -When calling a reusable workflow with `secrets: inherit`, all organization, repository, and environment secrets from the parent workflow are forwarded to the called workflow. This violates the principle of least privilege and increases the impact of a potential vulnerability in the reusable workflow. +When calling a reusable workflow with `secrets: inherit`, every secret the calling workflow can access (organization, repository, and environment secrets) is forwarded to the callee. This is convenient but broader than most callees require. If the reusable workflow has a vulnerability — for example, a template-injection flaw — the blast radius includes every inherited secret rather than just the ones it actually uses. ## Recommendation -Instead of using `secrets: inherit`, explicitly pass only the secrets that the reusable workflow actually needs via a `secrets:` block. +As a defense-in-depth measure, prefer passing only the secrets the reusable workflow needs via an explicit `secrets:` block. ## Example diff --git a/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql index 84e6e40c3c42..beda2b691fd0 100644 --- a/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql +++ b/actions/ql/src/experimental/Security/CWE-200/SecretsInherit.ql @@ -1,11 +1,11 @@ /** * @name Secrets inherited by reusable workflow - * @description Using `secrets: inherit` passes all parent secrets to a reusable workflow, - * violating the principle of least privilege. + * @description Using `secrets: inherit` passes every secret the calling workflow can access + * to a reusable workflow, which is more than most callees need. * @kind problem - * @precision high - * @security-severity 5.0 - * @problem.severity warning + * @precision medium + * @security-severity 3.0 + * @problem.severity recommendation * @id actions/secrets-inherit * @tags actions * security @@ -22,5 +22,5 @@ where secretsNode = job.(ExternalJobImpl).getNode().lookup("secrets") and secretsNode.getValue() = "inherit" select secretsNode, - "All parent secrets are unconditionally inherited by the reusable workflow $@. Pass only the secrets that are needed.", + "Every secret accessible to the calling workflow is forwarded to $@. Consider passing only the secrets it actually needs.", job.(Uses).getCalleeNode(), job.(Uses).getCallee()