codeql-action/upload-sarif@v1 doesn't accept empty sarif #390

ghost · 2021-02-12T20:46:53Z

Expected behaviour:
No error

Actual behaviour:
codeql-action/upload-sarif@v1 doesn't accept empty sarif

Exemple:

{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
  "version": "2.1.0",
  "runs": []
}

Result:

Uploading sarif files: ["final.sarif"]
Uploading results
Error: Invalid request.

1 item required; only 0 were supplied.
RequestError [HttpError]: Invalid request.

1 item required; only 0 were supplied.
    at /home/runner/work/_actions/github/codeql-action/v1/node_modules/@octokit/request/dist-node/index.js:66:23
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
    at async uploadPayload (/home/runner/work/_actions/github/codeql-action/v1/lib/upload-lib.js:60:22)
    at async uploadFiles (/home/runner/work/_actions/github/codeql-action/v1/lib/upload-lib.js:217:5)
    at async Object.uploadFromActions (/home/runner/work/_actions/github/codeql-action/v1/lib/upload-lib.js:91:12)
    at async run (/home/runner/work/_actions/github/codeql-action/v1/lib/upload-sarif-action.js:34:29)
    at async runWrapper (/home/runner/work/_actions/github/codeql-action/v1/lib/upload-sarif-action.js:46:9) {
  name: 'HttpError',
  status: 422,
  headers: {
    'access-control-allow-origin': '*',
    'access-control-expose-headers': 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset',
    connection: 'close',
    'content-length': '123',
    'content-security-policy': "default-src 'none'",
    'content-type': 'application/json; charset=utf-8',
    date: 'Fri, 12 Feb 2021 20:35:44 GMT',
    'referrer-policy': 'origin-when-cross-origin, strict-origin-when-cross-origin',
    server: 'GitHub.com',
    'strict-transport-security': 'max-age=31536000; includeSubdomains; preload',
    vary: 'Accept-Encoding, Accept, X-Requested-With',
    'x-content-type-options': 'nosniff',
    'x-frame-options': 'deny',
    'x-github-media-type': 'github.v3; format=json',
    'x-github-request-id': '07C1:040D:2E46AC:90C8F9:6026E6A0',
    'x-ratelimit-limit': '500',
    'x-ratelimit-remaining': '496',
    'x-ratelimit-reset': '1613162249',
    'x-ratelimit-used': '4',
    'x-xss-protection': '1; mode=block'
  },
  request: {
    method: 'PUT',
    url: 'https://api.github.com/repos/rizinorg/rizin/code-scanning/analysis',
    headers: {
      accept: 'application/vnd.github.v3+json',
      'user-agent': 'CodeQL Action octokit-core.js/3.1.2 Node.js/12.13.1 (linux; x64)',
      authorization: 'token [REDACTED]',
      'content-type': 'application/json; charset=utf-8'
    },
    body: '{"commit_oid":"820c45f5933c343c34a94e1d2382e91980359a8f","ref":"refs/pull/610/merge","analysis_key":".github/workflows/code-analysis.yml:build","analysis_name":"Code scanning","sarif":"H4sIAAAAAAAAA6tWKkstKs7Mz1OyUjLSM9QzUNJRKirNK1ayio6tBQBSlZKzHQAAAA==","workflow_run_id":562142767,"checkout_uri":"file:///home/runner/work/rizin/rizin","environment":"{\\n  \\"name\\": \\"CodeQL-javascript\\"\\n}","started_at":"2021-02-12T20:31:48.765Z","tool_names":[],"base_ref":"refs/heads/dev","base_sha":"af5ebbb92533d1015336f2257bfe5e7dc67c2494"}',
    request: { agent: [Agent], hook: [Function: bound bound register] }
  },
  documentation_url: 'https://docs.github.com/rest'
}

The sarif file was generated by github/codeql-action/analyze@v1 and sarif-multitool.

