fix: avoid eval in fish completion requests

cmd/completion.go

@@ -1,11 +1,28 @@
 package cmd
 
 import (
+	"bytes"
+	"errors"
+	"io"
 	"os"
+	"strings"
 
 	"github.com/spf13/cobra"
 )
 
+const unsafeFishCompletionRequest = `    # Disable ActiveHelp which is not supported for fish shell
+    set -l requestComp "YQ_ACTIVE_HELP=0 $args[1] __complete $args[2..-1] $lastArg"
+
+    __yq_debug "Calling $requestComp"
+    set -l results (eval $requestComp 2> /dev/null)`
+
+const safeFishCompletionRequest = `    # Disable ActiveHelp which is not supported for fish shell
+    set -lx YQ_ACTIVE_HELP 0
+    set -l requestComp $args[1] __complete $args[2..-1] $lastArg
+
+    __yq_debug "Calling $requestComp"
+    set -l results ($requestComp 2> /dev/null)`
+
 var completionCmd = &cobra.Command{
 	Use:     "completion [bash|zsh|fish|powershell]",
 	Aliases: []string{"shell-completion"},
@@ -52,11 +69,34 @@ $ yq completion fish > ~/.config/fish/completions/yq.fish
 		case "zsh":
 			err = cmd.Root().GenZshCompletion(os.Stdout)
 		case "fish":
-			err = cmd.Root().GenFishCompletion(os.Stdout, true)
+			err = writeFishCompletion(cmd.Root(), os.Stdout)
 		case "powershell":
 			err = cmd.Root().GenPowerShellCompletion(os.Stdout)
 		}
 		return err
 
 	},
 }
+
+func writeFishCompletion(root *cobra.Command, writer io.Writer) error {
+	var script bytes.Buffer
+	if err := root.GenFishCompletion(&script, true); err != nil {
+		return err
+	}
+
+	patchedScript, err := patchFishCompletionRequest(script.String())
+	if err != nil {
+		return err
+	}
+
+	_, err = io.WriteString(writer, patchedScript)
+	return err
+}
+
+func patchFishCompletionRequest(script string) (string, error) {
+	patchedScript := strings.Replace(script, unsafeFishCompletionRequest, safeFishCompletionRequest, 1)
+	if patchedScript == script {
+		return "", errors.New("failed to patch fish completion request")
+	}
+	return patchedScript, nil
+}

cmd/root_test.go

@@ -1,6 +1,9 @@
 package cmd
 
 import (
+	"bytes"
+	"io"
+	"os"
 	"strings"
 	"testing"
 )
@@ -263,3 +266,56 @@ func TestNew_FlagCompletions(t *testing.T) {
 		}
 	}
 }
+
+func TestFishCompletionDoesNotEvalCompletionRequest(t *testing.T) {
+	output := captureStdout(t, func() {
+		rootCmd := New()
+		rootCmd.SetArgs([]string{"completion", "fish"})
+
+		if err := rootCmd.Execute(); err != nil {
+			t.Fatalf("completion fish failed: %v", err)
+		}
+	})
+
+	if strings.Contains(output, "set -l results (eval $requestComp") {
+		t.Fatal("fish completion script should not eval the completion request")
+	}
+
+	if !strings.Contains(output, "set -l requestComp $args[1] __complete $args[2..-1] $lastArg") {
+		t.Fatal("fish completion script should build the completion request as a fish argument list")
+	}
+
+	if !strings.Contains(output, "set -l results ($requestComp 2> /dev/null)") {
+		t.Fatal("fish completion script should invoke the completion request directly")
+	}
+}
+
+func captureStdout(t *testing.T, run func()) string {
+	t.Helper()
+
+	originalStdout := os.Stdout
+	reader, writer, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("failed to create stdout pipe: %v", err)
+	}
+	os.Stdout = writer
+	defer func() {
+		os.Stdout = originalStdout
+	}()
+
+	run()
+
+	if err := writer.Close(); err != nil {
+		t.Fatalf("failed to close stdout writer: %v", err)
+	}
+
+	var output bytes.Buffer
+	if _, err := io.Copy(&output, reader); err != nil {
+		t.Fatalf("failed to read stdout pipe: %v", err)
+	}
+	if err := reader.Close(); err != nil {
+		t.Fatalf("failed to close stdout reader: %v", err)
+	}
+
+	return output.String()
+}