Harden Samples CI workflow for zizmor and add workflow linting#95
Conversation
Follow-up to #80, which added the zizmor pre-commit hook but excluded run-samples.yml (the repo's only substantive workflow), making the hook effectively a no-op. This fixes the underlying findings and removes the exclude so zizmor actually lints the workflow, locally and in CI. run-samples.yml: - Add top-level and grant each job only - Pin all actions to full commit SHAs with version comments (unpinned-uses) - Set on both checkout steps (artipacked) - Route github.event.inputs.run_mode, matrix.shard/splits and runner.temp through step-level env instead of inline interpolation (template-injection) - Quote and in the MSSQL step (shellcheck) .pre-commit-config.yaml: - Remove the run-samples.yml exclude on the zizmor hook lint_workflows.yml: - Add a dedicated workflow-linting job (actionlint + zizmor) mirroring localstack-pro, for CI parity and shellcheck coverage on run: blocks
|
@copilot please review the changes introduced in this PR and explained in the description field and provide suggestions and observations, if any. |
I reviewed the PR changes ( One optional tightening you could consider: in I don’t see any blocker-level issues. |
paolosalvatori
left a comment
paolosalvatori
left a comment
There was a problem hiding this comment.
LGTM (and Copilot), ship it! 🎉 P.S. Don't work in the middle of the night! 🌙
|
FYI @purcell |
Follow-up to #80, which added the zizmor pre-commit hook but excluded run-samples.yml (the repo's only substantive workflow), making the hook effectively a no-op. This fixes the underlying findings and removes the exclude so zizmor actually lints the workflow, locally and in CI. run-samples.yml: - Add top-level and grant each job only - Pin all actions to full commit SHAs with version comments (unpinned-uses) - Set on both checkout steps (artipacked) - Route github.event.inputs.run_mode, matrix.shard/splits and runner.temp through step-level env instead of inline interpolation (template-injection) - Quote and in the MSSQL step (shellcheck) .pre-commit-config.yaml: - Remove the run-samples.yml exclude on the zizmor hook lint_workflows.yml: - Add a dedicated workflow-linting job (actionlint + zizmor) mirroring localstack-pro, for CI parity and shellcheck coverage on run: blocks
Motivation
PR #80 introduced the
zizmorpre-commit hook for GitHub Actions securitylinting, but had to exclude
run-samples.yml(exclude: ^\.github/workflows/run-samples\.yml$)to land a test-regression fix without first resolving the findings in that
workflow.
Since
run-samples.ymlis the only substantive workflow in this repo, thatexclude left
zizmorscanning only the already-hardenedpre-commit.yml—effectively a no-op. This PR is the agreed follow-up: fix the actual findings,
remove the exclude so
zizmorlints the workflow for real, and align therepo's CI workflow-linting with localstack-pro.
Changes
.github/workflows/run-samples.yml— resolve allzizmorfindings:permissions: {}and grant each job onlycontents: read(excessive-permissions)# vX.Y.Zcomment (unpinned-uses)persist-credentials: falseon both checkout steps (artipacked)github.event.inputs.run_mode,matrix.shard/matrix.splits, andrunner.tempthrough step-levelenvinstead of inline interpolation (template-injection)$(lsb_release -rs)and$GITHUB_PATHin the MSSQL step (shellcheck, surfaced by actionlint).pre-commit-config.yaml:run-samples.ymlexclude from thezizmorhook.github/workflows/lint_workflows.yml(new):actionlint+zizmor,mirroring localstack-pro for CI parity and shellcheck coverage on
run:blockspermissions,persist-credentials: falseIntentionally not copied from localstack-pro: its
zizmor.yml(disablesexcessive-permissions, which would undo the fix here) and itsactionlint.yaml(only declares self-hosted runner labels, irrelevant to GitHub-hosted runners).
Tests
Verified locally against all workflows (
pre-commit.yml,run-samples.yml,lint_workflows.yml):actionlint(with shellcheck) — exit 0, no findingszizmorv1.25.2, default persona — "No findings to report"pre-commit run zizmor --all-files) — PassedRelated