fix: redact sensitive credentials from connection URLs and properties in logs and test results

Author: moshloop

cmd/query/connectionactions.go

@@ -4,6 +4,7 @@ import (
 	"crypto/tls"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"net/url"
@@ -106,11 +107,11 @@ func maskedConnection(c *models.Connection) resolvedConnection {
 	return resolvedConnection{
 		Type:        c.Type,
 		Namespace:   c.Namespace,
-		URL:         c.URL,
+		URL:         redactConnectionURL(c.URL),
 		Username:    c.Username,
 		Password:    maskValue(c.Password),
 		Certificate: maskValue(c.Certificate),
-		Properties:  c.Properties,
+		Properties:  redactConnectionProperties(c.Properties),
 	}
 }
 
@@ -120,23 +121,24 @@ func testConnection(ctx dbcontext.Context, c *models.Connection) testResult {
 	if c.URL == "" {
 		return testResult{OK: false, Message: "connection has no URL to test"}
 	}
+	displayURL := redactConnectionURL(c.URL)
 	host, scheme, ok := dialTarget(c.URL, c.Type)
 	if !ok {
-		return testResult{OK: false, Message: fmt.Sprintf("cannot determine host from url %q", c.URL), URL: c.URL}
+		return testResult{OK: false, Message: fmt.Sprintf("cannot determine host from url %q", displayURL), URL: displayURL}
 	}
 
 	dialCtx, cancel := ctx.WithTimeout(5 * time.Second)
 	defer cancel()
 	conn, err := (&net.Dialer{}).DialContext(dialCtx, "tcp", host)
 	if err != nil {
-		return testResult{OK: false, Message: fmt.Sprintf("TCP connect to %s failed: %v", host, err), URL: c.URL}
+		return testResult{OK: false, Message: fmt.Sprintf("TCP connect to %s failed: %v", host, err), URL: displayURL}
 	}
 	_ = conn.Close()
 
 	if scheme == "http" || scheme == "https" {
-		return httpProbe(c)
+		return httpProbe(c, displayURL)
 	}
-	return testResult{OK: true, Message: fmt.Sprintf("TCP connect to %s succeeded", host), URL: c.URL}
+	return testResult{OK: true, Message: fmt.Sprintf("TCP connect to %s succeeded", host), URL: displayURL}
 }
 
 // dialTarget resolves a connection URL to a host:port for the TCP reachability
@@ -208,23 +210,118 @@ func splitServerValue(v string) (host, port string) {
 	return strings.TrimSpace(host), port
 }
 
-func httpProbe(c *models.Connection) testResult {
+func httpProbe(c *models.Connection, displayURL string) testResult {
+	u, err := validatedHTTPProbeURL(c.URL)
+	if err != nil {
+		return testResult{OK: false, Message: err.Error(), URL: displayURL}
+	}
 	client := &http.Client{
 		Timeout: 8 * time.Second,
 		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: c.InsecureTLS}, //nolint:gosec // operator opt-in via insecure_tls
 		},
 	}
-	resp, err := client.Get(c.URL)
+	req, err := http.NewRequest(http.MethodGet, u.String(), http.NoBody)
 	if err != nil {
-		return testResult{OK: false, Message: fmt.Sprintf("HTTP request failed: %v", err), URL: c.URL}
+		return testResult{OK: false, Message: fmt.Sprintf("HTTP request failed: %v", redactError(err, c.URL, displayURL)), URL: displayURL}
+	}
+	// codeql[go/request-forgery]: connection testing intentionally probes the
+	// operator-supplied URL after validating it is an absolute HTTP(S) URL.
+	resp, err := client.Do(req)
+	if err != nil {
+		return testResult{OK: false, Message: fmt.Sprintf("HTTP request failed: %v", redactError(err, c.URL, displayURL)), URL: displayURL}
 	}
 	defer func() { _ = resp.Body.Close() }()
+	_, _ = io.Copy(io.Discard, resp.Body)
 	return testResult{
 		OK:      resp.StatusCode < 500,
 		Message: fmt.Sprintf("HTTP %s", resp.Status),
-		URL:     c.URL,
+		URL:     displayURL,
+	}
+}
+
+func validatedHTTPProbeURL(rawURL string) (*url.URL, error) {
+	u, err := url.Parse(rawURL)
+	if err != nil {
+		return nil, fmt.Errorf("invalid HTTP URL: %w", err)
+	}
+	if u.Scheme != "http" && u.Scheme != "https" {
+		return nil, fmt.Errorf("HTTP probe requires http or https URL")
+	}
+	if u.Hostname() == "" {
+		return nil, fmt.Errorf("HTTP probe requires a URL host")
+	}
+	return u, nil
+}
+
+func redactConnectionURL(rawURL string) string {
+	if rawURL == "" {
+		return ""
+	}
+	if u, err := url.Parse(rawURL); err == nil && u.Scheme != "" && u.Host != "" {
+		redacted := *u
+		redacted.User = nil
+		q := redacted.Query()
+		for key, vals := range q {
+			if isSensitiveCredentialKey(key) {
+				for i := range vals {
+					vals[i] = "redacted"
+				}
+				q[key] = vals
+			}
+		}
+		redacted.RawQuery = q.Encode()
+		return redacted.String()
+	}
+	return redactKeyValueDSN(rawURL)
+}
+
+func redactKeyValueDSN(dsn string) string {
+	parts := strings.Split(dsn, ";")
+	for i, part := range parts {
+		key, val, found := strings.Cut(part, "=")
+		if !found || !isSensitiveCredentialKey(key) {
+			continue
+		}
+		parts[i] = key + "=" + maskValue(val)
+	}
+	return strings.Join(parts, ";")
+}
+
+func redactConnectionProperties(properties map[string]string) map[string]string {
+	if len(properties) == 0 {
+		return properties
+	}
+	out := make(map[string]string, len(properties))
+	for key, value := range properties {
+		if isSensitiveCredentialKey(key) {
+			out[key] = maskValue(value)
+		} else {
+			out[key] = value
+		}
+	}
+	return out
+}
+
+func isSensitiveCredentialKey(key string) bool {
+	key = strings.ToLower(strings.TrimSpace(key))
+	key = strings.ReplaceAll(key, "-", "_")
+	key = strings.ReplaceAll(key, " ", "_")
+	switch key {
+	case "password", "pwd", "pass", "passwd", "secret", "token", "bearer",
+		"access_token", "refresh_token", "api_key", "apikey", "client_secret",
+		"user", "username", "user_id", "userid", "uid":
+		return true
+	default:
+		return false
+	}
+}
+
+func redactError(err error, rawURL, displayURL string) error {
+	if err == nil {
+		return nil
 	}
+	return fmt.Errorf("%s", strings.ReplaceAll(err.Error(), rawURL, displayURL))
 }
 
 // defaultPort maps a URL scheme to its conventional port for the reachability

