Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Append rule help text/markdown in alert view for CodeQL SARIF files #305

Open
cwong-scw opened this issue Nov 16, 2020 · 5 comments · May be fixed by aliscco/codeql-action#148 or aliscco/codeql-action#174

Comments

@cwong-scw
Copy link

In the SARIF files produced by CodeQL, the rule help text/markdown is ignored in favour of the corresponding .qhelp file content when displayed in code scanning alerts. Would it be possible to have the rule help text/markdown appended to the end of the markdown generated from the .qhelp file? This would allow any SARIF file pre-processors to enrich the displayed help text with additional material such as additional analysis results, contextual training resources, vulnerability risk ratings, etc.

@swinton
Copy link

swinton commented Jan 8, 2021

Hey @cwong-scw. I think one way you could achieve this would be to publish an action to GitHub's Marketplace that would perform "post-processing" of a SARIF file, before the SARIF gets uploaded to GitHub. You could use this "post-processing" step to attach additional rule help.

E.g. imagine a workflow like this, note the cwong-scw/post-process-sarif@v1 step prior to the github/codeql-action/upload-sarif@v1 step :

name: "CodeQL"

on:
  push:
    branches: [ main ]
  pull_request:
    branches: [ main ]
  schedule:
    - cron: '0 0 * * 0'

jobs:
  analyze:
    name: Analyze
    runs-on: ubuntu-latest

    strategy:
      fail-fast: false
      matrix:
        language: [ 'cpp', 'csharp', 'java', 'javascript' ]

    steps:
    - name: Checkout repository
      uses: actions/checkout@v2

    - name: Initialize CodeQL
      uses: github/codeql-action/init@v1
      with:
        languages: ${{ matrix.language }}

    - name: Autobuild
      uses: github/codeql-action/autobuild@v1

    - name: Perform CodeQL Analysis
      uses: github/codeql-action/analyze@v1
      with:
        upload: false
    
    - name: Perform post-processing of CodeQL Analysis
      uses: cwong-scw/post-process-sarif@v1
      with:
        sarif_file: '../results'
    
    - name: Upload SARIF
      uses: github/codeql-action/upload-sarif@v1
      with:
        sarif_file: '../results'

@cwong-scw
Copy link
Author

Hi @swinton! Thanks for the response! We have actually taken this exact approach to supplement the SARIF file prior to upload. However, the last time that I checked it, the problem we hit was that the help text in the processed SARIF file seemed to be ignored in the upload processing, and the CodeQL .qhelp file content was used instead. This meant that any additional help contents added to the SARIF file were not shown in the code scanning alerts once uploaded.

Our request here, which I can understand might be fairly low priority, is for a way to append any help text present in the SARIF rule to the main .qhelp file content generated by CodeQL during the upload processing. This would keep the valuable remediation advice from CodeQL while still allowing this to be supplemented by other tools. Do you think something like this would be possible?

@swinton
Copy link

swinton commented Jan 12, 2021

Our request here, which I can understand might be fairly low priority, is for a way to append any help text present in the SARIF rule to the main .qhelp file content generated by CodeQL during the upload processing

Can you please help me understand what you mean by the main .qhelp file? I'm not sure what this is referring to, and I see no mention of qhelp in the SARIF specification. Thank you.

the last time that I checked it, the problem we hit was that the help text in the processed SARIF file seemed to be ignored in the upload processing

You should be able to provide additional content in help.text or help.markdown -- if this isn't working, could you link to an example repo so we can take a closer look?

@cwong-scw
Copy link
Author

Yep no problem! Just to clarify, the .qhelp files are used by CodeQL only and are not part of the SARIF spec. Sorry for the confusion.

If we use a workflow such as the example above that contains the analyze step with upload set to false, and then use the upload-sarif step separately, the following seems to happen:

  1. CodeQL runs as normal and produces a SARIF file. The SARIF file will contain a number of rule elements corresponding to the .ql files that contain the queries being run by CodeQL to identify the vulnerabilities (e.g. https://github.com/github/codeql/blob/main/javascript/ql/src/Security/CWE-094/CodeInjection.ql)
  2. These rule elements initially do not contain any help properties. The post-process step creates the help.text or help.markdown properties within the rule and adds content into these properties.
  3. When the post-processed SARIF file is uploaded, CodeQL-produced SARIF files appear to be treated slightly differently (perhaps based on tool.driver.name?) than other SARIF files and any help text or markdown included in a rule is not used for display in the Code scanning alerts section of the Security tab.

For example, if the post-processed rule is something like this:

{
    "id": "js/code-injection",
    "name": "js/code-injection",
    "shortDescription": {
        "text": "Code injection"
    },
    "fullDescription": {
        "text": "Interpreting unsanitized user input as code allows a malicious user arbitrary code execution."
    },
    "defaultConfiguration": {
        "level": "error"
    },
    "properties": {
        "tags": [
            "security",
            "external/cwe/cwe-094",
            "external/cwe/cwe-079",
            "external/cwe/cwe-116"
        ],
        "kind": "path-problem",
        "precision": "high",
        "name": "Code injection",
        "description": "Interpreting unsanitized user input as code allows a malicious user arbitrary\n              code execution.",
        "id": "js/code-injection",
        "problem.severity": "error"
    },
    "help": {
        "text": "Some help text here",
        "markdown": "Some help text here"
    }
}

When uploaded, "Some help text here" is not shown in the code scanning alert. Instead, the help text section seems to be populated from the .qhelp file corresponding to the .ql file (e.g. https://github.com/github/codeql/blob/main/javascript/ql/src/Security/CWE-094/CodeInjection.qhelp). In the header of the help text section there is a Query field in addition to the regular Tool and Rule ID fields, with a View Source link to the corresponding .ql file (e.g. https://github.com/github/codeql/blob/main/javascript/ql/src/Security/CWE-094/CodeInjection.ql).

An example of this occurring: https://github.com/cwong-scw/action-playground/security/code-scanning/225

My guess is that CodeQL rules are processed in a special manner and make use of the more expressive .qhelp templating to generate the markdown that is displayed in the help text section of code scanning alerts. I am wondering if it would be possible to append the help.text or help.markdown from the SARIF file to the markdown generated from the .qhelp file for CodeQL rules to support the suggested post-processing approach. Hopefully this helps explain it better!

@jsoref
Copy link
Contributor

jsoref commented Oct 9, 2022

@cwong-scw: unfortunately, security/code-scanning/ urls tend to be private to people w/ write access to the repository.

Sample rendering of a code injection based on the aforementioned repository image image image

I'm very interested in this because I'd love to generate .markdown content, but at least some of the github sarif handling only supports .text, and at least some basic markdown elements appear to be stripped (esp. backticks...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants