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

[COMPARISION] shellcheck-sarif/sarif-fmt 🆚 csgrep #137

Open
jamacku opened this issue Aug 15, 2023 · 7 comments
Open

[COMPARISION] shellcheck-sarif/sarif-fmt 🆚 csgrep #137

jamacku opened this issue Aug 15, 2023 · 7 comments

Comments

@jamacku
Copy link
Member

jamacku commented Aug 15, 2023

This is not an actual issue in csdiff. I will demonstrate the differences between sarif-rs utilities (soon available in Fedora) and csutils.

The main purpose of this issue is to show how other tools deal with the over-complicated specification of the SARIF format. And maybe get some inspiration for new features or enhancements.

Shell

# shell.sh
echo $1                           # Unquoted variables
find . -name *.ogg                # Unquoted find/grep patterns
rm "~/my file.txt"                # Quoted tilde expansion
v='--verbose="true"'; cmd $v      # Literal quotes in variables
for f in "*.ogg"                  # Incorrectly quoted 'for' loops
touch $@                          # Unquoted $@
shellcheck --format=json1 src/index.sh | csgrep --mode=sarif      
generated SARIF
{
    "$schema": "https://json.schemastore.org/sarif-2.1.0.json",
    "version": "2.1.0",
    "runs": [
        {
            "tool": {
                "driver": {
                    "name": "csdiff",
                    "version": "3.0.3",
                    "informationUri": "https://github.com/csutils/csdiff",
                    "rules": [
                        {
                            "id": "SHELLCHECK_WARNING: error[SC1058]",
                            "name": "SC1058",
                            "properties": {
                                "tags": [
                                    "ShellCheck"
                                ]
                            },
                            "help": {
                                "text": "Defect reference: https://github.com/koalaman/shellcheck/wiki/SC1058",
                                "markdown": "Defect reference: [SC1058](https://github.com/koalaman/shellcheck/wiki/SC1058)"
                            }
                        },
                        {
                            "id": "SHELLCHECK_WARNING: error[SC1072]",
                            "name": "SC1072",
                            "properties": {
                                "tags": [
                                    "ShellCheck"
                                ]
                            },
                            "help": {
                                "text": "Defect reference: https://github.com/koalaman/shellcheck/wiki/SC1072",
                                "markdown": "Defect reference: [SC1072](https://github.com/koalaman/shellcheck/wiki/SC1072)"
                            }
                        },
                        {
                            "id": "SHELLCHECK_WARNING: error[SC1073]",
                            "name": "SC1073",
                            "properties": {
                                "tags": [
                                    "ShellCheck"
                                ]
                            },
                            "help": {
                                "text": "Defect reference: https://github.com/koalaman/shellcheck/wiki/SC1073",
                                "markdown": "Defect reference: [SC1073](https://github.com/koalaman/shellcheck/wiki/SC1073)"
                            }
                        }
                    ]
                }
            },
            "results": [
                {
                    "ruleId": "SHELLCHECK_WARNING: error[SC1073]",
                    "level": "error",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "shell.sh"
                                },
                                "region": {
                                    "startLine": 5
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Couldn't parse this for loop. Fix to allow more checks."
                    },
                    "codeFlows": [
                        {
                            "threadFlows": [
                                {
                                    "locations": [
                                        {
                                            "location": {
                                                "id": 0,
                                                "physicalLocation": {
                                                    "artifactLocation": {
                                                        "uri": "shell.sh"
                                                    },
                                                    "region": {
                                                        "startLine": 5
                                                    }
                                                },
                                                "message": {
                                                    "text": "Couldn't parse this for loop. Fix to allow more checks."
                                                }
                                            },
                                            "nestingLevel": 0,
                                            "kinds": [
                                                "error[SC1073]"
                                            ]
                                        }
                                    ]
                                }
                            ]
                        }
                    ]
                },
                {
                    "ruleId": "SHELLCHECK_WARNING: error[SC1058]",
                    "level": "error",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "shell.sh"
                                },
                                "region": {
                                    "startLine": 6
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Expected 'do'."
                    },
                    "codeFlows": [
                        {
                            "threadFlows": [
                                {
                                    "locations": [
                                        {
                                            "location": {
                                                "id": 0,
                                                "physicalLocation": {
                                                    "artifactLocation": {
                                                        "uri": "shell.sh"
                                                    },
                                                    "region": {
                                                        "startLine": 6
                                                    }
                                                },
                                                "message": {
                                                    "text": "Expected 'do'."
                                                }
                                            },
                                            "nestingLevel": 0,
                                            "kinds": [
                                                "error[SC1058]"
                                            ]
                                        }
                                    ]
                                }
                            ]
                        }
                    ]
                },
                {
                    "ruleId": "SHELLCHECK_WARNING: error[SC1072]",
                    "level": "error",
                    "locations": [
                        {
                            "id": 0,
                            "physicalLocation": {
                                "artifactLocation": {
                                    "uri": "shell.sh"
                                },
                                "region": {
                                    "startLine": 6
                                }
                            }
                        }
                    ],
                    "message": {
                        "text": "Expected 'do'. Fix any mentioned problems and try again."
                    },
                    "codeFlows": [
                        {
                            "threadFlows": [
                                {
                                    "locations": [
                                        {
                                            "location": {
                                                "id": 0,
                                                "physicalLocation": {
                                                    "artifactLocation": {
                                                        "uri": "shell.sh"
                                                    },
                                                    "region": {
                                                        "startLine": 6
                                                    }
                                                },
                                                "message": {
                                                    "text": "Expected 'do'. Fix any mentioned problems and try again."
                                                }
                                            },
                                            "nestingLevel": 0,
                                            "kinds": [
                                                "error[SC1072]"
                                            ]
                                        }
                                    ]
                                }
                            ]
                        }
                    ]
                }
            ]
        }
    ]
}
shellcheck --format=json shell.sh | shellcheck-sarif     
generated SARIF
{
  "runs": [
    {
      "results": [
        {
          "fixes": [],
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "shell.sh"
                },
                "region": {
                  "endColumn": 1,
                  "endLine": 5,
                  "startColumn": 1,
                  "startLine": 5
                }
              }
            }
          ],
          "message": {
            "text": "Couldn't parse this for loop. Fix to allow more checks."
          },
          "relatedLocations": [],
          "ruleId": "1073",
          "ruleIndex": 0
        },
        {
          "fixes": [],
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "shell.sh"
                },
                "region": {
                  "endColumn": 1,
                  "endLine": 6,
                  "startColumn": 1,
                  "startLine": 6
                }
              }
            }
          ],
          "message": {
            "text": "Expected 'do'."
          },
          "relatedLocations": [],
          "ruleId": "1058",
          "ruleIndex": 1
        },
        {
          "fixes": [],
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "shell.sh"
                },
                "region": {
                  "endColumn": 1,
                  "endLine": 6,
                  "startColumn": 1,
                  "startLine": 6
                }
              }
            }
          ],
          "message": {
            "text": "Expected 'do'. Fix any mentioned problems and try again."
          },
          "relatedLocations": [],
          "ruleId": "1072",
          "ruleIndex": 2
        }
      ],
      "tool": {
        "driver": {
          "name": "shellcheck",
          "rules": [
            {
              "fullDescription": {
                "text": "For more information: https://www.shellcheck.net/wiki/SC1073"
              },
              "id": "1073",
              "name": "1073",
              "shortDescription": {
                "text": "SC1073"
              }
            },
            {
              "fullDescription": {
                "text": "For more information: https://www.shellcheck.net/wiki/SC1058"
              },
              "id": "1058",
              "name": "1058",
              "shortDescription": {
                "text": "SC1058"
              }
            },
            {
              "fullDescription": {
                "text": "For more information: https://www.shellcheck.net/wiki/SC1072"
              },
              "id": "1072",
              "name": "1072",
              "shortDescription": {
                "text": "SC1072"
              }
            }
          ]
        }
      }
    }
  ],
  "version": "2.1.0"
}
shellcheck --format=json1 shell.sh | csgrep --mode=sarif | sarif-fmt
error: Couldn't parse this for loop. Fix to allow more checks.
  ┌─ shell.sh:5:1

