bug: Getting error jq: error (at monokle-1707400873136.sarif:0): Cannot iterate over null (null)

[nsalfos]

Version

2.0.1

Current Behavior

im testing Monokle GHA and it does creat a SARIF file. but when using sarif-to-comment-action im getting the error jq: error (at monokle-1707485394343.sarif:0): Cannot iterate over null (null)

Expected Behavior

i would expect it to post the findings from monokle as c omment on the PR

Steps to Reproduce

pull any helm chart locally (i used bitnami/airflow )

Pull Chart

`helm pull bitnami/airflow --untar`

in root dir create the monokle.validation.yaml rules file:

monokle.validation.yaml

``` plugins: yaml-syntax: true open-policy-agent: true kubernetes-schema: true labels: false rules: yaml-syntax/no-bad-alias: "warn" yaml-syntax/no-bad-directive: false open-policy-agent/no-latest-image: "err" open-policy-agent/cpu-limit: "err" open-policy-agent/memory-limit: "err" open-policy-agent/memory-request: "err" practices/no-low-user-id: true practices/no-low-group-id: true ```

GH Workflow

``` name: Lint and Test Charts on: pull_request permissions: write-all jobs: validate: runs-on: ubuntu-22.04 steps: - name: Checkout uses: actions/checkout@v3 with: fetch-depth: 0 - name: Set up Helm uses: azure/setup-helm@v3 with: version: v3.12.1 - id: bake name: Bake chart uses: azure/k8s-bake@v2.4 with: renderEngine: "helm" helmChart: "./helm-charts/airflow" - id: validate name: Validate Manifest uses: kubeshop/monokle-action@v0.3.2 continue-on-error: true with: path: ${{ steps.bake.outputs.manifestsBundle }} config: /monokle.validation.yaml. # Update this path - name: Test for file. # this is not really needed, using it as a debug. run: | ls cat ${{ steps.validate.outputs.sarif }} jq . ${{ steps.validate.outputs.sarif }} - name: Post SARIF findings in the issue uses: sett-and-hive/sarif-to-comment-action@v2.0.1 with: token: ${{ secrets.GITHUB_TOKEN }} repository: ${{ github.repository }} branch: ${{ github.head_ref }} pr-number: ${{ github.event.number }} sarif-file: ${{ steps.validate.outputs.sarif }} title: "Security scanning results" odc-sarif: false dry-run: false ```

Run any draft PR against the branch to make the workflow execute.

Additional Information

With above rules file monokle should return this report for bitnami/airflow

Run kubernetes-schema (26 problems)
Run yaml-syntax
Run open-policy-agent (39 problems)

[tomwillis608]

@nsalfos Thank you for reporting this issue!

Can you experiment with setting odc-sarif to true in your workflow? TIA

[nsalfos]

@nsalfos Thank you for reporting this issue!

Can you experiment with setting odc-sarif to true in your workflow? TIA

Yup, tried with no changes on the result i'm afraid. just curious, were you able to reproduce the issue with my config?

[nsalfos]

Also tried to re-run the failed job with debug... but the action is not very verbose... This was all the output for the sarif-to-comment-action

jq: error (at monokle-1707923891796.sarif:0): Cannot iterate over null (null)
##[debug]Docker Action run completed with exit code 5
##[debug]Finishing: Post SARIF findings in the issue

[tomwillis608]

Do you have a sample file that you could share with me for troubleshooting?

[nsalfos]

here. monokle-1707930423249.sarif.gz

had to gzip it otherwise GH wouldn't let me upload

[tomwillis608]

I have reproduced they symptom you see jq: error (at ./test/fixtures/monokle.sarif:0): Cannot iterate over null (null) So I have a starting point for troubleshooting.

[tomwillis608]

After a little more troubleshooting and making the tests in entrypoint.sh more verbose, your monokle SARIF file is failing the test for "runs" in the SARIF file.

The instrumented output when I add you file to our tests is:

Test if if SARIF is valid JSON.
Test if SARIF has runs.
jq: error (at ./test/fixtures/monokle.sarif:0): Cannot iterate over null (null)

In your file, this test fails: str=$(jq -e '.runs[].tool.driver.rules[]' "$SARIF_FILE")

IIRC, I wrote this test to ensure that there were records in the runs part of the tree so that the @security-alert/sarif-to-comment Node package that does the real work would not complain.

If I edit the script the action runs to send your Monokle SARIF file to the Node package, then the package returns: It will not post, because the markdown is empty from line 83 of security-alert/packages/sarif-to-comment/src/index.ts

[nsalfos]

Apologies for my very limited understanding, but what does this exactly mean? Is Monokle not generating a "regular SARIF format" file ? I'm trying to understand how/why when i run the kubeshop/monokle-action@v0.3.2 i get "ok looking" findings there... but then what you saw and tested happened with the post-to-comment

These are 2 small snaps of what im seeing in the output of the action:

btw, thanks for your time on this.

[tomwillis608]

I'm no SARIF expert either :-) I wrote this action to post SARIF output from OWASP Dependency Check to PR comments from a GHA workflow using the @security-alert/sarif-to-comment NPM package, built from https://github.com/security-alert/security-alert.

That Node package expects to be able to find a run node in the SARIF file and does not find it in the Monokle files.

