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

fix: sanitize level property for SARIF #4831

Merged
merged 4 commits into from
Jun 19, 2024
Merged

fix: sanitize level property for SARIF #4831

merged 4 commits into from
Jun 19, 2024

Conversation

Zxilly
Copy link
Contributor

@Zxilly Zxilly commented Jun 19, 2024

https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/sarif-v2.1.0-errata01-os-complete.html#_Toc141790898

level can only be none, note, warning, error, some linter like gosec may report something like high

For real-world error, see https://github.com/Zxilly/go-size-analyzer/actions/runs/9586342361/job/26434117667

@ldez ldez added the area: output Related to issue output label Jun 19, 2024
@ldez
Copy link
Member

ldez commented Jun 19, 2024

There is no severity convention inside golangci-lint but you can override or define your own inside the severity section.

https://golangci-lint.run/usage/configuration/#severity-configuration

@ldez ldez self-requested a review June 19, 2024 20:02
@ldez ldez added this to the next milestone Jun 19, 2024
@Zxilly
Copy link
Contributor Author

Zxilly commented Jun 19, 2024

I'm not familiar with the golangci lint codebase, but I was wondering if an e2e test exists? Maybe we can add to these tests for sarif and we can validate the output json file with a json schema.

@ldez
Copy link
Member

ldez commented Jun 19, 2024

we don't need e2e to validate the output, and to be honest, I was thinking of adding a test about that.
But those tests will not be more useful than the current tests.

@ldez ldez changed the title fix: sarif level property should be enum fix: sanitize level property for SARIF Jun 19, 2024
@ldez ldez added the bug Something isn't working label Jun 19, 2024
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM

@ldez ldez merged commit 304e22a into golangci:master Jun 19, 2024
16 checks passed
@ldez ldez modified the milestones: next, v1.60 Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: output Related to issue output bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants