diff --git a/pkg/cmd/release/download/download.go b/pkg/cmd/release/download/download.go index 56e6ac72a..bb0df7243 100644 --- a/pkg/cmd/release/download/download.go +++ b/pkg/cmd/release/download/download.go @@ -10,6 +10,9 @@ import ( "os" "path/filepath" "regexp" + "runtime" + "slices" + "strings" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" @@ -174,6 +177,12 @@ func downloadRun(opts *DownloadOptions) error { if len(opts.FilePatterns) > 0 && !matchAny(opts.FilePatterns, a.Name) { continue } + // Note that if we need to start checking for reserved filenames on + // more operating systems we should move to using a build constraints + // pattern rather than checking the operating system at runtime. + if runtime.GOOS == "windows" && isWindowsReservedFilename(a.Name) { + return fmt.Errorf("unable to download release due to asset with reserved filename %q", a.Name) + } toDownload = append(toDownload, a) } } @@ -391,3 +400,23 @@ func (w destinationWriter) Copy(name string, r io.Reader) (copyErr error) { _, copyErr = io.Copy(f, r) return } + +func isWindowsReservedFilename(filename string) bool { + // Windows terminals should prevent the creation of these files + // but that behavior is not enforced across terminals. Prevent + // the user from downloading files with these reserved names as + // they represent an exploit vector for bad actors. + // Reserved filenames defined at: + // https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#win32-file-namespaces + reservedFilenames := []string{"CON", "PRN", "AUX", "NUL", "COM0", + "COM1", "COM2", "COM3", "COM4", "COM5", + "COM6", "COM7", "COM8", "COM9", "COM¹", + "COM²", "COM³", "LPT0", "LPT1", "LPT2", + "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", + "LPT8", "LPT9", "LPT¹", "LPT²", "LPT³"} + + // Normalize type case and remove file type extension from filename. + filename = strings.ToUpper(strings.Split(filename, ".")[0]) + + return slices.Contains(reservedFilenames, filename) +} diff --git a/pkg/cmd/release/download/download_test.go b/pkg/cmd/release/download/download_test.go index 782056314..3c2abf32d 100644 --- a/pkg/cmd/release/download/download_test.go +++ b/pkg/cmd/release/download/download_test.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "path/filepath" + "runtime" "testing" "github.com/cli/cli/v2/internal/ghrepo" @@ -553,6 +554,87 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) { } } +func Test_downloadRun_windowsReservedFilename(t *testing.T) { + if runtime.GOOS != "windows" { + t.SkipNow() + } + + tagName := "v1.2.3" + + ios, _, _, _ := iostreams.Test() + + reg := &httpmock.Registry{} + defer reg.Verify(t) + + shared.StubFetchRelease(t, reg, "OWNER", "REPO", tagName, `{ + "assets": [ + { "name": "valid-asset.zip", "size": 12, + "url": "https://api.github.com/assets/1234" }, + { "name": "valid-asset-2.zip", "size": 34, + "url": "https://api.github.com/assets/3456" }, + { "name": "CON.tgz", "size": 56, + "url": "https://api.github.com/assets/5678" } + ], + "tarball_url": "https://api.github.com/repos/OWNER/REPO/tarball/v1.2.3", + "zipball_url": "https://api.github.com/repos/OWNER/REPO/zipball/v1.2.3" + }`) + + opts := &DownloadOptions{ + IO: ios, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("OWNER/REPO") + }, + TagName: tagName, + } + + err := downloadRun(opts) + + assert.EqualError(t, err, `unable to download release due to asset with reserved filename "CON.tgz"`) +} + +func TestIsWindowsReservedFilename(t *testing.T) { + tests := []struct { + name string + filename string + want bool + }{ + { + name: "non-reserved filename", + filename: "test", + want: false, + }, + { + name: "non-reserved filename with file type extension", + filename: "test.tar.gz", + want: false, + }, + { + name: "reserved filename", + filename: "NUL", + want: true, + }, + { + name: "reserved filename with file type extension", + filename: "NUL.tar.gz", + want: true, + }, + { + name: "reserved filename with mixed type case", + filename: "NuL", + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isWindowsReservedFilename(tt.filename)) + }) + } +} + func listFiles(dir string) ([]string, error) { var files []string err := filepath.Walk(dir, func(p string, f os.FileInfo, err error) error {