Repository: google/go-flow-levee
Pull request #33 (2020-07-21 00:10:35)

Consolidate fieldPropogatorMatcher and NameMatcher
* methodMatcher subsumes both, matching package, receiver, and method names.

------

I noticed that `NameMatcher.TypeRE` was unused -- its only use being in a function that was nowhere called.  

Judging by the dead `NameMatcher.matchNamedType` method, this was possibly due to a divergence in type between `fieldPropogatorMatcher` and `NameMatcher`.  I can see no reason for the underlying implementation of these to be kept separate, and so have unified them here.

This could also apply to the distinction between `FieldPropagator` and `TranformingPropagator`.  We should consider unifying both the implementation and the configuration of these to simply be `Propogator` in the future.

- [x] Tests pass
- [x] Appropriate changes to README are included in PR


Patch:
diff --git a/configuration/README.md b/configuration/README.md
index 3dd272e..d0ceb32 100644
--- a/configuration/README.md
+++ b/configuration/README.md
@@ -28,19 +28,21 @@ Sources are identified via regexp according to package, type, and field names.
 }
 ```
 
-Sinks and sanitizers are identified via regexp according to package and method name.
+Sinks, sanitizers, and some propagators (below) are identified via regexp according to package, method, and (optional) receiver name.
 
 ```json
 {
   "Sinks": [
     {
       "PackageRE": "<package path regexp>",
+      "ReceiverRE": "<type name regexp>",
       "MethodRE": "<method name regexp>"
     }
   ],
   "Sanitizers": [
     {
       "PackageRE": "<package path regexp>",
+      "ReceiverRE": "<type name regexp>",
       "MethodRE": "<method name regexp>"
     }
   ]
@@ -69,13 +71,15 @@ Argument propagators are identified via regexp matching the fully-qualified type
   "TransformingPropagators": [
     {
       "PackageRE": "<package path regexp>",
+      "ReceiverRE": "<type name regexp>",
       "MethodRE": "<method name regexp>"
     }
   ],
   "FieldPropagators": [
     {
-      "Receiver": "<fully qualified type path regexp>",
-      "AccessorRE": "<method name regexp>"
+      "PackageRE": "<package path regexp>",
+      "ReceiverRE": "<type name regexp>",
+      "MethodRE": "<method name regexp>"
     }
   ],
   "PropagatorArgs": {
@@ -84,6 +88,9 @@ Argument propagators are identified via regexp matching the fully-qualified type
 }
 ```
 
+For matchers that accept a `ReceiverRE` regexp matcher, an unspecified string will match any (or no) receiver.
+To match only methods without any receiver (i.e., a top-level function), use the matcher `^$` to match an empty-string receiver name.
+
 ### Example configuration
 
 The following configuration could be used to identify possible instances of credential logging in Kubernetes.
diff --git a/configuration/example-config.json b/configuration/example-config.json
index cc77745..5a2b883 100644
--- a/configuration/example-config.json
+++ b/configuration/example-config.json
@@ -24,6 +24,7 @@
   "Sinks": [
     {
       "PackageRE": "k?log$",
+      "ReceiverRE": "",
       "MethodRE": "Info|Warning|Error|Fatal|Exit"
     }
   ],
@@ -31,6 +32,7 @@
   "TransformingPropagators": [
     {
       "PackageName": ".*proto$",
+      "ReceiverRE": "",
       "MethodRE": "^Clone$|^Marshal"
     }
   ],
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index 87e080c..3cf131c 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -38,9 +38,9 @@ func init() {
 // config contains matchers and analysis scope information
 type Config struct {
 	Sources                 []sourceMatcher
-	Sinks                   []NameMatcher
-	Sanitizers              []NameMatcher
-	FieldPropagators        []fieldPropagatorMatcher
+	Sinks                   []callMatcher
+	Sanitizers              []callMatcher
+	FieldPropagators        []callMatcher
 	TransformingPropagators []transformingPropagatorMatcher
 	PropagatorArgs          argumentPropagatorMatcher
 	Whitelist               []packageMatcher
@@ -76,7 +76,7 @@ func (c Config) shouldSkip(pkg *types.Package) bool {
 
 func (c Config) IsSink(call *ssa.Call) bool {
 	for _, p := range c.Sinks {
-		if p.MatchMethodName(call) {
+		if p.Match(call) {
 			return true
 		}
 	}
@@ -86,7 +86,7 @@ func (c Config) IsSink(call *ssa.Call) bool {
 
 func (c Config) IsSanitizer(call *ssa.Call) bool {
 	for _, p := range c.Sanitizers {
-		if p.MatchMethodName(call) {
+		if p.Match(call) {
 			return true
 		}
 	}
@@ -152,7 +152,7 @@ func (c Config) isFieldPropagator(call *ssa.Call) bool {
 	}
 
 	for _, p := range c.FieldPropagators {
-		if p.match(call) {
+		if p.Match(call) {
 			return true
 		}
 	}
@@ -268,33 +268,36 @@ func (pm packageMatcher) match(pkg *types.Package) bool {
 	return pm.PackageNameRE.MatchString(pkg.Path())
 }
 
-type NameMatcher struct {
-	PackageRE regexp.Regexp
-	TypeRE    regexp.Regexp
-	MethodRE  regexp.Regexp
+type callMatcher struct {
+	PackageRE  regexp.Regexp
+	ReceiverRE regexp.Regexp
+	MethodRE   regexp.Regexp
 }
 
-func (r NameMatcher) matchPackage(p *types.Package) bool {
+func (r callMatcher) matchPackage(p *types.Package) bool {
 	return r.PackageRE.MatchString(p.Path())
 }
 
-func (r NameMatcher) MatchMethodName(c *ssa.Call) bool {
-	if c.Call.StaticCallee() == nil || c.Call.StaticCallee().Pkg == nil {
+// Match matches methods based on package, method, and receiver regexp.
+// To explicitly match a method with no receiver (i.e., a top-level function),
+// provide the ReceiverRE regexp `^$`.
+func (r callMatcher) Match(c *ssa.Call) bool {
+	callee := c.Call.StaticCallee()
+	if callee == nil || callee.Pkg == nil {
 		return false
 	}
 
-	return r.matchPackage(c.Call.StaticCallee().Pkg.Pkg) &&
-		r.MethodRE.MatchString(c.Call.StaticCallee().Name())
-}
-
-func (r NameMatcher) matchNamedType(n *types.Named) bool {
-	if types.IsInterface(n) {
-		// In our context, both sources and sanitizers are concrete types.
+	if !r.matchPackage(callee.Pkg.Pkg) || !r.MethodRE.MatchString(callee.Name()) {
 		return false
 	}
 
-	return r.PackageRE.MatchString(n.Obj().Pkg().Path()) &&
-		r.TypeRE.MatchString(n.Obj().Name())
+	recv := c.Call.Signature().Recv()
+	var recvName string
+	if recv != nil {
+		recvName = recv.Type().String()
+	}
+
+	return r.ReceiverRE.MatchString(recvName)
 }
 
 var readFileOnce sync.Once
diff --git a/internal/pkg/fieldtags/analyzer.go b/internal/pkg/fieldtags/analyzer.go
new file mode 100644
index 0000000..39b940c
--- /dev/null
+++ b/internal/pkg/fieldtags/analyzer.go
@@ -0,0 +1,121 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package fieldtags defines an analyzer that identifies struct fields identified
+// as sources via a field tag.
+package fieldtags
+
+import (
+	"go/ast"
+	"reflect"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+type keyValue struct {
+	key   string
+	value string
+}
+
+type sourcePatterns []keyValue
+
+var patterns sourcePatterns = []keyValue{
+	{"levee", "source"},
+}
+
+var Analyzer = &analysis.Analyzer{
+	Name: "fieldtags",
+	Doc:  "This analyzer identifies Source fields based on their tags. Tags are expected to satisfy the `go vet -structtag` format.",
+	Run:  run,
+	Requires: []*analysis.Analyzer{
+		inspect.Analyzer,
+	},
+	ResultType: reflect.TypeOf(new([]*ast.Field)).Elem(),
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspectResult := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	var results []*ast.Field
+
+	nodeFilter := []ast.Node{
+		(*ast.Field)(nil),
+	}
+
+	inspectResult.Preorder(nodeFilter, func(n ast.Node) {
+		field, ok := n.(*ast.Field)
+		if !ok {
+			return
+		}
+		if patterns.isSource(field) {
+			results = append(results, field)
+			pass.Reportf(field.Pos(), "tagged field")
+		}
+	})
+	return results, nil
+}
+
+func (sp *sourcePatterns) isSource(field *ast.Field) bool {
+	if field.Tag == nil {
+		return false
+	}
+
+	tag := field.Tag.Value
+
+	// TODO: consider refactoring this logic into a regex matcher
+	i := 1 // skip initial quote
+	j := 1
+	for j < len(tag) {
+		for j < len(tag) && tag[j] != ':' {
+			j++
+		}
+		key := tag[i:j]
+		if key == "" {
+			return false
+		}
+
+		i = j + 1 // skip colon
+		if tag[i] == '\\' {
+			i++
+		}
+		i++ // skip quote
+
+		j = i
+		for j < len(tag) && tag[j] != '"' {
+			// skip escape character
+			if tag[j] == '\\' {
+				j++
+			}
+			j++
+		}
+		value := tag[i:j]
+		if value == "" {
+			return false
+		}
+
+		for _, p := range *sp {
+			// value may be a list of comma separated values
+			if p.key == key && strings.Contains(value, p.value) {
+				return true
+			}
+		}
+
+		i = j + 2 // skip closing quote and space
+		j = i
+	}
+
+	return false
+}


Test patch:
diff --git a/internal/pkg/fieldtags/analyzer_test.go b/internal/pkg/fieldtags/analyzer_test.go
new file mode 100644
index 0000000..40abfec
--- /dev/null
+++ b/internal/pkg/fieldtags/analyzer_test.go
@@ -0,0 +1,27 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fieldtags
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func TestFieldTagsAnalysis(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	analysistest.Run(t, testdata, Analyzer, "tests")
+}
diff --git a/internal/pkg/fieldtags/testdata/src/tests/test.go b/internal/pkg/fieldtags/testdata/src/tests/test.go
new file mode 100644
index 0000000..7a44e12
--- /dev/null
+++ b/internal/pkg/fieldtags/testdata/src/tests/test.go
@@ -0,0 +1,23 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package fieldtags
+
+type Person struct {
+	password           string      `levee:"source"`               // want "tagged field"
+	secret             string      `json:"secret" levee:"source"` // want "tagged field"
+	another            interface{} "levee:\"source\""             // want "tagged field"
+	name               string      `some_key:"non_secret"`
+	someNotTaggedField int
+}
diff --git a/internal/testdata/test-config.json b/internal/testdata/test-config.json
index 101b32d..712f551 100644
--- a/internal/testdata/test-config.json
+++ b/internal/testdata/test-config.json
@@ -20,8 +20,9 @@
   ],
   "FieldPropagators": [
     {
-      "Receiver": "example.com/core.Source",
-      "AccessorRE": "^GetData$",
+      "PackageRE": "^example.com/core$",
+      "ReceiverRE": "Source",
+      "MethodRE": "^GetData$",
       "TODO": "Make this explicit configuration unnecessary."
     }
   ],