Legit-Labs/legitify

SARIF format results do no supply the actual violation results?

iozery opened this issue · 2 comments

iozery commented

TL;DR

SARIF format results do not include status of the violation - i.e. "PASSED", "FAILED" etc - the field level in the results corresponds to the severity of the security rule.

Expected behavior

Running legitify with the following command:
go run main.go analyze -t <token> --org <org-name> --scm github --output-format sarif --output-file results.json

Expected to view the violation status in the SARIF results

Observed behavior

SARIF-based format results

{
  "ruleId": "data.actions.actions_can_approve_pull_requests",
  "ruleIndex": 0,
  "level": "error",
  "message": {
    "text": "The default GitHub Actions configuration allows for workflows to approve pull requests. This could allow users to bypass code-review restrictions.  \nLink to organization actions: https://github.com/organizations/<org-name>/settings/actions  \nAuxiliary Info:  \nEntity Id: <entity-id>  \nEntity Name: <org-name>  \nOrganization Id: <org-id>  \n"
  },
  "locations": [
    {
      "physicalLocation": {
        "artifactLocation": {
          "uri": "github.com/organizations/<org-name>/settings/actions",
          "uriBaseId": "https://"
        }
      }
    }
  ],
  "hostedViewerUri": "https://github.com/organizations/<org-name>/settings/actions"
}

JSON-based Format results

{
  "violations": [
    {
      "violationEntityType": "organization actions",
      "canonicalLink": "https://github.com/organizations/<org-name>/settings/actions",
      "aux": {
        "entityId": "<entity-id>",
        "entityName": "<org-name>",
        "organizationId": "<org-id> "
      },
      "status": "PASSED"
    }
  ]
}

BTW, According to the actions settings, the JSON-based result is correct.

Version

v1.0.1

On which operating system are you using legitify?

Mac OS

Relevant log output

No response

Additional information

As it seems by the formatter_sarif.go file, there is no reference to the Status field of the violation:

for _, violation := range data.Violations {
			base, uri := f.URIFromLink(violation.CanonicalLink)
			run.AddDistinctArtifact(violation.ViolationEntityType)
			run.CreateResultForRule(policyInfo.FullyQualifiedPolicyName).
				WithLevel(sarifSeverity(policyInfo.Severity)).
				WithMessage(sarif.NewTextMessage(getViolationMessage(&violation, &policyInfo))).
				WithHostedViewerUri(violation.CanonicalLink).
				AddLocation(
					sarif.NewLocationWithPhysicalLocation(
						sarif.NewPhysicalLocation().
							WithArtifactLocation(
								sarif.NewArtifactLocation().
									WithUri(uri).WithUriBaseId(base),
							),
					),
				)
		}

Thanks @iozery.

I've created a PR to include only failed instances in the SARIF output https://github.com/Legit-Labs/legitify/pull/252/files

I'll update once merged.

I assume you don't expect to see PASSED entities in the sarif output, if you do please let us know.

Merged, closing the issue for now