SARIF report upload fails because of deleted file

[wesley-dean-flexion]

Describe the bug

TL;DR:
Megalinter's SARIF reporter was failing due to matching a file that no longer existed.

Longer version

 - Image Creation Date: 2022-08-01T06:04:12Z
 - Image Revision: b1e3b0eee
 - Image Version: v6

The SARIF reporter was failing saying:

Unable to upload "megalinter-reports/megalinter-report.sarif" as it is not valid SARIF: - instance.runs[2].results[3].locations[0].physicalLocation.region.endColumn must have a minimum value of 1

Exploring the SARIF file from the build artifact revealed:

{
  "locations": [
    {
      "physicalLocation": {
        "artifactLocation": {
          "uri": "path/to/filename.ext"
        },
        "region": {
          "endColumn": 0,
          "endLine": 6,
          "snippet": {
            "text": "[...snip...]"
          },
          "startColumn": 11,
          "startLine": 6
        }
      }
    }
  ]

As reported, endColumn was less than 1. (tangentially, it was also less than startColumn)

Digging deeper, the thing it found was in a file committed some time ago and subsequently removed. This made finding the issue more fun because the file structure had been changed such that the physicalLocation.artifactLocation.url no longer pointed to anything.

I was able to work around the issue by adding the following to .megalinter.yml configuration file:

REPOSITORY_GITLEAKS_ARGUMENTS: --no-git

That is, only scan the files and directories that are present in the local repository and not scan the git history..

This is less good. However, once a given commit is scanned and found to be ok, the only time that would change is it the heuristics / patterns in the tool were updated. So, while it's less good, the degree is mitigated to some degree. Let's hope our developers don't get more creative in embedding secrets into their code.

To Reproduce
Steps to reproduce the behavior:

1. merge a commit with a file with a secret in it
2. rename that file (git mv), push, and merge that commit
3. come to one's senses and setup Megalinter
4. enable SARIF_REPORTER
5. allow Megalinter to run repository/gitleaks

Expected behavior

If a file is not present in the local repository, so not include a reference to that file in a SARIF report.

Screenshots

https://github.com/IMLS/estimating-wifi/actions/runs/2791588499

Additional context

A workaround is to inhibit gitleaks from looking through the repository's history and only focus on the files present.

Alternatively, it may be diagnostically interesting to note that in this instance, endColumn was 0; perhaps one can filter out entries where .runs[].results[].locations[].physicalLocation.region.endColumn == 0 either when writing the report or using jq as a postprocessor..?

I've worked around this so our builds are now...building, so there's no immediate action requested.

[wesley-dean-flexion]

Also, the fetch-depth for the checkout action (for running via GitHub) doesn't affect this -- even setting to 1 doesn't affect whether or not gitleaks scans the git history or not. I've seen FAQ entries and issues about how configuration in .megalinter.yml doesn't affect the tooling. This is that. (included so folks coming afterwards can, hopefully, avoid going down that path like I did)

[nvuillam]

Some linters produce invalid SARIF, so I already added some refactoring of SARIF generated file so Github accepts it, it seems I forgot one !

It's done here ->

megalinter/megalinter/reporters/SarifReporter.py

Line 221 in 49fe52e

def fix_sarif_physical_location(self, physical_location):

I just add to add also such checks for region property... will do that someday, or you can do it if you have some time :)

[wesley-dean-flexion]

I'll be happy to help! However, it will be a while before I get there. I would support putting this in the backest of backlogs, the coldest of iceboxes..

Also, I'm not surprised in the slightest that you've a brilliant response.

[nvuillam]

Haha you flatterer ^^
Ok it works, and thanks to your nice sponsoring I have 2 free beers by months... the fix is on its way :)

[nvuillam]

@wesley-dean-flexion please can you try again with oxsecurity/megalinter:beta ? ( in about 30 mn when beta release job will be completed) -> https://github.com/oxsecurity/megalinter/actions/runs/2792887110