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
commandEnv CommandEnv
ccli ContainerCLI
containerLabelIncludeFilter map[string]string // Labels to filter containers by.
dccli DevcontainerCLI
Expand DownExpand Up@@ -106,6 +108,29 @@ func WithExecer(execer agentexec.Execer) Option {
}
}

// WithCommandEnv sets the CommandEnv implementation to use.
func WithCommandEnv(ce CommandEnv) Option {
return func(api *API) {
api.commandEnv = func(ei usershell.EnvInfoer, preEnv []string) (string, string, []string, error) {
shell, dir, env, err := ce(ei, preEnv)
if err != nil {
return shell, dir, env, err
}
env = slices.DeleteFunc(env, func(s string) bool {
// Ensure we filter out environment variables that come
// from the parent agent and are incorrect or not
// relevant for the devcontainer.
return strings.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=")
})
return shell, dir, env, nil
}
}
}

// WithContainerCLI sets the agentcontainers.ContainerCLI implementation
// to use. The default implementation uses the Docker CLI.
func WithContainerCLI(ccli ContainerCLI) 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.
func WithSubAgentEnv(env ...string) Option {
return func(api *API) {
api.subAgentEnv = env
Expand DownExpand Up@@ -256,6 +281,13 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
for _, opt := range options {
opt(api)
}
if api.commandEnv != nil {
api.execer = newCommandEnvExecer(
api.logger,
api.commandEnv,
api.execer,
)
}
if api.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 {
return nil
}

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

func (f *fakeExecer) CommandContext(ctx context.Context, cmd string, 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)
return c
}

func (f *fakeExecer) PTYCommandContext(ctx context.Context, cmd string, 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 {
if len(f.createdCommands) == 0 {
return nil
}
return f.createdCommands[len(f.createdCommands)-1]
}

func TestAPI(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, dir string, env []string, err error) {
return testShell, 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),
)
defer api.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")
if runtime.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...)

var stdoutBuf 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.
type CommandEnv func(ei usershell.EnvInfoer, addEnv []string) (shell, dir string, env []string, err error)

// 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.
type commandEnvExecer struct {
logger slog.Logger
commandEnv CommandEnv
execer agentexec.Execer
}

func newCommandEnvExecer(
logger slog.Logger,
commandEnv CommandEnv,
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, inName string, inArgs ...string) (name string, args []string, dir string, env []string) {
shell, dir, env, err := e.commandEnv(nil, nil)
if err != nil {
e.logger.Error(ctx, "get command environment failed", slog.Error(err))
return inName, inArgs, "", nil
}

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

func (e *commandEnvExecer) CommandContext(ctx context.Context, cmd string, 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
return c
}

func (e *commandEnvExecer) PTYCommandContext(ctx context.Context, cmd string, 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
return c
}
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 {
return xerrors.Errorf("sftp server closed with error: %w", err)
}

func (s *Server) CommandEnv(ei usershell.EnvInfoer, addEnv []string) (shell, dir string, env []string, err error) {
if ei == nil {
ei = &usershell.SystemEnvInfo{}
}

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

shell, err = ei.Shell(username)
if err != 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)
if dir == "" || err != nil {
// Default to user home if a directory is not set.
homedir, err := ei.HomeDir()
if err != 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)
if err != nil {
return "", "", nil, xerrors.Errorf("apply env: %w", err)
}

return shell, 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,
if ei == nil {
ei = &usershell.SystemEnvInfo{}
}
currentUser, err := ei.User()
if err != nil {
return nil, xerrors.Errorf("get current user: %w", err)
}
username := currentUser.Username

shell, err :=ei.Shell(username)
shell,dir, env,err :=s.CommandEnv(ei, env)
if err != nil {
return nil, xerrors.Errorf("get user shell: %w", err)
return nil, 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)
if cmd.Dir == "" || err != nil {
// Default to user home if a directory is not set.
homedir, err := ei.HomeDir()
if err != nil {
return nil, 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)
if err != nil {
return nil, xerrors.Errorf("apply env: %w", err)
}

return cmd, 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