Source root for conversion from absolute to relative with invocation.workingDirectory.uri does not work

[vivaat]

Summary

Specifying 'workingDirectory' in 'invocation' property does not convert absolute paths in result object.

Details

After specifying the option, the paths remain absolute and the preview to the file is thus absent.

PoC

In the below example, invocation property has no effect.

{
  "version": "2.1.0",
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "runs": [
    {
      "conversion": {
        "tool": {
          "driver": {
            "name": "PVS-Studio"
          }
        },
        "invocation": {
          "workingDirectory" : {
            "uri": "file:///C%3A"
          },
          "executionSuccessful": true
        }
      },

      "tool" : {
        "driver": {
          "name": "PVS-Studio",
          "semanticVersion": "7.32.0.1",
          "informationUri": "https://pvs-studio.com",
          "rules": [
            {
              "id": "V010",
              "name": "RuleV010",
              "help": {
                "text": "https://pvs-studio.com/en/docs/warnings/v010/"
              },
              "helpUri": "https://pvs-studio.com/en/docs/warnings/v010/"
            }
          ],
          "results": [
            {
              "ruleId": "V010",
              "message": {
                "text": "Analysis of 'Utility' type projects is not supported in this tool. Use direct analyzer integration or compiler monitoring instead."
              },
              "level": "error",
              "locations": [
                {
                  "physicalLocation": {
                    "artifactLocation": {
                      "uri": "file:///C%3A/include/geometry/BoundingBox.h"
                    },
                    "region": {
                      "startLine": 1,
                      "endLine": 1
                    }
                  }
                }
              ]
            }
          ]
        }
      }
    }
  ]
}

There is another way to specify it. Doesn't work either.

{
  "version": "2.1.0",
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "runs": [
    {
     "invocations" : [
        {
          "workingDirectory" : {
            "uri": "file:///C%3A"
          },
          "executionSuccessful": true
        }
      ],

      "tool" : {
        "driver": {
          "name": "PVS-Studio",
          "semanticVersion": "7.32.0.1",
          "informationUri": "https://pvs-studio.com",
          "rules": [
            {
              "id": "V010",
              "name": "RuleV010",
              "help": {
                "text": "https://pvs-studio.com/en/docs/warnings/v010/"
              },
              "helpUri": "https://pvs-studio.com/en/docs/warnings/v010/"
            }
          ],
          "results": [
            {
              "ruleId": "V010",
              "message": {
                "text": "Analysis of 'Utility' type projects is not supported in this tool. Use direct analyzer integration or compiler monitoring instead."
              },
              "level": "error",
              "locations": [
                {
                  "physicalLocation": {
                    "artifactLocation": {
                      "uri": "file:///C%3A/include/geometry/BoundingBox.h"
                    },
                    "region": {
                      "startLine": 1,
                      "endLine": 1
                    }
                  }
                }
              ]
            }
          ]
        }
      }
    }
  ]
}

[hvitved]

@shaikhul I believe this is one for your team?

[cannist]

I can confirm this is a bug. The sarif example given in the OP does not reproduce it because it contains no alerts (the results are in the wrong place) but this one does:

{
    "version": "2.1.0",
    "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
    "runs": [
        {
            "conversion": {
                "tool": {
                    "driver": {
                        "name": "PVS-Studio"
                    }
                },
                "invocation": {
                    "workingDirectory": {
                        "uri": "file:///C%3A"
                    },
                    "executionSuccessful": true
                }
            },
            "tool": {
                "driver": {
                    "name": "PVS-Studio",
                    "semanticVersion": "7.32.0.1",
                    "informationUri": "https://pvs-studio.com",
                    "rules": [
                        {
                            "id": "V010",
                            "name": "RuleV010",
                            "help": {
                                "text": "https://pvs-studio.com/en/docs/warnings/v010/"
                            },
                            "helpUri": "https://pvs-studio.com/en/docs/warnings/v010/"
                        }
                    ]
                }
            },
            "results": [
                {
                    "ruleId": "V010",
                    "message": {
                        "text": "Analysis of 'Utility' type projects is not supported in this tool. Use direct analyzer integration or compiler monitoring instead."
                    },
                    "level": "error",
                    "locations": [
                        {
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "file:///C%3A/include/geometry/BoundingBox.h"
                                },
                                "region": {
                                    "startLine": 1,
                                    "endLine": 1
                                }
                            }
                        }
                    ]
                }
            ]
        }
    ]
}

