url sanitation using regex replaced with parse and redact

carton/package.go

@@ -185,7 +185,8 @@ func (p Package) Create(options ...Option) {
 
 			f, err := cache.Artifact(dep, n.BasicAuth)
 			if err != nil {
-				config.exitHandler.Error(fmt.Errorf("unable to download %s\n%w", dep.URI, err))
+				logger.Debugf("fetching dependency %s failed\n%w", dep.Name, err)
+				config.exitHandler.Error(fmt.Errorf("unable to download %s. see DEBUG log level", dep.Name))
 				return
 			}
 			if err = f.Close(); err != nil {

dependency_cache.go

@@ -26,7 +26,6 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strconv"
 	"strings"
 	"time"
@@ -169,6 +168,7 @@ func (d *DependencyCache) Artifact(dependency BuildpackDependency, mods ...Reque
 		artifact string
 		file     string
 		uri      = dependency.URI
+		urlP     *url.URL
 	)
 
 	for d, u := range d.Mappings {
@@ -178,14 +178,20 @@ func (d *DependencyCache) Artifact(dependency BuildpackDependency, mods ...Reque
 		}
 	}
 
+	urlP, err := url.Parse(uri)
+	if err != nil {
+		d.Logger.Debugf("URI format invalid\n%w", err)
+		return nil, fmt.Errorf("unable to parse URI. see DEBUG log level")
+	}
+
 	if dependency.SHA256 == "" {
 		d.Logger.Headerf("%s Dependency has no SHA256. Skipping cache.",
 			color.New(color.FgYellow, color.Bold).Sprint("Warning:"))
 
-		d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), sanitizeUri(uri))
-		artifact = filepath.Join(d.DownloadPath, filepath.Base(uri))
-		if err := d.download(uri, artifact, mods...); err != nil {
-			return nil, fmt.Errorf("unable to download %s\n%w", sanitizeUri(uri), err)
+		d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), urlP.Redacted())
+		artifact = filepath.Join(d.DownloadPath, filepath.Base(urlP.Path))
+		if err := d.download(urlP, artifact, mods...); err != nil {
+			return nil, fmt.Errorf("unable to download %s\n%w", urlP.Redacted(), err)
 		}
 
 		return os.Open(artifact)
@@ -202,7 +208,7 @@ func (d *DependencyCache) Artifact(dependency BuildpackDependency, mods ...Reque
 
 	if dependency.Equals(actual) {
 		d.Logger.Bodyf("%s cached download from buildpack", color.GreenString("Reusing"))
-		return os.Open(filepath.Join(d.CachePath, dependency.SHA256, filepath.Base(uri)))
+		return os.Open(filepath.Join(d.CachePath, dependency.SHA256, filepath.Base(urlP.Path)))
 	}
 
 	file = filepath.Join(d.DownloadPath, fmt.Sprintf("%s.toml", dependency.SHA256))
@@ -216,13 +222,13 @@ func (d *DependencyCache) Artifact(dependency BuildpackDependency, mods ...Reque
 
 	if dependency.Equals(actual) {
 		d.Logger.Bodyf("%s previously cached download", color.GreenString("Reusing"))
-		return os.Open(filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(uri)))
+		return os.Open(filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(urlP.Path)))
 	}
 
-	d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), sanitizeUri(uri))
-	artifact = filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(uri))
-	if err := d.download(uri, artifact, mods...); err != nil {
-		return nil, fmt.Errorf("unable to download %s\n%w", sanitizeUri(uri), err)
+	d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), urlP.Redacted())
+	artifact = filepath.Join(d.DownloadPath, dependency.SHA256, filepath.Base(urlP.Path))
+	if err := d.download(urlP, artifact, mods...); err != nil {
+		return nil, fmt.Errorf("unable to download %s\n%w", urlP.Redacted(), err)
 	}
 
 	d.Logger.Body("Verifying checksum")
@@ -248,17 +254,12 @@ func (d *DependencyCache) Artifact(dependency BuildpackDependency, mods ...Reque
 	return os.Open(artifact)
 }
 
