client: check that GetByModule always calls Index

Also check that if http request was made, then the module path must be
in the index.

Change-Id: I31d32c6c6c341586eefde0af03e6903d4196d368
Reviewed-on: https://go-review.googlesource.com/c/vuln/+/437288
Reviewed-by: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
diff --git a/client/client.go b/client/client.go
index d633a2a..97a5d30 100644
--- a/client/client.go
+++ b/client/client.go
@@ -189,9 +189,18 @@
 	c      *http.Client
 	cache  Cache
 	dbName string
+
+	// indexCalls counts the number of times Index()
+	// method has been called. httpCalls counts the
+	// number of times GetByModule makes an http request
+	// to the vuln db for a module path. Used for testing
+	// privacy properties of httpSource.
+	indexCalls int
+	httpCalls  int
 }
 
 func (hs *httpSource) Index(ctx context.Context) (_ DBIndex, err error) {
+	hs.indexCalls++ // for testing privacy properties
 	defer derrors.Wrap(&err, "Index()")
 
 	var cachedIndex DBIndex
@@ -284,6 +293,7 @@
 	if err != nil {
 		return nil, err
 	}
+	hs.httpCalls++ // for testing privacy properties
 	entries, err := httpReadJSON[[]*osv.Entry](ctx, hs, epath+".json")
 	if err != nil || entries == nil {
 		return nil, err
diff --git a/client/client_test.go b/client/client_test.go
index 2469642..1d7d1ec 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -155,7 +155,58 @@
 				t.Errorf("got\n\t%s\nbut should start with\n\t%s", got, test.detailPrefix)
 			}
 		})
+	}
+}
 
+// TestMustUseIndex checks that httpSource in NewClient(...)
+//   - always calls Index function before making an http
+//     request in GetByModule.
+//   - if an http request was made, then the module path
+//     must be in the index.
+//
+// This test serves as an approximate mechanism to make sure
+// that we only send module info to the vuln db server if the
+// module has known vulnerabilities. Otherwise, we might send
+// unknown private module information to the db.
+func TestMustUseIndex(t *testing.T) {
+	if runtime.GOOS == "js" {
+		t.Skip("skipping test: no network on js")
+	}
+	ctx := context.Background()
+	// Create a local http database.
+	srv := newTestServer()
+	defer srv.Close()
+
+	// List of modules to query, some are repeated to exercise cache hits.
+	modulePaths := []string{"github.com/BeeGo/beego", "github.com/tidwall/gjson", "net/http", "abc.xyz", "github.com/BeeGo/beego"}
+	for _, cache := range []Cache{newTestCache(), nil} {
+		clt, err := NewClient([]string{srv.URL}, Options{HTTPCache: cache})
+		if err != nil {
+			t.Fatal(err)
+		}
+		hs := clt.(*client).sources[0].(*httpSource)
+		for _, modulePath := range modulePaths {
+			indexCalls := hs.indexCalls
+			httpCalls := hs.httpCalls
+			if _, err := clt.GetByModule(ctx, modulePath); err != nil {
+				t.Fatal(err)
+			}
+			// Number of index Calls should be increased.
+			if hs.indexCalls == indexCalls {
+				t.Errorf("GetByModule(ctx, %s) [cache:%t] did not call Index(...)", modulePath, cache != nil)
+			}
+			// If http request was made, then the modulePath must be in the index.
+			if hs.httpCalls > httpCalls {
+				index, err := hs.Index(ctx)
+				if err != nil {
+					t.Fatal(err)
+				}
+				_, present := index[modulePath]
+				if !present {
+					t.Errorf("GetByModule(ctx, %s) [cache:%t] issued http request for module not in Index(...)", modulePath, cache != nil)
+				}
+			}
+		}
 	}
 }