Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd49d57e

Browse files
authored
provisioner: don't pass CODER_ variables (#4638)
1 parent12cb4f1 commitd49d57e

File tree

7 files changed

+139
-123
lines changed

7 files changed

+139
-123
lines changed

‎provisioner/terraform/executor.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ type executor struct {
3232
func (eexecutor)basicEnv() []string {
3333
// Required for "terraform init" to find "git" to
3434
// clone Terraform modules.
35-
env:=os.Environ()
35+
env:=safeEnviron()
3636
// Only Linux reliably works with the Terraform plugin
3737
// cache directory. It's unknown why this is.
3838
ife.cachePath!=""&&runtime.GOOS=="linux" {
@@ -56,6 +56,10 @@ func (e executor) execWriteOutput(ctx, killCtx context.Context, args, env []stri
5656
returnctx.Err()
5757
}
5858

59+
ifisCanarySet(env) {
60+
returnxerrors.New("environment variables not sanitized, this is a bug within Coder")
61+
}
62+
5963
// #nosec
6064
cmd:=exec.CommandContext(killCtx,e.binaryPath,args...)
6165
cmd.Dir=e.workdir

‎provisioner/terraform/provision.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ func provisionVars(start *proto.Provision_Start) ([]string, error) {
188188
}
189189

190190
funcprovisionEnv(start*proto.Provision_Start) ([]string,error) {
191-
env:=os.Environ()
191+
env:=safeEnviron()
192192
env=append(env,
193193
"CODER_AGENT_URL="+start.Metadata.CoderUrl,
194194
"CODER_WORKSPACE_TRANSITION="+strings.ToLower(start.Metadata.WorkspaceTransition.String()),
@@ -232,12 +232,12 @@ var (
232232
)
233233

234234
funclogTerraformEnvVars(logrlogger)error {
235-
env:=os.Environ()
235+
env:=safeEnviron()
236236
for_,e:=rangeenv {
237237
ifstrings.HasPrefix(e,"TF_") {
238238
parts:=strings.SplitN(e,"=",2)
239239
iflen(parts)!=2 {
240-
panic("os.Environ() returned vars not in key=value form")
240+
panic("safeEnviron() returned vars not in key=value form")
241241
}
242242
if!tfEnvSafeToPrint[parts[0]] {
243243
parts[1]="<value redacted>"

‎provisioner/terraform/provision_test.go

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,7 @@ func TestProvision(t *testing.T) {
415415
// nolint:paralleltest
416416
funcTestProvision_ExtraEnv(t*testing.T) {
417417
// #nosec
418-
secretValue:="oinae3uinxase"
418+
constsecretValue="oinae3uinxase"
419419
t.Setenv("TF_LOG","INFO")
420420
t.Setenv("TF_SUPERSECRET",secretValue)
421421

@@ -459,3 +459,76 @@ func TestProvision_ExtraEnv(t *testing.T) {
459459
}
460460
require.True(t,found)
461461
}
462+
463+
// nolint:paralleltest
464+
funcTestProvision_SafeEnv(t*testing.T) {
465+
// #nosec
466+
const (
467+
passedValue="superautopets"
468+
secretValue="oinae3uinxase"
469+
)
470+
471+
t.Setenv("VALID_USER_ENV",passedValue)
472+
473+
// We ensure random CODER_ variables aren't passed through to avoid leaking
474+
// control plane secrets (e.g. PG URL).
475+
t.Setenv("CODER_SECRET",secretValue)
476+
477+
constechoResource=`
478+
resource "null_resource" "a" {
479+
provisioner "local-exec" {
480+
command = "env"
481+
}
482+
}
483+
484+
`
485+
486+
ctx,api:=setupProvisioner(t,nil)
487+
488+
directory:=t.TempDir()
489+
path:=filepath.Join(directory,"main.tf")
490+
err:=os.WriteFile(path, []byte(echoResource),0o600)
491+
require.NoError(t,err)
492+
493+
request:=&proto.Provision_Request{
494+
Type:&proto.Provision_Request_Start{
495+
Start:&proto.Provision_Start{
496+
Directory:directory,
497+
Metadata:&proto.Provision_Metadata{
498+
WorkspaceTransition:proto.WorkspaceTransition_START,
499+
},
500+
},
501+
},
502+
}
503+
response,err:=api.Provision(ctx)
504+
require.NoError(t,err)
505+
err=response.Send(request)
506+
require.NoError(t,err)
507+
var (
508+
foundUserEnv=false
509+
// Some CODER_ environment variables used by our Terraform provider
510+
// must make it through.
511+
foundCoderEnv=false
512+
)
513+
for {
514+
msg,err:=response.Recv()
515+
require.NoError(t,err)
516+
517+
iflog:=msg.GetLog();log!=nil {
518+
t.Log(log.Level.String(),log.Output)
519+
ifstrings.Contains(log.Output,passedValue) {
520+
foundUserEnv=true
521+
}
522+
ifstrings.Contains(log.Output,"CODER_") {
523+
foundCoderEnv=true
524+
}
525+
require.NotContains(t,log.Output,secretValue)
526+
}
527+
ifc:=msg.GetComplete();c!=nil {
528+
require.Empty(t,c.Error)
529+
break
530+
}
531+
}
532+
require.True(t,foundUserEnv)
533+
require.True(t,foundCoderEnv)
534+
}

‎provisioner/terraform/safeenv.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package terraform
2+
3+
import (
4+
"os"
5+
"strings"
6+
)
7+
8+
// We must clean CODER_ environment variables to avoid accidentally passing in
9+
// secrets like the Postgres connection string. See
10+
// https://github.com/coder/coder/issues/4635.
11+
//
12+
// safeEnviron() is provided as an os.Environ() alternative that strips CODER_
13+
// variables. As an additional precaution, we check a canary variable before
14+
// provisioner exec.
15+
//
16+
// We cannot strip all CODER_ variables at exec because some are used to
17+
// configure the provisioner.
18+
19+
constunsafeEnvCanary="CODER_DONT_PASS"
20+
21+
funcinit() {
22+
os.Setenv(unsafeEnvCanary,"true")
23+
}
24+
25+
funcenvName(envstring)string {
26+
parts:=strings.SplitN(env,"=",1)
27+
iflen(parts)>0 {
28+
returnparts[0]
29+
}
30+
return""
31+
}
32+
33+
funcisCanarySet(env []string)bool {
34+
for_,e:=rangeenv {
35+
ifenvName(e)==unsafeEnvCanary {
36+
returntrue
37+
}
38+
}
39+
returnfalse
40+
}
41+
42+
// safeEnviron wraps os.Environ but removes CODER_ environment variables.
43+
funcsafeEnviron() []string {
44+
env:=os.Environ()
45+
strippedEnv:=make([]string,0,len(env))
46+
47+
for_,e:=rangeenv {
48+
name:=envName(e)
49+
ifstrings.HasPrefix(name,"CODER_") {
50+
continue
51+
}
52+
strippedEnv=append(strippedEnv,e)
53+
}
54+
returnstrippedEnv
55+
}

‎site/src/components/Resources/AgentLatency.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,8 @@ export const AgentLatency: FC<{ agent: WorkspaceAgent }> = ({ agent }) => {
6868
>
6969
<HelpTooltipTitle>Latency</HelpTooltipTitle>
7070
<HelpTooltipText>
71-
Latency from relay servers, used whenconnections cannot connect
72-
peer-to-peer. Starindicates the preferred relay.
71+
This is the latency overhead on non peer to peerconnections. The star
72+
indicates the preferred relay.
7373
</HelpTooltipText>
7474

7575
<HelpTooltipText>

‎site/src/components/Resources/ResourceAgentLatency.stories.tsx

Lines changed: 0 additions & 46 deletions
This file was deleted.

‎site/src/components/Resources/ResourceAgentLatency.tsx

Lines changed: 0 additions & 70 deletions
This file was deleted.

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp