Skip to content

chore(ci): scope SC2086 shellcheck disable per-file instead of globally #2166

@finnoybu

Description

@finnoybu

Context

.shellcheckrc:28 disables SC2086 ("Double quote to prevent globbing and word splitting") globally for all workflow shell:

disable=SC2129,SC2001,SC2317,SC2050,SC2086,SC2016,SC2153

The comment block at lines 18-19 explains the intent — intentional word splitting in find-to-argument pipelines where each result is a separate positional argument. That's a legitimate exception, but applying it globally suppresses the warning for unrelated scripts where genuine quoting bugs could hide.

Why this isn't an immediate fix

SHELLCHECKRC is applied uniformly when workflow-lint.yml runs:

python3 scripts/extract_workflow_shell.py .github/workflows > workflow-shell-extracted.sh
shellcheck -s bash workflow-shell-extracted.sh

The extractor concatenates every run: block in every workflow into a single script for analysis (also why SC2317 is disabled — reachability is meaningless when independent blocks are stitched together). Switching to per-file disable requires either:

  1. Refactoring the extractor to emit one file per run: block, with # shellcheck disable=SC2086 inline at each block that intentionally word-splits, or
  2. Migrating from a single-file extraction to a directory-walk approach using shellcheck --source-path, with per-file .shellcheckrc overrides.

Both touch the extractor + the runner + every workflow that genuinely needs the relaxation, which is more scope than a single drive-by LOW.

Scope of the cleanup

Files that currently rely on the global SC2086 relaxation (rough scan):

  • .github/workflows/weekly-security-audit.ymlfind ... | xargs-style invocation at the dep-confusion-check job (already inline-tagged # shellcheck disable=SC2046, but adjacent unquoted expansions ride the global SC2086).
  • Likely a small handful of other find-to-positional-argument call sites; needs an audit pass once the extractor architecture is decided.

Proposed approach

  1. Audit each run: block that today relies on the global SC2086 relaxation. Most will not need it — "$VAR" quoting is the right answer for the majority of cases.
  2. For the remaining genuine word-splitting sites, replace the global disable with inline # shellcheck disable=SC2086 annotations at the call site, with a one-line comment explaining what's being split.
  3. Once the inline annotations are in, drop SC2086 from the global disable= list in .shellcheckrc.

Step 1 is a real audit pass — it's the reason this is being filed as an issue rather than a single PR.


Surfaced during independent audit conducted by @finnoybu (Ken Tannenbaum, AEGIS Initiative); [LOW, Infrastructure/CI].

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions