Skip to content

Harden CI workflows and add a zizmor lint workflow to the CI#19914

Draft
dkasak wants to merge 28 commits into
developfrom
dkasak/ci-fixes
Draft

Harden CI workflows and add a zizmor lint workflow to the CI#19914
dkasak wants to merge 28 commits into
developfrom
dkasak/ci-fixes

Conversation

@dkasak

@dkasak dkasak commented Jul 6, 2026

Copy link
Copy Markdown
Member

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

Signed-off-by: Denis Kasak dkasak@termina.org.uk

Notes for reviewers

This should be reviewable commit by commit. I generally attempted to go one workflow file at a time, except when I forgot something and then went back, or in the case of persist-credentials: false which is noisy enough so I did it all at once.

In triage-incoming.yml I removed issues: write because it seems to me that the write permission is never used for GITHUB_TOKEN. Instead the workflow calls the triage-incoming.yml reuseable workflow from matrix-org/backend-meta and passes it ELEMENT_BOT_TOKEN directly.

Another thing of note is that tests.yml was using an unpinned postgres image which I believe defaulted to latest, but I chose postgres:17 instead because that's the latest explicitly tested version in that file. Not sure if that's correct.

I generally replaced all interpolated expressions with a variant that passes them through an env var, regardless of whether they would be at all controllable by an attacker or not, because I think this pattern is the better practice. It automatically demonstrates that there's no way for shell injection to happen rather than having to reason through each interpolation spot.

Apart from fixing Zizmor and CodeQL lints regarding workflows, this also fixes a cache key which was using github.context.sha, which is undefined in GHA, instead of github.sha

dkasak added 28 commits July 6, 2026 20:58
`github.context` isn't defined in this context, so the entire
`github.context.sha` expression evaluates to an empty string. As the
comment above it says, the recipe was cribbed from a TypeScript script
where `github.context` *is* defined, unlike within a GHA workflow file.

The correct replacement in a workflow file is `github.sha`.
@dkasak dkasak force-pushed the dkasak/ci-fixes branch from 5a92fca to 15e8acd Compare July 7, 2026 15:13
@dkasak dkasak changed the title Harden CI workflows by specifying permissions Harden CI workflows and add a zizmor lint workflow to the CI Jul 7, 2026
@dkasak dkasak marked this pull request as ready for review July 7, 2026 15:25
@dkasak dkasak requested a review from a team as a code owner July 7, 2026 15:25
@dkasak

dkasak commented Jul 7, 2026

Copy link
Copy Markdown
Member Author

Ah oops, seems I have to re-run while giving zizmor some more access so it can detect some unpinned deps so converting to draft again while I sort that out.

@dkasak dkasak marked this pull request as draft July 7, 2026 15:27
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.

1 participant