Skip to content

chore(gha): remove benchmark github workflows#6322

Merged
mcollina merged 2 commits intomainfrom
sec-p
Sep 18, 2025
Merged

chore(gha): remove benchmark github workflows#6322
mcollina merged 2 commits intomainfrom
sec-p

Conversation

@Eomm
Copy link
Member

@Eomm Eomm commented Sep 18, 2025

Even tho these workflow are triggered manually, we must remove them 😢

This is not a farewell, to add them back, we need another workflow:

  • we move these workflow into: https://github.com/fastify/benchmarks
  • we add the label as before and we call a webhook to trigger the fastify/benchmarks workflow
  • the workflow will report back into the fastify's repo PR
image

https://scorecard.dev/viewer/?uri=github.com%2Ffastify%2Ffastify

Checklist

Signed-off-by: Manuel Spigolon <behemoth89@gmail.com>
Signed-off-by: Manuel Spigolon <behemoth89@gmail.com>
@github-actions github-actions bot added the github actions Github actions related label Sep 18, 2025
@mcollina
Copy link
Member

Why?

@Eomm
Copy link
Member Author

Eomm commented Sep 18, 2025

This is the full documentation:
https://github.com/ossf/scorecard/blob/40576783fda6698350fcbbeaea760ff827433034/docs/checks.md#dangerous-workflow

Since we are checking out external code, a malicious user may run external code in our org GHA context.

While this is almost impossible because:

  • we must add a label to run the workflow
  • and the label is automatically removed after the benchmark run
  • SO, a user can't commit an un-checked code while we are adding the label

Nevertheless, this is considered a potential security threat and is flagged as critical by the OpenSSF working group.
Reading the documentation there is not a why to mark this record as "invalid" or "we know it but it can't happen"

Our scope is to reduce any potential issue to zero, even tho it may require some extra work to move the GHA into a repo that could be exploited without harm

@Fdawgs
Copy link
Member

Fdawgs commented Sep 18, 2025

Happy for them to be removed for now to reduce any immediate potential nastiness.

We can revisit them and do what is suggested in https://securitylab.github.com/resources/github-actions-preventing-pwn-requests/ and split the workflows.

Note that https://github.com/fastify/workflows/blob/main/.github/workflows/plugins-benchmark-pr.yml also has the same codeql alerts.

@Uzlopak Uzlopak changed the title chore(gha): removing benchmark scripts chore(gha): removing benchmark github workflows Sep 18, 2025
@Uzlopak Uzlopak changed the title chore(gha): removing benchmark github workflows chore(gha): remove benchmark github workflows Sep 18, 2025
Copy link
Contributor

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mcollina mcollina merged commit 39d2e10 into main Sep 18, 2025
35 checks passed
@mcollina mcollina deleted the sec-p branch September 18, 2025 16:13
@Eomm Eomm mentioned this pull request Sep 21, 2025
2 tasks
Fdawgs added a commit to fastify/light-my-request that referenced this pull request Oct 18, 2025
See fastify/fastify#6322

Signed-off-by: Frazer Smith <frazer.dev@icloud.com>
mcollina pushed a commit to fastify/light-my-request that referenced this pull request Oct 18, 2025
See fastify/fastify#6322

Signed-off-by: Frazer Smith <frazer.dev@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github actions Github actions related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants