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

fix(agent/agentcontainers): use correct env for execer commands#18508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
mafredri merged 1 commit intomainfrommafredri/fix-agent-agentcontainers-env
Jun 24, 2025
Merged
Show file tree
Hide file tree
Changes fromall commits
Commits
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletionagent/agentcontainers/api.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -23,6 +23,7 @@ import (
"cdr.dev/slog"
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
"github.com/coder/coder/v2/agent/agentexec"
"github.com/coder/coder/v2/agent/usershell"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/codersdk/agentsdk"
Expand DownExpand Up@@ -54,6 +55,7 @@ type API struct {
logger slog.Logger
watcher watcher.Watcher
execer agentexec.Execer
commandEnvCommandEnv
ccliContainerCLI
containerLabelIncludeFiltermap[string]string// Labels to filter containers by.
dccliDevcontainerCLI
Expand DownExpand Up@@ -106,6 +108,29 @@ func WithExecer(execer agentexec.Execer) Option {
}
}

// WithCommandEnv sets the CommandEnv implementation to use.
funcWithCommandEnv(ceCommandEnv)Option {
returnfunc(api*API) {
api.commandEnv=func(ei usershell.EnvInfoer,preEnv []string) (string,string, []string,error) {
shell,dir,env,err:=ce(ei,preEnv)
iferr!=nil {
returnshell,dir,env,err
}
env=slices.DeleteFunc(env,func(sstring)bool {
// Ensure we filter out environment variables that come
// from the parent agent and are incorrect or not
// relevant for the devcontainer.
returnstrings.HasPrefix(s,"CODER_WORKSPACE_AGENT_NAME=")||
strings.HasPrefix(s,"CODER_WORKSPACE_AGENT_URL=")||
strings.HasPrefix(s,"CODER_AGENT_TOKEN=")||
strings.HasPrefix(s,"CODER_AGENT_AUTH=")||
strings.HasPrefix(s,"CODER_AGENT_DEVCONTAINERS_ENABLE=")
})
returnshell,dir,env,nil
}
}
}

// WithContainerCLI sets the agentcontainers.ContainerCLI implementation
// to use. The default implementation uses the Docker CLI.
funcWithContainerCLI(ccliContainerCLI)Option {
Expand DownExpand Up@@ -148,7 +173,7 @@ func WithSubAgentURL(url string) Option {
}
}

//WithSubAgent sets the environment variables for the sub-agent.
//WithSubAgentEnv sets the environment variables for the sub-agent.
funcWithSubAgentEnv(env...string)Option {
returnfunc(api*API) {
api.subAgentEnv=env
Expand DownExpand Up@@ -256,6 +281,13 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
for_,opt:=rangeoptions {
opt(api)
}
ifapi.commandEnv!=nil {
api.execer=newCommandEnvExecer(
api.logger,
api.commandEnv,
api.execer,
)
}
ifapi.ccli==nil {
api.ccli=NewDockerCLI(api.execer)
}
Expand Down
86 changes: 86 additions & 0 deletionsagent/agentcontainers/api_test.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,7 @@ import (
"net/http"
"net/http/httptest"
"os"
"os/exec"
"runtime"
"strings"
"sync"
Expand All@@ -26,7 +27,9 @@ import (
"github.com/coder/coder/v2/agent/agentcontainers"
"github.com/coder/coder/v2/agent/agentcontainers/acmock"
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
"github.com/coder/coder/v2/agent/usershell"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/coder/v2/pty"
"github.com/coder/coder/v2/testutil"
"github.com/coder/quartz"
)
Expand DownExpand Up@@ -291,6 +294,38 @@ func (m *fakeSubAgentClient) Delete(ctx context.Context, id uuid.UUID) error {
returnnil
}

// fakeExecer implements agentexec.Execer for testing and tracks execution details.
typefakeExecerstruct {
commands [][]string
createdCommands []*exec.Cmd
}

func (f*fakeExecer)CommandContext(ctx context.Context,cmdstring,args...string)*exec.Cmd {
f.commands=append(f.commands,append([]string{cmd},args...))
// Create a command that returns empty JSON for docker commands.
c:=exec.CommandContext(ctx,"echo","[]")
f.createdCommands=append(f.createdCommands,c)
returnc
}

func (f*fakeExecer)PTYCommandContext(ctx context.Context,cmdstring,args...string)*pty.Cmd {
f.commands=append(f.commands,append([]string{cmd},args...))
return&pty.Cmd{
Context:ctx,
Path:cmd,
Args:append([]string{cmd},args...),
Env: []string{},
Dir:"",
}
}

func (f*fakeExecer)getLastCommand()*exec.Cmd {
iflen(f.createdCommands)==0 {
returnnil
}
returnf.createdCommands[len(f.createdCommands)-1]
}

funcTestAPI(t*testing.T) {
t.Parallel()

Expand DownExpand Up@@ -1970,6 +2005,57 @@ func TestAPI(t *testing.T) {
// Then: We expected it to succeed
require.Len(t,fSAC.created,1)
})

t.Run("CommandEnv",func(t*testing.T) {
t.Parallel()

ctx:=testutil.Context(t,testutil.WaitShort)
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true}).Leveled(slog.LevelDebug)

// Create fake execer to track execution details.
fakeExec:=&fakeExecer{}

// Custom CommandEnv that returns specific values.
testShell:="/bin/custom-shell"
testDir:=t.TempDir()
testEnv:= []string{"CUSTOM_VAR=test_value","PATH=/custom/path"}

commandEnv:=func(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror) {
returntestShell,testDir,testEnv,nil
}

mClock:=quartz.NewMock(t)// Stop time.

// Create API with CommandEnv.
api:=agentcontainers.NewAPI(logger,
agentcontainers.WithClock(mClock),
agentcontainers.WithExecer(fakeExec),
agentcontainers.WithCommandEnv(commandEnv),
)
deferapi.Close()

// Call RefreshContainers directly to trigger CommandEnv usage.
_=api.RefreshContainers(ctx)// Ignore error since docker commands will fail.

// Verify commands were executed through the custom shell and environment.
require.NotEmpty(t,fakeExec.commands,"commands should be executed")

// Want: /bin/custom-shell -c "docker ps --all --quiet --no-trunc"
require.Equal(t,testShell,fakeExec.commands[0][0],"custom shell should be used")
ifruntime.GOOS=="windows" {
require.Equal(t,"/c",fakeExec.commands[0][1],"shell should be called with /c on Windows")
}else {
require.Equal(t,"-c",fakeExec.commands[0][1],"shell should be called with -c")
}
require.Len(t,fakeExec.commands[0],3,"command should have 3 arguments")
require.GreaterOrEqual(t,strings.Count(fakeExec.commands[0][2]," "),2,"command/script should have multiple arguments")

// Verify the environment was set on the command.
lastCmd:=fakeExec.getLastCommand()
require.NotNil(t,lastCmd,"command should be created")
require.Equal(t,testDir,lastCmd.Dir,"custom directory should be used")
require.Equal(t,testEnv,lastCmd.Env,"custom environment should be used")
})
}

// mustFindDevcontainerByPath returns the devcontainer with the given workspace
Expand Down
2 changes: 0 additions & 2 deletionsagent/agentcontainers/devcontainercli.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,6 @@ import (
"encoding/json"
"errors"
"io"
"os"

"golang.org/x/xerrors"

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

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

varstdoutBuf bytes.Buffer
Expand Down
77 changes: 77 additions & 0 deletionsagent/agentcontainers/execer.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
package agentcontainers

import (
"context"
"os/exec"
"runtime"

"github.com/kballard/go-shellquote"

"cdr.dev/slog"
"github.com/coder/coder/v2/agent/agentexec"
"github.com/coder/coder/v2/agent/usershell"
"github.com/coder/coder/v2/pty"
)

// CommandEnv is a function that returns the shell, working directory,
// and environment variables to use when executing a command. It takes
// an EnvInfoer and a pre-existing environment slice as arguments.
// This signature matches agentssh.Server.CommandEnv.
typeCommandEnvfunc(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror)

// commandEnvExecer is an agentexec.Execer that uses a CommandEnv to
// determine the shell, working directory, and environment variables
// for commands. It wraps another agentexec.Execer to provide the
// necessary context.
typecommandEnvExecerstruct {
logger slog.Logger
commandEnvCommandEnv
execer agentexec.Execer
}

funcnewCommandEnvExecer(
logger slog.Logger,
commandEnvCommandEnv,
execer agentexec.Execer,
)*commandEnvExecer {
return&commandEnvExecer{
logger:logger,
commandEnv:commandEnv,
execer:execer,
}
}

// Ensure commandEnvExecer implements agentexec.Execer.
var_ agentexec.Execer= (*commandEnvExecer)(nil)

func (e*commandEnvExecer)prepare(ctx context.Context,inNamestring,inArgs...string) (namestring,args []string,dirstring,env []string) {
shell,dir,env,err:=e.commandEnv(nil,nil)
iferr!=nil {
e.logger.Error(ctx,"get command environment failed",slog.Error(err))
returninName,inArgs,"",nil
}

caller:="-c"
ifruntime.GOOS=="windows" {
caller="/c"
}
name=shell
args= []string{caller,shellquote.Join(append([]string{inName},inArgs...)...)}
returnname,args,dir,env
}

func (e*commandEnvExecer)CommandContext(ctx context.Context,cmdstring,args...string)*exec.Cmd {
name,args,dir,env:=e.prepare(ctx,cmd,args...)
c:=e.execer.CommandContext(ctx,name,args...)
c.Dir=dir
c.Env=env
returnc
}

func (e*commandEnvExecer)PTYCommandContext(ctx context.Context,cmdstring,args...string)*pty.Cmd {
name,args,dir,env:=e.prepare(ctx,cmd,args...)
c:=e.execer.PTYCommandContext(ctx,name,args...)
c.Dir=dir
c.Env=env
returnc
}
77 changes: 47 additions & 30 deletionsagent/agentssh/agentssh.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -816,6 +816,49 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) error {
returnxerrors.Errorf("sftp server closed with error: %w",err)
}

func (s*Server)CommandEnv(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror) {
ifei==nil {
ei=&usershell.SystemEnvInfo{}
}

currentUser,err:=ei.User()
iferr!=nil {
return"","",nil,xerrors.Errorf("get current user: %w",err)
}
username:=currentUser.Username

shell,err=ei.Shell(username)
iferr!=nil {
return"","",nil,xerrors.Errorf("get user shell: %w",err)
}

dir=s.config.WorkingDirectory()

// If the metadata directory doesn't exist, we run the command
// in the users home directory.
_,err=os.Stat(dir)
ifdir==""||err!=nil {
// Default to user home if a directory is not set.
homedir,err:=ei.HomeDir()
iferr!=nil {
return"","",nil,xerrors.Errorf("get home dir: %w",err)
}
dir=homedir
}
env=append(ei.Environ(),addEnv...)
// Set login variables (see `man login`).
env=append(env,fmt.Sprintf("USER=%s",username))
env=append(env,fmt.Sprintf("LOGNAME=%s",username))
env=append(env,fmt.Sprintf("SHELL=%s",shell))

env,err=s.config.UpdateEnv(env)
iferr!=nil {
return"","",nil,xerrors.Errorf("apply env: %w",err)
}

returnshell,dir,env,nil
}

// CreateCommand processes raw command input with OpenSSH-like behavior.
// If the script provided is empty, it will default to the users shell.
// This injects environment variables specified by the user at launch too.
Expand All@@ -827,15 +870,10 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
ifei==nil {
ei=&usershell.SystemEnvInfo{}
}
currentUser,err:=ei.User()
iferr!=nil {
returnnil,xerrors.Errorf("get current user: %w",err)
}
username:=currentUser.Username

shell,err:=ei.Shell(username)
shell,dir,env,err:=s.CommandEnv(ei,env)
iferr!=nil {
returnnil,xerrors.Errorf("get user shell: %w",err)
returnnil,xerrors.Errorf("prepare command env: %w",err)
}

// OpenSSH executes all commands with the users current shell.
Expand DownExpand Up@@ -893,24 +931,8 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
)
}
cmd:=s.Execer.PTYCommandContext(ctx,modifiedName,modifiedArgs...)
cmd.Dir=s.config.WorkingDirectory()

// If the metadata directory doesn't exist, we run the command
// in the users home directory.
_,err=os.Stat(cmd.Dir)
ifcmd.Dir==""||err!=nil {
// Default to user home if a directory is not set.
homedir,err:=ei.HomeDir()
iferr!=nil {
returnnil,xerrors.Errorf("get home dir: %w",err)
}
cmd.Dir=homedir
}
cmd.Env=append(ei.Environ(),env...)
// Set login variables (see `man login`).
cmd.Env=append(cmd.Env,fmt.Sprintf("USER=%s",username))
cmd.Env=append(cmd.Env,fmt.Sprintf("LOGNAME=%s",username))
cmd.Env=append(cmd.Env,fmt.Sprintf("SHELL=%s",shell))
cmd.Dir=dir
cmd.Env=env

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

cmd.Env,err=s.config.UpdateEnv(cmd.Env)
iferr!=nil {
returnnil,xerrors.Errorf("apply env: %w",err)
}

returncmd,nil
}

Expand Down
1 change: 1 addition & 0 deletionsagent/api.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,6 +43,7 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e
ifa.experimentalDevcontainersEnabled {
containerAPIOpts:= []agentcontainers.Option{
agentcontainers.WithExecer(a.execer),
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
returna.logSender.GetScriptLogger(logSourceID)
}),
Expand Down
Loading
Loading
[8]ページ先頭
©2009-2025 Movatter.jp