feat(dev_tools): enhance SQL Query Tool — clear, copy, history, edit, hardening

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) <noreply@anthropic.com>
2026-05-17 11:24:40 +02:00
parent 64a178f45d
commit e94b6d07bb
5 changed files with 268 additions and 18 deletions
--- 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
--- 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."""
--- 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' });
+        },
    };
 }
--- 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 <span x-text="q.run_count"></span> time<span x-show="q.run_count !== 1">s</span>
                            </div>
                        </div>
-                        <button @click.stop="deleteSavedQuery(q.id)"
-                                class="opacity-0 group-hover:opacity-100 p-1 text-gray-400 hover:text-red-500 transition-opacity"
-                                title="Delete">
-                            <span x-html="$icon('trash', 'w-4 h-4')"></span>
-                        </button>
+                        <div class="flex items-center gap-1 opacity-0 group-hover:opacity-100 transition-opacity">
+                            <button @click.stop="openEditModal(q)"
+                                    class="p-1 text-gray-400 hover:text-indigo-500"
+                                    title="Edit">
+                                <span x-html="$icon('pencil', 'w-4 h-4')"></span>
+                            </button>
+                            <button @click.stop="deleteSavedQuery(q.id)"
+                                    class="p-1 text-gray-400 hover:text-red-500"
+                                    title="Delete">
+                                <span x-html="$icon('trash', 'w-4 h-4')"></span>
+                            </button>
+                        </div>
+                    </li>
+                </template>
+            </ul>
+        </div>
+
+        <!-- Recent Queries (history) -->
+        <div class="bg-white dark:bg-gray-800 rounded-lg shadow p-4">
+            <div class="flex items-center justify-between mb-3">
+                <h3 class="text-sm font-semibold text-gray-700 dark:text-gray-300 uppercase tracking-wider flex items-center gap-1.5">
+                    <span x-html="$icon('clock', 'w-4 h-4')"></span>
+                    Recent
+                </h3>
+                <button @click="clearHistory()"
+                        x-show="history.length > 0"
+                        class="text-xs text-gray-400 hover:text-red-500"
+                        title="Clear history">Clear</button>
+            </div>
+            <div x-show="history.length === 0" class="text-sm text-gray-400">No recent queries.</div>
+            <ul class="space-y-1">
+                <template x-for="(h, idx) in history" :key="h.ts">
+                    <li @click="loadHistory(h)"
+                        class="rounded-md px-2 py-1.5 text-xs cursor-pointer hover:bg-gray-100 dark:hover:bg-gray-700 transition-colors text-gray-600 dark:text-gray-400"
+                        :title="h.sql">
+                        <div class="truncate font-mono" x-text="h.preview"></div>
+                        <div class="text-[10px] text-gray-400 mt-0.5">
+                            <span x-text="formatHistoryTime(h.ts)"></span>
+                            <span x-show="h.row_count !== undefined"> · <span x-text="h.row_count"></span> row<span x-show="h.row_count !== 1">s</span></span>
+                            <span x-show="h.elapsed !== undefined"> · <span x-text="h.elapsed"></span>ms</span>
+                        </div>
                    </li>
                </template>
            </ul>
@@ -128,6 +164,22 @@
                    <span x-html="$icon('download', 'w-4 h-4 mr-1.5')"></span>
                    Export CSV
                </button>
+
+                <button @click="copyResults()"
+                        x-show="rows.length > 0"
+                        class="inline-flex items-center px-4 py-2 bg-gray-100 dark:bg-gray-700 text-gray-700 dark:text-gray-200 text-sm font-medium rounded-lg hover:bg-gray-200 dark:hover:bg-gray-600 transition-colors"
+                        title="Copy results as TSV (paste into spreadsheet)">
+                    <span x-html="$icon('clipboard', 'w-4 h-4 mr-1.5')"></span>
+                    Copy Results
+                </button>
+
+                <button @click="clearQuery()"
+                        :disabled="!sql && columns.length === 0 && !error"
+                        class="ml-auto inline-flex items-center px-4 py-2 bg-gray-100 dark:bg-gray-700 text-gray-700 dark:text-gray-200 text-sm font-medium rounded-lg hover:bg-gray-200 dark:hover:bg-gray-600 disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
+                        title="Clear editor and results">
+                    <span x-html="$icon('x-circle', 'w-4 h-4 mr-1.5')"></span>
+                    Clear
+                </button>
            </div>
        </div>

@@ -182,9 +234,10 @@
    </div>
 </div>

-<!-- Save Query Modal -->
+<!-- Save / Edit Query Modal -->
 {% call modal('saveQueryModal', 'Save Query', show_var='showSaveModal', size='sm', show_footer=false) %}
 <div class="space-y-4">
+    <div class="text-xs uppercase tracking-wider text-gray-400" x-text="editingSavedId ? 'Edit saved query' : 'New saved query'"></div>
    <div>
        <label class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">Name</label>
        <input type="text" x-model="saveName"
@@ -198,6 +251,13 @@
               class="w-full rounded-lg border-gray-300 dark:border-gray-600 dark:bg-gray-700 dark:text-gray-100 text-sm focus:ring-indigo-500 focus:border-indigo-500"
               placeholder="Brief description of what this query does">
    </div>
+    <div x-show="editingSavedId">
+        <label class="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">SQL</label>
+        <textarea x-model="saveSql"
+                  rows="6"
+                  spellcheck="false"
+                  class="w-full bg-gray-900 text-green-400 font-mono text-xs rounded-lg p-3 border border-gray-700 focus:border-indigo-500 focus:ring-1 focus:ring-indigo-500 resize-y"></textarea>
+    </div>
    <div class="flex justify-end gap-3 pt-2">
        <button @click="showSaveModal = false"
                class="px-4 py-2 text-sm font-medium text-gray-700 dark:text-gray-300 bg-gray-100 dark:bg-gray-700 rounded-lg hover:bg-gray-200 dark:hover:bg-gray-600 transition-colors">
@@ -206,7 +266,7 @@
        <button @click="saveQuery()"
                :disabled="!saveName.trim() || saving"
                class="px-4 py-2 text-sm font-medium text-white bg-indigo-600 rounded-lg hover:bg-indigo-700 disabled:opacity-50 transition-colors">
-            <span x-text="saving ? 'Saving...' : 'Save'"></span>
+            <span x-text="saving ? 'Saving...' : (editingSavedId ? 'Save changes' : 'Save')"></span>
        </button>
    </div>
 </div>
--- 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")