detekt / sarif4k

Kotlin data bindings for the Static Analysis Results Interchange Format (SARIF)
Apache License 2.0
15 stars 7 forks source link

Sarif report merger do not considering uriBaseId #122

Open DenisYelkin opened 6 months ago

DenisYelkin commented 6 months ago

SarifReportMerger.kt - merging only results. Without considering uriBaseId and originalUriBaseIds. It leads to inability to parse actual artifact location for particular result.

Example:

{
    "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
    "version": "2.1.0",
    "runs": [
        {
            "originalUriBaseIds": {
                "%SRCROOT%": {
                    "uri": "file:///Users/user/projects/android/modules/second/"
                }
            },
            "results": [
                {
                    "level": "warning",
                    "locations": [
                        {
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "src/main/kotlin/my/module/first/Utils.kt",
                                    "uriBaseId": "%SRCROOT%"
                                },
                ...

originalUriBaseIds - has only one reference to module second. Meanwhile results uriBaseId linked to wrong uri (Utils.kt located in another module).

BraisGabin commented 1 month ago

No idea how we didn't pick up this PR earliear. In the meantime we changed that part of the code a lot. Basically we move the responsability of merging two different reports to sarif4k. Can you check if this issue is still present in the new implementation? To be honest I don't use sarif that much so I don't understand completely the issue.

If it is still there, do you want to create a PR fixing it? The merge code is kind of simple so it shouldn't be difficult to implement: https://github.com/detekt/sarif4k/blob/main/src/commonMain/kotlin/io/github/detekt/sarif4k/Merging.kt