salus
salus copied to clipboard
coinbase
Update Semgrep to 0.59.0, disable version check
This updates Semgrep to 0.59.0 which has some important grammar fixes to various languages. This PR also disables the semgrep version check, as the version check output goes to STDERR for some semgrep invocations (specifically I noticed it when putting semgrep rules in an external file), and can cause issues in CI pipelines that look at STDERR.
FYI -- I've tested this with federacy/scan-action and it seems to work just fine. It's kind of an issue for our deployment as we're relying on Salus to run Semgrep in CI, and the old version of Semgrep included in Salus doesn't support Javascript private member syntax, which our codebase makes heavy use of.
@zbuc Thank you for your PR. Your fork is out of date with master, could you update?
Also, for some reason circle tests did not run with your PR. Hopefully it was just a circle ci glitch and tests will run with your update
Also, looks like the Semgrep upgrade will cause unit tests in spec/lib/salus/scanners/semgrep_spec.rb to fail. Could you do the following to fix the tests?
First, update the two occurences of
Could not parse ... as js to
Could not parse ... as javascript
Then, in
context "invalid pattern or settings which causes error" do
it "should record the STDERR of semgrep" do
Update
expect(errors[0][:stderr]).to include("Pattern could not be parsed as a Python " \
"semgrep pattern (error)\n\tCLI Input:1-1")
to
expect(errors[0][:stderr]).to eq("\n\n (error)\n\t")
@ghbren I've implemented all your feedback, thanks. I think the CircleCI tests didn't run originally because of a permissions thing with first-time PR requesters.
@zbuc Thanks for the updates. I couldn't find any documentation on this permission thing with first-time PR requester? Any chance you know how to fix the permission thing?
@ghbren my best guess is that it's related to this: https://docs.github.com/en/github/administering-a-repository/managing-repository-settings/disabling-or-limiting-github-actions-for-a-repository#configuring-required-approval-for-workflows-from-public-forks
@zbuc Just wanted to let you know I filed a ticket with github on why all the ci/circleci jobs got stuck, and still waiting for their reply.
@ghbren are you able to manually trigger them somehow? Or maybe force pushing to the branch will help?
Has there been any update from GitHub? Are you able to manually trigger the checks to run or check the settings for PRs from public forks?
Has there been any update from GitHub? Are you able to manually trigger the checks to run or check the settings for PRs from public forks?
Github told us to reach out to circle/ci support instead, and I'm not sure if this is actually a circle/ci issue. Would you be able to re-create this PR and see if that resolves the issue? If not, would you mind if we authorize the PR on your behalf, like we paste your changes and create the PR?
@ghbren I created https://github.com/coinbase/salus/pull/431 to try to get past the CI issue -- if that doesn't work feel free to copy/paste the changes out to a new PR.
Thank you!