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

Merge experimental / refactor #146

Merged
merged 26 commits into from
Jan 5, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9b08174
Process via packages instead of files
gcmurphy Apr 25, 2017
8df48f9
Fix to reporting to use output formats
gcmurphy Apr 26, 2017
cacf21f
Restructure to focus on lib rather than cli
gcmurphy Apr 26, 2017
bf78d02
Restructure and introduce a standalone config
gcmurphy Apr 28, 2017
50bbc53
Isolate import tracking functionality
gcmurphy May 10, 2017
5160048
Move rule definitions into own file
gcmurphy May 10, 2017
65b18da
Hack to address circular dependency in rulelist
gcmurphy May 10, 2017
026fe4c
Simplify analyzer and command line interface
gcmurphy May 10, 2017
f4b705a
Use glide to manage vendored dependencies
gcmurphy May 10, 2017
6943f9e
Major rework of codebase
gcmurphy Jul 19, 2017
3caf7c3
Add test cases
gcmurphy Sep 16, 2017
9c959ca
Issue.Line is already a string
lanzafame Oct 1, 2017
5a11336
remove commited binary
lanzafame Oct 1, 2017
27b2fd9
Merge pull request #136 from lanzafame/experimental
gcmurphy Oct 4, 2017
67dc432
use godep instead of glide
gcmurphy Dec 13, 2017
d4311c9
make it clear that these tests have not been implemented yet
gcmurphy Dec 13, 2017
02901b9
actually skip tests until implementation exists
gcmurphy Dec 13, 2017
e3b6fd9
update readme to provide info regarding package level scans
gcmurphy Dec 13, 2017
97cde35
update travis-ci to use ginkgo tests
gcmurphy Dec 13, 2017
cfa4327
fix hound-ci errors
gcmurphy Dec 13, 2017
af25ac1
fix golint errors picked up by hound-ci
gcmurphy Dec 13, 2017
25d74c6
address review comments
gcmurphy Dec 14, 2017
e925d3c
Migrated old test cases.
gcmurphy Dec 28, 2017
4c49716
move utils to separate executable
gcmurphy Dec 28, 2017
d452dcb
Fix ginko invocation
gcmurphy Jan 5, 2018
867d300
Fix lint issues
gcmurphy Jan 5, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Major rework of codebase
- Get rid of 'core' and move CLI to cmd/gas directory
- Migrate (most) tests to use Ginkgo and testutils framework
- GAS now expects package to reside in $GOPATH
- GAS now can resolve dependencies for better type checking (if package
  on GOPATH)
- Simplified public API
  • Loading branch information
