mirror of
https://github.com/coder/coder.git
synced 2026-09-22 05:05:20 +08:00
feat(agent): unify session env via EnvInfoer (#26099)
The agent resolved the session home directory two ways. agentssh went through usershell, while agentproc called os.UserHomeDir directly and skipped its user.Current fallback. Routing both through a single usershell.EnvInfoer makes the resolution consistent, and agentproc now gets the same fallback as the rest of the agent. The shared seam is injectable, so SSH session tests can drive environment resolution without touching real system state.
This commit is contained in:
+9
-2
@@ -52,6 +52,7 @@ import (
|
||||
"github.com/coder/coder/v2/agent/proto"
|
||||
"github.com/coder/coder/v2/agent/proto/resourcesmonitor"
|
||||
"github.com/coder/coder/v2/agent/reconnectingpty"
|
||||
"github.com/coder/coder/v2/agent/usershell"
|
||||
"github.com/coder/coder/v2/agent/x/agentdesktop"
|
||||
"github.com/coder/coder/v2/agent/x/agentmcp"
|
||||
"github.com/coder/coder/v2/buildinfo"
|
||||
@@ -89,7 +90,10 @@ type Options struct {
|
||||
Client Client
|
||||
ReconnectingPTYTimeout time.Duration
|
||||
EnvironmentVariables map[string]string
|
||||
Logger slog.Logger
|
||||
// EnvInfo overrides the session command environment source. Only
|
||||
// tests set this. Nil defaults to usershell.SystemEnvInfo.
|
||||
EnvInfo usershell.EnvInfoer
|
||||
Logger slog.Logger
|
||||
// IgnorePorts tells the api handler which ports to ignore when
|
||||
// listing all listening ports. This is helpful to hide ports that
|
||||
// are used by the agent, that the user does not care about.
|
||||
@@ -226,6 +230,7 @@ func New(options Options) Agent {
|
||||
statsReportInterval: options.StatsReportInterval,
|
||||
announcementBannersRefreshInterval: options.ServiceBannerRefreshInterval,
|
||||
sshMaxTimeout: options.SSHMaxTimeout,
|
||||
envInfo: options.EnvInfo,
|
||||
subsystems: options.Subsystems,
|
||||
logSender: agentsdk.NewLogSender(options.Logger),
|
||||
blockFileTransfer: options.BlockFileTransfer,
|
||||
@@ -303,6 +308,7 @@ type agent struct {
|
||||
announcementBannersRefreshInterval time.Duration
|
||||
sshServer *agentssh.Server
|
||||
sshMaxTimeout time.Duration
|
||||
envInfo usershell.EnvInfoer
|
||||
blockFileTransfer bool
|
||||
blockReversePortForwarding bool
|
||||
blockLocalPortForwarding bool
|
||||
@@ -365,6 +371,7 @@ func (a *agent) init() {
|
||||
AnnouncementBanners: func() *[]codersdk.BannerConfig { return a.announcementBanners.Load() },
|
||||
UpdateEnv: a.updateCommandEnv,
|
||||
WorkingDirectory: func() string { return a.manifest.Load().Directory },
|
||||
EnvInfo: a.envInfo,
|
||||
BlockFileTransfer: a.blockFileTransfer,
|
||||
BlockReversePortForwarding: a.blockReversePortForwarding,
|
||||
BlockLocalPortForwarding: a.blockLocalPortForwarding,
|
||||
@@ -420,7 +427,7 @@ func (a *agent) init() {
|
||||
|
||||
pathStore := agentgit.NewPathStore()
|
||||
a.filesAPI = agentfiles.NewAPI(a.logger.Named("files"), a.filesystem, pathStore)
|
||||
a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, a.updateCommandEnv, pathStore, func() string {
|
||||
a.processAPI = agentproc.NewAPI(a.logger.Named("processes"), a.execer, pathStore, a.envInfo, a.updateCommandEnv, func() string {
|
||||
if m := a.manifest.Load(); m != nil {
|
||||
return m.Directory
|
||||
}
|
||||
|
||||
@@ -1741,6 +1741,43 @@ func TestAgent_SSHConnectionLoginVars(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgent_SSHEnvInfoShell verifies that an agent.Options.EnvInfo whose
|
||||
// Shell() reports a custom shell is piped through to the SSH session, so the
|
||||
// session command runs under that shell instead of the host default.
|
||||
func TestAgent_SSHEnvInfoShell(t *testing.T) {
|
||||
t.Parallel()
|
||||
if runtime.GOOS == "windows" {
|
||||
t.Skip("the fake shell is a POSIX script")
|
||||
}
|
||||
|
||||
// A fake shell that ignores its arguments and prints a sentinel. The
|
||||
// sentinel only appears in the session output if the injected Shell() was
|
||||
// honored. Otherwise the command's own output ("should-not-run") appears.
|
||||
const marker = "injected-shell-was-used"
|
||||
shellPath := filepath.Join(t.TempDir(), "fakeshell")
|
||||
//nolint:gosec // Executable test shell with test-controlled content.
|
||||
err := os.WriteFile(shellPath, []byte("#!/bin/sh\necho "+marker+"\n"), 0o700)
|
||||
require.NoError(t, err)
|
||||
|
||||
session := setupSSHSession(t, agentsdk.Manifest{}, codersdk.ServiceBannerConfig{}, nil, func(_ *agenttest.Client, o *agent.Options) {
|
||||
o.EnvInfo = shellOverrideEnvInfo{shell: shellPath}
|
||||
})
|
||||
|
||||
output, err := session.Output("echo should-not-run")
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, string(output), marker)
|
||||
require.NotContains(t, string(output), "should-not-run")
|
||||
}
|
||||
|
||||
// shellOverrideEnvInfo is a usershell.EnvInfoer that delegates to the system
|
||||
// implementation but reports a custom shell.
|
||||
type shellOverrideEnvInfo struct {
|
||||
usershell.SystemEnvInfo
|
||||
shell string
|
||||
}
|
||||
|
||||
func (e shellOverrideEnvInfo) Shell(string) (string, error) { return e.shell, nil }
|
||||
|
||||
func TestAgent_Metadata(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -16,6 +16,7 @@ import (
|
||||
"github.com/coder/coder/v2/agent/agentchat"
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/agent/agentgit"
|
||||
"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/workspacesdk"
|
||||
@@ -36,10 +37,10 @@ type API struct {
|
||||
}
|
||||
|
||||
// NewAPI creates a new process API handler.
|
||||
func NewAPI(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), pathStore *agentgit.PathStore, workingDir func() string) *API {
|
||||
func NewAPI(logger slog.Logger, execer agentexec.Execer, pathStore *agentgit.PathStore, envInfo usershell.EnvInfoer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *API {
|
||||
return &API{
|
||||
logger: logger,
|
||||
manager: newManager(logger, execer, updateEnv, workingDir),
|
||||
manager: newManager(logger, execer, envInfo, updateEnv, workingDir),
|
||||
pathStore: pathStore,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -24,6 +25,7 @@ import (
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/agent/agentgit"
|
||||
"github.com/coder/coder/v2/agent/agentproc"
|
||||
"github.com/coder/coder/v2/agent/usershell"
|
||||
"github.com/coder/coder/v2/coderd/httpmw/loggermw"
|
||||
"github.com/coder/coder/v2/coderd/tracing"
|
||||
"github.com/coder/coder/v2/codersdk"
|
||||
@@ -136,19 +138,43 @@ func newTestAPIWithOptions(t *testing.T, updateEnv func([]string) ([]string, err
|
||||
logger := slogtest.Make(t, &slogtest.Options{
|
||||
IgnoreErrors: true,
|
||||
}).Leveled(slog.LevelDebug)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, updateEnv, nil, workingDir)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, updateEnv, workingDir)
|
||||
t.Cleanup(func() {
|
||||
_ = api.Close()
|
||||
})
|
||||
return agentchat.Middleware(api.Routes())
|
||||
}
|
||||
|
||||
// newTestAPIWithEnvInfo creates a new API with an injected EnvInfoer
|
||||
// and an optional workingDir hook.
|
||||
func newTestAPIWithEnvInfo(t *testing.T, workingDir func() string, envInfo usershell.EnvInfoer) http.Handler {
|
||||
t.Helper()
|
||||
|
||||
logger := slogtest.Make(t, &slogtest.Options{
|
||||
IgnoreErrors: true,
|
||||
}).Leveled(slog.LevelDebug)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, envInfo, nil, workingDir)
|
||||
t.Cleanup(func() {
|
||||
_ = api.Close()
|
||||
})
|
||||
return agentchat.Middleware(api.Routes())
|
||||
}
|
||||
|
||||
// homeOverrideEnvInfo is a usershell.EnvInfoer that delegates to the
|
||||
// system implementation but reports a custom home directory.
|
||||
type homeOverrideEnvInfo struct {
|
||||
usershell.SystemEnvInfo
|
||||
home string
|
||||
}
|
||||
|
||||
func (e homeOverrideEnvInfo) HomeDir() (string, error) { return e.home, nil }
|
||||
|
||||
func TestAccessLogIncludesChatID(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
sink := testutil.NewFakeSink(t)
|
||||
logger := sink.Logger()
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, nil, nil, nil, nil)
|
||||
t.Cleanup(func() {
|
||||
_ = api.Close()
|
||||
})
|
||||
@@ -403,6 +429,40 @@ func TestStartProcess(t *testing.T) {
|
||||
require.Equal(t, homeDir, proc.WorkDir)
|
||||
})
|
||||
|
||||
t.Run("DefaultWorkDirUsesInjectedEnvInfoHome", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// With no explicit or configured directory available,
|
||||
// the home fallback must come from the injected EnvInfo
|
||||
// rather than the real user home.
|
||||
homeDir := t.TempDir()
|
||||
handler := newTestAPIWithEnvInfo(t, func() string {
|
||||
return filepath.Join(t.TempDir(), "nonexistent")
|
||||
}, homeOverrideEnvInfo{home: homeDir})
|
||||
|
||||
id := startAndGetID(t, handler, workspacesdk.StartProcessRequest{
|
||||
Command: "echo ok",
|
||||
})
|
||||
|
||||
resp := waitForExit(t, handler, id)
|
||||
require.NotNil(t, resp.ExitCode)
|
||||
require.Equal(t, 0, *resp.ExitCode)
|
||||
|
||||
w := getList(t, handler)
|
||||
require.Equal(t, http.StatusOK, w.Code)
|
||||
var listResp workspacesdk.ListProcessesResponse
|
||||
require.NoError(t, json.NewDecoder(w.Body).Decode(&listResp))
|
||||
var proc *workspacesdk.ProcessInfo
|
||||
for i := range listResp.Processes {
|
||||
if listResp.Processes[i].ID == id {
|
||||
proc = &listResp.Processes[i]
|
||||
break
|
||||
}
|
||||
}
|
||||
require.NotNil(t, proc, "process not found in list")
|
||||
require.Equal(t, homeDir, proc.WorkDir)
|
||||
})
|
||||
|
||||
t.Run("CustomEnv", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -1084,9 +1144,9 @@ func TestHandleStartProcess_ChatHeaders_EmptyWorkDir_StillNotifies(t *testing.T)
|
||||
defer unsub()
|
||||
|
||||
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, func(current []string) ([]string, error) {
|
||||
api := agentproc.NewAPI(logger, agentexec.DefaultExecer, pathStore, nil, func(current []string) ([]string, error) {
|
||||
return current, nil
|
||||
}, pathStore, nil)
|
||||
}, nil)
|
||||
defer api.Close()
|
||||
|
||||
routes := agentchat.Middleware(api.Routes())
|
||||
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
|
||||
"cdr.dev/slog/v3"
|
||||
"github.com/coder/coder/v2/agent/agentexec"
|
||||
"github.com/coder/coder/v2/agent/usershell"
|
||||
"github.com/coder/coder/v2/codersdk/workspacesdk"
|
||||
"github.com/coder/quartz"
|
||||
)
|
||||
@@ -79,10 +80,14 @@ type manager struct {
|
||||
closed bool
|
||||
updateEnv func(current []string) (updated []string, err error)
|
||||
workingDir func() string
|
||||
envInfo usershell.EnvInfoer
|
||||
}
|
||||
|
||||
// newManager creates a new process manager.
|
||||
func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager {
|
||||
func newManager(logger slog.Logger, execer agentexec.Execer, envInfo usershell.EnvInfoer, updateEnv func(current []string) (updated []string, err error), workingDir func() string) *manager {
|
||||
if envInfo == nil {
|
||||
envInfo = &usershell.SystemEnvInfo{}
|
||||
}
|
||||
return &manager{
|
||||
logger: logger,
|
||||
execer: execer,
|
||||
@@ -90,6 +95,7 @@ func newManager(logger slog.Logger, execer agentexec.Execer, updateEnv func(curr
|
||||
procs: make(map[string]*process),
|
||||
updateEnv: updateEnv,
|
||||
workingDir: workingDir,
|
||||
envInfo: envInfo,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -379,7 +385,7 @@ func (m *manager) resolveWorkDir(requested string) string {
|
||||
}
|
||||
}
|
||||
}
|
||||
if home, err := os.UserHomeDir(); err == nil {
|
||||
if home, err := m.envInfo.HomeDir(); err == nil {
|
||||
return home
|
||||
}
|
||||
return ""
|
||||
|
||||
@@ -107,6 +107,10 @@ type Config struct {
|
||||
// where users will land when they connect via SSH. Default is the home
|
||||
// directory of the user.
|
||||
WorkingDirectory func() string
|
||||
// EnvInfo sources the session command environment. Default is
|
||||
// usershell.SystemEnvInfo. A container override still applies per
|
||||
// session when ExperimentalContainers is enabled.
|
||||
EnvInfo usershell.EnvInfoer
|
||||
// X11DisplayOffset is the offset to add to the X11 display number.
|
||||
// Default is 10.
|
||||
X11DisplayOffset *int
|
||||
@@ -189,6 +193,9 @@ func NewServer(ctx context.Context, logger slog.Logger, prometheusRegistry *prom
|
||||
return home
|
||||
}
|
||||
}
|
||||
if config.EnvInfo == nil {
|
||||
config.EnvInfo = &usershell.SystemEnvInfo{}
|
||||
}
|
||||
if config.ReportConnection == nil {
|
||||
config.ReportConnection = func(uuid.UUID, MagicSessionType, string) func(int, string) { return func(int, string) {} }
|
||||
}
|
||||
@@ -619,7 +626,7 @@ func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []str
|
||||
ptyLabel = "yes"
|
||||
}
|
||||
|
||||
var ei usershell.EnvInfoer
|
||||
ei := s.config.EnvInfo
|
||||
var err error
|
||||
if s.config.ExperimentalContainers && container != "" {
|
||||
ei, err = agentcontainers.EnvInfo(ctx, s.Execer, container, containerUser)
|
||||
|
||||
Reference in New Issue
Block a user