Add 'gas' for security problems scanning #830
Conversation
jpkrohling
requested review from
black-adder,
pavolloffay,
vprithvi and
yurishkuro
as code owners
ghost
assigned 
| @@ -59,7 +59,7 @@ func loadStrategies(strategiesFile string) (*strategies, error) { | |||
| if strategiesFile == "" { | |||
| return nil, nil | |||
| } | |||
| bytes, err := ioutil.ReadFile(strategiesFile) | |||
| bytes, err := ioutil.ReadFile(strategiesFile) /* nolint #nosec , this comes from an admin, not user */ | |||
There was a problem hiding this comment.
why does this require linter directives?
There was a problem hiding this comment.
The theory behind it is that strategiesFile can be manipulated by an end user, so that a file like /etc/shadow could be read. I tracked all instances of this case (G304, IIRC) and they all come from the CLI, meaning that they are set by an admin, and not by an end user.
.travis.yml
Outdated
| @@ -11,6 +11,7 @@ matrix: | |||
| env: | |||
| - TESTS=true | |||
| - COVERAGE=true | |||
| - LINT=true | |||
There was a problem hiding this comment.
what's the reason to separate this from the main make lint target? Does it run much slower?
There was a problem hiding this comment.
Interesting, I thought make lint wasn't being used on purpose, because it fails with:
$ make lint
go vet ./cmd/... ./model/... ./pkg/... ./plugin/... ./crossdock/... ./storage/... .
cmd/query/app/static_handler_test.go:121: possible formatting directive in Error call
exit status 1
make: *** [Makefile:114: lint] Error 1
This failure does not show up in Travis though. I just changed this PR to fix the lint failure above and to get make lint to also call the target gas
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
| @@ -128,6 +133,10 @@ install-go-bindata: | |||
| go get github.com/jteeuwen/go-bindata/... | |||
| go get github.com/elazarl/go-bindata-assetfs/... | |||
|
|
|||
| .PHONY: install-gas | |||
| install-gas: | |||
| go get github.com/GoASTScanner/gas/cmd/gas/... | |||
There was a problem hiding this comment.
I am still not clear why we need the extra makefile targets instead of rolling it up into the main install and lint (extra targets increase noise / entropy)
There was a problem hiding this comment.
install is quite clean, doing only a glide install. It does delegate the installation of the dependent tool (glide) to a separate target, so, there's a precedent :-)
About gas getting called directly from lint, I guess it's a matter of taste. When working on fixing the warnings provided by gas, I find it easier to run it in isolation as make gas.
* master: (38 commits) Preparing release 1.5.0 (jaegertracing#847) Add bounds to memory storage (jaegertracing#845) Add metric for debug traces (jaegertracing#796) Change metrics naming scheme (jaegertracing#776) Bump gocql version (jaegertracing#829) Remove ParentSpanID from domain model (jaegertracing#831) Make gas run quiet (jaegertracing#838) Revert "Make gas run quite" Revert "Install gas from install-ci" Install gas from install-ci Make gas run quite Add 'gas' for security problems scanning (jaegertracing#830) Add ability to adjust static sampling probabilities per operation (jaegertracing#827) Support log-level flag on agent (jaegertracing#828) Remove unused function (jaegertracing#822) Add healthcheck to standalone (jaegertracing#784) Do not use KeyValue fields directly and use KeyValues as decorator only (jaegertracing#810) Add ContaAzul to the adopters list (jaegertracing#806) Add ISSUE_TEMPLATE and PULL_REQUEST_TEMPLATE (jaegertracing#805) Upgrade to go 1.10 (jaegertracing#792) ... # Conflicts: # cmd/agent/app/builder.go # cmd/collector/main.go # cmd/query/main.go # cmd/standalone/main.go
Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de
Which problem is this PR solving?
static_analysis_common_vulnerabilitiesandstatic_analysis_fixedfrom the CII Best PracticesShort description of the changes
make gasgasthat those are OKgasOthers
gometalinterand activate onlygas, but got a different output, so, I think it might make sense to cherry-pick the linters and configure them individuallymake linttarget, but doesn't seem to be used anywhere. And returns quite a bunch of messages :) So, I'm naming the env varLINT, but ignoringmake lintfor now. Hopefully, we'll use it later.