cvburgess/SQLDataSource

Knex QueryBuilder toString is unreliable for producing cache keys for PG.

willrust opened this issue · 1 comments

I believe this is related to knex issue 3553, but wanted to surface it here because of a potential security issue that could result.

For PG, query.toString() will omit bigint values, yet the actual query can succeed.

Here's an example of the resulting string where a bigint id is used:
select "id", "username" from "users" where "id" = '' limit 1
Notice that id = ''. So in this case, the first user queried will be returned for subsequent queries as the cache key is always the same.

Also, the Knex docs suggest that toString() should be used for debugging, but we're using it for something quite important.

Great callout @willrust - when I created this utility for my team we used UUIDs for IDs across our app, never integers of any kind.

If you can suggest a better path forward I would gladly welcome a PR.

Personally, for security, I implement RLS so the only records selected belong to the user currently authenticated. I don't have a great idea for this particular scenario.