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

feat: fake ignore findings [IDE-176] #8

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Conversation

teodora-sandu
Copy link
Contributor

@teodora-sandu teodora-sandu commented Mar 12, 2024

We have agreed with Analysis and the CLI on what the Analysis API will return.

This PR updates the existing fake SARIF response for scanning to include ignores for one of the results, so that we can do development in the IDEs.

Ticket: https://snyksec.atlassian.net/browse/IDE-176

Tested it with snyk/snyk-ls#464 and snyk/snyk-intellij-plugin#490 and without those changes, to make sure it's still backwards compatible.

@CLAassistant
Copy link

CLAassistant commented Mar 12, 2024

CLA assistant check
All committers have signed the CLA.

Properties SuppressionProperties `json:"properties"`
}

type SuppressionProperties struct {
Copy link
Contributor Author

@teodora-sandu teodora-sandu Mar 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want the response returned by the SARIF to match what's in the designs, since it might also get printed by the CLI: https://www.figma.com/file/hcI2QHUtHfcIjgrpYlMqff/Holistic-Ignores?type=design&node-id=773-49520&mode=design&t=RrNM0S3glyhiAJUT-0

sarif_types.go Outdated
}

type SuppressionProperties struct {
Category string `json:"category"`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this? Does it map to reasonType: WONT_FIX|NOT_VULNERABLE|TEMPORARY_IGNORE?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's it. I originally had it as reasonType but I wanted it to match what we're meant to render in the CLI/IDEs. We could also leave it up to the CLI/IDE to translate these fields but I'd rather do it all in one place

@teodora-sandu teodora-sandu marked this pull request as ready for review March 12, 2024 16:11
@teodora-sandu teodora-sandu requested a review from a team as a code owner March 12, 2024 16:11
@github-actions github-actions bot added feature and removed feature labels Mar 12, 2024
@teodora-sandu teodora-sandu force-pushed the feat/fake-ignores branch 3 times, most recently from 369ff3f to 1a92073 Compare March 12, 2024 18:19
@github-actions github-actions bot added feature and removed feature labels Mar 13, 2024
sarif_types.go Outdated Show resolved Hide resolved

type IgnoredBy struct {
Name string `json:"name"`
Email *string `json:"email"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointer to allow for nullability?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep I based it off the Output in https://snyksec.atlassian.net/wiki/spaces/RD/pages/1898152058/SDK+for+calculating+Code+Ignores+on+Findings. Maybe I ought to make Expiration a pointer too

Copy link
Contributor

@bastiandoetsch bastiandoetsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only minor stuff.

scan.go Outdated Show resolved Hide resolved
@teodora-sandu teodora-sandu merged commit c5f9495 into main Mar 13, 2024
9 checks passed
@teodora-sandu teodora-sandu deleted the feat/fake-ignores branch March 13, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants