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

Commit3fb5d0b

Browse files
authored
fix(agent/agentcontainers): use correct env for execer commands (#18508)
1 parent7c40f86 commit3fb5d0b

File tree

7 files changed

+251
-35
lines changed

7 files changed

+251
-35
lines changed

‎agent/agentcontainers/api.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"cdr.dev/slog"
2525
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
2626
"github.com/coder/coder/v2/agent/agentexec"
27+
"github.com/coder/coder/v2/agent/usershell"
2728
"github.com/coder/coder/v2/coderd/httpapi"
2829
"github.com/coder/coder/v2/codersdk"
2930
"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -57,6 +58,7 @@ type API struct {
5758
logger slog.Logger
5859
watcher watcher.Watcher
5960
execer agentexec.Execer
61+
commandEnvCommandEnv
6062
ccliContainerCLI
6163
containerLabelIncludeFiltermap[string]string// Labels to filter containers by.
6264
dccliDevcontainerCLI
@@ -109,6 +111,29 @@ func WithExecer(execer agentexec.Execer) Option {
109111
}
110112
}
111113

114+
// WithCommandEnv sets the CommandEnv implementation to use.
115+
funcWithCommandEnv(ceCommandEnv)Option {
116+
returnfunc(api*API) {
117+
api.commandEnv=func(ei usershell.EnvInfoer,preEnv []string) (string,string, []string,error) {
118+
shell,dir,env,err:=ce(ei,preEnv)
119+
iferr!=nil {
120+
returnshell,dir,env,err
121+
}
122+
env=slices.DeleteFunc(env,func(sstring)bool {
123+
// Ensure we filter out environment variables that come
124+
// from the parent agent and are incorrect or not
125+
// relevant for the devcontainer.
126+
returnstrings.HasPrefix(s,"CODER_WORKSPACE_AGENT_NAME=")||
127+
strings.HasPrefix(s,"CODER_WORKSPACE_AGENT_URL=")||
128+
strings.HasPrefix(s,"CODER_AGENT_TOKEN=")||
129+
strings.HasPrefix(s,"CODER_AGENT_AUTH=")||
130+
strings.HasPrefix(s,"CODER_AGENT_DEVCONTAINERS_ENABLE=")
131+
})
132+
returnshell,dir,env,nil
133+
}
134+
}
135+
}
136+
112137
// WithContainerCLI sets the agentcontainers.ContainerCLI implementation
113138
// to use. The default implementation uses the Docker CLI.
114139
funcWithContainerCLI(ccliContainerCLI)Option {
@@ -151,7 +176,7 @@ func WithSubAgentURL(url string) Option {
151176
}
152177
}
153178

