From e94b6d07bbd5f9e9acf70fcca0577f8021b60aa9 Mon Sep 17 00:00:00 2001 From: Samir Boulahtit Date: Sun, 17 May 2026 11:24:40 +0200 Subject: [PATCH] =?UTF-8?q?feat(dev=5Ftools):=20enhance=20SQL=20Query=20To?= =?UTF-8?q?ol=20=E2=80=94=20clear,=20copy,=20history,=20edit,=20hardening?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UI: add Clear and Copy-to-clipboard (TSV) buttons, an in-page Recent Queries pane (localStorage, capped at 20, de-duped) and a pencil-edit flow for saved queries with a dedicated SQL field in the modal. Bind Ctrl/Cmd+S to open the save modal (or edit the active saved query). Backend: harden validate_query with a multi-statement guard that respects string literals + comments. Stop swallowing record_query_run errors silently — log via logger.exception so failures show up. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../dev_tools/routes/api/admin_sql_query.py | 5 +- .../dev_tools/services/sql_query_service.py | 14 ++ .../dev_tools/static/admin/js/sql-query.js | 180 +++++++++++++++++- .../templates/dev_tools/admin/sql-query.html | 74 ++++++- .../tests/unit/test_sql_query_service.py | 13 ++ 5 files changed, 268 insertions(+), 18 deletions(-) diff --git a/app/modules/dev_tools/routes/api/admin_sql_query.py b/app/modules/dev_tools/routes/api/admin_sql_query.py index f0567182..86258bd7 100644 --- a/app/modules/dev_tools/routes/api/admin_sql_query.py +++ b/app/modules/dev_tools/routes/api/admin_sql_query.py @@ -67,7 +67,10 @@ async def execute_sql( try: record_query_run(db, body.saved_query_id) db.commit() - except Exception: + except Exception: # noqa: EXC003 + logger.exception( + "Failed to record run for saved query %s", body.saved_query_id + ) db.rollback() return result diff --git a/app/modules/dev_tools/services/sql_query_service.py b/app/modules/dev_tools/services/sql_query_service.py index 0ef50559..3beb297f 100644 --- a/app/modules/dev_tools/services/sql_query_service.py +++ b/app/modules/dev_tools/services/sql_query_service.py @@ -46,6 +46,12 @@ def _strip_sql_comments(sql: str) -> str: return result +def _strip_string_literals(sql: str) -> str: + """Replace single-quoted string literals with a placeholder so their contents + don't trip the multi-statement check.""" + return re.sub(r"'(?:''|[^'])*'", "''", sql) + + def validate_query(sql: str) -> None: """Validate that the SQL query is SELECT-only (no DML/DDL).""" stripped = sql.strip().rstrip(";") @@ -60,6 +66,14 @@ def validate_query(sql: str) -> None: f"Forbidden SQL keyword: {match.group().upper()}. Only SELECT queries are allowed." ) + # Defense-in-depth: reject queries that chain a second statement after `;`. + # SQLAlchemy's `text()` over psycopg generally sends a single statement, but + # this guards against future driver changes and makes intent explicit. + if ";" in _strip_string_literals(code_only): + raise QueryValidationError( + "Multiple statements are not allowed. Submit one SELECT at a time." + ) + def _make_json_safe(value: Any) -> Any: """Convert a database value to a JSON-serializable representation.""" diff --git a/app/modules/dev_tools/static/admin/js/sql-query.js b/app/modules/dev_tools/static/admin/js/sql-query.js index 7f846b59..4cbcbbb5 100644 --- a/app/modules/dev_tools/static/admin/js/sql-query.js +++ b/app/modules/dev_tools/static/admin/js/sql-query.js @@ -31,12 +31,19 @@ function sqlQueryTool() { loadingSaved: false, activeSavedId: null, - // Save modal + // Save / edit modal showSaveModal: false, saveName: '', saveDescription: '', + saveSql: '', + editingSavedId: null, saving: false, + // Ad-hoc query history (localStorage) + history: [], + historyKey: 'sqlQueryTool:history:v1', + historyMax: 20, + // Schema explorer showPresets: true, expandedCategories: {}, @@ -323,11 +330,25 @@ function sqlQueryTool() { sqlLog.error('Failed to initialize:', e); } - // Ctrl+Enter shortcut + this.loadHistoryFromStorage(); + + // Keyboard shortcuts document.addEventListener('keydown', (e) => { if ((e.ctrlKey || e.metaKey) && e.key === 'Enter') { e.preventDefault(); this.executeQuery(); + } else if ((e.ctrlKey || e.metaKey) && (e.key === 's' || e.key === 'S')) { + if (this.sql.trim()) { + e.preventDefault(); + if (this.activeSavedId) { + const q = this.savedQueries.find(s => s.id === this.activeSavedId); + if (q) { + this.openEditModal(q); + return; + } + } + this.openSaveModal(); + } } }); }, @@ -342,8 +363,9 @@ function sqlQueryTool() { this.truncated = false; this.executionTimeMs = null; + const submittedSql = this.sql; try { - const payload = { sql: this.sql }; + const payload = { sql: submittedSql }; if (this.activeSavedId) { payload.saved_query_id = this.activeSavedId; } @@ -354,17 +376,65 @@ function sqlQueryTool() { this.truncated = data.truncated; this.executionTimeMs = data.execution_time_ms; + this.pushHistory({ + sql: submittedSql, + ts: Date.now(), + row_count: data.row_count, + elapsed: data.execution_time_ms, + }); + // Refresh saved queries to update run_count if (this.activeSavedId) { await this.loadSavedQueries(); } } catch (e) { this.error = e.message; + this.pushHistory({ sql: submittedSql, ts: Date.now(), error: true }); } finally { this.running = false; } }, + clearQuery() { + this.sql = ''; + this.columns = []; + this.rows = []; + this.rowCount = 0; + this.truncated = false; + this.executionTimeMs = null; + this.error = null; + this.activeSavedId = null; + }, + + async copyResults() { + if (!this.columns.length || !this.rows.length) return; + + const escape = (val) => { + if (val === null || val === undefined) return ''; + const s = typeof val === 'object' ? JSON.stringify(val) : String(val); + // TSV: replace tabs/newlines so cells stay one-line in spreadsheets + return s.replace(/\t/g, ' ').replace(/\r?\n/g, ' '); + }; + + const lines = [this.columns.join('\t')]; + for (const row of this.rows) { + lines.push(row.map(escape).join('\t')); + } + const text = lines.join('\n'); + + try { + await navigator.clipboard.writeText(text); + if (typeof Utils !== 'undefined' && Utils.showToast) { + Utils.showToast(`Copied ${this.rows.length} row${this.rows.length === 1 ? '' : 's'} to clipboard`, 'success'); + } + } catch (e) { + sqlLog.error('Failed to copy results:', e); + if (typeof Utils !== 'undefined' && Utils.showToast) { + Utils.showToast('Failed to copy results', 'error'); + } + } + }, + async loadSavedQueries() { this.loadingSaved = true; try { @@ -389,23 +459,57 @@ function sqlQueryTool() { }, openSaveModal() { + this.editingSavedId = null; this.saveName = ''; this.saveDescription = ''; + this.saveSql = ''; + this.showSaveModal = true; + }, + + openEditModal(q) { + this.editingSavedId = q.id; + this.saveName = q.name || ''; + this.saveDescription = q.description || ''; + this.saveSql = q.sql_text || ''; this.showSaveModal = true; }, async saveQuery() { - if (!this.saveName.trim() || !this.sql.trim()) return; + if (!this.saveName.trim()) return; this.saving = true; try { - const saved = await apiClient.post('/admin/sql-query/saved', { - name: this.saveName, - sql_text: this.sql, - description: this.saveDescription || null, - }); - this.activeSavedId = saved.id; + let saved; + if (this.editingSavedId) { + if (!this.saveSql.trim()) { + this.saving = false; + return; + } + saved = await apiClient.put(`/admin/sql-query/saved/${this.editingSavedId}`, { + name: this.saveName, + sql_text: this.saveSql, + description: this.saveDescription || null, + }); + if (this.activeSavedId === this.editingSavedId) { + this.sql = saved.sql_text; + } + } else { + if (!this.sql.trim()) { + this.saving = false; + return; + } + saved = await apiClient.post('/admin/sql-query/saved', { + name: this.saveName, + sql_text: this.sql, + description: this.saveDescription || null, + }); + this.activeSavedId = saved.id; + } this.showSaveModal = false; + this.editingSavedId = null; await this.loadSavedQueries(); + if (typeof Utils !== 'undefined' && Utils.showToast) { + Utils.showToast('Saved', 'success'); + } } catch (e) { this.error = e.message; } finally { @@ -461,5 +565,61 @@ function sqlQueryTool() { isNull(val) { return val === null || val === undefined; }, + + // ── History (localStorage) ───────────────────────────────────────────── + loadHistoryFromStorage() { + try { + const raw = localStorage.getItem(this.historyKey); + this.history = raw ? JSON.parse(raw) : []; + } catch (e) { + sqlLog.error('Failed to load history:', e); + this.history = []; + } + }, + + persistHistory() { + try { + localStorage.setItem(this.historyKey, JSON.stringify(this.history)); + } catch (e) { + sqlLog.error('Failed to persist history:', e); + } + }, + + pushHistory(entry) { + const sql = (entry.sql || '').trim(); + if (!sql) return; + const preview = sql.replace(/\s+/g, ' ').slice(0, 80); + // De-dupe against the most recent identical SQL + if (this.history.length > 0 && this.history[0].sql === sql) { + this.history.shift(); + } + this.history.unshift({ ...entry, sql, preview }); + if (this.history.length > this.historyMax) { + this.history.length = this.historyMax; + } + this.persistHistory(); + }, + + loadHistory(h) { + this.sql = h.sql; + this.activeSavedId = null; + this.error = null; + }, + + clearHistory() { + this.history = []; + this.persistHistory(); + }, + + formatHistoryTime(ts) { + if (!ts) return ''; + const d = new Date(ts); + const today = new Date(); + const sameDay = d.toDateString() === today.toDateString(); + if (sameDay) { + return d.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); + } + return d.toLocaleDateString([], { month: 'short', day: 'numeric' }); + }, }; } diff --git a/app/modules/dev_tools/templates/dev_tools/admin/sql-query.html b/app/modules/dev_tools/templates/dev_tools/admin/sql-query.html index b4679313..5766305c 100644 --- a/app/modules/dev_tools/templates/dev_tools/admin/sql-query.html +++ b/app/modules/dev_tools/templates/dev_tools/admin/sql-query.html @@ -79,11 +79,47 @@ Run times - +
+ + +
+ + + + + + +
+
+

+ + Recent +

+ +
+
No recent queries.
+
    +
@@ -128,6 +164,22 @@ Export CSV + + + +
@@ -182,9 +234,10 @@ - + {% call modal('saveQueryModal', 'Save Query', show_var='showSaveModal', size='sm', show_footer=false) %}
+
+
+ + +
diff --git a/app/modules/dev_tools/tests/unit/test_sql_query_service.py b/app/modules/dev_tools/tests/unit/test_sql_query_service.py index 8f8de009..5210a969 100644 --- a/app/modules/dev_tools/tests/unit/test_sql_query_service.py +++ b/app/modules/dev_tools/tests/unit/test_sql_query_service.py @@ -53,3 +53,16 @@ class TestValidateQuery: def test_select_with_where(self): validate_query("SELECT id, email FROM users WHERE is_active = true") + + def test_trailing_semicolon_allowed(self): + validate_query("SELECT 1;") + + def test_multi_statement_rejected(self): + with pytest.raises(QueryValidationError, match="Multiple statements"): + validate_query("SELECT 1; SELECT 2") + + def test_semicolon_in_string_literal_allowed(self): + validate_query("SELECT ';' AS sep") + + def test_semicolon_in_comment_allowed(self): + validate_query("SELECT 1 -- end; second\nFROM dual")