diff --git a/internal/app/download.go b/internal/app/download.go index c5a1838a..9ed820c7 100644 --- a/internal/app/download.go +++ b/internal/app/download.go @@ -4,17 +4,151 @@ package app import ( + "bufio" "errors" + "fmt" "io" + "math/rand/v2" "os" "path/filepath" + "strconv" + "strings" + "unicode/utf8" "gitea.dev/tea/internal/clierr" + "gitea.dev/tea/internal/terminal" "github.com/spf13/cobra" + "golang.org/x/term" ) +// downloadFlags are the destination flags of a download command. +type downloadFlags struct { + // output is a file, an existing directory to download into, or - for standard output. + output string + // dir is the directory that receives the download under its own name. + dir string + // clobber replaces an existing file; skipExisting keeps it and skips the download. + clobber, skipExisting bool +} + +// register adds -O/--output (and its older name --output-file), -D/--dir, --clobber and +// --skip-existing to cmd. +func (f *downloadFlags) register(cmd *cobra.Command) { + flags := cmd.Flags() + flags.StringVarP(&f.output, "output", "O", "", "File or directory to write to, or - for standard output (default: the download's name)") + flags.StringVar(&f.output, "output-file", "", "Alias of --output") + _ = flags.MarkHidden("output-file") + flags.StringVarP(&f.dir, "dir", "D", "", "Directory to download into") + flags.BoolVar(&f.clobber, "clobber", false, "Replace an existing file") + flags.BoolVar(&f.skipExisting, "skip-existing", false, "Keep an existing file and skip the download") +} + +// destination returns the path a download named name is written to: --output (inside it when it +// is a directory), else name inside --dir or the working directory, or "-" for standard +// output. skip reports an existing file that --skip-existing keeps. It fails before anything +// is downloaded when the file exists without --clobber or --skip-existing. +func (f downloadFlags) destination(name string) (path string, skip bool, err error) { + switch { + case f.output != "" && f.dir != "": + return "", false, clierr.Wrap(clierr.Usage, errors.New("--output and --dir cannot be combined")) + case f.clobber && f.skipExisting: + return "", false, clierr.Wrap(clierr.Usage, errors.New("--clobber and --skip-existing cannot be combined")) + case f.output == "-": + return "-", false, nil + } + path = f.output + if path == "" || isDirectory(path) { + if name != filepath.Base(name) || name == "." || name == ".." || strings.ContainsAny(name, `/\`) { + return "", false, clierr.Wrap(clierr.Usage, fmt.Errorf("cannot use %q as a file name; pass --output", name)) + } + path = filepath.Join(cmpOrDot(path, f.dir), name) + } + if _, err := os.Lstat(path); err == nil { + if f.skipExisting { + return path, true, nil + } + if !f.clobber { + return "", false, clierr.Wrap(clierr.Usage, fmt.Errorf("%s already exists; pass --clobber to replace it or --skip-existing to keep it", path)) + } + } else if !errors.Is(err, os.ErrNotExist) { + return "", false, err + } + return path, false, nil +} + +func cmpOrDot(values ...string) string { + for _, value := range values { + if value != "" { + return value + } + } + return "." +} + +func isDirectory(path string) bool { + info, err := os.Stat(path) + return (err == nil && info.IsDir()) || strings.HasSuffix(path, string(filepath.Separator)) || strings.HasSuffix(path, "/") +} + +// download writes body to path, a destination from downloadFlags.destination. A file is +// published complete with the permissions the umask allows; standard output refuses binary +// data on a terminal and strips terminal control sequences from text. +func (a *application) download(cmd *cobra.Command, f downloadFlags, path string, body io.Reader) error { + if path == "-" { + return writeTerminalSafe(cmd.OutOrStdout(), a.deps.Streams.OutTTY, body) + } + return publish(cmd, path, false, f.clobber, func(destination io.Writer) error { + _, err := io.Copy(destination, body) + return err + }) +} + func writeDownload(cmd *cobra.Command, body io.Reader, output string) error { - return writeOutput(cmd, output, func(destination io.Writer) error { _, err := io.Copy(destination, body); return err }) + if output == "" || output == "-" { + return writeTerminalSafe(cmd.OutOrStdout(), isTerminal(cmd.OutOrStdout()), body) + } + return publish(cmd, output, false, false, func(destination io.Writer) error { _, err := io.Copy(destination, body); return err }) +} + +// isTerminal reports whether w is a terminal, for writers without injected stream capabilities. +func isTerminal(w io.Writer) bool { + file, ok := w.(*os.File) + return ok && term.IsTerminal(int(file.Fd())) +} + +// binarySniffLength is how much of a download decides whether it is text. +const binarySniffLength = 8 << 10 + +// writeTerminalSafe copies body to w. On a terminal it refuses binary data, which would garble +// the terminal, and strips control sequences from text. +func writeTerminalSafe(w io.Writer, tty bool, body io.Reader) error { + if !tty { + _, err := io.Copy(w, body) + return err + } + reader := bufio.NewReaderSize(body, binarySniffLength) + head, err := reader.Peek(binarySniffLength) + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, bufio.ErrBufferFull) { + return err + } + if binary(head) { + return clierr.Wrap(clierr.Usage, errors.New("refusing to write binary data to the terminal; pass --output FILE or redirect standard output")) + } + _, err = io.Copy(terminal.SafeWriter(w, true, false), reader) + return err +} + +// binary reports data that is not UTF-8 text: a NUL byte or an invalid sequence other than a +// rune cut off at the end. +func binary(data []byte) bool { + for len(data) > 0 { + r, size := utf8.DecodeRune(data) + if r == 0 || (r == utf8.RuneError && size == 1 && (len(data) >= utf8.UTFMax || utf8.FullRune(data))) { + return true + } + data = data[size:] + } + return false } // writeOutput writes to stdout when output is empty or -; otherwise it publishes a complete @@ -23,7 +157,15 @@ func writeOutput(cmd *cobra.Command, output string, write func(io.Writer) error) if output == "" || output == "-" { return write(cmd.OutOrStdout()) } - file, err := os.CreateTemp(filepath.Dir(output), ".tea-download-*") + return publish(cmd, output, true, false, write) +} + +// publish writes a temporary file next to output and then links it into place, so output only +// ever holds a complete file. With replace it renames over an existing output; otherwise it +// never replaces a file created while it wrote. A private file is readable by its owner only; +// others get the permissions the umask allows, as files a shell redirection creates. +func publish(cmd *cobra.Command, output string, private, replace bool, write func(io.Writer) error) error { + file, err := createTemp(filepath.Dir(output), private) if err != nil { return err } @@ -39,14 +181,33 @@ func writeOutput(cmd *cobra.Command, output string, write func(io.Writer) error) if closeErr != nil { return closeErr } + if replace { + return os.Rename(file.Name(), output) + } // Linking never replaces a file created while the download ran. err = os.Link(file.Name(), output) if errors.Is(err, os.ErrExist) { - return clierr.Wrap(clierr.Usage, errors.New("output file already exists")) + return clierr.Wrap(clierr.Usage, fmt.Errorf("%s already exists", output)) } return err } +// createTemp creates a new temporary file in dir. Unlike os.CreateTemp, which always creates +// mode 0600, a file that is not private gets 0666 less the umask. +func createTemp(dir string, private bool) (*os.File, error) { + if private { + return os.CreateTemp(dir, ".tea-download-*") + } + for range 100 { + name := filepath.Join(dir, ".tea-download-"+strconv.FormatUint(rand.Uint64(), 36)) + file, err := os.OpenFile(name, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o666) + if !errors.Is(err, os.ErrExist) { + return file, err + } + } + return nil, errors.New("cannot create a temporary file in " + dir) +} + func validateDownloadDestination(output string) error { if output == "" { return clierr.Wrap(clierr.Usage, errors.New("supply --output-file, or --output-file - for stdout")) @@ -54,8 +215,11 @@ func validateDownloadDestination(output string) error { if output == "-" { return nil } + if isDirectory(output) { + return clierr.Wrap(clierr.Usage, fmt.Errorf("%s is a directory; pass a file path inside it", output)) + } if _, err := os.Lstat(output); err == nil { - return clierr.Wrap(clierr.Usage, errors.New("output file already exists")) + return clierr.Wrap(clierr.Usage, fmt.Errorf("%s already exists", output)) } else if !errors.Is(err, os.ErrNotExist) { return err } diff --git a/internal/app/download_test.go b/internal/app/download_test.go index 08170325..e7549a87 100644 --- a/internal/app/download_test.go +++ b/internal/app/download_test.go @@ -10,8 +10,11 @@ import ( "io" "os" "path/filepath" + "strings" "testing" + "gitea.dev/tea/internal/clierr" + "gitea.dev/tea/internal/terminal" "github.com/spf13/cobra" "github.com/stretchr/testify/require" ) @@ -20,7 +23,19 @@ type failingDownload struct{} func (failingDownload) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF } -func TestDownloadsPublishOnlyCompletePrivateFiles(t *testing.T) { +// umaskMode returns the mode the umask leaves of 0666, as for files a shell creates. +func umaskMode(t *testing.T) os.FileMode { + t.Helper() + probe := filepath.Join(t.TempDir(), "probe") + file, err := os.OpenFile(probe, os.O_CREATE|os.O_WRONLY, 0o666) + require.NoError(t, err) + require.NoError(t, file.Close()) + info, err := os.Stat(probe) + require.NoError(t, err) + return info.Mode().Perm() +} + +func TestDownloadsPublishOnlyCompleteFiles(t *testing.T) { cmd := &cobra.Command{} cmd.SetContext(context.Background()) var stdout bytes.Buffer @@ -38,7 +53,7 @@ func TestDownloadsPublishOnlyCompletePrivateFiles(t *testing.T) { require.NoError(t, writeDownload(cmd, bytes.NewBufferString("archive"), output)) info, err := os.Stat(output) require.NoError(t, err) - require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + require.Equal(t, umaskMode(t), info.Mode().Perm(), "downloads are not private files") require.ErrorContains(t, writeDownload(cmd, bytes.NewBufferString("replacement"), output), "already exists") data, err := os.ReadFile(output) require.NoError(t, err) @@ -46,4 +61,121 @@ func TestDownloadsPublishOnlyCompletePrivateFiles(t *testing.T) { entries, err = os.ReadDir(directory) require.NoError(t, err) require.Len(t, entries, 1) + + private := filepath.Join(directory, "response.json") + require.NoError(t, writeOutput(cmd, private, func(w io.Writer) error { _, err := io.WriteString(w, "{}"); return err })) + info, err = os.Stat(private) + require.NoError(t, err) + require.Equal(t, os.FileMode(0o600), info.Mode().Perm(), "API responses stay private") +} + +func TestDownloadDestinations(t *testing.T) { + work := t.TempDir() + t.Chdir(work) + require.NoError(t, os.Mkdir("dl", 0o755)) + require.NoError(t, os.WriteFile("existing.txt", []byte("old"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join("dl", "existing.txt"), []byte("old"), 0o644)) + for _, tc := range []struct { + name string + flags downloadFlags + file string + want string + skip bool + err string + }{ + {name: "default name", file: "notes.txt", want: "notes.txt"}, + {name: "--dir", flags: downloadFlags{dir: "dl"}, file: "notes.txt", want: filepath.Join("dl", "notes.txt")}, + {name: "--output file", flags: downloadFlags{output: "renamed.txt"}, file: "notes.txt", want: "renamed.txt"}, + {name: "--output directory", flags: downloadFlags{output: "dl"}, file: "notes.txt", want: filepath.Join("dl", "notes.txt")}, + {name: "--output directory with slash", flags: downloadFlags{output: "dl/"}, file: "notes.txt", want: filepath.Join("dl", "notes.txt")}, + {name: "stdout", flags: downloadFlags{output: "-"}, file: "notes.txt", want: "-"}, + {name: "existing", file: "existing.txt", err: "existing.txt already exists; pass --clobber to replace it or --skip-existing to keep it"}, + {name: "existing in --dir", flags: downloadFlags{dir: "dl"}, file: "existing.txt", err: filepath.Join("dl", "existing.txt") + " already exists"}, + {name: "--clobber", flags: downloadFlags{clobber: true}, file: "existing.txt", want: "existing.txt"}, + {name: "--skip-existing", flags: downloadFlags{skipExisting: true}, file: "existing.txt", want: "existing.txt", skip: true}, + {name: "--skip-existing new file", flags: downloadFlags{skipExisting: true}, file: "notes.txt", want: "notes.txt"}, + {name: "parent directory name", file: "../escape.txt", err: `cannot use "../escape.txt" as a file name; pass --output`}, + {name: "nested name", file: "a/b.txt", err: "cannot use"}, + {name: "dot dot", file: "..", err: "cannot use"}, + {name: "--output and --dir", flags: downloadFlags{output: "x", dir: "dl"}, file: "notes.txt", err: "--output and --dir cannot be combined"}, + {name: "--clobber and --skip-existing", flags: downloadFlags{clobber: true, skipExisting: true}, file: "notes.txt", err: "--clobber and --skip-existing cannot be combined"}, + } { + t.Run(tc.name, func(t *testing.T) { + path, skip, err := tc.flags.destination(tc.file) + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + require.Equal(t, clierr.Usage, clierr.ExitCode(err)) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, path) + require.Equal(t, tc.skip, skip) + }) + } +} + +func TestDownloadClobberReplacesTheFileCompletely(t *testing.T) { + t.Chdir(t.TempDir()) + require.NoError(t, os.WriteFile("asset.bin", []byte("old"), 0o600)) + deps, _, _ := testDeps(t) + a := &application{deps: deps} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + flags := downloadFlags{clobber: true} + path, _, err := flags.destination("asset.bin") + require.NoError(t, err) + require.NoError(t, a.download(cmd, flags, path, strings.NewReader("new"))) + data, err := os.ReadFile("asset.bin") + require.NoError(t, err) + require.Equal(t, "new", string(data)) + info, err := os.Stat("asset.bin") + require.NoError(t, err) + require.Equal(t, umaskMode(t), info.Mode().Perm()) + entries, err := os.ReadDir(".") + require.NoError(t, err) + require.Len(t, entries, 1, "no temporary file is left behind") +} + +func TestDownloadsToATerminalRefuseBinaryData(t *testing.T) { + for _, tc := range []struct { + name, body, want string + tty bool + err string + }{ + {name: "piped binary stays byte exact", body: "PK\x03\x04\x00\x01", want: "PK\x03\x04\x00\x01"}, + {name: "terminal text", body: "hello\nworld\n", want: "hello\nworld\n", tty: true}, + {name: "terminal text loses escapes", body: "title\x1b]0;PWNED\x07\n", want: "title]0;PWNED\n", tty: true}, + {name: "terminal binary", body: "PK\x03\x04\x00\x01", tty: true, err: "refusing to write binary data to the terminal"}, + {name: "terminal invalid UTF-8", body: "abc\xff\xfedef", tty: true, err: "refusing to write binary data"}, + {name: "terminal UTF-8 cut at the sniff boundary", body: strings.Repeat("a", binarySniffLength-1) + "é", want: strings.Repeat("a", binarySniffLength-1) + "é", tty: true}, + } { + t.Run(tc.name, func(t *testing.T) { + deps, out, _ := testDeps(t) + deps.Streams = terminal.Streams{Out: out, OutTTY: tc.tty} + a := &application{deps: deps} + cmd := &cobra.Command{} + cmd.SetContext(context.Background()) + cmd.SetOut(out) + err := a.download(cmd, downloadFlags{output: "-"}, "-", strings.NewReader(tc.body)) + if tc.err != "" { + require.ErrorContains(t, err, tc.err) + require.Empty(t, out.String()) + return + } + require.NoError(t, err) + require.Equal(t, tc.want, out.String()) + }) + } +} + +func TestDownloadFlagsRegisterGhNames(t *testing.T) { + var flags downloadFlags + cmd := &cobra.Command{} + flags.register(cmd) + require.NoError(t, cmd.Flags().Parse([]string{"-O", "file", "--clobber"})) + require.Equal(t, downloadFlags{output: "file", clobber: true}, flags) + require.NoError(t, cmd.Flags().Parse([]string{"--output-file", "legacy", "-D", "dir", "--skip-existing"})) + require.Equal(t, "legacy", flags.output) + require.Equal(t, "dir", flags.dir) + require.True(t, cmd.Flags().Lookup("output-file").Hidden) } diff --git a/internal/app/integration_helpers_test.go b/internal/app/integration_helpers_test.go index 4e8a2c01..6abf18c1 100644 --- a/internal/app/integration_helpers_test.go +++ b/internal/app/integration_helpers_test.go @@ -197,7 +197,7 @@ func verifyIssueAttachmentLifecycle(t *testing.T, deps Dependencies, group, repo require.Equal(t, "Disposable attachment content\n", string(content)) info, err := os.Stat(downloaded) require.NoError(t, err) - require.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + require.Equal(t, umaskMode(t), info.Mode().Perm()) deleted := false defer func() { if !deleted {