gcmurphy committed Jul 19, 2017
commit 6943f9e5e4d2c5e7bf5af00d0e9fa90a82a8af3d
20 changes: 15 additions & 5 deletions analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ package gas
import (
"go/ast"
"go/build"
"go/parser"
"go/token"
"go/types"
"log"
"os"
"path"
"reflect"
"strings"
Expand Down Expand Up @@ -64,10 +66,11 @@ type Analyzer struct {
// NewAnalyzer builds a new anaylzer.
func NewAnalyzer(conf Config, logger *log.Logger) *Analyzer {
ignoreNoSec := false
if val, err := conf.Get("ignoreNoSec"); err == nil {
if override, ok := val.(bool); ok {
ignoreNoSec = override
}
if setting, err := conf.GetGlobal("nosec"); err == nil {
ignoreNoSec = setting == "true" || setting == "enabled"
}
if logger == nil {
logger = log.New(os.Stderr, "[gas]", log.LstdFlags)
}
return &Analyzer{
ignoreNosec: ignoreNoSec,
Expand All @@ -94,7 +97,7 @@ func (gas *Analyzer) Process(packagePath string) error {
return err
}

packageConfig := loader.Config{Build: &build.Default}
packageConfig := loader.Config{Build: &build.Default, ParserMode: parser.ParseComments}
packageFiles := make([]string, 0)

Choose a reason for hiding this comment

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

can probably use "var packageFiles []string" instead

for _, filename := range basePackage.GoFiles {
packageFiles = append(packageFiles, path.Join(packagePath, filename))
Expand Down Expand Up @@ -168,3 +171,10 @@ func (gas *Analyzer) Visit(n ast.Node) ast.Visitor {
func (gas *Analyzer) Report() ([]*Issue, *Metrics) {
return gas.issues, gas.stats
}

// Reset clears state such as context, issues and metrics from the configured analyzer
func (gas *Analyzer) Reset() {
gas.context = &Context{}
gas.issues = make([]*Issue, 0, 16)
gas.stats = &Metrics{}
}
113 changes: 113 additions & 0 deletions analyzer_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package gas_test

import (
"bytes"
"io/ioutil"
"log"
"os"
"strings"

"github.com/GoASTScanner/gas"
"github.com/GoASTScanner/gas/rules"

"github.com/GoASTScanner/gas/testutils"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

var _ = Describe("Analyzer", func() {

var (
analyzer *gas.Analyzer
logger *log.Logger
output *bytes.Buffer
)
BeforeEach(func() {
logger, output = testutils.NewLogger()
analyzer = gas.NewAnalyzer(nil, logger)
})

Context("when processing a package", func() {

Copy link
Member

Choose a reason for hiding this comment

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

I would add a test for a package consisting of multiple files.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. Done.

It("should return an error if the package contains no Go files", func() {
analyzer.LoadRules(rules.Generate().Builders()...)
dir, err := ioutil.TempDir("", "empty")
defer os.RemoveAll(dir)
Expect(err).ShouldNot(HaveOccurred())
err = analyzer.Process(dir)
Expect(err).Should(HaveOccurred())
Expect(err.Error()).Should(MatchRegexp("no buildable Go source files"))
})

It("should return an error if the package fails to build", func() {
analyzer.LoadRules(rules.Generate().Builders()...)
pkg := testutils.NewTestPackage()
defer pkg.Close()
pkg.AddFile("wonky.go", `func main(){ println("forgot the package")}`)
pkg.Build()

err := analyzer.Process(pkg.Path)
Expect(err).Should(HaveOccurred())
Expect(err.Error()).Should(MatchRegexp(`expected 'package'`))

})

It("should find errors when nosec is not in use", func() {

// Rule for MD5 weak crypto usage
sample := testutils.SampleCodeG401[0]
source := sample.Code
analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...)

controlPackage := testutils.NewTestPackage()
defer controlPackage.Close()
controlPackage.AddFile("md5.go", source)
controlPackage.Build()
analyzer.Process(controlPackage.Path)
controlIssues, _ := analyzer.Report()
Expect(controlIssues).Should(HaveLen(sample.Errors))

})

It("should not report errors when a nosec comment is present", func() {
// Rule for MD5 weak crypto usage
sample := testutils.SampleCodeG401[0]
source := sample.Code
analyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...)

nosecPackage := testutils.NewTestPackage()
defer nosecPackage.Close()
nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec", 1)
nosecPackage.AddFile("md5.go", nosecSource)
nosecPackage.Build()

analyzer.Process(nosecPackage.Path)
nosecIssues, _ := analyzer.Report()
Expect(nosecIssues).Should(BeEmpty())
})
})

It("should be possible to overwrite nosec comments, and report issues", func() {

// Rule for MD5 weak crypto usage
sample := testutils.SampleCodeG401[0]
source := sample.Code

// overwrite nosec option
nosecIgnoreConfig := gas.NewConfig()
nosecIgnoreConfig.SetGlobal("nosec", "true")
customAnalyzer := gas.NewAnalyzer(nosecIgnoreConfig, logger)
customAnalyzer.LoadRules(rules.Generate(rules.NewRuleFilter(false, "G401")).Builders()...)

nosecPackage := testutils.NewTestPackage()
defer nosecPackage.Close()
nosecSource := strings.Replace(source, "h := md5.New()", "h := md5.New() // #nosec", 1)
nosecPackage.AddFile("md5.go", nosecSource)
nosecPackage.Build()

customAnalyzer.Process(nosecPackage.Path)
nosecIssues, _ := customAnalyzer.Report()
Expect(nosecIssues).Should(HaveLen(sample.Errors))

})
})
12 changes: 6 additions & 6 deletions call_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,19 @@ func (c CallList) Contains(selector, ident string) bool {

/// ContainsCallExpr resolves the call expression name and type
/// or package and determines if it exists within the CallList
func (c CallList) ContainsCallExpr(n ast.Node, ctx *Context) bool {
func (c CallList) ContainsCallExpr(n ast.Node, ctx *Context) *ast.CallExpr {
selector, ident, err := GetCallInfo(n, ctx)
if err != nil {
return false
return nil
}
// Try direct resolution
if c.Contains(selector, ident) {
return true
return n.(*ast.CallExpr)
}

// Also support explicit path
if path, ok := GetImportPath(selector, ctx); ok {
return c.Contains(path, ident)
if path, ok := GetImportPath(selector, ctx); ok && c.Contains(path, ident) {
return n.(*ast.CallExpr)
}
return false
return nil
}
134 changes: 80 additions & 54 deletions call_list_test.go
Original file line number Diff line number Diff line change
@@ -1,60 +1,86 @@
package gas
package gas_test

import (
"go/ast"
"testing"

"github.com/GoASTScanner/gas"
"github.com/GoASTScanner/gas/testutils"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"
)

type callListRule struct {
MetaData
callList CallList
matched int
}

func (r *callListRule) Match(n ast.Node, c *Context) (gi *Issue, err error) {
if r.callList.ContainsCallExpr(n, c) {
r.matched += 1
}
return nil, nil
}

func TestCallListContainsCallExpr(t *testing.T) {
config := map[string]interface{}{"ignoreNosec": false}
analyzer := NewAnalyzer(config, nil)
calls := NewCallList()
calls.AddAll("bytes.Buffer", "Write", "WriteTo")
rule := &callListRule{
MetaData: MetaData{
Severity: Low,
Confidence: Low,
What: "A dummy rule",
},
callList: calls,
matched: 0,
}
analyzer.AddRule(rule, []ast.Node{(*ast.CallExpr)(nil)})
source := `
package main
import (
"bytes"
"fmt"
var _ = Describe("call list", func() {
var (
calls gas.CallList
)
func main() {
var b bytes.Buffer
b.Write([]byte("Hello "))
fmt.Fprintf(&b, "world!")
}`

analyzer.ProcessSource("dummy.go", source)
if rule.matched != 1 {
t.Errorf("Expected to match a bytes.Buffer.Write call")
}
}

func TestCallListContains(t *testing.T) {
callList := NewCallList()
callList.Add("fmt", "Printf")
if !callList.Contains("fmt", "Printf") {
t.Errorf("Expected call list to contain fmt.Printf")
}
}
BeforeEach(func() {
calls = gas.NewCallList()
})

It("should not return any matches when empty", func() {
Expect(calls.Contains("foo", "bar")).Should(BeFalse())
})

It("should be possible to add a single call", func() {
Expect(calls).Should(HaveLen(0))
calls.Add("foo", "bar")
Expect(calls).Should(HaveLen(1))

expected := make(map[string]bool)
expected["bar"] = true
actual := map[string]bool(calls["foo"])
Expect(actual).Should(Equal(expected))
})

It("should be possible to add multiple calls at once", func() {
Expect(calls).Should(HaveLen(0))
calls.AddAll("fmt", "Sprint", "Sprintf", "Printf", "Println")

expected := map[string]bool{
"Sprint": true,
"Sprintf": true,
"Printf": true,
"Println": true,
}
actual := map[string]bool(calls["fmt"])
Expect(actual).Should(Equal(expected))
})

It("should not return a match if none are present", func() {
calls.Add("ioutil", "Copy")
Expect(calls.Contains("fmt", "Println")).Should(BeFalse())
})

It("should match a call based on selector and ident", func() {
calls.Add("ioutil", "Copy")
Expect(calls.Contains("ioutil", "Copy")).Should(BeTrue())
})

It("should match a call expression", func() {

// Create file to be scanned
pkg := testutils.NewTestPackage()
defer pkg.Close()
pkg.AddFile("md5.go", testutils.SampleCodeG401[0].Code)

ctx := pkg.CreateContext("md5.go")

// Search for md5.New()
calls.Add("md5", "New")

// Stub out visitor and count number of matched call expr
matched := 0
v := testutils.NewMockVisitor()
v.Context = ctx
v.Callback = func(n ast.Node, ctx *gas.Context) bool {
if _, ok := n.(*ast.CallExpr); ok && calls.ContainsCallExpr(n, ctx) != nil {
matched++
}
return true
}
ast.Walk(v, ctx.Root)
Expect(matched).Should(Equal(1))

})

})
Loading