fix(query): handle comments and improve single selection

close #3054

Author: farnabaz

src/runtime/internal/query.ts

@@ -137,7 +137,7 @@ export const collectionQueryBuilder = <T extends keyof Collections>(collection:
   function buildQuery(opts: { count?: { field: string, distinct: boolean }, limit?: number } = {}) {
     let query = 'SELECT '
     if (opts?.count) {
-      query += `COUNT(${opts.count.distinct ? 'DISTINCT' : ''} ${opts.count.field}) as count`
+      query += `COUNT(${opts.count.distinct ? 'DISTINCT ' : ''}${opts.count.field}) as count`
     }
     else {
       const fields = Array.from(new Set(params.selectedFields))

src/runtime/internal/security.ts

@@ -1,4 +1,7 @@
 const SQL_COMMANDS = /SELECT|INSERT|UPDATE|DELETE|DROP|ALTER/i
+const SQL_CLEANUN_REGEX = /(['"`])(?:\\.|[^\\])*?\1|\/\*[\s\S]*?\*\//g
+const SQL_COUNT_REGEX = /COUNT\((DISTINCT )?[a-z_]\w+\)/i
+const SQL_SELECT_REGEX = /^SELECT (.*) FROM (\w+)( WHERE .*)? ORDER BY (["\w,\s]+) (ASC|DESC)( LIMIT \d+)?( OFFSET \d+)?$/
 
 /**
  * Assert that the query is safe
@@ -10,7 +13,7 @@ const SQL_COMMANDS = /SELECT|INSERT|UPDATE|DELETE|DROP|ALTER/i
  * @returns True if the query is safe, false otherwise
  */
 export function assertSafeQuery(sql: string, collection: string) {
-  const match = sql.match(/^SELECT (.*) FROM (\w+)( WHERE .*)? ORDER BY (["\w,\s]+) (ASC|DESC)( LIMIT \d+)?( OFFSET \d+)?$/)
+  const match = sql.match(SQL_SELECT_REGEX)
   if (!match) {
     throw new Error('Invalid query')
   }
@@ -22,8 +25,8 @@ export function assertSafeQuery(sql: string, collection: string) {
   if (columns.length === 1) {
     if (
       columns[0] !== '*'
-      && !columns[0].startsWith('COUNT(')
-      && !columns[0].match(/^COUNT\((DISTINCT )?[a-z_]\w+\) as count$/)
+      && !columns[0].match(SQL_COUNT_REGEX)
+      && !columns[0].match(/^"[a-z_]\w+"$/)
     ) {
       throw new Error('Invalid query')
     }
@@ -42,7 +45,7 @@ export function assertSafeQuery(sql: string, collection: string) {
     if (!where.startsWith(' WHERE (') || !where.endsWith(')')) {
       throw new Error('Invalid query')
     }
-    const noString = where?.replace(/(['"`])(?:\\.|[^\\])*?\1/g, '')
+    const noString = where?.replace(SQL_CLEANUN_REGEX, '')
     if (noString.match(SQL_COMMANDS)) {
       throw new Error('Invalid query')
     }

test/unit/assertSafeQuery.test.ts

@@ -34,6 +34,10 @@ describe('decompressSQLDump', () => {
     'SELECT * FROM _content_test WHERE id = 1 ORDER BY id DESC LIMIT 10 OFFSET 10': false,
     'SELECT * FROM _content_test WHERE (id = 1) ORDER BY id DESC LIMIT 10 OFFSET 10': true,
     'SELECT * FROM _content_test WHERE (id = \'");\'); select * from ((SELECT * FROM sqlite_master where 1 <> "") as t) ORDER BY type DESC': false,
+    'SELECT "body" FROM _content_test ORDER BY body ASC': true,
+    // Advanced
+    'SELECT COUNT(*) UNION SELECT name /**/FROM sqlite_master-- FROM _content_test WHERE (1=1) ORDER BY id ASC': false,
+    'SELECT * FROM _content_test WHERE (id /*\'*/IN (SELECT id FROM _content_test) /*\'*/) ORDER BY id ASC': false,
   }
 
   Object.entries(queries).forEach(([query, isValid]) => {
@@ -47,7 +51,7 @@ describe('decompressSQLDump', () => {
     })
   })
 
-  it('throws error if query is not valid', async () => {
+  it('all queries should be valid', async () => {
     await collectionQueryBuilder(mockCollection, mockFetch).all()
     expect(() => assertSafeQuery(mockFetch.mock.lastCall![1], mockCollection)).not.toThrow()