Repository: jenkins-x/lighthouse
Pull request #1514 (2022-11-25 16:35:03)

feat: Add support for HMAC token volume
Similarly to https://github.com/jenkins-x/lighthouse/pull/1511 some highly restricted environments do not allow secrets to be supplied via environment variables since they are a common attack vector.

This PR adds support for reading the HMAC token from a path supplied through an environment variable `HMAC_TOKEN_PATH`. Corresponding tests and a Helm chart option `hmacTokenVolumeMount.enabled` have also been added.

Backwards compatibility is maintained by looking for the `HMAC_TOKEN` environment variable first.

@ankitm123 I tried to add table driven tests as suggested in https://github.com/jenkins-x/lighthouse/pull/1511. Not sure if the tests in this PR is what you meant? If they are then I can add them for https://github.com/jenkins-x/lighthouse/pull/1511 in a subsequent PR


Patch:
diff --git a/charts/lighthouse/templates/foghorn-deployment.yaml b/charts/lighthouse/templates/foghorn-deployment.yaml
index 9f288361..e6f8026d 100644
--- a/charts/lighthouse/templates/foghorn-deployment.yaml
+++ b/charts/lighthouse/templates/foghorn-deployment.yaml
@@ -48,12 +48,18 @@ spec:
                 key: oauth
 {{- end }}
 {{- end }}
+{{- if .Values.hmacTokenEnabled }}
+{{- if .Values.hmacTokenVolumeMount.enabled }}
+          - name: "HMAC_TOKEN_PATH"
+            value: /secrets/lighthouse-hmac-token/hmac
+{{- else }}
           - name: "HMAC_TOKEN"
             valueFrom:
               secretKeyRef:
                 name: {{ .Values.hmacSecretName | default "lighthouse-hmac-token" }}
                 key: hmac
-                optional: {{ not .Values.hmacTokenEnabled }}
+{{- end }}
+{{- end }}
           - name: "JX_LOG_FORMAT"
             value: "{{ .Values.logFormat }}"
           - name: "LOGRUS_FORMAT"
@@ -83,6 +89,11 @@ spec:
           - name: lighthouse-oauth-token
             mountPath: /secrets/lighthouse-oauth-token
             readOnly: true
+{{- end }}
+{{- if and .Values.hmacTokenEnabled .Values.hmacTokenVolumeMount.enabled }}
+          - name: lighthouse-hmac-token
+            mountPath: /secrets/lighthouse-hmac-token
+            readOnly: true
 {{- end }}
       volumes:
 {{- if .Values.githubApp.enabled }}
@@ -93,6 +104,11 @@ spec:
         - name: lighthouse-oauth-token
           secret:
             secretName: lighthouse-oauth-token
+{{- end }}
+{{- if and .Values.hmacTokenEnabled .Values.hmacTokenVolumeMount.enabled }}
+        - name: lighthouse-hmac-token
+          secret:
+            secretName: {{ .Values.hmacSecretName | default "lighthouse-hmac-token" }}
 {{- end }}
       terminationGracePeriodSeconds: {{ .Values.foghorn.terminationGracePeriodSeconds }}
 {{- with .Values.foghorn.nodeSelector }}
diff --git a/charts/lighthouse/templates/poller-deployment.yaml b/charts/lighthouse/templates/poller-deployment.yaml
index 8444a4e6..372c5396 100644
--- a/charts/lighthouse/templates/poller-deployment.yaml
+++ b/charts/lighthouse/templates/poller-deployment.yaml
@@ -85,12 +85,18 @@ spec:
               key: oauth
 {{- end }}
 {{- end }}
+{{- if .Values.hmacTokenEnabled }}
+{{- if .Values.hmacTokenVolumeMount.enabled }}
+        - name: "HMAC_TOKEN_PATH"
+          value: /secrets/lighthouse-hmac-token/hmac
+{{- else }}
         - name: "HMAC_TOKEN"
           valueFrom:
             secretKeyRef:
               name: {{ .Values.hmacSecretName | default "lighthouse-hmac-token" }}
               key: hmac
-              optional: {{ not .Values.hmacTokenEnabled }}
+{{- end }}
+{{- end }}
         - name: "JX_LOG_FORMAT"
           value: "{{ .Values.logFormat }}"
         - name: LOG_LEVEL
@@ -124,6 +130,11 @@ spec:
         - name: lighthouse-oauth-token
           mountPath: /secrets/lighthouse-oauth-token
           readOnly: true
