Skip to content

Commit a841285

Browse files
zimegmwbrooks
andauthored
fix: unquote environment variable passed to hook commands (#410)
Co-authored-by: Michael Brooks <michael@michaelbrooks.ca>
1 parent ba1b784 commit a841285

4 files changed

Lines changed: 22 additions & 15 deletions

File tree

internal/hooks/hook_executor_default_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
6060
opts: HookExecOpts{
6161
Hook: HookScript{Name: "happypath", Command: "echo {}"},
6262
Env: map[string]string{
63-
"batman": "robin",
64-
"yin": "yang",
63+
"BATMAN": "robin",
64+
"WHOAMI": "lumpy space princess",
65+
"YIN": "yang",
6566
},
6667
Exec: &MockExec{
6768
mockCommand: &MockCommand{
@@ -74,8 +75,9 @@ func Test_Hook_Execute_Default_Protocol(t *testing.T) {
7475
response, err := executor.Execute(ctx, opts)
7576
require.Equal(t, "test output", response)
7677
require.Equal(t, nil, err)
77-
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `batman="robin"`)
78-
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `yin="yang"`)
78+
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `BATMAN=robin`)
79+
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `WHOAMI=lumpy space princess`)
80+
require.Contains(t, opts.Exec.(*MockExec).mockCommand.Env, `YIN=yang`)
7981
},
8082
},
8183
"failed execution": {

internal/hooks/hook_executor_v2_test.go

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,9 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
5555
opts: HookExecOpts{
5656
Hook: HookScript{Name: "happypath", Command: "echo {}"},
5757
Env: map[string]string{
58-
"batman": "robin",
59-
"yin": "yang",
58+
"BATMAN": "robin",
59+
"WHOAMI": "lumpy space princess",
60+
"YIN": "yang",
6061
},
6162
Exec: &MockExec{
6263
mockCommand: &MockCommand{
@@ -68,16 +69,17 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
6869
check: func(t *testing.T, response string, err error, mockExec ExecInterface) {
6970
require.Equal(t, `{"message": "hello world"}`, response)
7071
require.Equal(t, nil, err)
71-
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `batman="robin"`)
72-
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `yin="yang"`)
72+
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `BATMAN=robin`)
73+
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `WHOAMI=lumpy space princess`)
74+
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `YIN=yang`)
7375
},
7476
},
7577
"successful execution with payload > 64kb": {
7678
opts: HookExecOpts{
7779
Hook: HookScript{Name: "happypath", Command: "echo {}"},
7880
Env: map[string]string{
79-
"batman": "robin",
80-
"yin": "yang",
81+
"BATMAN": "robin",
82+
"YIN": "yang",
8183
},
8284
Exec: &MockExec{
8385
mockCommand: &MockCommand{
@@ -89,8 +91,8 @@ func Test_Hook_Execute_V2_Protocol(t *testing.T) {
8991
check: func(t *testing.T, response string, err error, mockExec ExecInterface) {
9092
require.Equal(t, sixtyFourKBString, response)
9193
require.Equal(t, nil, err)
92-
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `batman="robin"`)
93-
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `yin="yang"`)
94+
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `BATMAN=robin`)
95+
require.Contains(t, mockExec.(*MockExec).mockCommand.Env, `YIN=yang`)
9496
},
9597
},
9698
"successful execution with payload > 512kb": {

internal/hooks/hooks.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ func processExecOpts(opts HookExecOpts) ([]string, []string, []string, error) {
5757
// To avoid removing any environment variables that are set in the current environment, we first set the cmd.Env to the current environment.
5858
// before adding any new environment variables.
5959
var cmdEnvVars = os.Environ()
60-
cmdEnvVars = append(cmdEnvVars, goutils.MapToStringSlice(opts.Env, "")...)
60+
for name, value := range opts.Env {
61+
cmdEnvVars = append(cmdEnvVars, name+"="+value)
62+
}
6163

6264
return cmdArgs, cmdArgVars, cmdEnvVars, nil
6365
}

internal/pkg/platform/localserver.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030
"github.com/gorilla/websocket"
3131
"github.com/radovskyb/watcher"
3232
"github.com/slackapi/slack-cli/internal/config"
33-
"github.com/slackapi/slack-cli/internal/goutils"
3433
"github.com/slackapi/slack-cli/internal/hooks"
3534
"github.com/slackapi/slack-cli/internal/iostreams"
3635
"github.com/slackapi/slack-cli/internal/pkg/apps"
@@ -307,7 +306,9 @@ func (r *LocalServer) StartDelegate(ctx context.Context) error {
307306
// To avoid removing any environment variables that are set in the current environment, we first set the cmd.Env to the current environment.
308307
// before adding any new environment variables.
309308
var cmdEnvVars = os.Environ()
310-
cmdEnvVars = append(cmdEnvVars, goutils.MapToStringSlice(sdkManagedConnectionStartHookOpts.Env, "")...)
309+
for name, value := range sdkManagedConnectionStartHookOpts.Env {
310+
cmdEnvVars = append(cmdEnvVars, name+"="+value)
311+
}
311312
cmd := sdkManagedConnectionStartHookOpts.Exec.Command(cmdEnvVars, os.Stdout, os.Stderr, nil, cmdArgs[0], cmdArgVars...)
312313

313314
// Store command reference for lifecycle management

0 commit comments

Comments
 (0)