github/workflows: workflow hardening (artipacked, template-injection, actionlint hygiene)#5966
Open
eriknordmark wants to merge 4 commits into
Open
github/workflows: workflow hardening (artipacked, template-injection, actionlint hygiene)#5966eriknordmark wants to merge 4 commits into
eriknordmark wants to merge 4 commits into
Conversation
zizmor's artipacked audit warns that actions/checkout writes GITHUB_TOKEN into .git/config by default, which can be exfiltrated by a later step that uploads the workspace as an artifact. Set persist-credentials: false on every checkout so the token is discarded immediately after the clone. The exception is build-alpine-base.yml's commit-hash job, which pushes the updated alpine-base image hash back to master and needs the token to remain. Mark that checkout with an inline zizmor ignore plus a comment explaining the dependency. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
actionlint (run via reviewdog with reporter: github-check) flags any
${{ github.event.pull_request.head.* }} or ${{ github.event.comment.body }}
expression interpolated directly into a run: script because the PR
branch name and comment body are attacker-influenced.
Move HEAD_SHA, HEAD_REF, and BODY into step env: blocks in
ascii-check.yml, commit-messages.yml, request_codeowners_review.yml,
rerun-ci.yml, and spdx.yml. The shell scripts now read "$HEAD_SHA",
"$HEAD_REF", "$BODY" — the values are still attacker-influenced but
shell quoting prevents script injection.
Also quote the reviewers_array expansion in
request_codeowners_review.yml so shellcheck SC2068 stops firing.
Signed-off-by: eriknordmark <erik@zededa.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A composite action's inputs metadata only accepts name, description, required, default, and deprecationMessage; the type key is reserved for reusable-workflow inputs (workflow_call). actionlint correctly rejects "type: string" and "type: boolean" here, which has been failing the Workflow Lint job on every PR touching .github/workflows. Strip the type keys. The clean default switches from the YAML boolean true to the string 'true' so the unchanged inputs.clean == 'true' comparison in this file still resolves the same way; the only existing caller that overrides clean uses clean: false (publish.yml), which coerces to the string "false" exactly as before. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
7 tasks
eriknordmark
changed the title
reviewdog/action-actionlint submits every shellcheck finding as a separate GitHub annotation, including info-level ones. PRs that touch enough workflow files push reviewdog past GitHub's per-check annotation cap and the Workflow Lint job fails on "Too many results (annotations) in diff" rather than on an actual finding. The level input on the action only sets a default severity for findings that don't carry their own; it does not filter the stream. Pass actionlint_flags so shellcheck stops emitting SC2086, SC2046, and SC2035 entirely (boilerplate quoting and glob suggestions that were the bulk of the noise). All actionlint rules and any error-level shellcheck rules still run. Signed-off-by: eriknordmark <erik@zededa.com> Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
eriknordmark
force-pushed
the
artipacked-fix
branch
from
aadfb79 to
e7279fd
Compare
rene
reviewed
May 18, 2026
| # surfaces as a transport-level failure rather than a real | ||
| # lint finding. The remaining shellcheck error-level rules | ||
| # still fire, as do all actionlint rules themselves. | ||
| actionlint_flags: -shellcheck=-e=SC2086,SC2046,SC2035 |
Contributor
There was a problem hiding this comment.
I still don't understand what is the point of disable these specific checks, we might have many others that will also generate lots of annotations and cross the annotation limit. These rules in general are to enforce double quote to prevent globing and word split. I don't think we should ignore it....
Member
|
@eriknordmark these are extracted from the workflow logs, this is what is fixed and should reflect in the Security tab. Zizmor Security Findings: Pre-fix vs Post-fix Comparison
Summary by Rule Type
Notes
|
Member
|
I'll run these on Rene's repo and report back here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Bundle of small, related workflow-security fixes. Lands four commits:
github/workflows: set persist-credentials false on checkouts—zizmor's
artipackedaudit. Addpersist-credentials: falsetoevery
actions/checkoutstep soGITHUB_TOKENis discardedimmediately after the clone and cannot leak via a later artifact
upload. One exception:
build-alpine-base.yml'scommit-hashjobpushes the updated alpine-base hash back to master and needs the
token; carry an inline
# zizmor: ignore[artipacked]plus acomment explaining the dependency.
github/workflows: lift PR-controlled refs to step env—actionlint's
expressionrule. Move${{ github.event.pull_request.head.* }}and${{ github.event.comment.body }}out ofrun:script bodies intostep
env:blocks inascii-check.yml,commit-messages.yml,request_codeowners_review.yml,rerun-ci.yml, andspdx.yml,and quote the
reviewers_arrayexpansion (shellcheck SC2068).github/actions/run-make: drop invalid type keys on inputs—composite-action input metadata does not accept
type:, onlyreusable-workflow inputs do. The bogus keys have been silently
ignored at runtime but actionlint correctly rejects them and the
Workflow Lint job fails on every PR that touches a workflow which
uses
run-make.github/workflows/actionlint: only report error-level findings—pin
level: erroronreviewdog/action-actionlintso reviewdogdoesn't submit shellcheck info/warning annotations. Without this,
PRs that touch enough workflow files hit GitHub's per-check-run
annotation cap ("Too many results (annotations) in diff") and the
lint job fails on transport-level errors rather than on actual
findings.
This is Layer B (mechanical fixes) of the broader zizmor cleanup;
Layer A (
.github/zizmor.yml) shipped separately as #5964 andaddressed the legitimate-by-design findings. The
excessive-permissionswork is split out as #5967, stacked on topof this PR.
PR dependencies
None.
How to test and validate this PR
artipackedfindings (one remains, suppressed inline on the commit-hash job).
origin :` against the public repo still succeed
without credentials (ascii-check, commit-messages, spdx,
request_codeowners_review).
must still be able to push the updated hash to master — exercised
on the next alpine-base bootstrap run.
Changelog notes
None. CI hardening only.
PR Backports
Checklist