5 │ for f in "*.ogg"                  # Incorrectly quoted 'for' loops
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Expected 'do'.
  ┌─ shell.sh:6:1

6 │ touch $@                          # Unquoted $@
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: Expected 'do'. Fix any mentioned problems and try again.
  ┌─ shell.sh:6:1

6 │ touch $@                          # Unquoted $@
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: 0 errors emitted
shellcheck --format=json shell.sh | shellcheck-sarif | sarif-fmt
error: Couldn't parse this for loop. Fix to allow more checks.
  ┌─ shell.sh:5:1

5 │ for f in "*.ogg"                  # Incorrectly quoted 'for' loops
  │ ^

  = SC1073
  = For more information: https://www.shellcheck.net/wiki/SC1073

error: Expected 'do'.
  ┌─ shell.sh:6:1

6 │ touch $@                          # Unquoted $@
  │ ^

  = SC1058
  = For more information: https://www.shellcheck.net/wiki/SC1058

error: Expected 'do'. Fix any mentioned problems and try again.
  ┌─ shell.sh:6:1

6 │ touch $@                          # Unquoted $@
  │ ^

  = SC1072
  = For more information: https://www.shellcheck.net/wiki/SC1072

error: 0 errors emitted

Rust

Support for clippy is currently missing in csutils, so this is only an example with sarif-rs utils.

