fix(http): redact credentials in request logs (CodeQL clear-text logging) (#328)

Author: adityathebe

files/files.go

@@ -393,7 +393,7 @@ func unzipWithResult(src, dest string, opts *UnarchiveOptions) (*Archive, error)
 			if dirMode == 0 || dirMode&0300 != 0300 {
 				dirMode = 0755
 			}
-			if err := root.MkdirAll(path, dirMode); err != nil {
+			if err := root.MkdirAll(filepath.Clean(path), dirMode); err != nil {
 				return archive, fmt.Errorf("failed to create directory %s: %w", path, err)
 			}
 			archive.Directories = append(archive.Directories, path)
@@ -601,7 +601,7 @@ func UntarWithFilterAndResult(tarball, target string, filter FileFilter, opts *U
 			if dirMode == 0 || dirMode&0300 != 0300 {
 				dirMode = 0755 // Default directory permissions
 			}
-			if err = root.MkdirAll(path, dirMode); err != nil {
+			if err = root.MkdirAll(filepath.Clean(path), dirMode); err != nil {
 				return archive, fmt.Errorf("failed to create directory %s: %w", path, err)
 			}
 			archive.Directories = append(archive.Directories, path)
@@ -676,7 +676,7 @@ func UntarWithFilterAndResult(tarball, target string, filter FileFilter, opts *U
 			if dirMode == 0 || dirMode&0300 != 0300 {
 				dirMode = 0755 // Default directory permissions
 			}
-			if err := root.MkdirAll(path, dirMode); err != nil {
+			if err := root.MkdirAll(filepath.Clean(path), dirMode); err != nil {
 				return archive, fmt.Errorf("failed to create directory %s: %w", path, err)
 			}
 			archive.Directories = append(archive.Directories, path)

http/digest.go

@@ -223,6 +223,15 @@ func (a *digestAuth) computeA2(req *http.Request) string {
 	return fmt.Sprintf("%s:%s", req.Method, a.uri)
 }
 
+// hashStr hashes s with the algorithm negotiated for this digest exchange.
+//
+// MD5 and MD5-sess are retained deliberately. RFC 7616 defines MD5 as the
+// default digest algorithm, the algorithm is selected by the server (not the
+// client), and many servers only offer MD5 — so dropping it would break
+// authentication against those servers. SHA-256 is already used whenever the
+// server advertises it. The md5.New() call below is required for protocol
+// interoperability, not a general-purpose hash of sensitive data; any weak-hash
+// scanner alert on it is a known false positive for HTTP Digest authentication.
 func (a *digestAuth) hashStr(s string) string {
 	var h hash.Hash
 	alg := strings.ToUpper(a.algorithm)

http/middlewares/logger.go

@@ -153,7 +153,9 @@ func accessURL(req *http.Request) string {
 	u := *req.URL
 	u.RawQuery = ""
 	u.Fragment = ""
-	return u.String()
+	// Redacted() masks any password embedded in the URL userinfo so credentials
+	// are never written to logs in clear text.
+	return u.Redacted()
 }
 
 func hasDetailedTrace(config TraceConfig) bool {
@@ -246,8 +248,10 @@ func jsonLogger(config TraceConfig, verbose logger.Verbose, rt http.RoundTripper
 
 	if err != nil {
 		// Transport errors surface at INFO so a failed request is visible at -v=0.
-		kv = append(kv, "error", err.Error())
-		jsonLogAt(verbose, req, 0, kv, "%s %s error %s", req.Method, req.URL, elapsed.Truncate(time.Millisecond))
+		// StripSecrets/Redacted keep any credentials embedded in the URL (e.g. a
+		// url.Error wrapping "https://user:pass@host") out of the logs.
+		kv = append(kv, "error", logger.StripSecrets(err.Error()))
+		jsonLogAt(verbose, req, 0, kv, "%s %s error %s", req.Method, req.URL.Redacted(), elapsed.Truncate(time.Millisecond))
 		return nil, err
 	}
 
@@ -272,15 +276,15 @@ func jsonLogger(config TraceConfig, verbose logger.Verbose, rt http.RoundTripper
 				kv = append(kv, "responseBody", sanitizeBody(body))
 			}
 		}
-		jsonLogAt(verbose, req, 0, kv, "%s %s %d %s", req.Method, req.URL, resp.StatusCode, elapsed.Truncate(time.Millisecond))
+		jsonLogAt(verbose, req, 0, kv, "%s %s %d %s", req.Method, req.URL.Redacted(), resp.StatusCode, elapsed.Truncate(time.Millisecond))
 		return resp, nil
 	}
 
 	// Error-only mode suppresses the success line (see logPrettyAccess).
 	if config.AccessLogErrorsOnly {
 		return resp, nil
 	}
-	jsonLogAt(verbose, req, level, kv, "%s %s %d %s", req.Method, req.URL, resp.StatusCode, elapsed.Truncate(time.Millisecond))
+	jsonLogAt(verbose, req, level, kv, "%s %s %d %s", req.Method, req.URL.Redacted(), resp.StatusCode, elapsed.Truncate(time.Millisecond))
 	return resp, nil
 }
 
@@ -368,7 +372,7 @@ func logPrettyAccess(config TraceConfig, verbose logger.Verbose, req *http.Reque
 	// error-only mode at -v=0; the body captures the cause (e.g. an HTML 404/500
 	// page) without raising verbosity.
 	if err != nil {
-		logAt(verbose, req, 0, "%s %s %s %s", method, url, console.Redf("error: %s", err.Error()), dur)
+		logAt(verbose, req, 0, "%s %s %s %s", method, url, console.Redf("error: %s", logger.StripSecrets(err.Error())), dur)
 		return
 	}
 	statusCode := 0