154-
//WithSubAgent sets the environment variables for the sub-agent.
179+
//WithSubAgentEnv sets the environment variables for the sub-agent.
155180
funcWithSubAgentEnv(env...string)Option {
156181
returnfunc(api*API) {
157182
api.subAgentEnv=env
@@ -259,6 +284,13 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
259284
for_,opt:=rangeoptions {
260285
opt(api)
261286
}
287+
ifapi.commandEnv!=nil {
288+
api.execer=newCommandEnvExecer(
289+
api.logger,
290+
api.commandEnv,
291+
api.execer,
292+
)
293+
}
262294
ifapi.ccli==nil {
263295
api.ccli=NewDockerCLI(api.execer)
264296
}

‎agent/agentcontainers/api_test.go

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http"
88
"net/http/httptest"
99
"os"
10+
"os/exec"
1011
"runtime"
1112
"strings"
1213
"sync"
@@ -26,7 +27,9 @@ import (
2627
"github.com/coder/coder/v2/agent/agentcontainers"
2728
"github.com/coder/coder/v2/agent/agentcontainers/acmock"
2829
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
30+
"github.com/coder/coder/v2/agent/usershell"
2931
"github.com/coder/coder/v2/codersdk"
32+
"github.com/coder/coder/v2/pty"
3033
"github.com/coder/coder/v2/testutil"
3134
"github.com/coder/quartz"
3235
)
@@ -291,6 +294,38 @@ func (m *fakeSubAgentClient) Delete(ctx context.Context, id uuid.UUID) error {
291294
returnnil
292295
}
293296

297+
// fakeExecer implements agentexec.Execer for testing and tracks execution details.
298+
typefakeExecerstruct {
299+
commands [][]string
300+
createdCommands []*exec.Cmd
301+
}
302+
303+
func (f*fakeExecer)CommandContext(ctx context.Context,cmdstring,args...string)*exec.Cmd {
304+
f.commands=append(f.commands,append([]string{cmd},args...))
305+
// Create a command that returns empty JSON for docker commands.
306+
c:=exec.CommandContext(ctx,"echo","[]")
307+
f.createdCommands=append(f.createdCommands,c)
308+
returnc
309+
}
310+
311+
func (f*fakeExecer)PTYCommandContext(ctx context.Context,cmdstring,args...string)*pty.Cmd {
312+
f.commands=append(f.commands,append([]string{cmd},args...))
313+
return&pty.Cmd{
314+
Context:ctx,
315+
Path:cmd,
316+
Args:append([]string{cmd},args...),
317+
Env: []string{},
318+
Dir:"",
319+
}
320+
}
321+
322+
func (f*fakeExecer)getLastCommand()*exec.Cmd {
323+
iflen(f.createdCommands)==0 {
324+
returnnil
325+
}
326+
returnf.createdCommands[len(f.createdCommands)-1]
327+
}
328+
294329
funcTestAPI(t*testing.T) {
295330
t.Parallel()
296331

@@ -1970,6 +2005,57 @@ func TestAPI(t *testing.T) {
19702005
// Then: We expected it to succeed
19712006
require.Len(t,fSAC.created,1)
19722007
})
2008+
2009+
t.Run("CommandEnv",func(t*testing.T) {
2010+
t.Parallel()
2011+
2012+
ctx:=testutil.Context(t,testutil.WaitShort)
2013+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true}).Leveled(slog.LevelDebug)
2014+
2015+
// Create fake execer to track execution details.
2016+
fakeExec:=&fakeExecer{}
2017+
2018+
// Custom CommandEnv that returns specific values.
2019+
testShell:="/bin/custom-shell"
2020+
testDir:=t.TempDir()
2021+
testEnv:= []string{"CUSTOM_VAR=test_value","PATH=/custom/path"}
2022+
2023+
commandEnv:=func(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror) {
2024+
returntestShell,testDir,testEnv,nil
2025+
}
2026+
2027+
mClock:=quartz.NewMock(t)// Stop time.
2028+
2029+
// Create API with CommandEnv.
2030+
api:=agentcontainers.NewAPI(logger,
2031+
agentcontainers.WithClock(mClock),
2032+
agentcontainers.WithExecer(fakeExec),
2033+
agentcontainers.WithCommandEnv(commandEnv),
2034+
)
2035+
deferapi.Close()
2036+
2037+
// Call RefreshContainers directly to trigger CommandEnv usage.
2038+
_=api.RefreshContainers(ctx)// Ignore error since docker commands will fail.
2039+
2040+
// Verify commands were executed through the custom shell and environment.
2041+
require.NotEmpty(t,fakeExec.commands,"commands should be executed")
2042+
2043+
// Want: /bin/custom-shell -c "docker ps --all --quiet --no-trunc"
2044+
require.Equal(t,testShell,fakeExec.commands[0][0],"custom shell should be used")
2045+
ifruntime.GOOS=="windows" {
2046+
require.Equal(t,"/c",fakeExec.commands[0][1],"shell should be called with /c on Windows")
2047+
}else {
2048+
require.Equal(t,"-c",fakeExec.commands[0][1],"shell should be called with -c")
2049+
}
2050+
require.Len(t,fakeExec.commands[0],3,"command should have 3 arguments")
2051+
require.GreaterOrEqual(t,strings.Count(fakeExec.commands[0][2]," "),2,"command/script should have multiple arguments")
2052+
2053+
// Verify the environment was set on the command.
2054+
lastCmd:=fakeExec.getLastCommand()
2055+
require.NotNil(t,lastCmd,"command should be created")
2056+
require.Equal(t,testDir,lastCmd.Dir,"custom directory should be used")
2057+
require.Equal(t,testEnv,lastCmd.Env,"custom environment should be used")
2058+
})
19732059
}
19742060

19752061
// mustFindDevcontainerByPath returns the devcontainer with the given workspace

‎agent/agentcontainers/devcontainercli.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"encoding/json"
88
"errors"
99
"io"
10-
"os"
1110

1211
"golang.org/x/xerrors"
1312

@@ -280,7 +279,6 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi
280279
}
281280

282281
c:=d.execer.CommandContext(ctx,"devcontainer",args...)
283-
c.Env=append(c.Env,"PATH="+os.Getenv("PATH"))
284282
c.Env=append(c.Env,env...)
285283

286284
varstdoutBuf bytes.Buffer

‎agent/agentcontainers/execer.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package agentcontainers
2+
3+
import (
4+
"context"
5+
"os/exec"
6+
"runtime"
7+
8+
"github.com/kballard/go-shellquote"
9+
10+
"cdr.dev/slog"
11+
"github.com/coder/coder/v2/agent/agentexec"
12+
"github.com/coder/coder/v2/agent/usershell"
13+
"github.com/coder/coder/v2/pty"
14+
)
15+
16+
// CommandEnv is a function that returns the shell, working directory,
17+
// and environment variables to use when executing a command. It takes
18+
// an EnvInfoer and a pre-existing environment slice as arguments.
19+
// This signature matches agentssh.Server.CommandEnv.
20+
typeCommandEnvfunc(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror)
21+
22+
// commandEnvExecer is an agentexec.Execer that uses a CommandEnv to
23+
// determine the shell, working directory, and environment variables
24+
// for commands. It wraps another agentexec.Execer to provide the
25+
// necessary context.
26+
typecommandEnvExecerstruct {
27+
logger slog.Logger
28+
commandEnvCommandEnv
29+
execer agentexec.Execer
30+
}
31+
32+
funcnewCommandEnvExecer(
33+
logger slog.Logger,
34+
commandEnvCommandEnv,
35+
execer agentexec.Execer,
36+
)*commandEnvExecer {
37+
return&commandEnvExecer{
38+
logger:logger,
39+
commandEnv:commandEnv,
40+
execer:execer,
41+
}
42+
}
43+
44+
// Ensure commandEnvExecer implements agentexec.Execer.
45+
var_ agentexec.Execer= (*commandEnvExecer)(nil)
46+
47+
func (e*commandEnvExecer)prepare(ctx context.Context,inNamestring,inArgs...string) (namestring,args []string,dirstring,env []string) {
48+
shell,dir,env,err:=e.commandEnv(nil,nil)
49+
iferr!=nil {
50+
e.logger.Error(ctx,"get command environment failed",slog.Error(err))
51+
returninName,inArgs,"",nil
52+
}
53+
54+
caller:="-c"
55+
ifruntime.GOOS=="windows" {
56+
caller="/c"
57+
}
58+
name=shell
59+
args= []string{caller,shellquote.Join(append([]string{inName},inArgs...)...)}
60+
returnname,args,dir,env
61+
}
62+
63+
func (e*commandEnvExecer)CommandContext(ctx context.Context,cmdstring,args...string)*exec.Cmd {
64+
name,args,dir,env:=e.prepare(ctx,cmd,args...)
65+
c:=e.execer.CommandContext(ctx,name,args...)
66+
c.Dir=dir
67+
c.Env=env
68+
returnc
69+
}
70+
71+
func (e*commandEnvExecer)PTYCommandContext(ctx context.Context,cmdstring,args...string)*pty.Cmd {
72+
name,args,dir,env:=e.prepare(ctx,cmd,args...)
73+
c:=e.execer.PTYCommandContext(ctx,name,args...)
74+
c.Dir=dir
75+
c.Env=env
76+
returnc
77+
}

‎agent/agentssh/agentssh.go

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,49 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) error {
816816
returnxerrors.Errorf("sftp server closed with error: %w",err)
817817
}
818818

