Oct 19, 2022 · Oct 18, 2022 · Oct 19, 2022 · Oct 19, 2022
diff --git a/provisioner/terraform/executor.go b/provisioner/terraform/executor.go
 func (e executor) basicEnv() []string {
 // Required for "terraform init" to find "git" to
 // clone Terraform modules.
 env :=os.Environ()
 env :=safeEnviron()
 // Only Linux reliably works with the Terraform plugin
 // cache directory. It's unknown why this is.
 if e.cachePath != "" && runtime.GOOS == "linux" {
 return ctx.Err()
 }

 if isCanarySet(env) {
 return xerrors.New("environment variables not sanitized, this is a bug within Coder")
 }

 // #nosec
 cmd := exec.CommandContext(killCtx, e.binaryPath, args...)
 cmd.Dir = e.workdir
diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go
 }

 func provisionEnv(start *proto.Provision_Start) ([]string, error) {
 env :=os.Environ()
 env :=safeEnviron()
 env = append(env,
 "CODER_AGENT_URL="+start.Metadata.CoderUrl,
 "CODER_WORKSPACE_TRANSITION="+strings.ToLower(start.Metadata.WorkspaceTransition.String()),
 )

 func logTerraformEnvVars(logr logger) error {
 env :=os.Environ()
 env :=safeEnviron()
 for _, e := range env {
 if strings.HasPrefix(e, "TF_") {
 parts := strings.SplitN(e, "=", 2)
 if len(parts) != 2 {
 panic("os.Environ() returned vars not in key=value form")
 panic("safeEnviron() returned vars not in key=value form")
 }
 if !tfEnvSafeToPrint[parts[0]] {
 parts[1] = "<value redacted>"
diff --git a/provisioner/terraform/provision_test.go b/provisioner/terraform/provision_test.go
 // nolint:paralleltest
 func TestProvision_ExtraEnv(t *testing.T) {
 // #nosec
 secretValue:= "oinae3uinxase"
 constsecretValue = "oinae3uinxase"
 t.Setenv("TF_LOG", "INFO")
 t.Setenv("TF_SUPERSECRET", secretValue)

 }
 require.True(t, found)
 }

 // nolint:paralleltest
 func TestProvision_SafeEnv(t *testing.T) {
 // #nosec
 const (
 passedValue = "superautopets"
 secretValue = "oinae3uinxase"
 )

 t.Setenv("VALID_USER_ENV", passedValue)

 // We ensure random 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)
 var (
 foundUserEnv = false
 // Some CODER_ environment variables used by our Terraform provider
 // must make it through.
 foundCoderEnv = 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) {
 foundUserEnv = true
 }
 if strings.Contains(log.Output, "CODER_") {
 foundCoderEnv = true
 }
 require.NotContains(t, log.Output, secretValue)
 }
 if c := msg.GetComplete(); c != nil {
 require.Empty(t, c.Error)
 break
 }
 }
 require.True(t, foundUserEnv)
 require.True(t, foundCoderEnv)
 }
diff --git a/provisioner/terraform/safeenv.go b/provisioner/terraform/safeenv.go
 package terraform

 import (
 "os"
 "strings"
 )

 // We must clean CODER_ environment variables to avoid accidentally passing in
 // secrets like the Postgres connection string. See
 // https://github.com/coder/coder/issues/4635.
 //
 // safeEnviron() is provided as an os.Environ() alternative that strips CODER_
 // variables. As an additional precaution, we check a canary variable before
 // provisioner exec.
 //
 // We cannot strip all CODER_ variables at exec because some are used to
 // configure the provisioner.

 const unsafeEnvCanary = "CODER_DONT_PASS"

 func init() {
 os.Setenv(unsafeEnvCanary, "true")
 }

 func envName(env string) string {
 parts := strings.SplitN(env, "=", 1)
 if len(parts) > 0 {
 return parts[0]
 }
 return ""
 }

 func isCanarySet(env []string) bool {
 for _, e := range env {
 if envName(e) == unsafeEnvCanary {
 return true
 }
 }
 return false
 }

 // safeEnviron wraps os.Environ but removes CODER_ environment variables.
 func safeEnviron() []string {
 env := os.Environ()
 strippedEnv := make([]string, 0, len(env))

 for _, e := range env {
 name := envName(e)
 if strings.HasPrefix(name, "CODER_") {
 continue
 }
 strippedEnv = append(strippedEnv, e)
 }
 return strippedEnv
 }
diff --git a/site/src/components/Resources/AgentLatency.tsx b/site/src/components/Resources/AgentLatency.tsx
      >
        <HelpTooltipTitle>Latency</HelpTooltipTitle>
        <HelpTooltipText>
 Latency from relay servers, used whenconnections cannot connect
 peer-to-peer. Starindicates the preferred relay.
 This is the latency overhead on non peer to peerconnections. The star
          indicates the preferred relay.
        </HelpTooltipText>

        <HelpTooltipText>
diff --git a/site/src/components/Resources/ResourceAgentLatency.stories.tsx b/site/src/components/Resources/ResourceAgentLatency.stories.tsx
diff --git a/site/src/components/Resources/ResourceAgentLatency.tsx b/site/src/components/Resources/ResourceAgentLatency.tsx
Original file line number	Diff line number	Diff line change
Expand Up		@@ -32,7 +32,7 @@ type executor struct {
		func (e executor) basicEnv() []string {
		// Required for "terraform init" to find "git" to
		// clone Terraform modules.
		env :=os.Environ()
		env :=safeEnviron()
		// Only Linux reliably works with the Terraform plugin
		// cache directory. It's unknown why this is.
		if e.cachePath != "" && runtime.GOOS == "linux" {
Expand All		@@ -56,6 +56,10 @@ func (e executor) execWriteOutput(ctx, killCtx context.Context, args, env []stri
		return ctx.Err()
		}

		if isCanarySet(env) {
		return xerrors.New("environment variables not sanitized, this is a bug within Coder")
		}

		// #nosec
		cmd := exec.CommandContext(killCtx, e.binaryPath, args...)
		cmd.Dir = e.workdir
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -188,7 +188,7 @@ func provisionVars(start *proto.Provision_Start) ([]string, error) {
		}

		func provisionEnv(start *proto.Provision_Start) ([]string, error) {
		env :=os.Environ()
		env :=safeEnviron()
		env = append(env,
		"CODER_AGENT_URL="+start.Metadata.CoderUrl,
		"CODER_WORKSPACE_TRANSITION="+strings.ToLower(start.Metadata.WorkspaceTransition.String()),
Expand DownExpand Up		@@ -232,12 +232,12 @@ var (
		)

		func logTerraformEnvVars(logr logger) error {
		env :=os.Environ()
		env :=safeEnviron()
		for _, e := range env {
		if strings.HasPrefix(e, "TF_") {
		parts := strings.SplitN(e, "=", 2)
		if len(parts) != 2 {
		panic("os.Environ() returned vars not in key=value form")
		panic("safeEnviron() returned vars not in key=value form")
		}
		if !tfEnvSafeToPrint[parts[0]] {
		parts[1] = "<value redacted>"
Expand Down
Original file line number	Diff line number	Diff line change
Expand Up		@@ -415,7 +415,7 @@ func TestProvision(t *testing.T) {
		// nolint:paralleltest
		func TestProvision_ExtraEnv(t *testing.T) {
		// #nosec
		secretValue:= "oinae3uinxase"
		constsecretValue = "oinae3uinxase"
		t.Setenv("TF_LOG", "INFO")
		t.Setenv("TF_SUPERSECRET", secretValue)

Expand DownExpand Up		@@ -459,3 +459,76 @@ 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("VALID_USER_ENV", passedValue)

		// We ensure random 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)
		var (
		foundUserEnv = false
		// Some CODER_ environment variables used by our Terraform provider
		// must make it through.
		foundCoderEnv = 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) {
		foundUserEnv = true
		}
		if strings.Contains(log.Output, "CODER_") {
		foundCoderEnv = true
		}
		require.NotContains(t, log.Output, secretValue)
		}
		if c := msg.GetComplete(); c != nil {
		require.Empty(t, c.Error)
		break
		}
		}
		require.True(t, foundUserEnv)
		require.True(t, foundCoderEnv)
		}
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,55 @@
		package terraform

		import (
		"os"
		"strings"
		)

		// We must clean CODER_ environment variables to avoid accidentally passing in
		// secrets like the Postgres connection string. See
		// https://github.com/coder/coder/issues/4635.
		//
		// safeEnviron() is provided as an os.Environ() alternative that strips CODER_
		// variables. As an additional precaution, we check a canary variable before
		// provisioner exec.
		//
		// We cannot strip all CODER_ variables at exec because some are used to
		// configure the provisioner.

		const unsafeEnvCanary = "CODER_DONT_PASS"

		func init() {
		os.Setenv(unsafeEnvCanary, "true")
		}

		func envName(env string) string {
		parts := strings.SplitN(env, "=", 1)
		if len(parts) > 0 {
		return parts[0]
		}
		return ""
		}

		func isCanarySet(env []string) bool {
		for _, e := range env {
		if envName(e) == unsafeEnvCanary {
		return true
		}
		}
		return false
		}

		// safeEnviron wraps os.Environ but removes CODER_ environment variables.
		func safeEnviron() []string {
		env := os.Environ()
		strippedEnv := make([]string, 0, len(env))

		for _, e := range env {
		name := envName(e)
		if strings.HasPrefix(name, "CODER_") {
		continue
		}
		strippedEnv = append(strippedEnv, e)
		}
		return strippedEnv
		}