Skip to content

fix: prevent action execution during shell completion after double dash#2342

Open
toller892 wants to merge 1 commit into
urfave:v2-maintfrom
toller892:fix/completion-after-double-dash
Open

fix: prevent action execution during shell completion after double dash#2342
toller892 wants to merge 1 commit into
urfave:v2-maintfrom
toller892:fix/completion-after-double-dash

Conversation

@toller892
Copy link
Copy Markdown

Description

When a shell autocomplete script appends --generate-bash-completion to a command line that already contains --, the completion flag is passed through to the action as a positional argument, causing the action to execute during a completion attempt.

This is particularly dangerous because tab-completion could trigger side effects (e.g., file creation, network calls) without the user pressing Enter.

Fixes #1993

Root Cause

PR #1938 added a check in checkShellCompleteFlag to disable shell completion when -- is present (correct behavior — after --, only positional args should be accepted). However, when this check fires, the function returns the unmodified arguments including --generate-bash-completion. The app then runs normally with --generate-bash-completion as a positional argument, executing the action.

Changes

help.gocheckShellCompleteFlag

  • Always strip --generate-bash-completion from arguments when it's detected (regardless of -- presence), so it never leaks into the action's positional arguments
  • Added a new return value completionDetected bool to indicate whether the completion flag was present
  • Return (false, true, stripped) when -- is present (completion disabled, but flag was detected and stripped)
  • Return (true, true, stripped) in the normal case (completion enabled, flag stripped)

app.goRunContext

  • Added early return (nil) when completionDetected && !shellComplete — the completion flag was present but shell completion is disabled (due to --). This prevents the action from executing during a completion attempt.

help_test.go

  • Updated Test_checkShellCompleteFlag to verify the new completionDetected return value
  • Updated the "arguments include double dash" test case: expected args changed from ["--", "foo", "--generate-bash-completion"] to ["--", "foo"]

Behavior Change

Scenario Before After
app --<TAB> Action executes with --generate-bash-completion as arg Returns nil, action not executed
app -- foo <TAB> Action executes with --generate-bash-completion as arg Returns nil, action not executed
app foo <TAB> Completion works normally No change
app <TAB> Completion works normally No change

When a shell autocomplete script appends --generate-bash-completion to a
command line that already contains "--", the completion flag was passed
through to the action as a positional argument, causing the action to
execute during a completion attempt.

This is particularly dangerous because tab-completion could trigger
side effects (e.g., file creation, network calls) without the user
pressing Enter.

Changes:
- checkShellCompleteFlag now strips --generate-bash-completion from
  arguments even when "--" is present, and returns a new boolean
  indicating whether the completion flag was detected
- RunContext returns early (without executing the action) when the
  completion flag was detected but shell completion is disabled
- Updated tests to verify the new behavior

Fixes urfave#1993
@toller892 toller892 requested a review from a team as a code owner May 27, 2026 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant