Feature: Detect unquoted shell variable expansions in run: blocks

[ManuelLerchnerQC]

Pre-submission checks

• I am not reporting a bug (crash, false positive/negative, etc). These must be filed via the bug report template.
• I have looked through both the open and closed issues for a duplicate request.

What's the problem this feature will solve?

Unquoted shell variable expansions in GitHub Actions run: blocks can cause unintended word splitting or glob expansion when variables contain spaces or special characters. This is a well-known class of issues and is flagged by tools such as ShellCheck.

Since GitHub context values (e.g. branch names or PR titles) are often user-controlled, unquoted expansions can lead to broken workflows or unsafe command execution.

Currently, zizmor does not detect these unquoted variable expansions.

Describe the solution you'd like

zizmor should detect potentially unsafe unquoted shell variable expansions in run: blocks and flag them as issues.

Example of problematic code:

on: pull_request

jobs:
  deploy:
    runs-on: ubuntu-latest
    permissions:
      contents: read
    steps:
      - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
        with:
          persist-credentials: false
      
      - name: Deploy with branch name
        run: |
          BRANCH_NAME=${GITHUB_HEAD_REF}
          echo "Deploying branch: $BRANCH_NAME"
          ./deploy.sh $BRANCH_NAME  # ⚠️ Unquoted variable

In this example, $BRANCH_NAME may contain spaces or other special characters, causing unexpected behavior when passed unquoted to shell commands.

Expected behavior / Suggested fix

zizmor should flag unquoted variable expansions used as shell command arguments and suggest proper quoting (potentially as an unsafe fix, since such usage may also be intentional).

For example, a suggested fix could look like this:

diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml
@@ -17,7 +17,7 @@ jobs:
       - name: Deploy with branch name
         run: |
           BRANCH_NAME=${GITHUB_HEAD_REF}
           echo "Deploying branch: $BRANCH_NAME"
-          ./deploy.sh $BRANCH_NAME
+          ./deploy.sh "$BRANCH_NAME"

Additional context

There is an existing issue around incorrect quoting in auto-fixes (e.g., single-quoting variable expansions) at #1346, but this request focuses on detecting missing quoting rather than on how fixes are applied.
Whether and how an autofix is offered is a separate concern, I think.

[woodruffw]

Hi @ManuelLerchnerQC, thanks for opening an issue.

Could you elaborate a bit on the security relevance of an audit here? In my experience unquoted shell variables don't generally result in security problems, although they are a code smell and do sometimes cause reliability problems.

Given that, I'm generally inclined to not support this kind of check in zizmor, and instead point people to actionlint for it (IIUC they wrap shellcheck to provide this check). However, if curious to hear arguments for it having more security salience and therefore being a good candidate for zizmor 🙂

[ytausch]

Hi! Joining the discussion as @ManuelLerchnerQC pointed me to it.

The biggest risk with unquoted variables is that they allow an attacker to pass multiple arguments to a single command when only one is expected. This Stack Exchange answer provides an example:

awk -v foo=$external_input '$2 == foo'

If external_input is attacker-controlled, they can execute arbitrary commands:

$ external_input='x BEGIN{system("uname")}'
$ awk -v foo=$external_input '$2 == foo'
Linux

So, unlike template-injection issues, the command that is used with the unquoted$external_input must have some kind of dangerous argument that the attacker can exploit. I think this is quite often the case in practice and thus relevant for zizmor.

The answer above also references other issues that are less critical, IMO, but should still be considered.

[woodruffw]

Yeah, there's something secondary here: a lot of tools (like awk, xargs, and others) exhibit code execution via arguments. However, quoting itself doesn't fully mitigate this: "${foo[@]}" for example is correctly quoted, but can still result in code execution if it's being expanded into the arguments of a susceptible tool. That in turn generalizes to a ShellCheck-sized problem, i.e. something that actionlint and ShellCheck are better equipped to support than zizmor itself.

Have you all tried actionlint for this purpose? To my understanding a lot of zizmor users combine the two effectively, with zizmor providing security checks and actionlint providing general quality and correctness checks.

[ytausch]

We will have a look into actionlint, thanks!

However, quoting itself doesn't fully mitigate this: "${foo[@]}" for example is correctly quoted, but can still result in code execution if it's being expanded into the arguments of a susceptible tool.

Of course. You could argue, though, that this is much less of an issue because you explicitly opted into passing everything as separate arguments with the [@] syntax. This is much less likely to be unintended compared to just forgetting quotes. Nevertheless, I see your point in drawing a line to actionlint here.