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

agent/agentcontainers/api.go

@@ -24,6 +24,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"
@@ -57,6 +58,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
@@ -109,6 +111,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 {
@@ -151,7 +176,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
@@ -259,6 +284,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)
 	}

agent/agentcontainers/api_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"runtime"
 	"strings"
 	"sync"
@@ -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"
 )
@@ -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()
 
@@ -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

agent/agentcontainers/devcontainercli.go

@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
-	"os"
 
 	"golang.org/x/xerrors"
 
@@ -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

agent/agentcontainers/execer.go

@@ -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
+}

agent/agentssh/agentssh.go

@@ -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.
@@ -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.
@@ -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()
+	cmd.Dir = dir
-
+	cmd.Env = env
-	// 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))
 
 	// Set SSH connection environment variables (these are also set by OpenSSH
 	// 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,
 	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
 }
 

agent/api.go

@@ -43,6 +43,7 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e
 	if a.experimentalDevcontainersEnabled {
 		containerAPIOpts := []agentcontainers.Option{
 			agentcontainers.WithExecer(a.execer),
+			agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
 			agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
 				return a.logSender.GetScriptLogger(logSourceID)
 			}),

cli/exp_rpty_test.go

@@ -87,6 +87,8 @@ func TestExpRpty(t *testing.T) {
 			t.Skip("Skipping test on non-Linux platform")
 		}
 
+		wantLabel := "coder.devcontainers.TestExpRpty.Container"
+
 		client, workspace, agentToken := setupWorkspaceForAgent(t)
 		ctx := testutil.Context(t, testutil.WaitLong)
 		pool, err := dockertest.NewPool("")
@@ -94,7 +96,10 @@ func TestExpRpty(t *testing.T) {
 		ct, err := pool.RunWithOptions(&dockertest.RunOptions{
 			Repository: "busybox",
 			Tag:        "latest",
-			Cmd:        []string{"sleep", "infnity"},
+			Cmd:        []string{"sleep", "infinity"},
+			Labels: map[string]string{
+				wantLabel: "true",
+			},
 		}, func(config *docker.HostConfig) {
 			config.AutoRemove = true
 			config.RestartPolicy = docker.RestartPolicy{Name: "no"}
@@ -113,7 +118,7 @@ func TestExpRpty(t *testing.T) {
 		_ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) {
 			o.ExperimentalDevcontainersEnabled = true
 			o.ContainerAPIOptions = append(o.ContainerAPIOptions,
-				agentcontainers.WithContainerLabelIncludeFilter("this.label.does.not.exist.ignore.devcontainers", "true"),
+				agentcontainers.WithContainerLabelIncludeFilter(wantLabel, "true"),
 			)
 		})
 		_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()