Skip to content

Introduce the GitHub Actions Workflow Standards page#159

Open
johnbillion wants to merge 2 commits into
masterfrom
github-actions-standards
Open

Introduce the GitHub Actions Workflow Standards page#159
johnbillion wants to merge 2 commits into
masterfrom
github-actions-standards

Conversation

@johnbillion
Copy link
Copy Markdown
Member

@johnbillion johnbillion commented May 3, 2026

Work has been ongoing recently to improve the quality and security of GitHub Actions workflow files in the wordpress-develop and gutenberg repos. Examples:

While the Actionlint and Zizmor tools that we now use are great for surfacing problems in workflow files, we also need good user-facing documentation that provides guidance on addressing common issues and what automation is in place.

Members of the security team have been working on this page and it's already live at https://developer.wordpress.org/coding-standards/wordpress-coding-standards/github-actions/. It now needs to be moved into the docs repo alongside the other pages.

Copy link
Copy Markdown
Member

@dingo-d dingo-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had just one comment, otherwise looks good 👍🏼

Comment thread wordpress-coding-standards/github-actions.md Outdated
@jrfnl
Copy link
Copy Markdown
Member

jrfnl commented May 5, 2026

@johnbillion You seem to be mixing two different tasks in this single PR:

  • Adding a new page
  • Improving the security of the GH Actions workflow(s) for this repo.

As those are different decision points, with different justifications, could you please move the workflow changes to their own PR ?

@johnbillion
Copy link
Copy Markdown
Member Author

could you please move the workflow changes to their own PR ?

Done: #161

@johnbillion johnbillion moved this from In progress to In review in WordPress Project Build Tooling May 6, 2026
Copy link
Copy Markdown
Member

@desrosj desrosj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnbillion some small suggestions. Some may be a bit deeper than you're looking to go in this high-level guide, and some are stylistic preferences.


```yaml
- name: Print title
run: echo "Title: ${PR_TITLE}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit and admittedly a personal preference, but I prefer that run: comes last in the step definition because it ensures the reader notices all conditions, variables, environments, etc. before reading the actual command that is run.

**Correct:**

```yaml
- uses: actions/github-script@...
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same preference comment here about the uses: being last. However, I feel more strongly that any inputs passed should be immediately after the uses: line.


Writing to `$GITHUB_ENV` or `$GITHUB_PATH` from a shell script is dangerous if the input is user-controlled, because an attacker can inject arbitrary environment variables or prepend to `PATH`.

- Only write to `$GITHUB_ENV` or `$GITHUB_OUTPUT` with values that are fully controlled by the workflow, not with values derived from pull request content, issue bodies, commit messages, or other user-controllable inputs.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we include an example about how to do this properly by wrapping $GITHUB_OUTPUT in double quotes?

- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
```

Always include a version comment after the SHA to make the pinned version human-readable. When updating an action, update both the SHA and the version comment.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be worth noting that the version specified within the online comment must match the actual version tag published by the action's repository.


Using GitHub Actions caching in workflows that produce release artifacts is risky. A cache can be poisoned by an attacker in a separate workflow, allowing the poisoned cache to inject malicious content into a release.

Avoid using `actions/cache` or built-in caching features in workflows that build and publish packages or release artifacts. If caching is necessary in such workflows, ensure the cache key is scoped tightly and the cache contents are verified before use.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache poisoning can also occur when a workflow specifies restore-keys. We should explicitly discourage the use of partial cache key matching patterns in favor of explicit hash-based keys where the hash was produced using the relevant lock files.

Copy link
Copy Markdown
Contributor

@rodrigoprimo rodrigoprimo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me overall. I left a couple of inline comments with questions about small things that caught my attention.


Writing to `$GITHUB_ENV` or `$GITHUB_PATH` from a shell script is dangerous if the input is user-controlled, because an attacker can inject arbitrary environment variables or prepend to `PATH`.

- Only write to `$GITHUB_ENV` or `$GITHUB_OUTPUT` with values that are fully controlled by the workflow, not with values derived from pull request content, issue bodies, commit messages, or other user-controllable inputs.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The paragraph above flags writing to $GITHUB_ENV or $GITHUB_PATH, but this line mentions writing to $GITHUB_ENV or $GITHUB_OUTPUT. Should it say $GITHUB_PATH instead of $GITHUB_OUTPUT? I might be missing something, as I'm not super familiar with how GitHub Actions uses those environment variables.


**Incorrect:**

```yaml
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The YAML code blocks on the published page are not syntax-highlighted. They render as <code class=""> rather than <code class="language-yaml"> like the PHP and CSS blocks do on other pages. Do you know if ```yaml fences work with the importer? I don't know whether the published page was generated from a version of this Markdown file.

Copy link
Copy Markdown
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the PR body still says "This PR also fixes some issues in the qa.yml workflow file in this repo so it adheres to these standards.", even though that was moved out.

<h2>Language-specific Standards</h2>
<ul>
<li><a href="https://developer.wordpress.org/coding-standards/wordpress-coding-standards/css/">CSS Coding Standards</a></li>
<li><a href="https://developer.wordpress.org/coding-standards/wordpress-coding-standards/github-actions/">GitHub Actions Workflow Standards</a></li>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s slotted under “Language-specific Standards” alongside CSS/HTML/JS/PHP. GitHub Actions isn’t a language; it’s YAML config for a CI platform. This will read oddly to anyone scanning the index, and it implies a parity (“this is a WordPress coding standard”) that the content doesn’t quite live up to. A separate “Infrastructure / Tooling Standards” grouping would be more honest, and would leave room for similar future docs (Composer, npm, Docker).


GitHub Actions workflows operate in a highly privileged software supply chain environment. Workflows can access repository secrets, push code, create releases, publish packages, and interact with external services. A security weakness in a workflow file can have severe consequences.

WordPress uses two complementary linting tools to help maintain the quality and security of workflow files in the `.github/workflows` directory: [Actionlint](https://github.com/rhysd/actionlint) and [Zizmor](https://github.com/zizmorcore/zizmor). This page documents the tools and how contributors should address errors or warnings that they report.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The scope is undefined. The doc says “WordPress uses…” then only names wordpress-develop as an enforcement point. Does this apply to Gutenberg? Plugin/theme contributors? The wpcs-docs repo itself? The PR body hints at all of them, but the page should state its scope explicitly — otherwise it’s a guideline dressed as a standard.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The end goal is to add enforcement of these practices org-wide to ensure all repositories remain secure. But that will take time.

That said, we could maybe add a section linking to the repositories enforcing this to date. Gutenberg runs a workflow, but I don't believe it is configured as a check that is required to pass currently. I could be wrong on that and would need to check.

@@ -0,0 +1,185 @@
# GitHub Actions Workflow Standards
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a title vs content imbalance here. It's called “GitHub Actions Workflow Standards”, but the substance is almost entirely Zizmor audit findings rephrased. Actionlint gets one paragraph and zero concrete examples. Either rename it to something like “Workflow Security Standards”, or beef up the Actionlint side (common findings, shellcheck guidance, expression typing pitfalls).

For a document presented as the standard, I’d expect at least a mention of:

  • timeout-minutes (DoS / runaway jobs)
  • Concurrency controls (concurrency: block)
  • GITHUB_TOKEN scoping vs PATs
  • etc.

contents: read
```

### Artipacked credentials
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a Zizmor audit name. For readers who’ve never used Zizmor, it’s jargon. “Persistent checkout credentials (Zizmor: artipacked)” reads more naturally.


Using GitHub Actions caching in workflows that produce release artifacts is risky. A cache can be poisoned by an attacker in a separate workflow, allowing the poisoned cache to inject malicious content into a release.

Avoid using `actions/cache` or built-in caching features in workflows that build and publish packages or release artifacts. If caching is necessary in such workflows, ensure the cache key is scoped tightly and the cache contents are verified before use.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“Scope the cache key tightly and verify the cache contents before use” — how? A concrete example or a hard rule (“don’t cache in release workflows, full stop”) would be more useful than the current half-advice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

6 participants