Skip to content

Conversation

@ferozsalam
Copy link
Contributor

@ferozsalam ferozsalam commented Jan 28, 2026

  • Add a comment to clearly indicate the untrusted code checkout
  • Split the release and dev CI steps into two separate workflows
  • Explicitly specify the permissions of the GitHub token in the workflows, to minimize the impact of token compromise.
  • Split the individual workflows into separate jobs, such that the build happens in a different job to the push, which will allow the separation of the untrusted code build process from the authenticated push of the built artifact.

Thanks to @cedricvanrompay for reporting this.

@ferozsalam ferozsalam added the area/CI Continuous Integration testing issue or flake label Jan 28, 2026
@ferozsalam ferozsalam force-pushed the pr/feroz/cli-build-hardening branch from b29095d to ba44f1a Compare January 28, 2026 10:52
@ferozsalam ferozsalam force-pushed the pr/feroz/cli-build-hardening branch from ba44f1a to e6230fc Compare January 28, 2026 11:01
@ferozsalam ferozsalam force-pushed the pr/feroz/cli-build-hardening branch from e6230fc to 1e13657 Compare January 28, 2026 11:27
@ferozsalam ferozsalam force-pushed the pr/feroz/cli-build-hardening branch from 1e13657 to 64ba43d Compare January 28, 2026 11:49
@ferozsalam ferozsalam force-pushed the pr/feroz/cli-build-hardening branch from 64ba43d to 167f632 Compare January 28, 2026 11:57
@ferozsalam
Copy link
Contributor Author

Test of the CI build here, used pull_request as the trigger to test: https://github.com/cilium/cilium-cli/actions/runs/21437055364/job/61730258277?pr=3174

Not certain of the best way to test the equivalent release build.

@ferozsalam ferozsalam marked this pull request as ready for review January 28, 2026 12:00
@ferozsalam ferozsalam requested review from a team as code owners January 28, 2026 12:00
@ferozsalam ferozsalam requested a review from brlbil January 28, 2026 12:00
Copy link
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

Minor nit but LGTM. I reviewed mainly just by doing diff -u between the original workflow and each new workflow, and between each new workflow here to identify any discrepancies.

- Add a comment to clearly indicate the untrusted code checkout
- Split the release and dev CI steps into two separate workflows
- Explicitly specify the permissions of the GitHub token in the workflows,
  to minimize the impact of token compromise.
- Split the individual workflows into separate jobs, such that the build
  happens in a different job to the push, which will allows the separataion of
  the untrusted code build process from the authenticated push of the built
  artifact.

Signed-off-by: Feroz Salam <[email protected]>
Reported-by: Cédric Van Rompay <[email protected]>
@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 29, 2026
Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Test of the CI build here, used pull_request as the trigger to test: https://github.com/cilium/cilium-cli/actions/runs/21437055364/job/61730258277?pr=3174

Not certain of the best way to test the equivalent release build.

So far we've been testing changes to release workflows only when cutting a release anyway, making adjustments as we hit issues. This approach is not optimal but I can't think of a good way to test this without too much complication. So IMO it is fine to merge this untested and follow-up with fixes in case we hit issues on the next release.

@tklauser tklauser merged commit 04f98c9 into main Jan 29, 2026
6 checks passed
@tklauser tklauser deleted the pr/feroz/cli-build-hardening branch January 29, 2026 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/CI Continuous Integration testing issue or flake ready-to-merge This PR has passed all tests and received consensus from code owners to merge.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants