From 986b20977d7e6b936a2b7fd645a801b7253cd244 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 13:32:18 -0700 Subject: [PATCH 01/15] feat: add variables to ".env" for non-hosted apps --- cmd/env/add.go | 136 ++++++++++++++++++++---- cmd/env/add_test.go | 254 +++++++++++++++++++++++++++++++------------- 2 files changed, 294 insertions(+), 96 deletions(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index f48c47dd..d5510fe1 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -19,12 +19,15 @@ import ( "fmt" "strings" + "github.com/joho/godotenv" "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackdotenv" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" + "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -33,12 +36,15 @@ func NewEnvAddCommand(clients *shared.ClientFactory) *cobra.Command { Use: "add [flags]", Short: "Add an environment variable to the app", Long: strings.Join([]string{ - "Add an environment variable to an app deployed to Slack managed infrastructure.", + "Add an environment variable to the app.", "", "If a name or value is not provided, you will be prompted to provide these.", "", - "This command is supported for apps deployed to Slack managed infrastructure but", - "other apps can attempt to run the command with the --force flag.", + "Commands that run in the context of a project source environment variables from", + "the \".env\" file. This includes the \"run\" command.", + "", + "The \"deploy\" command gathers environment variables from the \".env\" file as well", + "unless the app is using ROSI features.", }, "\n"), Example: style.ExampleCommandsf([]style.ExampleCommand{ { @@ -69,18 +75,11 @@ func NewEnvAddCommand(clients *shared.ClientFactory) *cobra.Command { return cmd } -// preRunEnvAddCommandFunc determines if the command is supported for a project +// preRunEnvAddCommandFunc determines if the command is run in a valid project // and configures flags func preRunEnvAddCommandFunc(ctx context.Context, clients *shared.ClientFactory, cmd *cobra.Command) error { clients.Config.SetFlags(cmd) - err := cmdutil.IsValidProjectDirectory(clients) - if err != nil { - return err - } - if clients.Config.ForceFlag { - return nil - } - return cmdutil.IsSlackHostedProject(ctx, clients) + return cmdutil.IsValidProjectDirectory(clients) } // runEnvAddCommandFunc sets an app environment variable to given values @@ -88,7 +87,7 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg ctx := cmd.Context() // Get the workspace from the flag or prompt - selection, err := appSelectPromptFunc(ctx, clients, prompts.ShowHostedOnly, prompts.ShowInstalledAppsOnly) + selection, err := appSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly) if err != nil { return err } @@ -127,15 +126,24 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg variableValue = args[1] } - err = clients.API().AddVariable( - ctx, - selection.Auth.Token, - selection.App.AppID, - variableName, - variableValue, - ) - if err != nil { - return err + // Add the environment variable using either the Slack API method or the + // project ".env" file depending on the app hosting. + if !selection.App.IsDev && cmdutil.IsSlackHostedProject(ctx, clients) == nil { + err = clients.API().AddVariable( + ctx, + selection.Auth.Token, + selection.App.AppID, + variableName, + variableValue, + ) + if err != nil { + return err + } + } else { + err = setDotEnv(clients.Fs, variableName, variableValue) + if err != nil { + return err + } } clients.IO.PrintTrace(ctx, slacktrace.EnvAddSuccess) @@ -151,3 +159,87 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg })) return nil } + +// setDotEnv sets a single environment variable in the .env file, preserving +// comments, blank lines, and other formatting. If the key already exists its +// value is replaced in-place. Otherwise the entry is appended. The file is +// created if it does not exist. +func setDotEnv(fs afero.Fs, name string, value string) error { + newEntry, err := godotenv.Marshal(map[string]string{name: value}) + if err != nil { + return err + } + + // Check for an existing .env file and parse it to detect existing keys. + existing, err := slackdotenv.Read(fs) + if err != nil { + return err + } + + // If the file does not exist or the key is new, append the entry. + if existing == nil { + return afero.WriteFile(fs, ".env", []byte(newEntry+"\n"), 0644) + } + + oldValue, found := existing[name] + if !found { + raw, err := afero.ReadFile(fs, ".env") + if err != nil { + return err + } + content := string(raw) + if len(content) > 0 && !strings.HasSuffix(content, "\n") { + content += "\n" + } + return afero.WriteFile(fs, ".env", []byte(content+newEntry+"\n"), 0644) + } + + // The key exists — replace the old entry in the raw file content. Build + // possible representations of the old entry to find and replace in the raw + // bytes, since the file format may differ from what Marshal produces. + raw, err := afero.ReadFile(fs, ".env") + if err != nil { + return err + } + + // Strip the "NAME=" prefix from the marshaled new entry to get just the + // value portion, then build the old entry alternatives using the same + // prefix variations the file might contain. + marshaledValue := strings.TrimPrefix(newEntry, name+"=") + oldMarshaledValue := marshaledValue + oldEntry, err := godotenv.Marshal(map[string]string{name: oldValue}) + if err == nil { + oldMarshaledValue = strings.TrimPrefix(oldEntry, name+"=") + } + + // Try each possible form of the old entry, longest (most specific) first. + entries := []string{ + "export " + name + "=" + oldMarshaledValue, + "export " + name + "=" + oldValue, + "export " + name + "=" + "'" + oldValue + "'", + name + "=" + oldMarshaledValue, + name + "=" + oldValue, + name + "=" + "'" + oldValue + "'", + } + + content := string(raw) + replaced := false + for _, entry := range entries { + if strings.Contains(content, entry) { + replacement := newEntry + if strings.HasPrefix(entry, "export ") { + replacement = "export " + newEntry + } + content = strings.Replace(content, entry, replacement, 1) + replaced = true + break + } + } + if !replaced { + if !strings.HasSuffix(content, "\n") { + content += "\n" + } + content += newEntry + "\n" + } + return afero.WriteFile(fs, ".env", []byte(content), 0644) +} diff --git a/cmd/env/add_test.go b/cmd/env/add_test.go index 20b4ab86..c33b80d9 100644 --- a/cmd/env/add_test.go +++ b/cmd/env/add_test.go @@ -28,6 +28,7 @@ import ( "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/test/testutil" + "github.com/spf13/afero" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -46,88 +47,22 @@ var mockApp = types.App{ func Test_Env_AddCommandPreRun(t *testing.T) { tests := map[string]struct { - mockFlagForce bool - mockManifestResponse types.SlackYaml - mockManifestError error - mockManifestSource config.ManifestSource mockWorkingDirectory string expectedError error }{ - "continues if the application is hosted on slack": { - mockManifestResponse: types.SlackYaml{ - AppManifest: types.AppManifest{ - Settings: &types.AppSettings{ - FunctionRuntime: types.SlackHosted, - }, - }, - }, - mockManifestError: nil, - mockManifestSource: config.ManifestSourceLocal, - mockWorkingDirectory: "/slack/path/to/project", - expectedError: nil, - }, - "errors if the application is not hosted on slack": { - mockManifestResponse: types.SlackYaml{ - AppManifest: types.AppManifest{ - Settings: &types.AppSettings{ - FunctionRuntime: types.Remote, - }, - }, - }, - mockManifestError: nil, - mockManifestSource: config.ManifestSourceLocal, - mockWorkingDirectory: "/slack/path/to/project", - expectedError: slackerror.New(slackerror.ErrAppNotHosted), - }, - "continues if the force flag is used in a project": { - mockFlagForce: true, + "continues if the command is run in a project": { mockWorkingDirectory: "/slack/path/to/project", expectedError: nil, }, - "errors if the project manifest cannot be retrieved": { - mockManifestResponse: types.SlackYaml{}, - mockManifestError: slackerror.New(slackerror.ErrSDKHookInvocationFailed), - mockManifestSource: config.ManifestSourceLocal, - mockWorkingDirectory: "/slack/path/to/project", - expectedError: slackerror.New(slackerror.ErrSDKHookInvocationFailed), - }, "errors if the command is not run in a project": { - mockManifestResponse: types.SlackYaml{}, - mockManifestError: slackerror.New(slackerror.ErrSDKHookNotFound), mockWorkingDirectory: "", expectedError: slackerror.New(slackerror.ErrInvalidAppDirectory), }, - "errors if the manifest source is set to remote": { - mockManifestSource: config.ManifestSourceRemote, - mockWorkingDirectory: "/slack/path/to/project", - expectedError: slackerror.New(slackerror.ErrAppNotHosted), - }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { clientsMock := shared.NewClientsMock() - manifestMock := &app.ManifestMockObject{} - manifestMock.On( - "GetManifestLocal", - mock.Anything, - mock.Anything, - mock.Anything, - ).Return( - tc.mockManifestResponse, - tc.mockManifestError, - ) - clientsMock.AppClient.Manifest = manifestMock - projectConfigMock := config.NewProjectConfigMock() - projectConfigMock.On( - "GetManifestSource", - mock.Anything, - ).Return( - tc.mockManifestSource, - nil, - ) - clientsMock.Config.ProjectConfig = projectConfigMock clients := shared.NewClientFactory(clientsMock.MockClientFactory(), func(cf *shared.ClientFactory) { - cf.Config.ForceFlag = tc.mockFlagForce cf.SDKConfig.WorkingDirectory = tc.mockWorkingDirectory }) cmd := NewEnvAddCommand(clients) @@ -146,7 +81,7 @@ func Test_Env_AddCommand(t *testing.T) { "add a variable using arguments": { CmdArgs: []string{"ENV_NAME", "ENV_VALUE"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddCommandMocks(ctx, cm, cf) + setupEnvAddHostedMocks(ctx, cm, cf) }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertCalled( @@ -170,7 +105,7 @@ func Test_Env_AddCommand(t *testing.T) { "provide a variable name by argument and value by prompt": { CmdArgs: []string{"ENV_NAME"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddCommandMocks(ctx, cm, cf) + setupEnvAddHostedMocks(ctx, cm, cf) cm.IO.On( "PasswordPrompt", mock.Anything, @@ -201,7 +136,7 @@ func Test_Env_AddCommand(t *testing.T) { "provide a variable name by argument and value by flag": { CmdArgs: []string{"ENV_NAME", "--value", "example_value"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddCommandMocks(ctx, cm, cf) + setupEnvAddHostedMocks(ctx, cm, cf) cm.IO.On( "PasswordPrompt", mock.Anything, @@ -232,7 +167,7 @@ func Test_Env_AddCommand(t *testing.T) { "provide both variable name and value by prompt": { CmdArgs: []string{}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddCommandMocks(ctx, cm, cf) + setupEnvAddHostedMocks(ctx, cm, cf) cm.IO.On( "InputPrompt", mock.Anything, @@ -269,6 +204,60 @@ func Test_Env_AddCommand(t *testing.T) { ) }, }, + "add a variable to the .env file for non-hosted app": { + CmdArgs: []string{"ENV_NAME", "ENV_VALUE"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvAddDotenvMocks(ctx, cm, cf) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "AddVariable") + cm.IO.AssertCalled( + t, + "PrintTrace", + mock.Anything, + slacktrace.EnvAddSuccess, + mock.Anything, + ) + }, + }, + "add a variable preserving existing variables and comments in .env": { + CmdArgs: []string{"NEW_VAR", "new_value"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvAddDotenvMocks(ctx, cm, cf) + err := afero.WriteFile(cf.Fs, ".env", []byte("# Config\nEXISTING=value\n"), 0644) + assert.NoError(t, err) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "AddVariable") + }, + }, + "overwrite an existing variable in .env file": { + CmdArgs: []string{"MY_VAR", "new_value"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvAddDotenvMocks(ctx, cm, cf) + err := afero.WriteFile(cf.Fs, ".env", []byte("MY_VAR=old_value\n"), 0644) + assert.NoError(t, err) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "AddVariable") + }, + }, + "create .env file when it does not exist": { + CmdArgs: []string{"FIRST_VAR", "first_value"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvAddDotenvMocks(ctx, cm, cf) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "AddVariable") + cm.IO.AssertCalled( + t, + "PrintTrace", + mock.Anything, + slacktrace.EnvAddSuccess, + mock.Anything, + ) + }, + }, }, func(cf *shared.ClientFactory) *cobra.Command { cmd := NewEnvAddCommand(cf) cmd.PreRunE = func(cmd *cobra.Command, args []string) error { return nil } @@ -276,17 +265,134 @@ func Test_Env_AddCommand(t *testing.T) { }) } -// setupEnvAddCommandMocks prepares common mocks for these tests -func setupEnvAddCommandMocks(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { +// setupEnvAddHostedMocks prepares common mocks for hosted app tests +func setupEnvAddHostedMocks(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { cf.SDKConfig = hooks.NewSDKConfigMock() cm.AddDefaultMocks() _ = cf.AppClient().SaveDeployed(ctx, mockApp) appSelectMock := prompts.NewAppSelectMock() appSelectPromptFunc = appSelectMock.AppSelectPrompt - appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{Auth: mockAuth, App: mockApp}, nil) + appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{Auth: mockAuth, App: mockApp}, nil) cm.Config.Flags.String("value", "", "mock value flag") cm.API.On("AddVariable", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{ + AppManifest: types.AppManifest{ + Settings: &types.AppSettings{ + FunctionRuntime: types.SlackHosted, + }, + }, + }, + nil, + ) + cm.AppClient.Manifest = manifestMock + projectConfigMock := config.NewProjectConfigMock() + projectConfigMock.On("GetManifestSource", mock.Anything).Return(config.ManifestSourceLocal, nil) + cm.Config.ProjectConfig = projectConfigMock + cf.SDKConfig.WorkingDirectory = "/slack/path/to/project" +} + +// setupEnvAddDotenvMocks prepares common mocks for non-hosted (dotenv) app tests +func setupEnvAddDotenvMocks(_ context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + cf.SDKConfig = hooks.NewSDKConfigMock() + cm.AddDefaultMocks() + + mockDevApp := types.App{ + TeamID: "T1", + TeamDomain: "team1", + AppID: "A0123456789", + IsDev: true, + } + appSelectMock := prompts.NewAppSelectMock() + appSelectPromptFunc = appSelectMock.AppSelectPrompt + appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{Auth: mockAuth, App: mockDevApp}, nil) + + cm.Config.Flags.String("value", "", "mock value flag") +} + +func Test_setDotEnv(t *testing.T) { + tests := map[string]struct { + existingEnv string + writeExisting bool + name string + value string + expectedFile string + }{ + "creates .env file when it does not exist": { + name: "FOO", + value: "bar", + expectedFile: "FOO=\"bar\"\n", + }, + "adds a variable to an empty .env file": { + existingEnv: "", + writeExisting: true, + name: "FOO", + value: "bar", + expectedFile: "FOO=\"bar\"\n", + }, + "adds a variable preserving existing variables": { + existingEnv: "EXISTING=value\n", + writeExisting: true, + name: "NEW_VAR", + value: "new_value", + expectedFile: "EXISTING=value\nNEW_VAR=\"new_value\"\n", + }, + "adds a variable preserving comments and blank lines": { + existingEnv: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\n", + writeExisting: true, + name: "NEW_VAR", + value: "new_value", + expectedFile: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\nNEW_VAR=\"new_value\"\n", + }, + "updates an existing unquoted variable in-place": { + existingEnv: "# Config\nFOO=old_value\nBAR=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "# Config\nFOO=\"new_value\"\nBAR=keep\n", + }, + "updates an existing quoted variable in-place": { + existingEnv: "FOO=\"old_value\"\nBAR=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "FOO=\"new_value\"\nBAR=keep\n", + }, + "updates a variable with export prefix": { + existingEnv: "export SECRET=old_secret\nOTHER=keep\n", + writeExisting: true, + name: "SECRET", + value: "new_secret", + expectedFile: "export SECRET=\"new_secret\"\nOTHER=keep\n", + }, + "escapes special characters in values": { + name: "SPECIAL", + value: "has \"quotes\" and $vars and \\ backslash", + expectedFile: "SPECIAL=\"has \\\"quotes\\\" and \\$vars and \\\\ backslash\"\n", + }, + "round-trips through LoadDotEnv": { + name: "ROUND_TRIP", + value: "hello world", + expectedFile: "ROUND_TRIP=\"hello world\"\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fs := afero.NewMemMapFs() + if tc.writeExisting { + err := afero.WriteFile(fs, ".env", []byte(tc.existingEnv), 0644) + assert.NoError(t, err) + } + err := setDotEnv(fs, tc.name, tc.value) + assert.NoError(t, err) + content, err := afero.ReadFile(fs, ".env") + assert.NoError(t, err) + assert.Equal(t, tc.expectedFile, string(content)) + }) + } } From 81b16ac3832dfd9fbd6cf9d790240087763de61c Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 16:06:25 -0700 Subject: [PATCH 02/15] fix: guard against invalid set marshal from slackdotenv package --- cmd/env/add.go | 88 +------------------- cmd/env/add_test.go | 82 ------------------ internal/slackdotenv/slackdotenv.go | 101 +++++++++++++++++++++++ internal/slackdotenv/slackdotenv_test.go | 101 +++++++++++++++++++++++ internal/slackerror/errors.go | 12 +++ 5 files changed, 215 insertions(+), 169 deletions(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index d5510fe1..2456446d 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -19,7 +19,6 @@ import ( "fmt" "strings" - "github.com/joho/godotenv" "github.com/slackapi/slack-cli/internal/cmdutil" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/prompts" @@ -27,7 +26,6 @@ import ( "github.com/slackapi/slack-cli/internal/slackdotenv" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" - "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -140,7 +138,7 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg return err } } else { - err = setDotEnv(clients.Fs, variableName, variableValue) + err = slackdotenv.Set(clients.Fs, variableName, variableValue) if err != nil { return err } @@ -159,87 +157,3 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg })) return nil } - -// setDotEnv sets a single environment variable in the .env file, preserving -// comments, blank lines, and other formatting. If the key already exists its -// value is replaced in-place. Otherwise the entry is appended. The file is -// created if it does not exist. -func setDotEnv(fs afero.Fs, name string, value string) error { - newEntry, err := godotenv.Marshal(map[string]string{name: value}) - if err != nil { - return err - } - - // Check for an existing .env file and parse it to detect existing keys. - existing, err := slackdotenv.Read(fs) - if err != nil { - return err - } - - // If the file does not exist or the key is new, append the entry. - if existing == nil { - return afero.WriteFile(fs, ".env", []byte(newEntry+"\n"), 0644) - } - - oldValue, found := existing[name] - if !found { - raw, err := afero.ReadFile(fs, ".env") - if err != nil { - return err - } - content := string(raw) - if len(content) > 0 && !strings.HasSuffix(content, "\n") { - content += "\n" - } - return afero.WriteFile(fs, ".env", []byte(content+newEntry+"\n"), 0644) - } - - // The key exists — replace the old entry in the raw file content. Build - // possible representations of the old entry to find and replace in the raw - // bytes, since the file format may differ from what Marshal produces. - raw, err := afero.ReadFile(fs, ".env") - if err != nil { - return err - } - - // Strip the "NAME=" prefix from the marshaled new entry to get just the - // value portion, then build the old entry alternatives using the same - // prefix variations the file might contain. - marshaledValue := strings.TrimPrefix(newEntry, name+"=") - oldMarshaledValue := marshaledValue - oldEntry, err := godotenv.Marshal(map[string]string{name: oldValue}) - if err == nil { - oldMarshaledValue = strings.TrimPrefix(oldEntry, name+"=") - } - - // Try each possible form of the old entry, longest (most specific) first. - entries := []string{ - "export " + name + "=" + oldMarshaledValue, - "export " + name + "=" + oldValue, - "export " + name + "=" + "'" + oldValue + "'", - name + "=" + oldMarshaledValue, - name + "=" + oldValue, - name + "=" + "'" + oldValue + "'", - } - - content := string(raw) - replaced := false - for _, entry := range entries { - if strings.Contains(content, entry) { - replacement := newEntry - if strings.HasPrefix(entry, "export ") { - replacement = "export " + newEntry - } - content = strings.Replace(content, entry, replacement, 1) - replaced = true - break - } - } - if !replaced { - if !strings.HasSuffix(content, "\n") { - content += "\n" - } - content += newEntry + "\n" - } - return afero.WriteFile(fs, ".env", []byte(content), 0644) -} diff --git a/cmd/env/add_test.go b/cmd/env/add_test.go index c33b80d9..9aa28507 100644 --- a/cmd/env/add_test.go +++ b/cmd/env/add_test.go @@ -314,85 +314,3 @@ func setupEnvAddDotenvMocks(_ context.Context, cm *shared.ClientsMock, cf *share cm.Config.Flags.String("value", "", "mock value flag") } - -func Test_setDotEnv(t *testing.T) { - tests := map[string]struct { - existingEnv string - writeExisting bool - name string - value string - expectedFile string - }{ - "creates .env file when it does not exist": { - name: "FOO", - value: "bar", - expectedFile: "FOO=\"bar\"\n", - }, - "adds a variable to an empty .env file": { - existingEnv: "", - writeExisting: true, - name: "FOO", - value: "bar", - expectedFile: "FOO=\"bar\"\n", - }, - "adds a variable preserving existing variables": { - existingEnv: "EXISTING=value\n", - writeExisting: true, - name: "NEW_VAR", - value: "new_value", - expectedFile: "EXISTING=value\nNEW_VAR=\"new_value\"\n", - }, - "adds a variable preserving comments and blank lines": { - existingEnv: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\n", - writeExisting: true, - name: "NEW_VAR", - value: "new_value", - expectedFile: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\nNEW_VAR=\"new_value\"\n", - }, - "updates an existing unquoted variable in-place": { - existingEnv: "# Config\nFOO=old_value\nBAR=keep\n", - writeExisting: true, - name: "FOO", - value: "new_value", - expectedFile: "# Config\nFOO=\"new_value\"\nBAR=keep\n", - }, - "updates an existing quoted variable in-place": { - existingEnv: "FOO=\"old_value\"\nBAR=keep\n", - writeExisting: true, - name: "FOO", - value: "new_value", - expectedFile: "FOO=\"new_value\"\nBAR=keep\n", - }, - "updates a variable with export prefix": { - existingEnv: "export SECRET=old_secret\nOTHER=keep\n", - writeExisting: true, - name: "SECRET", - value: "new_secret", - expectedFile: "export SECRET=\"new_secret\"\nOTHER=keep\n", - }, - "escapes special characters in values": { - name: "SPECIAL", - value: "has \"quotes\" and $vars and \\ backslash", - expectedFile: "SPECIAL=\"has \\\"quotes\\\" and \\$vars and \\\\ backslash\"\n", - }, - "round-trips through LoadDotEnv": { - name: "ROUND_TRIP", - value: "hello world", - expectedFile: "ROUND_TRIP=\"hello world\"\n", - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - fs := afero.NewMemMapFs() - if tc.writeExisting { - err := afero.WriteFile(fs, ".env", []byte(tc.existingEnv), 0644) - assert.NoError(t, err) - } - err := setDotEnv(fs, tc.name, tc.value) - assert.NoError(t, err) - content, err := afero.ReadFile(fs, ".env") - assert.NoError(t, err) - assert.Equal(t, tc.expectedFile, string(content)) - }) - } -} diff --git a/internal/slackdotenv/slackdotenv.go b/internal/slackdotenv/slackdotenv.go index c85d58db..f4191d38 100644 --- a/internal/slackdotenv/slackdotenv.go +++ b/internal/slackdotenv/slackdotenv.go @@ -21,6 +21,7 @@ package slackdotenv import ( "os" + "strings" "github.com/joho/godotenv" "github.com/slackapi/slack-cli/internal/slackerror" @@ -49,3 +50,103 @@ func Read(fs afero.Fs) (map[string]string, error) { } return vars, nil } + +// Set sets a single environment variable in the .env file, preserving +// comments, blank lines, and other formatting. If the key already exists its +// value is replaced in-place. Otherwise the entry is appended. The file is +// created if it does not exist. +func Set(fs afero.Fs, name string, value string) error { + newEntry, err := godotenv.Marshal(map[string]string{name: value}) + if err != nil { + return slackerror.Wrap(err, slackerror.ErrDotEnvVarMarshal). + WithMessage("Failed to marshal the .env variable: %s", err) + } + + // Verify the marshaled entry can be parsed back to avoid writing values + // that would corrupt the .env file for future reads. + if _, err := godotenv.Unmarshal(newEntry); err != nil { + return slackerror.Wrap(err, slackerror.ErrDotEnvVarMarshal). + WithMessage("Failed to marshal the .env variable: %s", err) + } + + // Check for an existing .env file and parse it to detect existing keys. + existing, err := Read(fs) + if err != nil { + return err + } + + // If the file does not exist, create it with the new entry. + if existing == nil { + return writeFile(fs, []byte(newEntry+"\n")) + } + + // Read the raw file content once for either the append or replace path. + raw, err := afero.ReadFile(fs, ".env") + if err != nil { + return slackerror.Wrap(err, slackerror.ErrDotEnvFileRead). + WithMessage("Failed to read the .env file: %s", err) + } + content := string(raw) + + // If the key is new, append the entry. + oldValue, found := existing[name] + if !found { + if len(content) > 0 && !strings.HasSuffix(content, "\n") { + content += "\n" + } + return writeFile(fs, []byte(content+newEntry+"\n")) + } + + // Marshal the old value to find its canonical form in the file. + oldEntry, err := godotenv.Marshal(map[string]string{name: oldValue}) + if err != nil { + return slackerror.Wrap(err, slackerror.ErrDotEnvVarMarshal). + WithMessage("Failed to marshal the .env variable: %s", err) + } + oldMarshaledValue := strings.TrimPrefix(oldEntry, name+"=") + + // Try each possible form of the old entry, longest (most specific) first. + // For multiline values, also try the double-quoted raw form since the file + // stores actual newlines while Marshal escapes them. + entries := []string{ + "export " + name + "=" + oldMarshaledValue, + "export " + name + "=" + `"` + oldValue + `"`, + "export " + name + "=" + oldValue, + "export " + name + "=" + "'" + oldValue + "'", + name + "=" + oldMarshaledValue, + name + "=" + `"` + oldValue + `"`, + name + "=" + oldValue, + name + "=" + "'" + oldValue + "'", + } + + replaced := false + for _, entry := range entries { + if strings.Contains(content, entry) { + if strings.HasPrefix(entry, "export ") { + content = strings.Replace(content, entry, "export "+newEntry, 1) + } else { + content = strings.Replace(content, entry, newEntry, 1) + } + replaced = true + break + } + } + if !replaced { + if !strings.HasSuffix(content, "\n") { + content += "\n" + } + content += newEntry + "\n" + } + return writeFile(fs, []byte(content)) +} + +// writeFile writes data to the .env file, wrapping any error with a structured +// error code. +func writeFile(fs afero.Fs, data []byte) error { + err := afero.WriteFile(fs, ".env", data, 0600) + if err != nil { + return slackerror.Wrap(err, slackerror.ErrDotEnvFileWrite). + WithMessage("Failed to write the .env file: %s", err) + } + return nil +} diff --git a/internal/slackdotenv/slackdotenv_test.go b/internal/slackdotenv/slackdotenv_test.go index 73eff8e1..27215a15 100644 --- a/internal/slackdotenv/slackdotenv_test.go +++ b/internal/slackdotenv/slackdotenv_test.go @@ -111,3 +111,104 @@ func Test_Read(t *testing.T) { }) } } + +func Test_Set(t *testing.T) { + tests := map[string]struct { + existingEnv string + writeExisting bool + name string + value string + expectedFile string + expectErr string + }{ + "creates .env file when it does not exist": { + name: "FOO", + value: "bar", + expectedFile: "FOO=\"bar\"\n", + }, + "adds a variable to an empty .env file": { + existingEnv: "", + writeExisting: true, + name: "FOO", + value: "bar", + expectedFile: "FOO=\"bar\"\n", + }, + "adds a variable preserving existing variables": { + existingEnv: "EXISTING=value\n", + writeExisting: true, + name: "NEW_VAR", + value: "new_value", + expectedFile: "EXISTING=value\nNEW_VAR=\"new_value\"\n", + }, + "adds a variable preserving comments and blank lines": { + existingEnv: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\n", + writeExisting: true, + name: "NEW_VAR", + value: "new_value", + expectedFile: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\nNEW_VAR=\"new_value\"\n", + }, + "updates an existing unquoted variable in-place": { + existingEnv: "# Config\nFOO=old_value\nBAR=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "# Config\nFOO=\"new_value\"\nBAR=keep\n", + }, + "updates an existing quoted variable in-place": { + existingEnv: "FOO=\"old_value\"\nBAR=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "FOO=\"new_value\"\nBAR=keep\n", + }, + "updates a variable with export prefix": { + existingEnv: "export SECRET=old_secret\nOTHER=keep\n", + writeExisting: true, + name: "SECRET", + value: "new_secret", + expectedFile: "export SECRET=\"new_secret\"\nOTHER=keep\n", + }, + "escapes special characters in values": { + name: "SPECIAL", + value: "has \"quotes\" and $vars and \\ backslash", + expectedFile: "SPECIAL=\"has \\\"quotes\\\" and \\$vars and \\\\ backslash\"\n", + }, + "replaces a multiline value in-place": { + existingEnv: "export DB_KEY=\"---START---\npassword\n---END---\"\nOTHER=keep\n", + writeExisting: true, + name: "DB_KEY", + value: "new_key", + expectedFile: "export DB_KEY=\"new_key\"\nOTHER=keep\n", + }, + "returns error for value that cannot round-trip": { + name: "KEY", + value: `idk\`, + expectErr: slackerror.ErrDotEnvVarMarshal, + }, + "round-trips through Read": { + name: "ROUND_TRIP", + value: "hello world", + expectedFile: "ROUND_TRIP=\"hello world\"\n", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + fs := afero.NewMemMapFs() + if tc.writeExisting { + err := afero.WriteFile(fs, ".env", []byte(tc.existingEnv), 0644) + assert.NoError(t, err) + } + err := Set(fs, tc.name, tc.value) + if tc.expectErr != "" { + var slackErr *slackerror.Error + require.ErrorAs(t, err, &slackErr) + assert.Equal(t, tc.expectErr, slackErr.Code) + return + } + assert.NoError(t, err) + content, err := afero.ReadFile(fs, ".env") + assert.NoError(t, err) + assert.Equal(t, tc.expectedFile, string(content)) + }) + } +} diff --git a/internal/slackerror/errors.go b/internal/slackerror/errors.go index 4b5b50a7..807eb16b 100644 --- a/internal/slackerror/errors.go +++ b/internal/slackerror/errors.go @@ -99,6 +99,8 @@ const ( ErrDocsSearchFlagRequired = "docs_search_flag_required" ErrDotEnvFileParse = "dotenv_file_parse_error" ErrDotEnvFileRead = "dotenv_file_read_error" + ErrDotEnvFileWrite = "dotenv_file_write_error" + ErrDotEnvVarMarshal = "dotenv_var_marshal_error" ErrEnterpriseNotFound = "enterprise_not_found" ErrFailedAddingCollaborator = "failed_adding_collaborator" ErrFailedCreatingApp = "failed_creating_app" @@ -704,6 +706,16 @@ Otherwise start your app for local development with: %s`, Message: "Failed to read the .env file", }, + ErrDotEnvFileWrite: { + Code: ErrDotEnvFileWrite, + Message: "Failed to write the .env file", + }, + + ErrDotEnvVarMarshal: { + Code: ErrDotEnvVarMarshal, + Message: "Failed to marshal the .env variable", + }, + ErrEnterpriseNotFound: { Code: ErrEnterpriseNotFound, Message: "The `enterprise` was not found", From b85d49ae993635f9f63c388568c5fe529c39c66b Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 16:06:33 -0700 Subject: [PATCH 03/15] chore: claude --- .claude/settings.json | 1 + 1 file changed, 1 insertion(+) diff --git a/.claude/settings.json b/.claude/settings.json index 9d288d05..d8f7e469 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -16,6 +16,7 @@ "Bash(git log:*)", "Bash(git status:*)", "Bash(go build:*)", + "Bash(go doc:*)", "Bash(go mod graph:*)", "Bash(go mod tidy:*)", "Bash(go mod tidy:*)", From ed80e34ffdd975d6a8e04445cd3948f91cff8743 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 16:12:48 -0700 Subject: [PATCH 04/15] docs: be truthful about earlier writes and later marshals --- internal/slackdotenv/slackdotenv.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/slackdotenv/slackdotenv.go b/internal/slackdotenv/slackdotenv.go index f4191d38..8a0b4b31 100644 --- a/internal/slackdotenv/slackdotenv.go +++ b/internal/slackdotenv/slackdotenv.go @@ -106,8 +106,8 @@ func Set(fs afero.Fs, name string, value string) error { oldMarshaledValue := strings.TrimPrefix(oldEntry, name+"=") // Try each possible form of the old entry, longest (most specific) first. - // For multiline values, also try the double-quoted raw form since the file - // stores actual newlines while Marshal escapes them. + // The file can store multiline values with actual newlines, so also try + // the double-quoted raw form. entries := []string{ "export " + name + "=" + oldMarshaledValue, "export " + name + "=" + `"` + oldValue + `"`, From ae490ab3a3bafe755260aef00c7e42167bffec47 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 16:17:57 -0700 Subject: [PATCH 05/15] test: assert file contents --- cmd/env/add_test.go | 46 ++---------------------- internal/slackdotenv/slackdotenv_test.go | 2 +- 2 files changed, 4 insertions(+), 44 deletions(-) diff --git a/cmd/env/add_test.go b/cmd/env/add_test.go index 9aa28507..5b8f5243 100644 --- a/cmd/env/add_test.go +++ b/cmd/env/add_test.go @@ -205,57 +205,17 @@ func Test_Env_AddCommand(t *testing.T) { }, }, "add a variable to the .env file for non-hosted app": { - CmdArgs: []string{"ENV_NAME", "ENV_VALUE"}, - Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddDotenvMocks(ctx, cm, cf) - }, - ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.API.AssertNotCalled(t, "AddVariable") - cm.IO.AssertCalled( - t, - "PrintTrace", - mock.Anything, - slacktrace.EnvAddSuccess, - mock.Anything, - ) - }, - }, - "add a variable preserving existing variables and comments in .env": { CmdArgs: []string{"NEW_VAR", "new_value"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { setupEnvAddDotenvMocks(ctx, cm, cf) - err := afero.WriteFile(cf.Fs, ".env", []byte("# Config\nEXISTING=value\n"), 0644) + err := afero.WriteFile(cf.Fs, ".env", []byte("# Config\nEXISTING=value\n"), 0600) assert.NoError(t, err) }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "AddVariable") - }, - }, - "overwrite an existing variable in .env file": { - CmdArgs: []string{"MY_VAR", "new_value"}, - Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddDotenvMocks(ctx, cm, cf) - err := afero.WriteFile(cf.Fs, ".env", []byte("MY_VAR=old_value\n"), 0644) + content, err := afero.ReadFile(cm.Fs, ".env") assert.NoError(t, err) - }, - ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.API.AssertNotCalled(t, "AddVariable") - }, - }, - "create .env file when it does not exist": { - CmdArgs: []string{"FIRST_VAR", "first_value"}, - Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - setupEnvAddDotenvMocks(ctx, cm, cf) - }, - ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.API.AssertNotCalled(t, "AddVariable") - cm.IO.AssertCalled( - t, - "PrintTrace", - mock.Anything, - slacktrace.EnvAddSuccess, - mock.Anything, - ) + assert.Equal(t, "# Config\nEXISTING=value\nNEW_VAR=\"new_value\"\n", string(content)) }, }, }, func(cf *shared.ClientFactory) *cobra.Command { diff --git a/internal/slackdotenv/slackdotenv_test.go b/internal/slackdotenv/slackdotenv_test.go index 27215a15..af6cfd9f 100644 --- a/internal/slackdotenv/slackdotenv_test.go +++ b/internal/slackdotenv/slackdotenv_test.go @@ -195,7 +195,7 @@ func Test_Set(t *testing.T) { t.Run(name, func(t *testing.T) { fs := afero.NewMemMapFs() if tc.writeExisting { - err := afero.WriteFile(fs, ".env", []byte(tc.existingEnv), 0644) + err := afero.WriteFile(fs, ".env", []byte(tc.existingEnv), 0600) assert.NoError(t, err) } err := Set(fs, tc.name, tc.value) From bac8afa20f42e1ae53995e40ccec90aec6220557 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 16:30:47 -0700 Subject: [PATCH 06/15] feat: output the project or app environment variable specifics added --- cmd/env/add.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index 2456446d..095f89a3 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -137,23 +137,27 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg if err != nil { return err } + clients.IO.PrintTrace(ctx, slacktrace.EnvAddSuccess) + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "evergreen_tree", + Text: "App Environment", + Secondary: []string{ + fmt.Sprintf("Successfully added \"%s\" as an app environment variable", variableName), + }, + })) } else { err = slackdotenv.Set(clients.Fs, variableName, variableValue) if err != nil { return err } + clients.IO.PrintTrace(ctx, slacktrace.EnvAddSuccess) + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "evergreen_tree", + Text: "App Environment", + Secondary: []string{ + fmt.Sprintf("Successfully added \"%s\" as a project environment variable", variableName), + }, + })) } - - clients.IO.PrintTrace(ctx, slacktrace.EnvAddSuccess) - clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ - Emoji: "evergreen_tree", - Text: "App Environment", - Secondary: []string{ - fmt.Sprintf( - "Successfully added \"%s\" as an environment variable", - variableName, - ), - }, - })) return nil } From 98a23ea24a84ffafad08a8ecec1db6b6c9246122 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 22:21:52 -0700 Subject: [PATCH 07/15] test: confirm numeric values from prompt are saved to file --- cmd/env/add_test.go | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/cmd/env/add_test.go b/cmd/env/add_test.go index 5b8f5243..377dda9f 100644 --- a/cmd/env/add_test.go +++ b/cmd/env/add_test.go @@ -204,6 +204,41 @@ func Test_Env_AddCommand(t *testing.T) { ) }, }, + "add a numeric variable using prompts to the .env file": { + CmdArgs: []string{}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvAddDotenvMocks(ctx, cm, cf) + cm.IO.On( + "InputPrompt", + mock.Anything, + "Variable name", + mock.Anything, + ).Return( + "PORT", + nil, + ) + cm.IO.On( + "PasswordPrompt", + mock.Anything, + "Variable value", + iostreams.MatchPromptConfig(iostreams.PasswordPromptConfig{ + Flag: cm.Config.Flags.Lookup("value"), + }), + ).Return( + iostreams.PasswordPromptResponse{ + Prompt: true, + Value: "3000", + }, + nil, + ) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "AddVariable") + content, err := afero.ReadFile(cm.Fs, ".env") + assert.NoError(t, err) + assert.Equal(t, "PORT=3000\n", string(content)) + }, + }, "add a variable to the .env file for non-hosted app": { CmdArgs: []string{"NEW_VAR", "new_value"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { From 774e2b1f342ee3600b51b3d1914dd83866105151 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Mon, 30 Mar 2026 22:22:06 -0700 Subject: [PATCH 08/15] docs: update env help --- cmd/env/env.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cmd/env/env.go b/cmd/env/env.go index 78a93e10..abf6e9ea 100644 --- a/cmd/env/env.go +++ b/cmd/env/env.go @@ -36,11 +36,13 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { Aliases: []string{"var", "vars", "variable", "variables"}, Short: "Add, remove, or list environment variables", Long: strings.Join([]string{ - "Add, remove, or list environment variables for apps deployed to Slack managed", - "infrastructure.", + "Add, remove, or list environment variables for the app.", "", - "This command is supported for apps deployed to Slack managed infrastructure but", - "other apps can attempt to run the command with the --force flag.", + "Commands that run in the context of a project source environment variables from", + "the \".env\" file. This includes the \"run\" command.", + "", + "The \"deploy\" command gathers environment variables from the \".env\" file as well", + "unless the app is using ROSI features.", "", `Explore more: {{LinkText "https://docs.slack.dev/tools/slack-cli/guides/using-environment-variables-with-the-slack-cli"}}`, }, "\n"), From 51641d2e7a9e7213534541208a85ba0c3e9a5066 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 14:13:34 -0700 Subject: [PATCH 09/15] docs: note env commands are applied to the project Co-authored-by: Michael Brooks --- cmd/env/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index 095f89a3..bbd12879 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -32,9 +32,9 @@ import ( func NewEnvAddCommand(clients *shared.ClientFactory) *cobra.Command { cmd := &cobra.Command{ Use: "add [flags]", - Short: "Add an environment variable to the app", + Short: "Add an environment variable to the project", Long: strings.Join([]string{ - "Add an environment variable to the app.", + "Add an environment variable to the project.", "", "If a name or value is not provided, you will be prompted to provide these.", "", From 9d1972258e3e41f1e6a72e62e7850cc3c209e55d Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 15:25:55 -0700 Subject: [PATCH 10/15] fix: capture edge cases with regex --- internal/slackdotenv/slackdotenv.go | 57 +++++++++------------ internal/slackdotenv/slackdotenv_test.go | 63 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 34 deletions(-) diff --git a/internal/slackdotenv/slackdotenv.go b/internal/slackdotenv/slackdotenv.go index 8a0b4b31..df3a233d 100644 --- a/internal/slackdotenv/slackdotenv.go +++ b/internal/slackdotenv/slackdotenv.go @@ -21,6 +21,7 @@ package slackdotenv import ( "os" + "regexp" "strings" "github.com/joho/godotenv" @@ -89,7 +90,7 @@ func Set(fs afero.Fs, name string, value string) error { content := string(raw) // If the key is new, append the entry. - oldValue, found := existing[name] + _, found := existing[name] if !found { if len(content) > 0 && !strings.HasSuffix(content, "\n") { content += "\n" @@ -97,41 +98,29 @@ func Set(fs afero.Fs, name string, value string) error { return writeFile(fs, []byte(content+newEntry+"\n")) } - // Marshal the old value to find its canonical form in the file. - oldEntry, err := godotenv.Marshal(map[string]string{name: oldValue}) - if err != nil { - return slackerror.Wrap(err, slackerror.ErrDotEnvVarMarshal). - WithMessage("Failed to marshal the .env variable: %s", err) - } - oldMarshaledValue := strings.TrimPrefix(oldEntry, name+"=") + // Build a regex that matches any form of the existing entry, allowing + // optional spaces around the equals sign and optional export prefix. + // The value portion matches to the end of the line, handling quoted + // (single, double, backtick) and unquoted values, including multiline + // double-quoted values with embedded newlines. + re := regexp.MustCompile( + `(?m)(^[^\S\n]*export[^\S\n]+|^[^\S\n]*)` + regexp.QuoteMeta(name) + `[^\S\n]*=[^\S\n]*` + + `(?:` + + `"(?:[^"\\]|\\.)*"` + // double-quoted (with escapes) + `|'[^']*'` + // single-quoted + "|`[^`]*`" + // backtick-quoted + `|[^\n]*` + // unquoted to end of line + `)`, + ) - // Try each possible form of the old entry, longest (most specific) first. - // The file can store multiline values with actual newlines, so also try - // the double-quoted raw form. - entries := []string{ - "export " + name + "=" + oldMarshaledValue, - "export " + name + "=" + `"` + oldValue + `"`, - "export " + name + "=" + oldValue, - "export " + name + "=" + "'" + oldValue + "'", - name + "=" + oldMarshaledValue, - name + "=" + `"` + oldValue + `"`, - name + "=" + oldValue, - name + "=" + "'" + oldValue + "'", - } - - replaced := false - for _, entry := range entries { - if strings.Contains(content, entry) { - if strings.HasPrefix(entry, "export ") { - content = strings.Replace(content, entry, "export "+newEntry, 1) - } else { - content = strings.Replace(content, entry, newEntry, 1) - } - replaced = true - break + loc := re.FindStringIndex(content) + if loc != nil { + prefix := "" + if strings.Contains(content[loc[0]:loc[1]], "export") { + prefix = "export " } - } - if !replaced { + content = content[:loc[0]] + prefix + newEntry + content[loc[1]:] + } else { if !strings.HasSuffix(content, "\n") { content += "\n" } diff --git a/internal/slackdotenv/slackdotenv_test.go b/internal/slackdotenv/slackdotenv_test.go index af6cfd9f..ec750e04 100644 --- a/internal/slackdotenv/slackdotenv_test.go +++ b/internal/slackdotenv/slackdotenv_test.go @@ -190,6 +190,69 @@ func Test_Set(t *testing.T) { value: "hello world", expectedFile: "ROUND_TRIP=\"hello world\"\n", }, + "updates a variable with spaces around equals": { + existingEnv: "BEFORE=keep\nFOO = old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates a variable with space before equals": { + existingEnv: "BEFORE=keep\nFOO =old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates a variable with space after equals": { + existingEnv: "BEFORE=keep\nFOO= old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates an existing empty value": { + existingEnv: "BEFORE=keep\nFOO=\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates an existing empty value with spaces": { + existingEnv: "BEFORE=keep\nFOO = \nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates export variable with spaces around equals": { + existingEnv: "BEFORE=keep\nexport FOO = old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nexport FOO=\"new_value\"\nAFTER=keep\n", + }, + "updates a variable with leading spaces": { + existingEnv: "BEFORE=keep\n FOO=old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates a variable with leading tab": { + existingEnv: "BEFORE=keep\n\tFOO=old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\"\nAFTER=keep\n", + }, + "updates export variable with leading spaces": { + existingEnv: "BEFORE=keep\n export FOO=old_value\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nexport FOO=\"new_value\"\nAFTER=keep\n", + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { From 3e6fff1ea3e7d6cd789d09567e44e6955b1091ea Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 21:07:53 -0700 Subject: [PATCH 11/15] style: backtick to avoid escaped quotes inlined Co-authored-by: Michael Brooks --- cmd/env/add.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index bbd12879..d4ed8786 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -39,9 +39,9 @@ func NewEnvAddCommand(clients *shared.ClientFactory) *cobra.Command { "If a name or value is not provided, you will be prompted to provide these.", "", "Commands that run in the context of a project source environment variables from", - "the \".env\" file. This includes the \"run\" command.", + `the ".env" file. This includes the "run" command.`, "", - "The \"deploy\" command gathers environment variables from the \".env\" file as well", + `The "deploy" command gathers environment variables from the ".env" file as well`, "unless the app is using ROSI features.", }, "\n"), Example: style.ExampleCommandsf([]style.ExampleCommand{ From aa718e0aff11caea2dd7d72dc9e3c5e465b75098 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 21:09:32 -0700 Subject: [PATCH 12/15] docs: make the value argument optional in description Co-authored-by: Michael Brooks --- cmd/env/add.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index d4ed8786..187348cf 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -31,7 +31,7 @@ import ( func NewEnvAddCommand(clients *shared.ClientFactory) *cobra.Command { cmd := &cobra.Command{ - Use: "add [flags]", + Use: "add [value] [flags]", Short: "Add an environment variable to the project", Long: strings.Join([]string{ "Add an environment variable to the project.", From 3fe1cbf0d50dbdad4a0983dc00a0f583ff1fabbb Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 21:34:08 -0700 Subject: [PATCH 13/15] fix: avoid removing inline comments --- internal/slackdotenv/slackdotenv.go | 17 +++++++++++------ internal/slackdotenv/slackdotenv_test.go | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/internal/slackdotenv/slackdotenv.go b/internal/slackdotenv/slackdotenv.go index df3a233d..2cae1533 100644 --- a/internal/slackdotenv/slackdotenv.go +++ b/internal/slackdotenv/slackdotenv.go @@ -109,17 +109,22 @@ func Set(fs afero.Fs, name string, value string) error { `"(?:[^"\\]|\\.)*"` + // double-quoted (with escapes) `|'[^']*'` + // single-quoted "|`[^`]*`" + // backtick-quoted - `|[^\n]*` + // unquoted to end of line - `)`, + `|(?:[^\s\n#]|\S#)*` + // unquoted: stop before inline comment (space + #) + `)` + + `([^\S\n]+#[^\n]*)?`, // optional inline comment ) - loc := re.FindStringIndex(content) - if loc != nil { + match := re.FindStringSubmatchIndex(content) + if match != nil { prefix := "" - if strings.Contains(content[loc[0]:loc[1]], "export") { + if strings.Contains(content[match[0]:match[1]], "export") { prefix = "export " } - content = content[:loc[0]] + prefix + newEntry + content[loc[1]:] + comment := "" + if match[4] >= 0 { + comment = content[match[4]:match[5]] + } + content = content[:match[0]] + prefix + newEntry + comment + content[match[1]:] } else { if !strings.HasSuffix(content, "\n") { content += "\n" diff --git a/internal/slackdotenv/slackdotenv_test.go b/internal/slackdotenv/slackdotenv_test.go index ec750e04..e5586120 100644 --- a/internal/slackdotenv/slackdotenv_test.go +++ b/internal/slackdotenv/slackdotenv_test.go @@ -253,6 +253,27 @@ func Test_Set(t *testing.T) { value: "new_value", expectedFile: "BEFORE=keep\nexport FOO=\"new_value\"\nAFTER=keep\n", }, + "preserves inline comment on unquoted value": { + existingEnv: "BEFORE=keep\nFOO=old_value # important note\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\" # important note\nAFTER=keep\n", + }, + "preserves inline comment on quoted value": { + existingEnv: "BEFORE=keep\nFOO=\"old_value\" # important note\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nFOO=\"new_value\" # important note\nAFTER=keep\n", + }, + "preserves inline comment on export variable": { + existingEnv: "BEFORE=keep\nexport FOO=old_value # important note\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + value: "new_value", + expectedFile: "BEFORE=keep\nexport FOO=\"new_value\" # important note\nAFTER=keep\n", + }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { From 7f314926c7d0b433633b9d72d5001af1ec8c2eb4 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 21:36:37 -0700 Subject: [PATCH 14/15] test: make case specific to comment expected Co-authored-by: Michael Brooks --- internal/slackdotenv/slackdotenv_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/slackdotenv/slackdotenv_test.go b/internal/slackdotenv/slackdotenv_test.go index e5586120..56294283 100644 --- a/internal/slackdotenv/slackdotenv_test.go +++ b/internal/slackdotenv/slackdotenv_test.go @@ -140,7 +140,7 @@ func Test_Set(t *testing.T) { value: "new_value", expectedFile: "EXISTING=value\nNEW_VAR=\"new_value\"\n", }, - "adds a variable preserving comments and blank lines": { + "adds a variable preserving newline comments and blank lines": { existingEnv: "# Database config\nDB_HOST=localhost\n\n# API keys\nAPI_KEY=secret\n", writeExisting: true, name: "NEW_VAR", From f708074b00149d228d1261c17572e744b20f4137 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 31 Mar 2026 21:56:31 -0700 Subject: [PATCH 15/15] feat: output a note of version control recommendations --- cmd/env/add.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/cmd/env/add.go b/cmd/env/add.go index 187348cf..a9c6bb47 100644 --- a/cmd/env/add.go +++ b/cmd/env/add.go @@ -26,6 +26,7 @@ import ( "github.com/slackapi/slack-cli/internal/slackdotenv" "github.com/slackapi/slack-cli/internal/slacktrace" "github.com/slackapi/slack-cli/internal/style" + "github.com/spf13/afero" "github.com/spf13/cobra" ) @@ -146,17 +147,24 @@ func runEnvAddCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, arg }, })) } else { + exists, err := afero.Exists(clients.Fs, ".env") + if err != nil { + return err + } err = slackdotenv.Set(clients.Fs, variableName, variableValue) if err != nil { return err } clients.IO.PrintTrace(ctx, slacktrace.EnvAddSuccess) + var details []string + if !exists { + details = append(details, "Created a project .env file that shouldn't be added to version control") + } + details = append(details, fmt.Sprintf("Successfully added \"%s\" as a project environment variable", variableName)) clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ - Emoji: "evergreen_tree", - Text: "App Environment", - Secondary: []string{ - fmt.Sprintf("Successfully added \"%s\" as a project environment variable", variableName), - }, + Emoji: "evergreen_tree", + Text: "App Environment", + Secondary: details, })) } return nil