Let some people know they can try out newer versions, if we know they aren't bad! #225

DarthHater · 2021-04-13T06:30:25Z

It dawned on me that because go list -m -json -u all let's us know the newest version of something, we can run that version past OSS Index and see if it's vulnerable or not. If it's not, let's suggest someone update to it!

This pull request makes the following changes:

way too many to count right now, putting this up to start getting feedback!

Of note, this is slower, but it's because go list -m -json -u all is making requests to I assume pkg.go.dev to see what the newest version of a package is

TODO:

go through and fix busted tests, add good new ones
modify how we pass things around such that we know if someone used dep or go mod, so we can give them the right instructions on how to fix things
suggest a go get -u command to users if the dependency is in the go.mod file, and then the replace if the dependency is not in their go.mod file?

This builds the path towards a nancy solve command where we can go ok, you want us to take care of this? We will give it a shot! That command can work with both ossindex and iq, because we should get similar data from IQ, knowing what the recommended version of something is.

cc @bhamail / @DarthHater

DarthHater · 2021-04-13T07:12:55Z

go.mod

+// // fix vulnerability: CVE-2020-15114 in etcd v3.3.13+incompatible
+// replace github.com/coreos/etcd => github.com/coreos/etcd v3.3.24+incompatible


I have these commented out for easy local testing of vulnerable dependencies

DarthHater · 2021-04-13T07:13:00Z

go.mod

+// // fix vulnerability: CVE-2021-3121 in github.com/gogo/protobuf v1.2.1
+// replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2


I have these commented out for easy local testing of vulnerable dependencies

DarthHater · 2021-04-13T07:13:31Z

internal/audit/auditlog.go


