snyk / snyk-to-html

export test reports from CLI to html
Other
90 stars 77 forks source link

snyk-to-html should calculate a return code #130

Open sebsnyk opened 2 years ago

sebsnyk commented 2 years ago

At the moment, snyk-to-html "eats" the return code of snyk. This prevents a easy solution to the usecase of generating a HTML artifact for a failed snyk run within a CI/CD process, for example.

Given this invocation in a repo like juice-shop:

snyk test --json

Then we can see JSON output and the return code $? is correctly set (to 1).

If you now add snyk-to-html:

snyk test --json | snyk-to-html > report.html

Then the return code is 0.

In a sense, this is correct. snyk-to-html did not fail to do its job. However, during the above mentioned CI/CD usecase, more code is needed:

First, typically in a before_script area, a set +e is needed to prevent the CI from aborting the step when a command failed. Then, code like this is required:

snyk test --json-file-output=results.json
RESULT=$?
snyk-to-html -o results-open-source.html < results.json
exit $RESULT

Using this approach, the snyk exit code is captured in $RESULT and used to exit the step.

Depending on the CI system, you can then see the step failed AND see the reason in a HTML file artifact.

IMHO, there are different approaches to solving this:

  1. Let snyk include the return code in the JSON file, so that snyk-to-html can reuse it. This is helpful as we support more than 1 exit code https://docs.snyk.io/snyk-cli/cli-reference#exit-codes
  2. Let snyk-to-html perform a simple calculation and exit with 1 if vulnerabilities are found
  3. Recommend the above code snippet within our CI/CD examples.

For 1 & 2, I could see a new --set-return-code argument which (if specified) would perform the logic so that we don't break backwards compatibility.

CC @alexeisnyk @patricia-v

theabecastillo commented 8 months ago

Thanks @sebsnyk, this helped me understand a few things regarding how snyk-to-html works. That feature --set-return-code should be added, it is a must.

In my case I will add a code snippet that might be helpful for others. I needed to scan only the modified files on the current Merge Request. I'm using Gitlab.

      # MODIFIED_FILES contains the list of modified files in your merge request 
      MODIFIED_FILES=$(git diff --name-only ${CI_MERGE_REQUEST_DIFF_BASE_SHA} --)
      for filePath in $MODIFIED_FILES; do
      # I don't need to scan the .gitlab-ci.yaml at this point, if you do, remove the if condition
        if [ "$filePath" != ".gitlab-ci.yml" ]; then
          # Execute the scan only in the modified files not the whole repository, change the flag if exit with a non-zero condition
          snyk code test "$(pwd)/$filePath" --severity-threshold=medium --json > results.json | snyk-to-html -o report.html
        fi
      done

Please note that, at the moment of writing this message, the following error is produced:

[Error: ENOENT: no such file or directory, open '/builds/core/index.php'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: '/builds/core/index.php'
}

The report is generated, the artefact is stored correctly, but unfortunately, the report is not useful at this time since it cannot relate the issues to the right path.

According to the support team:

The tool snyk-to-html runs from the location of the json file, which should be at the root of the code test, and as such there is no way to change that location by design.

Please add a feature to change the base path of the files in the report.