Ensure temporary directory symlinks do not cause errors (#560)

* Add test for validating git subdirectory symlink failure

Fix the test

* Replace go-safetemp as it is superfluous to our needs

Remove go-safetemp dependency

Author: cam-stitt

client.go

@@ -13,7 +13,6 @@ import (
 	"strings"
 
 	urlhelper "github.com/hashicorp/go-getter/helper/url"
-	safetemp "github.com/hashicorp/go-safetemp"
 )
 
 // ErrSymlinkCopy means that a copy of a symlink was encountered on a request with DisableSymlinks enabled.
@@ -143,7 +142,7 @@ func (c *Client) Get() error {
 			subDir = subDir[1:]
 		}
 
-		td, tdcloser, err := safetemp.Dir("", "getter")
+		td, tdcloser, err := mkdirTemp("", "getter")
 		if err != nil {
 			return err
 		}

get_git.go

@@ -19,7 +19,6 @@ import (
 	"time"
 
 	urlhelper "github.com/hashicorp/go-getter/helper/url"
-	safetemp "github.com/hashicorp/go-safetemp"
 	version "github.com/hashicorp/go-version"
 )
 
@@ -148,7 +147,7 @@ func (g *GitGetter) Get(dst string, u *url.URL) error {
 // GetFile for Git doesn't support updating at this time. It will download
 // the file every time.
 func (g *GitGetter) GetFile(dst string, u *url.URL) error {
-	td, tdcloser, err := safetemp.Dir("", "getter")
+	td, tdcloser, err := mkdirTemp("", "getter")
 	if err != nil {
 		return err
 	}

get_git_test.go

@@ -894,6 +894,53 @@ func TestGitGetter_subdirectory(t *testing.T) {
 	}
 }
 
+func TestGitGetter_subdirectory_ok(t *testing.T) {
+	if !testHasGit {
+		t.Skip("git not found, skipping")
+	}
+
+	g := new(GitGetter)
+	dst := filepath.Join(t.TempDir(), "target")
+
+	repo := testGitRepo(t, "basic")
+	err := os.Mkdir(filepath.Join(repo.dir, "nested"), os.ModePerm)
+	if err != nil {
+		t.Fatal(err)
+	}
+	repo.commitFile("nested/foo.txt", "hello")
+
+	u, err := url.Parse(fmt.Sprintf("git::%s//nested", repo.url.String()))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	client := &Client{
+		Src: u.String(),
+		Dst: dst,
+		Pwd: ".",
+
+		Mode: ClientModeDir,
+
+		Detectors: []Detector{
+			new(GitDetector),
+		},
+		Getters: map[string]Getter{
+			"git": g,
+		},
+	}
+
+	err = client.Get()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	// Verify the main file exists
+	mainPath := filepath.Join(dst, "foo.txt")
+	if _, err := os.Stat(mainPath); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+}
+
 func TestGitGetter_BadRemoteUrl(t *testing.T) {
 
 	if !testHasGit {

get_hg.go

@@ -14,7 +14,6 @@ import (
 	"time"
 
 	urlhelper "github.com/hashicorp/go-getter/helper/url"
-	safetemp "github.com/hashicorp/go-safetemp"
 )
 
 // HgGetter is a Getter implementation that will download a module from
@@ -85,7 +84,7 @@ func (g *HgGetter) Get(dst string, u *url.URL) error {
 func (g *HgGetter) GetFile(dst string, u *url.URL) error {
 	// Create a temporary directory to store the full source. This has to be
 	// a non-existent directory.
-	td, tdcloser, err := safetemp.Dir("", "getter")
+	td, tdcloser, err := mkdirTemp("", "getter")
 	if err != nil {
 		return err
 	}

get_http.go

@@ -18,7 +18,6 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-cleanhttp"
-	safetemp "github.com/hashicorp/go-safetemp"
 )
 
 // HttpGetter is a Getter implementation that will download from an HTTP
@@ -514,7 +513,7 @@ func (g *HttpGetter) GetFile(dst string, src *url.URL) error {
 func (g *HttpGetter) getSubdir(ctx context.Context, dst, source, subDir string, opts ...ClientOption) error {
 	// Create a temporary directory to store the full source. This has to be
 	// a non-existent directory.
-	td, tdcloser, err := safetemp.Dir("", "getter")
+	td, tdcloser, err := mkdirTemp("", "getter")
 	if err != nil {
 		return err
 	}

go.mod

@@ -11,7 +11,6 @@ require (
 	github.com/cheggaaa/pb v1.0.27
 	github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.65
 	github.com/hashicorp/go-cleanhttp v0.5.2
-	github.com/hashicorp/go-safetemp v1.0.0
 	github.com/hashicorp/go-version v1.6.0
 	github.com/klauspost/compress v1.15.11
 	github.com/mitchellh/go-homedir v1.1.0

go.sum

@@ -101,8 +101,6 @@ github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.65 h1:81+kWbE1yErFBMjME0I5k3
 github.com/hashicorp/aws-sdk-go-base/v2 v2.0.0-beta.65/go.mod h1:WtMzv9T++tfWVea+qB2MXoaqxw33S8bpJslzUike2mQ=
 github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
 github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
-github.com/hashicorp/go-safetemp v1.0.0 h1:2HR189eFNrjHQyENnQMMpCiBAsRxzbTMIgBhEyExpmo=
-github.com/hashicorp/go-safetemp v1.0.0/go.mod h1:oaerMy3BhqiTbVye6QuFhFtIceqFoDHxNAB65b+Rj1I=
 github.com/hashicorp/go-version v1.6.0 h1:feTTfFNnjP967rlCxM/I9g701jU+RN74YKx2mOkIeek=
 github.com/hashicorp/go-version v1.6.0/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA=
 github.com/klauspost/compress v1.15.11 h1:Lcadnb3RKGin4FYM/orgq0qde+nc15E5Cbqg4B9Sx9c=

tempdir.go

@@ -0,0 +1,54 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package getter
+
+import (
+	"io"
+	"os"
+	"path/filepath"
+)
+
+// mkdirTemp creates a new temporary directory that isn't yet created. This
+// can be used with calls that expect a non-existent directory.
+//
+// The temporary directory is also evaluated for symlinks upon creation
+// as some operating systems provide symlinks by default when created.
+//
+// The directory is created as a child of a temporary directory created
+// within the directory dir starting with prefix. The temporary directory
+// returned is always named "temp". The parent directory has the specified
+// prefix.
+//
+// The returned io.Closer should be used to clean up the returned directory.
+// This will properly remove the returned directory and any other temporary
+// files created.
+//
+// If an error is returned, the Closer does not need to be called (and will
+// be nil).
+func mkdirTemp(dir, prefix string) (string, io.Closer, error) {
+	// Create the temporary directory
+	td, err := os.MkdirTemp(dir, prefix)
+	if err != nil {
+		return "", nil, err
+	}
+
+	// we evaluate symlinks as some operating systems (eg: MacOS), that
+	// actually has any temporary directory created as a symlink.
+	// As we have only just created the temporary directory, this is a safe
+	// evaluation to make at this time.
+	td, err = filepath.EvalSymlinks(td)
+	if err != nil {
+		return "", nil, err
+	}
+
+	return filepath.Join(td, "temp"), pathCloser(td), nil
+}
+
+// pathCloser implements io.Closer to remove the given path on Close.
+type pathCloser string
+
+// Close deletes this path.
+func (p pathCloser) Close() error {
+	return os.RemoveAll(string(p))
+}

tempdir_unix_test.go

@@ -0,0 +1,52 @@
+//go:build !windows
+// +build !windows
+
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package getter
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func Test_mkdirTemp(t *testing.T) {
+	d, c, err := mkdirTemp("", "test")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if _, err := os.Stat(d); err == nil || !os.IsNotExist(err) {
+		t.Fatalf("directory %q should not exist", d)
+	}
+
+	parent := filepath.Dir(d)
+	fi, err := os.Stat(parent)
+	if err != nil {
+		t.Fatalf("parent directory error: %s", err)
+	}
+	if v := fi.Mode().Perm(); v != 0700 {
+		t.Fatalf("parent directory should be 0700: %s", v)
+	}
+
+	// Create the directory
+	if err := os.MkdirAll(d, 0755); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if _, err := os.Stat(d); err != nil {
+		t.Fatalf("directory %q should exist", d)
+	}
+
+	// Close should remove it
+	if err := c.Close(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if _, err := os.Stat(d); err == nil || !os.IsNotExist(err) {
+		t.Fatalf("directory %q should not exist", d)
+	}
+	if _, err := os.Stat(parent); err == nil || !os.IsNotExist(err) {
+		t.Fatalf("directory %q should not exist", parent)
+	}
+}

tempdir_windows_test.go

@@ -0,0 +1,52 @@
+//go:build windows
+// +build windows
+
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package getter
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func Test_mkdirTemp(t *testing.T) {
+	d, c, err := mkdirTemp("", "test")
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	if _, err := os.Stat(d); err == nil || !os.IsNotExist(err) {
+		t.Fatalf("directory %q should not exist", d)
+	}
+
+	parent := filepath.Dir(d)
+	fi, err := os.Stat(parent)
+	if err != nil {
+		t.Fatalf("parent directory error: %s", err)
+	}
+	if v := fi.Mode().Perm(); v != 0777 {
+		t.Fatalf("parent directory should be 0777: %s", v)
+	}
+
+	// Create the directory
+	if err := os.MkdirAll(d, 0755); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if _, err := os.Stat(d); err != nil {
+		t.Fatalf("directory %q should exist", d)
+	}
+
+	// Close should remove it
+	if err := c.Close(); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if _, err := os.Stat(d); err == nil || !os.IsNotExist(err) {
+		t.Fatalf("directory %q should not exist", d)
+	}
+	if _, err := os.Stat(parent); err == nil || !os.IsNotExist(err) {
+		t.Fatalf("directory %q should not exist", parent)
+	}
+}