From 06d5252042e4cc9e8f693d33f6bd2408072f4907 Mon Sep 17 00:00:00 2001
From: sanfrancrisko <ciaran.mccrisken@puppet.com>
Date: Thu, 10 Jun 2021 14:44:35 +0100
Subject: [PATCH] (GH-15) Unset necessary env vars in pdkshell

Prior to this commit, if a user was managing their rubies with RVM,
execution of the PDK from the PCT's pdkshell package would fail due
to the RVM injections in PATH taking priority over the PDK's bundled
2.4 instance.

This commit emulates the unsetting of the necessary ENV VARS, similar
to the [pdk_env_wrapper](https://github.com/puppetlabs/pdk-vanagon/blob/main/resources/files/posix/pdk_env_wrapper) script, whilst preserving any user / system defined ENV VARs.
---
 internal/pkg/pdkshell/env_var.go      | 51 +++++++++++++++++++++++++++
 internal/pkg/pdkshell/env_var_test.go | 45 +++++++++++++++++++++++
 internal/pkg/pdkshell/os_utils.go     | 13 +++++++
 internal/pkg/pdkshell/pdkshell.go     |  4 ++-
 4 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 internal/pkg/pdkshell/env_var.go
 create mode 100644 internal/pkg/pdkshell/env_var_test.go
 create mode 100644 internal/pkg/pdkshell/os_utils.go

diff --git a/internal/pkg/pdkshell/env_var.go b/internal/pkg/pdkshell/env_var.go
new file mode 100644
index 00000000..b446b9b8
--- /dev/null
+++ b/internal/pkg/pdkshell/env_var.go
@@ -0,0 +1,51 @@
+package pdkshell
+
+import (
+	"strings"
+
+	"github.com/rs/zerolog/log"
+)
+
+var osUtil osHelpers
+
+func init() {
+	osUtil = osHelpersImpl{}
+}
+
+// There are a number of ENV VARs that need to be unset on POSIX systems before execution:
+// https://github.com/puppetlabs/pdk-vanagon/blob/0aa54c9129b137c2deabb0a417d59215df36fd91/resources/files/posix/pdk_env_wrapper
+func getEnvVarsToUnset() []string {
+	return []string{
+		"GEM_HOME",
+		"GEM_PATH",
+		"DLN_LIBRARY_PATH",
+		"RUBYLIB",
+		"RUBYLIB_PREFIX",
+		"RUBYOPT",
+		"RUBYPATH",
+		"RUBYSHELL",
+		"LD_LIBRARY_PATH",
+		"LD_PRELOAD",
+	}
+}
+
+func validEnvVar(envVar string) (valid bool) {
+	for _, illegalEnvVar := range getEnvVarsToUnset() {
+		if strings.HasPrefix(envVar, illegalEnvVar) {
+			log.Trace().Msgf("Dropping ENV VAR: %s", envVar)
+			return false
+		}
+	}
+	log.Trace().Msgf("Permitting ENV VAR: %s", envVar)
+	return true
+}
+
+func getEnvironVars() []string {
+	env := []string{}
+	for _, envVar := range osUtil.Environ() {
+		if validEnvVar(envVar) {
+			env = append(env, envVar)
+		}
+	}
+	return env
+}
diff --git a/internal/pkg/pdkshell/env_var_test.go b/internal/pkg/pdkshell/env_var_test.go
new file mode 100644
index 00000000..090ae4a1
--- /dev/null
+++ b/internal/pkg/pdkshell/env_var_test.go
@@ -0,0 +1,45 @@
+package pdkshell
+
+import (
+	"reflect"
+	"testing"
+)
+
+type osHelpersImplMock struct{}
+
+var mockedEnvironReturn []string
+
+func (osHelpersImplMock) Environ() []string {
+	return mockedEnvironReturn
+}
+
+func Test_getEnvironVars(t *testing.T) {
+
+	osUtil = osHelpersImplMock{}
+
+	tests := []struct {
+		name                string
+		mockedEnvironReturn []string
+		want                []string
+	}{
+		{
+			name:                "Drops ENV VAR to unset and passes through permitted",
+			mockedEnvironReturn: []string{"GEM_HOME=/foo/bar", "VALID_VAR=/baz/qux"},
+			want:                []string{"VALID_VAR=/baz/qux"},
+		},
+		{
+			name:                "Handles multiple instances of the same ENV VAR to be dropped",
+			mockedEnvironReturn: []string{"RUBYPATH=/foo/bar", "VALID_VAR=/baz/qux", "RUBYPATH=/foo/bar/bizz/buzz"},
+			want:                []string{"VALID_VAR=/baz/qux"},
+		},
+	}
+
+	for _, tt := range tests {
+		mockedEnvironReturn = tt.mockedEnvironReturn
+		t.Run(tt.name, func(t *testing.T) {
+			if got := getEnvironVars(); !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("getEnvironVars() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/internal/pkg/pdkshell/os_utils.go b/internal/pkg/pdkshell/os_utils.go
new file mode 100644
index 00000000..3d934019
--- /dev/null
+++ b/internal/pkg/pdkshell/os_utils.go
@@ -0,0 +1,13 @@
+package pdkshell
+
+import "os"
+
+type osHelpers interface {
+	Environ() []string
+}
+
+type osHelpersImpl struct{}
+
+func (osHelpersImpl) Environ() []string {
+	return os.Environ()
+}
diff --git a/internal/pkg/pdkshell/pdkshell.go b/internal/pkg/pdkshell/pdkshell.go
index 673e7299..dbb8d083 100644
--- a/internal/pkg/pdkshell/pdkshell.go
+++ b/internal/pkg/pdkshell/pdkshell.go
@@ -26,7 +26,7 @@ func Execute(args []string) (int, error) {
 	i := getPDKInfo()
 	executable := buildExecutable(i.RubyExecutable)
 	args = buildCommandArgs(args, i.RubyExecutable, i.PDKExecutable)
-	env := os.Environ()
+	env := getEnvironVars()
 	env = append(env, fmt.Sprintf("SSL_CERT_DIR=%s", i.CertDirectory), fmt.Sprintf("SSL_CERT_FILE=%s", i.CertPemFile))
 	cmd := &exec.Cmd{
 		Path:   executable,
@@ -37,6 +37,8 @@ func Execute(args []string) (int, error) {
 		Env:    env,
 	}
 
+	log.Trace().Msgf("PATH: %s", os.Getenv("PATH"))
+	log.Trace().Msgf("ENV: %s", env)
 	log.Trace().Msgf("args: %s", args)
 	if err := cmd.Run(); err != nil {
 		log.Fatal().Msgf("pdk failed with '%s'\n", err)