819+
func (s*Server)CommandEnv(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror) {
820+
ifei==nil {
821+
ei=&usershell.SystemEnvInfo{}
822+
}
823+
824+
currentUser,err:=ei.User()
825+
iferr!=nil {
826+
return"","",nil,xerrors.Errorf("get current user: %w",err)
827+
}
828+
username:=currentUser.Username
829+
830+
shell,err=ei.Shell(username)
831+
iferr!=nil {
832+
return"","",nil,xerrors.Errorf("get user shell: %w",err)
833+
}
834+
835+
dir=s.config.WorkingDirectory()
836+
837+
// If the metadata directory doesn't exist, we run the command
838+
// in the users home directory.
839+
_,err=os.Stat(dir)
840+
ifdir==""||err!=nil {
841+
// Default to user home if a directory is not set.
842+
homedir,err:=ei.HomeDir()
843+
iferr!=nil {
844+
return"","",nil,xerrors.Errorf("get home dir: %w",err)
845+
}
846+
dir=homedir
847+
}
848+
env=append(ei.Environ(),addEnv...)
849+
// Set login variables (see `man login`).
850+
env=append(env,fmt.Sprintf("USER=%s",username))
851+
env=append(env,fmt.Sprintf("LOGNAME=%s",username))
852+
env=append(env,fmt.Sprintf("SHELL=%s",shell))
853+
854+
env,err=s.config.UpdateEnv(env)
855+
iferr!=nil {
856+
return"","",nil,xerrors.Errorf("apply env: %w",err)
857+
}
858+
859+
returnshell,dir,env,nil
860+
}
861+
819862
// CreateCommand processes raw command input with OpenSSH-like behavior.
820863
// If the script provided is empty, it will default to the users shell.
821864
// This injects environment variables specified by the user at launch too.
@@ -827,15 +870,10 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
827870
ifei==nil {
828871
ei=&usershell.SystemEnvInfo{}
829872
}
830-
currentUser,err:=ei.User()
831-
iferr!=nil {
832-
returnnil,xerrors.Errorf("get current user: %w",err)
833-
}
834-
username:=currentUser.Username
835873

836-
shell,err:=ei.Shell(username)
874+
shell,dir,env,err:=s.CommandEnv(ei,env)
837875
iferr!=nil {
838-
returnnil,xerrors.Errorf("get user shell: %w",err)
876+
returnnil,xerrors.Errorf("prepare command env: %w",err)
839877
}
840878

841879
// OpenSSH executes all commands with the users current shell.
@@ -893,24 +931,8 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
893931
)
894932
}
895933
cmd:=s.Execer.PTYCommandContext(ctx,modifiedName,modifiedArgs...)
896-
cmd.Dir=s.config.WorkingDirectory()
897-
898-
// If the metadata directory doesn't exist, we run the command
899-
// in the users home directory.
900-
_,err=os.Stat(cmd.Dir)
901-
ifcmd.Dir==""||err!=nil {
902-
// Default to user home if a directory is not set.
903-
homedir,err:=ei.HomeDir()
904-
iferr!=nil {
905-
returnnil,xerrors.Errorf("get home dir: %w",err)
906-
}
907-
cmd.Dir=homedir
908-
}
909-
cmd.Env=append(ei.Environ(),env...)
910-
// Set login variables (see `man login`).
911-
cmd.Env=append(cmd.Env,fmt.Sprintf("USER=%s",username))
912-
cmd.Env=append(cmd.Env,fmt.Sprintf("LOGNAME=%s",username))
913-
cmd.Env=append(cmd.Env,fmt.Sprintf("SHELL=%s",shell))
934+
cmd.Dir=dir
935+
cmd.Env=env
914936

915937
// Set SSH connection environment variables (these are also set by OpenSSH
916938
// and thus expected to be present by SSH clients). Since the agent does
@@ -921,11 +943,6 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
921943
cmd.Env=append(cmd.Env,fmt.Sprintf("SSH_CLIENT=%s %s %s",srcAddr,srcPort,dstPort))
922944
cmd.Env=append(cmd.Env,fmt.Sprintf("SSH_CONNECTION=%s %s %s %s",srcAddr,srcPort,dstAddr,dstPort))
923945

924-
cmd.Env,err=s.config.UpdateEnv(cmd.Env)
925-
iferr!=nil {
926-
returnnil,xerrors.Errorf("apply env: %w",err)
927-
}
928-
929946
returncmd,nil
930947
}
931948

‎agent/api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e
4343
ifa.experimentalDevcontainersEnabled {
4444
containerAPIOpts:= []agentcontainers.Option{
4545
agentcontainers.WithExecer(a.execer),
46+
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
4647
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
4748
returna.logSender.GetScriptLogger(logSourceID)
4849
}),

0 commit comments

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