Merge pull request #312 from bitgully/main

Obfuscate Credentials in Logs

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

@@ -168,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 {
@@ -177,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"), uri)
+		d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), urlP.Redacted())
 		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", uri, err)
+		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)
@@ -201,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))
@@ -215,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"), uri)
+	d.Logger.Bodyf("%s from %s", color.YellowString("Downloading"), urlP.Redacted())
 	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", uri, err)
+	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")
@@ -247,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", 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 {
@@ -284,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", uri, err)
+		return fmt.Errorf("unable to create new GET request for %s\n%w", url.Redacted(), err)
 	}
 
 	if d.UserAgent != "" {
@@ -315,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", 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", 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 {
@@ -334,7 +336,7 @@ 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", uri, destination, err)
+		return fmt.Errorf("unable to copy from %s to %s\n%w", url.Redacted(), destination, err)
 	}
 
 	return nil

dependency_cache_test.go

@@ -17,9 +17,11 @@
 package libpak_test
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"os"
 	"path/filepath"
 	"testing"
@@ -32,6 +34,7 @@ import (
 	"github.com/sclevine/spec"
 
 	"github.com/paketo-buildpacks/libpak"
+	"github.com/paketo-buildpacks/libpak/bard"
 )
 
 func testDependencyCache(t *testing.T, context spec.G, it spec.S) {
@@ -344,5 +347,34 @@ func testDependencyCache(t *testing.T, context spec.G, it spec.S) {
 
 			Expect(io.ReadAll(a)).To(Equal([]byte("test-fixture")))
 		})
+
+		it("hide uri credentials from log", func() {
+			server.AppendHandlers(ghttp.CombineHandlers(
+				ghttp.RespondWith(http.StatusOK, "test-fixture"),
+			))
+
+			url, err := url.ParseRequestURI(dependency.URI)
+			Expect(err).NotTo(HaveOccurred())
+			credentials := "username:password"
+			uriWithBasicCreds := url.Scheme + "://" + credentials + "@" + url.Hostname() + ":" + url.Port() + url.Path
+			dependency.URI = uriWithBasicCreds
+
+			var logBuffer bytes.Buffer
+			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("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()