Skip to content

Semgrep pre-commit hook#241

Open
peppescg wants to merge 2 commits intomindersec:mainfrom
peppescg:semgrep_secrets_scanning
Open

Semgrep pre-commit hook#241
peppescg wants to merge 2 commits intomindersec:mainfrom
peppescg:semgrep_secrets_scanning

Conversation

@peppescg
Copy link
Copy Markdown
Contributor

No description provided.

@CLAassistant
Copy link
Copy Markdown

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 <kd@stacklok.com>
@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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