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

Sarif format tweaks #1063

Open
shaopeng-gh opened this issue Oct 19, 2021 · 3 comments
Open

Sarif format tweaks #1063

shaopeng-gh opened this issue Oct 19, 2021 · 3 comments

Comments

@shaopeng-gh
Copy link
Contributor

  • terrascan version: Current
  • Operating System: Windows 10

Description

Hello! this is Shaopeng from Microsoft Sarif team. Our team creates and maintains the Sarif format standard.
https://github.com/oasis-tcs/sarif-spec
https://github.com/microsoft/sarif-sdk
https://github.com/microsoft/sarif-js-sdk
https://github.com/microsoft/sarif-website

Glad to see ‎Terrascan supports output of Sarif format.
We are using -o sarif option, the output Sarif works well.
We do see some minor issues with the result Sarif file, can we suggest a few tweaks so that the output Sarif file can be optimized and better meet the specification?

What I Did

with -o sarif option
@cesar-rodriguez
Copy link
Contributor

Hi @shaopeng-gh,

Thanks for reaching out! Your input to improve this feature would be much appreciated.

@shaopeng-gh
Copy link
Contributor Author

shaopeng-gh commented Oct 26, 2021

Thanks @cesar-rodriguez , below are my inputs:

  1. Let's use the latest schema: https://json.schemastore.org/sarif-2.1.0-rtm.5.json
    I see this is from the package used "owenrumney/go-sarif". I have submitted a PR there. Do we know if the owner of the repo is still active?

  2. Provide "helpUri" if possible
    helpUri can be added in Rule node, so that user can see related help. For example we are using it to scan ARM templates, I see there are related help in this page like https://docs.accurics.com/projects/accurics-terrascan/en/latest/policies/azure/#azurerm_network_watcher_flow_log. I don't see it is in the code object module. Not sure if you can add it to the Sarif output, it is a nice to have.

  3. The message in the result is the same as in the rules. If that is the case, we can use the "messageStrings" from the rules and just make a linking between results and rules, to simplify.
    I see the package used does not support this messageStrings so looks like we can not have it. https://github.com/owenrumney/go-sarif/blob/main/sarif/rule.go

fyi the correct usage is like,

in rule node:
"messageStrings": {
"Default": {
"text": "Ensure 'Enforce SSL connection' is set to 'ENABLED' for MySQL Database Server"
}
}
in result node:
"message": {
"id": "Default"
}

  1. The locations should conform with https://tools.ietf.org/html/rfc3986, we currently get "file://C:\\reporoot\\input\\azuredeploy.json"
    I think this is just a bug in the code for running in Windows, in Linux it is correct. The correct uri path for Windows should be like "file:///C:/reporoot/input/azuredeploy.json" I have submitted a PR for review: Fix Sarif file uri path invalid in Windows and update go-sarif to latest #1070

  2. The paths should be relative and use originalUriBaseIds if possible, to simplify
    My understanding is the go-sarif package also does not really have the support for this to be correctly used. It only have the URIBaseId but not the originalUriBaseIds

fyi the correct usage is like,

in result node:
"artifactLocation": {
"uri": "input/azuredeploy.json",
"uriBaseId": "REPO_ROOT"
},
the originalUriBaseIds node:
"originalUriBaseIds": {
"REPO_ROOT": {
"uri": "file:///c:/reporoot/"
}
}

  1. side question I see you have a forGithub version, can you let me know the context, is it that GitHub throw some errors? Bringing this up because may be this is related to the uri invalid issue above, by moving the path to the uriBaseId and only leave the file name in the uri will pass the uri field validation if there is any.

  2. fyi in Sarif .NET SDK we have a Sarif file format validator and the only location url issue above will make the Sarif file as invalid so this one should be fixed.
    Unfortunately we don't have a SDK for GoLang.

@shaopeng-gh
Copy link
Contributor Author

update for No.1 about schema version, "owenrumney/go-sarif" has approved my PR and created a new version. We just need to update the package version then. I have updated my PR. #1070

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

No branches or pull requests

2 participants