Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typo-check in CI, run only for pull requests because of security reasons #4433

Merged
merged 1 commit into from
Jun 12, 2024

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jun 12, 2024

Motivation

typo-check is currently broken in CI,

Downloading 'typos' v1.22.5
--[20](https://github.com/apache/bookkeeper/actions/runs/9485228747/job/26136650763?pr=4426#step:3:22)24-06-12 15:31:09--  https://github.com/crate-ci/typos/releases/download/v1.[22](https://github.com/apache/bookkeeper/actions/runs/9485228747/job/26136650763?pr=4426#step:3:24).5/typos-v1.22.5-x86_64-unknown-linux-musl.tar.gz
Resolving github.com (github.com)... 140.82.112.3
Connecting to github.com (github.com)|140.82.112.3|:443... connected.
HTTP request sent, awaiting response... 404 Not Found
20[24](https://github.com/apache/bookkeeper/actions/runs/9485228747/job/26136650763?pr=4426#step:3:26)-06-12 15:31:09 ERROR 404: Not Found.

example: https://github.com/apache/bookkeeper/actions/runs/9485228747/job/26136651150?pr=4426#step:3:21

Changes

  • use v1.22.4 which contains the binary for linux-musl
  • limit running typos check for pull requests because of security reasons
    • we shouldn't use any external actions for builds that run in the security context of the repository

@lhotari lhotari requested a review from shoothzj June 12, 2024 15:55
@lhotari lhotari self-assigned this Jun 12, 2024
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

@lhotari
Copy link
Member Author

lhotari commented Jun 12, 2024

I also reported the issue here: crate-ci/typos#1034 . However this PR will also address the security hole that was opened by using an external action in our CI.

@lhotari lhotari merged commit ac2f8a2 into apache:master Jun 12, 2024
23 checks passed
@shoothzj
Copy link
Member

Thanks I'll make sure to adjust the approach accordingly for future contributions.

Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants