From f19acbdc94578194d19a6758f01cd8eed85b792e Mon Sep 17 00:00:00 2001
From: Matthew Fisher <matt.fisher@microsoft.com>
Date: Thu, 17 Sep 2020 14:54:55 -0700
Subject: [PATCH] fix: allow serverInfo field on index files

A recent change merged into Helm fixes a number of security issues related to parsing malformed index files. Unfortunately, it also broke the ability for users to load index files from chartmuseum, which adds a "server info" field to add additional metadata.

This commit adds that field so that index files from chartmuseum can be validated. Since Helm does not use this field for anything, the information is discarded and unused.

Signed-off-by: Matthew Fisher <matt.fisher@microsoft.com>
---
 pkg/repo/index.go                        |  2 +
 pkg/repo/index_test.go                   | 85 +++++++++++++++---------
 pkg/repo/testdata/chartmuseum-index.yaml | 50 ++++++++++++++
 3 files changed, 105 insertions(+), 32 deletions(-)
 create mode 100644 pkg/repo/testdata/chartmuseum-index.yaml

diff --git a/pkg/repo/index.go b/pkg/repo/index.go
index 8b831029f..55b984eea 100644
--- a/pkg/repo/index.go
+++ b/pkg/repo/index.go
@@ -77,6 +77,8 @@ func (c ChartVersions) Less(a, b int) bool {
 
 // IndexFile represents the index file in a chart repository
 type IndexFile struct {
+	// This is used ONLY for validation against chartmuseum's index files and is discarded after validation.
+	ServerInfo map[string]interface{}   `json:"serverInfo,omitempty"`
 	APIVersion string                   `json:"apiVersion"`
 	Generated  time.Time                `json:"generated"`
 	Entries    map[string]ChartVersions `json:"entries"`
diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go
index 77b3a90ab..c22588971 100644
--- a/pkg/repo/index_test.go
+++ b/pkg/repo/index_test.go
@@ -35,9 +35,31 @@ import (
 )
 
 const (
-	testfile          = "testdata/local-index.yaml"
-	unorderedTestfile = "testdata/local-index-unordered.yaml"
-	testRepo          = "test-repo"
+	testfile            = "testdata/local-index.yaml"
+	chartmuseumtestfile = "testdata/chartmuseum-index.yaml"
+	unorderedTestfile   = "testdata/local-index-unordered.yaml"
+	testRepo            = "test-repo"
+	indexWithDuplicates = `
+apiVersion: v1
+entries:
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz
+      name: nginx
+      description: string
+      version: 0.2.0
+      home: https://github.com/something/else
+      digest: "sha256:1234567890abcdef"
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/alpine-1.0.0.tgz
+        - http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
+      name: alpine
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      digest: "sha256:1234567890abcdef"
+`
 )
 
 func TestIndexFile(t *testing.T) {
@@ -84,39 +106,38 @@ func TestIndexFile(t *testing.T) {
 }
 
 func TestLoadIndex(t *testing.T) {
-	b, err := ioutil.ReadFile(testfile)
-	if err != nil {
-		t.Fatal(err)
+
+	tests := []struct {
+		Name     string
+		Filename string
+	}{
+		{
+			Name:     "regular index file",
+			Filename: testfile,
+		},
+		{
+			Name:     "chartmuseum index file",
+			Filename: chartmuseumtestfile,
+		},
 	}
-	i, err := loadIndex(b)
-	if err != nil {
-		t.Fatal(err)
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.Name, func(t *testing.T) {
+			t.Parallel()
+			b, err := ioutil.ReadFile(tc.Filename)
+			if err != nil {
+				t.Fatal(err)
+			}
+			i, err := loadIndex(b)
+			if err != nil {
+				t.Fatal(err)
+			}
+			verifyLocalIndex(t, i)
+		})
 	}
-	verifyLocalIndex(t, i)
 }
 
-const indexWithDuplicates = `
-apiVersion: v1
-entries:
-  nginx:
-    - urls:
-        - https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz
-      name: nginx
-      description: string
-      version: 0.2.0
-      home: https://github.com/something/else
-      digest: "sha256:1234567890abcdef"
-  nginx:
-    - urls:
-        - https://kubernetes-charts.storage.googleapis.com/alpine-1.0.0.tgz
-        - http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
-      name: alpine
-      description: string
-      version: 1.0.0
-      home: https://github.com/something
-      digest: "sha256:1234567890abcdef"
-`
-
 // TestLoadIndex_Duplicates is a regression to make sure that we don't non-deterministically allow duplicate packages.
 func TestLoadIndex_Duplicates(t *testing.T) {
 	if _, err := loadIndex([]byte(indexWithDuplicates)); err == nil {
diff --git a/pkg/repo/testdata/chartmuseum-index.yaml b/pkg/repo/testdata/chartmuseum-index.yaml
new file mode 100644
index 000000000..3077596f4
--- /dev/null
+++ b/pkg/repo/testdata/chartmuseum-index.yaml
@@ -0,0 +1,50 @@
+serverInfo:
+  contextPath: /v1/helm
+apiVersion: v1
+entries:
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz
+      name: nginx
+      description: string
+      version: 0.2.0
+      home: https://github.com/something/else
+      digest: "sha256:1234567890abcdef"
+      keywords:
+        - popular
+        - web server
+        - proxy
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.1.0.tgz
+      name: nginx
+      description: string
+      version: 0.1.0
+      home: https://github.com/something
+      digest: "sha256:1234567890abcdef"
+      keywords:
+        - popular
+        - web server
+        - proxy
+  alpine:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/alpine-1.0.0.tgz
+        - http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
+      name: alpine
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      keywords:
+        - linux
+        - alpine
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"
+  chartWithNoURL:
+    - name: chartWithNoURL
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      keywords:
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"