[tomwillis608]

Nick, I know nothing about this repo, good or not, but if you explore this SARIF comment action, I would love to learn about your experience. I just want to add that I am so glad that you saw potential value in our work.

[tomwillis608]

I think the issue is that the SARIF to markdown code in the Node package we call expects the SARIF to contain rules in the driver section of the tool section under the runs. If you look in the Monokle sample you provided, the tools section is quite terse:

      "tool": {
        "driver": {
          "name": "monokle"
        },

In an example that the Node package can handle,, the tools.driver section has this form:

      "tool" : {
        "driver" : {
          "name" : "CodeQL command-line toolchain",
          "organization" : "GitHub",
          "semanticVersion" : "2.2.4",
          "rules" : [ {
            "id" : "js/xss",
            "name" : "js/xss",
            "shortDescription" : {
              "text" : "Client-side cross-site scripting"
            },
            "fullDescription" : {
              "text" : "Writing user input directly to the DOM allows for a cross-site scripting vulnerability."
            },
            "defaultConfiguration" : {
              "level" : "error"
            },
            "properties" : {
              "tags" : [ "security", "external/cwe/cwe-079", "external/cwe/cwe-116" ],
              "kind" : "path-problem",
              "precision" : "high",
              "name" : "Client-side cross-site scripting",
              "description" : "Writing user input directly to the DOM allows for\n              a cross-site scripting vulnerability.",
              "id" : "js/xss",
              "problem.severity" : "error"
            }
          } ]
        }
      },

The @security-alert/sarif-to-comment code expects this driver rules in the tool section at runs[].tool.driver.rules[].

This is why the package is not returning anything to comment about, even though the Monokle SARIF does contain results from the run.

[nsalfos]

Hey @tomwillis608 sorry for dissappearing, had to pick up other "more urgent" work, but i wanted to circle back on this. i appreciate your analysis here.. and i just had the time to try and understand what you mentioned. thanks to your pointers also was able to replicate the issue, but only from my CLI using jq

As you very well pointed out there, it seems monokle uses a slightly different format for the sarif findings, but they are there nonetheless, just under diff path .runs[].tool.extensions[].rules[]

> jq -e '.runs[].tool.extensions[].rules[]' monokle-1707930423249.sarif   | head -30
{
  "id": "K8S001",
  "name": "schema-violated",
  "shortDescription": {
    "text": "The resource is formatted incorrectly."
  },
  "fullDescription": {
    "text": "The resource is violating the schema violation. The Kubernetes API will not accept this resource."
  },
  "help": {
    "text": "Check whether the property is used correctly. You can hover the key for documentation."
  }
}
{
  "id": "K8S002",
  "name": "deprecation-violated",
  "shortDescription": {
    "text": "The resource uses deprecated \"apiVersion\" value."
  },
  "fullDescription": {
    "text": "The resource is violating the deprecation violation. The Kubernetes API may not accept this resource or can behave unexpectedly."
  },
  "help": {
    "text": "Change \"apiVersion\" for up-to-date value. You can hover the key for documentation."
  }
}

So i thought of trying to catch this outlier case wtih monokle by editing the entrypoint.sh like so

test_if_sarif_has_runs() {
  driver=$(jq -e '.runs[].tool.driver.name' "$SARIF_FILE")
  if [ ${#driver} = 'monokle' ]; then
    str=$(jq -e '.runs[].tool.extensions[].rules[]' "$SARIF_FILE")
  else
    str=$(jq -e '.runs[].tool.driver.rules[]' "$SARIF_FILE")
  fi
  if [ ${#str} = 0 ]; then
    echo "ERROR: Bad SARIF format in $SARIF_FILE"
    exit 4
  fi
}

i've tried to fork the action and use it like that but im still receiving the "iterate over null" error, and i think it has to do with the tests, and/or the npx @security-alert/sarif-to-comment line at the end of it...

let me know if you have some time to test that change, i could fork and PR it if you'd like, but as i couldnt get it to work in my personal env im sure it wont work until something else is modified as well.

thoughts?

[nsalfos]

Nick, I know nothing about this repo, good or not, but if you explore this SARIF comment action, I would love to learn about your experience. I just want to add that I am so glad that you saw potential value in our work.

Hey man!!! so... it turns out, this action, altho not officially uploaded to the marketplace... works as i expected it to do. clone the repo and ran the action locally and Voilà!

Feel free to close the issue if you want.

[tomwillis608]

@nsalfos Glad you found a solution. Looking at the code in the npm my action uses, from https://github.com/security-alert/security-alert, we see that the security-alert code expects the rules[] array with the violations to be at .runs[].tool.driver.rules[], whereas monokle keeps them in arrays under .runs[].tool.extensions[].rules[].

I think the security-alert code needs to be more flexible for SARIF that does not follow the CodeQL dialect. I don't have time to give them a well-tested PR at this time.

[nsalfos]

indeed! i mentioned that in the previous comment actually!

https://github.com/sett-and-hive/sarif-to-comment-action/issues/267#issuecomment-2047422476

and i thought i had a good suggestion going, but when i tested locally it didnt work. i think it was because of the tests. i will try to PR a solution when i have some time as well. but thanks for everything !

[tomwillis608]

If you can apply a fix in the security-alert code, that would be amazing.