-func (d DependencyCache) download(uri string, destination string, mods ...RequestModifierFunc) error {
-	url, err := url.Parse(uri)
-	if err != nil {
-		return fmt.Errorf("unable to parse URI %s\n%w", sanitizeUri(uri), err)
-	}
-
+func (d DependencyCache) download(url *url.URL, destination string, mods ...RequestModifierFunc) error {
 	if url.Scheme == "file" {
 		return d.downloadFile(url.Path, destination, mods...)
 	}
 
-	return d.downloadHttp(uri, destination, mods...)
+	return d.downloadHttp(url, destination, mods...)
 }
 
 func (d DependencyCache) downloadFile(source string, destination string, mods ...RequestModifierFunc) error {
@@ -285,10 +286,10 @@ func (d DependencyCache) downloadFile(source string, destination string, mods ..
 	return nil
 }
 
-func (d DependencyCache) downloadHttp(uri string, destination string, mods ...RequestModifierFunc) error {
-	req, err := http.NewRequest("GET", uri, nil)
+func (d DependencyCache) downloadHttp(url *url.URL, destination string, mods ...RequestModifierFunc) error {
+	req, err := http.NewRequest("GET", url.String(), nil)
 	if err != nil {
-		return fmt.Errorf("unable to create new GET request for %s\n%w", sanitizeUri(uri), err)
+		return fmt.Errorf("unable to create new GET request for %s\n%w", url.Redacted(), err)
 	}
 
 	if d.UserAgent != "" {
@@ -316,12 +317,12 @@ func (d DependencyCache) downloadHttp(uri string, destination string, mods ...Re
 	}
 	resp, err := client.Do(req)
 	if err != nil {
-		return fmt.Errorf("unable to request %s\n%w", sanitizeUri(uri), err)
+		return fmt.Errorf("unable to request %s\n%w", url.Redacted(), err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode < 200 || resp.StatusCode > 299 {
-		return fmt.Errorf("could not download %s: %d", sanitizeUri(uri), resp.StatusCode)
+		return fmt.Errorf("could not download %s: %d", url.Redacted(), resp.StatusCode)
 	}
 
 	if err := os.MkdirAll(filepath.Dir(destination), 0755); err != nil {
@@ -335,18 +336,12 @@ func (d DependencyCache) downloadHttp(uri string, destination string, mods ...Re
 	defer out.Close()
 
 	if _, err := io.Copy(out, resp.Body); err != nil {
-		return fmt.Errorf("unable to copy from %s to %s\n%w", sanitizeUri(uri), destination, err)
+		return fmt.Errorf("unable to copy from %s to %s\n%w", url.Redacted(), destination, err)
 	}
 
 	return nil
 }
 
-func sanitizeUri(uri string) string {
-	uriCreds := regexp.MustCompile(`:\/\/.*:.*@`)
-	uriCredsRepl := "://***:***@"
-	return uriCreds.ReplaceAllString(uri, uriCredsRepl)
-}
-
 func (DependencyCache) verify(path string, expected string) error {
 	s := sha256.New()
 

dependency_cache_test.go

@@ -353,19 +353,28 @@ func testDependencyCache(t *testing.T, context spec.G, it spec.S) {
 				ghttp.RespondWith(http.StatusOK, "test-fixture"),
 			))
 
-			url, err := url.Parse(dependency.URI)
+			url, err := url.ParseRequestURI(dependency.URI)
 			Expect(err).NotTo(HaveOccurred())
-			credentials := "basic-username:basic-password"
-			urlWithBasicCreds := url.Scheme + "://" + credentials + "@" + url.Hostname() + ":" + url.Port() + url.Path
-			dependency.URI = urlWithBasicCreds
+			credentials := "username:password"
+			uriWithBasicCreds := url.Scheme + "://" + credentials + "@" + url.Hostname() + ":" + url.Port() + url.Path
+			dependency.URI = uriWithBasicCreds
 
 			var logBuffer bytes.Buffer
-			captureLogger := bard.NewLogger(&logBuffer)
-			dependencyCache.Logger = captureLogger
-			a, err := dependencyCache.Artifact(dependency)
-			Expect(err).NotTo(HaveOccurred())
+			dependencyCache.Logger = bard.NewLogger(&logBuffer)
+
+			// Make sure the password is not part of the log output.
+			a, errA := dependencyCache.Artifact(dependency)
+			Expect(errA).NotTo(HaveOccurred())
 			Expect(a).NotTo(BeNil())
-			Expect(logBuffer.String()).NotTo(ContainSubstring(credentials))
+			Expect(logBuffer.String()).NotTo(ContainSubstring("password"))
+			logBuffer.Reset()
+
+			// Make sure the password is not part of the log output when an error occurs.
+			dependency.URI = "foo://username:[email protected]"
+			b, errB := dependencyCache.Artifact(dependency)
+			Expect(errB).To(HaveOccurred())
+			Expect(b).To(BeNil())
+			Expect(logBuffer.String()).NotTo(ContainSubstring("password"))
 		})
 	})
 }

layer.go

@@ -291,7 +291,8 @@ func (d *DependencyLayerContributor) Contribute(layer libcnb.Layer, f Dependency
 	return lc.Contribute(layer, func() (libcnb.Layer, error) {
 		artifact, err := d.DependencyCache.Artifact(d.Dependency, d.RequestModifierFuncs...)
 		if err != nil {
-			return libcnb.Layer{}, fmt.Errorf("unable to get dependency %s\n%w", d.Dependency.ID, err)
+			d.Logger.Debugf("fetching dependency %s failed\n%w", d.Dependency.Name, err)
+			return libcnb.Layer{}, fmt.Errorf("unable to get dependency %s. see DEBUG log level", d.Dependency.Name)
 		}
 		defer artifact.Close()