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

WIP: changes for sarif format #48

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

prashbnair
Copy link

@arajkumar @deepak1725 This is a WIP PR, i wanted some feedback on the approach.

This implements the SARIF output for maven, npm and python. GoLang need to be incorporated.

Command : crda analyse -v -s

@prashbnair prashbnair force-pushed the sarif-output branch 4 times, most recently from 43e0ca9 to 6429ff5 Compare April 16, 2021 06:47
changing version of testify

removed stretchr from go.sum

correcting dependencies in go.mod

changing function call to support older version of go
@codecov-commenter
Copy link

codecov-commenter commented Apr 30, 2021

Codecov Report

Merging #48 (9731cc1) into main (902e40f) will decrease coverage by 2.66%.
The diff coverage is 1.40%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #48      +/-   ##
==========================================
- Coverage   35.74%   33.07%   -2.67%     
==========================================
  Files          23       24       +1     
  Lines         831      901      +70     
==========================================
+ Hits          297      298       +1     
- Misses        506      575      +69     
  Partials       28       28              
Impacted Files Coverage Δ
analyses/verbose/helper.go 75.96% <ø> (ø)
analyses/verbose/sarif_helper.go 0.00% <0.00%> (ø)
cmd/analyse.go 12.50% <50.00%> (+1.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 902e40f...9731cc1. Read the comment docs.

Copy link
Member

@deepak1725 deepak1725 left a comment

Choose a reason for hiding this comment

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

Crashing on 0 Vulnerabilities/Issues

Copy link
Member

@deepak1725 deepak1725 left a comment

Choose a reason for hiding this comment

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

Minor fixes

"regexp"
"strings"
)

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// RegexDependencyLocator --relevant comment--

In Golang, every exported member has a relevant comment

EndIndices []int
DependencyNodeIndex []int
}

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// ProcessSarif -- a valid comment can be added--

ditto for other exported members

report, err := sarif.New(sarif.Version210)

if err != nil {
log.Fatal().Msg("Error forming SARIF respose")
Copy link
Member

Choose a reason for hiding this comment

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

Fatal exits with exit code 1, will not go to the next step

func (r *RegexDependencyLocator) SetUp(manifestFilePath string, ecosystem string) error{
content, err := ioutil.ReadFile(manifestFilePath)
if err != nil {
log.Fatal().Msg("Unable to load manifest File " + manifestFilePath)
Copy link
Member

Choose a reason for hiding this comment

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

ditto

}
}

report.Write(os.Stdout)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
report.Write(os.Stdout)
err = report.Write(os.Stdout)
if err != nil {
return false, err
}


}

func (r *RegexDependencyLocator) LocateDependency(dependency string) (int, int){
Copy link
Member

@deepak1725 deepak1725 Apr 30, 2021

Choose a reason for hiding this comment

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

i feel LocateDependency is not imported outside of this package, No point in exporting it.

It should be locateDependency

}


func (r *RegexDependencyLocator) SetUp(manifestFilePath string, ecosystem string) error{
Copy link
Member

Choose a reason for hiding this comment

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

ditto, can be setUp
All non-exported members start with lowercase letter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants