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

hardcode gitleaks version #78

Merged
merged 1 commit into from
Aug 2, 2022
Merged

hardcode gitleaks version #78

merged 1 commit into from
Aug 2, 2022

Conversation

zricethezav
Copy link
Collaborator

@zricethezav zricethezav commented Aug 1, 2022

@weineran mind reviewing this too? I think we should default to using a hardcoded version rather than always pulling latest for two reasons:

  1. We might accidentally push noisy, false positive prone rules to the default gitleaks configuration which would automatically get pulled into the gitleaks-action, causing unnecessary noise to our users.
  2. It would get us looking at the gitleaks-action code more frequently.

@zricethezav zricethezav requested a review from weineran August 1, 2022 22:17
Copy link
Contributor

@weineran weineran left a comment

Choose a reason for hiding this comment

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

Suggested improvement for future PR

let gitleaksVersion =
process.env.GITLEAKS_VERSION || (await gitleaks.Latest(octokit));

let gitleaksVersion = process.env.GITLEAKS_VERSION || "8.9.0";
Copy link
Contributor

Choose a reason for hiding this comment

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

Not specific to this PR, but what happens if the user sets the env variable to an invalid version string by mistake? e.g. "8.90" instead of "8.9.0"

Can we catch that on line 120?

@weineran weineran merged commit f2f91c8 into master Aug 2, 2022
@weineran weineran deleted the hardcode-version branch August 2, 2022 22:27
spaze added a commit to spaze/michalspacek.cz that referenced this pull request Jan 27, 2023
spaze added a commit to spaze/michalspacek.cz that referenced this pull request Jan 27, 2023
spaze added a commit to spaze/michalspacek.cz that referenced this pull request Jan 27, 2023
spaze added a commit to spaze/michalspacek.cz that referenced this pull request Jan 27, 2023
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.

2 participants