Repository: google/go-flow-levee
Pull request #1 (2020-05-05 22:02:23)

Add some top-level testing.
- [x] Tests pass
- [n/a] Appropriate changes to README are included in PR

This PR introduces a smattering of top-level tests for the analyzer, while identifying some gaps in coverage.

@immutablet



Patch:
diff --git a/internal/levee.go b/internal/levee.go
index 2648863..62d9ec7 100644
--- a/internal/levee.go
+++ b/internal/levee.go
@@ -564,10 +564,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
 						if v.Call.Signature().Variadic() && len(v.Call.Args) > 0 {
 							lastArg := v.Call.Args[len(v.Call.Args)-1]
 							if varargs, ok := lastArg.(*ssa.Slice); ok {
-								sinkVarargs := newVarargs(varargs, sources)
-								for _, s := range sinkVarargs.sources {
-									if !s.isSanitizedAt(v) {
-										report(pass, s, v)
+								if sinkVarargs := newVarargs(varargs, sources); sinkVarargs != nil {
+									for _, s := range sinkVarargs.sources {
+										if !s.isSanitizedAt(v) {
+											report(pass, s, v)
+										}
 									}
 								}
 							}


Test patch:
diff --git a/internal/levee_test.go b/internal/levee_test.go
index b165566..1f5b9f0 100644
--- a/internal/levee_test.go
+++ b/internal/levee_test.go
@@ -14,4 +14,25 @@
 
 package internal
 
-// TODO Some tests should exist.
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+var patterns = []string{
+	"example.com/tests/arguments",
+	"example.com/tests/declarations",
+	"example.com/tests/dominance",
+	"example.com/tests/fields",
+	"example.com/tests/receivers",
+	"example.com/tests/sinks",
+}
+
+func TestLevee(t *testing.T) {
+	dir := analysistest.TestData()
+	if err := Analyzer.Flags.Set("config", dir+"/test-config.json"); err != nil {
+		t.Error(err)
+	}
+	analysistest.Run(t, dir, Analyzer, patterns...)
+}
diff --git a/internal/testdata/src/example.com/core/sanitize.go b/internal/testdata/src/example.com/core/sanitize.go
new file mode 100644
index 0000000..8dc1135
--- /dev/null
+++ b/internal/testdata/src/example.com/core/sanitize.go
@@ -0,0 +1,9 @@
+package core
+
+func Sanitize(args ...interface{}) []interface{} {
+	return args
+}
+
+func SanitizeSource(s Source) Source {
+	return Source{ID: s.ID}
+}
diff --git a/internal/testdata/src/example.com/core/sink.go b/internal/testdata/src/example.com/core/sink.go
new file mode 100644
index 0000000..d9b5f7a
--- /dev/null
+++ b/internal/testdata/src/example.com/core/sink.go
@@ -0,0 +1,11 @@
+package core
+
+import "io"
+
+func Sink(args ...interface{}) {}
+
+func Sinkf(format string, args ...interface{}) {}
+
+func FSinkf(writer io.Writer, args ...interface{}) {}
+
+func OneArgSink(interface{}) {}
diff --git a/internal/testdata/src/example.com/core/source.go b/internal/testdata/src/example.com/core/source.go
new file mode 100644
index 0000000..228c559
--- /dev/null
+++ b/internal/testdata/src/example.com/core/source.go
@@ -0,0 +1,29 @@
+package core
+
+// Source will be configured to be detected as a source struct, with Source.Data as the source field.
+type Source struct {
+	Data string
+	ID   int
+}
+
+func (s Source) GetID() int {
+	return s.ID
+}
+
+func (s Source) GetData() string {
+	return s.Data
+}
+
+// Innocuous will _not_ be configured to be a source, even though underlying types are equal.
+type Innocuous struct {
+	Data string
+	ID   int
+}
+
+func (i Innocuous) GetID() int {
+	return i.ID
+}
+
+func (i Innocuous) GetData() string {
+	return i.Data
+}
diff --git a/internal/testdata/src/example.com/tests/arguments/tests.go b/internal/testdata/src/example.com/tests/arguments/tests.go
new file mode 100644
index 0000000..b49de43
--- /dev/null
+++ b/internal/testdata/src/example.com/tests/arguments/tests.go
@@ -0,0 +1,31 @@
+package arguments
+
+import (
+	"example.com/core"
+)
+
+func TestSourceFromParamByReference(s *core.Source) {
+	core.Sink("Source in the parameter %v", s) // want "a source has reached a sink"
+}
+
+func TestSourceMethodFromParamByReference(s *core.Source) {
+	core.Sink("Source in the parameter %v", s.Data) // want "a source has reached a sink"
+}
+
+func TestSourceFromParamByReferenceInfo(s *core.Source) {
+	core.Sink(s) // want "a source has reached a sink"
+}
+
+func TestSourceFromParamByValue(s core.Source) {
+	core.Sink("Source in the parameter %v", s) // want "a source has reached a sink"
+}
+
+func TestUpdatedSource(s *core.Source) {
+	s.Data = "updated"
+	core.Sink("Updated %v", s) // want "a source has reached a sink"
+}
+
+func TestSourceFromAPointerCopy(s *core.Source) {
+	cp := s
+	core.Sink("Pointer copy of the source %v", cp) // want "a source has reached a sink"
+}
diff --git a/internal/testdata/src/example.com/tests/declarations/tests.go b/internal/testdata/src/example.com/tests/declarations/tests.go
new file mode 100644
index 0000000..2fd044e
--- /dev/null
+++ b/internal/testdata/src/example.com/tests/declarations/tests.go
@@ -0,0 +1,21 @@
+// Package declarations contains test-cases for testing PII leak detection when sources are introduced via declarations.
+package declarations
+
+import (
+	"example.com/core"
+)
+
+func TestSourceDeclaredInBody() {
+	s := &core.Source{}
+	core.Sinkf("%v", s) // want "a source has reached a sink"
+
+	i := &core.Innocuous{}
+	core.Sinkf("%v", i)
+}
+
+func TestSourceViaClosure() func() {
+	s := &core.Source{}
+	return func() {
+		core.Sinkf("%v", s) // want "a source has reached a sink"
+	}
+}
diff --git a/internal/testdata/src/example.com/tests/dominance/tests.go b/internal/testdata/src/example.com/tests/dominance/tests.go
new file mode 100644
index 0000000..f24b3d1
--- /dev/null
+++ b/internal/testdata/src/example.com/tests/dominance/tests.go
@@ -0,0 +1,32 @@
+// Package dominance contains test-cases for testing PII sanitization.
+package dominance
+
+import (
+	"time"
+
+	"example.com/core"
+)
+
+func TestSanitizedSourceDoesNotTriggerFinding(c *core.Source) {
+	sanitized := core.Sanitize(c)
+	core.Sinkf("Sanitized %v", sanitized)
+}
+
+func TestSanitizedSourceDoesNotTriggerFindingWhenTypeAsserted(c *core.Source) {
+	sanitized := core.Sanitize(c)[0].(*core.Source)
+	core.Sinkf("Sanitized %v", sanitized)
+}
+
+func TestSanitizedSourceDoesNotTriggerFindingWithTypedSanitizer(c core.Source) {
+	sanitized := core.SanitizeSource(c)
+	// TODO This should not trigger
+	core.Sinkf("Sanitized %v", sanitized)  // want "a source has reached a sink,"
+}
+
+func TestNotGuaranteedSanitization(c *core.Source) {
+	p := c
+	if time.Now().Weekday() == time.Monday {
+		p = core.Sanitize(c)[0].(*core.Source)
+	}
+	core.Sinkf("Sometimes sanitized: %v", p) // want "a source has reached a sink"
+}
diff --git a/internal/testdata/src/example.com/tests/fields/tests.go b/internal/testdata/src/example.com/tests/fields/tests.go
new file mode 100644
index 0000000..7289b86
--- /dev/null
+++ b/internal/testdata/src/example.com/tests/fields/tests.go
@@ -0,0 +1,33 @@
+// Package fields contains tests related to fields accessors
+package fields
+
+import (
+	"example.com/core"
+)
+
+func TestFieldAccessors(s core.Source, ptr *core.Source) {
+	core.Sinkf("Data: %v", s.GetData()) // want "a source has reached a sink"
+	core.Sinkf("ID: %v", s.GetID())
+
+	core.Sinkf("Data: %v", ptr.GetData()) // want "a source has reached a sink"
+	core.Sinkf("ID: %v", ptr.GetID())
+}
+
+func TestDirectFieldAccess(c *core.Source) {
+	core.Sinkf("Data: %v", c.Data) // want "a source has reached a sink"
+	core.Sinkf("ID: %v", c.ID)
+}
+
+func TestProtoStyleFieldAccessorSanitizedPII(c *core.Source) {
+	core.Sinkf("Source data: %v", core.Sanitize(c.GetData()))
+}
+
+func TestProtoStyleFieldAccessorPIISecondLevel(wrapper struct{ *core.Source }) {
+	core.Sinkf("Source data: %v", wrapper.Source.GetData()) // want "a source has reached a sink"
+	core.Sinkf("Source id: %v", wrapper.Source.GetID())
+}
+
+func tesDirectFieldAccessorPIISecondLevel(wrapper struct{ *core.Source }) {
+	core.Sinkf("Source data: %v", wrapper.Source.Data) // want "a source has reached a sink"
+	core.Sinkf("Source id: %v", wrapper.Source.ID)
+}
diff --git a/internal/testdata/src/example.com/tests/receivers/tests.go b/internal/testdata/src/example.com/tests/receivers/tests.go
new file mode 100644
index 0000000..8d4b0b4
--- /dev/null
+++ b/internal/testdata/src/example.com/tests/receivers/tests.go
@@ -0,0 +1,21 @@
+// Package receivers contains test-cases for testing PII leak detection when sources are introduced via receivers.
+package receivers
+
+import (
+	"example.com/core"
+)
+
+type sourceBuilder struct {
+	sourcePtr *core.Source
+	sourceVal core.Source
+}
+
+func (b *sourceBuilder) buildP() {
+	core.Sinkf("Building cluster %v", b.sourcePtr) // want "a source has reached a sink"
+	core.Sinkf("Building cluster %v", b.sourceVal) // want "a source has reached a sink"
+}
+
+func (b sourceBuilder) buildV() {
+	core.Sinkf("Building cluster %v", b.sourcePtr) // want "a source has reached a sink"
+	core.Sinkf("Building cluster %v", b.sourceVal) // want "a source has reached a sink"
+}
diff --git a/internal/testdata/src/example.com/tests/sinks/tests.go b/internal/testdata/src/example.com/tests/sinks/tests.go
new file mode 100644
index 0000000..8bbc686
--- /dev/null
+++ b/internal/testdata/src/example.com/tests/sinks/tests.go
@@ -0,0 +1,47 @@
+package sinks
+
+import (
+	"io"
+
+	"example.com/core"
+)
+
+func TestSinks(s core.Source, writer io.Writer) {
+	core.Sink(s)                  // want "a source has reached a sink"
+	core.Sinkf("a source: %v", s) // want "a source has reached a sink"
+	core.FSinkf(writer, s)        // want "a source has reached a sink"
+	core.OneArgSink(s)            // TODO want "a source has reached a sink"
+
+	core.Sink([]interface{}{s, s, s}...) // TODO want "a source has reached a sink"
+	core.Sink([]interface{}{s, s, s})    // TODO want "a source has reached a sink"
+}
+
+func TestSinksWithRef(s *core.Source, writer io.Writer) {
+	core.Sink(s)                  // want "a source has reached a sink"
+	core.Sinkf("a source: %v", s) // want "a source has reached a sink"
+	core.FSinkf(writer, s)        // want "a source has reached a sink"
+	core.OneArgSink(s)            // TODO want "a source has reached a sink"
+
+	core.Sink([]interface{}{s, s, s}...) // TODO want "a source has reached a sink"
+	core.Sink([]interface{}{s, s, s})    // TODO want "a source has reached a sink"
+}
+
+func TestSinksInnocuous(innoc core.Innocuous, writer io.Writer) {
+	core.Sink(innoc)
+	core.Sinkf("a source: %v", innoc)
+	core.FSinkf(writer, innoc)
+	core.OneArgSink(innoc)
+
+	core.Sink([]interface{}{innoc, innoc, innoc}...)
+	core.Sink([]interface{}{innoc, innoc, innoc})
+}
+
+func TestSinksWithInnocuousRef(innoc *core.Innocuous, writer io.Writer) {
+	core.Sink(innoc)
+	core.Sinkf("a source: %v", innoc)
+	core.FSinkf(writer, innoc)
+	core.OneArgSink(innoc)
+
+	core.Sink([]interface{}{innoc, innoc, innoc}...)
+	core.Sink([]interface{}{innoc, innoc, innoc})
+}
diff --git a/internal/testdata/test-config.json b/internal/testdata/test-config.json
new file mode 100644
index 0000000..101b32d
--- /dev/null
+++ b/internal/testdata/test-config.json
@@ -0,0 +1,33 @@
+{
+  "Sources": [
+    {
+      "PackageRE": "^example.com/core$",
+      "TypeRE": "^Source$",
+      "FieldRE": "^Data"
+    }
+  ],
+  "Sinks": [
+    {
+      "PackageRE": "^example.com/core$",
+      "MethodRE": "Sink"
+    }
+  ],
+  "Sanitizers": [
+    {
+      "PackageRE": "^example.com/core$",
+      "MethodRE": "^Sanitize"
+    }
+  ],
+  "FieldPropagators": [
+    {
+      "Receiver": "example.com/core.Source",
+      "AccessorRE": "^GetData$",
+      "TODO": "Make this explicit configuration unnecessary."
+    }
+  ],
+  "TransformingPropagators": [
+  ],
+  "PropagatorArgs": {},
+  "Whitelist": [],
+  "AnalysisScope": []
+}
\ No newline at end of file