Skip to content

fix: validate column before query #87

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ren0503 merged 2 commits into from
Jan 7, 2026
Merged

fix: validate column before query #87

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed0da54
fix: validate column before query
ren0503 Dec 29, 2025
190036d
Merge branch 'master' into fix/ren/84-validate-column-before-query
ren0503 Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"reflect"
"strings"

"github.com/tinh-tinh/tinhtinh/v2/dto/validator"
"gorm.io/gorm"
)

Expand All @@ -24,18 +25,27 @@ type QueryBuilder struct {
}

func (q *QueryBuilder) Equal(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
Comment on lines +28 to +30
Copy link

@coderabbitai coderabbitai bot Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Major: Silent validation failure obscures errors and could cause incorrect query results.

When validation fails, the method returns the unchanged QueryBuilder without any error, log, or indication. This means:

  • Queries silently omit the intended WHERE clause
  • Developers have no feedback that validation failed
  • Security-critical filters could be silently dropped, potentially causing data leaks

Consider one of these approaches:

  1. Return an error alongside the QueryBuilder
  2. Log a warning when validation fails
  3. Panic if invalid columns indicate a programming error

This comment applies to all 14 methods with validation guards.

query := column + " = ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) Not(column string, args ...interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " = ?"
q.qb = q.qb.Not(query, args...)
return q
}

func (q *QueryBuilder) Or(column string, args ...interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " = ?"
q.qb = q.qb.Or(query, args...)
return q
Expand All @@ -46,54 +56,81 @@ func (q *QueryBuilder) In(column string, values ...interface{}) *QueryBuilder {
for i := range values {
placeholders[i] = "?"
}
if !isValidColumn(column) {
return q
}
query := column + " IN (" + strings.Join(placeholders, ", ") + ")"
q.qb = q.qb.Where(query, values...)
return q
}

func (q *QueryBuilder) MoreThan(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " > ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) MoreThanOrEqual(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " >= ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) LessThan(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " < ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) LessThanOrEqual(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " <= ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) Like(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " LIKE ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) ILike(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " ILIKE ?"
q.qb = q.qb.Where(query, value)
return q
}

func (q *QueryBuilder) Between(column string, start interface{}, end interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " BETWEEN ? AND ?"
q.qb = q.qb.Where(query, start, end)
return q
}

func (q *QueryBuilder) NotEqual(column string, value interface{}) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " <> ?"
q.qb = q.qb.Where(query, value)
return q
Expand All @@ -104,12 +141,18 @@ func (q *QueryBuilder) NotIn(column string, values ...interface{}) *QueryBuilder
for i := range values {
placeholders[i] = "?"
}
if !isValidColumn(column) {
return q
}
query := column + " NOT IN (" + strings.Join(placeholders, ", ") + ")"
q.qb = q.qb.Where(query, values...)
return q
}

