Repository: google/go-flow-levee
Pull request #182 (2020-11-11 00:45:43)

Allow configuration via regexp or string literals.
* Expand testing of matchers, now that the interface is reduced to basic types.
* Update docs.
* Unspecified elements continue to match vacuously.

- [x] Tests pass
- [x] Running against a large codebase such as [Kubernetes](https://github.com/kubernetes/kubernetes) does not error out.
- [x] Appropriate changes to README are included in PR


Patch:
diff --git a/configuration/README.md b/configuration/README.md
index 0061b7c..786c983 100644
--- a/configuration/README.md
+++ b/configuration/README.md
@@ -13,14 +13,18 @@ For design details concerning value and instruction classification, see [/design
 
 Configuration is provided to `go-flow-levee` via YAML.
 
-Objects of interest are identified primarily via regexp. An empty regexp will match any string.
+Sources are identified by package, type, and field names.
+You may specify these with a combination of string literals or regexp.
+Use `Package`, `Type`, and `Field` to specify by string literal.
+Use `PackageRE`, `TypeRE`, and `FieldRE` to specify by regexp.
+If neither a literal nor a regexp is provided, a wildcard matcher is assumed.
+Providing both a literal and a regexp matcher is considered a misconfiguration and will error.
 
-Sources are identified via regexp according to package, type, and field names.
 ```yaml
 Sources:
-- PackageRE: "<package path regexp>"
-  TypeRE: "<type name regexp>"
-  FieldRE: "<field name regexp>"
+- Package: "full/package/path"
+  # Neither Type nor TypeRE are specified - match all type names
+  FieldRE: "Token|Password|Secret" 
 ```
 
 Sources may also be identified via field tags:
@@ -37,35 +41,40 @@ FieldTags:
   Val: source
 ```
 
-Sinks and sanitizers are identified via regexp according to package, method, and (optional) receiver name.
+Sinks and sanitizers are identified by package, method, and (if applicable) receiver name.
+As with source configuration, these may be specified by either a provided string literal or regexp.
+Use `Package`, `Receiver`, and `Method` to specify by string literal.
+Use `PackageRE`, `ReceiverRE`, and `MethodRE` to specify by regexp.
+If neither a literal nor a regexp is provided, a wildcard matcher is assumed.
+Providing both a literal and a regexp matcher is considered a misconfiguration and will error.
 
 ```yaml
 Sinks:
-- PackageRE: <package path regexp>
-  ReceiverRE: <type name regexp>
-  MethodRE: <method name regexp>
+- PackageRE:  ".*/sinks(/v1)?"  # Regexp match a collection of packages 
+  # Neither Receiver nor ReceiverRE is provided - match any (or no) receiver
+  Method: "Sink"  # Match only functions named exactly "Sink"
 Sanitizers:
-- PackageRE: <package path regexp>
-  ReceiverRE: <type name regexp>
-  MethodRE: <method name regexp>
+- # Neither Package nor PackageRE is provided - match any package
+  ReceiverRE: "^Safe"  # Match any receiver beginning with "Safe"
+  Method: "sanitize"  # Match methods named exactly "sanitize"
 ```
 
-Taint propagation is performed automatically and does not need to be explicitly configured.
+To explicitly match an empty string, such as top-level functions without a receiver, explicitly configure an empty string matcher, e.g., `Receiver: ""`.
 
-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.
+Taint propagation is performed automatically and does not need to be explicitly configured.
 
 ### Restricting analysis scope
 
-Functions can be explicitly excluded from analysis using regexps,
+Functions can be explicitly excluded from analysis using string literals or regexps,
 constructed similarly to those used to identify sanitizers and sinks:
 ```yaml
 Exclude:
-- PathRE: "^myproject/mypackage$"
-  MethodRE: "^myfunction$"
+- Package: "myproject/mypackage"
+  MethodRE: "^my.*"
 ```
 
-The above will match the function `myfunction` from the `myproject/mypackage` package. It will also match a method named `myfunction` in the same package.
+The above will match any function beginning with "my" in the `myproject/mypackage` package.
+Since no receiver matcher was provided, it will match any method beginning with "my" bound to any (or no) receiver.
 
 As just two examples, this may be used to avoid analyzing test code, or to suppress "false positive" reports.
 
diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go
index e9305f1..5e045a5 100644
--- a/internal/pkg/config/config.go
+++ b/internal/pkg/config/config.go
@@ -15,6 +15,7 @@
 package config
 
 import (
+	"encoding/json"
 	"flag"
 	"fmt"
 	"io/ioutil"
@@ -120,31 +121,129 @@ func (c Config) IsSourceField(path, typeName, fieldName string) bool {
 	return false
 }
 
-// A sourceMatcher defines what types are or contain sources.
-// Within a given type, specific field access can be specified as the actual source data
-// via the fieldRE.
+type stringMatcher interface {
+	MatchString(string) bool
+}
+
+type literalMatcher string
+
+func (lm literalMatcher) MatchString(s string) bool {
+	return string(lm) == s
+}
+
+type vacuousMatcher struct{}
+
+func (vacuousMatcher) MatchString(string) bool {
+	return true
+}
+
+// Returns the first non-nil matcher.  If all are nil, returns a vacuousMatcher.
+func matcherFrom(lm *literalMatcher, r *regexp.Regexp) stringMatcher {
+	switch {
+	case lm != nil:
+		return lm
+	case r != nil:
+		return r
+	default:
+		return vacuousMatcher{}
+	}
+}
+
+// A sourceMatcher matches by package, type, and field.
+// Matching may be done against string literals Package, Type, Field,
+// or against regexp PackageRE, TypeRE, FieldRE.
 type sourceMatcher struct {
-	PackageRE regexp.Regexp
-	TypeRE    regexp.Regexp
-	FieldRE   regexp.Regexp
+	Package stringMatcher
+	Type    stringMatcher
+	Field   stringMatcher
+}
+
+// this type uses the default unmarshaler and mirrors configuration key-value pairs
+type rawSourceMatcher struct {
+	Package   *literalMatcher
+	Type      *literalMatcher
+	Field     *literalMatcher
+	PackageRE *regexp.Regexp
+	TypeRE    *regexp.Regexp
+	FieldRE   *regexp.Regexp
+}
+
+func (s *sourceMatcher) UnmarshalJSON(bytes []byte) error {
+	raw := rawSourceMatcher{}
+	if err := json.Unmarshal(bytes, &raw); err != nil {
+		return err
+	}
+
+	// validation: do not double-specify any attribute with literal and regexp
+	if raw.Package != nil && raw.PackageRE != nil {
+		return fmt.Errorf("expected only one of Package, PackageRE to be configured")
+	}
+	if raw.Type != nil && raw.TypeRE != nil {
+		return fmt.Errorf("expected only one of Type, TypeRE to be configured")
+	}
+	if raw.Field != nil && raw.FieldRE != nil {
+		return fmt.Errorf("expected only one of Field, FieldRE to be configured")
+	}
+
+	*s = sourceMatcher{
+		Package: matcherFrom(raw.Package, raw.PackageRE),
+		Type:    matcherFrom(raw.Type, raw.TypeRE),
+		Field:   matcherFrom(raw.Field, raw.FieldRE),
+	}
+	return nil
 }
 
 func (s sourceMatcher) MatchType(path, typeName string) bool {
-	return s.PackageRE.MatchString(path) && s.TypeRE.MatchString(typeName)
+	return s.Package.MatchString(path) && s.Type.MatchString(typeName)
 }
 
 func (s sourceMatcher) MatchField(path, typeName, fieldName string) bool {
-	return s.PackageRE.MatchString(path) && s.TypeRE.MatchString(typeName) && s.FieldRE.MatchString(fieldName)
+	return s.Package.MatchString(path) && s.Type.MatchString(typeName) && s.Field.MatchString(fieldName)
 }
 
 type funcMatcher struct {
-	PackageRE  regexp.Regexp
-	ReceiverRE regexp.Regexp
-	MethodRE   regexp.Regexp
+	Package  stringMatcher
+	Receiver stringMatcher
+	Method   stringMatcher
+}
+
+// this type uses the default unmarshaler and mirrors configuration key-value pairs
+type rawFuncMatcher struct {
+	Package    *literalMatcher
+	Receiver   *literalMatcher
+	Method     *literalMatcher
+	PackageRE  *regexp.Regexp
+	ReceiverRE *regexp.Regexp
+	MethodRE   *regexp.Regexp
+}
+
+func (fm *funcMatcher) UnmarshalJSON(bytes []byte) error {
+	raw := rawFuncMatcher{}
+	if err := json.Unmarshal(bytes, &raw); err != nil {
+		return err
+	}
+
+	// validation: do not double-specify any attribute with literal and regexp
+	if raw.Package != nil && raw.PackageRE != nil {
+		return fmt.Errorf("expected at most one of Package, PackageRE to be configured")
+	}
+	if raw.Receiver != nil && raw.ReceiverRE != nil {
+		return fmt.Errorf("expected at most one of Receiver, ReceiverRE to be configured")
+	}
+	if raw.Method != nil && raw.MethodRE != nil {
+		return fmt.Errorf("expected at most one of Method, MethodRE to be configured")
+	}
+
+	*fm = funcMatcher{
+		Package:  matcherFrom(raw.Package, raw.PackageRE),
+		Receiver: matcherFrom(raw.Receiver, raw.ReceiverRE),
+		Method:   matcherFrom(raw.Method, raw.MethodRE),
+	}
+	return nil
 }
 
 func (fm funcMatcher) MatchFunction(path, receiver, name string) bool {
-	return fm.PackageRE.MatchString(path) && fm.ReceiverRE.MatchString(receiver) && fm.MethodRE.MatchString(name)
+	return fm.Package.MatchString(path) && fm.Receiver.MatchString(receiver) && fm.Method.MatchString(name)
 }
 
 var readFileOnce sync.Once
diff --git a/internal/pkg/config/regexp/regexp.go b/internal/pkg/config/regexp/regexp.go
index 54a84a8..57252f6 100644
--- a/internal/pkg/config/regexp/regexp.go
+++ b/internal/pkg/config/regexp/regexp.go
@@ -26,6 +26,11 @@ type Regexp struct {
 	r *regexp.Regexp
 }
 
+func New(s string) (*Regexp, error) {
+	r, err := regexp.Compile(s)
+	return &Regexp{r: r}, err
+}
+
 // MatchString delegates matching to the regex package.
 func (mr *Regexp) MatchString(s string) bool {
 	return mr.r == nil || mr.r.MatchString(s)
diff --git a/internal/pkg/fieldpropagator/analyzer.go b/internal/pkg/fieldpropagator/analyzer.go
index 8904cb8..1dcf5a5 100644
--- a/internal/pkg/fieldpropagator/analyzer.go
+++ b/internal/pkg/fieldpropagator/analyzer.go
@@ -74,8 +74,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		}
 		methods := ssaProg.MethodSets.MethodSet(ssaType.Type())
 		for i := 0; i < methods.Len(); i++ {
-			meth := ssaProg.MethodValue(methods.At(i))
-			analyzeBlocks(pass, conf, taggedFields, meth)
+			if meth := ssaProg.MethodValue(methods.At(i)); meth != nil {
+				analyzeBlocks(pass, conf, taggedFields, meth)
+			}
 		}
 	}
 
diff --git a/internal/pkg/fieldtags/analyzer.go b/internal/pkg/fieldtags/analyzer.go
index 0b23d7c..bad0f7b 100644
--- a/internal/pkg/fieldtags/analyzer.go
+++ b/internal/pkg/fieldtags/analyzer.go
@@ -53,15 +53,11 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	taggedFields := map[types.Object]bool{}
 
 	nodeFilter := []ast.Node{
-		(*ast.TypeSpec)(nil),
+		(*ast.StructType)(nil),
 	}
 
 	inspectResult.Preorder(nodeFilter, func(n ast.Node) {
-		t, ok := n.(*ast.TypeSpec)
-		if !ok {
-			return
-		}
-		s, ok := t.Type.(*ast.StructType)
+		s, ok := n.(*ast.StructType)
 		if !ok || s.Fields == nil {
 			return
 		}
diff --git a/internal/pkg/sourceinfer/analyzer.go b/internal/pkg/sourceinfer/analyzer.go
index 9ace505..c20df47 100644
--- a/internal/pkg/sourceinfer/analyzer.go
+++ b/internal/pkg/sourceinfer/analyzer.go
@@ -115,94 +115,52 @@ func createObjectGraph(pass *analysis.Pass, ins *inspector.Inspector) objectGrap
 			return
 		}
 
-		typeBeingDefined := pass.TypesInfo.ObjectOf(ts.Name)
-
-		// Find selector expressions and add them to the graph.
-		// We need to look at SelectorExprs first, because they
-		// contain identifiers, e.g. foo.Bar contains the identifiers
-		// foo (qualifying package) and Bar (unqualified type).
-		// We need to look at the whole expression in order to resolve
-		// the type correctly.
-		selectorFinder := &selectorFinder{nil, map[*ast.Ident]bool{}}
-		ast.Walk(selectorFinder, (ast.Node)(ts.Type))
-		for _, sel := range selectorFinder.foundSelectors {
-			named, ok := pass.TypesInfo.TypeOf(sel).(*types.Named)
-			if !ok {
-				continue
-			}
-			obj := named.Obj()
-			objGraph[obj] = append(objGraph[obj], typeBeingDefined)
-		}
+		objectBeingDefined := pass.TypesInfo.ObjectOf(ts.Name)
 
-		// Find identifiers that aren't in selector expressions
-		// and add them to the graph.
-		idFinder := &identFinder{}
-		ast.Walk(idFinder, (ast.Node)(ts.Type))
-		for _, id := range idFinder.foundIdentifiers {
-			// identifier is part of a SelectorExpr and has already been handled
-			if selectorFinder.foundIdentifiers[id] {
-				continue
-			}
-			obj := pass.TypesInfo.ObjectOf(id)
-			objGraph[obj] = append(objGraph[obj], typeBeingDefined)
+		for o := range findObjects(pass.TypesInfo.TypeOf(ts.Type)) {
+			objGraph[o] = append(objGraph[o], objectBeingDefined)
 		}
 	})
 
 	return objGraph
 }
 
-type selectorFinder struct {
-	foundSelectors   []*ast.SelectorExpr
-	foundIdentifiers map[*ast.Ident]bool
-}
-
-func (sf *selectorFinder) Visit(n ast.Node) ast.Visitor {
-	sel, ok := n.(*ast.SelectorExpr)
-	if !ok {
-		return sf
-	}
-	pkg, ok := sel.X.(*ast.Ident)
-	if !ok {
-		return nil
-	}
-	sf.foundSelectors = append(sf.foundSelectors, sel)
-	sf.foundIdentifiers[pkg] = true
-	sf.foundIdentifiers[sel.Sel] = true
-	return nil
-}
-
-type identFinder struct {
-	foundIdentifiers []*ast.Ident
-}
-
-func (i *identFinder) Visit(n ast.Node) ast.Visitor {
-	if id, ok := n.(*ast.Ident); ok {
-		i.foundIdentifiers = append(i.foundIdentifiers, id)
-		return nil
-	}
-	return i
-}
-
-func findNamedTypes(t types.Type) map[*types.Named]bool {
-	namedTypes := map[*types.Named]bool{}
-
-	var find func(t types.Type)
-	find = func(t types.Type) {
+// findObjects traverses a type to find objects of interest within that type.
+// Objects of interest are objects that could be sources, either via configuration
+// or via inference. Specifically, this includes:
+// 1. objects related to named types,
+// 2. objects related to struct fields.
+// Some types whose objects cannot be sources are avoided.
+// In particular, functions (Signatures) are avoided because they can't be sources, but
+// their types can contain types that are tied to relevant objects, e.g. func(s Source) contains
+// a named type "Source" that would be relevant if it did not occur within a function type.
+func findObjects(t types.Type) map[types.Object]bool {
+	objects := map[types.Object]bool{}
+
+	var traverse func(t types.Type)
+	traverse = func(t types.Type) {
 		deref := utils.Dereference(t)
 		switch tt := deref.(type) {
 		case *types.Named:
-			namedTypes[tt] = true
+			objects[tt.Obj()] = true
 		case *types.Array:
-			find(tt.Elem())
+			traverse(tt.Elem())
 		case *types.Slice:
-			find(tt.Elem())
+			traverse(tt.Elem())
 		case *types.Chan:
-			find(tt.Elem())
+			traverse(tt.Elem())
 		case *types.Map:
-			find(tt.Key())
-			find(tt.Elem())
-		case *types.Basic, *types.Struct, *types.Tuple, *types.Interface, *types.Signature:
-			// these types cannot hold named types
+			traverse(tt.Key())
+			traverse(tt.Elem())
+		case *types.Struct:
+			for i := 0; i < tt.NumFields(); i++ {
+				f := tt.Field(i)
+				// The field itself could be a source, e.g. in the case of a tagged field.
+				objects[f] = true
+				traverse(f.Type())
+			}
+		case *types.Basic, *types.Tuple, *types.Interface, *types.Signature:
+			// these do not contain relevant objects
 		case *types.Pointer:
 			// this should be unreachable due to the dereference above
 		default:
@@ -211,9 +169,9 @@ func findNamedTypes(t types.Type) map[*types.Named]bool {
 		}
 	}
 
-	find(t)
+	traverse(t)
 
-	return namedTypes
+	return objects
 }
 
 func inferSources(pass *analysis.Pass, conf *config.Config, ft fieldtags.ResultType, objGraph objectGraph) ResultType {
@@ -284,8 +242,8 @@ func topoSort(graph objectGraph) []types.Object {
 }
 
 func isSourceType(c *config.Config, t types.Type) bool {
-	for nt := range findNamedTypes(t) {
-		if c.IsSourceType(utils.DecomposeType(nt)) {
+	for o := range findObjects(t) {
+		if c.IsSourceType(utils.DecomposeType(o.Type())) {
 			return true
 		}
 	}


Test patch:
diff --git a/internal/pkg/config/matcher_test.go b/internal/pkg/config/matcher_test.go
new file mode 100644
index 0000000..38d91e4
--- /dev/null
+++ b/internal/pkg/config/matcher_test.go
@@ -0,0 +1,369 @@
+// 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 config
+
+import (
+	"testing"
+
+	"github.com/google/go-flow-levee/internal/pkg/config/regexp"
+	"sigs.k8s.io/yaml"
+)
+
+func TestFuncMatcherUnmarshaling(t *testing.T) {
+	testCases := []struct {
+		desc, yaml        string
+		shouldErrorOnLoad bool
+	}{
+		{
+			desc: "Unmarshaling is strict",
+			yaml: `
+Blahblah: foo
+PackageRE: bar`,
+			shouldErrorOnLoad: false, // TODO true
+		},
+		{
+			desc: "Malformed YAML errors gracefully",
+			yaml: `
+PackageRE: "No ending quote`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Malformed regexp errors gracefully",
+			yaml: `
+PackageRE: "(?:NoEndingParen"`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Do not permit both Package and PackageRE",
+			yaml: `
+Package: foo
+PackageRE: bar`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Do not permit both Receiver and ReceiverRE",
+			yaml: `
+Receiver: foo
+ReceiverRE: bar`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Do not permit both Field and FieldRE",
+			yaml: `
+Method: foo
+MethodRE: bar`,
+			shouldErrorOnLoad: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			fm := funcMatcher{}
+			err := yaml.UnmarshalStrict([]byte(tc.yaml), &fm)
+
+			if (err != nil) != tc.shouldErrorOnLoad {
+				t.Errorf("error expectation = %v, but got err=%v", tc.shouldErrorOnLoad, err)
+			}
+		})
+	}
+}
+
+func TestFuncMatcherMatching(t *testing.T) {
+	testCases := []struct {
+		desc, yaml       string
+		path, recv, name string
+		shouldMatch      bool
+	}{
+		{
+			desc: "Literal foo.bar should match foo.bar; no receiver arg",
+			yaml: `
+Package: foo
+Method: bar`,
+			path:        "foo",
+			recv:        "",
+			name:        "bar",
+			shouldMatch: true,
+		},
+		{
+			desc: "Literal foo.bar should match foo.(baz).bar; no receiver arg",
+			yaml: `
+Package: foo
+Method: bar`,
+			path:        "foo",
+			recv:        "baz",
+			name:        "bar",
+			shouldMatch: true,
+		},
+		{
+			desc: "Literal foo.bar should NOT match foodstuff.bar; no receiver arg",
+			yaml: `
+Package: foo
+Method: bar`,
+			path:        "foodstuff",
+			recv:        "",
+			name:        "bar",
+			shouldMatch: false,
+		},
+		{
+			desc: "Mixed regexp and literal matchers are permitted - positive case",
+			yaml: `
+PackageRE: foo
+Receiver: baz
+Method: bar`,
+			path:        "foodstuff",
+			recv:        "baz",
+			name:        "bar",
+			shouldMatch: true,
+		},
+		{
+			desc: "Mixed regexp and literal matchers are permitted - negative case",
+			yaml: `
+PackageRE: foo
+Receiver: baz
+Method: bar`,
+			path:        "foodstuff",
+			recv:        "bazinga",
+			name:        "bar",
+			shouldMatch: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			fm := funcMatcher{}
+			if err := yaml.UnmarshalStrict([]byte(tc.yaml), &fm); err != nil {
+				t.Errorf("Unexpected error unmarshalling funcMatcher: %v", err)
+			}
+
+			if tc.shouldMatch != fm.MatchFunction(tc.path, tc.recv, tc.name) {
+				t.Errorf("MatchFunction(%q, %q, %q) = %v; got %v", tc.path, tc.recv, tc.name, tc.shouldMatch, !tc.shouldMatch)
+			}
+		})
+	}
+}
+
+func TestSourceMatcherUnmarshaling(t *testing.T) {
+	testCases := []struct {
+		desc, yaml        string
+		shouldErrorOnLoad bool
+	}{
+		{
+			desc: "Unmarshaling is strict",
+			yaml: `
+Blahblah: foo
+PackageRE: bar`,
+			shouldErrorOnLoad: false, // TODO true
+		},
+		{
+			desc: "Malformed YAML errors gracefully",
+			yaml: `
+PackageRE: "No ending quote`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Malformed regexp errors gracefully",
+			yaml: `
+PackageRE: "(?:NoEndingParen"`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Do not permit both Package and PackageRE",
+			yaml: `
+Package: foo
+PackageRE: bar`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Do not permit both Type and TypeRE",
+			yaml: `
+Type: foo
+TypeRE: bar`,
+			shouldErrorOnLoad: true,
+		},
+		{
+			desc: "Do not permit both Field and FieldRE",
+			yaml: `
+Field: foo
+FieldRE: bar`,
+			shouldErrorOnLoad: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			sm := sourceMatcher{}
+			err := yaml.UnmarshalStrict([]byte(tc.yaml), &sm)
+
+			if (err != nil) != tc.shouldErrorOnLoad {
+				t.Errorf("error expectation = %v, but got err=%v", tc.shouldErrorOnLoad, err)
+			}
+		})
+	}
+}
+
+func TestSourceMatcherMatching(t *testing.T) {
+	testCases := []struct {
+		desc, yaml           string
+		path, typ, fieldName string
+		shouldMatchType      bool
+		shouldMatchField     bool
+	}{
+		{
+			desc: "Literal foo.bar (no field args) should match foo.bar and foo.bar.baz",
+			yaml: `
+Package: foo
+Type: bar`,
+			path:             "foo",
+			typ:              "bar",
+			fieldName:        "baz",
+			shouldMatchType:  true,
+			shouldMatchField: true,
+		},
+		{
+			desc: "Literal foo.bar (no field args) should NOT match foodstuff.bar or foodstuff.bar.baz",
+			yaml: `
+Package: foo
+Type: bar`,
+			path:             "foodstuff",
+			typ:              "bar",
+			fieldName:        "baz",
+			shouldMatchType:  false,
+			shouldMatchField: false,
+		},
+		{
+			desc: "Regexp foo.bar (no field args) should match foodstuff.bar and foodstuff.bar.baz",
+			yaml: `
+PackageRE: foo
+TypeRE: bar`,
+			path:             "foodstuff",
+			typ:              "bar",
+			fieldName:        "baz",
+			shouldMatchType:  true,
+			shouldMatchField: true,
+		},
+		{
+			desc: "Mixed regexp and literal matchers are permitted",
+			yaml: `
+PackageRE: foo
+Type: bar
+Field: baz`,
+			path:             "foodstuff",
+			typ:              "bar",
+			fieldName:        "baz",
+			shouldMatchType:  true,
+			shouldMatchField: true,
+		},
+		{
+			desc: "Literal foo.bar.qux should MatchType foo.bar but not MatchField foo.bar.baz",
+			yaml: `
+PackageRE: foo
+Type: bar
+Field: qux`,
+			path:             "foodstuff",
+			typ:              "bar",
+			fieldName:        "baz",
+			shouldMatchType:  true,
+			shouldMatchField: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			sm := sourceMatcher{}
+			if err := yaml.UnmarshalStrict([]byte(tc.yaml), &sm); err != nil {
+				t.Errorf("Unexpected error unmarshalling sourceMatcher: %v", err)
+			}
+
+			if tc.shouldMatchType != sm.MatchType(tc.path, tc.typ) {
+				t.Errorf("MatchType(%q, %q) = %v; got %v", tc.path, tc.typ, tc.shouldMatchType, !tc.shouldMatchType)
+			}
+			if tc.shouldMatchField != sm.MatchField(tc.path, tc.typ, tc.fieldName) {
+				t.Errorf("MatchField(%q, %q, %q) = %v; got %v", tc.path, tc.typ, tc.fieldName, tc.shouldMatchType, !tc.shouldMatchType)
+			}
+		})
+	}
+}
+
+func TestMatcherTypes(t *testing.T) {
+	testCases := []struct {
+		desc        string
+		matcher     stringMatcher
+		s           string
+		shouldMatch bool
+	}{
+		{
+			desc:        "literal matcher foo == foo",
+			matcher:     literalMatcher("foo"),
+			s:           "foo",
+			shouldMatch: true,
+		},
+		{
+			desc:        "literal matcher foo != food",
+			matcher:     literalMatcher("foo"),
+			s:           "food",
+			shouldMatch: false,
+		},
+		{
+			desc:        "literal matcher foo != bar",
+			matcher:     literalMatcher("foo"),
+			s:           "bar",
+			shouldMatch: false,
+		},
+		{
+			desc:        "regexp matcher /foo/ matches foo",
+			matcher:     func() stringMatcher { r, _ := regexp.New("foo"); return r }(),
+			s:           "foo",
+			shouldMatch: true,
+		},
+		{
+			desc:        "regexp matcher /foo/ matches food",
+			matcher:     func() stringMatcher { r, _ := regexp.New("foo"); return r }(),
+			s:           "food",
+			shouldMatch: true,
+		},
+		{
+			desc:        "regexp matcher /foo/ does not match bar",
+			matcher:     func() stringMatcher { r, _ := regexp.New("foo"); return r }(),
+			s:           "bar",
+			shouldMatch: false,
+		},
+		{
+			desc:        "vacuous matcher matches foo",
+			matcher:     vacuousMatcher{},
+			s:           "foo",
+			shouldMatch: true,
+		},
+		{
+			desc:        "vacuous matcher matches food",
+			matcher:     vacuousMatcher{},
+			s:           "food",
+			shouldMatch: true,
+		},
+		{
+			desc:        "vacuous matcher matches bar",
+			matcher:     vacuousMatcher{},
+			s:           "bar",
+			shouldMatch: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			if tc.matcher.MatchString(tc.s) != tc.shouldMatch {
+				t.Errorf("Expected matcher (%T) %v to return MatchString(%q) == %v, got %v", tc.matcher, tc.matcher, tc.s, tc.shouldMatch, !tc.shouldMatch)
+			}
+		})
+	}
+}
diff --git a/internal/pkg/fieldtags/analyzer_test.go b/internal/pkg/fieldtags/analyzer_test.go
index 262ac5e..e70bc06 100644
--- a/internal/pkg/fieldtags/analyzer_test.go
+++ b/internal/pkg/fieldtags/analyzer_test.go
@@ -44,6 +44,7 @@ func TestFieldTagsAnalysis(t *testing.T) {
 		"another",
 		"hasCustomFieldTag",
 		"hasTagWithMultipleValues",
+		"adminSecret",
 	}
 
 	var got []string
diff --git a/internal/pkg/fieldtags/testdata/src/tests/test.go b/internal/pkg/fieldtags/testdata/src/tests/test.go
index 511ff3f..4e18ccc 100644
--- a/internal/pkg/fieldtags/testdata/src/tests/test.go
+++ b/internal/pkg/fieldtags/testdata/src/tests/test.go
@@ -24,3 +24,9 @@ type Person struct {
 	spaceAfterFinalQuote     string      `key:"value" `
 	someNotTaggedField       int
 }
+
+type Nester struct {
+	Nested struct {
+		adminSecret string `levee:"source"` // want "tagged field: adminSecret"
+	}
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/collections/arrays.go b/internal/pkg/levee/testdata/src/example.com/tests/collections/arrays.go
index 9168132..eb0fbf0 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/collections/arrays.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/collections/arrays.go
@@ -40,3 +40,12 @@ func TestArrayRemainsTaintedWhenSourceIsOverwritten(s core.Source) {
 	arr[0] = nil
 	core.Sink(arr) // want "a source has reached a sink"
 }
+
+func TestRangeOverArray() {
+	sources := [1]core.Source{core.Source{Data: "password1234"}}
+	for i, s := range sources {
+		core.Sink(s) // want "a source has reached a sink"
+		// TODO want no diagnostic reported for string value
+		core.Sink(i) // want "a source has reached a sink"
+	}
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/collections/chans.go b/internal/pkg/levee/testdata/src/example.com/tests/collections/chans.go
index 752cafa..d7d87a2 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/collections/chans.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/collections/chans.go
@@ -39,3 +39,9 @@ func TestChannelIsNoLongerTaintedWhenNilledOut(sources chan core.Source) {
 	sources = nil
 	core.Sink(sources)
 }
+
+func TestRangeOverChan(sources chan core.Source) {
+	for s := range sources {
+		core.Sink(s) // want "a source has reached a sink"
+	}
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go b/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go
index f2e32c7..b3a2de4 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/collections/maps.go
@@ -52,3 +52,39 @@ func TestMapRemainsTaintedWhenSourceIsDeleted(s core.Source) {
 	delete(m, s)
 	core.Sink(m) // want "a source has reached a sink"
 }
+
+func TestDeletingFromTaintedMapDoesNotTaintTheKey(key string, sources map[string]core.Source) {
+	delete(sources, key)
+	// TODO: no report should be produced here
+	core.Sink(key) // want "a source has reached a sink"
+}
+
+func TestMapUpdateWithTaintedValueDoesNotTaintTheKey(key string, value core.Source, sources map[string]core.Source) {
+	sources[key] = value
+	// TODO: no report should be produced here
+	core.Sink(key) // want "a source has reached a sink"
+}
+
+func TestMapUpdateWithTaintedKeyDoesNotTaintTheValue(key core.Source, value string, sources map[core.Source]string) {
+	sources[key] = value
+	// TODO: no report should be produced here
+	core.Sink(value) // want "a source has reached a sink"
+}
+
+func TestRangeOverMapWithSourceAsValue() {
+	m := map[string]core.Source{"secret": core.Source{Data: "password1234"}}
+	for k, s := range m {
+		core.Sink(s) // want "a source has reached a sink"
+		// TODO want no diagnostic reported for string key
+		core.Sink(k) // want "a source has reached a sink"
+	}
+}
+
+func TestRangeOverMapWithSourceAsKey() {
+	m := map[core.Source]string{core.Source{Data: "password1234"}: "don't sink me"}
+	for src, str := range m {
+		core.Sink(src) // want "a source has reached a sink"
+		// TODO want no diagnostic reported for string value
+		core.Sink(str) // want "a source has reached a sink"
+	}
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/collections/slices.go b/internal/pkg/levee/testdata/src/example.com/tests/collections/slices.go
index 859996e..ec63ebf 100644
--- a/internal/pkg/levee/testdata/src/example.com/tests/collections/slices.go
+++ b/internal/pkg/levee/testdata/src/example.com/tests/collections/slices.go
@@ -26,3 +26,20 @@ func TestSlices(s core.Source) {
 	core.Sink([]interface{}{0, "", s})    // want "a source has reached a sink"
 	core.Sink([]interface{}{0, "", s}...) // want "a source has reached a sink"
 }
+
+func TestRangeOverSlice() {
+	sources := []core.Source{core.Source{Data: "password1234"}}
+	for i, s := range sources {
+		core.Sink(s) // want "a source has reached a sink"
+		// TODO want no diagnostic reported for string value
+		core.Sink(i) // want "a source has reached a sink"
+	}
+}
+
+func TestRangeOverInterfaceSlice() {
+	for i, s := range []interface{}{core.Source{Data: "password1235"}} {
+		core.Sink(s) // want "a source has reached a sink"
+		// TODO want no diagnostic reported for string value
+		core.Sink(i) // want "a source has reached a sink"
+	}
+}
diff --git a/internal/pkg/levee/testdata/src/example.com/tests/phi/tests.go b/internal/pkg/levee/testdata/src/example.com/tests/phi/tests.go
new file mode 100644
index 0000000..bf7a38f
--- /dev/null
+++ b/internal/pkg/levee/testdata/src/example.com/tests/phi/tests.go
@@ -0,0 +1,44 @@
+// 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 phi
+
+import (
+	"example.com/core"
+)
+
+func TestPhiNodeDoesntPropagateTaintToOperands(i *core.Innocuous) {
+	for {
+		s := core.Source{}
+		var ss, ii interface{} = s, i
+		if true {
+			ss = ii
+		}
+		// TODO: no report should be produced for ii
+		core.Sink(ii) // want "a source has reached a sink"
+		// TODO: no report should be produced for i
+		core.Sink(i)  // want "a source has reached a sink"
+		core.Sink(ss) // want "a source has reached a sink"
+	}
+}
+
+func TestPhiNodeIsTaintedByTaintedOperand(s core.Source, i *core.Innocuous) {
+	var ss, ii interface{} = s, i
+	if true {
+		ii = ss
+	}
+	core.Sink(ss) // want "a source has reached a sink"
+	core.Sink(ii) // want "a source has reached a sink"
+	core.Sink(i)
+}
diff --git a/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/field.go b/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/field.go
index 116dfeb..e57231c 100644
--- a/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/field.go
+++ b/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/field.go
@@ -48,6 +48,18 @@ type (
 		sh SourceHolder
 	}
 
+	SourceWrapperHolder struct { // want SourceWrapperHolder:"inferred source"
+		Wrapped struct {
+			s source.Source
+		}
+	}
+
+	TaggedWrapperHolder struct { // want TaggedWrapperHolder:"inferred source"
+		Wrapped struct {
+			s string `levee:"source"`
+		}
+	}
+
 	TaggedHolder struct { // want TaggedHolder:"inferred source"
 		t source.Tagged
 	}
@@ -57,6 +69,14 @@ type (
 	NotSourceHolder struct {
 		ns source.NotSource
 	}
+
+	FieldFuncWithSourceArg struct {
+		f func(s source.Source)
+	}
+
+	FieldFuncWithSourceRetval struct {
+		f func() source.Source
+	}
 )
 
 func Field() {
diff --git a/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/typedef.go b/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/typedef.go
index 3417990..93204aa 100644
--- a/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/typedef.go
+++ b/internal/pkg/sourceinfer/testdata/src/example.com/tests/core/typedef.go
@@ -30,7 +30,8 @@ type (
 )
 
 type (
-	NotDefinedSource source.NotSource
+	NotDefinedSource       source.NotSource
+	FunctionsAreNotSources func() map[string]source.Source
 )
 
 func Typedef() {