-	for _, c := range coordinates {
-		c.ExcludeVulnerabilities(exclusions)
+	if exclusions == nil {


I moved this further up because it seemed odd it would make this AFTER we try using exclusions?

so the only reason it was down there was b/c if you log it it would not work correctly i believe. So if you passed in nil we wanted it to present to the user as []. Its used in the ExcludeVulnerabilites but range over nil is noop ¯_(ツ)_/¯

https://github.com/sonatype-nexus-community/go-sona-types/blob/b469b94b5ebb013125d69c3524ba5d6f759eede0/ossindex/types/types.go#L103-L107

So really we only do it for presentation. If we can find a better way im cool with it. But moving it up or down doesn't mean much just has to be before the log statement at line 46 hits.

DarthHater · 2021-04-13T07:14:22Z

internal/audit/auditlogtextformatter.go

-func splitPackages(entries []types.Coordinate) (nonVulnerable []types.Coordinate, vulnerable []types.Coordinate) {
-	for _, v := range entries {
-		if v.IsVulnerable() {
-			vulnerable = append(vulnerable, v)
-		} else {
-			nonVulnerable = append(nonVulnerable, v)
-		}
-	}
-	return
-}


This wasn't even being used, or rather it was, but we already had two lists of data. I removed it because LOL

packages/packages.go

bhamail

Seems mostly sane to me. Project=Dep and Coordinate stuff gets confusing, but I don't see a clearer path. "Naming" (pronounce like Seinfeld "Newman").

For later discussion (version 2?): When we find a "clean" newer version, is a "replace" directive the best solution to suggest? If we could determine if the dependency is non-transitive (is direct - not indirect), then maybe suggesting a go get -u mydep@v command is better? Probably a lot more trouble, but wanted to ask. In my mind, the problem with adding replace directives is they are a form of technical debt - ideally, you always prefer upgrading to new versions of deps instead. With replace directives, you need to go back "someday" and determine if the directive can be removed in favor of a newer dependency version.

bhamail · 2021-04-13T18:31:05Z

internal/audit/auditlogtextformatter.go

@@ -76,19 +77,19 @@ func logInvalidSemVerWarning(sb *strings.Builder, noColor bool, quiet bool, inva
 	}
 }

-func logVulnerablePackage(sb *strings.Builder, noColor bool, coordinate types.Coordinate) {
+func logVulnerablePackage(sb *strings.Builder, noColor bool, coordinate types.Projects) {


I was struggling with the change to types.Projects. Made more sense once I saw how the term is used in the dep code, e.g.: dep.Project - oh yeah, a coordinate really refers to a given "project" that we depend on...Oh the joys of Naming.

Yeah I'm not attached to this name TBH. The changes in this PR would merit a new major version of Nancy, so if we want to clean up some naming, let's do it!

bhamail · 2021-04-13T18:43:29Z

internal/cmd/root.go

 	if err != nil {
 		return
 	}

+	// Wittle down list of audited to vulnerable stuff, so we can work faster


typo: Whittle ;)

WOOD YOU GIVE ME A BREAK!?!

Co-authored-by: gmohre <[email protected]>

DarthHater · 2021-04-13T19:34:53Z

Seems mostly sane to me. Project=Dep and Coordinate stuff gets confusing, but I don't see a clearer path. "Naming" (pronounce like Seinfeld "Newman").

For later discussion (version 2?): When we find a "clean" newer version, is a "replace" directive the best solution to suggest? If we could determine if the dependency is non-transitive (is direct - not indirect), then maybe suggesting a go get -u mydep@v command is better? Probably a lot more trouble, but wanted to ask. In my mind, the problem with adding replace directives is they are a form of technical debt - ideally, you always prefer upgrading to new versions of deps instead. With replace directives, you need to go back "someday" and determine if the directive can be removed in favor of a newer dependency version.

I absolutely agree with you. What I was THINKING is we can parse go.mod to see if the dependency is in there, and if it is, we can suggest go get -u <dependency>, and if not, suggest the replace. It'd be nice if the go API had the ability to let us see what brought something in (which is slightly possible with go mod graph but it's not maybe the cleanest way of getting things)

What do you think about changing Project to Dependency because that might be clearer?

bhamail · 2021-04-13T20:38:08Z

What do you think about changing Project to Dependency because that might be clearer?

Yes, I think that change might ease the cognitive load.

…ancy into NancyHelpsYou

zendern · 2021-04-16T01:29:27Z

internal/audit/csvformatter.go

 		buildVersion := entry.Data["version"].(string)
+		numVulnerable := len(vulnerableEntries.(map[string]interface{}))
+		numPackages := len(auditedEntries)

 		var summaryHeader = []string{"Audited Count", "Vulnerable Count", "Build Version"}
 		var invalidHeader = []string{"Count", "Package", "Reason"}
 		var auditedHeader = []string{"Count", "Package", "Is Vulnerable", "Num Vulnerabilities", "Vulnerabilities"}


Should we also maybe add these "can be updated" packages to the csv output too?? Maybe a column of "Latest non-vulnerable version" or something like that??

zendern · 2021-04-16T01:43:12Z

packages/packages.go

+	version = strings.Replace(version, "v", "", -1)
+	version = strings.Replace(version, "+incompatible", "", -1)


Do we need to strip more here?? +meta, -pre, etc etc.

https://golang.org/ref/mod#versions

I THINK I stripped that version part out for OSS Index, IIRC. I'd have to look further into it.

sonarqubecloud · 2023-08-03T18:35:17Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
11 Code Smells

No Coverage information
0.0% Duplication

You bet I busted some stuff, but lets start seeing what people think

6a5ca1d

DarthHater requested review from bhamail and zendern April 13, 2021 06:30

DarthHater added enhancement New feature or request reduce-friction labels Apr 13, 2021

DarthHater added 6 commits April 12, 2021 22:46

Add a comment on what it fixes

26acba3

add in some color, why not

3c452f5

Simplify this a bit

2d6749e

Fix up CSV

a514a96

Add some comments to more rapidly explain what I did

bd7f252

Various

9feac53

DarthHater modified the milestone: v.1.0.0 Apr 13, 2021

DarthHater commented Apr 13, 2021

View reviewed changes

packages/packages.go Outdated Show resolved Hide resolved

bhamail approved these changes Apr 13, 2021

View reviewed changes

Update packages/packages.go

b07433a

Co-authored-by: gmohre <[email protected]>

DarthHater added 4 commits April 14, 2021 00:12

Rename Projects to Dependency, and do some refactoring while I am at it

453613b

Merge branch 'NancyHelpsYou' of github.com:sonatype-nexus-community/n…

45979ee

…ancy into NancyHelpsYou

Turns out this DOES need to be public, at least for now

af2d3f2

Fix tests, more to go

e9e94b0

zendern reviewed Apr 16, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let some people know they can try out newer versions, if we know they aren't bad! #225

Let some people know they can try out newer versions, if we know they aren't bad! #225

DarthHater commented Apr 13, 2021 •

edited

Loading

DarthHater Apr 13, 2021

DarthHater Apr 13, 2021

DarthHater Apr 13, 2021

zendern Apr 16, 2021

DarthHater Apr 13, 2021

bhamail left a comment •

edited

Loading

bhamail Apr 13, 2021

DarthHater Apr 13, 2021

bhamail Apr 13, 2021

DarthHater Apr 13, 2021

zendern Apr 16, 2021

DarthHater commented Apr 13, 2021

bhamail commented Apr 13, 2021

zendern Apr 16, 2021

zendern Apr 16, 2021

DarthHater Apr 16, 2021

sonarqubecloud bot commented Aug 3, 2023

		// // fix vulnerability: CVE-2020-15114 in etcd v3.3.13+incompatible
		// replace github.com/coreos/etcd => github.com/coreos/etcd v3.3.24+incompatible

		// // fix vulnerability: CVE-2021-3121 in github.com/gogo/protobuf v1.2.1
		// replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2

		version = strings.Replace(version, "v", "", -1)
		version = strings.Replace(version, "+incompatible", "", -1)

Let some people know they can try out newer versions, if we know they aren't bad! #225

Are you sure you want to change the base?

Let some people know they can try out newer versions, if we know they aren't bad! #225

Conversation

DarthHater commented Apr 13, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhamail left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarthHater commented Apr 13, 2021

bhamail commented Apr 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 3, 2023

DarthHater commented Apr 13, 2021 •

edited

Loading

bhamail left a comment •

edited

Loading