Merge pull request #2127 from anderbubble/fix-leaky-wwclient

Prevent `assetkey` from leaking into wwclient logs

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ### Fixed
 
+- Prevent `assetkey` from leaking into wwclient logs.
 - Remove requisite dependency between ignition disk target and ignition service. #2083
 - Return HTTP 409 status when creating an existing overlay
 - Allow whitespace to be trimmed for wwdoc comments. #2109

internal/app/wwclient/root.go

@@ -28,6 +28,7 @@ import (
 	"github.com/warewulf/warewulf/internal/pkg/pidfile"
 	"github.com/warewulf/warewulf/internal/pkg/version"
 	"github.com/warewulf/warewulf/internal/pkg/wwlog"
+	"github.com/warewulf/warewulf/internal/pkg/wwurl"
 )
 
 var (
@@ -340,7 +341,7 @@ func updateSystem(target string, ipaddr string, port int, wwid string, tag strin
 				counter = 0
 			}
 			if counter == 0 {
-				wwlog.Error("%s", err)
+				wwlog.Error("%s", wwurl.SanitizeError(err))
 			}
 			counter++
 		}

internal/pkg/warewulfd/parser.go

@@ -53,7 +53,7 @@ func initHandleRequest(w http.ResponseWriter, req *http.Request) (*requestContex
 
 	if remoteNode.AssetKey != "" && remoteNode.AssetKey != rinfo.assetkey {
 		w.WriteHeader(http.StatusUnauthorized)
-		wwlog.Denied("incorrect asset key: node %s: %s", remoteNode.Id(), rinfo.assetkey)
+		wwlog.Denied("incorrect asset key for node %s:", remoteNode.Id())
 		updateStatus(remoteNode.Id(), rinfo.stage, "BAD_ASSET", rinfo.ipaddr)
 		return nil, fmt.Errorf("incorrect asset key")
 	}

internal/pkg/wwurl/sanitize.go

@@ -0,0 +1,41 @@
+package wwurl
+
+import (
+	"net/url"
+	"regexp"
+)
+
+// sensitiveParams are query parameter keys that should be redacted in logs.
+var sensitiveParams = []string{"assetkey"}
+
+// embeddedURLPattern matches an http(s) URL within a larger string, stopping at
+// whitespace or a double quote so it works whether the URL is quoted or not.
+var embeddedURLPattern = regexp.MustCompile(`https?://[^\s"]+`)
+
+// SanitizeURL returns a copy of the URL string with sensitive query parameters
+// replaced by "REDACTED". Useful for logging URLs without leaking secrets.
+func SanitizeURL(rawURL string) string {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return rawURL
+	}
+	q := u.Query()
+	redacted := false
+	for _, key := range sensitiveParams {
+		if q.Has(key) {
+			q.Set(key, "REDACTED")
+			redacted = true
+		}
+	}
+	if redacted {
+		u.RawQuery = q.Encode()
+	}
+	return u.String()
+}
+
+// SanitizeError returns the error message with sensitive URL query parameters
+// redacted. Go's net/http embeds the full request URL (including query params)
+// in transport errors, so this prevents secrets from appearing in log output.
+func SanitizeError(err error) string {
+	return embeddedURLPattern.ReplaceAllStringFunc(err.Error(), SanitizeURL)
+}

internal/pkg/wwurl/sanitize_test.go

@@ -0,0 +1,60 @@
+package wwurl
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestSanitizeURL(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "redacts assetkey",
+			input: "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?assetkey=secretvalue&compress=gz&stage=runtime&uuid=62184d56-6d53-9895-0b51-035f457c496f",
+			want:  "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?assetkey=REDACTED&compress=gz&stage=runtime&uuid=62184d56-6d53-9895-0b51-035f457c496f",
+		},
+		{
+			name:  "no assetkey unchanged",
+			input: "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?compress=gz&stage=runtime",
+			want:  "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?compress=gz&stage=runtime",
+		},
+		{
+			name:  "invalid URL returned as-is",
+			input: "not a url ://bad",
+			want:  "not a url ://bad",
+		},
+		{
+			name:  "empty string",
+			input: "",
+			want:  "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := SanitizeURL(tt.input)
+			if got != tt.want {
+				t.Errorf("SanitizeURL(%q)\n got:  %q\n want: %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestSanitizeError(t *testing.T) {
+	// Simulate a Go net/http transport error that embeds the full URL.
+	errMsg := `Get "https://192.168.3.1:9874/provision/00:0c:29:7c:49:6f?assetkey=mysecret&compress=gz&stage=runtime&uuid=abc123": dial tcp 192.168.3.1:9874: connect: connection refused`
+	err := fmt.Errorf("%s", errMsg)
+	got := SanitizeError(err)
+	if got == errMsg {
+		t.Error("SanitizeError did not redact assetkey from error message")
+	}
+	if strings.Contains(got, "mysecret") {
+		t.Errorf("SanitizeError still contains secret: %q", got)
+	}
+	if !strings.Contains(got, "REDACTED") {
+		t.Errorf("SanitizeError missing REDACTED marker: %q", got)
+	}
+}