From 0c8ca2239744d6976b6f8e927b09be319738c86b Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 16 Aug 2026 19:09:09 -0400 Subject: [PATCH] fix: never run command action on shell completion past a double dash A trailing --generate-shell-completion is now always treated as a completion request, even when a '--' separator precedes the token being completed. Previously the request was declined in that case, which left the completion flag in the arguments and ran the command action (https://github.com/urfave/cli/issues/1993). The state is recorded per run on the root command; runCompletion emits nothing past a '--' because only positional arguments are accepted after it, while completing '--' itself still suggests flags. --- command.go | 4 ++ command_run.go | 5 ++ completion_test.go | 112 +++++++++++++++++++++++++++++++++++++++++++-- help.go | 18 ++++++-- help_test.go | 13 +++--- 5 files changed, 139 insertions(+), 13 deletions(-) diff --git a/command.go b/command.go index f0cf9a715d..62c3a1ab4d 100644 --- a/command.go +++ b/command.go @@ -160,6 +160,10 @@ type Command struct { didSetupDefaults bool // whether in shell completion mode shellCompletion bool + // whether the shell completion request came after a "--" separator, + // after which only positional arguments are accepted and nothing is + // suggested. The request is still a completion, never a command run. + shellCompletionPastDoubleDash bool // whether global help flag was added globaHelpFlagAdded bool // whether global version flag was added diff --git a/command_run.go b/command_run.go index 8d5907151e..7244499dcf 100644 --- a/command_run.go +++ b/command_run.go @@ -125,6 +125,11 @@ func (cmd *Command) run(ctx context.Context, osArgs []string) (_ context.Context // note that we can only do this because the shell autocomplete function // always appends the completion flag at the end of the command tracef("checking osArgs %v (cmd=%[2]q)", osArgs, cmd.Name) + // completion request state is per-run: a Command answering several + // requests (tests, REPL, embedded use) must not carry one request + // into the next + cmd.shellCompletion = false + cmd.shellCompletionPastDoubleDash = false cmd.shellCompletion, osArgs = checkShellCompleteFlag(cmd, osArgs) tracef("setting cmd.shellCompletion=%[1]v from checkShellCompleteFlag (cmd=%[2]q)", cmd.shellCompletion && cmd.EnableShellCompletion, cmd.Name) diff --git a/completion_test.go b/completion_test.go index 8550f6b41a..a3af22485b 100644 --- a/completion_test.go +++ b/completion_test.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "os" "strings" "testing" @@ -288,6 +289,9 @@ func TestCompletionSubcommand(t *testing.T) { msg string msgArgs []any notContains bool + // wantNoAction asserts that the command action must not run, even + // though shell completion is requested (https://github.com/urfave/cli/issues/1993). + wantNoAction bool }{ { name: "subcommand general completion", @@ -352,7 +356,8 @@ func TestCompletionSubcommand(t *testing.T) { msgArgs: []any{ "-g", }, - notContains: true, + notContains: true, + wantNoAction: true, }, { name: "subcommand partial double dash flag completion", @@ -377,6 +382,7 @@ func TestCompletionSubcommand(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { out := &bytes.Buffer{} + actionRan := false cmd := &Command{ EnableShellCompletion: true, @@ -389,7 +395,10 @@ func TestCompletionSubcommand(t *testing.T) { Name: "l1", }, }, - Action: func(ctx context.Context, c *Command) error { return nil }, + Action: func(ctx context.Context, c *Command) error { + actionRan = true + return nil + }, Commands: []*Command{ { Name: "xyz", @@ -401,7 +410,10 @@ func TestCompletionSubcommand(t *testing.T) { }, }, }, - Action: func(ctx context.Context, c *Command) error { return nil }, + Action: func(ctx context.Context, c *Command) error { + actionRan = true + return nil + }, }, }, }, @@ -416,10 +428,104 @@ func TestCompletionSubcommand(t *testing.T) { } else { r.Containsf(out.String(), test.contains, test.msg, test.msgArgs...) } + if test.wantNoAction { + r.False(actionRan, "command action must not run for a completion request") + } + }) + } +} + +func TestCompletionAfterDoubleDashNeverRunsAction(t *testing.T) { + // Regression test for https://github.com/urfave/cli/issues/1993: + // pressing tab on a command line that holds a "--" must never execute + // the command action, and nothing is suggested past the "--" because + // only positional arguments are accepted after it. + + tests := []struct { + name string + args []string + }{ + { + name: "root command past double dash", + args: []string{"foo", "--", "somearg", completionFlag}, + }, + { + name: "root command past double dash multiple words", + args: []string{"foo", "--", "bar", "baz", completionFlag}, + }, + { + name: "subcommand past double dash", + args: []string{"foo", "sub", "--", "somearg", completionFlag}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + out := &bytes.Buffer{} + actionRan := false + + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Action: func(ctx context.Context, c *Command) error { + actionRan = true + return nil + }, + Commands: []*Command{ + { + Name: "sub", + Action: func(ctx context.Context, c *Command) error { + actionRan = true + return nil + }, + }, + }, + } + + r := require.New(t) + r.NoError(cmd.Run(buildTestContext(t), test.args)) + r.Empty(out.String(), "no suggestions expected past a --") + r.False(actionRan, "command action must not run for a completion request") }) } } +func TestCompletionAfterDoubleDashDoesNotLeakToNextRun(t *testing.T) { + // A Command answering several completion requests must answer each one + // on its own terms: a request past a "--" must not make the next + // request behave as though it were past one too. + origArgv := os.Args + t.Cleanup(func() { os.Args = origArgv }) + + out := &bytes.Buffer{} + + cmd := &Command{ + EnableShellCompletion: true, + Writer: out, + Flags: []Flag{ + &BoolFlag{ + Name: "verbose", + }, + }, + Commands: []*Command{ + { + Name: "sub", + }, + }, + } + + r := require.New(t) + + os.Args = []string{"foo", "--", "somearg"} + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "--", "somearg", completionFlag})) + r.Empty(out.String()) + + out.Reset() + os.Args = []string{"foo", "-", completionFlag} + r.NoError(cmd.Run(buildTestContext(t), []string{"foo", "-", completionFlag})) + r.Contains(out.String(), "-verbose") +} + func TestCompletionSubcommandCustomShellComplete(t *testing.T) { out := &bytes.Buffer{} diff --git a/help.go b/help.go index a0f6c8cd5d..c91865ab13 100644 --- a/help.go +++ b/help.go @@ -487,13 +487,16 @@ func checkShellCompleteFlag(c *Command, arguments []string) (bool, []string) { return false, arguments } - // If arguments include "--" before the token being completed, shell completion - // is disabled because after "--" only positional arguments are accepted. + // If the token being completed is preceded by a "--", only positional + // arguments are accepted after it, so nothing will be suggested. // https://unix.stackexchange.com/a/11382 // Note: The token being completed is at position pos-1 (immediately before completionFlag). - // We only check arguments before that position, so completing "--" itself still works. + // A "--" at exactly that position is the token being completed, not a + // separator, so completing "--" itself still suggests flags. + // The request is still recognized as a completion so that the command + // action is never executed (https://github.com/urfave/cli/issues/1993). if pos >= 1 && slices.Contains(arguments[:pos-1], "--") { - return false, arguments[:pos] + c.shellCompletionPastDoubleDash = true } return true, arguments[:pos] @@ -520,6 +523,13 @@ func shouldRunCompletion(cmd *Command) bool { } func runCompletion(ctx context.Context, cmd *Command) { + // Nothing is suggested past a "--": after it, only positional arguments + // are accepted. The request is still treated as a completion so that the + // command action is never executed (https://github.com/urfave/cli/issues/1993). + if cmd.Root().shellCompletionPastDoubleDash { + tracef("completion requested past double dash; suggesting nothing (cmd=%[1]q)", cmd.Name) + return + } if cmd.ShellComplete != nil { tracef("running shell completion func for command %[1]q", cmd.Name) cmd.ShellComplete(ctx, cmd) diff --git a/help_test.go b/help_test.go index a726c1cce8..9e70ad0d24 100644 --- a/help_test.go +++ b/help_test.go @@ -1991,6 +1991,7 @@ func Test_checkShellCompleteFlag(t *testing.T) { cmd *Command arguments []string wantShellCompletion bool + wantPastDoubleDash bool wantArgs []string }{ { @@ -2019,12 +2020,11 @@ func Test_checkShellCompleteFlag(t *testing.T) { wantArgs: []string{"foo"}, }, { - name: "arguments include double dash", - arguments: []string{"--", "foo", completionFlag}, - cmd: &Command{ - EnableShellCompletion: true, - }, - wantShellCompletion: false, + name: "arguments include double dash", + arguments: []string{"--", "foo", completionFlag}, + cmd: &Command{EnableShellCompletion: true}, + wantShellCompletion: true, + wantPastDoubleDash: true, wantArgs: []string{"--", "foo"}, }, { @@ -2062,6 +2062,7 @@ func Test_checkShellCompleteFlag(t *testing.T) { t.Parallel() shellCompletion, args := checkShellCompleteFlag(tt.cmd, tt.arguments) assert.Equal(t, tt.wantShellCompletion, shellCompletion) + assert.Equal(t, tt.wantPastDoubleDash, tt.cmd.shellCompletionPastDoubleDash) assert.Equal(t, tt.wantArgs, args) }) }