I'll look more into it.

[cannist]

On further investigation, it turned out that there was another problem with the input that I had missed. It does not look like there is a bug on our side.

The invocation object in the above sarif file is contained in runs.conversion. The working directory of a sarif converter has no relation to result locations. To specify the working directory for the alert producer the location object needs to be put into runs.invocations, more specifically we are looking at the first element in the list.

This sarif file works as expected:

{
    "version": "2.1.0",
    "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
    "runs": [
        {
            "invocations": [
                {
                    "workingDirectory": {
                        "uri": "file:///C%3A"
                    },
                    "executionSuccessful": true
                }
            ],
            "tool": {
                "driver": {
                    "name": "PVS-Studio",
                    "semanticVersion": "7.32.0.1",
                    "informationUri": "https://pvs-studio.com",
                    "rules": [
                        {
                            "id": "V010",
                            "name": "RuleV010",
                            "help": {
                                "text": "https://pvs-studio.com/en/docs/warnings/v010/"
                            },
                            "helpUri": "https://pvs-studio.com/en/docs/warnings/v010/"
                        }
                    ]
                }
            },
            "results": [
                {
                    "ruleId": "V010",
                    "message": {
                        "text": "Analysis of 'Utility' type projects is not supported in this tool. Use direct analyzer integration or compiler monitoring instead."
                    },
                    "level": "error",
                    "locations": [
                        {
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "file:///C%3A/include/geometry/BoundingBox.h"
                                },
                                "region": {
                                    "startLine": 1,
                                    "endLine": 1
                                }
                            }
                        }
                    ]
                }
            ]
        }
    ]
}

So, while there is no bug on our side I think this identifies a flaw in our documentation here which just says to use the "invocation.workingDirectory.uri property in the SARIF file" without saying where the invocation object needs to be located. I'll create an issue with the docs team to get this improved.

[vivaat]

Hey @cannist!

Please note that I provided two variations of the sarif report: one with conversion object and one with invocations list. Both of them does not work in my case.

After running exactly the file that you've attached, the preview still does not appear:

[cannist]

Hmm, weird. I admit I had not tested end-to-end and only observed that the filepath was getting correctly relativized in the db in local dev.

Are you sure you're not still looking at the page for the old alert? The alert from the new sarif upload would now be considered a new one, since it has a different location. What repo are you testing in?

[vivaat]

Yes, 100% positive. Changing uri of the result to relative path shows the preview.
Please check the example here -- https://github.com/vivaat/covid-sim/blob/master/out.sarif.

[cannist]

Ok... I have a test that reproduces it and I know why it is happening and why I could initially not reproduce it in local dev.

There are two ways to specify the source root: in the sarif file or via API parameter. Our server code currently prefers the API one over the sarif one. The upload-sarif action seems to always set the parameter to the checkout directory on the worker if it is not explicitly set as input to the action.

We'll have to fix this but there are multiple solutions and all of them have the potential to affect existing setups. I'll have to do some data gathering before I make any changes, possibly implement some metrics on Monday and have them collect data for at least a week.

[vivaat]

Sounds wonderful, thank you! Please let me know if I can help somehow.

[cannist]

@vivaat, just to keep you in the loop: We've started collecting metrics for how a change would affect existing setups. It is looking good so far but I still want to wait and collect more data and then make a decision at the beginning of next week.

[cannist]

Today we've shipped the change and the server now prefers the information from the sarif file over the api parameter. This should resolve the issue. I have confirmed this also in my testing repo. 🙂