Skip to content

Include DirScanErrors info in SARIF file #1398

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

Merged

Conversation

shaopeng-gh
Copy link
Contributor

Hello, currently the DirScanErrors messages are not recorded in the SARIF file,
this information could be useful for the SARIF file consumer.
This pr added this missing info so that the consumer of the SARIF file can have this info and act as necessary.
These messages are considered warning so the change is adding a new node to the json which should not break existing user.
We have updated the go-sarif sdk to the v2, which is necessary for this change.
And I see v2 have much more SARIF properties support than v1 which is great changes.
The json properties order changed slightly so the text cases are fixed accordingly.
Fixed a issue that multierr.Append() does not assign the result so it is missing from the object for later usage.
golang.org/x/exp/typeparams is removed by tidy mod.

@shaopeng-gh shaopeng-gh requested review from a team and bkizer-tenable as code owners September 20, 2022 05:31
@codecov-commenter
Copy link

codecov-commenter commented Sep 20, 2022

Codecov Report

Merging #1398 (8f4e507) into master (849eef7) will increase coverage by 0.02%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1398      +/-   ##
==========================================
+ Coverage   77.27%   77.30%   +0.02%     
==========================================
  Files         279      279              
  Lines        7856     7866      +10     
==========================================
+ Hits         6071     6081      +10     
  Misses       1426     1426              
  Partials      359      359              
Impacted Files Coverage Δ
pkg/iac-providers/cft/v1/load-file.go 83.67% <50.00%> (ø)
pkg/writer/sarif.go 82.35% <100.00%> (+3.04%) ⬆️

gaurav-gogia
gaurav-gogia previously approved these changes Sep 20, 2022
Copy link
Contributor

@gaurav-gogia gaurav-gogia left a comment

Choose a reason for hiding this comment

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

LGTM

@shaopeng-gh
Copy link
Contributor Author

@gaurav-gogia thanks for the review.

@shaopeng-gh
Copy link
Contributor Author

@nasir-rabbani Let me know if looks good to merge, thanks.

@VeraBE
Copy link
Contributor

VeraBE commented Oct 20, 2022

Pinging, it would be great if this improvement was merged soon

@shaopeng-gh
Copy link
Contributor Author

note: The build / validate (pull_request) Failing after 16m happens after I fetch latest from master, I see other PRs have the same error so looks like not a issue of the PR.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@shaopeng-gh
Copy link
Contributor Author

Update: Test passed.

Copy link
Contributor

@gaurav-gogia gaurav-gogia left a comment

Choose a reason for hiding this comment

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

LGTM

@gaurav-gogia gaurav-gogia merged commit 7cf9d3c into tenable:master Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants