diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 2f9d68b..956c409 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -19,7 +19,9 @@ implementation changes were made in this review. ### 1. Keep shell completion data out of Zsh evaluation -Priority: P1. Status: open. +Priority: P1. Status: complete. + +- [x] Completed: Bash and Zsh pass literal query argv without evaluation; Zsh uses direct argument-safe completion calls. Quoted and escaped Tab-only regressions and the existing Bash/Zsh adapter matrices pass. [Zsh script adjustment](../../internal/cobra/completion.go#L146) leaves the generator's `eval _describe ... $flagPrefix` in place. The generator builds diff --git a/internal/cobra/completion.go b/internal/cobra/completion.go index a5a68be..03c4404 100644 --- a/internal/cobra/completion.go +++ b/internal/cobra/completion.go @@ -96,6 +96,46 @@ func CompletionScript(w io.Writer, executable, shell string) error { func adjustBashCompletion(script, name string) (string, error) { var err error + script, err = replaceCompletionSection(script, " local requestComp lastParam lastChar args", " # Extract the directive", ` local -a requestWords=() + local word char quote decoded next i backtick=$'\x60' + # Bash's completion helpers supply shell spelling, not evaluated argv. + # Remove quoting and escapes without expanding substitutions or globs. + for word in "${words[@]}"; do + quote= + decoded= + for ((i=0; i<${#word}; i++)); do + char=${word:i:1} + case "$quote:$char" in + "':'"|'":"') quote= ;; + ':"'|":'") quote=$char ;; + :\\|'"':\\) + if ((i+1 < ${#word})); then + next=${word:i+1:1} + if [[ $quote == '"' && $next != '\' && $next != '"' && $next != '$' && $next != "$backtick" ]]; then + decoded+=$char + continue + fi + ((i+=1)) + decoded+=${word:i:1} + else + decoded+=$char + fi ;; + *) decoded+=$char ;; + esac + done + requestWords+=("$decoded") + done + if [[ -z $cur && ${words[${#words[@]}-1]} != *= ]]; then + # The Bash helpers may omit the empty word at a trailing space. + if ((${#requestWords[@]} <= cword)); then requestWords+=(""); fi + fi + if [[ ${cur} == -*=* ]]; then cur="${cur#*=}"; fi + out=$(COBRA_ACTIVE_HELP=0 "${requestWords[0]}" __complete "${requestWords[@]:1}" 2>/dev/null) + +`) + if err != nil { + return "", err + } script, err = replaceCompletionText(script, `local out directive`, `local out directive completionPrefix completionOriginalCur`) if err != nil { return "", err @@ -145,29 +185,68 @@ func adjustBashCompletion(script, name string) (string, error) { func adjustZshCompletion(script string) (string, error) { var err error - script, err = replaceCompletionText(script, `words=("${=words[1,CURRENT]}")`, `words=("${(@)words[1,CURRENT]}")`) + script, err = replaceCompletionText(script, `words=("${=words[1,CURRENT]}")`, `words=("${(@Q)words[1,CURRENT]}")`) if err != nil { return "", err } - script, err = replaceCompletionText(script, `=~ '-.*='`, `=~ '-[^=]*='`) + script, err = replaceCompletionSection(script, " # For zsh, when completing a flag", " # Extract the directive", ` local -a prefixArgs orderArgs spaceArgs + if [[ "$lastParam" == -*=* ]]; then + flagPrefix="${lastParam%%=*}=" + fi + out=$(COBRA_ACTIVE_HELP=0 "${words[1]}" __complete "${words[@]:1}" 2>/dev/null) + +`) if err != nil { return "", err } - return replaceCompletionText(script, `local activeHelpMarker="_activeHelp_ "`, `if [[ "$out" == `+completionPathPrefixMarker+`-* ]]; then + script, err = replaceCompletionText(script, `local activeHelpMarker="_activeHelp_ "`, `if [[ "$out" == `+completionPathPrefixMarker+`-* ]]; then local pathPrefix="${out%%$'\n'*}" pathPrefix="${pathPrefix#`+completionPathPrefixMarker+`}" if [[ "$pathPrefix" != --* && "$lastParam" == "$pathPrefix"* ]]; then out="${out#*$'\n'}" - if [ $((directive & shellCompDirectiveNoFileComp)) -ne 0 ]; then - flagPrefix="-P ${pathPrefix}" - else - flagPrefix="" - compset -P "${(b)pathPrefix}" - fi + flagPrefix="$pathPrefix" fi fi + if [[ -n "$flagPrefix" ]]; then + prefixArgs=(-P "$flagPrefix") + fi local activeHelpMarker="_activeHelp_ "`) + if err != nil { + return "", err + } + return replaceCompletionSection(script, " if [ $((directive & shellCompDirectiveNoSpace))", "\n}\n", ` if [ $((directive & shellCompDirectiveNoSpace)) -ne 0 ]; then + spaceArgs=(-S "") + fi + if [ $((directive & shellCompDirectiveKeepOrder)) -ne 0 ]; then + orderArgs=(-V) + fi + + if [ $((directive & shellCompDirectiveNoFileComp)) -ne 0 ]; then + _describe "${orderArgs[@]}" completions completions "${prefixArgs[@]}" "${spaceArgs[@]}" + else + if [[ -n "$flagPrefix" ]]; then + compset -P "${(b)flagPrefix}" + fi + if [ $((directive & shellCompDirectiveFilterDirs)) -ne 0 ]; then + _files -/ + else + _files + fi + fi +`) +} + +func replaceCompletionSection(script, start, end, replacement string) (string, error) { + i := strings.Index(script, start) + if i < 0 { + return "", fmt.Errorf("unsupported generated completion script") + } + j := strings.Index(script[i+len(start):], end) + if j < 0 { + return "", fmt.Errorf("unsupported generated completion script") + } + return script[:i] + replacement + script[i+len(start)+j:], nil } func replaceCompletionText(script, old, replacement string) (string, error) { diff --git a/internal/cobra/completion_test.go b/internal/cobra/completion_test.go index a348ccb..cb211d9 100644 --- a/internal/cobra/completion_test.go +++ b/internal/cobra/completion_test.go @@ -161,6 +161,32 @@ sys.stdout.write(os.environ['PROBE_REPLY']) } }) } + if shell == "bash" || shell == "zsh" { + for _, tc := range []struct{ name, line string }{ + {"quoted", `probe --format '-$(touch sentinel)=x'`}, + {"escaped", `probe --format -\$\(touch\ sentinel\)=x`}, + } { + t.Run(shell+"/literal query/"+tc.name, func(t *testing.T) { + log := filepath.Join(dir, "query-log") + if err := os.WriteFile(log, nil, 0o600); err != nil { + t.Fatal(err) + } + cmd := exec.Command("python3", "testdata/completion_terminal.py", shell, script, tc.line) + cmd.Env = append(os.Environ(), "PATH="+dir+string(os.PathListSeparator)+os.Getenv("PATH"), "HOME="+dir, "TERM=xterm", "PROBE_DIR="+dir, "PROBE_LOG="+log, "PROBE_TAB_ONLY=1", "PROBE_REPLY=:4\n") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("tab-only completion: %v\n%s", err, out) + } + if _, err := os.Stat(filepath.Join(dir, "sentinel")); !os.IsNotExist(err) { + t.Fatalf("completion executed literal input: %v", err) + } + got, err := os.ReadFile(log) + if err != nil || !strings.Contains(string(got), `["__complete", "--format", "-$(touch sentinel)=x"]`) { + t.Fatalf("literal query was not preserved: %q, %v", got, err) + } + }) + } + } + for _, executable := range []string{"probe.app", "café"} { t.Run(shell+"/executable "+executable, func(t *testing.T) { var generated bytes.Buffer diff --git a/internal/cobra/testdata/completion_terminal.py b/internal/cobra/testdata/completion_terminal.py index 0b13f43..a140378 100644 --- a/internal/cobra/testdata/completion_terminal.py +++ b/internal/cobra/testdata/completion_terminal.py @@ -30,6 +30,15 @@ elif shell == "fish": else: raise SystemExit(f"unsupported shell: {shell}") +tab_only = os.environ.get("PROBE_TAB_ONLY") == "1" +if tab_only: + if shell == "zsh": + setup += "; function ck_tab_done { print CK_TAB_FINISHED; }; zle -N ck_tab_done; bindkey '^X^T' ck_tab_done" + elif shell == "bash": + setup += r"""; bind -x '"\C-x\C-t": "printf CK_TAB_FINISHED"'""" + else: + raise SystemExit("tab-only probe requires Bash or Zsh") + child = pexpect.spawn(executable, args, env=os.environ.copy(), cwd=os.environ["PROBE_DIR"], encoding="utf-8", timeout=15, dimensions=(24, 120)) child.delaybeforesend = 0.05 @@ -39,7 +48,11 @@ try: child.expect_exact("PROMPT> ") child.send(line) child.send("\t") - if os.environ.get("PROBE_LIST") == "1": + if tab_only: + child.sendcontrol("x") + child.sendcontrol("t") + child.expect_exact("CK_TAB_FINISHED") + elif os.environ.get("PROBE_LIST") == "1": child.send("\t") remaining = ["Readable text", "YAML output"] captured = []