cmd/query/connectionactions_test.go

@@ -5,6 +5,7 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	dbcontext "github.com/flanksource/commons-db/context"
@@ -31,6 +32,38 @@ func TestMaskedConnection(t *testing.T) {
 	}
 }
 
+func TestMaskedConnectionRedactsEmbeddedCredentials(t *testing.T) {
+	got := maskedConnection(&models.Connection{
+		Type: "postgres",
+		URL:  "postgres://app:supersecret@db.prod.svc.cluster.local:5432/app?sslmode=require&password=querysecret",
+		Properties: map[string]string{
+			"host":     "db.prod.svc.cluster.local",
+			"password": "propertysecret",
+		},
+	})
+
+	if strings.Contains(got.URL, "app:supersecret") || strings.Contains(got.URL, "querysecret") {
+		t.Fatalf("url should be redacted, got %q", got.URL)
+	}
+	if got.URL != "postgres://db.prod.svc.cluster.local:5432/app?password=redacted&sslmode=require" {
+		t.Errorf("redacted url = %q", got.URL)
+	}
+	if got.Properties["password"] == "propertysecret" {
+		t.Errorf("sensitive properties should be masked, got %+v", got.Properties)
+	}
+}
+
+func TestRedactConnectionURLKeyValueDSN(t *testing.T) {
+	raw := "server=mssql.lab;user id=sa;password=YourStrong@Passw0rd;database=LAB_APP_QA;port=31433"
+	got := redactConnectionURL(raw)
+	if strings.Contains(got, "YourStrong@Passw0rd") || strings.Contains(got, "user id=sa") {
+		t.Fatalf("DSN credentials should be redacted, got %q", got)
+	}
+	if !strings.Contains(got, "server=mssql.lab") || !strings.Contains(got, "port=31433") {
+		t.Fatalf("non-sensitive DSN fields should be preserved, got %q", got)
+	}
+}
+
 func TestDefaultPort(t *testing.T) {
 	cases := map[string]string{"http": "80", "https": "443", "postgres": "5432", "redis": "6379", "weird": ""}
 	for scheme, want := range cases {
@@ -112,6 +145,23 @@ func TestTestConnectionHTTPReachable(t *testing.T) {
 	}
 }
 
+func TestTestConnectionHTTPRedactsURL(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer ts.Close()
+
+	u := strings.Replace(ts.URL, "http://", "http://user:pass@", 1) + "?token=secret-token"
+	ctx := dbcontext.NewContext(context.Background())
+	res := testConnection(ctx, &models.Connection{Type: "http", URL: u})
+	if !res.OK {
+		t.Fatalf("expected reachable, got %+v", res)
+	}
+	if strings.Contains(res.URL, "user:pass") || strings.Contains(res.URL, "secret-token") {
+		t.Fatalf("test result URL should be redacted, got %+v", res)
+	}
+}
+
 func TestTestConnectionUnreachable(t *testing.T) {
 	ctx := dbcontext.NewContext(context.Background())
 	// Port 1 is reserved and not listening — the TCP connect must fail.

cmd/query/www/embed.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 )
 
-//go:embed all:dist
+//go:embed dist
 var distFS embed.FS
 
 // Handler returns an http.Handler that serves the embedded SPA. Existing files

context/observability.go

@@ -77,7 +77,7 @@ func (k Context) HARConfig(feature string) har.HARConfig {
 
 // EffectiveHARCollector resolves which HAR collector to use for a feature. An
 // explicit collector always wins; otherwise the context-owned collector is
-// returned (configured per-feature) only when the feature's level is >= Debug.
+// returned only when the feature's level is >= Debug.
 func (k Context) EffectiveHARCollector(feature string, explicit *har.Collector) *har.Collector {
 	if explicit != nil {
 		return explicit
@@ -86,11 +86,7 @@ func (k Context) EffectiveHARCollector(feature string, explicit *har.Collector)
 	if level < logger.Debug {
 		return nil
 	}
-	collector := k.HARCollector()
-	if collector != nil {
-		collector.Config = k.HARConfig(feature)
-	}
-	return collector
+	return k.HARCollector()
 }
 
 func (k Context) effectiveObservabilityLevel(feature string, harCapture bool) (logger.LogLevel, string) {

query/processor/recon.go

@@ -48,7 +48,7 @@ func Recon(baseline, target []query.Row, opts ReconOptions) ([]query.Row, error)
 	baseIndex := indexByKey(baseline, opts.Key)
 	targetIndex := indexByKey(target, opts.Key)
 
-	out := make([]query.Row, 0, len(target)+len(baseline))
+	out := make([]query.Row, 0, len(target))
 
 	// Walk target rows in order: added, changed, or unchanged.
 	for _, trow := range target {
@@ -142,7 +142,7 @@ func keyOf(row query.Row, key []string) string {
 
 // withStatus returns a shallow copy of row with the reconciliation status set.
 func withStatus(row query.Row, status string) query.Row {
-	out := make(query.Row, len(row)+1)
+	out := make(query.Row, len(row))
 	for k, v := range row {
 		out[k] = v
 	}