From 37c08b0cce0541bc9cdc8c559a5148826343e19a Mon Sep 17 00:00:00 2001
From: Keith Duncan <keith_duncan@me.com>
Date: Wed, 29 Sep 2021 11:21:35 +1000
Subject: [PATCH 1/2] Revert "ArtifactDownloader: retain destination trailing
 slash"

---
 agent/artifact_downloader.go      | 13 +------------
 agent/artifact_downloader_test.go | 13 -------------
 2 files changed, 1 insertion(+), 25 deletions(-)

diff --git a/agent/artifact_downloader.go b/agent/artifact_downloader.go
index 07762c624e..708f5a966a 100644
--- a/agent/artifact_downloader.go
+++ b/agent/artifact_downloader.go
@@ -52,20 +52,9 @@ func NewArtifactDownloader(l logger.Logger, ac APIClient, c ArtifactDownloaderCo
 	}
 }
 
-func getDownloadDestination(destination string) string {
-	// Filepath.Abs calls filepath.Clean on the result, which removes any
-	// trailing forward slash. We will then retain any trailing forward slash
-	// as it may indicate path merging behavior for download.getTargetPath.
-	downloadDestination, _ := filepath.Abs(destination)
-	if strings.HasSuffix(destination, string(os.PathSeparator)) && len(destination) > 1 {
-		downloadDestination += string(os.PathSeparator)
-	}
-	return downloadDestination
-}
-
 func (a *ArtifactDownloader) Download() error {
 	// Turn the download destination into an absolute path and confirm it exists
-	downloadDestination := getDownloadDestination(a.conf.Destination)
+	downloadDestination, _ := filepath.Abs(a.conf.Destination)
 	fileInfo, err := os.Stat(downloadDestination)
 	if err != nil {
 		return fmt.Errorf("Could not find information about destination: %s %v",
diff --git a/agent/artifact_downloader_test.go b/agent/artifact_downloader_test.go
index 3819f14d49..c7c9a48e74 100644
--- a/agent/artifact_downloader_test.go
+++ b/agent/artifact_downloader_test.go
@@ -5,12 +5,10 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/buildkite/agent/v3/api"
 	"github.com/buildkite/agent/v3/logger"
-	"github.com/stretchr/testify/assert"
 )
 
 func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
@@ -48,14 +46,3 @@ func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
 		t.Fatal(err)
 	}
 }
-
-func TestGetDownloadDestination(t *testing.T) {
-	workingDirectory, _ := filepath.Abs(".")
-	assert.Equal(t, workingDirectory + string(os.PathSeparator) + "a", getDownloadDestination("a"))
-	assert.Equal(t, workingDirectory + string(os.PathSeparator) + "a" + string(os.PathSeparator), getDownloadDestination("a" + string(os.PathSeparator)))
-
-	// Test that we don't get a double // on unix, must use filepath.Abs
-	// to handle the Windows case which normalises to C:\
-	root, _ := filepath.Abs("/")
-	assert.Equal(t, root, getDownloadDestination("/"))
-}

From 9374b5840b41cec614e2ecf6b15a0384f34580e3 Mon Sep 17 00:00:00 2001
From: Keith Duncan <keith.d@buildkite.com>
Date: Wed, 29 Sep 2021 11:26:00 +1000
Subject: [PATCH 2/2] Remove docs for inaccessible escape hatch

---
 clicommand/artifact_download.go | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/clicommand/artifact_download.go b/clicommand/artifact_download.go
index 32c3bd34cc..eb057c8d44 100644
--- a/clicommand/artifact_download.go
+++ b/clicommand/artifact_download.go
@@ -26,11 +26,6 @@ Description:
    'foo/app' will write any matched artifact files to 'foo/app/logs/', relative
    to the current working directory.
 
-   To avoid this behaviour, use a <destination> argument with a trailing slash.
-   For example, a query of 'app/logs/*' and a destination of 'foo/app/' will
-   write the matched artifact files to 'foo/app/app/logs/', relative to the
-   current working directory.
-
    You can also change working directory to the intended destination and use a
    <destination> of '.' to always create a directory hierarchy matching the
    artifact paths.