// src/main.rs
fn main() {
  let vec: Vec<isize> = Vec::new();
  if vec.len() <= 0 {}
  if 100 > i32::MAX {}
}
# Cargo.toml
[package]
name = "data"
version = "0.1.0"
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[workspace]
cargo clippy --message-format=json | clippy-sarif
{
  "$schema": "https://schemastore.azurewebsites.net/schemas/json/sarif-2.1.0-rtm.5.json",
  "runs": [
    {
      "results": [
        {
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "src/main.rs"
                },
                "region": {
                  "byteLength": 14,
                  "byteOffset": 53,
                  "endColumn": 20,
                  "endLine": 3,
                  "startColumn": 6,
                  "startLine": 3
                }
              }
            }
          ],
          "message": {
            "text": "this comparison involving the minimum or maximum element for this type contains a case that is always true or always false"
          },
          "ruleId": "clippy::absurd_extreme_comparisons",
          "ruleIndex": 0
        },
        {
          "level": "error",
          "locations": [
            {
              "physicalLocation": {
                "artifactLocation": {
                  "uri": "src/main.rs"
                },
                "region": {
                  "byteLength": 14,
                  "byteOffset": 76,
                  "endColumn": 20,
                  "endLine": 4,
                  "startColumn": 6,
                  "startLine": 4
                }
              }
            }
          ],
          "message": {
            "text": "this comparison involving the minimum or maximum element for this type contains a case that is always true or always false"
          },
          "ruleId": "clippy::absurd_extreme_comparisons",
          "ruleIndex": 0
        }
      ],
      "tool": {
        "driver": {
          "informationUri": "https://rust-lang.github.io/rust-clippy/",
          "name": "clippy",
          "rules": [
            {
              "fullDescription": {
                "text": "because `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `vec.len() == 0` instead\nfor further information visit https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons\n`#[deny(clippy::absurd_extreme_comparisons)]` on by default\n"
              },
              "id": "clippy::absurd_extreme_comparisons"
            }
          ]
        }
      }
    }
  ],
  "version": "2.1.0"
}
cargo clippy --message-format=json | clippy-sarif | sarif-fmt
    Checking data v0.1.0 (/home/jamacku/Source/forks/sarif-rs/sarif-fmt/tests/data)
error: could not compile `data` due to 3 previous errors
error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
  ┌─ src/main.rs:3:6
  │
