Skip to content

Tighten CI security and remove dead code in workflows#7139

Merged
bentsherman merged 6 commits into
masterfrom
more-gha-updates
May 14, 2026
Merged

Tighten CI security and remove dead code in workflows#7139
bentsherman merged 6 commits into
masterfrom
more-gha-updates

Conversation

@ewels
Copy link
Copy Markdown
Member

@ewels ewels commented May 13, 2026

Follow-up to #gha-updates (#7138) after a deeper pass through build.yml, claude.yml, docs.yml. Driven by a manual security review on top of zizmor — no zizmor findings before or after, but the pass surfaced real issues zizmor doesn't catch: secret-dumping debug lines, dead permissions, a multi-line $GITHUB_ENV bug, and a trigger that fires the runner with no possible match.

build.yml

  • Remove env | sort (×2) and cat $HOME/.nextflow/scm. GitHub's log masking is a fragile exact-string replace; dumping every env var or cating a file that contains NXF_GITHUB_ACCESS_TOKEN is the pattern that leaked secrets in the March 2025 tj-actions/changed-files incident. Both looked like leftover debug.
  • Rewrite Get the commit message. ${{ secrets.GITHUB_TOKEN }}, ${{ github.event.pull_request.head.sha }} and ${{ github.repository }} moved into env:; curl + jq swapped for gh api (pre-installed, picks up GH_TOKEN automatically). Same shape as several recent supply-chain incidents — safe today only because GitHub controls every value.
  • GCP credentials → $RUNNER_TEMP. Were being written to $GITHUB_WORKSPACE, which an over-broad upload-artifact glob could exfiltrate by accident. $RUNNER_TEMP is outside the workspace and auto-cleaned.
  • test-e2epermissions: {}. Job dispatches a workflow in seqeralabs/showcase-automation via AUTOMATION_GITHUB_TOKEN (separate PAT) and touches nothing here. Previous actions: write + contents: write on the default token were dead.
  • Heredoc for COMMIT_MESSAGE in $GITHUB_ENV. Old form embedded literal quotes into the value and broke on multi-line / "-containing messages. Latent rather than blocking (run.sh just greps for [e2e prod]) but worth fixing.
  • release permissions → contents: write only. Was actions/contents/packages/pull-requests/issues: write. Verified by grepping *.gradle/*.sh/Makefile for ghcr.io, gh issue, gh pr, gh workflow — only hit is gh release create/upload. Docker pushes go to Docker Hub and public.cr.seqera.io, not ghcr.io.
  • Workflow-level concurrency:. Cancels in-flight pull_request runs on new pushes; master / STABLE-* / test* / dev* are never cancelled so a partial release can't be aborted.

claude.yml

  • Drop assigned from issues: trigger. The if clause only checks @claude in issue.body, so every assignment was starting a runner just to skip.
  • concurrency: keyed on issue/PR number. Multiple @claude mentions on the same thread queue rather than race.

docs.yml

  • Python 3.9.253.14. Renovate had us on a near-EOL branch; no docs-specific reason to stay.
  • cache: 'pip' with cache-dependency-path: docs/requirements.txt. ~30s/PR.
  • Workflow-level concurrency:.

ewels added 4 commits May 13, 2026 11:40
Signed-off-by: Phil Ewels <phil.ewels@seqera.io>
… config

- Add explicit permissions blocks (workflow + job level, contents: read
  where possible) to build, cffconvert, docs, stale
- stale job gets issues: write + pull-requests: write (only what
  actions/stale actually needs)
- Move github.event.pusher.name/email out of the git config run script
  and into env vars to prevent shell injection via a malicious pusher

Signed-off-by: Phil Ewels <phil.ewels@seqera.io>
build.yml:
- Replace `${{ ... }}` template interpolation in `Get the commit message`
  with env vars + `gh api` (was curl with token in template, also let
  drift toward shell injection through the SHA/repo expansions)
- Drop two `env | sort` blocks and a `cat $HOME/.nextflow/scm` debug
  line that printed a file containing NXF_GITHUB_ACCESS_TOKEN. GitHub
  masks known secrets in logs but the protection is fragile
- Write GCP credentials to $RUNNER_TEMP instead of the workspace so they
  cannot be accidentally captured by upload-artifact globs
- test-e2e: drop `actions: write` + `contents: write` (job uses
  AUTOMATION_GITHUB_TOKEN against a different repo, touches nothing here)
- test-e2e: switch the COMMIT_MESSAGE env var to heredoc delimiter so
  multi-line / quote-containing commit messages survive intact
- release: trim permissions to just `contents: write` (gh release create
  is all the make targets need against this repo; Docker pushes go to
  Docker Hub and Seqera CR, not ghcr.io)
- Workflow-level `concurrency:` so PR pushes cancel earlier runs

claude.yml:
- Drop `assigned` from issues trigger (the `if` clause never matched it
  so every assign was burning a runner start)
- `concurrency:` keyed on issue/PR number

docs.yml:
- Bump Python from 3.9.25 (EOL branch) to 3.14
- Add pip cache via setup-python
- Workflow-level `concurrency:`

Signed-off-by: Phil Ewels <phil.ewels@seqera.io>
@ewels ewels force-pushed the more-gha-updates branch from b1cd0e0 to 6a03a53 Compare May 13, 2026 10:29
Base automatically changed from gha-updates to master May 13, 2026 13:20
@bentsherman bentsherman requested a review from pditommaso May 13, 2026 13:21
@bentsherman
Copy link
Copy Markdown
Member

@ewels can you fix the merge conflicts? then I will merge it

@ewels
Copy link
Copy Markdown
Member Author

ewels commented May 13, 2026

@copilot resolve the merge conflicts in this pull request

Signed-off-by: GitHub <noreply@github.com>

Co-authored-by: ewels <465550+ewels@users.noreply.github.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented May 13, 2026

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 5246adc
🔍 Latest deploy log https://app.netlify.com/projects/nextflow-docs-staging/deploys/6a04cba214dcc80008c54476

Copy link
Copy Markdown

Copilot AI commented May 13, 2026

@copilot resolve the merge conflicts in this pull request

Merge conflicts resolved in 83e3634. build.yml had three conflicts (kept the concurrency: block, heredoc-style COMMIT_MESSAGE, and our checkout line without the zizmor ignore comment); docs.yml had two conflicts (kept the concurrency: block and de-duplicated the checkout with: block).

Copilot finished work on behalf of ewels May 13, 2026 19:01
@ewels
Copy link
Copy Markdown
Member Author

ewels commented May 13, 2026

@copilot careful - I think you added "persist-credentials: false" back to the build.yml workflow. But the credentials are needed for the script that pushes the release. This edit was reverted in the upstream PR and comments explaining why added. You just removed this in the merge conflict resolution I think.

Copy link
Copy Markdown

Copilot AI commented May 13, 2026

@copilot careful - I think you added "persist-credentials: false" back to the build.yml workflow. But the credentials are needed for the script that pushes the release. This edit was reverted in the u...

Fixed in 5246adc — removed the persist-credentials: false that was incorrectly added to the release job's checkout during merge conflict resolution. The other three checkouts (build, test, test-e2e) still correctly have persist-credentials: false.

Copilot finished work on behalf of ewels May 13, 2026 19:07
@ewels
Copy link
Copy Markdown
Member Author

ewels commented May 13, 2026

Should be good now I think @bentsherman

@bentsherman bentsherman merged commit 2bec402 into master May 14, 2026
26 of 29 checks passed
@bentsherman bentsherman deleted the more-gha-updates branch May 14, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants