From dd8c5c9dae205ef7c2450eeedfda26ff22fdea29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Tue, 13 Dec 2022 19:39:23 +0100
Subject: [PATCH] Fix clobbering old files in release download (#6694)

Ensures that the old file to be clobbered is first truncated before writing.
---
 pkg/cmd/release/download/download.go      |  2 +-
 pkg/cmd/release/download/download_test.go | 33 +++++++++++++++++------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/pkg/cmd/release/download/download.go b/pkg/cmd/release/download/download.go
index edeba47a2..a07226b2a 100644
--- a/pkg/cmd/release/download/download.go
+++ b/pkg/cmd/release/download/download.go
@@ -370,7 +370,7 @@ func (w destinationWriter) Copy(name string, r io.Reader) error {
 		}
 	}
 
-	f, err := os.OpenFile(fp, os.O_WRONLY|os.O_CREATE, 0644)
+	f, err := os.OpenFile(fp, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0644)
 	if err != nil {
 		return err
 	}
diff --git a/pkg/cmd/release/download/download_test.go b/pkg/cmd/release/download/download_test.go
index 0442c52e6..4e6ca193f 100644
--- a/pkg/cmd/release/download/download_test.go
+++ b/pkg/cmd/release/download/download_test.go
@@ -391,6 +391,11 @@ func Test_downloadRun(t *testing.T) {
 }
 
 func Test_downloadRun_cloberAndSkip(t *testing.T) {
+	oldAssetContents := "older copy to be clobbered"
+	oldZipballContents := "older zipball to be clobbered"
+	// this should be shorter than oldAssetContents and oldZipballContents
+	newContents := "somedata"
+
 	tests := []struct {
 		name            string
 		opts            DownloadOptions
@@ -407,7 +412,9 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 				Destination:  "tmp/packages",
 				Concurrency:  2,
 			},
-			wantErr: "already exists (use `--clobber` to overwrite file or `--skip-existing` to skip file)",
+			wantErr:         "already exists (use `--clobber` to overwrite file or `--skip-existing` to skip file)",
+			wantFileSize:    int64(len(oldAssetContents)),
+			wantArchiveSize: int64(len(oldZipballContents)),
 		},
 		{
 			name: "clobber",
@@ -419,9 +426,10 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 				OverwriteExisting: true,
 			},
 			httpStubs: func(reg *httpmock.Registry) {
-				reg.Register(httpmock.REST("GET", "assets/3456"), httpmock.StringResponse("somedata"))
+				reg.Register(httpmock.REST("GET", "assets/3456"), httpmock.StringResponse(newContents))
 			},
-			wantFileSize: 8,
+			wantFileSize:    int64(len(newContents)),
+			wantArchiveSize: int64(len(oldZipballContents)),
 		},
 		{
 			name: "clobber archive",
@@ -436,11 +444,12 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 				reg.Register(
 					httpmock.REST("GET", "repos/OWNER/REPO/zipball/v1.2.3"),
 					httpmock.WithHeader(
-						httpmock.StringResponse("somedata"), "content-disposition", "attachment; filename=zipball.zip",
+						httpmock.StringResponse(newContents), "content-disposition", "attachment; filename=zipball.zip",
 					),
 				)
 			},
-			wantArchiveSize: 8,
+			wantFileSize:    int64(len(oldAssetContents)),
+			wantArchiveSize: int64(len(newContents)),
 		},
 		{
 			name: "skip",
@@ -451,6 +460,8 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 				Concurrency:  2,
 				SkipExisting: true,
 			},
+			wantFileSize:    int64(len(oldAssetContents)),
+			wantArchiveSize: int64(len(oldZipballContents)),
 		},
 		{
 			name: "skip archive",
@@ -465,10 +476,12 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 				reg.Register(
 					httpmock.REST("GET", "repos/OWNER/REPO/zipball/v1.2.3"),
 					httpmock.WithHeader(
-						httpmock.StringResponse("somedata"), "content-disposition", "attachment; filename=zipball.zip",
+						httpmock.StringResponse(newContents), "content-disposition", "attachment; filename=zipball.zip",
 					),
 				)
 			},
+			wantFileSize:    int64(len(oldAssetContents)),
+			wantArchiveSize: int64(len(oldZipballContents)),
 		},
 	}
 	for _, tt := range tests {
@@ -481,9 +494,13 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 			archive := filepath.Join(dest, "zipball.zip")
 			f1, err := os.Create(file)
 			assert.NoError(t, err)
+			_, err = f1.WriteString(oldAssetContents)
+			assert.NoError(t, err)
 			f1.Close()
 			f2, err := os.Create(archive)
 			assert.NoError(t, err)
+			_, err = f2.WriteString(oldZipballContents)
+			assert.NoError(t, err)
 			f2.Close()
 
 			tt.opts.Destination = dest
@@ -523,8 +540,8 @@ func Test_downloadRun_cloberAndSkip(t *testing.T) {
 			assert.NoError(t, err)
 			as, err := os.Stat(archive)
 			assert.NoError(t, err)
-			assert.Equal(t, fs.Size(), tt.wantFileSize)
-			assert.Equal(t, as.Size(), tt.wantArchiveSize)
+			assert.Equal(t, tt.wantFileSize, fs.Size())
+			assert.Equal(t, tt.wantArchiveSize, as.Size())
 		})
 	}
 }