Prevent downloading releases with assets that match windows reserved filenames (#8517)
* Prevent downloading releases with assets that match windows reserved filenames * Add comment noting potential use of build constraints in the future
This commit is contained in:
parent
0cf5d22ead
commit
785a340f78
2 changed files with 111 additions and 0 deletions
|
|
@ -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)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue