From efe266b026bbca9b1be3c37f31fd4ae4ebaa50ce Mon Sep 17 00:00:00 2001 From: Scott Gress Date: Wed, 28 Jan 2026 16:08:33 -0600 Subject: [PATCH] Use forked node-sql-parser, fix CTE issues in parsed SQL (#38744) **Related issue:** Resolves #34635 # Details This PR switches us to a [fork of node-sql-parser](https://github.com/sgress454/node-sql-parser) that I'm maintaining to fast-track fixes to the SQLite implementation. The first published version of the fork is 5.4.0-fork.1 (forked from v5.4.0 of the upstream), and includes fixes for #34635 and #30109 that haven't made it to the upstream yet. Fixes in 5.4.0-fork.1: * https://github.com/sgress454/node-sql-parser/pull/7 * https://github.com/sgress454/node-sql-parser/pull/5 * https://github.com/sgress454/node-sql-parser/pull/4 # Checklist for submitter If some of the following don't apply, delete the relevant line. - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://github.com/fleetdm/fleet/blob/main/docs/Contributing/guides/committing-changes.md#changes-files) for more information. ## Testing - [X] Added/updated automated tests - Granular tests are added [in the package itself](https://github.com/sgress454/node-sql-parser/blob/5.4.0-fork.1/test/sqlite.spec.js), and new regression tests for the Fleet issues are added in the Fleet. - [X] QA'd all new/changed functionality manually - Pasted the offending queries into the editor and saw no syntax errors --- .vscode/launch.json | 4 +- changes/34635-fix-cte-syntax-for-frontend | 1 + .../forms/validators/validate_query/index.ts | 3 +- .../validate_query/validate_query.tests.ts | 70 +++++++++++++++++++ frontend/test/jest.config.js | 2 + frontend/utilities/sql_tools.ts | 3 +- package.json | 1 + tsconfig.json | 5 +- webpack.config.js | 2 +- yarn.lock | 18 +++++ 10 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 changes/34635-fix-cte-syntax-for-frontend diff --git a/.vscode/launch.json b/.vscode/launch.json index ed3343bd9a..abc79b0d02 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -109,7 +109,7 @@ "program": "${workspaceRoot}/node_modules/.bin/jest", "args": [ "--config", - "./frontend/test/jest.config.ts", + "./frontend/test/jest.config.js", "${relativeFile}" ], "console": "integratedTerminal", @@ -122,7 +122,7 @@ "program": "${workspaceRoot}/node_modules/.bin/jest", "args": [ "--config", - "./frontend/test/jest.config.ts" + "./frontend/test/jest.config.js" ], "console": "integratedTerminal", "internalConsoleOptions": "neverOpen" diff --git a/changes/34635-fix-cte-syntax-for-frontend b/changes/34635-fix-cte-syntax-for-frontend new file mode 100644 index 0000000000..36275a04b1 --- /dev/null +++ b/changes/34635-fix-cte-syntax-for-frontend @@ -0,0 +1 @@ +- Fixed an issue where queries with common table expressions (CTEs) were marked as having invalid syntax diff --git a/frontend/components/forms/validators/validate_query/index.ts b/frontend/components/forms/validators/validate_query/index.ts index 3e1228571a..110ae9cef6 100644 --- a/frontend/components/forms/validators/validate_query/index.ts +++ b/frontend/components/forms/validators/validate_query/index.ts @@ -1,5 +1,4 @@ -// @ts-ignore -import { Parser } from "utilities/node-sql-parser/sqlite"; +import { Parser } from "node-sql-parser"; export const EMPTY_QUERY_ERR = "Query text must be present"; export const INVALID_SYNTAX_ERR = "Syntax error. Please review before saving."; diff --git a/frontend/components/forms/validators/validate_query/validate_query.tests.ts b/frontend/components/forms/validators/validate_query/validate_query.tests.ts index 46fedab69b..a051e8064f 100644 --- a/frontend/components/forms/validators/validate_query/validate_query.tests.ts +++ b/frontend/components/forms/validators/validate_query/validate_query.tests.ts @@ -43,3 +43,73 @@ describe("validateQuery", () => { }); }); }); + +describe("node-sql-parser integration", () => { + it("#30109 - allow custom escape characters in LIKE clauses", () => { + const query = ` +WITH localusers AS ( + SELECT username, directory || '/.gitconfig' AS gc_path + FROM users + WHERE + shell != '/usr/bin/false' + AND username NOT LIKE '\\_%' ESCAPE '\\' + AND username NOT IN ('root', 'person1', 'person2', 'SYSTEM', 'LOCAL SERVICE', 'NETWORK SERVICE') + AND directory != '' +) +SELECT username, value AS git_signingkey_path +FROM parse_ini +LEFT JOIN localusers ON parse_ini.path=localusers.gc_path +WHERE path IN (SELECT gc_path FROM localusers) AND fullkey = 'user/signingkey'; +`; + const { error, valid } = validateQuery(query); + expect(valid).toEqual(true); + expect(error).toBeFalsy(); + }); + + it("#34635 - allow VALUES in CTEs, and table names in IN clauses", () => { + const query = ` +-- Step 1: Define config file path suffixes for each supported application +WITH path_suffixes(path) AS ( + VALUES + ('/.cursor/mcp.json'), -- Cursor, macOS/Linux/Windows + ('/Library/Application Support/Claude/claude_desktop_config.json'), -- Claude Desktop, macOS + ('\\AppData\\Roaming\\Claude\\claude_desktop_config.json'), -- Claude Desktop, Windows + ('/.claude.json'), -- Claude Code, macOS/Linux + ('/Library/Application Support/Code/User/mcp.json'), -- VSCode, macOS + ('/.config/Code/User/mcp.json'), -- VSCode, Linux + ('\\AppData\\Roaming\\Code\\User\\mcp.json'), -- VSCode, Windows + ('/.codeium/windsurf/mcp_config.json'), -- Windsurf, macOS + ('/.gemini/settings.json'), -- Gemini CLI, macOS/Linux/Windows + ('/.lmstudio/mcp.json') -- LMStudio, macOS/Linux/Windows +), +-- Step 2: Build full file paths by combining each user's home directory with the path suffixes +full_paths AS ( + SELECT directory || path AS full_path + FROM users + JOIN path_suffixes +), +-- Step 3: Read config files that exist and concatenate their lines into complete JSON strings +config_files AS ( + SELECT path, group_concat(line, '') AS contents + FROM file_lines + WHERE path IN full_paths + GROUP BY path +) +-- Step 4: Parse JSON and extract each MCP server configuration +SELECT + config_files.path, + key AS name, + value AS mcp_config +FROM config_files +JOIN json_each( + COALESCE( + config_files.contents->'$.mcpServers', + config_files.contents->'$.servers' + ) -- Most configs use 'mcpServers' key, but some use 'servers' key +) +`; + const { error, valid } = validateQuery(query); + expect(valid).toEqual(true); + expect(error).toBeFalsy(); + }); +}); diff --git a/frontend/test/jest.config.js b/frontend/test/jest.config.js index 29896404b4..9c526e615c 100644 --- a/frontend/test/jest.config.js +++ b/frontend/test/jest.config.js @@ -36,6 +36,8 @@ const config = { "/frontend/__mocks__/fileMock.js", "\\.(sh|ps1)$": "/frontend/__mocks__/fileMock.js", "\\.(css|scss|sass)$": "identity-obj-proxy", + "^node-sql-parser$": + "/node_modules/@sgress454/node-sql-parser/umd/sqlite.umd.js", }, testMatch: ["**/*tests.[jt]s?(x)"], setupFilesAfterEnv: ["/frontend/test/test-setup.ts"], diff --git a/frontend/utilities/sql_tools.ts b/frontend/utilities/sql_tools.ts index b484f19c3e..58ae505a97 100644 --- a/frontend/utilities/sql_tools.ts +++ b/frontend/utilities/sql_tools.ts @@ -1,5 +1,4 @@ -// @ts-ignore -import { Parser } from "utilities/node-sql-parser/sqlite"; +import { Parser } from "node-sql-parser"; import { intersection, isPlainObject, uniq } from "lodash"; import { osqueryTablesAvailable } from "utilities/osquery_tables"; import { diff --git a/package.json b/package.json index efa58f3e19..4cab61257a 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "build-storybook": "storybook build" }, "dependencies": { + "@sgress454/node-sql-parser": "5.4.0-fork.1", "@types/dompurify": "3.0.2", "ace-builds": "1.4.14", "axios": "1.12.0", diff --git a/tsconfig.json b/tsconfig.json index 6139b4c325..e359b24f38 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -7,7 +7,10 @@ "jsx": "react", "allowSyntheticDefaultImports": true, "resolveJsonModule": true, - "lib": ["ES2021.String", "ES2020.Promise"] + "lib": ["ES2021.String", "ES2020.Promise"], + "paths": { + "node-sql-parser": ["../node_modules/@sgress454/node-sql-parser"] + } }, "include": ["./frontend/**/*"], "exclude": ["node_modules"], diff --git a/webpack.config.js b/webpack.config.js index aefafa0929..2b57803156 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -138,7 +138,7 @@ const config = { alias: { "node-sql-parser": path.resolve( __dirname, - "node_modules/node-sql-parser/umd/sqlite.umd.js" + "node_modules/@sgress454/node-sql-parser/umd/sqlite.umd.js" ), }, }, diff --git a/yarn.lock b/yarn.lock index 8823de03c5..0790c4191a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2917,6 +2917,14 @@ "@parcel/watcher-win32-ia32" "2.5.0" "@parcel/watcher-win32-x64" "2.5.0" +"@sgress454/node-sql-parser@5.4.0-fork.1": + version "5.4.0-fork.1" + resolved "https://registry.yarnpkg.com/@sgress454/node-sql-parser/-/node-sql-parser-5.4.0-fork.1.tgz#b8d33d7d5e571d4e278bfaeb9d0689c3a6760c1f" + integrity sha512-c8lAK4EYKYpEWkYeFbEpZP2LC0j4QmC8LX86tCV+ogPo8dsbYWW56VNQRZp7d0XvVbvtfo21rooy+wCI/YoGmg== + dependencies: + "@types/pegjs" "^0.10.0" + big-integer "^1.6.48" + "@sideway/address@^4.1.5": version "4.1.5" resolved "https://registry.yarnpkg.com/@sideway/address/-/address-4.1.5.tgz#4bc149a0076623ced99ca8208ba780d65a99b9d5" @@ -3774,6 +3782,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" @@ -4919,6 +4932,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"