Skip to content

Commit 02c5026

Browse files
authored
Allow ESCAPE in LIKE clauses to be valid SQL (#31222)
for #30109 # Details This PR fixes an issue in our current SQL parsing library that was causing queries like this to be marked invalid: ``` SELECT * FROM table_name WHERE column_name LIKE '\_%' ESCAPE '\' ``` This is valid in SQLite because the `\` is not considered an escape character by default. From [the SQLite docs](https://www.sqlite.org/lang_expr.html) (see section 3 "Literal Values (Constants)"; emphasis mine): > A string constant is formed by enclosing the string in single quotes ('). A single quote within the string can be encoded by putting two single quotes in a row - as in Pascal. C-style escapes using the backslash character are not supported because they are not standard SQL. # Use of forked code Part of the fix for this was [submitted as a PR to the node-sql-parser library](taozhi8833998/node-sql-parser#2496) we now use, and merged. I then found that another fix was needed, which I submitted as [a separate PR](taozhi8833998/node-sql-parser#2512). As these fixes have yet to be made part of an official release of the library, I made a fork off of the release we were using (5.3.10) and bundled the necessary build artifacts with Fleet. We have an [ADR proposing the use of submodules for this purpose](#31079); I'm happy to implement that instead if we approve that, although for a front-end module with a build step it's a bit more complicated. Hopefully this code will be released in `node-sql-parser` soon and we can revert back to using the dependency. Here is the [full set of changes](taozhi8833998/node-sql-parser@master...sgress454:node-sql-parser:5.3.10-plus). # Checklist for submitter - [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. - [X] Manual QA for all new/changed functionality
1 parent f4814f6 commit 02c5026

File tree

12 files changed

+23
-22
lines changed

12 files changed

+23
-22
lines changed

.eslintignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
frontend/utilities/node-sql-parser

.prettierignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ website/
3434
# certain frontend files that are not meant to be formatted
3535
frontend/components/FleetAce/mode.ts
3636
frontend/components/FleetAce/theme.ts
37+
frontend/utilities/node-sql-parser
3738

3839
# github workflow yaml, which may contain shell scripts that shouldn't be formatted
3940
.github/workflows/*

changes/30109-fix-sql-like-clause

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fixed an issue where using ESCAPE in a LIKE clause caused SQL validation to fail

frontend/components/forms/validators/validate_query/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Parser } from "node-sql-parser";
1+
import { Parser } from "utilities/node-sql-parser/sqlite";
22
import { includes, some } from "lodash";
33

44
const invalidQueryResponse = (message) => {
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# About
2+
3+
This is compiled from a [fork](https://github.com/taozhi8833998/node-sql-parser/compare/master...sgress454:node-sql-parser:5.3.10-plus) of the 5.3.10 release of [node-sql-parser library](https://github.com/sgress454/node-sql-parser/tree/sgress454/add-escape-to-sqlite-like) created to fix issue [#30109](https://github.com/fleetdm/fleet/issues/30109).
4+
5+
Once a new release of node-sql-parser comes out with this code in it, we can revert back to using `node-sql-parser` as a dependency in Fleet.
6+
7+
# To compile
8+
9+
1. Check out https://github.com/sgress454/node-sql-parser/tree/5.3.10-plus
10+
2. `npm install`
11+
3. `npm run build`
12+
4. `cp output/prod/build/sqlite.* /path/to/fleet/frontend/utilities/node-sql-parser`
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from '../types';

frontend/utilities/node-sql-parser/sqlite.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/utilities/node-sql-parser/sqlite.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/utilities/sql_tools.tests.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { checkTable } from "./sql_tools";
22

33
describe("checkTable", () => {
44
// from https://github.com/fleetdm/fleet/issues/26366
5+
// and https://github.com/fleetdm/fleet/issues/30109
56
const SQL = `
67
WITH extension_safety_hub_menu_notifications AS (
78
SELECT
@@ -42,7 +43,7 @@ SELECT path,
4243
timestamp,
4344
triggering_extension
4445
FROM problematic_extensions
45-
WHERE triggering_extension IS NOT NULL;
46+
WHERE triggering_extension IS NOT NULL AND username NOT LIKE '\\_%' ESCAPE '\\';
4647
`;
4748
it("should return only real tables by default", () => {
4849
const { tables, error } = checkTable(SQL);

frontend/utilities/sql_tools.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// @ts-ignore
2-
import { Parser } from "node-sql-parser";
2+
import { Parser } from "utilities/node-sql-parser/sqlite";
33
import { intersection, isPlainObject, uniq } from "lodash";
44
import { osqueryTablesAvailable } from "utilities/osquery_tables";
55
import {

0 commit comments

Comments
 (0)