+{{- end }}
+{{- if and .Values.hmacTokenEnabled .Values.hmacTokenVolumeMount.enabled }}
+        - name: lighthouse-hmac-token
+          mountPath: /secrets/lighthouse-hmac-token
+          readOnly: true
 {{- end }}
       volumes:
 {{- if .Values.githubApp.enabled }}
@@ -135,6 +146,11 @@ spec:
         secret:
           secretName: lighthouse-oauth-token
 {{- end }}
+{{- if and .Values.hmacTokenEnabled .Values.hmacTokenVolumeMount.enabled }}
+      - name: lighthouse-hmac-token
+        secret:
+          secretName: {{ .Values.hmacSecretName | default "lighthouse-hmac-token" }}
+{{- end }}
 {{- with .Values.poller.nodeSelector }}
       nodeSelector:
 {{ toYaml . | indent 8 }}
diff --git a/charts/lighthouse/templates/webhooks-deployment.yaml b/charts/lighthouse/templates/webhooks-deployment.yaml
index e5b8dac9..452bd9b1 100644
--- a/charts/lighthouse/templates/webhooks-deployment.yaml
+++ b/charts/lighthouse/templates/webhooks-deployment.yaml
@@ -65,12 +65,18 @@ spec:
                 key: oauth
 {{- end }}
 {{- end }}
+{{- if .Values.hmacTokenEnabled }}
+{{- if .Values.hmacTokenVolumeMount.enabled }}
+          - name: "HMAC_TOKEN_PATH"
+            value: /secrets/lighthouse-hmac-token/hmac
+{{- else }}
           - name: "HMAC_TOKEN"
             valueFrom:
               secretKeyRef:
                 name: {{ .Values.hmacSecretName | default "lighthouse-hmac-token" }}
                 key: hmac
-                optional: {{ not .Values.hmacTokenEnabled }}
+{{- end }}
+{{- end }}
           - name: "JX_LOG_FORMAT"
             value: "{{ .Values.logFormat }}"
           - name: "LOGRUS_FORMAT"
@@ -121,6 +127,11 @@ spec:
           - name: lighthouse-oauth-token
             mountPath: /secrets/lighthouse-oauth-token
             readOnly: true
+{{- end }}
+{{- if and .Values.hmacTokenEnabled .Values.hmacTokenVolumeMount.enabled }}
+          - name: lighthouse-hmac-token
+            mountPath: /secrets/lighthouse-hmac-token
+            readOnly: true
 {{- end }}
       volumes:
 {{- if .Values.githubApp.enabled }}
@@ -131,6 +142,11 @@ spec:
         - name: lighthouse-oauth-token
           secret:
             secretName: lighthouse-oauth-token
+{{- end }}
+{{- if and .Values.hmacTokenEnabled .Values.hmacTokenVolumeMount.enabled }}
+        - name: lighthouse-hmac-token
+          secret:
+            secretName: {{ .Values.hmacSecretName | default "lighthouse-hmac-token" }}
 {{- end }}
       terminationGracePeriodSeconds: {{ .Values.webhooks.terminationGracePeriodSeconds }}
 {{- with .Values.webhooks.nodeSelector }}
diff --git a/charts/lighthouse/values.yaml b/charts/lighthouse/values.yaml
index 39d87e52..a43ddbdd 100644
--- a/charts/lighthouse/values.yaml
+++ b/charts/lighthouse/values.yaml
@@ -38,6 +38,10 @@ hmacSecretName: ""
 # hmacTokenEnabled -- Enables the use of a hmac token. This should always be enabled if possible - though some git providers don't support it such as bitbucket cloud
 hmacTokenEnabled: true
 
+# hmacTokenVolumeMount -- Mount hmac token as a volume instead of using an environment variable Secret reference
+hmacTokenVolumeMount:
+  enabled: false
+
 # logFormat -- Log format either json or stackdriver
 logFormat: "json"
 