3 │   if vec.len() <= 0 {}
  │      ^^^^^^^^^^^^^^
  │
  = because `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `vec.len() == 0` instead
    for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
    `#[deny(clippy::absurd_extreme_comparisons)]` on by default

error: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
  ┌─ src/main.rs:4:6
  │
4 │   if 100 > i32::MAX {}
  │      ^^^^^^^^^^^^^^
  │
  = because `0` is the minimum value for this type, the case where the two sides are not equal never occurs, consider using `vec.len() == 0` instead
    for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#absurd_extreme_comparisons
    `#[deny(clippy::absurd_extreme_comparisons)]` on by default

error: 0 errors emitted

csgrep is able to parse SARIF format produced by clippy-sarif.

cargo clippy --message-format=json | clippy-sarif | csgrep
Error: UNKNOWN_SARIF_WARNING:
src/main.rs:3:6: error[clippy::absurd_extreme_comparisons]: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false

Error: UNKNOWN_SARIF_WARNING:
src/main.rs:4:6: error[clippy::absurd_extreme_comparisons]: this comparison involving the minimum or maximum element for this type contains a case that is always true or always false
{
    "defects": [
        {
            "checker": "UNKNOWN_SARIF_WARNING",
            "tool": "unknown-sarif",
            "key_event_idx": 0,
            "events": [
                {
                    "file_name": "src/main.rs",
                    "line": 3,
                    "column": 6,
                    "event": "error[clippy::absurd_extreme_comparisons]",
                    "message": "this comparison involving the minimum or maximum element for this type contains a case that is always true or always false",
                    "verbosity_level": 0
                }
            ]
        },
        {
            "checker": "UNKNOWN_SARIF_WARNING",
            "tool": "unknown-sarif",
            "key_event_idx": 0,
            "events": [
                {
                    "file_name": "src/main.rs",
                    "line": 4,
                    "column": 6,
                    "event": "error[clippy::absurd_extreme_comparisons]",
                    "message": "this comparison involving the minimum or maximum element for this type contains a case that is always true or always false",
                    "verbosity_level": 0
                }
            ]
        }
    ]
}
@jamacku jamacku changed the title [COMPARISION] shellcheck-sarif/sarif/fmt 🆚 csgrep [COMPARISION] shellcheck-sarif/sarif-fmt 🆚 csgrep Aug 15, 2023
@lzaoral
Copy link
Member

lzaoral commented Apr 3, 2024

The rules generated by csutils must include the shortDescription.text and fullDescription.text keys for the rule be included on GitHub: https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/sarif-support-for-code-scanning#reportingdescriptor-object

@kdudka
Copy link
Member

kdudka commented Apr 3, 2024

@lzaoral We already provide help text to be shown by GitHub:

Do we have any other information to put into shortDescription.text and fullDescription.text?

@lzaoral
Copy link
Member

lzaoral commented Apr 3, 2024

Yes, I know. But according to the documentation above, everything from this list:

  • id
  • fullDescription.text
  • shortDescription.text
  • help.text

id and help.text are already handled. name is optional, but is probably sufficient to use its value for shortDescription.text. So the only thing remaining is fullDescription.text.

@lzaoral
Copy link
Member

lzaoral commented Apr 29, 2024

@kdudka Would it make sense to use JSON output instead of the the GCC format of ShellCheck in OSH?

@kdudka
Copy link
Member

kdudka commented Apr 29, 2024

That could be difficult because we use ShellCheck-0.3.5 on el7 and ShellCheck-0.6.0 on el8. These versions did not support the json1 format.

@kdudka
Copy link
Member

kdudka commented Apr 29, 2024

Actually, we use ShellCheck-0.8.0, which knows json1, on el8 but we still have a problem on el7/el6.

@kdudka
Copy link
Member

kdudka commented Nov 8, 2024

Thanks to csutils/csmock#182, csmock and consequently OSH now use ShellCheck with the JSON output format only.

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

No branches or pull requests

3 participants