define.TempDirForURL(): always use an intermediate subdirectory

Ensure that the temporary directory that we create is never itself the
top-level directory of the content that we're downloading, in case it's
an archive which includes a "." with weird permissions.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>

add.go

@@ -495,8 +495,8 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 			wg.Add(1)
 			if sourceIsGit(src) {
 				go func() {
-					var cloneDir string
-					cloneDir, _, getErr = define.TempDirForURL(tmpdir.GetTempDir(), "", src)
+					var cloneDir, subdir string
+					cloneDir, subdir, getErr = define.TempDirForURL(tmpdir.GetTempDir(), "", src)
 					getOptions := copier.GetOptions{
 						UIDMap:         srcUIDMap,
 						GIDMap:         srcGIDMap,
@@ -511,7 +511,8 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
 						StripStickyBit: options.StripStickyBit,
 					}
 					writer := io.WriteCloser(pipeWriter)
-					getErr = copier.Get(cloneDir, cloneDir, getOptions, []string{"."}, writer)
+					repositoryDir := filepath.Join(cloneDir, subdir)
+					getErr = copier.Get(repositoryDir, repositoryDir, getOptions, []string{"."}, writer)
 					pipeWriter.Close()
 					wg.Done()
 				}()

define/types.go

@@ -169,13 +169,13 @@ type SBOMScanOptions struct {
 	MergeStrategy   SBOMMergeStrategy // how to merge the outputs of multiple scans
 }
 
-// TempDirForURL checks if the passed-in string looks like a URL or -.  If it is,
-// TempDirForURL creates a temporary directory, arranges for its contents to be
-// the contents of that URL, and returns the temporary directory's path, along
-// with the name of a subdirectory which should be used as the build context
-// (which may be empty or ".").  Removal of the temporary directory is the
-// responsibility of the caller.  If the string doesn't look like a URL,
-// TempDirForURL returns empty strings and a nil error code.
+// TempDirForURL checks if the passed-in string looks like a URL or "-".  If it
+// is, TempDirForURL creates a temporary directory, arranges for its contents
+// to be the contents of that URL, and returns the temporary directory's path,
+// along with the relative name of a subdirectory which should be used as the
+// build context (which may be empty or ".").  Removal of the temporary
+// directory is the responsibility of the caller.  If the string doesn't look
+// like a URL or "-", TempDirForURL returns empty strings and a nil error code.
 func TempDirForURL(dir, prefix, url string) (name string, subdir string, err error) {
 	if !strings.HasPrefix(url, "http://") &&
 		!strings.HasPrefix(url, "https://") &&
@@ -188,19 +188,24 @@ func TempDirForURL(dir, prefix, url string) (name string, subdir string, err err
 	if err != nil {
 		return "", "", fmt.Errorf("creating temporary directory for %q: %w", url, err)
 	}
+	downloadDir := filepath.Join(name, "download")
+	if err = os.MkdirAll(downloadDir, 0o700); err != nil {
+		return "", "", fmt.Errorf("creating directory %q for %q: %w", downloadDir, url, err)
+	}
 	urlParsed, err := urlpkg.Parse(url)
 	if err != nil {
 		return "", "", fmt.Errorf("parsing url %q: %w", url, err)
 	}
 	if strings.HasPrefix(url, "git://") || strings.HasSuffix(urlParsed.Path, ".git") {
-		combinedOutput, gitSubDir, err := cloneToDirectory(url, name)
+		combinedOutput, gitSubDir, err := cloneToDirectory(url, downloadDir)
 		if err != nil {
 			if err2 := os.RemoveAll(name); err2 != nil {
 				logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 			}
 			return "", "", fmt.Errorf("cloning %q to %q:\n%s: %w", url, name, string(combinedOutput), err)
 		}
-		return name, gitSubDir, nil
+		logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, gitSubDir))
+		return name, filepath.Join(filepath.Base(downloadDir), gitSubDir), nil
 	}
 	if strings.HasPrefix(url, "github.com/") {
 		ghurl := url
@@ -209,28 +214,29 @@ func TempDirForURL(dir, prefix, url string) (name string, subdir string, err err
 		subdir = path.Base(ghurl) + "-master"
 	}
 	if strings.HasPrefix(url, "http://") || strings.HasPrefix(url, "https://") {
-		err = downloadToDirectory(url, name)
+		err = downloadToDirectory(url, downloadDir)
 		if err != nil {
 			if err2 := os.RemoveAll(name); err2 != nil {
 				logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 			}
-			return "", subdir, err
+			return "", "", err
 		}
-		return name, subdir, nil
+		logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir))
+		return name, filepath.Join(filepath.Base(downloadDir), subdir), nil
 	}
 	if url == "-" {
-		err = stdinToDirectory(name)
+		err = stdinToDirectory(downloadDir)
 		if err != nil {
 			if err2 := os.RemoveAll(name); err2 != nil {
 				logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 			}
-			return "", subdir, err
+			return "", "", err
 		}
-		logrus.Debugf("Build context is at %q", name)
-		return name, subdir, nil
+		logrus.Debugf("Build context is at %q", filepath.Join(downloadDir, subdir))
+		return name, filepath.Join(filepath.Base(downloadDir), subdir), nil
 	}
 	logrus.Debugf("don't know how to retrieve %q", url)
-	if err2 := os.Remove(name); err2 != nil {
+	if err2 := os.RemoveAll(name); err2 != nil {
 		logrus.Debugf("error removing temporary directory %q: %v", name, err2)
 	}
 	return "", "", errors.New("unreachable code reached")