npx @microsoft/sarif-multitool merge reports/*.sarif

The text was updated successfully, but these errors were encountered:

Detect if issues were found and upload SARIF report only in that case, because reports without entries are not accepted. See github/codeql-action#390. The analyzer is run twice in order to get a SARIF report as well as an exit status when issues are detected, as the scan-build --status-bugs parameter doesn't work when the output format is SARIF.

hefloryd · 2021-02-25T15:24:20Z

I'm uploading results from clang scan-build and seeing this also when no issues are detected. As a workaround the report is only uploaded if issues were found however I suspect this would mean that fixed issues may never get flagged as closed if there are no more issues.

robertbrignull · 2021-02-25T17:00:38Z

You're right that not uploading when there are no alerts will lead to alerts being left open erroneously. Code scanning decides that an alert is closed when a SARIF file is uploaded for that tool and ref that doesn't contain the alert.

I would consider what does it mean to upload an empty SARIF file. It means that you ran zero code scanning tools. If you ran a tool and it found zero alerts then there should be an entry in the runs list for that tool and then that run has an empty list of alerts. So even if uploading an empty SARIF file was allowed, then code scanning wouldn't know that clang was run but found no issues. Essentially it would tell you zero information.

I think the real problem here lies with clang or the other tools that's generating a SARIF file with an empty runs list. In the case of @microsoft/sarif-multitool I expect the reports directory doesn't contain any SARIF files so it would be good to investigate why that is.

hefloryd · 2021-02-26T10:01:07Z

Thanks, that makes sense. Note that I am also using sarif-multitool to convert clang individual files to a merged report. Clang produces one report per analyzed file that looks like this if there are no alerts:

{
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "runs": [
    {
      "artifacts": [],
      "columnKind": "unicodeCodePoints",
      "results": [],
      "tool": {
        "driver": {
          "fullName": "clang static analyzer",
          "language": "en-US",
          "name": "clang",
          "rules": [],
          "version": "clang version 10.0.0-4ubuntu1 "
        }
      }
    }
  ],
  "version": "2.1.0"
}

Running sarif-multitool merge on those files produces:

{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
  "version": "2.1.0",
  "runs": []
}

so it appears information is lost. Perhaps this is an issue with the sarif-mutiltool, there are some bug reports that could be related.

The reason I am merging clang reports is that when these were uploaded using codeql-action/upload-sarif@v1, unfixed alerts would get randomly closed and reappear on the Github Security tab between runs. Merging the reports made them appear consistently. Is there a problem with the sarif output from clang? Here's an example where an alert was found:


{
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "runs": [
    {
      "artifacts": [
        {
          "length": 35831,
          "location": {
            "uri": "file:///home/runner/work/p-net/p-net/src/device/pf_cmrdr.c"
          },
          "mimeType": "text/plain",
          "roles": [
            "resultFile"
          ]
        }
      ],
      "columnKind": "unicodeCodePoints",
      "results": [
        {
          "codeFlows": [
            {
              "threadFlows": [
                {
                  "locations": [
                    {
                      "importance": "essential",
                      "location": {
                        "message": {
                          "text": "Value stored to 'ret' is never read"
                        },
                        "physicalLocation": {
                          "artifactLocation": {
                            "index": 0,
                            "uri": "file:///home/runner/work/p-net/p-net/src/device/pf_cmrdr.c"
                          },
                          "region": {
                            "endColumn": 7,
                            "startColumn": 7,
                            "startLine": 108
                          }
                        }
                      }
                    }
                  ]
                }
              ]
            }
          ],
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "index": 0,
                  "uri": "file:///home/runner/work/p-net/p-net/src/device/pf_cmrdr.c"
                },
                "region": {
                  "endColumn": 7,
                  "startColumn": 7,
                  "startLine": 108
                }
              }
            }
          ],
          "message": {
            "text": "Value stored to 'ret' is never read"
          },
          "ruleId": "deadcode.DeadStores",
          "ruleIndex": 0
        }
      ],
      "tool": {
        "driver": {
          "fullName": "clang static analyzer",
          "language": "en-US",
          "name": "clang",
          "rules": [
            {
              "fullDescription": {
                "text": "Check for values stored to variables that are never read afterwards"
              },
              "helpUri": "https://clang-analyzer.llvm.org/available_checks.html#deadcode.DeadStores",
              "id": "deadcode.DeadStores",
              "name": "deadcode.DeadStores"
            }
          ],
          "version": "clang version 10.0.0-4ubuntu1 "
        }
      }
    }
  ],
  "version": "2.1.0"
}

I'll open a new issue for this once it's clear where the problem is.

jsoref · 2022-10-09T02:35:22Z

Bugs

@microsoft/sarif-multitool

I've opened the above bug. It's very clear that the merge agent is buggy. Whether there are other bugs is subject to discussion.

github/codeql

From my perspective, there's a bug in github/codeql as well.

1 item required; only 0 were supplied.

Is not remotely human friendly. If you (=codeql team) require that there be at least one runs item, then the error message should clearly mention runs. That it doesn't is a bug.

github/codeql-action

Also note that while the codeql backend should provide a friendly error akin to the above, this codeql-action should be able to preprocess, detect the same error and report it without hitting the codeql endpoint (saving everyone X api calls from their github api call quota).

It's ok for rare edge cases not to be handled in wrappers like this, but this is not going to be a rare edge case. Especially when a major vendor (👋 Microsoft) is generating bogus outputs of this form and you're actively encouraging people to use your tool.

hefloryd mentioned this issue Feb 25, 2021

Upload clang analysis only if issues were found rtlabs-com/osal#8

Merged

jsoref mentioned this issue Oct 9, 2022

npx @microsoft/sarif-multitool merge reports/*.sarif yields empty runs for empty results microsoft/sarif-sdk#2551

Open

Phantsure mentioned this issue Nov 15, 2022

Scorecards.yml: do not upload the result in the security dashboard if it is a pull request actions/starter-workflows#1820

Closed

pdil mentioned this issue Dec 11, 2022

Invalid SARIF file created when there are no results r-lib/lintr#1836

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

codeql-action/upload-sarif@v1 doesn't accept empty sarif #390

codeql-action/upload-sarif@v1 doesn't accept empty sarif #390

ghost commented Feb 12, 2021

hefloryd commented Feb 25, 2021

robertbrignull commented Feb 25, 2021

hefloryd commented Feb 26, 2021

jsoref commented Oct 9, 2022

codeql-action/upload-sarif@v1 doesn't accept empty sarif #390

codeql-action/upload-sarif@v1 doesn't accept empty sarif #390

Comments

ghost commented Feb 12, 2021

hefloryd commented Feb 25, 2021

robertbrignull commented Feb 25, 2021

hefloryd commented Feb 26, 2021

jsoref commented Oct 9, 2022

Bugs

@microsoft/sarif-multitool

github/codeql

github/codeql-action