provisioner: don't pass CODER_ variables

ammario · ammario · commit b69d383625a8 · 2022-10-18T21:59:08.000Z
diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go
@@ -41,6 +41,19 @@ func (e executor) basicEnv() []string {
 	return env
 }
 
+// stripCoderEnv removes CODER_ environment variables to prevent accidentally
+// passing in secrets. See https://github.com/coder/coder/issues/4635.
+func stripCoderEnv(env []string) []string {
+	strippedEnv := make([]string, 0, len(env))
+	for _, e := range env {
+		if strings.HasPrefix(e, "CODER_") {
+			continue
+		}
+		strippedEnv = append(strippedEnv, e)
+	}
+	return strippedEnv
+}
+
 func (e executor) execWriteOutput(ctx, killCtx context.Context, args, env []string, stdOutWriter, stdErrWriter io.WriteCloser) (err error) {
 	defer func() {
 		closeErr := stdOutWriter.Close()
@@ -59,7 +72,7 @@ func (e executor) execWriteOutput(ctx, killCtx context.Context, args, env []stri
 	// #nosec
 	cmd := exec.CommandContext(killCtx, e.binaryPath, args...)
 	cmd.Dir = e.workdir
-	cmd.Env = env
+	cmd.Env = stripCoderEnv(env)
 
 	// We want logs to be written in the correct order, so we wrap all logging
 	// in a sync.Mutex.
diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go
@@ -415,7 +415,7 @@ func TestProvision(t *testing.T) {
 // nolint:paralleltest
 func TestProvision_ExtraEnv(t *testing.T) {
 	// #nosec
-	secretValue := "oinae3uinxase"
+	const secretValue = "oinae3uinxase"
 	t.Setenv("TF_LOG", "INFO")
 	t.Setenv("TF_SUPERSECRET", secretValue)
 
@@ -459,3 +459,65 @@ func TestProvision_ExtraEnv(t *testing.T) {
 	}
 	require.True(t, found)
 }
+
+// nolint:paralleltest
+func TestProvision_SafeEnv(t *testing.T) {
+	// #nosec
+	const (
+		passedValue = "superautopets"
+		secretValue = "oinae3uinxase"
+	)
+	t.Setenv("SOME_ENV", passedValue)
+	// We must ensure CODER_ variables aren't passed through to avoid leaking
+	// control plane secrets (e.g. PG URL).
+	t.Setenv("CODER_SECRET", secretValue)
+
+	const echoResource = `
+	resource "null_resource" "a" {
+		provisioner "local-exec" {
+		  command = "env"
+		}
+	  }
+
+	`
+
+	ctx, api := setupProvisioner(t, nil)
+
+	directory := t.TempDir()
+	path := filepath.Join(directory, "main.tf")
+	err := os.WriteFile(path, []byte(echoResource), 0o600)
+	require.NoError(t, err)
+
+	request := &proto.Provision_Request{
+		Type: &proto.Provision_Request_Start{
+			Start: &proto.Provision_Start{
+				Directory: directory,
+				Metadata: &proto.Provision_Metadata{
+					WorkspaceTransition: proto.WorkspaceTransition_START,
+				},
+			},
+		},
+	}
+	response, err := api.Provision(ctx)
+	require.NoError(t, err)
+	err = response.Send(request)
+	require.NoError(t, err)
+	found := false
+	for {
+		msg, err := response.Recv()
+		require.NoError(t, err)
+
+		if log := msg.GetLog(); log != nil {
+			t.Log(log.Level.String(), log.Output)
+			if strings.Contains(log.Output, passedValue) {
+				found = true
+			}
+			require.NotContains(t, log.Output, secretValue)
+		}
+		if c := msg.GetComplete(); c != nil {
+			require.Empty(t, c.Error)
+			break
+		}
+	}
+	require.True(t, found)
+}
