sonatype-nexus-community / nancy

A tool to check for vulnerabilities in your Golang dependencies, powered by Sonatype OSS Index
Apache License 2.0
564 stars 74 forks source link

Beef up user agent string #86

Closed DarthHater closed 4 years ago

DarthHater commented 4 years ago

Basically, we send in a user-agent string, but it was really "simple". Let's add a couple bits more info:

What is the goal here? Well, not to be creepy, that's for sure. It's to get some information that we can use to help prioritize new work, like building more orbs, working on new orbs, etc...

This pull request makes the following changes:

cc @bhamail / @DarthHater / @zendern / @greut / @fitzoh

greut commented 4 years ago

A way to put this in the http.Client once and for all, https://github.com/hashicorp/terraform-plugin-sdk/blob/master/internal/httpclient/client.go

bhamail commented 4 years ago

I would like to always check and use the SC_CALLER_INFO env var (regardless of if we detect a CI in use). I'm working on a branch that shows this idea.

The SC_CALLER_INFO is meant to be potentially more of a SC_CALL_STACK, containing multiple caller info's, and would not replace the current user agent info. e.g. in TestGetUserAgentScCallerInfo(): instead of:

expected := "nancy-client/development (bitbucket-nancy-pipe-0.1.9; linux amd64)"

have:

expected := "nancy-client/development (ci usage; linux amd64)__(bitbucket-nancy-pipe-0.1.9; linux amd64)"

Basically, the SC_CALL_STACK feature is not tied to CI detection. Will have to figure out the best way to embed this call tree info so HDS is happy. I'm also thinking about adding some simple delimiter to keep regex madness at bay when we try to read this string. (e.g. the double underscore used to delimit caller's call info above).

DarthHater commented 4 years ago

@bhamail I'm struggling to think of a case where it wouldn't be in CI, what do you imagine instead?

bhamail commented 4 years ago

@bhamail I'm struggling to think of a case where it wouldn't be in CI, what do you imagine instead?

Not sure I have common use cases, but couldn't see a reason not to use the env var if set. Grasping at some edge cases:

  1. Running the pipe.sh script from the BitBucket Pipe locally to validate/reproduce any failures that happened in CI.
  2. Some shiny new CI where for whatever reason, we don't detect being in CI. We'd still get the call tree info.
  3. Simplified debugging of the call tree behavior (though it is probably just as easy to fake setting of the expected CI env vars - so this use case is really a stretch).

Here's the PR where I made some changes: https://github.com/sonatype-nexus-community/nancy/pull/87 The build_number stuff could (should?) easily be removed (just added it because I was there). I'm also happy to remove the "always read SC_CALLER_INFO env var" if we decide that is best. The main goal I'm after is the getting the DansCrazyCallTree string stuffed into the Other slot of the User-Agent string.