Lint GitHub Actions workflows with actionlint

[ascheman]

This adds a small CI job running actionlint over .github/workflows/** — it checks workflow syntax, expression contexts, and embedded shell (via shellcheck). We have no workflow linting today.

It triggers on any branch push and any PR (path-filtered to .github/workflows/**), so contributors get early feedback even on their forks, before a PR exists. A concurrency group cancels superseded runs on the same ref.

On its first run it flagged 4 real issues — unquoted $GITHUB_ENV / $GITHUB_PATH (SC2086) in maven.yml's two "Extract Maven distribution" steps — fixed in the second commit.

Opening as a draft to gather opinions before committing to it — @slawekjaranowski (and others):

• Do we want workflow linting on apache/maven?
• If yes, should the reusable logic live in apache/maven-gh-actions-shared (so every Maven repo adopts it over time via a small caller workflow), rather than a standalone workflow per repo? This PR is the standalone demo; happy to move it to the shared repo if that's preferred.
• Mechanism/pinning: docker:// image (digest-pinned for a final version) vs reviewdog/action-actionlint vs a pinned download — preference?
• Advisory, or a required check?

Note: actionlint does not flag the if: ${{ ... }} pattern, so that stays as-is.

[gnodet]

Review of draft PR — since this is explicitly seeking feedback:

The SC2086 quoting fixes in maven.yml are correct and should be uncontroversial.

The new actionlint.yml workflow is well structured — proper ASF header, path filtering, concurrency group, minimal permissions, persist-credentials: false. A few observations:

1. Docker image pinning: The comment in the workflow already acknowledges this, but to be explicit — the project convention (visible in maven.yml) is SHA-pinned actions (uses: actions/checkout@3d3c42e5aac5...). For docker:// images the equivalent is a digest pin: docker://rhysd/actionlint@sha256:.... Before merging, this should be aligned with the project convention.

2. Trigger scope vs. maven.yml: The push trigger has no branches filter (unlike maven.yml which restricts to master), meaning actionlint runs on every branch push touching workflows. The pull_request trigger similarly has no branches filter. This is probably intentional for a linting workflow, but it's a different scope than the main CI — worth an explicit note in the workflow comment if kept.

3. Regarding the draft questions — the shared-workflow approach (maven-gh-actions-shared) seems like the right long-term home if multiple Maven repos should adopt this, since it avoids duplicating the workflow definition. This standalone version is good as a proof of concept.

Overall: clean PR, sensible addition. The quoting fixes are independently mergeable.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Per project convention, this should be pinned by image digest rather than version tag:

Suggested change

	with:
	uses: docker://rhysd/actionlint@sha256:<digest>

You can find the digest with:

docker pull rhysd/actionlint:1.7.12
docker inspect --format='{{index .RepoDigests 0}}' rhysd/actionlint:1.7.12

The code comment at line 52-54 already flags this — just leaving a concrete suggestion for the final version.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Adds a GitHub Actions workflow that lints other workflows using actionlint, and fixes the initial shellcheck findings it reported in the existing Maven workflow.

Changes:

• Add .github/workflows/actionlint.yml to run actionlint on workflow changes (push + PR), with concurrency cancellation.
• Fix shellcheck SC2086 findings in .github/workflows/maven.yml by quoting $GITHUB_ENV / $GITHUB_PATH.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
.github/workflows/maven.yml	Quotes $GITHUB_ENV/$GITHUB_PATH in two steps to satisfy shellcheck/actionlint.
.github/workflows/actionlint.yml	Introduces a new CI workflow to lint .github/workflows/** with actionlint.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gnodet]

Follow-up review after force-push — the approach has changed significantly.

Good rework. Switching from a standalone docker://rhysd/actionlint image to github/super-linter/slim is a pragmatic solution: it satisfies the ASF allowed-actions policy (GitHub-owned), gives you actionlint + shellcheck in one step, and the SHA pin (b807e99d... = v7) follows the repo convention. The workflow-level comment explaining the rationale is helpful.

A few observations on the new version:

1. github/super-linter fork freshness: The github/super-linter fork (v7, Aug 2024) lags the upstream super-linter/super-linter by 1300+ commits — that's two years of actionlint updates, bug fixes, and linter version bumps. If the ASF policy were ever relaxed to allow super-linter/super-linter, switching would be trivial. For now the trade-off is fine, just worth noting.

2. push trigger scope: Still no branches filter on push (unlike maven.yml which restricts to master). As noted in the previous review, this means the workflow runs on every branch push touching .github/workflows/**, including contributor forks. This is arguably desirable for a linting workflow (early feedback), but it's a departure from maven.yml's pattern — the PR description acknowledges this is intentional.

3. Previous review findings addressed:

   • ✅ Image pinning → now SHA-pinned github/super-linter/slim@b807e99d...
   • ✅ Permissions → contents: read instead of the old {}
   • ✅ Checkout SHA → matches maven.yml (3d3c42e5... = v7.0.1)

4. The SC2086 quoting fixes in maven.yml are correct and independently mergeable.

No blocking issues found. The draft questions in the PR description (standalone vs. shared repo, advisory vs. required check) are the right discussion points for the community.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[ascheman]

Update after review — reworked, and now green on CI.

The original docker://rhysd/actionlint approach was blocked by the ASF allowed-actions policy (the run was a startup_failure — external Docker images aren't on the org allowlist), so digest-pinning wouldn't have helped. Reworked to:

• github/super-linter/slim — GitHub-owned, so it's on the ASF allowlist — restricted to VALIDATE_GITHUB_ACTIONS, i.e. it runs actionlint. SHA-pinned (b807e99…, v7).
• Added fetch-depth: 0 (super-linter needs full history).

CI is green now ✅. This also resolves the earlier feedback: @gnodet's pinning note (now SHA-pinned) and the Copilot permissions note (now contents: read).

Bonus: super-linter bundles shellcheck/yaml/etc., so coverage can be extended later via more VALIDATE_* flags without allowlisting new actions; and the SHA+comment pin is auto-renovated by the repo's existing github-actions Dependabot.

Supporting data: I ran actionlint across 121 locally-checked-out Maven repos — 17 have ≥1 finding (86 total). ~79% are SC2086 quoting nits, but there's also a real broken shell test in maven-gh-actions-shared and a few stale action pins. So a shared reusable workflow in apache/maven-gh-actions-shared would catch these ecosystem-wide, adopted per-repo over time — which is the open question here: standalone per-repo vs centralize? WDYT @slawekjaranowski?

[Copilot]

🟡 Changes recommended

The new workflow hard-codes DEFAULT_BRANCH: master, which can cause incorrect behavior on forks with different default branch names and undermines the stated goal of early feedback for contributors on forks.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Lite

[gnodet]

Incremental review after rebase + fetch-depth: 0 addition.

The only delta since the previous review is adding fetch-depth: 0 — makes sense, super-linter uses the commit range for its diff mode. With VALIDATE_ALL_CODEBASE: true it is less critical (all files are linted regardless), but it avoids a warning from super-linter about shallow clones, so no objection.

Re Copilot's latest flag on DEFAULT_BRANCH: master: the repo's default branch is master, so the value is correct. That said, the super-linter docs note that on GitHub Actions it auto-detects the default branch, so the explicit DEFAULT_BRANCH env var could simply be removed — it's unnecessary and removing it would also sidestep the fork concern Copilot raised.

No new issues beyond what was noted in the previous reviews.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Re-review after latest push (commit 5854606)

The only delta since the previous review is fetch-depth: 0 on the checkout step, with a comment stating super-linter needs full history to resolve the commit range.

One observation:

• fetch-depth: 0 vs VALIDATE_ALL_CODEBASE: true: With VALIDATE_ALL_CODEBASE: true, super-linter lints every matching file in the repo — it doesn't diff against DEFAULT_BRANCH, so there's no commit range to resolve and a shallow clone would suffice. The fetch-depth: 0 becomes relevant when VALIDATE_ALL_CODEBASE is set to false (lint only changed files, which requires the merge-base). If the intent is to lint all workflow files on every run, the full clone is unnecessary overhead; if the intent is to eventually switch to changed-files-only (which makes more sense for a large repo), then VALIDATE_ALL_CODEBASE should be false. Either way, one of the two settings is mismatched.

No other changes. Previous review observations (push trigger scope, super-linter fork freshness) still apply.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[slawekjaranowski]

why not, we can also add it to shared-action