psalm icon indicating copy to clipboard operation
psalm copied to clipboard

Broken SARIF output

Open provokateurin opened this issue 2 years ago • 4 comments

Trying to run https://github.com/nextcloud/server/pull/37390 results in error messages like this one when uploading the report:

  {
    "property": "instance.runs[0].results[71].locations[0].physicalLocation.region.startLine",
    "message": "must have a minimum value of 1",
    "schema": {
      "description": "The line number of the first character in the region.",
      "type": "integer",
      "minimum": 1
    },
    "name": "minimum",
    "argument": 1,
    "stack": "instance.runs[0].results[71].locations[0].physicalLocation.region.startLine must have a minimum value of 1"
  }

https://sarifweb.azurewebsites.net/Validation gives the same error messages. This is the generated report: results.txt

provokateurin avatar Apr 25 '23 14:04 provokateurin

Hey @provokateurin, can you reproduce the issue on https://psalm.dev ?

psalm-github-bot[bot] avatar Apr 25 '23 14:04 psalm-github-bot[bot]

Sarif reporter needs to apply default values for the region elements.

weirdan avatar Apr 25 '23 15:04 weirdan

This problem happens when enabling findUnusedBaselineEntry. Our baseline has no unused baseline entries, so the errors in the taint analysis are wrong.

provokateurin avatar May 02 '23 11:05 provokateurin

I'm just stumbling across the same problem. The unused baseline entries are reported with

"physicalLocation": {
  "artifactLocation": {
    "uri": "src/Infrastructure/Logger/SimpleConsoleLogger.php"
  },
  "region": {
    "startLine": 0,
    "endLine": 0,
    "startColumn": 0,
    "endColumn": 0
  }
}

which is wrong. The location should be the baseline file and the line numbers should be valid. Using the source file that formerly contained an issue is incorrect, as is using zero for the line numbers.

Notes:

  • I'm not sure if there is a way to report an issue without an actual location, which could be a less complex change. I haven't looked at the according standard or JSON schema at all yet.
  • Using a baseline when you upload your SARIF report to GitHub is actually a mistake. Letting GitHub manage the reports and annotate pull requests with the according info is kind of a baseline 2.0. It completely relieves you from the chore of baseline maintenance and instead gives you a nice UI where you can discuss new issues in pull requests.
  • I have preserved the state of the project I'm working on under https://github.com/UlrichEckhardt/simplequeue/pull/2, where you can directly observe the behaviour.
  • I'm using Psalm 6.6.1@dae5a05eac03b55e8f50ae00f4cd2ba5d5588d59.

UlrichEckhardt avatar Apr 26 '25 17:04 UlrichEckhardt