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

de-duplicate SAST workflow scanning #3745

Closed
spencerschrock opened this issue Dec 19, 2023 · 1 comment · Fixed by #3743
Closed

de-duplicate SAST workflow scanning #3745

spencerschrock opened this issue Dec 19, 2023 · 1 comment · Fixed by #3743

Comments

@spencerschrock
Copy link
Contributor

Between the 3 existing detected SAST tools, there's a lot of duplication:

codeQLWorkflows, err := codeQLInCheckDefinitions(c)
if err != nil {
return data, err
}
data.Workflows = append(data.Workflows, codeQLWorkflows...)
sonarWorkflows, err := getSonarWorkflows(c)
if err != nil {
return data, err
}
data.Workflows = append(data.Workflows, sonarWorkflows...)
snykWorkflows, err := getSnykWorkflows(c)
if err != nil {
return data, err
}
data.Workflows = append(data.Workflows, snykWorkflows...)
return data, nil

(although getSonarWorkflows might be a special case?)

Especially as more tools are added (#3743 ), we shouldn't have the same code copy/pasted for each workflow detection.

It seems like there's a common state for these github action detections:
mapping some SASTWorkflowType value (Snyk, Sonar, CodeQL, etc) to some action regex:

Snyk: strings.HasPrefix(action, "snyk/actions/")
CodeQL: action == "github/codeql-action/analyze"

so as regex maybe?
Snyk: "^snyk/actions/.*" (prefix)
CodeQL: "^github/codeql-action/analyze$" full match
qodona: "^JetBrains/qodana-action$" full match
pysa: "^facebook/pysa-action$" full match

@spencerschrock spencerschrock added this to the Structured results milestone Dec 19, 2023
DavidKorczynski added a commit to DavidKorczynski/scorecard that referenced this issue Dec 21, 2023
Ref: ossf#3745

Signed-off-by: David Korczynski <david@adalogics.com>
@DavidKorczynski
Copy link
Contributor

Added fix in #3743

spencerschrock pushed a commit that referenced this issue Jan 2, 2024
* Add SAST Pysa probe

Signed-off-by: David Korczynski <david@adalogics.com>

* Add Pysa positive unit test

Signed-off-by: David Korczynski <david@adalogics.com>

* Add Qodana as well

Signed-off-by: David Korczynski <david@adalogics.com>

* fix some styling

Signed-off-by: David Korczynski <david@adalogics.com>

* fix some messaging

Signed-off-by: David Korczynski <david@adalogics.com>

* checks: raw: sast: dedup by way of regex

Ref: #3745

Signed-off-by: David Korczynski <david@adalogics.com>

* deduplicate SAST score checker

Signed-off-by: David Korczynski <david@adalogics.com>

* fix styling

Signed-off-by: David Korczynski <david@adalogics.com>

* fix styling

Signed-off-by: David Korczynski <david@adalogics.com>

* Rename variables appropriately

Signed-off-by: David Korczynski <david@adalogics.com>

* fix error message

Signed-off-by: David Korczynski <david@adalogics.com>

* rename useRegex to usesRegex and add comment

Signed-off-by: David Korczynski <david@adalogics.com>

* Force regex to compile

Signed-off-by: David Korczynski <david@adalogics.com>

---------

Signed-off-by: David Korczynski <david@adalogics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants