fix: Sanitize error messages to prevent information disclosure (fixes #34) (#66)

Addresses CWE-209 by removing internal implementation details from
user-facing error messages throughout the codebase.

Changes:
- Created errors.go with ConversionError type for secure error handling
- User-facing messages are generic and don't leak schema/type information
- Internal details preserved for logging (WithLogger option)
- Updated error messages in:
  * cel2sql.go: ~20 locations (expression types, operators, comprehensions)
  * pg/provider.go: 2 locations (enum and type errors)
  * comprehensions.go: 1 location (pattern recognition)
  * timestamps.go: 4 locations (operation and duration errors)
- Added comprehensive test suite in error_messages_test.go
- All tests pass with 90%+ coverage
- Linting clean (revive, unparam)

Before: "unsupported expr: &{ExprKind:0x...}"
After:  "unsupported expression type"

Internal details still available via structured logging for debugging.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude <noreply@anthropic.com>

Author: richardwooding

cel2sql.go

@@ -243,7 +243,7 @@ func (con *converter) visit(expr *exprpb.Expr) error {
 	case *exprpb.Expr_StructExpr:
 		return con.visitStruct(expr)
 	}
-	return fmt.Errorf("unsupported expr: %v", expr)
+	return newConversionErrorf(errMsgUnsupportedExpression, "expr type: %T, id: %d", expr.ExprKind, expr.Id)
 }
 
 // isFieldJSON checks if a field in a table is a JSON/JSONB type using schema information
@@ -459,7 +459,7 @@ func (con *converter) visitCallBinary(expr *exprpb.Expr) error {
 	} else if op, found := operators.FindReverseBinaryOperator(fun); found {
 		operator = op
 	} else {
-		return fmt.Errorf("cannot unmangle operator: %s", fun)
+		return newConversionErrorf(errMsgInvalidOperator, "binary operator: %s", fun)
 	}
 
 	con.logger.LogAttrs(context.Background(), slog.LevelDebug,
@@ -840,7 +840,7 @@ func (con *converter) visitCallFunc(expr *exprpb.Expr) error {
 				con.str.WriteString(", 1)")
 				return nil
 			default:
-				return fmt.Errorf("unsupported type: %v", argType)
+				return newConversionErrorf(errMsgUnsupportedType, "size() argument type: %s", argType.String())
 			}
 		} else {
 			sqlFun = strings.ToUpper(fun)
@@ -926,7 +926,7 @@ func (con *converter) visitCallUnary(expr *exprpb.Expr) error {
 	} else if op, found := operators.FindReverse(fun); found {
 		operator = op
 	} else {
-		return fmt.Errorf("cannot unmangle operator: %s", fun)
+		return newConversionErrorf(errMsgInvalidOperator, "unary operator: %s", fun)
 	}
 	con.str.WriteString(operator)
 	nested := isComplexOperator(args[0])
@@ -962,7 +962,7 @@ func (con *converter) visitComprehension(expr *exprpb.Expr) error {
 	case ComprehensionTransformMapEntry:
 		return con.visitTransformMapEntryComprehension(expr, info)
 	default:
-		return fmt.Errorf("unsupported comprehension type: %v", info.Type)
+		return newConversionErrorf(errMsgUnsupportedComprehension, "comprehension type: %s", info.Type.String())
 	}
 }
 
@@ -975,7 +975,7 @@ func (con *converter) visitAllComprehension(expr *exprpb.Expr, info *Comprehensi
 
 	comprehension := expr.GetComprehensionExpr()
 	if comprehension == nil {
-		return errors.New("expression is not a comprehension")
+		return newConversionError(errMsgUnsupportedComprehension, "expression is not a comprehension (ALL)")
 	}
 
 	iterRange := comprehension.GetIterRange()
@@ -988,13 +988,13 @@ func (con *converter) visitAllComprehension(expr *exprpb.Expr, info *Comprehensi
 		con.str.WriteString(jsonFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in ALL comprehension: %w", err)
+			return wrapConversionError(err, "visiting iter range in ALL comprehension")
 		}
 		con.str.WriteString(")")
 	} else {
 		con.str.WriteString("UNNEST(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in ALL comprehension: %w", err)
+			return wrapConversionError(err, "visiting iter range in ALL comprehension")
 		}
 		con.str.WriteString(")")
 	}
