From 4479de2f4c9b467cb09ebe043863b3369eb14642 Mon Sep 17 00:00:00 2001 From: kylecarbs Date: Thu, 16 Jun 2022 20:38:11 +0000 Subject: [PATCH 1/2] fix: Start login shells on macOS and Linux This appends `-l` to the shell command on macOS and Linux. It also adds environment variable expansion to allow for chaining from `coder_agent.env`. --- agent/agent.go | 11 +++++++++-- agent/agent_test.go | 33 ++++++++++++++++++--------------- cli/agent.go | 10 ++++++++++ go.mod | 2 -- go.sum | 1 - 5 files changed, 37 insertions(+), 20 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index dbb51445d9793..7615de25270c4 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -328,6 +328,11 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri command := rawCommand if len(command) == 0 { command = shell + if runtime.GOOS != "windows" { + // On Linux and macOS, we should start a login + // shell to consume juicy environment variables! + command += " -l" + } } // OpenSSH executes all commands with the users current shell. @@ -348,7 +353,6 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri return nil, xerrors.Errorf("getting os executable: %w", err) } cmd.Env = append(cmd.Env, fmt.Sprintf("USER=%s", username)) - cmd.Env = append(cmd.Env, fmt.Sprintf(`PATH=%s%c%s`, os.Getenv("PATH"), filepath.ListSeparator, filepath.Dir(executablePath))) // Git on Windows resolves with UNIX-style paths. // If using backslashes, it's unable to find the executable. unixExecutablePath := strings.ReplaceAll(executablePath, "\\", "/") @@ -363,7 +367,10 @@ func (a *agent) createCommand(ctx context.Context, rawCommand string, env []stri // Load environment variables passed via the agent. // These should override all variables we manually specify. for key, value := range metadata.EnvironmentVariables { - cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, value)) + // Expanding environment variables allows for customization + // of the $PATH, among other variables. Customers can prepand + // or append to the $PATH, so allowing expand is required! + cmd.Env = append(cmd.Env, fmt.Sprintf("%s=%s", key, os.ExpandEnv(value))) } // Agent-level environment variables should take over all! diff --git a/agent/agent_test.go b/agent/agent_test.go index 923ce46290b5d..f1d9c6ee22cb8 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -68,21 +68,6 @@ func TestAgent(t *testing.T) { require.True(t, strings.HasSuffix(strings.TrimSpace(string(output)), "gitssh --")) }) - t.Run("PATHHasCoder", func(t *testing.T) { - t.Parallel() - session := setupSSHSession(t, agent.Metadata{}) - command := "sh -c 'echo $PATH'" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo %PATH%" - } - output, err := session.Output(command) - require.NoError(t, err) - ex, err := os.Executable() - t.Log(ex) - require.NoError(t, err) - require.True(t, strings.Contains(strings.TrimSpace(string(output)), filepath.Dir(ex))) - }) - t.Run("SessionTTY", func(t *testing.T) { t.Parallel() if runtime.GOOS == "windows" { @@ -182,6 +167,24 @@ func TestAgent(t *testing.T) { require.Equal(t, value, strings.TrimSpace(string(output))) }) + t.Run("EnvironmentVariableExpansion", func(t *testing.T) { + t.Parallel() + key := "EXAMPLE" + session := setupSSHSession(t, agent.Metadata{ + EnvironmentVariables: map[string]string{ + key: "$SOMETHINGNOTSET", + }, + }) + command := "sh -c 'echo $" + key + "'" + if runtime.GOOS == "windows" { + command = "cmd.exe /c echo %" + key + "%" + } + output, err := session.Output(command) + require.NoError(t, err) + // Output should be empty, because the variable is not set! + require.Equal(t, "", strings.TrimSpace(string(output))) + }) + t.Run("StartupScript", func(t *testing.T) { t.Parallel() tempPath := filepath.Join(os.TempDir(), "content.txt") diff --git a/cli/agent.go b/cli/agent.go index 287f18f7fbefa..0113606d20392 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -2,6 +2,7 @@ package cli import ( "context" + "fmt" "net/http" _ "net/http/pprof" //nolint: gosec "net/url" @@ -138,6 +139,15 @@ func workspaceAgent() *cobra.Command { } } + executablePath, err := os.Executable() + if err != nil { + return xerrors.Errorf("getting os executable: %w", err) + } + err = os.Setenv("PATH", fmt.Sprintf("%s%c%s", os.Getenv("PATH"), filepath.ListSeparator, filepath.Dir(executablePath))) + if err != nil { + return xerrors.Errorf("add executable to $PATH: %w", err) + } + closer := agent.New(client.ListenWorkspaceAgent, &agent.Options{ Logger: logger, EnvironmentVariables: map[string]string{ diff --git a/go.mod b/go.mod index d278dfc669629..cd792574803e1 100644 --- a/go.mod +++ b/go.mod @@ -134,7 +134,6 @@ require github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 // indirect require ( github.com/agnivade/levenshtein v1.0.1 // indirect - github.com/stretchr/objx v0.2.0 // indirect github.com/vektah/gqlparser/v2 v2.4.4 // indirect github.com/yuin/goldmark v1.4.12 // indirect ) @@ -234,7 +233,6 @@ require ( github.com/spf13/jwalterweatherman v1.1.0 // indirect github.com/tadvi/systray v0.0.0-20190226123456-11a2b8fa57af // indirect github.com/tdewolff/parse/v2 v2.6.0 // indirect - github.com/vektah/gqlparser/v2 v2.4.4 // indirect github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xeipuuv/gojsonschema v1.2.0 // indirect diff --git a/go.sum b/go.sum index 97b4b8e376ee0..e10eb82a29a17 100644 --- a/go.sum +++ b/go.sum @@ -1510,7 +1510,6 @@ github.com/stoewer/go-strcase v1.2.0/go.mod h1:IBiWB2sKIp3wVVQ3Y035++gc+knqhUQag github.com/stretchr/objx v0.0.0-20180129172003-8a3f7159479f/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48= github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE= github.com/stretchr/testify v0.0.0-20151208002404-e3a8ff8ce365/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= github.com/stretchr/testify v0.0.0-20180303142811-b89eecf5ca5d/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= From 1bd2ade89d54c7d8f9ad6d89ade75488c5a418db Mon Sep 17 00:00:00 2001 From: kylecarbs Date: Fri, 17 Jun 2022 05:28:54 +0000 Subject: [PATCH 2/2] Fix expansion on Windows --- agent/agent_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index f1d9c6ee22cb8..612aa93b17034 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -181,8 +181,12 @@ func TestAgent(t *testing.T) { } output, err := session.Output(command) require.NoError(t, err) + expect := "" + if runtime.GOOS == "windows" { + expect = "%EXAMPLE%" + } // Output should be empty, because the variable is not set! - require.Equal(t, "", strings.TrimSpace(string(output))) + require.Equal(t, expect, strings.TrimSpace(string(output))) }) t.Run("StartupScript", func(t *testing.T) {