diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 834b428..ebc40ef 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,9 +14,13 @@ jobs: - uses: actions/setup-go@v5 with: go-version-file: go.mod - - run: sudo apt-get update && sudo apt-get install -y dbus + - run: sudo apt-get update && sudo apt-get install -y dbus zsh fish - run: go vet ./... - run: go test -race ./... + # The completion scripts run in each shell; a missing one fails instead of skipping. + - run: go test -v -count=1 -run '^TestCompletionScripts' ./internal/cli + env: + TAIGA_TEST_SHELLS: bash,zsh,fish,pwsh - uses: golangci/golangci-lint-action@v8 with: version: v2.14.0 diff --git a/README.md b/README.md index a88a23f..436b267 100644 --- a/README.md +++ b/README.md @@ -235,6 +235,8 @@ Without a terminal on stdout, `taiga` prints JSON; on a terminal it prints text. Every curated command (all but `taiga api`, which prints Taiga's answer as is) hides the value of token parameters in everything it prints, on stdout and on stderr (errors and warnings), `--dry-run` plans included, keeping the keys and the rest of the URL (only the printout changes: a real write sends the values as given and as stored): user photos (`photo`, `big_photo` in `owner_extra_info`, `assigned_to_extra_info`, the comment author), project logos and attachment links are signed media URLs that open the file without authentication while the token lasts. `story get` shows `"photo": "https://…/media/user/…/photo.png?token=…"`. +Shell completion (`taiga completion bash|zsh|fish|powershell`) never calls Taiga: it completes command and flag names and leaves arguments and flag values to the shell (file names). When it cannot (an unknown command, an unknown flag, a bad flag value), the error it writes to stderr and, with `BASH_COMP_DEBUG_FILE` set, to that file has the typed words with token values hidden and control and bidi characters escaped (`\u202e`); a flag redacted whole (`[redacted: the text carries a credential]`) no longer reads as a flag, so no flag error is printed for it. The scripts that `taiga completion` prints are Cobra's, with one block added: Cobra's debug writer (`__taiga_debug`, which appends the typed command line, tokens included, to `BASH_COMP_DEBUG_FILE`) is redefined, before any completion runs, to do nothing. With the variable set, the file only gets the binary's redacted errors; what the scripts complete is Cobra's. A script saved by an earlier version still logs the command line: generate it again. + | Exit | Meaning | |---|---| | 0 | OK | diff --git a/docs/guia.md b/docs/guia.md index b68c9a0..d49879c 100644 --- a/docs/guia.md +++ b/docs/guia.md @@ -132,6 +132,8 @@ taiga story update 246 --replace-epic 15 --confirm-delete Toda saída dos comandos curados (todos menos `taiga api`, que mostra a resposta do Taiga como veio) esconde o valor dos parâmetros de token em tudo o que imprime, na saída e nos erros e avisos (stderr), inclusive nos planos de `--dry-run`, mantendo as chaves e o resto da URL. Só a impressão muda: a escrita de verdade envia os valores como vieram e como estão guardados. As fotos de usuário (`photo` e `big_photo` em `owner_extra_info`, `assigned_to_extra_info` e no autor dos comentários), o logo do projeto e os links de anexo são URLs assinadas de mídia que abrem o arquivo sem autenticação enquanto o token vale. Em `story get`, a foto sai como `"photo": "https://…/media/user/…?token=…"`. +O completion do shell (`taiga completion bash|zsh|fish|powershell`) nunca chama o Taiga: completa nomes de comandos e flags e deixa argumentos e valores de flag para o shell (nomes de arquivo). Quando não consegue (comando ou flag desconhecido, valor de flag inválido), o erro que escreve no stderr e, com `BASH_COMP_DEBUG_FILE` definida, nesse arquivo traz as palavras digitadas com o valor de token escondido e controles e bidi escapados (`\u202e`); uma flag redigida inteira (`[redacted: the text carries a credential]`) deixa de ser flag, e então nenhum erro de flag sai. Os scripts que `taiga completion` imprime são os do Cobra com um bloco a mais: o escritor de debug do Cobra (`__taiga_debug`, que grava no `BASH_COMP_DEBUG_FILE` a linha digitada, token incluído) é redefinido, antes de qualquer completion, para não fazer nada. Com a variável definida, o arquivo só recebe os erros já redigidos do binário; o que os scripts completam é o do Cobra. Um script salvo por uma versão anterior continua gravando a linha digitada: gere-o de novo. + ## Anexos ```sh diff --git a/internal/cli/completion.go b/internal/cli/completion.go new file mode 100644 index 0000000..2143d97 --- /dev/null +++ b/internal/cli/completion.go @@ -0,0 +1,155 @@ +package cli + +import ( + "bytes" + "fmt" + "regexp" + "strings" + + "github.com/BasisTI/taiga-cli/internal/output" + "github.com/BasisTI/taiga-cli/internal/taiga" + "github.com/spf13/cobra" +) + +// completeRedacted makes Cobra's hidden __complete command see the command line redacted and +// escaped, as presented elsewhere. +// +// Cobra reports a completion error (unknown command, unknown flag, bad flag value) with +// CompErrorln, which writes to os.Stderr and appends to $BASH_COMP_DEBUG_FILE on its own: no +// writer of the command reaches it, so the lineRedactor of wireOutput never sees that text. +// The message only joins fixed text, the names of commands and flags and the arguments typed, +// so redacting the arguments before Cobra reads them is what keeps a token or a control +// character out of both. Completion never calls Taiga, and TestOutputGoesThroughPresent keeps +// completion functions (which could print server text) out of the CLI. +// +// An argument changes only when it carries a token or an unsafe rune, and such an argument +// never names a command or a flag, so the candidates stay the same; the escape drops Quote's +// outer quotes so that `--flag…` still reads as a flag. +func completeRedacted(cmd *cobra.Command) { + run := cmd.Run + cmd.Run = func(c *cobra.Command, args []string) { + safe := make([]string, len(args)) + for i, s := range args { + safe[i] = completionArg(s) + } + run(c, safe) + } +} + +// completionArg is s redacted, with each unsafe rune escaped and no quotes around it. Only +// what Quote quoted loses its quotes: a redacted value can take the unsafe rune with it. +func completionArg(s string) string { + r := taiga.RedactTokens(s) + if q := output.Quote(r); q != r { + return q[1 : len(q)-1] + } + return r +} + +// completionCmd gives Cobra's `completion bash|zsh|fish|powershell` (its help and flags kept) +// scripts whose own debug log writes nothing. That log appends the command line as typed, +// tokens included, to $BASH_COMP_DEBUG_FILE; the binary's log there stays, redacted by +// completeRedacted. +func (a *App) completionCmd(root *cobra.Command) { + root.InitDefaultCompletionCmd() + for _, c := range root.Commands() { + if c.Name() != "completion" { + continue + } + for _, sh := range c.Commands() { + shell := sh.Name() + sh.RunE = func(cmd *cobra.Command, _ []string) error { + noDesc, _ := cmd.Flags().GetBool("no-descriptions") + s, err := completionScript(cmd.Root(), shell, !noDesc) + if err != nil { + return err + } + return a.writeScript(cmd, s) + } + } + } +} + +// completionScript is Cobra's script for shell with its debug writer redefined to do nothing. +func completionScript(root *cobra.Command, shell string, desc bool) (string, error) { + var b bytes.Buffer + var err error + switch shell { + case "bash": + err = root.GenBashCompletionV2(&b, desc) + case "zsh": + if desc { + err = root.GenZshCompletion(&b) + } else { + err = root.GenZshCompletionNoDesc(&b) + } + case "fish": + err = root.GenFishCompletion(&b, desc) + case "powershell": + if desc { + err = root.GenPowerShellCompletionWithDesc(&b) + } else { + err = root.GenPowerShellCompletion(&b) + } + default: + return "", fmt.Errorf("no completion script for %s", shell) + } + if err != nil { + return "", err + } + return withQuietDebug(b.String(), "__"+root.Name()+"_debug", shell) +} + +// quietWriter redefines the debug writer (%s) as a function that does nothing and returns 0, +// as the writer itself does when the variable is unset. +var quietWriter = map[string]string{"bash": "%s() { :; }", "zsh": "%s() { :; }", "fish": "function %s; end", "powershell": "function %s { }"} + +// declaration finds a function declared on a line, at any indent: "name()" and "function +// name" (bash, zsh), "function name" (fish), "function name", "filter name" and +// "[scriptblock]${name} =" (PowerShell). +var declaration = regexp.MustCompile(`^\s*(?:(?:function|filter)\s+([^\s(){};]+)|([A-Za-z_][\w:.-]*)\s*\(\)|\[scriptblock\]\$\{([^}]+)\}\s*=)`) + +// declared is the function a line declares, or "". +func declared(line string) string { + m := declaration.FindStringSubmatch(line) + if m == nil { + return "" + } + return m[1] + m[2] + m[3] +} + +// withQuietDebug is script with writer redefined to do nothing, or an error and no script. +// Nothing of Cobra's script is removed or changed: the redefinition is added before the first +// function declared at the margin after writer's (last) declaration, above the comment lines +// that go with that function. Writer's body has ended there, and nothing has run yet: zsh +// autoloaded from fpath runs the file on first use and calls the completion at its end, and +// fish completes once while loading, so the end of the script would be too late. Every call +// then goes to the redefinition, PowerShell's included (same scope as the script's own +// functions). Without writer's declaration, or with no declaration after it, the script is +// refused. +func withQuietDebug(script, writer, shell string) (string, error) { + lines := strings.SplitAfter(script, "\n") + last := -1 + for i, line := range lines { + if declared(strings.TrimRight(line, "\n")) == writer { + last = i + } + } + if last < 0 { + return "", fmt.Errorf("the %s completion script does not declare %s", shell, writer) + } + for i := last + 1; i < len(lines); i++ { + text := strings.TrimRight(lines[i], "\n") + if declared(text) == "" || text != strings.TrimLeft(text, " \t") { + continue + } + for i > last+1 && strings.HasPrefix(lines[i-1], "#") { + i-- // above the comment that goes with that function + } + quiet := "# taiga: " + writer + " above appends the command line as typed, tokens included, to\n" + + "# $BASH_COMP_DEBUG_FILE. Redefined here, before any completion runs, it writes nothing.\n" + + fmt.Sprintf(quietWriter[shell], writer) + "\n\n" + return strings.Join(lines[:i], "") + quiet + strings.Join(lines[i:], ""), nil + } + return "", fmt.Errorf("the %s completion script declares nothing after %s", shell, writer) +} diff --git a/internal/cli/completion_script_test.go b/internal/cli/completion_script_test.go new file mode 100644 index 0000000..7ef84d3 --- /dev/null +++ b/internal/cli/completion_script_test.go @@ -0,0 +1,320 @@ +package cli + +import ( + "bytes" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +var completionShells = []string{"bash", "zsh", "fish", "powershell"} + +// cobraScript is the script Cobra itself generates for shell, debug log included. +func cobraScript(t *testing.T, shell string, desc bool) string { + t.Helper() + root := (&App{}).root() + var b bytes.Buffer + var err error + switch shell { + case "bash": + err = root.GenBashCompletionV2(&b, desc) + case "zsh": + if desc { + err = root.GenZshCompletion(&b) + } else { + err = root.GenZshCompletionNoDesc(&b) + } + case "fish": + err = root.GenFishCompletion(&b, desc) + case "powershell": + if desc { + err = root.GenPowerShellCompletionWithDesc(&b) + } else { + err = root.GenPowerShellCompletion(&b) + } + } + if err != nil { + t.Fatal(err) + } + return b.String() +} + +// generatedScript is what `taiga completion shell` prints. +func generatedScript(t *testing.T, shell string, desc bool) string { + t.Helper() + args := []string{"completion", shell} + if !desc { + args = append(args, "--no-descriptions") + } + out, stderr, code := runIn(t, nil, "", args...) + if code != 0 { + t.Fatalf("%v: exit %d: %s", args, code, stderr) + } + return out +} + +// quietBlock is what `taiga completion shell` adds to Cobra's script, and all it adds. +func quietBlock(shell string) string { + def := map[string]string{"bash": "__taiga_debug() { :; }", "zsh": "__taiga_debug() { :; }", + "fish": "function __taiga_debug; end", "powershell": "function __taiga_debug { }"}[shell] + return "# taiga: __taiga_debug above appends the command line as typed, tokens included, to\n" + + "# $BASH_COMP_DEBUG_FILE. Redefined here, before any completion runs, it writes nothing.\n" + def + "\n\n" +} + +// The scripts are Cobra's with one block added after the debug writer's definition, which +// redefines the writer to do nothing; nothing else is added, removed or changed. +func TestCompletionScriptsOnlyAddTheQuietWriter(t *testing.T) { + for _, shell := range completionShells { + for _, desc := range []bool{true, false} { + got, cobra := generatedScript(t, shell, desc), cobraScript(t, shell, desc) + at := strings.Index(got, quietBlock(shell)) + if at < 0 || got[:at]+got[at+len(quietBlock(shell)):] != cobra { + t.Errorf("%s (descriptions %v) is not Cobra's script plus the quiet writer", shell, desc) + continue + } + if first := strings.Index(cobra, "__taiga_debug \""); at > first || at < strings.Index(cobra, "__taiga_debug") { + t.Errorf("%s (descriptions %v): the quiet writer at %d is not between the writer and its first call (%d)", shell, desc, at, first) + } + } + } +} + +// Without the writer's declaration under the expected name, or with nothing declared after +// it, the script is refused: an error and no script. +func TestCompletionScriptRefusedWithoutTheWriter(t *testing.T) { + cobra := cobraScript(t, "bash", true) + for name, script := range map[string]string{ + "renamed writer": strings.ReplaceAll(cobra, "__taiga_debug", "__taiga_log"), + "no writer": "_start() { :; }\n", + "nothing after": "__taiga_debug()\n{\n :\n}\n", + "declared inside": "f() {\n __taiga_debug() { :; }\n}\n", + } { + if got, err := withQuietDebug(script, "__taiga_debug", "bash"); err == nil || got != "" { + t.Errorf("%s: error %v, got %q", name, err, got) + } + } +} + +// statusCheck loads $SCRIPT in a shell, calls the writer after a failure and prints the status: +// the redefinition returns 0, as the writer itself. +var statusCheck = map[string][]string{ + "bash": {"bash", "--norc", "--noprofile", "-c", `source "$SCRIPT"; false; __taiga_debug "x"; echo "status=$?"`}, + "zsh": {"zsh", "-f", "-c", `compdef() { :; }; source "$SCRIPT"; false; __taiga_debug "x"; echo "status=$?"`}, + "fish": {"fish", "--no-config", "-c", `source $SCRIPT; false; __taiga_debug "x"; echo "status=$status"`}, + "powershell": {"pwsh", "-NoProfile", "-NonInteractive", "-Command", `. $env:SCRIPT; $out = __taiga_debug "x"; if ($? -and $null -eq $out) { "status=0" } else { "status=1" }`}, +} + +// With BASH_COMP_DEBUG_FILE set, the redefined writer returns 0, prints nothing and writes +// nothing. +func TestCompletionScriptsQuietWriter(t *testing.T) { + for _, shell := range completionShells { + t.Run(shell, func(t *testing.T) { + c := statusCheck[shell] + requireShell(t, c[0]) + dir := t.TempDir() + path, log := filepath.Join(dir, "script.ps1"), filepath.Join(dir, "debug.log") + if err := os.WriteFile(path, []byte(generatedScript(t, shell, true)), 0o600); err != nil { + t.Fatal(err) + } + cmd := exec.Command(c[0], c[1:]...) + cmd.Dir = t.TempDir() + cmd.Env = append(os.Environ(), "SCRIPT="+path, "BASH_COMP_DEBUG_FILE="+log) + out, err := cmd.Output() + if err != nil || string(out) != "status=0\n" { + t.Errorf("%v: %q", err, out) + } + if b, err := os.ReadFile(log); err == nil { + t.Errorf("the writer wrote %q", b) + } + }) + } +} + +// requireShell skips the test when exe is not installed, unless exe is bash or listed in +// TAIGA_TEST_SHELLS (comma-separated, as the CI sets it): then it fails, so no shell goes +// untested in silence. +func requireShell(t *testing.T, exe string) { + t.Helper() + if _, err := exec.LookPath(exe); err == nil { + return + } + for _, want := range append([]string{"bash"}, strings.Split(os.Getenv("TAIGA_TEST_SHELLS"), ",")...) { + if strings.TrimSpace(want) == exe { + t.Fatalf("%s is required and not installed", exe) + } + } + t.Skipf("%s is not installed", exe) +} + +// parsers check a script without running it. +var parsers = map[string][]string{ + "bash": {"bash", "-n"}, + "zsh": {"zsh", "-n"}, + "fish": {"fish", "--no-execute"}, + "powershell": {"pwsh", "-NoProfile", "-NonInteractive", "-Command", "$e = $null; $null = [System.Management.Automation.Language.Parser]::ParseFile($env:PARSE_FILE, [ref]$null, [ref]$e); if ($e) { $e; exit 1 }"}, +} + +// Each generated script parses in its shell; a shell that is not installed is skipped. +func TestCompletionScriptsParse(t *testing.T) { + for _, shell := range completionShells { + t.Run(shell, func(t *testing.T) { + p := parsers[shell] + requireShell(t, p[0]) + for _, desc := range []bool{true, false} { + path := filepath.Join(t.TempDir(), "script.ps1") + if err := os.WriteFile(path, []byte(generatedScript(t, shell, desc)), 0o600); err != nil { + t.Fatal(err) + } + cmd := exec.Command(p[0], append(p[1:], path)...) // pwsh -Command ignores the path and reads PARSE_FILE + cmd.Env = append(os.Environ(), "PARSE_FILE="+path) + if out, err := cmd.CombinedOutput(); err != nil { + t.Errorf("descriptions %v: %v\n%s", desc, err, out) + } + } + }) + } +} + +// buildTaiga builds the binary the scripts call, as `taiga` in a directory of its own. +func buildTaiga(t *testing.T) string { + t.Helper() + dir := t.TempDir() + cmd := exec.Command("go", "build", "-o", filepath.Join(dir, "taiga"), "../../cmd/taiga") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("go build: %v\n%s", err, out) + } + return dir +} + +// No word is a glob: zsh and fish refuse an unmatched one before calling the binary. +// +// Each harness loads the script, completes `taiga WORDS...` through the shell's own entry +// point and prints the candidates. $SCRIPT is the script, $BIN the binary; WORDS are the +// arguments, the last one being completed (empty after a space). +var completionHarness = map[string]struct { + shell string + args func(harness string) []string + code string +}{ + "bash": {"bash", func(h string) []string { return []string{"--norc", "--noprofile", "-c", h, "bash"} }, ` +ARGS=("$@") +source "$SCRIPT" +_init_completion() { words=("$BIN" "${ARGS[@]}"); cword=$(( ${#words[@]} - 1 )); cur=${words[cword]}; prev=${words[cword-1]}; } +compopt() { :; } +COMP_TYPE=9 +__start_taiga +printf '%s\n' "${COMPREPLY[@]}" +`}, + "zsh": {"zsh", func(h string) []string { return []string{"-f", "-c", h, "zsh"} }, ` +ARGS=("$@") +compdef() { :; } +source "$SCRIPT" +compadd() { print -r -- "compadd $*"; } +_describe() { print -r -- "describe $* :: ${(j:|:)completions}"; return 0; } +_arguments() { print -r -- "arguments $*"; } +words=("$BIN" "${ARGS[@]}") +CURRENT=${#words} +_taiga +`}, + // zsh as installed: the script is _taiga in fpath, autoloaded, and the file itself runs on + // the first completion. + "zsh-autoload": {"zsh", func(h string) []string { return []string{"-f", "-c", h, "zsh"} }, ` +ARGS=("$@") +dir=$(mktemp -d) +cp "$SCRIPT" "$dir/_taiga" +fpath=("$dir" $fpath) +compdef() { :; } +compadd() { print -r -- "compadd $*"; } +_describe() { print -r -- "describe $* :: ${(j:|:)completions}"; return 0; } +_arguments() { print -r -- "arguments $*"; } +autoload -U _taiga +words=("$BIN" "${ARGS[@]}") +CURRENT=${#words} +_taiga +rm -r "$dir" +`}, + "fish": {"fish", func(h string) []string { return []string{"--no-config", "-c", h, "--"} }, ` +source $SCRIPT +set -l words (string escape -- $argv) +if test -z "$argv[-1]" + set words $words[1..-2] '' +end +complete -C (string join ' ' -- taiga $words) +`}, + "powershell": {"pwsh", func(h string) []string { return []string{"-NoProfile", "-NonInteractive", "-Command", h} }, ` +. $env:SCRIPT +$line = 'taiga ' + ($env:WORDS -split "\n" -join ' ') +(TabExpansion2 -inputScript $line -cursorColumn $line.Length).CompletionMatches | ForEach-Object { $_.CompletionText } +`}, +} + +// complete runs the harness of shell over script with words and returns the candidates and +// the debug file. +func complete(t *testing.T, shell, bin, script string, words ...string) (out, debug string) { + t.Helper() + h := completionHarness[shell] + dir := t.TempDir() + path, log := filepath.Join(dir, "script"), filepath.Join(dir, "debug.log") + if shell == "powershell" { + path += ".ps1" // dot-sourcing runs only a .ps1 + } + if err := os.WriteFile(path, []byte(script), 0o600); err != nil { + t.Fatal(err) + } + args := h.args(h.code) + if shell != "powershell" { + args = append(args, words...) + } + cmd := exec.Command(h.shell, args...) + cmd.Dir = t.TempDir() // empty: file completion lists neither the script nor the log + cmd.Env = append(os.Environ(), "SCRIPT="+path, "BIN="+filepath.Join(bin, "taiga"), "BASH_COMP_DEBUG_FILE="+log, + "WORDS="+strings.Join(words, "\n"), "PATH="+bin+string(os.PathListSeparator)+os.Getenv("PATH"), "LC_ALL=C.UTF-8") + var stdout, stderr bytes.Buffer + cmd.Stdout, cmd.Stderr = &stdout, &stderr + if err := cmd.Run(); err != nil { + t.Fatalf("%s %q: %v\n%s%s", shell, words, err, stdout.String(), stderr.String()) + } + b, _ := os.ReadFile(log) + return stdout.String(), string(b) +} + +// With BASH_COMP_DEBUG_FILE set, completing a command line with a token, a control and a bidi +// character leaves only the binary's redacted log in the file, no line of the shell's; the candidates are those of +// Cobra's own script. A shell that is not installed is skipped. +func TestCompletionScriptsKeepTokensOutOfTheDebugFile(t *testing.T) { + bin := buildTaiga(t) + for _, harness := range []string{"bash", "zsh", "zsh-autoload", "fish", "powershell"} { + shell := strings.TrimSuffix(harness, "-autoload") + t.Run(harness, func(t *testing.T) { + requireShell(t, completionHarness[harness].shell) + for _, desc := range []bool{true, false} { + script := generatedScript(t, shell, desc) + _, debug := complete(t, harness, bin, script, "nope", "token=SWEEPSECRET", "a\u202eb", "x\x01y", "") + if strings.Contains(debug, "SWEEPSECRET") || strings.ContainsAny(debug, "\u202e\x01") { + t.Errorf("descriptions %v: debug file leaked:\n%q", desc, debug) + } + for _, line := range strings.Split(strings.TrimSuffix(debug, "\n"), "\n") { + if !strings.HasPrefix(line, "[Debug] ") { + t.Errorf("descriptions %v: the shell wrote %q", desc, line) + } + } + if !strings.Contains(debug, "[Debug] [Error] unable to find a command for arguments: [nope token=…") { + t.Errorf("descriptions %v: the binary's own log is gone:\n%q", desc, debug) + } + } + for _, desc := range []bool{true, false} { + script, cobra := generatedScript(t, shell, desc), cobraScript(t, shell, desc) + for _, words := range [][]string{{"st"}, {"story", ""}, {"story", "l"}, {"story", "list", "--outp"}, {"story", "list", "--"}, {"completion", ""}, {"attachment", "upload", "--t"}} { + got, _ := complete(t, harness, bin, script, words...) + want, _ := complete(t, harness, bin, cobra, words...) + t.Logf("descriptions %v, %q: %q", desc, words, got) + if got != want || got == "" { + t.Errorf("descriptions %v, %q: candidates %q, Cobra's script gives %q", desc, words, got, want) + } + } + } + }) + } +} diff --git a/internal/cli/completion_test.go b/internal/cli/completion_test.go new file mode 100644 index 0000000..65ea91b --- /dev/null +++ b/internal/cli/completion_test.go @@ -0,0 +1,120 @@ +package cli + +import ( + "io" + "net/http" + "os" + "path/filepath" + "strings" + "testing" +) + +// runComplete runs `taiga req args...` (req is __complete or __completeNoDesc) the way the +// shell does, with BASH_COMP_DEBUG_FILE set, and returns stdout, everything written to stderr +// (App.Err and the process stderr, where Cobra's CompErrorln writes) and the debug file. +func runComplete(t *testing.T, env map[string]string, req string, args ...string) (out, stderr, debug string) { + t.Helper() + path := filepath.Join(t.TempDir(), "comp.log") + t.Setenv("BASH_COMP_DEBUG_FILE", path) + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + saved := os.Stderr + os.Stderr = w + read := make(chan string) + go func() { + b, _ := io.ReadAll(r) + read <- string(b) + }() + stdout, appErr, code := runIn(t, env, "", append([]string{req}, args...)...) + os.Stderr = saved + _ = w.Close() + process := <-read + if code != 0 { + t.Fatalf("__complete %q: exit %d: %s%s", args, code, appErr, process) + } + b, err := os.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + return stdout, appErr + process, string(b) +} + +// Completion errors echo the command line (an unknown command, a flag name, a flag value): +// stderr and the debug file get them redacted and escaped, like every other command. +func TestCompletionHidesTokens(t *testing.T) { + for _, c := range []struct { + args []string + want string // what still identifies the error + }{ + {[]string{"nope?token=SWEEPSECRET", ""}, "unable to find a command"}, + {[]string{"nope", sweepURL, ""}, "unable to find a command"}, + {[]string{"story", "list", "--bogus?access_token=SWEEPSECRET=1", ""}, "unknown flag"}, + {[]string{"story", "list", "--url", "x", "--bogus=" + sweepURL, ""}, "unknown flag"}, + {[]string{"attachment", "upload", "--task=token=SWEEPSECRET", ""}, "invalid argument"}, + // Redacted whole, the argument is no flag any more: no error, and nothing printed. + {[]string{"story", "list", "--access_token%3DSWEEPSECRET="}, ""}, + // The control character goes with the token value: nothing is quoted, nothing unquoted. + {[]string{"nope", "\"x?token=SWEEP\x01SECRET", ""}, `[nope "x?token=…]`}, + // A word that starts with a quote stays a word, not a flag: no error, as without redaction. + {[]string{"story", "list", "\"--x?token=SWEEP\x01SECRET"}, ""}, + {[]string{"story", "list", "--a\u202eb="}, `does not support flag 'a\u202eb'`}, + {[]string{"story", "list", "--a\u202eb=1", ""}, `--a\u202eb`}, + {[]string{"nope\nforged", ""}, `nope\nforged`}, + } { + for _, req := range []string{"__complete", "__completeNoDesc"} { + out, stderr, debug := runComplete(t, nil, req, c.args...) + for name, text := range map[string]string{"stdout": out, "stderr": stderr, "debug file": debug} { + if strings.Contains(text, "SWEEPSECRET") || strings.ContainsAny(text, "\u202e") || strings.Contains(text, "\nforged") { + t.Errorf("%s %q: %s leaked: %q", req, c.args, name, text) + } + } + if c.want == "" { + if strings.Contains(stderr+debug, "[Error]") { + t.Errorf("%s %q: printed an error\nstderr: %q\ndebug: %q", req, c.args, stderr, debug) + } + continue + } + if !strings.Contains(stderr, c.want) || !strings.Contains(debug, c.want) { + t.Errorf("%s %q: the error is gone (want %q)\nstderr: %q\ndebug: %q", req, c.args, c.want, stderr, debug) + } + } + } +} + +// Completion stays what Cobra computes: redaction only touches arguments that carry a token +// or an unsafe rune, which never name a command or a flag. +func TestCompletionCandidatesUnchanged(t *testing.T) { + for _, c := range []struct { + args []string + want []string + }{ + {[]string{"story", "l"}, []string{"list\t", ":4"}}, + {[]string{"story", "list", "--url", sweepURL, "--outp"}, []string{"--output\t", ":4"}}, + {[]string{"story", "list", "--url=" + sweepURL, "--outp"}, []string{"--output\t", ":4"}}, + {[]string{"story", "list", "--a\u202eb", ""}, []string{":0"}}, + } { + out, _, _ := runComplete(t, nil, "__complete", c.args...) + for _, w := range c.want { + if !strings.Contains(out, w) { + t.Errorf("%q: want %q in %q", c.args, w, out) + } + } + } +} + +// Completion never calls Taiga, so no server text can reach it: nothing in the CLI registers +// a completion function (TestOutputGoesThroughPresent keeps it that way). +func TestCompletionNeverCallsTaiga(t *testing.T) { + srv, calls := fakeTaiga(t, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`[{"id": 1, "name": "` + sweepURL + `", "slug": "` + sweepURL + `"}]`)) + }) + env := map[string]string{"TAIGA_URL": srv.URL, "TAIGA_TOKEN": "tok", "TAIGA_PROJECT": "infra-2025"} + for _, args := range [][]string{{""}, {"story", "get", ""}, {"story", "list", "--project", ""}, {"epic", "link", ""}, {"task", "close", ""}} { + out, stderr, debug := runComplete(t, env, "__complete", args...) + if len(*calls) != 0 || strings.Contains(out+stderr+debug, "SWEEPSECRET") { + t.Fatalf("%q: %d calls\n%s%s%s", args, len(*calls), out, stderr, debug) + } + } +} diff --git a/internal/cli/present.go b/internal/cli/present.go index c6661cb..ea97e44 100644 --- a/internal/cli/present.go +++ b/internal/cli/present.go @@ -76,6 +76,13 @@ func (l *lineRedactor) flush() { } } +// writeScript prints a generated completion script on Cobra's stdout, the lineRedactor that +// carried Cobra's own script. +func (a *App) writeScript(cmd *cobra.Command, s string) error { + _, err := io.WriteString(cmd.OutOrStdout(), s) + return err +} + // emitJSON, emitFields, emitOut and emitErr write presented content, without redacting again. func (a *App) emitJSON(v any) error { return output.WriteJSON(a.Out, v) } diff --git a/internal/cli/present_test.go b/internal/cli/present_test.go index bde8796..09a0d5d 100644 --- a/internal/cli/present_test.go +++ b/internal/cli/present_test.go @@ -25,6 +25,13 @@ var outputRule = map[string][]string{ var cobraWriters = map[string]bool{"OutOrStdout": true, "OutOrStderr": true, "ErrOrStderr": true, "SetOut": true, "SetErr": true, "SetOutput": true, "Print": true, "Println": true, "Printf": true, "PrintErr": true, "PrintErrln": true, "PrintErrf": true} +// completionHooks would let completion print text of its own (server names, errors) through +// CompErrorln, which completeRedacted cannot redact. +var completionHooks = map[string]bool{"ValidArgsFunction": true, "RegisterFlagCompletionFunc": true} + +// compWriters are Cobra's completion printers: os.Stderr and $BASH_COMP_DEBUG_FILE, unredacted. +var compWriters = map[string]bool{"CompDebug": true, "CompDebugln": true, "CompError": true, "CompErrorln": true} + func allowed(rule, file string) bool { for _, f := range outputRule[rule] { if file == f { @@ -50,7 +57,7 @@ func outputViolations(file string, src []byte) ([]string, error) { if imp.Name != nil { name = imp.Name.Name } - if name == "." && (path == "os" || path == "fmt") { + if name == "." && (path == "os" || path == "fmt" || path == "github.com/spf13/cobra") { out = append(out, "dot import of "+path) } if path == "log" { @@ -67,6 +74,11 @@ func outputViolations(file string, src []byte) ([]string, error) { if id, ok := x.Fun.(*ast.Ident); ok && (id.Name == "print" || id.Name == "println") { add(x, "builtin "+id.Name+" writes to stderr") } + case *ast.Ident: + // A field (ValidArgsFunction: f), a method or a selector: every use of the name. + if completionHooks[x.Name] { + add(x, x.Name+": completion errors go to os.Stderr unredacted (see completion.go)") + } case *ast.SelectorExpr: name := x.Sel.Name if pkg, ok := x.X.(*ast.Ident); ok && imports[pkg.Name] != "" { @@ -75,6 +87,8 @@ func outputViolations(file string, src []byte) ([]string, error) { add(x, "os."+name+" outside the constructor") case imports[pkg.Name] == "fmt" && strings.HasPrefix(name, "Print"): add(x, "fmt."+name+" writes to stdout") + case imports[pkg.Name] == "github.com/spf13/cobra" && compWriters[name]: + add(x, "cobra."+name+" writes to os.Stderr and $BASH_COMP_DEBUG_FILE") } return true } @@ -138,6 +152,11 @@ func TestOutputGuardCatchesBypasses(t *testing.T) { `package cli; func f(a *App) { a.emitFields(nil) }`, `package cli; func f(a *App) { _ = a.rawOut() }`, `package cli; func f(a *App) error { return a.writeError(nil) }; var _ = (*App).emitJSON`, + `package cli; import "github.com/spf13/cobra"; func f() { cobra.CompErrorln("x") }`, + `package cli; import c "github.com/spf13/cobra"; func f() { c.CompDebug("x", true) }`, + `package cli; import . "github.com/spf13/cobra"; func f() { CompErrorln("x") }`, + `package cli; import "github.com/spf13/cobra"; func f() { _ = &cobra.Command{ValidArgsFunction: nil} }`, + `package cli; import "github.com/spf13/cobra"; func f(cmd *cobra.Command) { _ = cmd.RegisterFlagCompletionFunc("x", nil) }`, } { v, err := outputViolations("review_bypass.go", []byte(src)) if err != nil { @@ -147,6 +166,10 @@ func TestOutputGuardCatchesBypasses(t *testing.T) { t.Errorf("not caught: %s", src) } } + // Completion options and types are no printers. + if v, _ := outputViolations("root.go", []byte(`package cli; import "github.com/spf13/cobra"; var _ = cobra.CompletionOptions{}; var _ cobra.Completion`)); len(v) != 0 { + t.Errorf("root.go: %v", v) + } // What present.go and the constructor may do stays allowed. if v, _ := outputViolations("present.go", []byte(`package cli; func f(a *App) { _ = a.Out; _ = a.Err; a.emitFields(nil); a.root().SetOut(a.Out) }`)); len(v) != 0 { t.Errorf("present.go: %v", v) diff --git a/internal/cli/root.go b/internal/cli/root.go index d867fba..553019e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -48,7 +48,11 @@ func (a *App) root() *cobra.Command { Short: "Command line client for the Taiga REST API", SilenceErrors: true, SilenceUsage: true, - PersistentPreRunE: func(*cobra.Command, []string) error { + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + if cmd.Name() == cobra.ShellCompRequestCmd { + completeRedacted(cmd) + return nil + } // Flags parsed: from here on errors come from the command, not from cobra. a.ran = true _, err := output.DetectMode(a.output, a.OutTTY) @@ -81,6 +85,7 @@ func (a *App) root() *cobra.Command { root.AddCommand(a.userCmd()) root.AddCommand(a.milestoneCmd()) root.AddCommand(a.epicCmd()) + a.completionCmd(root) return root }