-
-
Notifications
You must be signed in to change notification settings - Fork 620
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
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 8df48f9
Fix to reporting to use output formats
gcmurphy cacf21f
Restructure to focus on lib rather than cli
gcmurphy bf78d02
Restructure and introduce a standalone config
gcmurphy 50bbc53
Isolate import tracking functionality
gcmurphy 5160048
Move rule definitions into own file
gcmurphy 65b18da
Hack to address circular dependency in rulelist
gcmurphy 026fe4c
Simplify analyzer and command line interface
gcmurphy f4b705a
Use glide to manage vendored dependencies
gcmurphy 6943f9e
Major rework of codebase
gcmurphy 3caf7c3
Add test cases
gcmurphy 9c959ca
Issue.Line is already a string
lanzafame 5a11336
remove commited binary
lanzafame 27b2fd9
Merge pull request #136 from lanzafame/experimental
gcmurphy 67dc432
use godep instead of glide
gcmurphy d4311c9
make it clear that these tests have not been implemented yet
gcmurphy 02901b9
actually skip tests until implementation exists
gcmurphy e3b6fd9
update readme to provide info regarding package level scans
gcmurphy 97cde35
update travis-ci to use ginkgo tests
gcmurphy cfa4327
fix hound-ci errors
gcmurphy af25ac1
fix golint errors picked up by hound-ci
gcmurphy 25d74c6
address review comments
gcmurphy e925d3c
Migrated old test cases.
gcmurphy 4c49716
move utils to separate executable
gcmurphy d452dcb
Fix ginko invocation
gcmurphy 867d300
Fix lint issues
gcmurphy File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
commit 6943f9e5e4d2c5e7bf5af00d0e9fa90a82a8af3d
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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() { | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would add a test for a package consisting of multiple files. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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)) | ||
|
||
}) | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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)) | ||
|
||
}) | ||
|
||
}) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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