snyk-prevent-gh-commit-status fails for new dependencies that have not been previously monitored

[unitysipu]

using this tool in a pipeline when new projects are introduced that weren't monitored previously throw an error with exit code 2

Couldn't find find this resource
2021-04-22T09:35:02.505Z snyk NotFoundError: Snyk API - Could not find a monitored project matching.                                         Make sure to specify the right org when snyk test using --org
    at Object.getProjectUUID (/snapshot/project/node_modules/snyk-delta/dist/snyk/snyk.js:18:15)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at Object.getDelta (/snapshot/project/node_modules/snyk-delta/dist/index.js:55:35)
    at main (/snapshot/project/dist/index.js:43:28)
script returned exit code 2

This seems to be an issue of snyk-delta, but i'd like to understand the appropriate implementation for a CI pipeline that handles this scenario. If i run snyk monitor prior to running snyk-delta/prevent-gh-commit-status i will never find 'new' issues.

Ultimately it would make sense if a project has not been monitored before, it would be considered as newly introduced, disregarding the delta for that particular file.

If you run snyk for the very first time through the pipeline in an onboarding scenario i will also hit this situation.

[aarlaud]

The idea is to have a baseline at first. snyk-delta compares against a baseline. this is meant for PRs to give you the delta while the commit to the default branch updates the baseline on every commit to it.

With that said, what is the desired behavior? If you're adding a new project (what snyk considers a project) that doesn't have a baseline, but has vulns in it, do you want to report this as a failed check because it's new? or you want to let it pass so you can then get a baseline for the following PRs?

The concern is that if this is a new thing you're just adding from somewhere else, and without baseline but with vulns we return a failed check which can preclude you from merging it to the default branch and therefore getting a baseline, so you get a bit blocked.

Admittedly, it really depends on the required or not required parameter for those statuses.

Thoughts?

[unitysipu]

For newly introduced package manifests that aren't being monitored yet, the baseline on the server is of course "0", which means any new potential issues introduced by that new manifest are "new issues". They shouldn't be considered as errors, like they are now. Newly introduced manifests that aren't yet monitored should be treated the same way as new vulnerabilities in existing package manifests. I can implement flags that allow users to control the CI behavior if this should fail the pipeline or not.

I think it's fine if the check is considered as "failed" as long as the message the user sees is sensible and allows them to manage the problem either by just ignoring them and merging or going back and fixing their branch / PR. Snyk monitor will only be run on the default / main branch.

[unitysipu]

I think silently passing a new manifest with vulnerabilities to the main branch (and therefore likely to production) sounds like something we might not want, because the whole purpose of snyk is to inform the developers of potential issues they are introducing. Running snyk test / snyk prevent on all non-default branches can inform of such issues immediately before the PR even goes for review.

[aarlaud]

okey dokey, I'll implement this over the weekend hopefully, Monday otherwise.

[unitysipu]

@aarlaud pls don't work outside of office hours :)

[aarlaud]

1.2.0 should to the trick. https://github.com/snyk-tech-services/snyk-prevent-gh-commit-status/releases/tag/v1.2.0 or npm install. let me know if any bugs

[unitysipu]

Scanning with the latest version i get this, posting the github checks appears to fail. With prior version i see the previous behavior of failing unmonitored files.

(node:433) UnhandledPromiseRejectionWarning: Error: Error: Request failed with status code 422
    at main (/snapshot/project/dist/index.js:90:15)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:433) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:433) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.