Summary
resolveSortColumn was introduced in PR #299 to safely validate user-supplied sortBy column names against the database schema and the query AST before injecting them into an ORDER BY clause. However, executeSelectQuery never calls it - the function is dead exported code.
Affected file
src/services/node.logic.service.ts
Detail
The function validates a sortBy string in three ways:
- Regex check: must match
^[A-Za-z_][A-Za-z0-9_]*$
- AST check: the column must appear in the query's ORDER BY clause
- DB schema check: the column must exist in
information_schema.columns for at least one of the query's base tables
All three checks are sound. The problem is that executeSelectQuery does not accept a sortBy parameter and never passes one through to resolveSortColumn.
Risk
Low immediate risk - the function is unreachable. However:
- Dead exported code can mislead a future contributor into thinking
sortBy injection is already guarded.
- If someone adds a
sortBy parameter to executeSelectQuery without remembering to call resolveSortColumn, they may inject an unvalidated column name into an ORDER BY clause.
Recommended action
Choose one of:
- Wire it up: Add
sortBy?: string to the executeSelectQuery input, call resolveSortColumn, and inject a validated ORDER BY into innerQuery before the outer LIMIT wrapper.
- Remove it: If
sortBy injection is not planned for this service, delete the function to avoid confusion. Open a follow-up issue if it is wanted.
- Document the intent: If this is scaffolding for a future PR, add a
// TODO(#<issue>): comment referencing the tracking issue.
Related
Summary
resolveSortColumnwas introduced in PR #299 to safely validate user-suppliedsortBycolumn names against the database schema and the query AST before injecting them into an ORDER BY clause. However,executeSelectQuerynever calls it - the function is dead exported code.Affected file
src/services/node.logic.service.tsDetail
The function validates a
sortBystring in three ways:^[A-Za-z_][A-Za-z0-9_]*$information_schema.columnsfor at least one of the query's base tablesAll three checks are sound. The problem is that
executeSelectQuerydoes not accept asortByparameter and never passes one through toresolveSortColumn.Risk
Low immediate risk - the function is unreachable. However:
sortByinjection is already guarded.sortByparameter toexecuteSelectQuerywithout remembering to callresolveSortColumn, they may inject an unvalidated column name into an ORDER BY clause.Recommended action
Choose one of:
sortBy?: stringto theexecuteSelectQueryinput, callresolveSortColumn, and inject a validatedORDER BYintoinnerQuerybefore the outer LIMIT wrapper.sortByinjection is not planned for this service, delete the function to avoid confusion. Open a follow-up issue if it is wanted.// TODO(#<issue>):comment referencing the tracking issue.Related