github/codeql-action/upload-sarif@v1 Failed to call git to get current commit, silently fails to upload the SARIF file and returns a false success status

[HariSekhon]

I've just discovered that if github/codeql-action/upload-sarif@v1 fails to get the git commit (relates to #58), it doesn't raise an error.

This is very bad for security scanning tools like static code analysis, because it likely means that nothing was scanned but gives a false positive success status for the workflow.

Output:

Run github/codeql-action/upload-sarif@v1
fatal: not a git repository (or any of the parent directories): .git
Failed to call git to get current commit. Continuing with data from environment or input: Error: The process '/usr/bin/git' failed with exit code 128
Error: The process '/usr/bin/git' failed with exit code 128
    at ExecState._setResult (/home/runner/work/_actions/github/codeql-action/v1/node_modules/@actions/exec/lib/toolrunner.js:592:25)
    at ExecState.CheckComplete (/home/runner/work/_actions/github/codeql-action/v1/node_modules/@actions/exec/lib/toolrunner.js:575:18)
    at ChildProcess.<anonymous> (/home/runner/work/_actions/github/codeql-action/v1/node_modules/@actions/exec/lib/toolrunner.js:469:27)
    at ChildProcess.emit (events.js:[21](https://github.com/HariSekhon/Templates/runs/5267697089?check_suite_focus=true#step:6:21)0:5)
    at maybeClose (internal/child_process.js:1021:16)
    at Socket.<anonymous> (internal/child_process.js:4[30](https://github.com/HariSekhon/Templates/runs/5267697089?check_suite_focus=true#step:6:30):11)
    at Socket.emit (events.js:210:5)
    at Pipe.<anonymous> (net.js:659:12)
Uploading results
  Processing sarif files: ["results.sarif"]
  Uploading results
  Successfully uploaded results

Here is an example public workflow of mine showing the problem:

https://github.com/HariSekhon/Templates/runs/5267697089?check_suite_focus=true

I've corrected the error in my workflow so newer versions of this workflow do work (I missed the checkout step when porting to a reusable workflow for some reason), but the point is that the github/codeql-action/upload-sarif@v1 action should fail when it detects that it is not in a git repo unless an optional flag is given to permit this condition as it will often be a serious error in the workflow, causing nothing to be scanned.

Any error in determining the git commit should result in an error code being propagated upwards and failing the entire workflow for safety to detect when you are not receiving security alerts due to not being in a checkout.

Update: technically an empty SARIF file is probably being uploaded after a scan of an empty non-checkout dir - some tools may pull docker images and in those cases it's fine to not spend time checking out the repo - but the default behaviour should be to error out on any error such as not being in a git checkout for safety, unless passing a flag to indicate that was intended.

[RasmusWL]

Thanks for raising this, I've passed it on to the relevant team 👍

[RasmusWL]

Hi @HariSekhon, I can see that you updated your issue with this text

Update: technically an empty SARIF file is probably being uploaded after a scan of an empty non-checkout dir - some tools may pull docker images and in those cases it's fine to not spend time checking out the repo - but the default behaviour should be to error out on any error such as not being in a git checkout for safety, unless passing a flag to indicate that was intended.

After consulting with my colleagues, that is the same conclusion. The error message looks very dangerous, but it uploaded the SARIF file just fine, and the SARIF file is most likely just empty. You can store it as an artifact if you want to confirm.

[HariSekhon]

@hmakholm was this bug fixed to error out early on any issue rather than letting to upload a blank SARIF file?