@@ -1007,14 +1007,14 @@ func (con *converter) visitAllComprehension(expr *exprpb.Expr, info *Comprehensi
 	// Add null checks for JSON arrays
 	if isJSONArray {
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range for null check: %w", err)
+			return wrapConversionError(err,"visiting iter range for null check")
 		}
 		con.str.WriteString(" IS NOT NULL AND ")
 		typeofFunc := con.getJSONTypeofFunction(iterRange)
 		con.str.WriteString(typeofFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range for type check: %w", err)
+			return wrapConversionError(err,"visiting iter range for type check")
 		}
 		con.str.WriteString(") = 'array'")
 
@@ -1026,7 +1026,7 @@ func (con *converter) visitAllComprehension(expr *exprpb.Expr, info *Comprehensi
 	if info.Predicate != nil {
 		con.str.WriteString("NOT (")
 		if err := con.visit(info.Predicate); err != nil {
-			return fmt.Errorf("failed to visit predicate in ALL comprehension: %w", err)
+			return wrapConversionError(err,"visiting predicate in ALL comprehension")
 		}
 		con.str.WriteString(")")
 	}
@@ -1042,7 +1042,7 @@ func (con *converter) visitExistsComprehension(expr *exprpb.Expr, info *Comprehe
 
 	comprehension := expr.GetComprehensionExpr()
 	if comprehension == nil {
-		return errors.New("expression is not a comprehension")
+		return newConversionError(errMsgUnsupportedComprehension, "expression is not a comprehension (EXISTS)")
 	}
 
 	iterRange := comprehension.GetIterRange()
@@ -1055,13 +1055,13 @@ func (con *converter) visitExistsComprehension(expr *exprpb.Expr, info *Comprehe
 		con.str.WriteString(jsonFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in EXISTS comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in EXISTS comprehension")
 		}
 		con.str.WriteString(")")
 	} else {
 		con.str.WriteString("UNNEST(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in EXISTS comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in EXISTS comprehension")
 		}
 		con.str.WriteString(")")
 	}
@@ -1074,14 +1074,14 @@ func (con *converter) visitExistsComprehension(expr *exprpb.Expr, info *Comprehe
 	// Add null checks for JSON arrays
 	if isJSONArray {
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range for null check: %w", err)
+			return wrapConversionError(err,"visiting iter range for null check")
 		}
 		con.str.WriteString(" IS NOT NULL AND ")
 		typeofFunc := con.getJSONTypeofFunction(iterRange)
 		con.str.WriteString(typeofFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range for type check: %w", err)
+			return wrapConversionError(err,"visiting iter range for type check")
 		}
 		con.str.WriteString(") = 'array'")
 
@@ -1092,7 +1092,7 @@ func (con *converter) visitExistsComprehension(expr *exprpb.Expr, info *Comprehe
 
 	if info.Predicate != nil {
 		if err := con.visit(info.Predicate); err != nil {
-			return fmt.Errorf("failed to visit predicate in EXISTS comprehension: %w", err)
+			return wrapConversionError(err,"visiting predicate in EXISTS comprehension")
 		}
 	}
 
@@ -1107,7 +1107,7 @@ func (con *converter) visitExistsOneComprehension(expr *exprpb.Expr, info *Compr
 
 	comprehension := expr.GetComprehensionExpr()
 	if comprehension == nil {
-		return errors.New("expression is not a comprehension")
+		return newConversionError(errMsgUnsupportedComprehension, "expression is not a comprehension (EXISTS_ONE)")
 	}
 
 	iterRange := comprehension.GetIterRange()
@@ -1120,13 +1120,13 @@ func (con *converter) visitExistsOneComprehension(expr *exprpb.Expr, info *Compr
 		con.str.WriteString(jsonFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in EXISTS_ONE comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in EXISTS_ONE comprehension")
 		}
 		con.str.WriteString(")")
 	} else {
 		con.str.WriteString("UNNEST(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in EXISTS_ONE comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in EXISTS_ONE comprehension")
 		}
 		con.str.WriteString(")")
 	}
@@ -1139,14 +1139,14 @@ func (con *converter) visitExistsOneComprehension(expr *exprpb.Expr, info *Compr
 	// Add null checks for JSON arrays
 	if isJSONArray {
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range for null check: %w", err)
+			return wrapConversionError(err,"visiting iter range for null check")
 		}
 		con.str.WriteString(" IS NOT NULL AND ")
 		typeofFunc := con.getJSONTypeofFunction(iterRange)
 		con.str.WriteString(typeofFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range for type check: %w", err)
+			return wrapConversionError(err,"visiting iter range for type check")
 		}
 		con.str.WriteString(") = 'array'")
 
@@ -1157,7 +1157,7 @@ func (con *converter) visitExistsOneComprehension(expr *exprpb.Expr, info *Compr
 
 	if info.Predicate != nil {
 		if err := con.visit(info.Predicate); err != nil {
-			return fmt.Errorf("failed to visit predicate in EXISTS_ONE comprehension: %w", err)
+			return wrapConversionError(err,"visiting predicate in EXISTS_ONE comprehension")
 		}
 	}
 
@@ -1172,7 +1172,7 @@ func (con *converter) visitMapComprehension(expr *exprpb.Expr, info *Comprehensi
 
 	comprehension := expr.GetComprehensionExpr()
 	if comprehension == nil {
-		return errors.New("expression is not a comprehension")
+		return newConversionError(errMsgUnsupportedComprehension, "expression is not a comprehension (MAP)")
 	}
 
 	iterRange := comprehension.GetIterRange()
@@ -1183,7 +1183,7 @@ func (con *converter) visitMapComprehension(expr *exprpb.Expr, info *Comprehensi
 	// Visit the transform expression
 	if info.Transform != nil {
 		if err := con.visit(info.Transform); err != nil {
-			return fmt.Errorf("failed to visit transform in MAP comprehension: %w", err)
+			return wrapConversionError(err,"visiting transform in MAP comprehension")
 		}
 	} else {
 		// If no transform, just return the variable itself
@@ -1197,13 +1197,13 @@ func (con *converter) visitMapComprehension(expr *exprpb.Expr, info *Comprehensi
 		con.str.WriteString(jsonFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in MAP comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in MAP comprehension")
 		}
 		con.str.WriteString(")")
 	} else {
 		con.str.WriteString("UNNEST(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in MAP comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in MAP comprehension")
 		}
 		con.str.WriteString(")")
 	}
@@ -1215,7 +1215,7 @@ func (con *converter) visitMapComprehension(expr *exprpb.Expr, info *Comprehensi
 	if info.Filter != nil {
 		con.str.WriteString(" WHERE ")
 		if err := con.visit(info.Filter); err != nil {
-			return fmt.Errorf("failed to visit filter in MAP comprehension: %w", err)
+			return wrapConversionError(err,"visiting filter in MAP comprehension")
 		}
 	}
 
@@ -1230,7 +1230,7 @@ func (con *converter) visitFilterComprehension(expr *exprpb.Expr, info *Comprehe
 
 	comprehension := expr.GetComprehensionExpr()
 	if comprehension == nil {
-		return errors.New("expression is not a comprehension")
+		return newConversionError(errMsgUnsupportedComprehension, "expression is not a comprehension (FILTER)")
 	}
 
 	iterRange := comprehension.GetIterRange()
@@ -1245,13 +1245,13 @@ func (con *converter) visitFilterComprehension(expr *exprpb.Expr, info *Comprehe
 		con.str.WriteString(jsonFunc)
 		con.str.WriteString("(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in FILTER comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in FILTER comprehension")
 		}
 		con.str.WriteString(")")
 	} else {
 		con.str.WriteString("UNNEST(")
 		if err := con.visit(iterRange); err != nil {
-			return fmt.Errorf("failed to visit iter range in FILTER comprehension: %w", err)
+			return wrapConversionError(err,"visiting iter range in FILTER comprehension")
 		}
 		con.str.WriteString(")")
 	}
@@ -1262,7 +1262,7 @@ func (con *converter) visitFilterComprehension(expr *exprpb.Expr, info *Comprehe
 	if info.Predicate != nil {
 		con.str.WriteString(" WHERE ")
 		if err := con.visit(info.Predicate); err != nil {
-			return fmt.Errorf("failed to visit predicate in FILTER comprehension: %w", err)
+			return wrapConversionError(err,"visiting predicate in FILTER comprehension")
 		}
 	}
 
@@ -1276,15 +1276,15 @@ func (con *converter) visitTransformListComprehension(expr *exprpb.Expr, info *C
 
 	comprehension := expr.GetComprehensionExpr()
 	if comprehension == nil {
-		return errors.New("expression is not a comprehension")
+		return newConversionError(errMsgUnsupportedComprehension, "expression is not a comprehension (TRANSFORM_LIST)")
 	}
 
 	con.str.WriteString("ARRAY(SELECT ")
 
 	// Visit the transform expression
 	if info.Transform != nil {
 		if err := con.visit(info.Transform); err != nil {
-			return fmt.Errorf("failed to visit transform in TRANSFORM_LIST comprehension: %w", err)
+			return wrapConversionError(err,"visiting transform in TRANSFORM_LIST comprehension")
 		}
 	} else {
 		// If no transform, just return the variable itself
@@ -1295,7 +1295,7 @@ func (con *converter) visitTransformListComprehension(expr *exprpb.Expr, info *C
 
 	// Visit the iterable range (the array/list being comprehended over)
 	if err := con.visit(comprehension.GetIterRange()); err != nil {
-		return fmt.Errorf("failed to visit iter range in TRANSFORM_LIST comprehension: %w", err)
+		return wrapConversionError(err,"visiting iter range in TRANSFORM_LIST comprehension")
 	}
 
 	con.str.WriteString(") AS ")
@@ -1305,7 +1305,7 @@ func (con *converter) visitTransformListComprehension(expr *exprpb.Expr, info *C
 	if info.Filter != nil {
 		con.str.WriteString(" WHERE ")
 		if err := con.visit(info.Filter); err != nil {
-			return fmt.Errorf("failed to visit filter in TRANSFORM_LIST comprehension: %w", err)
+			return wrapConversionError(err,"visiting filter in TRANSFORM_LIST comprehension")
 		}
 	}
 
@@ -1365,7 +1365,7 @@ func (con *converter) visitConst(expr *exprpb.Expr) error {
 		ui := strconv.FormatUint(c.GetUint64Value(), 10)
 		con.str.WriteString(ui)
 	default:
-		return fmt.Errorf("unimplemented : %v", expr)
+		return newConversionErrorf(errMsgUnsupportedExpression, "constant type: %T", c.ConstantKind)
 	}
 	return nil
 }

comprehensions.go

@@ -3,7 +3,6 @@ package cel2sql
 import (
 	"context"
 	"errors"
-	"fmt"
 	"log/slog"
 
 	"github.com/google/cel-go/common/operators"
@@ -183,7 +182,7 @@ func (con *converter) analyzeComprehensionPattern(comp *exprpb.Expr_Comprehensio
 
 	// If we can't identify the pattern, mark as unknown for now
 	info.Type = ComprehensionUnknown
-	return info, fmt.Errorf("unrecognized comprehension pattern for %s", comp.String())
+	return info, newConversionError(errMsgUnsupportedComprehension, "unrecognized comprehension pattern")
 }
 
 // Helper functions to identify patterns in comprehension expressions