func (q *QueryBuilder) IsNull(column string) *QueryBuilder {
if !isValidColumn(column) {
return q
}
query := column + " IS NULL"
q.qb = q.qb.Where(query)
return q
Expand All @@ -119,3 +162,7 @@ func (q *QueryBuilder) Raw(sql string, values ...interface{}) *QueryBuilder {
q.qb = q.qb.Raw(sql, values...)
return q
}

func isValidColumn(column string) bool {
return validator.IsAlphanumeric(column)
}
Comment on lines +166 to +168
Copy link

@coderabbitai coderabbitai bot Dec 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the validator package and IsAlphanumeric implementation
rg -nP 'IsAlphanumeric' --type=go -B 2 -A 5

Repository: tinh-tinh/sqlorm

Length of output: 204

🏁 Script executed:

#!/bin/bash
# Check if validator is in the imports and from where
rg -nP 'package\s+validator|func\s+IsAlphanumeric' --type=go -A 5

Repository: tinh-tinh/sqlorm

Length of output: 42

🏁 Script executed:

#!/bin/bash
# Search for column usages with underscores in builder method calls
rg -nP '\.(Equal|Not|Or|In|MoreThan|LessThan|Like|ILike|Between|NotEqual|NotIn|IsNull)\s*\(\s*["\x60][a-zA-Z_][a-zA-Z0-9_]*' --type=go

Repository: tinh-tinh/sqlorm

Length of output: 744

🏁 Script executed:

#!/bin/bash
# Check the import statement for the validator package
head -20 builder.go | grep -E 'import|validator'

Repository: tinh-tinh/sqlorm

Length of output: 118

🏁 Script executed:

#!/bin/bash
# Search for any documentation, comments, or examples about column validation
rg -nP 'column|Column' --type=go builder.go -B 2 -A 2

Repository: tinh-tinh/sqlorm

Length of output: 3692

🏁 Script executed:

#!/bin/bash
# Look for the validator package files in vendor or any local definitions
find . -type f -name "validator.go" -o -name "validator" -type d

Repository: tinh-tinh/sqlorm

Length of output: 42

🌐 Web query:

github.com/tinh-tinh/tinhtinh validator IsAlphanumeric documentation

💡 Result:

Do you mean the IsAlphanumeric validator inside the TinhTinh repo (github.com/tinh-tinh/tinhtinh) or the isAlphanumeric from validator.js (npm/validator)? I can look up the specific documentation — which one should I fetch?

🏁 Script executed:

#!/bin/bash
# Check go.mod for validator package details
cat go.mod | grep -E 'tinhtinh|require'

Repository: tinh-tinh/sqlorm

Length of output: 120

🏁 Script executed:

#!/bin/bash
# Check if there's any documentation in comments about column validation
rg -nP '//.*column|/\*.*column' --type=go -i

Repository: tinh-tinh/sqlorm

Length of output: 42

🌐 Web query:

TinhTinh Go validator IsAlphanumeric dto package behavior implementation

💡 Result:

I’m not finding a clear match for “TinhTinh” + Go validator. Do you mean a specific GitHub repo or package (e.g., govalidator / go-playground/validator), or do you want how an IsAlphanumeric validator should behave/be implemented for a DTO in Go? If you point me to the repo or confirm which you mean, I’ll fetch the code and explain the behavior and implementation.

🏁 Script executed:

#!/bin/bash
# Try to find more about the validator package in go.sum
cat go.sum | grep tinhtinh

Repository: tinh-tinh/sqlorm

Length of output: 242

🌐 Web query:

site:github.com tinh-tinh/tinhtinh dto validator IsAlphanumeric

💡 Result:

I couldn't find a GitHub file matching "site:github.com tinh-tinh/tinhtinh dto validator IsAlphanumeric". The tinh-tinh organization and tinhtinh modules exist (Go packages) but no direct DTO validator/IsAlphanumeric hit under tinhtinh was returned by the search. [1][2]

If you want, I can:

  • search the tinhtinh repo tree on GitHub for "validator", "dto" or "IsAlphanumeric"; or
  • search more broadly for DTO validators (e.g., class-validator, validator.js) and examples of an "IsAlphanumeric" check.

Sources:
[1] Libraries.io entry for github.com/tinh-tinh/tinhtinh.
[2] validator.js (string validator reference).

🌐 Web query:

IsAlphanumeric validator Go implementation standard behavior

💡 Result:

Short answer: no — in the common Go validator (go-playground/validator) the plain "alphanum" / IsAlphanumeric validator is ASCII-only; there is a separate "alphanumunicode" validator that accepts Unicode letters/digits. [1][2]

Sources:

  • go-playground validator baked-in validators list showing "alphanum" and "alphanumunicode". [1]
  • go-playground/validator README (validators list / docs). [2]

Critical: IsAlphanumeric validation will reject valid SQL column names with underscores.

Standard IsAlphanumeric validators only accept [a-zA-Z0-9] and will reject snake_case column names like user_id, created_at, first_name — which are the standard SQL naming convention. This will silently fail at runtime (returns q without error), making bugs difficult to debug.

Either expand the validation to allow underscores and other valid SQL identifier characters, or document why this restrictive validation is intentional.

🤖 Prompt for AI Agents 
In builder.go around lines 166 to 168, isValidColumn currently uses
validator.IsAlphanumeric which rejects common SQL identifiers containing
underscores; change the validation to allow underscores and valid identifier
forms (e.g. use a regex like ^[A-Za-z_][A-Za-z0-9_]*$ or a configurable pattern
that permits underscores and digits after the first character), and ensure
callers return an explicit error when a column name fails validation instead of
silently proceeding; update tests/docs accordingly.

236 changes: 236 additions & 0 deletions builder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,239 @@ func Test_QueryBuilder(t *testing.T) {
require.Equal(t, 1, len(docs))
require.Equal(t, "test", docs[0].Name)
}

func Test_IsValidColumn(t *testing.T) {
require.NotPanics(t, func() {
createDatabaseForTest("test_valid_column")
})
dsn := "host=localhost user=postgres password=postgres dbname=test_valid_column port=5432 sslmode=disable TimeZone=Asia/Shanghai"

db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{
Logger: logger.Default.LogMode(logger.Info),
})
require.Nil(t, err)
db.Exec("CREATE EXTENSION IF NOT EXISTS \"uuid-ossp\";")

type TestEntity struct {
gorm.Model
Name string `gorm:"type:varchar(255)"`
Value int `gorm:"type:int"`
}
err = db.AutoMigrate(&TestEntity{})
require.Nil(t, err)

repo := sqlorm.Repository[TestEntity]{DB: db}

// Create test data
count, err := repo.Count(nil)
require.Nil(t, err)

if count == 0 {
_, err = repo.Create(&TestEntity{Name: "valid", Value: 1})
require.Nil(t, err)
}

// Define invalid column names to test
invalidColumns := []string{
"name; DROP TABLE users;--",
"column' OR '1'='1",
"col=1",
"column!",
"col@name",
"col#name",
"col$name",
"col%name",
"col^name",
"col&name",
"col*name",
"column()",
"[column]",
"<column>",
"col/name",
"col\\name",
"col|name",
"col`name",
"col~name",
"col+name",
"col name",
"col-name",
"col.name",
"col:name",
"col;name",
"col'name",
"col\"name",
"col{name}",
}

// Test Equal with invalid columns
t.Run("Equal_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.Equal(invalidCol, "test")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test Not with invalid columns
t.Run("Not_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.Not(invalidCol, "test")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test Or with invalid columns
t.Run("Or_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.Or(invalidCol, "test")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test In with invalid columns
t.Run("In_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.In(invalidCol, "test", "test2")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test MoreThan with invalid columns
t.Run("MoreThan_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.MoreThan(invalidCol, 0)
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test MoreThanOrEqual with invalid columns
t.Run("MoreThanOrEqual_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.MoreThanOrEqual(invalidCol, 0)
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test LessThan with invalid columns
t.Run("LessThan_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.LessThan(invalidCol, 100)
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test LessThanOrEqual with invalid columns
t.Run("LessThanOrEqual_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.LessThanOrEqual(invalidCol, 100)
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test Like with invalid columns
t.Run("Like_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.Like(invalidCol, "%test%")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test ILike with invalid columns
t.Run("ILike_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.ILike(invalidCol, "%TEST%")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test Between with invalid columns
t.Run("Between_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.Between(invalidCol, 0, 100)
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test NotEqual with invalid columns
t.Run("NotEqual_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.NotEqual(invalidCol, "nonexistent")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test NotIn with invalid columns
t.Run("NotIn_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.NotIn(invalidCol, "nonexistent1", "nonexistent2")
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})

// Test IsNull with invalid columns
t.Run("IsNull_InvalidColumn", func(t *testing.T) {
for _, invalidCol := range invalidColumns {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.IsNull(invalidCol)
})
require.Nil(t, err)
require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
}
})
Comment on lines +238 to +390
Copy link

@coderabbitai coderabbitai bot Jan 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Strengthen the test assertions to verify filters are ignored.

The current assertions use require.GreaterOrEqual(t, len(docs), 1, ...), which only confirms at least one result was returned. This doesn't definitively prove that the invalid column filter was ignored—it just shows some data came back.

A more robust approach would be to compare against the total count in the database, proving that no filtering occurred:

♻️ Stronger assertion approach

Before the sub-tests, capture the expected count:

+	// Get total count for validation that filters are ignored
+	totalCount, err := repo.Count(nil)
+	require.Nil(t, err)
+
 	// Test Equal with invalid columns
 	t.Run("Equal_InvalidColumn", func(t *testing.T) {
 		for _, invalidCol := range invalidColumns {
 			docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
 				qb.Equal(invalidCol, "test")
 			})
 			require.Nil(t, err)
-			require.GreaterOrEqual(t, len(docs), 1, "Invalid column %q should not filter results", invalidCol)
+			require.Equal(t, totalCount, int64(len(docs)), "Invalid column %q should not filter results", invalidCol)
 		}
 	})

Apply the same pattern to all other sub-tests.

Committable suggestion skipped: line range outside the PR's diff.


// Test valid column names still work
t.Run("ValidColumn_Equal", func(t *testing.T) {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.Equal("Name", "valid")
})
require.Nil(t, err)
require.Equal(t, 1, len(docs))
require.Equal(t, "valid", docs[0].Name)
})

t.Run("ValidColumn_MoreThan", func(t *testing.T) {
docs, err := repo.FindAll(func(qb *sqlorm.QueryBuilder) {
qb.MoreThan("Value", 0)
})
require.Nil(t, err)
require.Equal(t, 1, len(docs))
})
}