From d411f04975699343d078b8b6b9c849a6817c3277 Mon Sep 17 00:00:00 2001 From: onenewcode Date: Mon, 31 Aug 2026 11:48:26 -0400 Subject: [PATCH] fix(elasticsearch): treat rest reads as read-only --- .../sql/sqlRisk.elasticsearch.spec.ts | 67 +++++++++++ .../__tests__/readOnlyWriteAccess.spec.ts | 14 +++ .../elasticsearch/elasticsearchRequestRisk.ts | 110 ++++++++++++++++++ apps/desktop/src/lib/sql/sqlRisk.ts | 28 ++++- 4 files changed, 218 insertions(+), 1 deletion(-) create mode 100644 apps/desktop/src/lib/__tests__/sql/sqlRisk.elasticsearch.spec.ts create mode 100644 apps/desktop/src/lib/elasticsearch/elasticsearchRequestRisk.ts diff --git a/apps/desktop/src/lib/__tests__/sql/sqlRisk.elasticsearch.spec.ts b/apps/desktop/src/lib/__tests__/sql/sqlRisk.elasticsearch.spec.ts new file mode 100644 index 0000000000..723a3d0bc4 --- /dev/null +++ b/apps/desktop/src/lib/__tests__/sql/sqlRisk.elasticsearch.spec.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { classifySqlRisk, classifySqlStatementRisk } from "@/lib/sql/sqlRisk"; +import { sqlLooksLikeMutation } from "@/lib/database/readOnlyWriteAccess"; + +const SEARCH_REQUEST = 'POST /demo/_search\n{\n "from": 0,\n "size": 100,\n "sort": [\n "_doc"\n ]\n}'; + +describe("Elasticsearch request risk", () => { + it("treats searches and other retrieval requests as reads", () => { + for (const dialect of ["elasticsearch", "easysearch"] as const) { + expect(classifySqlRisk(SEARCH_REQUEST, { dialect }).risk).toBe("read"); + expect(classifySqlRisk("GET /demo/_doc/1", { dialect }).risk).toBe("read"); + expect(classifySqlRisk("GET /_cat/indices?format=json", { dialect }).risk).toBe("read"); + expect(classifySqlRisk("HEAD /demo", { dialect }).risk).toBe("read"); + expect(classifySqlRisk('POST /demo/_count\n{"query":{"match_all":{}}}', { dialect }).risk).toBe("read"); + expect(classifySqlRisk('POST /_sql\n{"query":"SELECT * FROM demo"}', { dialect }).risk).toBe("read"); + } + }); + + it("treats document changes as writes and index changes as DDL", () => { + expect(classifySqlRisk('POST /demo/_doc\n{"a":1}', { dialect: "elasticsearch" }).risk).toBe("write"); + expect(classifySqlRisk('PUT /demo/_doc/1\n{"a":1}', { dialect: "elasticsearch" }).risk).toBe("write"); + expect(classifySqlRisk("DELETE /demo/_doc/1", { dialect: "elasticsearch" }).risk).toBe("write"); + expect(classifySqlRisk("DELETE /demo", { dialect: "elasticsearch" }).risk).toBe("ddl"); + expect(classifySqlRisk('PUT /demo\n{"settings":{}}', { dialect: "elasticsearch" }).risk).toBe("ddl"); + expect(classifySqlRisk('POST /demo/_delete_by_query\n{"query":{"match_all":{}}}', { dialect: "elasticsearch" }).risk).toBe("ddl"); + }); + + it("reports the highest risk across every request in the editor text", () => { + const source = `${SEARCH_REQUEST}\n\nDELETE /demo`; + expect(classifySqlRisk(source, { dialect: "elasticsearch" }).risk).toBe("ddl"); + expect(classifySqlStatementRisk(SEARCH_REQUEST, { dialect: "elasticsearch" }).risk).toBe("read"); + }); + + it("reads the request line through query strings, trailing slashes and CRLF endings", () => { + expect(classifySqlRisk("GET /demo/_search?size=1", { dialect: "elasticsearch" }).risk).toBe("read"); + expect(classifySqlRisk("post /demo/_search/\r\n{}", { dialect: "elasticsearch" }).risk).toBe("read"); + expect(classifySqlRisk("DELETE /demo/_doc/1?refresh=true", { dialect: "elasticsearch" }).risk).toBe("write"); + expect(classifySqlRisk(" GET /demo/_search // inline comment", { dialect: "elasticsearch" }).risk).toBe("read"); + }); + + it("ignores commented-out requests", () => { + expect(classifySqlRisk(`# DELETE /demo\n// DELETE /demo\n/* DELETE /demo */\n${SEARCH_REQUEST}`, { dialect: "elasticsearch" }).risk).toBe("read"); + expect(classifySqlRisk("/*\nDELETE /demo\n*/\nGET /demo/_search", { dialect: "elasticsearch" }).risk).toBe("read"); + }); + + it("still classifies Elasticsearch SQL through the SQL rules", () => { + expect(classifySqlRisk("SELECT * FROM demo", { dialect: "elasticsearch" }).risk).toBe("read"); + expect(classifySqlRisk("DROP TABLE demo", { dialect: "elasticsearch" }).risk).toBe("ddl"); + }); + + it("leaves text that does not start with a request line to the SQL rules", () => { + // The backend anchors on the first line too, so a stray request line further + // down must not turn unparseable text into a read. + expect(classifySqlRisk('{"query":{"match_all":{}}}\nGET /demo/_search', { dialect: "elasticsearch" }).risk).toBe("unknown"); + }); + + it("keeps REST requests classified as SQL for other database types", () => { + expect(classifySqlRisk(SEARCH_REQUEST, { dialect: "mysql" }).risk).toBe("unknown"); + }); + + it("does not ask a read-only connection to unlock writes for a search", () => { + expect(sqlLooksLikeMutation(SEARCH_REQUEST, "elasticsearch")).toBe(false); + expect(sqlLooksLikeMutation("GET /demo/_search", "easysearch")).toBe(false); + expect(sqlLooksLikeMutation("DELETE /demo", "elasticsearch")).toBe(true); + expect(sqlLooksLikeMutation('POST /demo/_doc\n{"a":1}', "elasticsearch")).toBe(true); + }); +}); diff --git a/apps/desktop/src/lib/database/__tests__/readOnlyWriteAccess.spec.ts b/apps/desktop/src/lib/database/__tests__/readOnlyWriteAccess.spec.ts index e9c518dd58..45c4291fcf 100644 --- a/apps/desktop/src/lib/database/__tests__/readOnlyWriteAccess.spec.ts +++ b/apps/desktop/src/lib/database/__tests__/readOnlyWriteAccess.spec.ts @@ -94,6 +94,20 @@ describe("read-only write unlock", () => { expect(connectionWriteUnlockState).not.toHaveBeenCalled(); }); + it("does not prompt for Elasticsearch searches on a read-only connection", async () => { + const store = useReadOnlyUnlockStore(); + const connection = { id: "es", name: "es-prod", read_only: true, db_type: "elasticsearch" as const }; + + await expect(ensureReadOnlyWriteAccess({ connection, sql: 'POST /demo/_search\n{\n "size": 100\n}' })).resolves.toBe(true); + await expect(ensureReadOnlyWriteAccess({ connection, sql: "GET /demo/_doc/1" })).resolves.toBe(true); + expect(store.pending).toBeUndefined(); + + const pending = ensureReadOnlyWriteAccess({ connection, sql: "DELETE /demo" }); + await waitForPending("es"); + store.cancel(); + await expect(pending).resolves.toBe(false); + }); + it("reuses an existing backend window without prompting again", async () => { connectionWriteUnlockState.mockResolvedValue(45_000); const connection = { id: "prod", name: "prod-db", read_only: true, db_type: "mysql" as const }; diff --git a/apps/desktop/src/lib/elasticsearch/elasticsearchRequestRisk.ts b/apps/desktop/src/lib/elasticsearch/elasticsearchRequestRisk.ts new file mode 100644 index 0000000000..0d0fb16b41 --- /dev/null +++ b/apps/desktop/src/lib/elasticsearch/elasticsearchRequestRisk.ts @@ -0,0 +1,110 @@ +/** + * Read/write classification for Elasticsearch-compatible REST requests. + * + * Mirrors `classify_search_engine_query_risk` in + * `crates/dbx-core/src/query_execution_sql.rs` so the desktop guards (read-only + * unlock, production safety) agree with the backend read-only gate instead of + * treating every `GET`/`POST` request as an unrecognized — and therefore + * unsafe — statement. + */ + +export type ElasticsearchRequestRisk = "read" | "write" | "dangerous"; + +const REQUEST_LINE = /^(GET|POST|PUT|PATCH|DELETE|HEAD|OPTIONS)\s+(\S+)/i; +const READ_ONLY_POST_ENDPOINTS = ["_search", "_count", "_sql", "_msearch", "_field_caps", "_terms_enum", "_validate", "_explain", "_rank_eval", "_search_shards"]; +const WRITE_POST_ENDPOINTS = ["_doc", "_create", "_update", "_bulk"]; +const RISK_ORDER: Record = { read: 0, write: 1, dangerous: 2 }; + +interface ParsedRequestLine { + method: string; + segments: string[]; +} + +function parseRequestLine(line: string): ParsedRequestLine | null { + const match = line.trim().match(REQUEST_LINE); + if (!match) return null; + const path = (match[2].split("?", 1)[0] ?? "").replace(/\/+$/, ""); + return { method: match[1].toUpperCase(), segments: path.split("/").filter(Boolean) }; +} + +function classifyParsedRequest({ method, segments }: ParsedRequestLine): ElasticsearchRequestRisk { + const hasSegment = (candidate: string) => segments.some((segment) => segment.toLowerCase() === candidate); + const hasDocumentId = (candidate: string) => { + const index = segments.findIndex((segment) => segment.toLowerCase() === candidate); + return index >= 0 && !!segments[index + 1]; + }; + + if (method === "GET" || method === "HEAD" || method === "OPTIONS") return "read"; + if (method === "POST") { + if (READ_ONLY_POST_ENDPOINTS.some((endpoint) => hasSegment(endpoint))) return "read"; + return WRITE_POST_ENDPOINTS.some((endpoint) => hasSegment(endpoint)) ? "write" : "dangerous"; + } + if (method === "PUT" && (hasDocumentId("_doc") || hasDocumentId("_create"))) return "write"; + if (method === "DELETE" && hasDocumentId("_doc")) return "write"; + return "dangerous"; +} + +/** + * Classifies a single request: the request line is the first non-comment line, + * everything after it is the JSON/NDJSON body. Returns null when the text is + * not a REST request — Elasticsearch also accepts SQL, classified elsewhere. + */ +export function classifyElasticsearchRequestRisk(request: string): ElasticsearchRequestRisk | null { + const parsed = parseRequestLine(uncommentedLines(request)[0] ?? ""); + return parsed ? classifyParsedRequest(parsed) : null; +} + +/** + * Classifies every request in the editor text and returns the highest risk, so + * a read request followed by a write is still guarded. Like the backend, the + * text only counts as REST when its first non-comment line is a request line; + * anything else is left to the SQL classification. + */ +export function classifyElasticsearchSourceRisk(source: string): ElasticsearchRequestRisk | null { + const lines = uncommentedLines(source); + if (!parseRequestLine(lines[0] ?? "")) return null; + + let highest: ElasticsearchRequestRisk = "read"; + for (const line of lines) { + const parsed = parseRequestLine(line); + if (!parsed) continue; + const risk = classifyParsedRequest(parsed); + if (RISK_ORDER[risk] > RISK_ORDER[highest]) highest = risk; + } + return highest; +} + +/** + * Returns each line with its leading comments removed, dropping lines that hold + * nothing else. Block comments spanning several lines are skipped as a whole so + * a commented-out request line is never classified. + */ +function uncommentedLines(source: string): string[] { + const lines: string[] = []; + let inBlockComment = false; + + for (const rawLine of source.split("\n")) { + let offset = 0; + while (offset < rawLine.length) { + if (inBlockComment) { + const close = rawLine.indexOf("*/", offset); + if (close < 0) break; + inBlockComment = false; + offset = close + 2; + continue; + } + while (offset < rawLine.length && /\s/.test(rawLine[offset] ?? "")) offset += 1; + if (offset >= rawLine.length) break; + if (rawLine.startsWith("/*", offset)) { + inBlockComment = true; + offset += 2; + continue; + } + if (rawLine[offset] === "#" || rawLine.startsWith("//", offset)) break; + lines.push(rawLine.slice(offset)); + break; + } + } + + return lines; +} diff --git a/apps/desktop/src/lib/sql/sqlRisk.ts b/apps/desktop/src/lib/sql/sqlRisk.ts index 890d3bd383..d6575ed306 100644 --- a/apps/desktop/src/lib/sql/sqlRisk.ts +++ b/apps/desktop/src/lib/sql/sqlRisk.ts @@ -1,4 +1,5 @@ -import type { DatabaseType } from "@/types/database"; +import { classifyElasticsearchRequestRisk, classifyElasticsearchSourceRisk, type ElasticsearchRequestRisk } from "@/lib/elasticsearch/elasticsearchRequestRisk"; +import { isElasticsearchCompatibleDatabaseType, type DatabaseType } from "@/types/database"; export type SqlRiskLevel = "read" | "write" | "ddl" | "transaction" | "unknown"; @@ -44,6 +45,11 @@ export function splitSqlStatementsForSafety(sql: string): string[] { } export function classifySqlRisk(sql: string, options: SqlRiskOptions = {}): SqlRiskAssessment { + // REST requests carry a JSON body that must not be split on semicolons, and + // every request in the text is classified so the highest risk wins. + const searchEngineRisk = searchEngineAssessment(sql, options.dialect, classifyElasticsearchSourceRisk); + if (searchEngineRisk) return { ...searchEngineRisk, statements: [searchEngineRisk] }; + const statements = splitSqlStatementsForSafety(sql).map((statement) => classifySqlStatementRisk(statement, options)); if (!statements.length) return { risk: "unknown", statements: [] }; const highest = statements.reduce((current, statement) => (RISK_ORDER[statement.risk] > RISK_ORDER[current.risk] ? statement : current), { risk: "read" }); @@ -53,9 +59,29 @@ export function classifySqlRisk(sql: string, options: SqlRiskOptions = {}): SqlR export function classifySqlStatementRisk(sql: string, options: SqlRiskOptions = {}): SqlRiskStatementAssessment { const dynamodbRisk = classifyDynamoDbStatementRisk(sql, options.dialect); if (dynamodbRisk) return dynamodbRisk; + const searchEngineRisk = searchEngineAssessment(sql, options.dialect, classifyElasticsearchRequestRisk); + if (searchEngineRisk) return searchEngineRisk; return classifyTokens(tokenizeSqlForRisk(sql)); } +/** + * Elasticsearch-compatible connections run REST requests whose method and path + * decide the risk; `GET`/`POST _search` reads must not be mistaken for writes. + * Text that is not a REST request (Elasticsearch SQL) falls through to the + * ordinary SQL classification. + * + * The HTTP method is deliberately not reported as the first keyword: callers + * map that keyword onto SQL semantics (`insert` is a low-risk write, `create` + * is a schema change, ...), which a request path does not carry. Reporting + * `rest` keeps REST mutations as opaque as they were before this branch existed. + */ +function searchEngineAssessment(sql: string, dialect: DatabaseType | string | undefined, classify: (value: string) => ElasticsearchRequestRisk | null): SqlRiskStatementAssessment | null { + if (!isElasticsearchCompatibleDatabaseType(dialect as DatabaseType | undefined)) return null; + const risk = classify(sql); + if (!risk) return null; + return { risk: risk === "read" ? "read" : risk === "write" ? "write" : "ddl", firstKeyword: "rest" }; +} + function classifyDynamoDbStatementRisk(sql: string, dialect?: DatabaseType | string): SqlRiskStatementAssessment | null { if (dialect !== "dynamodb") return null; const header = sql