Skip to content

Conversation

@peppescg
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Co-authored-by: Dániel Kántor <[email protected]>
@peppescg peppescg force-pushed the semgrep_secrets_scanning branch from 5a760b4 to b4a51d2 Compare December 19, 2024 15:31
@peppescg peppescg marked this pull request as ready for review December 19, 2024 15:36
@peppescg peppescg requested a review from a team as a code owner December 19, 2024 15:36
Copy link
Member

@evankanderson evankanderson left a comment

Choose a reason for hiding this comment

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

A few small nits, but approving in case they aren't needed.

Comment on lines +30 to +31
import future.keywords.if
import future.keywords.every
Copy link
Member

Choose a reason for hiding this comment

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

I think import rego.v1 brings you both of these.

Suggested change
import future.keywords.if
import future.keywords.every
import rego.v1

semgrep_hooks[hook_id].id == "semgrep"
}

message := "" if allow
Copy link
Member

Choose a reason for hiding this comment

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

I believe message is only used when allow is false, so you can just do:

Suggested change
message := "" if allow
message := "Semgrep pre-commit hook is not configured for the repository"

repo_data := parsed_data.repos[repo_id]
endswith(repo_data["repo"], "https://github.com/semgrep/pre-commit")
semgrep_hooks = repo_data["hooks"]
semgrep_hooks[hook_id].id == "semgrep"
Copy link
Member

Choose a reason for hiding this comment

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

It looks like Semgrep has 4 configurations: semgrep, semgrep-ci, semgrep-docker, and semgrep-docker-develop. It looks like this requires the semgrep one and not (for example) the CI one. Is this usage intentional?

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.

5 participants