fix: validate column before query #87
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThe changes add runtime validation of SQL column names to the QueryBuilder by introducing a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.5.0)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
builder.go (1)
54-64: Move validation before building placeholders.The validation check happens after building the
placeholdersarray (lines 55-58). If the column is invalid, this work is wasted. Move the validation to the beginning of the method for consistency with other methods likeEqual,Not, andOr.The same issue exists in
NotIn(lines 139-150).🔎 Proposed fix
func (q *QueryBuilder) In(column string, values ...interface{}) *QueryBuilder { + if !isValidColumn(column) { + return q + } placeholders := make([]string, len(values)) 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 }
| if !isValidColumn(column) { | ||
| return q | ||
| } |
There was a problem hiding this comment.
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:
- Return an error alongside the QueryBuilder
- Log a warning when validation fails
- Panic if invalid columns indicate a programming error
This comment applies to all 14 methods with validation guards.
| func isValidColumn(column string) bool { | ||
| return validator.IsAlphanumeric(column) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the validator package and IsAlphanumeric implementation
rg -nP 'IsAlphanumeric' --type=go -B 2 -A 5Repository: 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 5Repository: 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=goRepository: 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 2Repository: 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 dRepository: 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 -iRepository: 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 tinhtinhRepository: 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.
No description provided.