don't log terraform environment variables we don't know are safe

spikecurtis · spikecurtis · commit c6c42983a2f7 · 2022-06-13T16:51:22.000-07:00
Signed-off-by: Spike Curtis &lt;spike@coder.com&gt;
diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go
@@ -181,11 +181,38 @@ func provisionEnv(start *proto.Provision_Start) ([]string, error) {
 	return env, nil
 }
 
+var (
+	// tfEnvSafeToPrint is the set of terraform environment variables that we are quite sure won't contain secrets,
+	// and therefore it's ok to log their values
+	tfEnvSafeToPrint = map[string]bool{
+		"TF_LOG":                      true,
+		"TF_LOG_PATH":                 true,
+		"TF_INPUT":                    true,
+		"TF_DATA_DIR":                 true,
+		"TF_WORKSPACE":                true,
+		"TF_IN_AUTOMATION":            true,
+		"TF_REGISTRY_DISCOVERY_RETRY": true,
+		"TF_REGISTRY_CLIENT_TIMEOUT":  true,
+		"TF_CLI_CONFIG_FILE":          true,
+		"TF_IGNORE":                   true,
+	}
+)
+
 func logTerraformEnvVars(logger provisionersdk.Logger) error {
 	env := os.Environ()
 	for _, e := range env {
 		if strings.HasPrefix(e, "TF_") {
-			err := logger.Log(&proto.Log{Level: proto.LogLevel_WARN, Output: "terraform environment variable: " + e})
+			parts := strings.SplitN(e, "=", 2)
+			if len(parts) != 2 {
+				panic("os.Environ() returned vars not in key=value form")
+			}
+			if !tfEnvSafeToPrint[parts[0]] {
+				parts[1] = "<value redacted>"
+			}
+			err := logger.Log(&proto.Log{
+				Level:  proto.LogLevel_WARN,
+				Output: fmt.Sprintf("terraform environment variable: %s=%s", parts[0], parts[1]),
+			})
 			if err != nil {
 				return err
 			}
diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go
@@ -22,7 +22,7 @@ import (
 	"github.com/coder/coder/provisionersdk/proto"
 )
 
-func testProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) {
+func setupProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient) {
 	client, server := provisionersdk.TransportPipe()
 	ctx, cancelFunc := context.WithCancel(context.Background())
 	t.Cleanup(func() {
@@ -46,7 +46,7 @@ func testProvisioner(t *testing.T) (context.Context, proto.DRPCProvisionerClient
 func TestProvision(t *testing.T) {
 	t.Parallel()
 
-	ctx, api := testProvisioner(t)
+	ctx, api := setupProvisioner(t)
 
 	for _, testCase := range []struct {
 		Name     string
@@ -298,9 +298,11 @@ func TestProvision(t *testing.T) {
 
 // nolint:paralleltest
 func TestProvision_ExtraEnv(t *testing.T) {
+	secretValue := "oinae3uinxase"
 	t.Setenv("TF_LOG", "INFO")
+	t.Setenv("TF_SUPERSECRET", secretValue)
 
-	ctx, api := testProvisioner(t)
+	ctx, api := setupProvisioner(t)
 
 	directory := t.TempDir()
 	path := filepath.Join(directory, "main.tf")
@@ -330,6 +332,7 @@ func TestProvision_ExtraEnv(t *testing.T) {
 			if strings.Contains(log.Output, "TF_LOG") {
 				found = true
 			}
+			require.NotContains(t, log.Output, secretValue)
 		}
 		if c := msg.GetComplete(); c != nil {
 			require.Empty(t, c.Error)