diff --git a/cmd/poller/main.go b/cmd/poller/main.go
index 76f92823..eff0ba8c 100644
--- a/cmd/poller/main.go
+++ b/cmd/poller/main.go
@@ -54,7 +54,7 @@ type options struct {
 
 func (o *options) Validate() error {
 	if o.hmacToken == "" {
-		o.hmacToken = os.Getenv("HMAC_TOKEN")
+		o.hmacToken = util.HMACToken()
 	}
 	return nil
 }
diff --git a/pkg/util/scmclient.go b/pkg/util/scmclient.go
index 275c7b5b..fabb5098 100644
--- a/pkg/util/scmclient.go
+++ b/pkg/util/scmclient.go
@@ -179,9 +179,24 @@ func getSCMTokenFromPath(gitKind string) (string, error) {
 	return string(b), nil
 }
 
-// HMACToken gets the HMAC token from the environment
+// HMACToken gets the HMAC token from the environment or filesystem
 func HMACToken() string {
-	return os.Getenv("HMAC_TOKEN")
+	hmacToken := os.Getenv("HMAC_TOKEN")
+	// For backwards compatibility we only attempt to read from the filesystem
+	// if the HMAC token is not set in the environment
+	if len(hmacToken) == 0 {
+		// If HMAC_TOKEN_PATH is specified then attempt to read from the filesystem
+		hmacTokenPath := os.Getenv("HMAC_TOKEN_PATH")
+		if len(hmacTokenPath) > 0 {
+			b, err := os.ReadFile(hmacTokenPath)
+			if err != nil {
+				logrus.Errorf("failed to read HMAC_TOKEN_PATH %s: %s", hmacTokenPath, err)
+				return hmacToken
+			}
+			hmacToken = string(b)
+		}
+	}
+	return hmacToken
 }
 
 // BlobURLForProvider gets the link to the blob for an individual file in a commit or branch


Test patch:
diff --git a/pkg/util/scmclient_test.go b/pkg/util/scmclient_test.go
index 0fd720f8..bbbd327d 100644
--- a/pkg/util/scmclient_test.go
+++ b/pkg/util/scmclient_test.go
@@ -63,3 +63,63 @@ func TestGetSCMTokenUnsetError(t *testing.T) {
 	_, err = util.GetSCMToken("github")
 	require.Error(t, err)
 }
+
+// The following tests verify that the HMACToken function works correctly
+func TestHMACToken(t *testing.T) {
+	tests := map[string]struct {
+		envVars   map[string]string
+		wantError bool
+		hmacToken string
+	}{
+		"missing env vars": {
+			envVars:   map[string]string{},
+			hmacToken: "",
+		},
+		"hmac token env var": {
+			envVars: map[string]string{
+				"HMAC_TOKEN": "myhmactokenfromenvvar",
+			},
+			hmacToken: "myhmactokenfromenvvar",
+		},
+		"hmac token path env var": {
+			envVars: map[string]string{
+				"HMAC_TOKEN_PATH": filepath.Join("test_data", "secret_dir", "hmac-token"),
+			},
+			hmacToken: "myhmactokenfrompath",
+		},
+		"hmac token env var and path env var": {
+			envVars: map[string]string{
+				"HMAC_TOKEN":      "myhmactokenfromenvvar",
+				"HMAC_TOKEN_PATH": filepath.Join("test_data", "secret_dir", "hmac-token"),
+			},
+			hmacToken: "myhmactokenfromenvvar",
+		},
+		"hmac token missing path env var": {
+			envVars: map[string]string{
+				"HMAC_TOKEN_PATH": filepath.Join("test_data", "secret_dir", "does-not-exist"),
+			},
+			hmacToken: "",
+		},
+	}
+
+	for name, test := range tests {
+		t.Run(name, func(t *testing.T) {
+			// Set environment variables
+			for envVarName, envVarValue := range test.envVars {
+				os.Setenv(envVarName, envVarValue)
+			}
+
+			// Attempt to retrieve HMAC token
+			hmacToken := util.HMACToken()
+
+			// Verify HMAC token value
+			assert.Equal(t, test.hmacToken, hmacToken, "failed to get expected HMAC token: %s", hmacToken)
+
+			// Unset environment variables
+			for envVarName := range test.envVars {
+				err := os.Unsetenv(envVarName)
+				require.NoErrorf(t, err, "failed to unset environment variable %s", envVarName)
+			}
+		})
+	}
+}
diff --git a/pkg/util/test_data/secret_dir/hmac-token b/pkg/util/test_data/secret_dir/hmac-token
new file mode 100644
index 00000000..ca7972d7
--- /dev/null
+++ b/pkg/util/test_data/secret_dir/hmac-token
@@ -0,0 +1,1 @@
+myhmactokenfrompath
\ No newline at end of file