Skip to content

Harden GitHub Actions workflows per zizmor, poutine and octoscan - #6900

Open
ondrejmirtes wants to merge 1 commit into
2.3.xfrom
harden-workflow-lint-findings
Open

ondrejmirtes wants to merge 1 commit into
2.3.xfrom
harden-workflow-lint-findings

Conversation

@ondrejmirtes

Copy link
Copy Markdown
Member

Fixes what the linters in lint-workflows.yml report. Run locally with the same versions and flags CI uses (actionlint 1.7.12, octoscan v0.1.7, poutine 1.1.4, zizmor 1.20.0 --persona=auditor):

Linter Before After
actionlint 0 0
zizmor 185 findings (14 high) 0 (26 ignored inline, each with a reason)
poutine 133 0
octoscan 43 0

Fixes

  • template-injection: expressions in run: scripts (matrix scripts, step outputs, vars.*, github.ref_name, PR number and URL, GITHUB_TOKEN for docker login) now go through env:. Matrix scripts run with eval "$MATRIX_SCRIPT", which keeps the same parsing, quoting and -e/pipefail behaviour as the old inline script.
  • excessive-permissions / undocumented-permissions: every workflow now declares permissions. The repo default token is already read-only (Contents: read, Metadata: read, Packages: read, per a recent run's log), so this only narrows. Jobs that only authenticate with PHPSTAN_BOT_TOKEN get {}. claude-react-on-review-dispatch drops from actions: write to actions: read, which is enough to download the artifact. Write and OIDC permissions now have a comment explaining them.
  • artipacked: persist-credentials: false on every checkout whose job never pushes. git fetch from this public repo still works without credentials. create-pull-request sets up its own token.
  • concurrency-limits: added groups where a newer run supersedes an older one: PR-scoped groups, the standard head_ref || run_id group, and single groups for the push and cron workflows.
  • anonymous-definition: unnamed jobs get a name. Where a job is a PR check, its name is its ID, so check names stay the same.
  • poutine unverified_script_exec: actionlint now runs from docker://rhysd/actionlint:1.7.12@sha256:… instead of bash <(curl …/main/…). Note that this pins actionlint instead of always using the latest version.

Linter configuration

  • octoscan: filter_triggers: external, octoscan's default, replaces ''. Its checkout, local-action and GITHUB_OUTPUT rules only matter for workflows that outside contributors can trigger with privileges, and those workflows are still scanned. runner-label is disabled because every runs-on expression resolves to a GitHub-hosted label.
  • poutine (.github/poutine.yml): ubuntu-24.04-arm is allowed as a runner (it's GitHub-hosted). The SHA-pinned third-party actions already in use are acknowledged, so a newly added action still gets reported.

By-design exceptions (inline zizmor: ignore)

  • dangerous-triggers: the three pull_request_target workflows and the workflow_run dispatch. None of them check out or run PR code.
  • artipacked: merge-maintained-branch, create-tag and the phpstan-dist checkout, which push with the persisted bot credentials.
  • concurrency-limits: apiref-infra queues instead of cancelling, so a CDK deploy is never interrupted. Review dispatch and create-tag runs don't supersede one another.
  • cache-poisoning (phar.yml, Alpine PHP build cache): only branch pushes can write the cache that a tag build reads.
  • obfuscation (phar.yml): computed lookups into the origins/prereleases JSON maps, and shell: cmd for MSVC builds.

- Pass expressions to run scripts through env variables instead of
  expanding them into the script (template-injection)
- Declare explicit least-privilege permissions on every workflow and
  document the non-obvious ones
- Do not persist checkout credentials unless a later step pushes with them
- Add concurrency groups where a newer run supersedes an older one
- Name the remaining anonymous jobs after their IDs, keeping check names
- Install actionlint from its digest-pinned image instead of piping a
  downloaded script into bash
- Scan only externally triggerable workflows with octoscan, its default
- Configure poutine's allowed GitHub-hosted arm runner and the reviewed
  third-party actions
- Document by-design exceptions with inline zizmor ignores

Amp-Thread-ID: https://ampcode.com/threads/T-01a197a7-c816-7129-bba7-bbd97e6de236
run: "php -d memory_limit=599M bin/phpstan analyse --generate-baseline baseline-php-${{ matrix.php-version }}.neon"
env:
MATRIX_PHP_VERSION: ${{ matrix.php-version }}
run: "php -d memory_limit=599M bin/phpstan analyse --generate-baseline baseline-php-${MATRIX_PHP_VERSION}.neon"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants