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

Commitb948f2d

Browse files
authored
fix: Use environment variables for agent authentication (#1238)
* fix: Update GIT_COMMITTER_NAME to use usernameThis was a mistake when adding the committer fields 🤦.* fix: Use environment variables for agent authenticationUsing files led to situations where running "coder server --dev" wouldbreak `gitssh`. This is applicable in a production environment too. Usersshould be able to log into another Coder deployment from their workspace.Users can still set "CODER_URL" if they'd like with agent env vars!
1 parenteb60692 commitb948f2d

File tree

19 files changed

+143
-115
lines changed

19 files changed

+143
-115
lines changed

‎agent/agent.go

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040

4141
typeOptionsstruct {
4242
ReconnectingPTYTimeout time.Duration
43+
EnvironmentVariablesmap[string]string
4344
Logger slog.Logger
4445
}
4546

@@ -66,6 +67,7 @@ func New(dialer Dialer, options *Options) io.Closer {
6667
logger:options.Logger,
6768
closeCancel:cancelFunc,
6869
closed:make(chanstruct{}),
70+
envVars:options.EnvironmentVariables,
6971
}
7072
server.init(ctx)
7173
returnserver
@@ -83,23 +85,21 @@ type agent struct {
8385
closeMutex sync.Mutex
8486
closedchanstruct{}
8587

86-
// Environment variables sent by Coder to inject for shell sessions.
87-
// These are atomic because values can change after reconnect.
88-
envVars atomic.Value
89-
ownerEmail atomic.String
90-
ownerUsername atomic.String
88+
envVarsmap[string]string
89+
// metadata is atomic because values can change after reconnection.
90+
metadata atomic.Value
9191
startupScript atomic.Bool
9292
sshServer*ssh.Server
9393
}
9494

9595
func (a*agent)run(ctx context.Context) {
96-
varoptionsMetadata
96+
varmetadataMetadata
9797
varpeerListener*peerbroker.Listener
9898
varerrerror
9999
// An exponential back-off occurs when the connection is failing to dial.
100100
// This is to prevent server spam in case of a coderd outage.
101101
forretrier:=retry.New(50*time.Millisecond,10*time.Second);retrier.Wait(ctx); {
102-
options,peerListener,err=a.dialer(ctx,a.logger)
102+
metadata,peerListener,err=a.dialer(ctx,a.logger)
103103
iferr!=nil {
104104
iferrors.Is(err,context.Canceled) {
105105
return
@@ -118,14 +118,12 @@ func (a *agent) run(ctx context.Context) {
118118
return
119119
default:
120120
}
121-
a.envVars.Store(options.EnvironmentVariables)
122-
a.ownerEmail.Store(options.OwnerEmail)
123-
a.ownerUsername.Store(options.OwnerUsername)
121+
a.metadata.Store(metadata)
124122

125123
ifa.startupScript.CAS(false,true) {
126124
// The startup script has not ran yet!
127125
gofunc() {
128-
err:=a.runStartupScript(ctx,options.StartupScript)
126+
err:=a.runStartupScript(ctx,metadata.StartupScript)
129127
iferrors.Is(err,context.Canceled) {
130128
return
131129
}
@@ -172,7 +170,7 @@ func (*agent) runStartupScript(ctx context.Context, script string) error {
172170
writer,err=gsyslog.NewLogger(gsyslog.LOG_INFO,"USER","coder-startup-script")
173171
iferr!=nil {
174172
// If the syslog isn't supported or cannot be created, use a text file in temp.
175-
writer,err=os.CreateTemp("","coder-startup-script.txt")
173+
writer,err=os.CreateTemp("","coder-startup-script-*.txt")
176174
iferr!=nil {
177175
returnxerrors.Errorf("open startup script log file: %w",err)
178176
}
@@ -319,6 +317,15 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri
319317
returnnil,xerrors.Errorf("get user shell: %w",err)
320318
}
321319

320+
rawMetadata:=a.metadata.Load()
321+
ifrawMetadata==nil {
322+
returnnil,xerrors.Errorf("no metadata was provided: %w",err)
323+
}
324+
metadata,valid:=rawMetadata.(Metadata)
325+
if!valid {
326+
returnnil,xerrors.Errorf("metadata is the wrong type: %T",metadata)
327+
}
328+
322329
// gliderlabs/ssh returns a command slice of zero
323330
// when a shell is requested.
324331
command:=rawCommand
@@ -344,22 +351,23 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri
344351
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_SSH_COMMAND=%s gitssh --`,executablePath))
345352
// These prevent the user from having to specify _anything_ to successfully commit.
346353
// Both author and committer must be set!
347-
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_AUTHOR_EMAIL=%s`,a.ownerEmail.Load()))
348-
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_COMMITTER_EMAIL=%s`,a.ownerEmail.Load()))
349-
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_AUTHOR_NAME=%s`,a.ownerUsername.Load()))
350-
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_COMMITTER_NAME=%s`,a.ownerUsername.Load()))
354+
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_AUTHOR_EMAIL=%s`,metadata.OwnerEmail))
355+
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_COMMITTER_EMAIL=%s`,metadata.OwnerEmail))
356+
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_AUTHOR_NAME=%s`,metadata.OwnerUsername))
357+
cmd.Env=append(cmd.Env,fmt.Sprintf(`GIT_COMMITTER_NAME=%s`,metadata.OwnerUsername))
351358

352359
// Load environment variables passed via the agent.
353360
// These should override all variables we manually specify.
354-
envVars:=a.envVars.Load()
355-
ifenvVars!=nil {
356-
envVarMap,ok:=envVars.(map[string]string)
357-
ifok {
358-
forkey,value:=rangeenvVarMap {
359-
cmd.Env=append(cmd.Env,fmt.Sprintf("%s=%s",key,value))
360-
}
361-
}
361+
forkey,value:=rangemetadata.EnvironmentVariables {
362+
cmd.Env=append(cmd.Env,fmt.Sprintf("%s=%s",key,value))
363+
}
364+
365+
// Agent-level environment variables should take over all!
366+
// This is used for setting agent-specific variables like "CODER_AGENT_TOKEN".
367+
forkey,value:=rangea.envVars {
368+
cmd.Env=append(cmd.Env,fmt.Sprintf("%s=%s",key,value))
362369
}
370+
363371
returncmd,nil
364372
}
365373

‎cli/agent.go

Lines changed: 13 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,16 @@ import (
2121

2222
funcworkspaceAgent()*cobra.Command {
2323
var (
24-
rawURLstring
25-
authstring
26-
tokenstring
24+
authstring
2725
)
2826
cmd:=&cobra.Command{
2927
Use:"agent",
3028
// This command isn't useful to manually execute.
3129
Hidden:true,
3230
RunE:func(cmd*cobra.Command,args []string)error {
33-
ifrawURL=="" {
34-
returnxerrors.New("CODER_URL must be set")
31+
rawURL,err:=cmd.Flags().GetString(varAgentURL)
32+
iferr!=nil {
33+
returnxerrors.Errorf("CODER_AGENT_URL must be set: %w",err)
3534
}
3635
coderURL,err:=url.Parse(rawURL)
3736
iferr!=nil {
@@ -46,8 +45,9 @@ func workspaceAgent() *cobra.Command {
4645
varexchangeTokenfunc(context.Context) (codersdk.WorkspaceAgentAuthenticateResponse,error)
4746
switchauth {
4847
case"token":
49-
iftoken=="" {
50-
returnxerrors.Errorf("CODER_TOKEN must be set for token auth")
48+
token,err:=cmd.Flags().GetString(varAgentToken)
49+
iferr!=nil {
50+
returnxerrors.Errorf("CODER_AGENT_TOKEN must be set for token auth: %w",err)
5151
}
5252
client.SessionToken=token
5353
case"google-instance-identity":
@@ -115,27 +115,19 @@ func workspaceAgent() *cobra.Command {
115115
}
116116
}
117117

118-
cfg:=createConfig(cmd)
119-
err=cfg.AgentSession().Write(client.SessionToken)
120-
iferr!=nil {
121-
returnxerrors.Errorf("writing agent session token to config: %w",err)
122-
}
123-
err=cfg.URL().Write(client.URL.String())
124-
iferr!=nil {
125-
returnxerrors.Errorf("writing agent url to config: %w",err)
126-
}
127-
128118
closer:=agent.New(client.ListenWorkspaceAgent,&agent.Options{
129119
Logger:logger,
120+
EnvironmentVariables:map[string]string{
121+
// Override the "CODER_AGENT_TOKEN" variable in all
122+
// shells so "gitssh" works!
123+
"CODER_AGENT_TOKEN":client.SessionToken,
124+
},
130125
})
131126
<-cmd.Context().Done()
132127
returncloser.Close()
133128
},
134129
}
135130

136-
cliflag.StringVarP(cmd.Flags(),&auth,"auth","","CODER_AUTH","token","Specify the authentication type to use for the agent")
137-
cliflag.StringVarP(cmd.Flags(),&rawURL,"url","","CODER_URL","","Specify the URL to access Coder")
138-
cliflag.StringVarP(cmd.Flags(),&token,"token","","CODER_TOKEN","","Specifies the authentication token to access Coder")
139-
131+
cliflag.StringVarP(cmd.Flags(),&auth,"auth","","CODER_AGENT_AUTH","token","Specify the authentication type to use for the agent")
140132
returncmd
141133
}

‎cli/agent_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func TestWorkspaceAgent(t *testing.T) {
4646
workspace:=coderdtest.CreateWorkspace(t,client,user.OrganizationID,template.ID)
4747
coderdtest.AwaitWorkspaceBuildJob(t,client,workspace.LatestBuild.ID)
4848

49-
cmd,_:=clitest.New(t,"agent","--auth","azure-instance-identity","--url",client.URL.String())
49+
cmd,_:=clitest.New(t,"agent","--auth","azure-instance-identity","--agent-url",client.URL.String())
5050
ctx,cancelFunc:=context.WithCancel(context.Background())
5151
defercancelFunc()
5252
gofunc() {
@@ -100,7 +100,7 @@ func TestWorkspaceAgent(t *testing.T) {
100100
workspace:=coderdtest.CreateWorkspace(t,client,user.OrganizationID,template.ID)
101101
coderdtest.AwaitWorkspaceBuildJob(t,client,workspace.LatestBuild.ID)
102102

103-
cmd,_:=clitest.New(t,"agent","--auth","aws-instance-identity","--url",client.URL.String())
103+
cmd,_:=clitest.New(t,"agent","--auth","aws-instance-identity","--agent-url",client.URL.String())
104104
ctx,cancelFunc:=context.WithCancel(context.Background())
105105
defercancelFunc()
106106
gofunc() {
@@ -154,7 +154,7 @@ func TestWorkspaceAgent(t *testing.T) {
154154
workspace:=coderdtest.CreateWorkspace(t,client,user.OrganizationID,template.ID)
155155
coderdtest.AwaitWorkspaceBuildJob(t,client,workspace.LatestBuild.ID)
156156

157-
cmd,_:=clitest.New(t,"agent","--auth","google-instance-identity","--url",client.URL.String())
157+
cmd,_:=clitest.New(t,"agent","--auth","google-instance-identity","--agent-url",client.URL.String())
158158
ctx,cancelFunc:=context.WithCancel(context.Background())
159159
defercancelFunc()
160160
gofunc() {

‎cli/cliflag/cliflag.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ import (
1919
"github.com/spf13/pflag"
2020
)
2121

22+
// String sets a string flag on the given flag set.
23+
funcString(flagset*pflag.FlagSet,name,shorthand,env,def,usagestring) {
24+
v,ok:=os.LookupEnv(env)
25+
if!ok||v=="" {
26+
v=def
27+
}
28+
flagset.StringP(name,shorthand,v,fmtUsage(usage,env))
29+
}
30+
2231
// StringVarP sets a string flag on the given flag set.
2332
funcStringVarP(flagset*pflag.FlagSet,p*string,namestring,shorthandstring,envstring,defstring,usagestring) {
2433
v,ok:=os.LookupEnv(env)

‎cli/cliflag/cliflag_test.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,28 @@ import (
1616
//nolint:paralleltest
1717
funcTestCliflag(t*testing.T) {
1818
t.Run("StringDefault",func(t*testing.T) {
19+
flagset,name,shorthand,env,usage:=randomFlag()
20+
def,_:=cryptorand.String(10)
21+
cliflag.String(flagset,name,shorthand,env,def,usage)
22+
got,err:=flagset.GetString(name)
23+
require.NoError(t,err)
24+
require.Equal(t,def,got)
25+
require.Contains(t,flagset.FlagUsages(),usage)
26+
require.Contains(t,flagset.FlagUsages(),fmt.Sprintf(" - consumes $%s",env))
27+
})
28+
29+
t.Run("StringEnvVar",func(t*testing.T) {
30+
flagset,name,shorthand,env,usage:=randomFlag()
31+
envValue,_:=cryptorand.String(10)
32+
t.Setenv(env,envValue)
33+
def,_:=cryptorand.String(10)
34+
cliflag.String(flagset,name,shorthand,env,def,usage)
35+
got,err:=flagset.GetString(name)
36+
require.NoError(t,err)
37+
require.Equal(t,envValue,got)
38+
})
39+
40+
t.Run("StringVarPDefault",func(t*testing.T) {
1941
varptrstring
2042
flagset,name,shorthand,env,usage:=randomFlag()
2143
def,_:=cryptorand.String(10)
@@ -28,7 +50,7 @@ func TestCliflag(t *testing.T) {
2850
require.Contains(t,flagset.FlagUsages(),fmt.Sprintf(" - consumes $%s",env))
2951
})
3052

31-
t.Run("StringEnvVar",func(t*testing.T) {
53+
t.Run("StringVarPEnvVar",func(t*testing.T) {
3254
varptrstring
3355
flagset,name,shorthand,env,usage:=randomFlag()
3456
envValue,_:=cryptorand.String(10)

‎cli/config/file.go

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,6 @@ func (r Root) Organization() File {
2121
returnFile(filepath.Join(string(r),"organization"))
2222
}
2323

24-
func (rRoot)AgentSession()File {
25-
returnFile(filepath.Join(string(r),"agentsession"))
26-
}
27-
2824
// File provides convenience methods for interacting with *os.File.
2925
typeFilestring
3026

‎cli/gitssh.go

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package cli
22

33
import (
44
"fmt"
5-
"net/url"
65
"os"
76
"os/exec"
87
"strings"
@@ -11,7 +10,6 @@ import (
1110
"golang.org/x/xerrors"
1211

1312
"github.com/coder/coder/cli/cliui"
14-
"github.com/coder/coder/codersdk"
1513
)
1614

1715
funcgitssh()*cobra.Command {
@@ -20,22 +18,10 @@ func gitssh() *cobra.Command {
2018
Hidden:true,
2119
Short:`Wraps the "ssh" command and uses the coder gitssh key for authentication`,
2220
RunE:func(cmd*cobra.Command,args []string)error {
23-
cfg:=createConfig(cmd)
24-
rawURL,err:=cfg.URL().Read()
21+
client,err:=createAgentClient(cmd)
2522
iferr!=nil {
26-
returnxerrors.Errorf("read agenturl from config: %w",err)
23+
returnxerrors.Errorf("create agentclient: %w",err)
2724
}
28-
parsedURL,err:=url.Parse(rawURL)
29-
iferr!=nil {
30-
returnxerrors.Errorf("parse agent url from config: %w",err)
31-
}
32-
session,err:=cfg.AgentSession().Read()
33-
iferr!=nil {
34-
returnxerrors.Errorf("read agent session from config: %w",err)
35-
}
36-
client:=codersdk.New(parsedURL)
37-
client.SessionToken=session
38-
3925
key,err:=client.AgentGitSSHKey(cmd.Context())
4026
iferr!=nil {
4127
returnxerrors.Errorf("get agent git ssh token: %w",err)

‎cli/gitssh_test.go

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,10 @@ import (
99

1010
"github.com/gliderlabs/ssh"
1111
"github.com/google/uuid"
12-
"github.com/spf13/cobra"
1312
"github.com/stretchr/testify/require"
1413
gossh"golang.org/x/crypto/ssh"
1514

1615
"github.com/coder/coder/cli/clitest"
17-
"github.com/coder/coder/cli/config"
1816
"github.com/coder/coder/coderd/coderdtest"
1917
"github.com/coder/coder/codersdk"
2018
"github.com/coder/coder/provisioner/echo"
@@ -61,7 +59,7 @@ func TestGitSSH(t *testing.T) {
6159
coderdtest.AwaitWorkspaceBuildJob(t,client,workspace.LatestBuild.ID)
6260

6361
// start workspace agent
64-
cmd,root:=clitest.New(t,"agent","--token",agentToken,"--url",client.URL.String())
62+
cmd,root:=clitest.New(t,"agent","--agent-token",agentToken,"--agent-url",client.URL.String())
6563
agentClient:=&*client
6664
clitest.SetupConfig(t,agentClient,root)
6765
ctx,cancelFunc:=context.WithCancel(context.Background())
@@ -92,7 +90,7 @@ func TestGitSSH(t *testing.T) {
9290
// as long as we get a successful session we don't care if the server errors
9391
_=ssh.Serve(l,func(s ssh.Session) {
9492
atomic.AddInt64(&inc,1)
95-
t.Log("got authenticatedsesion")
93+
t.Log("got authenticatedsession")
9694
err:=s.Exit(0)
9795
require.NoError(t,err)
9896
},publicKeyOption)
@@ -101,22 +99,10 @@ func TestGitSSH(t *testing.T) {
10199
// start ssh session
102100
addr,ok:=l.Addr().(*net.TCPAddr)
103101
require.True(t,ok)
104-
cfgDir:=createConfig(cmd)
105102
// set to agent config dir
106-
cmd,root=clitest.New(t,"gitssh","--global-config="+string(cfgDir),"--",fmt.Sprintf("-p%d",addr.Port),"-o","StrictHostKeyChecking=no","127.0.0.1")
107-
clitest.SetupConfig(t,agentClient,root)
108-
103+
cmd,_=clitest.New(t,"gitssh","--agent-url",agentClient.URL.String(),"--agent-token",agentToken,"--",fmt.Sprintf("-p%d",addr.Port),"-o","StrictHostKeyChecking=no","127.0.0.1")
109104
err=cmd.ExecuteContext(context.Background())
110105
require.NoError(t,err)
111106
require.EqualValues(t,1,inc)
112107
})
113108
}
114-
115-
// createConfig consumes the global configuration flag to produce a config root.
116-
funccreateConfig(cmd*cobra.Command) config.Root {
117-
globalRoot,err:=cmd.Flags().GetString("global-config")
118-
iferr!=nil {
119-
panic(err)
120-
}
121-
returnconfig.Root(globalRoot)
122-
}

0 commit comments

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