diff --git a/cmd/env/remove.go b/cmd/env/remove.go index 6e161c14..fc3a57d8 100644 --- a/cmd/env/remove.go +++ b/cmd/env/remove.go @@ -17,12 +17,14 @@ package env import ( "context" "fmt" + "sort" "strings" "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/cobra" @@ -31,15 +33,17 @@ import ( func NewEnvRemoveCommand(clients *shared.ClientFactory) *cobra.Command { cmd := &cobra.Command{ Use: "remove [flags]", - Short: "Remove an environment variable from the app", + Short: "Remove an environment variable from the project", Long: strings.Join([]string{ - "Remove an environment variable from an app deployed to Slack managed", - "infrastructure.", + "Remove an environment variable from the project.", "", "If no variable name is provided, you will be prompted to select one.", "", - "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{ { @@ -66,35 +70,33 @@ func NewEnvRemoveCommand(clients *shared.ClientFactory) *cobra.Command { return cmd } -// preRunEnvRemoveCommandFunc determines if the command is supported for a project +// preRunEnvRemoveCommandFunc determines if the command is run in a valid project // and configures flags func preRunEnvRemoveCommandFunc(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) } // runEnvRemoveCommandFunc removes an environment variable from an app func runEnvRemoveCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, args []string) error { - var ctx = cmd.Context() + ctx := cmd.Context() - // Get the workspace from the flag or prompt - selection, err := appSelectPromptFunc(ctx, clients, prompts.ShowHostedOnly, prompts.ShowInstalledAppsOnly) - if err != nil { - return err + // Hosted apps require selecting an app before gathering variable inputs. + hosted := isHostedRuntime(ctx, clients) + var selection prompts.SelectedApp + if hosted { + s, err := appSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAppsOnly) + if err != nil { + return err + } + selection = s } - // Get the variable name from the flags, args, or select from the environment + // Get the variable name from args, or prompt from the appropriate source. var variableName string if len(args) > 0 { variableName = args[0] - } else { + } else if hosted && !selection.App.IsDev { variables, err := clients.API().ListVariables( ctx, selection.Auth.Token, @@ -114,7 +116,41 @@ func runEnvRemoveCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, })) return nil } - selection, err := clients.IO.SelectPrompt( + selected, err := clients.IO.SelectPrompt( + ctx, + "Select a variable to remove", + variables, + iostreams.SelectPromptConfig{ + Flag: clients.Config.Flags.Lookup("name"), + Required: true, + }, + ) + if err != nil { + return err + } + variableName = selected.Option + } else { + dotEnv, err := slackdotenv.Read(clients.Fs) + if err != nil { + return err + } + if len(dotEnv) <= 0 { + clients.IO.PrintTrace(ctx, slacktrace.EnvRemoveSuccess) + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "evergreen_tree", + Text: "App Environment", + Secondary: []string{ + "The project has no environment variables to remove", + }, + })) + return nil + } + variables := make([]string, 0, len(dotEnv)) + for k := range dotEnv { + variables = append(variables, k) + } + sort.Strings(variables) + selected, err := clients.IO.SelectPrompt( ctx, "Select a variable to remove", variables, @@ -125,32 +161,43 @@ func runEnvRemoveCommandFunc(clients *shared.ClientFactory, cmd *cobra.Command, ) if err != nil { return err - } else { - variableName = selection.Option } + variableName = selected.Option } - err = clients.API().RemoveVariable( - ctx, - selection.Auth.Token, - selection.App.AppID, - variableName, - ) - if err != nil { - return err + // Remove the environment variable using either the Slack API method or the + // project ".env" file depending on the app hosting. + if hosted && !selection.App.IsDev { + err := clients.API().RemoveVariable( + ctx, + selection.Auth.Token, + selection.App.AppID, + variableName, + ) + if err != nil { + return err + } + clients.IO.PrintTrace(ctx, slacktrace.EnvRemoveSuccess) + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "evergreen_tree", + Text: "App Environment", + Secondary: []string{ + fmt.Sprintf("Successfully removed \"%s\" as an app environment variable", variableName), + }, + })) + } else { + err := slackdotenv.Unset(clients.Fs, variableName) + if err != nil { + return err + } + clients.IO.PrintTrace(ctx, slacktrace.EnvRemoveSuccess) + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "evergreen_tree", + Text: "App Environment", + Secondary: []string{ + fmt.Sprintf("Successfully removed \"%s\" as a project environment variable", variableName), + }, + })) } - - clients.IO.PrintTrace(ctx, slacktrace.EnvRemoveSuccess) - clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ - Emoji: "evergreen_tree", - Text: "App Environment", - Secondary: []string{ - fmt.Sprintf( - "Successfully removed \"%s\" from the app's environment variables", - variableName, - ), - }, - })) - return nil } diff --git a/cmd/env/remove_test.go b/cmd/env/remove_test.go index 28c5bb26..1b0679be 100644 --- a/cmd/env/remove_test.go +++ b/cmd/env/remove_test.go @@ -20,6 +20,7 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" + "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" @@ -27,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" @@ -34,88 +36,22 @@ import ( func Test_Env_RemoveCommandPreRun(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 := NewEnvRemoveCommand(clients) @@ -131,30 +67,40 @@ func Test_Env_RemoveCommandPreRun(t *testing.T) { func Test_Env_RemoveCommand(t *testing.T) { testutil.TableTestCommand(t, testutil.CommandTests{ - "remove a variable using arguments": { + "exit without errors when .env has no variables to remove": { + CmdArgs: []string{}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvRemoveDotenvMocks(ctx, cm, cf) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{}, nil) + cm.AppClient.Manifest = manifestMock + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "RemoveVariable") + }, + ExpectedStdoutOutputs: []string{ + "The project has no environment variables to remove", + }, + }, + "exit without errors when hosted app has zero variables": { + CmdArgs: []string{}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvRemoveHostedMocks(ctx, cm, cf) + cm.API.ExpectedCalls = nil + cm.API.On("ListVariables", mock.Anything, mock.Anything, mock.Anything).Return([]string{}, nil) + cm.API.On("RemoveVariable", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "RemoveVariable") + }, + ExpectedStdoutOutputs: []string{ + "The app has no environment variables to remove", + }, + }, + "remove a hosted variable using arguments": { CmdArgs: []string{"ENV_NAME"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - cm.API.On( - "ListVariables", - mock.Anything, - mock.Anything, - mock.Anything, - ).Return( - []string{"example"}, - nil, - ) - cm.API.On( - "RemoveVariable", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return( - nil, - ) - appSelectMock := prompts.NewAppSelectMock() - appSelectPromptFunc = appSelectMock.AppSelectPrompt - appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{}, nil) + setupEnvRemoveHostedMocks(ctx, cm, cf) }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertCalled( @@ -162,7 +108,7 @@ func Test_Env_RemoveCommand(t *testing.T) { "RemoveVariable", mock.Anything, mock.Anything, - mock.Anything, + mockApp.AppID, "ENV_NAME", ) cm.IO.AssertCalled( @@ -174,34 +120,18 @@ func Test_Env_RemoveCommand(t *testing.T) { ) }, }, - "remove a variable using prompt": { + "remove a hosted variable using prompt": { CmdArgs: []string{}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - cm.API.On( - "ListVariables", - mock.Anything, - mock.Anything, - mock.Anything, - ).Return( - []string{"example"}, - nil, - ) - cm.API.On( - "RemoveVariable", - mock.Anything, - mock.Anything, - mock.Anything, - mock.Anything, - ).Return( - nil, - ) + setupEnvRemoveHostedMocks(ctx, cm, cf) cm.IO.On( "SelectPrompt", mock.Anything, "Select a variable to remove", mock.Anything, iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{ - Flag: cm.Config.Flags.Lookup("name"), + Flag: cm.Config.Flags.Lookup("name"), + Required: true, }), ).Return( iostreams.SelectPromptResponse{ @@ -211,52 +141,33 @@ func Test_Env_RemoveCommand(t *testing.T) { }, nil, ) - appSelectMock := prompts.NewAppSelectMock() - appSelectPromptFunc = appSelectMock.AppSelectPrompt - appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{}, nil) }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.API.AssertCalled( - t, - "ListVariables", - mock.Anything, - mock.Anything, - mock.Anything, - ) cm.API.AssertCalled( t, "RemoveVariable", mock.Anything, mock.Anything, - mock.Anything, + mockApp.AppID, "SELECTED_ENV_VAR", ) }, }, - "exit without errors when prompting zero environment variables": { - CmdArgs: []string{}, + "remove a variable from the .env file for remote runtime": { + CmdArgs: []string{"SECRET"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - cm.API.On( - "ListVariables", - mock.Anything, - mock.Anything, - mock.Anything, - ).Return( - []string{}, + setupEnvRemoveDotenvMocks(ctx, cm, cf) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{AppManifest: types.AppManifest{Settings: &types.AppSettings{FunctionRuntime: types.Remote}}}, nil, ) - appSelectMock := prompts.NewAppSelectMock() - appSelectPromptFunc = appSelectMock.AppSelectPrompt - appSelectMock.On("AppSelectPrompt", mock.Anything, mock.Anything, prompts.ShowHostedOnly, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{}, nil) + cm.AppClient.Manifest = manifestMock + err := afero.WriteFile(cf.Fs, ".env", []byte("SECRET=abc\nOTHER=keep\n"), 0600) + assert.NoError(t, err) }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { - cm.API.AssertCalled( - t, - "ListVariables", - mock.Anything, - mock.Anything, - mock.Anything, - ) + cm.API.AssertNotCalled(t, "RemoveVariable") cm.IO.AssertCalled( t, "PrintTrace", @@ -264,9 +175,69 @@ func Test_Env_RemoveCommand(t *testing.T) { slacktrace.EnvRemoveSuccess, mock.Anything, ) + content, err := afero.ReadFile(cm.Fs, ".env") + assert.NoError(t, err) + assert.Equal(t, "OTHER=keep\n", string(content)) }, ExpectedStdoutOutputs: []string{ - "The app has no environment variables to remove", + "Successfully removed \"SECRET\" as a project environment variable", + }, + }, + "remove a variable from the .env file using prompt": { + CmdArgs: []string{}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvRemoveDotenvMocks(ctx, cm, cf) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{AppManifest: types.AppManifest{Settings: &types.AppSettings{FunctionRuntime: types.Remote}}}, + nil, + ) + cm.AppClient.Manifest = manifestMock + err := afero.WriteFile(cf.Fs, ".env", []byte("ALPHA=one\nBETA=two\n"), 0600) + assert.NoError(t, err) + cm.IO.On( + "SelectPrompt", + mock.Anything, + "Select a variable to remove", + mock.Anything, + iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{ + Flag: cm.Config.Flags.Lookup("name"), + Required: true, + }), + ).Return( + iostreams.SelectPromptResponse{ + Prompt: true, + Option: "ALPHA", + Index: 0, + }, + nil, + ) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "RemoveVariable") + content, err := afero.ReadFile(cm.Fs, ".env") + assert.NoError(t, err) + assert.Equal(t, "BETA=two\n", string(content)) + }, + }, + "remove a variable from the .env file when manifest fetch errors": { + CmdArgs: []string{"SECRET"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + setupEnvRemoveDotenvMocks(ctx, cm, cf) + manifestMock := &app.ManifestMockObject{} + manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return( + types.SlackYaml{}, + slackerror.New(slackerror.ErrSDKHookNotFound), + ) + cm.AppClient.Manifest = manifestMock + err := afero.WriteFile(cf.Fs, ".env", []byte("SECRET=abc\nOTHER=keep\n"), 0600) + assert.NoError(t, err) + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "RemoveVariable") + content, err := afero.ReadFile(cm.Fs, ".env") + assert.NoError(t, err) + assert.Equal(t, "OTHER=keep\n", string(content)) }, }, }, func(cf *shared.ClientFactory) *cobra.Command { @@ -275,3 +246,45 @@ func Test_Env_RemoveCommand(t *testing.T) { return cmd }) } + +// setupEnvRemoveHostedMocks prepares common mocks for hosted app tests +func setupEnvRemoveHostedMocks(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.ShowAllEnvironments, prompts.ShowInstalledAppsOnly).Return(prompts.SelectedApp{Auth: mockAuth, App: mockApp}, nil) + + cm.Config.Flags.String("name", "", "mock name flag") + + cm.API.On("ListVariables", mock.Anything, mock.Anything, mock.Anything).Return([]string{"example"}, nil) + cm.API.On("RemoveVariable", 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" +} + +// setupEnvRemoveDotenvMocks prepares common mocks for non-hosted (dotenv) app tests. +// Callers must set their own manifest mock on cm.AppClient.Manifest. +func setupEnvRemoveDotenvMocks(_ context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + cf.SDKConfig = hooks.NewSDKConfigMock() + cm.AddDefaultMocks() + + cm.Config.Flags.String("name", "", "mock name flag") +} diff --git a/internal/slackdotenv/slackdotenv.go b/internal/slackdotenv/slackdotenv.go index 2cae1533..060625b4 100644 --- a/internal/slackdotenv/slackdotenv.go +++ b/internal/slackdotenv/slackdotenv.go @@ -98,22 +98,7 @@ func Set(fs afero.Fs, name string, value string) error { return writeFile(fs, []byte(content+newEntry+"\n")) } - // 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 - `|(?:[^\s\n#]|\S#)*` + // unquoted: stop before inline comment (space + #) - `)` + - `([^\S\n]+#[^\n]*)?`, // optional inline comment - ) - + re := entryPattern(name) match := re.FindStringSubmatchIndex(content) if match != nil { prefix := "" @@ -134,6 +119,64 @@ func Set(fs afero.Fs, name string, value string) error { return writeFile(fs, []byte(content)) } +// Unset removes a single environment variable from the .env file, preserving +// comments, blank lines, and other formatting. If the file does not exist or +// the key is not found, no action is taken. +func Unset(fs afero.Fs, name string) error { + // Check for an existing .env file and parse it to detect existing keys. + existing, err := Read(fs) + if err != nil { + return err + } + if existing == nil { + return nil + } + + _, found := existing[name] + if !found { + return nil + } + + // Read the raw file content to find and remove the entry. + 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) + + re := entryPattern(name) + match := re.FindStringIndex(content) + if match != nil { + // Remove the matched entry and its trailing newline if present. + end := match[1] + if end < len(content) && content[end] == '\n' { + end++ + } + content = content[:match[0]] + content[end:] + return writeFile(fs, []byte(content)) + } + + return nil +} + +// entryPattern builds a regex that matches a .env entry for the given variable +// name. It handles optional export prefix, leading whitespace, spaces around +// the equals sign, quoted (double, single, backtick) and unquoted values +// including multiline double-quoted values, and optional inline comments. +func entryPattern(name string) *regexp.Regexp { + return 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 + `|(?:[^\s\n#]|\S#)*` + // unquoted: stop before inline comment (space + #) + `)` + + `([^\S\n]+#[^\n]*)?`, // optional inline comment + ) +} + // writeFile writes data to the .env file, wrapping any error with a structured // error code. func writeFile(fs afero.Fs, data []byte) error { diff --git a/internal/slackdotenv/slackdotenv_test.go b/internal/slackdotenv/slackdotenv_test.go index 56294283..00d0e286 100644 --- a/internal/slackdotenv/slackdotenv_test.go +++ b/internal/slackdotenv/slackdotenv_test.go @@ -15,6 +15,7 @@ package slackdotenv import ( + "os" "testing" "github.com/slackapi/slack-cli/internal/slackerror" @@ -296,3 +297,150 @@ func Test_Set(t *testing.T) { }) } } + +func Test_Unset(t *testing.T) { + tests := map[string]struct { + existingEnv string + writeExisting bool + name string + expectedFile string + }{ + "no-op when .env file does not exist": { + name: "FOO", + expectedFile: "", + }, + "no-op when key does not exist": { + existingEnv: "OTHER=value\n", + writeExisting: true, + name: "FOO", + expectedFile: "OTHER=value\n", + }, + "removes a simple key-value pair": { + existingEnv: "FOO=bar\nBAZ=qux\n", + writeExisting: true, + name: "FOO", + expectedFile: "BAZ=qux\n", + }, + "removes a quoted value": { + existingEnv: "TOKEN=\"my secret\"\nOTHER=keep\n", + writeExisting: true, + name: "TOKEN", + expectedFile: "OTHER=keep\n", + }, + "removes a key with export prefix": { + existingEnv: "export SECRET=mysecret\nOTHER=keep\n", + writeExisting: true, + name: "SECRET", + expectedFile: "OTHER=keep\n", + }, + "preserves comments and blank lines": { + existingEnv: "# Config\nFOO=bar\n\n# Keys\nAPI_KEY=secret\n", + writeExisting: true, + name: "FOO", + expectedFile: "# Config\n\n# Keys\nAPI_KEY=secret\n", + }, + "removes the only variable": { + existingEnv: "FOO=bar\n", + writeExisting: true, + name: "FOO", + expectedFile: "", + }, + "removes a multiline value": { + existingEnv: "export DB_KEY=\"---START---\npassword\n---END---\"\nOTHER=keep\n", + writeExisting: true, + name: "DB_KEY", + expectedFile: "OTHER=keep\n", + }, + "removes a variable with spaces around equals": { + existingEnv: "BEFORE=keep\nFOO = bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes a variable with space before equals": { + existingEnv: "BEFORE=keep\nFOO =bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes a variable with space after equals": { + existingEnv: "BEFORE=keep\nFOO= bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes an empty value": { + existingEnv: "BEFORE=keep\nFOO=\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes an empty value with spaces": { + existingEnv: "BEFORE=keep\nFOO = \nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes export variable with spaces around equals": { + existingEnv: "BEFORE=keep\nexport FOO = bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes a variable with leading spaces": { + existingEnv: "BEFORE=keep\n FOO=bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes a variable with leading tab": { + existingEnv: "BEFORE=keep\n\tFOO=bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes export variable with leading spaces": { + existingEnv: "BEFORE=keep\n export FOO=bar\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes a variable with inline comment": { + existingEnv: "BEFORE=keep\nFOO=bar # important note\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes a quoted variable with inline comment": { + existingEnv: "BEFORE=keep\nFOO=\"bar\" # important note\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\n", + }, + "removes an export variable with inline comment": { + existingEnv: "BEFORE=keep\nexport FOO=bar # important note\nAFTER=keep\n", + writeExisting: true, + name: "FOO", + expectedFile: "BEFORE=keep\nAFTER=keep\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), 0600) + assert.NoError(t, err) + } + err := Unset(fs, tc.name) + assert.NoError(t, err) + if !tc.writeExisting { + _, err := fs.Stat(".env") + assert.True(t, os.IsNotExist(err)) + return + } + content, err := afero.ReadFile(fs, ".env") + assert.NoError(t, err) + assert.Equal(t, tc.expectedFile, string(content)) + }) + } +}