diff --git a/cmd/platform/deploy.go b/cmd/platform/deploy.go index 18ccd363..88eed572 100644 --- a/cmd/platform/deploy.go +++ b/cmd/platform/deploy.go @@ -181,6 +181,7 @@ func deployHook(ctx context.Context, clients *shared.ClientFactory) error { // so we instantiate the default here. shell := hooks.HookExecutorDefaultProtocol{ IO: clients.IO, + Fs: clients.Fs, } if _, err := shell.Execute(ctx, hookExecOpts); err != nil { return err diff --git a/cmd/platform/deploy_test.go b/cmd/platform/deploy_test.go index f5cabbc0..00a656e0 100644 --- a/cmd/platform/deploy_test.go +++ b/cmd/platform/deploy_test.go @@ -279,7 +279,7 @@ func TestDeployCommand_DeployHook(t *testing.T) { clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(clients *shared.ClientFactory) { clients.SDKConfig = sdkConfigMock - clients.HookExecutor = hooks.GetHookExecutor(clientsMock.IO, sdkConfigMock) + clients.HookExecutor = hooks.GetHookExecutor(clientsMock.IO, clients.Fs, sdkConfigMock) }) cmd := NewDeployCommand(clients) cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return nil } diff --git a/internal/hooks/hook_executor_default.go b/internal/hooks/hook_executor_default.go index eb9ab2b6..c033e19f 100644 --- a/internal/hooks/hook_executor_default.go +++ b/internal/hooks/hook_executor_default.go @@ -21,6 +21,7 @@ import ( "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" ) // HookExecutorDefaultProtocol uses the original protocol between the CLI and the SDK where diagnostic info @@ -28,11 +29,12 @@ import ( // exception of the 'start' hook, for which it is printed. type HookExecutorDefaultProtocol struct { IO iostreams.IOStreamer + Fs afero.Fs } // Execute processes the data received by the SDK. func (e *HookExecutorDefaultProtocol) Execute(ctx context.Context, opts HookExecOpts) (string, error) { - cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(opts) + cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(ctx, opts, e.Fs, e.IO) if err != nil { return "", err } diff --git a/internal/hooks/hook_executor_default_test.go b/internal/hooks/hook_executor_default_test.go index 9556287d..1c9748f8 100644 --- a/internal/hooks/hook_executor_default_test.go +++ b/internal/hooks/hook_executor_default_test.go @@ -25,6 +25,7 @@ import ( "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -80,6 +81,31 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) { require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `YIN=yang`) }, }, + "dotenv vars are loaded": { + opts: HookExecOpts{ + Hook: HookScript{Name: "happypath", Command: "echo {}"}, + Env: map[string]string{ + "OPTS_VAR": "from_opts", + }, + Exec: &MockExec{ + mockCommand: &MockCommand{ + MockStdout: []byte("test output"), + Err: nil, + }, + }, + }, + handler: func(t *testing.T, ctx context.Context, executor HookExecutor, opts HookExecOpts) { + // Write a .env file to the mock filesystem + e := executor.(*HookExecutorDefaultProtocol) + _ = afero.WriteFile(e.Fs, ".env", []byte("DOTENV_VAR=from_dotenv\n"), 0644) + + response, err := executor.Execute(ctx, opts) + require.Equal(t, "test output", response) + require.NoError(t, err) + require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `DOTENV_VAR=from_dotenv`) + require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `OPTS_VAR=from_opts`) + }, + }, "failed execution": { opts: HookExecOpts{ Hook: HookScript{Command: "boom", Name: "sadpath"}, @@ -156,6 +182,7 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) { ios.AddDefaultMocks() hookExecutor := &HookExecutorDefaultProtocol{ IO: ios, + Fs: afero.NewMemMapFs(), } if tc.handler != nil { tc.handler(t, ctx, hookExecutor, tc.opts) diff --git a/internal/hooks/hook_executor_v2.go b/internal/hooks/hook_executor_v2.go index 96b146f5..e9ad4613 100644 --- a/internal/hooks/hook_executor_v2.go +++ b/internal/hooks/hook_executor_v2.go @@ -25,6 +25,7 @@ import ( "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" ) // HookExecutorMessageBoundaryProtocol uses a protocol between the CLI and the SDK where diagnostic info @@ -32,6 +33,7 @@ import ( // message boundary. Only one message payload can be received. type HookExecutorMessageBoundaryProtocol struct { IO iostreams.IOStreamer + Fs afero.Fs } // generateBoundary is a function for creating boundaries that can be mocked @@ -39,7 +41,7 @@ var generateBoundary = generateMD5FromRandomString // Execute processes the data received by the SDK. func (e *HookExecutorMessageBoundaryProtocol) Execute(ctx context.Context, opts HookExecOpts) (string, error) { - cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(opts) + cmdArgs, cmdArgVars, cmdEnvVars, err := processExecOpts(ctx, opts, e.Fs, e.IO) if err != nil { return "", err } diff --git a/internal/hooks/hook_executor_v2_test.go b/internal/hooks/hook_executor_v2_test.go index 8b83db94..acfe766c 100644 --- a/internal/hooks/hook_executor_v2_test.go +++ b/internal/hooks/hook_executor_v2_test.go @@ -25,6 +25,7 @@ import ( "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -41,6 +42,7 @@ func mockBoundaryStringGenerator() string { func Test_Hook_Execute_V2_Protocol(t *testing.T) { tests := map[string]struct { opts HookExecOpts + setup func(afero.Fs) check func(*testing.T, string, error, ExecInterface) }{ "error if hook command unavailable": { @@ -132,6 +134,29 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) { ) }, }, + "dotenv vars are loaded": { + opts: HookExecOpts{ + Hook: HookScript{Name: "happypath", Command: "echo {}"}, + Env: map[string]string{ + "OPTS_VAR": "from_opts", + }, + Exec: &MockExec{ + mockCommand: &MockCommand{ + MockStdout: []byte(mockBoundaryString + `{"ok": true}` + mockBoundaryString), + Err: nil, + }, + }, + }, + setup: func(fs afero.Fs) { + _ = afero.WriteFile(fs, ".env", []byte("DOTENV_VAR=from_dotenv\n"), 0644) + }, + check: func(t *testing.T, response string, err error, mockExec ExecInterface) { + require.NoError(t, err) + require.Equal(t, `{"ok": true}`, response) + require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `DOTENV_VAR=from_dotenv`) + require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `OPTS_VAR=from_opts`) + }, + }, "fail to parse payload due to improper boundary strings": { opts: HookExecOpts{ Hook: HookScript{Name: "happypath", Command: "echo {}"}, @@ -176,8 +201,13 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) { config := config.NewConfig(fs, os) ios := iostreams.NewIOStreamsMock(config, fs, os) ios.AddDefaultMocks() + memFs := afero.NewMemMapFs() hookExecutor := &HookExecutorMessageBoundaryProtocol{ IO: ios, + Fs: memFs, + } + if tc.setup != nil { + tc.setup(memFs) } response, err := hookExecutor.Execute(ctx, tc.opts) tc.check(t, response, err, tc.opts.Exec) diff --git a/internal/hooks/hooks.go b/internal/hooks/hooks.go index 49ebf8a6..841d07ea 100644 --- a/internal/hooks/hooks.go +++ b/internal/hooks/hooks.go @@ -19,29 +19,49 @@ import ( "os" "strings" + "github.com/joho/godotenv" "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" + "github.com/spf13/afero" ) type HookExecutor interface { Execute(ctx context.Context, opts HookExecOpts) (response string, err error) } -func GetHookExecutor(ios iostreams.IOStreamer, cfg SDKCLIConfig) HookExecutor { +// LoadDotEnv reads and parses a .env file from the working directory using the +// provided filesystem. It returns nil if the file does not exist. +func LoadDotEnv(fs afero.Fs) (map[string]string, error) { + if fs == nil { + return nil, nil + } + file, err := afero.ReadFile(fs, ".env") + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + return godotenv.UnmarshalBytes(file) +} + +func GetHookExecutor(ios iostreams.IOStreamer, fs afero.Fs, cfg SDKCLIConfig) HookExecutor { protocol := cfg.Config.SupportedProtocols.Preferred() switch protocol { case HookProtocolV2: return &HookExecutorMessageBoundaryProtocol{ IO: ios, + Fs: fs, } default: return &HookExecutorDefaultProtocol{ IO: ios, + Fs: fs, } } } -func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) { +func processExecOpts(ctx context.Context, opts HookExecOpts, fs afero.Fs, io iostreams.IOStreamer) ([]string, []string, []string, error) { cmdStr, err := opts.Hook.Get() if err != nil { return []string{}, []string{}, []string{}, err @@ -53,13 +73,39 @@ func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) { var cmdArgVars = cmdArgs[1:] // omit the first item because that is the command name cmdArgVars = append(cmdArgVars, goutils.MapToStringSlice(opts.Args, "--")...) + // Load .env file variables + dotEnv, err := LoadDotEnv(fs) + if err != nil { + io.PrintDebug(ctx, "Warning: failed to parse .env file: %s", err) + } + if len(dotEnv) > 0 { + keys := make([]string, 0, len(dotEnv)) + for k := range dotEnv { + keys = append(keys, k) + } + io.PrintDebug(ctx, "loaded variables from .env file: %s", strings.Join(keys, ", ")) + } + // Whatever cmd.Env is set to will be the ONLY environment variables that the `cmd` will have access to when it runs. - // To avoid removing any environment variables that are set in the current environment, we first set the cmd.Env to the current environment. - // before adding any new environment variables. - var cmdEnvVars = os.Environ() + // + // Order of precedence from lowest to highest: + // 1. Provided "opts.Env" variables + // 2. Saved ".env" file + // 3. Existing shell environment + // + // > Each entry is of the form "key=value". + // > ... + // > If Env contains duplicate environment keys, only the last value in the slice for each duplicate key is used. + // + // https://pkg.go.dev/os/exec#Cmd.Env + var cmdEnvVars []string for name, value := range opts.Env { cmdEnvVars = append(cmdEnvVars, name+"="+value) } + for k, v := range dotEnv { + cmdEnvVars = append(cmdEnvVars, k+"="+v) + } + cmdEnvVars = append(cmdEnvVars, os.Environ()...) return cmdArgs, cmdArgVars, cmdEnvVars, nil } diff --git a/internal/hooks/hooks_test.go b/internal/hooks/hooks_test.go index d28c223c..4ac4b63e 100644 --- a/internal/hooks/hooks_test.go +++ b/internal/hooks/hooks_test.go @@ -20,9 +20,86 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/slackdeps" + "github.com/spf13/afero" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) +func Test_Hooks_LoadDotEnv(t *testing.T) { + tests := map[string]struct { + fs afero.Fs + dotenv string + writeDotenv bool + expected map[string]string + expectErr bool + }{ + "returns nil when fs is nil": { + fs: nil, + expected: nil, + }, + "returns nil when .env file does not exist": { + fs: afero.NewMemMapFs(), + expected: nil, + }, + "returns empty map for empty .env file": { + fs: afero.NewMemMapFs(), + dotenv: "", + writeDotenv: true, + expected: map[string]string{}, + }, + "parses single variable": { + fs: afero.NewMemMapFs(), + dotenv: "FOO=bar\n", + writeDotenv: true, + expected: map[string]string{"FOO": "bar"}, + }, + "parses multiple variables": { + fs: afero.NewMemMapFs(), + dotenv: "FOO=bar\nBAZ=qux\n", + writeDotenv: true, + expected: map[string]string{"FOO": "bar", "BAZ": "qux"}, + }, + "parses quoted values": { + fs: afero.NewMemMapFs(), + dotenv: `TOKEN="my secret token"` + "\n", + writeDotenv: true, + expected: map[string]string{"TOKEN": "my secret token"}, + }, + "skips comment lines": { + fs: afero.NewMemMapFs(), + dotenv: "# this is a comment\nFOO=bar\n", + writeDotenv: true, + expected: map[string]string{"FOO": "bar"}, + }, + "handles values with equals signs": { + fs: afero.NewMemMapFs(), + dotenv: "URL=https://example.com?foo=bar&baz=qux\n", + writeDotenv: true, + expected: map[string]string{"URL": "https://example.com?foo=bar&baz=qux"}, + }, + "handles empty values": { + fs: afero.NewMemMapFs(), + dotenv: "EMPTY=\n", + writeDotenv: true, + expected: map[string]string{"EMPTY": ""}, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + if tc.writeDotenv && tc.fs != nil { + _ = afero.WriteFile(tc.fs, ".env", []byte(tc.dotenv), 0644) + } + result, err := LoadDotEnv(tc.fs) + if tc.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + assert.Equal(t, tc.expected, result) + }) + } +} + func Test_Hooks_GetHookExecutor(t *testing.T) { tests := map[string]struct { protocolVersions ProtocolVersions @@ -54,7 +131,7 @@ func Test_Hooks_GetHookExecutor(t *testing.T) { io := iostreams.NewIOStreamsMock(config, fs, os) sdkConfig := NewSDKConfigMock() sdkConfig.Config.SupportedProtocols = tc.protocolVersions - hookExecutor := GetHookExecutor(io, sdkConfig) + hookExecutor := GetHookExecutor(io, fs, sdkConfig) require.IsType(t, tc.expectedType, hookExecutor) }) } diff --git a/internal/pkg/platform/localserver.go b/internal/pkg/platform/localserver.go index ca038042..5324aaef 100644 --- a/internal/pkg/platform/localserver.go +++ b/internal/pkg/platform/localserver.go @@ -308,13 +308,39 @@ func (r *LocalServer) StartDelegate(ctx context.Context) error { cmdArgs := strings.Fields(cmdStr) var cmdArgVars = cmdArgs[1:] // omit the first item because that is the command name + // Load .env file variables + dotEnv, err := hooks.LoadDotEnv(r.clients.Fs) + if err != nil { + r.clients.IO.PrintDebug(ctx, "Warning: failed to parse .env file: %s", err) + } + if len(dotEnv) > 0 { + keys := make([]string, 0, len(dotEnv)) + for k := range dotEnv { + keys = append(keys, k) + } + r.clients.IO.PrintDebug(ctx, "Loaded variables from .env file: %s", strings.Join(keys, ", ")) + } + // Whatever cmd.Env is set to will be the ONLY environment variables that the `cmd` will have access to when it runs. - // To avoid removing any environment variables that are set in the current environment, we first set the cmd.Env to the current environment. - // before adding any new environment variables. - var cmdEnvVars = os.Environ() - for name, value := range sdkManagedConnectionStartHookOpts.Env { - cmdEnvVars = append(cmdEnvVars, name+"="+value) + // + // Order of precedence from lowest to highest: + // 1. Provided "opts.Env" variables + // 2. Saved ".env" file + // 3. Existing shell environment + // + // > Each entry is of the form "key=value". + // > ... + // > If Env contains duplicate environment keys, only the last value in the slice for each duplicate key is used. + // + // https://pkg.go.dev/os/exec#Cmd.Env + var cmdEnvVars []string + for k, v := range sdkManagedConnectionStartHookOpts.Env { + cmdEnvVars = append(cmdEnvVars, k+"="+v) + } + for k, v := range dotEnv { + cmdEnvVars = append(cmdEnvVars, k+"="+v) } + cmdEnvVars = append(cmdEnvVars, os.Environ()...) cmd := sdkManagedConnectionStartHookOpts.Exec.Command(cmdEnvVars, os.Stdout, os.Stderr, nil, cmdArgs[0], cmdArgVars...) // Store command reference for lifecycle management diff --git a/internal/shared/clients.go b/internal/shared/clients.go index e61b5ab9..ab7a0649 100644 --- a/internal/shared/clients.go +++ b/internal/shared/clients.go @@ -84,6 +84,7 @@ func NewClientFactory(options ...func(*ClientFactory)) *ClientFactory { clients.IO = iostreams.NewIOStreams(clients.Config, clients.Fs, clients.Os) clients.HookExecutor = &hooks.HookExecutorDefaultProtocol{ IO: clients.IO, + Fs: clients.Fs, } clients.EventTracker = tracking.NewEventTracker() clients.API = clients.defaultAPIFunc @@ -237,7 +238,7 @@ func (c *ClientFactory) InitSDKConfig(ctx context.Context, dirPath string) error // TODO: this is a side-effect-y way of signaling to the rest of the codebase "we are in an app project directory now" c.SDKConfig.WorkingDirectory = dirPath - c.HookExecutor = hooks.GetHookExecutor(c.IO, c.SDKConfig) + c.HookExecutor = hooks.GetHookExecutor(c.IO, c.Fs, c.SDKConfig) return err } @@ -277,6 +278,7 @@ func (c *ClientFactory) InitSDKConfigFromJSON(ctx context.Context, configFileByt } defaultExecutor := hooks.HookExecutorDefaultProtocol{ IO: c.IO, + Fs: c.Fs, } if SDKHooksResponse, err = defaultExecutor.Execute(ctx, hookExecOpts); err != nil { return err