fix(rest): exempt developer-supplied search columns from isSafeColumn
What & why
isSafeColumn (added in v1.0.15/16 to stop user-derived column identifiers from reaching SQL via filter/sort keys) was also applied inside search() on both the GORM and Mongo backends. Search columns, however, are developer-supplied via SetParams(params, searchColumns...) — never request-derived. Only the ?search= value is user input, and it is parameter-bound (GORM) / regex-escaped (Mongo).
Result: the gate silently dropped legitimate search columns and broke search across consumers:
-
GORM — concat expressions:
first_name || ' ' || last_name,CONCAT_WS(' ', first_name, last_name)→ full-name search returned empty. -
Mongo — nested field paths:
profile.address.city→ nested-field search returned empty.
This breaks full-name search in service-v2 today on the v1.0.16 line (driver list, job list incl. plate number, notification-receiver, empty-towing). Every other consumer hits it the moment it bumps to ≥ v1.0.15.
Change
- Remove
isSafeColumnfromsearch()on GORM (rest/gorm.go) and Mongo (rest/mongo.go).filter()/sort()keep the gate — their keys are genuinely request-derived. - Document the
searchColumnstrust boundary onIPagination.SetParams(rest/paginate.go). - Add a regression test for the GORM search path (
rest/gorm_test.go) using the pure-Goglebarez/sqlitedriver (no CGO). Verified it fails with the gate present, passes without. Suite is green withCGO_ENABLED=0.
Security
No injection reopened. isSafeColumn defends against user-controlled column identifiers; only filter() / sort() interpolate request-derived names, and they remain gated. search() columns are developer literals; the user value stays bound (GORM) / regex-escaped (Mongo). Data flow traced and validated by two independent reviewers.
Follow-ups
- Consumers bump to the resulting release (v1.0.17); service-v2 develop/uat first (heals the 4 endpoints above).
- Pre-existing, out of scope: LIKE
%/_not escaped in search values. - Optional: examples still use
gorm.io/driver/sqlite(CGO); could migrate toglebarez/sqliteto drop the mattn driver entirely.