diff --git a/changes/26366-update-sql-parser b/changes/26366-update-sql-parser new file mode 100644 index 0000000000..b57dce129f --- /dev/null +++ b/changes/26366-update-sql-parser @@ -0,0 +1 @@ +- Updated the parser used when editing SQL in the UI to handle modern expressions like window functions diff --git a/frontend/components/forms/validators/validate_query/index.js b/frontend/components/forms/validators/validate_query/index.js index 3468d54bd1..3a8436217d 100644 --- a/frontend/components/forms/validators/validate_query/index.js +++ b/frontend/components/forms/validators/validate_query/index.js @@ -1,12 +1,11 @@ -import sqliteParser from "sqlite-parser"; +import { Parser } from "node-sql-parser"; import { includes, some } from "lodash"; -const BLACKLISTED_ACTIONS = []; -const invalidQueryErrorMessage = "Blacklisted query action"; const invalidQueryResponse = (message) => { return { valid: false, error: message }; }; const validQueryResponse = { valid: true, error: null }; +const parser = new Parser(); export const validateQuery = (queryText) => { if (!queryText) { @@ -14,19 +13,12 @@ export const validateQuery = (queryText) => { } try { - const ast = sqliteParser(queryText); - const { statement } = ast; - const invalidQuery = some(statement, (obj) => { - return includes(BLACKLISTED_ACTIONS, obj.variant.toLowerCase()); - }); - - if (invalidQuery) { - return invalidQueryResponse(invalidQueryErrorMessage); - } - + parser.astify(queryText, { database: "sqlite" }); return validQueryResponse; } catch (error) { - return invalidQueryResponse(error.message); + return invalidQueryResponse( + "There is a syntax error in your query; please resolve in order to save." + ); } }; diff --git a/frontend/components/forms/validators/validate_query/validate_query.tests.js b/frontend/components/forms/validators/validate_query/validate_query.tests.js index 35e3611cee..36368a1791 100644 --- a/frontend/components/forms/validators/validate_query/validate_query.tests.js +++ b/frontend/components/forms/validators/validate_query/validate_query.tests.js @@ -22,7 +22,9 @@ describe("validateQuery", () => { const { error, valid } = validateQuery(query); expect(valid).toEqual(false); - expect(error).toMatch(/Syntax error found near .+/); + expect(error).toMatch( + "There is a syntax error in your query; please resolve in order to save." + ); }); }); diff --git a/frontend/utilities/sql_tools.tests.ts b/frontend/utilities/sql_tools.tests.ts new file mode 100644 index 0000000000..b94ebe5f5a --- /dev/null +++ b/frontend/utilities/sql_tools.tests.ts @@ -0,0 +1,71 @@ +import { checkTable } from "./sql_tools"; + +describe("checkTable", () => { + // from https://github.com/fleetdm/fleet/issues/26366 + const SQL = ` +WITH extension_safety_hub_menu_notifications AS ( + SELECT + parse_json.key, + parse_json.fullkey, + parse_json.path, + parse_json.value + FROM ( + SELECT file.filename, file.path, file.btime FROM file WHERE path LIKE "/Users/%/Library/Application Support/Google/Chrome/%/Preferences" ORDER BY file.btime DESC limit 20 + ) as chrome_preferences + JOIN parse_json ON chrome_preferences.path = parse_json.path + WHERE parse_json.path LIKE "/Users/%/Library/Application Support/Google/Chrome/%/Preferences" AND ( + fullkey IN ("profile/name", "profile/safety_hub_menu_notifications/extensions/isCurrentlyActive", "profile/safety_hub_menu_notifications/extensions/result/timestamp") + OR fullkey Like "profile/safety_hub_menu_notifications/extensions/result/triggeringExtensions%" + ) +), +extension_details AS ( + SELECT path, + CASE WHEN key = 'name' THEN value END AS profile_name, + CASE WHEN key = 'isCurrentlyActive' THEN value END AS notification_active, + CASE WHEN key GLOB '*[0-9]' THEN value END AS triggering_extension, + CASE WHEN key = 'timestamp' THEN value END AS timestamp + FROM extension_safety_hub_menu_notifications + GROUP BY path, profile_name, notification_active, triggering_extension, timestamp +), +problematic_extensions AS ( + SELECT + path, + MAX(profile_name) OVER (PARTITION by path) AS profile_name, + MAX(notification_active) AS notification_active, + MAX(timestamp) AS timestamp, + triggering_extension + FROM extension_details +) +SELECT path, + profile_name, + notification_active, + timestamp, + triggering_extension +FROM problematic_extensions +WHERE triggering_extension IS NOT NULL; +`; + it("should return only real tables by default", () => { + const { tables, error } = checkTable(SQL); + expect(error).toBeNull(); + expect(tables).toEqual(["file", "parse_json"]); + }); + + it("should return all tables if 'includeVirtualTables' is set", () => { + const { tables, error } = checkTable(SQL, true); + expect(error).toBeNull(); + // Note that json_each is _not_ returned here, as it is a function table. + expect(tables).toEqual([ + "file", + "parse_json", + "chrome_preferences", + "extension_safety_hub_menu_notifications", + "extension_details", + "problematic_extensions", + ]); + }); + + it("should return an error if SQL is in valid", () => { + const result = checkTable("SELECTx * FROM users"); + expect(result.error).not.toBeNull(); + }); +}); diff --git a/frontend/utilities/sql_tools.ts b/frontend/utilities/sql_tools.ts index 6fdca47fd6..7a503b3c2f 100644 --- a/frontend/utilities/sql_tools.ts +++ b/frontend/utilities/sql_tools.ts @@ -1,6 +1,6 @@ // @ts-ignore -import sqliteParser from "sqlite-parser"; -import { intersection, isPlainObject } from "lodash"; +import { Parser } from "node-sql-parser"; +import { intersection, isPlainObject, uniq } from "lodash"; import { osqueryTablesAvailable } from "utilities/osquery_tables"; import { MACADMINS_EXTENSION_TABLES, @@ -9,6 +9,8 @@ import { } from "interfaces/platform"; import { TableSchemaPlatform } from "interfaces/osquery_table"; +const parser = new Parser(); + type IAstNode = Record; // TODO: Research if there are any preexisting types for osquery schema @@ -39,17 +41,20 @@ const _isNode = (node: unknown): node is IAstNode => { const _visit = ( abstractSyntaxTree: IAstNode, - callback: (ast: IAstNode) => void + callback: (ast: IAstNode, parentKey: string) => void, + parentKey = "" ) => { if (abstractSyntaxTree) { - callback(abstractSyntaxTree); + callback(abstractSyntaxTree, parentKey); Object.keys(abstractSyntaxTree).forEach((key) => { const childNode = abstractSyntaxTree[key]; if (Array.isArray(childNode)) { - childNode.forEach((grandchildNode) => _visit(grandchildNode, callback)); + childNode.forEach((grandchildNode) => + _visit(grandchildNode, callback, key) + ); } else if (childNode && _isNode(childNode)) { - _visit(childNode, callback); + _visit(childNode, callback, key); } }); } @@ -73,46 +78,82 @@ const filterCompatiblePlatforms = ( export const parseSqlTables = ( sqlString: string, - includeCteTables = false + includeVirtualTables = false ): string[] => { let results: string[] = []; - // Tables defined via common table expression will be excluded from results by default - const cteTables: string[] = []; + // Tables defined via common table expression (WITH ... AS syntax) or as subselects + // will be excluded from results by default. + const virtualTables: string[] = []; - const _callback = (node: IAstNode) => { + // Tables defined via functions like `json_each` will always be excluded from results. + const functionTables: string[] = []; + + const _callback = (node: IAstNode, parentKey: string) => { if (!node) { return; } + // Common Table Expressions (CTEs) using "WITH ... AS" syntax. if ( - (node.variant === "common" || node.variant === "recursive") && - node.format === "table" && - node.type === "expression" + parentKey === "with" && + node.name && + (node.name as IAstNode).type === "default" ) { - const targetName = node.target && (node.target as IAstNode).name; - targetName && - typeof targetName === "string" && - cteTables.push(targetName); + const withTable = node.name as IAstNode; + if (typeof withTable.value === "string") { + virtualTables.push(withTable.value); + } return; } - node.variant === "table" && - // ignore table-valued functions (see, e.g., https://www.sqlite.org/json1.html#jeach) - node.type !== "function" && - node.name && - typeof node.name === "string" && - results.push(node.name); + // Parse tables referenced by FROM or JOIN clauses. + if (parentKey === "from" || parentKey === "left" || parentKey === "right") { + // Subselects and JSON functions. + if (node.expr) { + // Check if the node is a function call. + if ((node.expr as IAstNode).type === "function") { + // Get the function name from node.expr.name.name[0].value + // and push it to functionTables. + const nodeExprName = (node.expr as IAstNode).name as IAstNode; + const nodeExprNameArr = nodeExprName.name as IAstNode[]; + if (nodeExprNameArr.length > 0) { + const functionName = nodeExprNameArr[0].value as string; + if (functionName) { + functionTables.push(functionName); + } + } + return; + } + // Otherwise push it to the set of virtual tables. + virtualTables.push(node.as as string); + return; + } + + // Plain ol' tables. + if (node.table) { + results.push(node.table as string); + } + } }; try { - const sqlTree = sqliteParser(sqlString); - _visit(sqlTree, _callback); + const sqlTree = parser.astify(sqlString, { database: "sqlite" }) as unknown; + _visit(sqlTree as IAstNode, _callback); - if (cteTables.length && !includeCteTables) { - results = results.filter((r: string) => !cteTables.includes(r)); + // Remove virtual tables unless includeVirtualTables is true. + if (virtualTables.length && !includeVirtualTables) { + results = results.filter((r: string) => !virtualTables.includes(r)); } + // Always remove function tables. + if (functionTables.length) { + results = results.filter((r: string) => !functionTables.includes(r)); + } + + // Remove duplicates. + results = uniq(results); + return results; } catch (err) { // console.log(`sqlite-parser error: ${err}\n\n${sqlString}`); @@ -123,11 +164,11 @@ export const parseSqlTables = ( export const checkTable = ( sqlString = "", - includeCteTables = false + includeVirtualTables = false ): { tables: string[] | null; error: Error | null } => { let sqlTables: string[] | undefined; try { - sqlTables = parseSqlTables(sqlString, includeCteTables); + sqlTables = parseSqlTables(sqlString, includeVirtualTables); } catch (err) { return { tables: null, error: new Error(`${err}`) }; } @@ -146,12 +187,12 @@ export const checkTable = ( export const checkPlatformCompatibility = ( sqlString: string, - includeCteTables = false + includeVirtualTables = false ): { platforms: QueryablePlatform[] | null; error: Error | null } => { let sqlTables: string[] | undefined; try { // get tables from str - sqlTables = parseSqlTables(sqlString, includeCteTables); + sqlTables = parseSqlTables(sqlString, includeVirtualTables); } catch (err) { return { platforms: null, error: new Error(`${err}`) }; } diff --git a/package.json b/package.json index e13a6aa8c3..145600a01a 100644 --- a/package.json +++ b/package.json @@ -17,7 +17,7 @@ }, "dependencies": { "@types/dompurify": "3.0.2", - "ace-builds": "1.4.12", + "ace-builds": "1.4.14", "axios": "1.8.3", "content-disposition": "0.5.4", "core-js": "3.25.1", @@ -33,6 +33,7 @@ "js-yaml": "3.14.1", "lodash": "4.17.21", "memoize-one": "5.2.1", + "node-sql-parser": "5.3.8", "normalizr": "3.6.2", "prop-types": "15.8.1", "proxy-middleware": "0.15.0", @@ -56,7 +57,6 @@ "sass": "1.83.4", "select": "1.1.2", "sockjs-client": "1.6.1", - "sqlite-parser": "1.0.1", "use-debounce": "9.0.4", "uuid": "8.3.2", "validator": "13.11.0", diff --git a/webpack.config.js b/webpack.config.js index a6aa4faad0..8786f73cd1 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -140,6 +140,12 @@ const config = { extensions: [".tsx", ".ts", ".js", ".jsx", ".json"], modules: [path.resolve(path.join(repo, "./frontend")), "node_modules"], fallback: { path: require.resolve("path-browserify") }, + alias: { + "node-sql-parser": path.resolve( + __dirname, + "node_modules/node-sql-parser/umd/sqlite.umd.js" + ), + }, }, }; diff --git a/yarn.lock b/yarn.lock index 9f58a345b8..349d2494b2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3677,6 +3677,11 @@ resolved "https://registry.npmjs.org/@types/parse-json/-/parse-json-4.0.0.tgz" integrity sha512-//oorEZjL6sbPcKUaCdIGlIUeH26mgzimjBB77G6XRgnDl/L5wOnpyBGRe/Mmf5CVW3PwEBE1NjiMZ/ssFh4wA== +"@types/pegjs@^0.10.0": + version "0.10.6" + resolved "https://registry.yarnpkg.com/@types/pegjs/-/pegjs-0.10.6.tgz#bc20fc4809fed4cddab8d0dbee0e568803741a82" + integrity sha512-eLYXDbZWXh2uxf+w8sXS8d6KSoXTswfps6fvCUuVAGN8eRpfe7h9eSRydxiSJvo9Bf+GzifsDOr9TMQlmJdmkw== + "@types/prettier@^2.1.5": version "2.7.1" resolved "https://registry.yarnpkg.com/@types/prettier/-/prettier-2.7.1.tgz#dfd20e2dc35f027cdd6c1908e80a5ddc7499670e" @@ -4266,12 +4271,7 @@ abab@^2.0.3, abab@^2.0.5, abab@^2.0.6: resolved "https://registry.yarnpkg.com/abab/-/abab-2.0.6.tgz#41b80f2c871d19686216b82309231cfd3cb3d291" integrity sha512-j2afSsaIENvHZN2B8GOpF566vZ5WVk5opAiMTvWgaQT8DkbOqsTfvNAvHoRGU2zzP8cPoqys+xHTRDWW8L+/BA== -ace-builds@1.4.12: - version "1.4.12" - resolved "https://registry.npmjs.org/ace-builds/-/ace-builds-1.4.12.tgz" - integrity sha512-G+chJctFPiiLGvs3+/Mly3apXTcfgE45dT5yp12BcWZ1kUs+gm0qd3/fv4gsz6fVag4mM0moHVpjHDIgph6Psg== - -ace-builds@^1.4.6: +ace-builds@1.4.14, ace-builds@^1.4.6: version "1.4.14" resolved "https://registry.npmjs.org/ace-builds/-/ace-builds-1.4.14.tgz" integrity sha512-NBOQlm9+7RBqRqZwimpgquaLeTJFayqb9UEPtTkpC3TkkwDnlsT/TwsCC0svjt9kEZ6G9mH5AEOHSz6Q/HrzQQ== @@ -4877,6 +4877,11 @@ big-integer@^1.6.16: resolved "https://registry.npmjs.org/big-integer/-/big-integer-1.6.51.tgz" integrity sha512-GPEid2Y9QU1Exl1rpO9B2IPJGHPSupF5GnVIP0blYvNOMer2bTvSWs1jGOUg04hTmu67nmLsQ9TBo1puaotBHg== +big-integer@^1.6.48: + version "1.6.52" + resolved "https://registry.yarnpkg.com/big-integer/-/big-integer-1.6.52.tgz#60a887f3047614a8e1bffe5d7173490a97dc8c85" + integrity sha512-QxD8cf2eVqJOOz63z6JIN9BzvVs/dlySa5HGSBH5xtR8dPteIRQnBxxKqkNTiT6jbDTF6jAfrd4oMcND9RGbQg== + big.js@^5.2.2: version "5.2.2" resolved "https://registry.yarnpkg.com/big.js/-/big.js-5.2.2.tgz#65f0af382f578bcdc742bd9c281e9cb2d7768328" @@ -10701,6 +10706,14 @@ node-sass-magic-importer@^5.3.3: postcss-scss "^3.0.2" resolve "^1.17.0" +node-sql-parser@5.3.8: + version "5.3.8" + resolved "https://registry.yarnpkg.com/node-sql-parser/-/node-sql-parser-5.3.8.tgz#b71db4cb80dc5302fc77d7b6ca3bcb269b2124bf" + integrity sha512-aqGTzK8kPJAwQ6tqdS0l+vX358LXMmZDw902ePfiPn3PSDsg2HeR2tHFioXNHJW00YHoic6VVYY80waFb/zdxw== + dependencies: + "@types/pegjs" "^0.10.0" + big-integer "^1.6.48" + normalize-path@^3.0.0, normalize-path@~3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/normalize-path/-/normalize-path-3.0.0.tgz#0dcd69ff23a1c9b11fd0978316644a0388216a65" @@ -12531,11 +12544,6 @@ sprintf-js@~1.0.2: resolved "https://registry.yarnpkg.com/sprintf-js/-/sprintf-js-1.0.3.tgz#04e6926f662895354f3dd015203633b857297e2c" integrity sha512-D9cPgkvLlV3t3IzL0D0YLvGA9Ahk4PcvVwUbN0dSGr1aP0Nrt4AEnTUbuGvquEC0mA64Gqt1fzirlRs5ibXx8g== -sqlite-parser@1.0.1: - version "1.0.1" - resolved "https://registry.npmjs.org/sqlite-parser/-/sqlite-parser-1.0.1.tgz" - integrity sha1-EQGD8mgvBKxsfYrQnEREbvl21ew= - stack-utils@^2.0.3: version "2.0.5" resolved "https://registry.yarnpkg.com/stack-utils/-/stack-utils-2.0.5.tgz#d25265fca995154659dbbfba3b49254778d2fdd5"