Thanks for opening this PR! We would appreciate it if you could provide us with more info about your PR :pray:

Jul 03 '22 19:07 request-info[bot]

Codecov Report

Base: 89.21% // Head: 89.25% // Increases project coverage by +0.04% :tada:

Coverage data is based on head (91efdd9) compared to base (7f1caf7). Patch coverage: 100.00% of modified lines in pull request are covered.

@@            Coverage Diff             @@
##             main     #151      +/-   ##
==========================================
+ Coverage   89.21%   89.25%   +0.04%     
==========================================
  Files          16       16              
  Lines         482      484       +2     
  Branches       38       38              
==========================================
+ Hits          430      432       +2     
  Misses         51       51              
  Partials        1        1              

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Jul 03 '22 19:07 codecov[bot]

This one runs the SCA check, i.e: dependencies. You could add another step with a custom command of code test which will also test for security issues in your own code, i.e: SAST.

Jul 19 '22 18:07 lirantal

@lirantal added SAST, I'm just not sure I added it correctly: https://github.com/practicajs/practica/pull/151/files#diff-6102d4f2cddb56c446459bde9bf8e8044b87ce3f98a24c2bba99debfd62461dcR21

In the build, I don't see an indication for SCA, only for SAST (Test type: Static code analysis):

Link to build: https://github.com/practicajs/practica/runs/7476884507?check_suite_focus=true

Jul 22 '22 23:07 mikicho

@mikicho correct, because the command directive overrides the default test with code test which means you replace the default SCA with the SAST one.

So, to have both, duplicate that job so that each runs a different scan (parallelism), or you can add another step.

Jul 23 '22 11:07 lirantal

Looks like it is working now, and also that you have a couple of findings: https://github.com/practicajs/practica/runs/7480982678?check_suite_focus=true :-)

I would go ahead and add a .snyk file to ignore those vulns as they seem to be impacting your dev dependencies, right?

Another thing to flag is that you may want to customize the SCA part with this command argument --all-projects (add: command: test --all-projects) in order to scan across monorepos and other package manifest locations.

Jul 23 '22 18:07 lirantal

@lirantal added --all-project option. I'm trying to produce a sarif file for both SAST and SCA but I get this error, is this a bug?

https://github.com/practicajs/practica/runs/7483982273?check_suite_focus=true

Jul 23 '22 22:07 mikicho

@mikicho Maybe merge the CVE scan first then we will figure out the static code analysis?

Jul 24 '22 07:07 goldbergyoni

@mikicho this one is incorrect: https://github.com/practicajs/practica/actions/runs/2725251965/workflow#L27

It should be code test

Aug 01 '22 16:08 lirantal

@lirantal why I can't find code test command in the docs?

Sep 04 '22 08:09 mikicho

@goldbergyoni I fixed the Snyk part :) why does the coverage fail? :( @lirantal I disabled this one because I already added this check in the github actions. p.s: Now I'm not sure what the advantage of the Snyk (GitHub) action is if it already checks for vulns and do the static check for me automatically (?)

Sep 04 '22 20:09 mikicho

@lirantal why I can't find code test command in the docs?

It's there, if you click into snyk code in that list, you get here: https://docs.snyk.io/snyk-cli/commands/code which mentions snyk code test as a subcommand.

@goldbergyoni I fixed the Snyk part :) why does the coverage fail? :( @lirantal I disabled this one because I already added this check in the github actions. p.s: Now I'm not sure what the advantage of the Snyk (GitHub) action is if it already checks for vulns and do the static check for me automatically (?)

The value of the native GitHub integration into Snyk is quite high:

1. You get automatically monitoring your project. GitHub Actions only run when they're executed. Unless you explicitly set some type of cronjob for them, they could theoretically be stalled for months until a new commit or other trigger causes them to execute. You could miss many vulnerabilities during this time frame.
2. The native GitHub integration allows the Snyk bot to open automatic PRs to fix and suggest package upgrades and Dockerfile fixes
3. You can enable status checks as part of the PR and how it behaves (allow it to block the PR if you want to) which is unlike a GitHub Action execution that could be an all-or-nothing, depending on how you built it.

Sep 05 '22 06:09 lirantal

@mikicho I could not understand why codecov is angry. Since we know that there is no code logic here or coverage drop, let's ignore this status fail. Do you have enough rights?

Sep 05 '22 15:09 goldbergyoni

@mikicho I could not understand why codecov is angry. Since we know that there is no code logic here or coverage drop, let's ignore this status fail. Do you have enough rights?

@goldbergyoni I'm afraid it will break the ci because it will fail later PRs, am I wrong?

Sep 05 '22 19:09 mikicho

Ok lets merge this, and decide later about the snyk github check, (the integration is working, I just disabled the PR check)

Sep 08 '22 17:09 mikicho