From 42e9431949b742a62c287e8ef42a2a074eec3b69 Mon Sep 17 00:00:00 2001 From: Andrea Debernardi Date: Wed, 19 Aug 2026 07:59:14 +0200 Subject: [PATCH 1/2] fix: unify safety confirmation guards --- src-tauri/src/config.rs | 16 ++- src/components/notebook/NotebookView.tsx | 32 ++++-- src/components/settings/GeneralTab.tsx | 12 +++ src/contexts/ProductionGuardContext.tsx | 24 ++++- src/contexts/SettingsContext.ts | 3 + src/hooks/useDangerousQueryGuard.ts | 9 +- src/i18n/locales/de.json | 2 + src/i18n/locales/en.json | 2 + src/i18n/locales/es.json | 2 + src/i18n/locales/fr.json | 2 + src/i18n/locales/it.json | 2 + src/i18n/locales/ja.json | 2 + src/i18n/locales/ko.json | 2 + src/i18n/locales/pt-BR.json | 2 + src/i18n/locales/ru.json | 2 + src/i18n/locales/tl.json | 2 + src/i18n/locales/zh.json | 4 +- src/pages/Editor.tsx | 33 +++++-- src/utils/environment.ts | 15 +++ src/utils/queryGuard.ts | 16 +++ .../contexts/ProductionGuardContext.test.tsx | 98 +++++++++++++++++++ tests/contexts/SettingsProvider.test.tsx | 3 + tests/hooks/useDangerousQueryGuard.test.ts | 12 +++ tests/utils/environment.test.ts | 23 +++++ tests/utils/queryGuard.test.ts | 43 ++++++++ 25 files changed, 334 insertions(+), 29 deletions(-) create mode 100644 src/utils/queryGuard.ts create mode 100644 tests/contexts/ProductionGuardContext.test.tsx create mode 100644 tests/utils/environment.test.ts create mode 100644 tests/utils/queryGuard.test.ts diff --git a/src-tauri/src/config.rs b/src-tauri/src/config.rs index 76b1d5eca..e82f8e052 100644 --- a/src-tauri/src/config.rs +++ b/src-tauri/src/config.rs @@ -3,9 +3,9 @@ use once_cell::sync::Lazy; use serde::{Deserialize, Serialize}; use std::fs; use std::path::PathBuf; +use std::sync::RwLock; use tauri::AppHandle; use tauri::Manager; -use std::sync::RwLock; use std::collections::HashMap; @@ -79,6 +79,8 @@ pub struct AppConfig { /// Default: `true` — matches the behaviour users expect from most editors. pub editor_accept_suggestion_on_enter: Option, pub run_statement_under_cursor: Option, + /// Delay destructive-query and production-write confirmations for five seconds. Default: false. + pub safety_confirmation_delay_enabled: Option, // ----- SQL Formatter ----- pub formatter_keyword_case: Option, pub formatter_indent_style: Option, @@ -372,6 +374,10 @@ pub fn save_config(app: AppHandle, config: AppConfig) -> Result<(), String> { if config.run_statement_under_cursor.is_some() { existing_config.run_statement_under_cursor = config.run_statement_under_cursor; } + if config.safety_confirmation_delay_enabled.is_some() { + existing_config.safety_confirmation_delay_enabled = + config.safety_confirmation_delay_enabled; + } if config.ping_interval.is_some() { let old_interval = existing_config.ping_interval; existing_config.ping_interval = config.ping_interval; @@ -923,6 +929,7 @@ mod tests { #[test] fn editor_fields_default_to_none() { let config = AppConfig::default(); + assert!(config.safety_confirmation_delay_enabled.is_none()); assert!(config.editor_theme.is_none()); assert!(config.editor_font_family.is_none()); assert!(config.editor_font_size.is_none()); @@ -944,6 +951,7 @@ mod tests { config.editor_show_line_numbers = Some(true); config.editor_theme = Some("tabularis-light".to_string()); config.editor_accept_suggestion_on_enter = Some(true); + config.safety_confirmation_delay_enabled = Some(true); let json = serde_json::to_string(&config).unwrap(); assert!(json.contains("editorFontFamily")); @@ -954,9 +962,11 @@ mod tests { assert!(json.contains("editorShowLineNumbers")); assert!(json.contains("editorTheme")); assert!(json.contains("editorAcceptSuggestionOnEnter")); + assert!(json.contains("safetyConfirmationDelayEnabled")); // snake_case must not appear assert!(!json.contains("editor_font_family")); assert!(!json.contains("editor_accept_suggestion_on_enter")); + assert!(!json.contains("safety_confirmation_delay_enabled")); } #[test] @@ -969,7 +979,8 @@ mod tests { "editorWordWrap": true, "editorShowLineNumbers": true, "editorTheme": "tabularis-dark", - "editorAcceptSuggestionOnEnter": true + "editorAcceptSuggestionOnEnter": true, + "safetyConfirmationDelayEnabled": true }"#; let config: AppConfig = serde_json::from_str(json).unwrap(); @@ -980,6 +991,7 @@ mod tests { assert_eq!(config.editor_show_line_numbers, Some(true)); assert_eq!(config.editor_theme.as_deref(), Some("tabularis-dark")); assert_eq!(config.editor_accept_suggestion_on_enter, Some(true)); + assert_eq!(config.safety_confirmation_delay_enabled, Some(true)); } #[test] diff --git a/src/components/notebook/NotebookView.tsx b/src/components/notebook/NotebookView.tsx index b63e0a456..b7bc4bfe5 100644 --- a/src/components/notebook/NotebookView.tsx +++ b/src/components/notebook/NotebookView.tsx @@ -60,6 +60,8 @@ import { import { useDatabase } from "../../hooks/useDatabase"; import { useSqlAutocompleteRegistration } from "../../hooks/useSqlAutocompleteRegistration"; import { usesMultiDatabaseLayout } from "../../utils/database"; +import { isProductionConnection } from "../../utils/environment"; +import { passQueryGuards } from "../../utils/queryGuard"; import { useSettings } from "../../hooks/useSettings"; import { useAlert } from "../../hooks/useAlert"; import { useKeybindings } from "../../hooks/useKeybindings"; @@ -92,8 +94,13 @@ export function NotebookView({ isActive, }: NotebookViewProps) { const { t } = useTranslation(); - const { activeSchema, activeCapabilities, selectedDatabases, activeDriver } = - useDatabase(); + const { + activeSchema, + activeCapabilities, + selectedDatabases, + activeDriver, + connections, + } = useDatabase(); const isMultiDb = usesMultiDatabaseLayout(activeCapabilities, selectedDatabases); const effectiveSchema = tab.schema || activeSchema || (isMultiDb ? selectedDatabases[0] : null); @@ -102,13 +109,17 @@ export function NotebookView({ enabled: isActive, }); const { settings } = useSettings(); + const hasProductionConnection = isProductionConnection( + connections, + connectionId, + ); const { showAlert } = useAlert(); const { matchesShortcut } = useKeybindings(); const { pending: dangerousQuery, guardQuery: guardDangerousQuery, resolve: resolveDangerousQuery, - } = useDangerousQueryGuard(); + } = useDangerousQueryGuard(!hasProductionConnection); const guardProductionWrite = useProductionGuard(); // Local notebook state — loaded from store/disk, NOT from tab @@ -369,12 +380,11 @@ export function NotebookView({ return; } - if (!(await guardDangerousQuery(resolvedSql))) { - updateCell(cellId, { isLoading: false }); - return; - } - - if (!(await guardProductionWrite(connectionId, resolvedSql))) { + const mayRun = await passQueryGuards({ + guardProduction: () => guardProductionWrite(connectionId, resolvedSql), + guardDangerousQuery: () => guardDangerousQuery(resolvedSql), + }); + if (!mayRun) { updateCell(cellId, { isLoading: false }); return; } @@ -863,7 +873,9 @@ export function NotebookView({ sql={dangerousQuery?.sql} confirmLabel={t("editor.dangerousQueryConfirm")} variant="danger" - confirmDelaySeconds={5} + confirmDelaySeconds={ + settings.safetyConfirmationDelayEnabled ? 5 : undefined + } /> {showHistory && ( diff --git a/src/components/settings/GeneralTab.tsx b/src/components/settings/GeneralTab.tsx index a39ec56b3..a267f4c6d 100644 --- a/src/components/settings/GeneralTab.tsx +++ b/src/components/settings/GeneralTab.tsx @@ -146,6 +146,18 @@ export function GeneralTab() { onChange={(v) => updateSetting("runStatementUnderCursor", v)} /> + + + + updateSetting("safetyConfirmationDelayEnabled", v) + } + /> + diff --git a/src/contexts/ProductionGuardContext.tsx b/src/contexts/ProductionGuardContext.tsx index c77e45b4a..6768428d1 100644 --- a/src/contexts/ProductionGuardContext.tsx +++ b/src/contexts/ProductionGuardContext.tsx @@ -1,8 +1,9 @@ -import { useCallback, useState, type ReactNode } from "react"; +import { useCallback, useEffect, useState, type ReactNode } from "react"; import { useTranslation } from "react-i18next"; import { TriangleAlert, X } from "lucide-react"; import { Modal } from "../components/ui/Modal"; import { SqlPreview } from "../components/ui/SqlPreview"; +import { useSettings } from "../hooks/useSettings"; import { ProductionGuardContext, snoozedConnectionIds, @@ -23,25 +24,37 @@ interface PendingPrompt { export function ProductionGuardProvider({ children }: { children: ReactNode }) { const { t } = useTranslation(); + const { settings } = useSettings(); const [pending, setPending] = useState(null); const [snooze, setSnooze] = useState(false); + const [remaining, setRemaining] = useState(0); const request = useCallback( (connectionId, connectionName, sql) => { return new Promise((resolve) => { setSnooze(false); + setRemaining(settings.safetyConfirmationDelayEnabled ? 5 : 0); setPending({ connectionId, connectionName, sql, resolve }); }); }, - [], + [settings.safetyConfirmationDelayEnabled], ); + useEffect(() => { + if (!pending || remaining <= 0) return; + const timeout = setTimeout(() => { + setRemaining((previous) => Math.max(0, previous - 1)); + }, 1000); + return () => clearTimeout(timeout); + }, [pending, remaining]); + const finish = (ok: boolean) => { if (!pending) return; if (ok && snooze && pending.connectionId) { snoozedConnectionIds.add(pending.connectionId); } pending.resolve(ok); + setRemaining(0); setPending(null); }; @@ -97,9 +110,12 @@ export function ProductionGuardProvider({ children }: { children: ReactNode }) { diff --git a/src/contexts/SettingsContext.ts b/src/contexts/SettingsContext.ts index d988bc018..c488765f7 100644 --- a/src/contexts/SettingsContext.ts +++ b/src/contexts/SettingsContext.ts @@ -64,6 +64,8 @@ export interface Settings { editorShowLineNumbers?: boolean; editorAcceptSuggestionOnEnter?: boolean; runStatementUnderCursor?: boolean; + /** Delay destructive-query and production-write confirmations for five seconds. Default: false. */ + safetyConfirmationDelayEnabled?: boolean; // SQL Formatter formatterKeywordCase?: "upper" | "lower" | "preserve"; formatterIndentStyle?: "standard" | "tabularLeft" | "tabularRight"; @@ -162,6 +164,7 @@ export const DEFAULT_SETTINGS: Settings = { editorShowLineNumbers: true, editorAcceptSuggestionOnEnter: true, runStatementUnderCursor: true, + safetyConfirmationDelayEnabled: false, formatterKeywordCase: "upper", formatterIndentStyle: "standard", formatterTabWidth: 2, diff --git a/src/hooks/useDangerousQueryGuard.ts b/src/hooks/useDangerousQueryGuard.ts index db03967ca..e651bb817 100644 --- a/src/hooks/useDangerousQueryGuard.ts +++ b/src/hooks/useDangerousQueryGuard.ts @@ -39,9 +39,10 @@ export const DANGEROUS_QUERY_I18N: Record< * dialog) for safe statements; for dangerous ones it opens the dialog and * resolves once the user answers. A second dangerous statement submitted while * a dialog is already open is declined immediately instead of replacing the - * pending one, so the first caller's promise always settles. + * pending one, so the first caller's promise always settles. Pass `false` to + * disable this guard when a higher-priority safety prompt handles the action. */ -export function useDangerousQueryGuard() { +export function useDangerousQueryGuard(enabled = true) { const [pending, setPending] = useState(null); const resolverRef = useRef<((confirmed: boolean) => void) | null>(null); @@ -64,6 +65,8 @@ export function useDangerousQueryGuard() { const guardQuery = useCallback( (sqlOrQueries: string | string[]): Promise => { + if (!enabled) return Promise.resolve(true); + const statements = Array.isArray(sqlOrQueries) ? sqlOrQueries : [sqlOrQueries]; @@ -80,7 +83,7 @@ export function useDangerousQueryGuard() { if (!first) return Promise.resolve(true); return requestConfirmation({ ...first, count }); }, - [requestConfirmation], + [enabled, requestConfirmation], ); return { pending, isPending: pending !== null, guardQuery, resolve }; diff --git a/src/i18n/locales/de.json b/src/i18n/locales/de.json index 198aedfd9..259d8ae8b 100644 --- a/src/i18n/locales/de.json +++ b/src/i18n/locales/de.json @@ -471,6 +471,8 @@ "queryExecution": "Abfrageausführung", "runStatementUnderCursor": "Anweisung unter dem Cursor ausführen", "runStatementUnderCursorDesc": "Wenn ein Skript mehrere Anweisungen enthält, wird die Anweisung unter dem Cursor ausgeführt, statt die Abfrageauswahl anzuzeigen. Markiere Text, um einen bestimmten Bereich auszuführen.", + "safetyConfirmationDelay": "Sicherheitsbestätigungen verzögern", + "safetyConfirmationDelayDesc": "Deaktiviert Bestätigungsschaltflächen für destruktive Abfragen und Produktionsschreibvorgänge fünf Sekunden lang.", "formatter_title": "SQL-Formatierung", "formatter_keywordCase": "Schlüsselwort-Schreibweise", "formatter_keywordCaseDesc": "Groß-/Kleinschreibung von SQL-Schlüsselwörtern (SELECT, FROM, WHERE) nach der Formatierung.", diff --git a/src/i18n/locales/en.json b/src/i18n/locales/en.json index f03428542..18ae6f791 100644 --- a/src/i18n/locales/en.json +++ b/src/i18n/locales/en.json @@ -506,6 +506,8 @@ "queryExecution": "Query Execution", "runStatementUnderCursor": "Run statement under cursor", "runStatementUnderCursorDesc": "When a script has multiple statements, run the one under the cursor instead of showing the query picker. Select text to run a specific range.", + "safetyConfirmationDelay": "Delay safety confirmations", + "safetyConfirmationDelayDesc": "Disable confirmation buttons for five seconds for destructive queries and production writes.", "formatter_title": "SQL Formatter", "formatter_keywordCase": "Keyword Case", "formatter_keywordCaseDesc": "How SQL keywords (SELECT, FROM, WHERE) are capitalized after formatting.", diff --git a/src/i18n/locales/es.json b/src/i18n/locales/es.json index 9d0639f73..3a9ad23c1 100644 --- a/src/i18n/locales/es.json +++ b/src/i18n/locales/es.json @@ -496,6 +496,8 @@ "queryExecution": "Ejecución de consultas", "runStatementUnderCursor": "Ejecutar la instrucción bajo el cursor", "runStatementUnderCursorDesc": "Cuando un script tiene varias instrucciones, ejecuta la que está bajo el cursor en lugar de mostrar el selector de consultas. Selecciona texto para ejecutar un rango específico.", + "safetyConfirmationDelay": "Retrasar las confirmaciones de seguridad", + "safetyConfirmationDelayDesc": "Desactiva los botones de confirmación durante cinco segundos para consultas destructivas y escrituras en producción.", "formatter_title": "Formateador SQL", "formatter_keywordCase": "Mayúsculas en palabras clave", "formatter_keywordCaseDesc": "Cómo se capitalizan las palabras clave SQL (SELECT, FROM, WHERE) después del formateo.", diff --git a/src/i18n/locales/fr.json b/src/i18n/locales/fr.json index e62286f14..c7936dcae 100644 --- a/src/i18n/locales/fr.json +++ b/src/i18n/locales/fr.json @@ -501,6 +501,8 @@ "queryExecution": "Exécution des requêtes", "runStatementUnderCursor": "Exécuter l'instruction sous le curseur", "runStatementUnderCursorDesc": "Lorsqu'un script contient plusieurs instructions, exécute celle sous le curseur au lieu d'afficher le sélecteur de requêtes. Sélectionnez du texte pour exécuter une plage spécifique.", + "safetyConfirmationDelay": "Retarder les confirmations de sécurité", + "safetyConfirmationDelayDesc": "Désactive les boutons de confirmation pendant cinq secondes pour les requêtes destructives et les écritures en production.", "formatter_title": "Formateur SQL", "formatter_keywordCase": "Casse des mots-clés", "formatter_keywordCaseDesc": "Comment les mots-clés SQL (SELECT, FROM, WHERE) sont capitalisés après le formatage.", diff --git a/src/i18n/locales/it.json b/src/i18n/locales/it.json index c45f47f80..4a0b4fba4 100644 --- a/src/i18n/locales/it.json +++ b/src/i18n/locales/it.json @@ -496,6 +496,8 @@ "queryExecution": "Esecuzione query", "runStatementUnderCursor": "Esegui l'istruzione sotto il cursore", "runStatementUnderCursorDesc": "Quando uno script contiene più istruzioni, esegue quella sotto il cursore invece di mostrare il selettore di query. Seleziona il testo per eseguire un intervallo specifico.", + "safetyConfirmationDelay": "Ritarda le conferme di sicurezza", + "safetyConfirmationDelayDesc": "Disabilita i pulsanti di conferma per cinque secondi per le query distruttive e le scritture in produzione.", "formatter_title": "Formattatore SQL", "formatter_keywordCase": "Maiuscole parole chiave", "formatter_keywordCaseDesc": "Come vengono capitalizzate le parole chiave SQL (SELECT, FROM, WHERE) dopo la formattazione.", diff --git a/src/i18n/locales/ja.json b/src/i18n/locales/ja.json index 59b119f0a..fbcdc98f4 100644 --- a/src/i18n/locales/ja.json +++ b/src/i18n/locales/ja.json @@ -482,6 +482,8 @@ "queryExecution": "クエリの実行", "runStatementUnderCursor": "カーソル位置のステートメントを実行", "runStatementUnderCursorDesc": "スクリプトに複数のステートメントがある場合、クエリ選択画面を表示せずにカーソル位置のステートメントを実行します。特定の範囲を実行するにはテキストを選択してください。", + "safetyConfirmationDelay": "安全確認を遅延", + "safetyConfirmationDelayDesc": "破壊的なクエリと本番環境への書き込みでは、確認ボタンを5秒間無効にします。", "formatter_title": "SQLフォーマッター", "formatter_keywordCase": "キーワードの大文字/小文字", "formatter_keywordCaseDesc": "フォーマット後のSQLキーワード(SELECT、FROM、WHERE)の大文字/小文字。", diff --git a/src/i18n/locales/ko.json b/src/i18n/locales/ko.json index 4f12917d6..c5fe6b519 100644 --- a/src/i18n/locales/ko.json +++ b/src/i18n/locales/ko.json @@ -446,6 +446,8 @@ "queryExecution": "쿼리 실행", "runStatementUnderCursor": "커서 위치의 구문 실행", "runStatementUnderCursorDesc": "스크립트에 여러 구문이 있을 때 쿼리 선택 창을 표시하지 않고 커서 위치의 구문을 실행합니다. 특정 범위를 실행하려면 텍스트를 선택하세요.", + "safetyConfirmationDelay": "안전 확인 지연", + "safetyConfirmationDelayDesc": "파괴적인 쿼리와 프로덕션 쓰기의 확인 버튼을 5초 동안 비활성화합니다.", "formatter_title": "SQL 포맷터", "formatter_keywordCase": "키워드 대소문자", "formatter_keywordCaseDesc": "포맷 후 SQL 키워드(SELECT, FROM, WHERE)의 대소문자 처리.", diff --git a/src/i18n/locales/pt-BR.json b/src/i18n/locales/pt-BR.json index 3d8b5c479..557867b11 100644 --- a/src/i18n/locales/pt-BR.json +++ b/src/i18n/locales/pt-BR.json @@ -477,6 +477,8 @@ "queryExecution": "Execução de Consultas", "runStatementUnderCursor": "Executar instrução sob o cursor", "runStatementUnderCursorDesc": "Quando um script tem várias instruções, executa a que está sob o cursor em vez de mostrar o seletor de consultas. Selecione um texto para executar um intervalo específico.", + "safetyConfirmationDelay": "Atrasar confirmações de segurança", + "safetyConfirmationDelayDesc": "Desativa os botões de confirmação por cinco segundos para consultas destrutivas e gravações em produção.", "formatter_title": "Formatador SQL", "formatter_keywordCase": "Caixa das Palavras-Chave", "formatter_keywordCaseDesc": "Como as palavras-chave SQL (SELECT, FROM, WHERE) são capitalizadas após a formatação.", diff --git a/src/i18n/locales/ru.json b/src/i18n/locales/ru.json index a17204ef5..63048fd20 100644 --- a/src/i18n/locales/ru.json +++ b/src/i18n/locales/ru.json @@ -466,6 +466,8 @@ "queryExecution": "Выполнение запросов", "runStatementUnderCursor": "Выполнять запрос под курсором", "runStatementUnderCursorDesc": "Если скрипт содержит несколько запросов, выполняется тот, что под курсором, вместо показа списка запросов. Выделите текст, чтобы выполнить определённый фрагмент.", + "safetyConfirmationDelay": "Задержка подтверждений безопасности", + "safetyConfirmationDelayDesc": "Отключает кнопки подтверждения на пять секунд для разрушительных запросов и записи в production.", "formatter_title": "Форматирование SQL", "formatter_keywordCase": "Регистр ключевых слов", "formatter_keywordCaseDesc": "Регистр ключевых слов SQL (SELECT, FROM, WHERE) после форматирования.", diff --git a/src/i18n/locales/tl.json b/src/i18n/locales/tl.json index 43f767818..f69b6ea05 100644 --- a/src/i18n/locales/tl.json +++ b/src/i18n/locales/tl.json @@ -501,6 +501,8 @@ "queryExecution": "Pagpapatakbo ng Query", "runStatementUnderCursor": "Patakbuhin ang statement sa ilalim ng cursor", "runStatementUnderCursorDesc": "Kapag maraming statement ang isang script, patakbuhin ang nasa ilalim ng cursor sa halip na ipakita ang query picker. Pumili ng teksto para patakbuhin ang isang partikular na saklaw.", + "safetyConfirmationDelay": "Iantala ang mga kumpirmasyon sa kaligtasan", + "safetyConfirmationDelayDesc": "I-disable ang mga confirmation button nang limang segundo para sa mapanirang query at production writes.", "formatter_title": "SQL Formatter", "formatter_keywordCase": "Case ng Keyword", "formatter_keywordCaseDesc": "Paano icapitalize ang SQL keywords (SELECT, FROM, WHERE) pagkatapos i-format.", diff --git a/src/i18n/locales/zh.json b/src/i18n/locales/zh.json index 7a5f42fe5..16e8c61e3 100644 --- a/src/i18n/locales/zh.json +++ b/src/i18n/locales/zh.json @@ -778,7 +778,9 @@ }, "queryExecution": "查询执行", "runStatementUnderCursor": "执行光标所在的语句", - "runStatementUnderCursorDesc": "当脚本包含多条语句时,执行光标所在的语句,而不显示查询选择器。选中文本可执行指定范围。" + "runStatementUnderCursorDesc": "当脚本包含多条语句时,执行光标所在的语句,而不显示查询选择器。选中文本可执行指定范围。", + "safetyConfirmationDelay": "延迟安全确认", + "safetyConfirmationDelayDesc": "对于破坏性查询和生产环境写入,将确认按钮禁用五秒。" }, "update": { "newVersionAvailable": "有新版本可用", diff --git a/src/pages/Editor.tsx b/src/pages/Editor.tsx index c3e87c521..8aa4b43a2 100644 --- a/src/pages/Editor.tsx +++ b/src/pages/Editor.tsx @@ -15,6 +15,8 @@ import { usesMultiDatabaseLayout, } from "../utils/database"; import { isReadonly, supportsExplain } from "../utils/driverCapabilities"; +import { isProductionConnection } from "../utils/environment"; +import { passQueryGuards } from "../utils/queryGuard"; import { useClickOutside } from "../hooks/useClickOutside"; import { useDangerousQueryGuard, @@ -236,6 +238,10 @@ export const Editor = ({ commandScopeId }: EditorProps) => { const driverReadonly = isReadonly(activeCapabilities); const driverSupportsExplain = supportsExplain(activeCapabilities); const activeDialect = activeCapabilities?.sql_dialect; + const hasProductionConnection = isProductionConnection( + connections, + activeConnectionId, + ); // Editor panes stay mounted (hidden with display:none) so Monaco never // remounts. Render them sorted by id, decoupled from the tab-strip order: @@ -413,7 +419,7 @@ export const Editor = ({ commandScopeId }: EditorProps) => { pending: dangerousQuery, guardQuery: guardDangerousQuery, resolve: resolveDangerousQuery, - } = useDangerousQueryGuard(); + } = useDangerousQueryGuard(!hasProductionConnection); const guardProductionWrite = useProductionGuard(); const [isTabSwitcherOpen, setIsTabSwitcherOpen] = useState(false); const [isRunDropdownOpen, setIsRunDropdownOpen] = useState(false); @@ -929,8 +935,12 @@ export const Editor = ({ commandScopeId }: EditorProps) => { if (!textToRun || !textToRun.trim()) return; - if (!(await guardDangerousQuery(textToRun))) return; - if (!(await guardProductionWrite(activeConnectionId, textToRun))) return; + const mayRun = await passQueryGuards({ + guardProduction: () => + guardProductionWrite(activeConnectionId, textToRun), + guardDangerousQuery: () => guardDangerousQuery(textToRun), + }); + if (!mayRun) return; // Check for parameters const params = extractQueryParams(textToRun, activeDialect); @@ -1171,10 +1181,12 @@ export const Editor = ({ commandScopeId }: EditorProps) => { const targetTab = tabsRef.current.find((t) => t.id === targetTabId); if (!targetTab) return; - if (!(await guardDangerousQuery(queries))) return; - if (!(await guardProductionWrite(activeConnectionId, queries.join(";\n")))) { - return; - } + const mayRun = await passQueryGuards({ + guardProduction: () => + guardProductionWrite(activeConnectionId, queries.join(";\n")), + guardDangerousQuery: () => guardDangerousQuery(queries), + }); + if (!mayRun) return; // Collect all unique parameters across all queries const allParams = [ @@ -4687,7 +4699,9 @@ export const Editor = ({ commandScopeId }: EditorProps) => { sql={dangerousQuery?.sql} confirmLabel={t("editor.dangerousQueryConfirm")} variant="danger" - confirmDelaySeconds={5} + confirmDelaySeconds={ + settings.safetyConfirmationDelayEnabled ? 5 : undefined + } /> { schema={activeTab?.schema ?? activeSchema ?? undefined} onInsert={(q) => { updateActiveTab({ query: q }); - runQuery(q, 1); + // AI-generated SQL uses the same production-first safety pipeline. + void runQuery(q, 1); }} /> + connection.id === connectionId && connection.environment === "production", + ); +} + /** Tailwind classes for the environment badge chip, per tier. */ export const ENVIRONMENT_BADGE_CLASSES: Record = { development: diff --git a/src/utils/queryGuard.ts b/src/utils/queryGuard.ts new file mode 100644 index 000000000..b0fc0e84e --- /dev/null +++ b/src/utils/queryGuard.ts @@ -0,0 +1,16 @@ +export interface QueryGuardPipeline { + guardProduction: () => Promise; + guardDangerousQuery: () => Promise; +} + +/** + * Runs query safety checks in priority order. Production gets the first chance + * to block the operation; the standard dangerous-query guard runs afterwards. + */ +export async function passQueryGuards({ + guardProduction, + guardDangerousQuery, +}: QueryGuardPipeline): Promise { + if (!(await guardProduction())) return false; + return guardDangerousQuery(); +} diff --git a/tests/contexts/ProductionGuardContext.test.tsx b/tests/contexts/ProductionGuardContext.test.tsx new file mode 100644 index 000000000..40d157a43 --- /dev/null +++ b/tests/contexts/ProductionGuardContext.test.tsx @@ -0,0 +1,98 @@ +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useContext, type ReactNode } from "react"; +import { ProductionGuardProvider } from "../../src/contexts/ProductionGuardContext"; +import { ProductionGuardContext } from "../../src/hooks/useProductionGuard"; + +const settingsState = vi.hoisted(() => ({ delayEnabled: false })); + +vi.mock("../../src/hooks/useSettings", () => ({ + useSettings: () => ({ + settings: { + safetyConfirmationDelayEnabled: settingsState.delayEnabled, + }, + }), +})); + +vi.mock("lucide-react", () => ({ + TriangleAlert: () => , + X: () => , +})); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ + t: (key: string) => + key === "environment.warnConfirm" ? "Run anyway" : key, + }), +})); + +vi.mock("../../src/components/ui/SqlPreview", () => ({ + SqlPreview: () =>
, +})); + +vi.mock("../../src/components/ui/Modal", () => ({ + Modal: ({ + isOpen, + children, + }: { + isOpen: boolean; + children: ReactNode; + }) => (isOpen ?
{children}
: null), +})); + +function GuardRequester() { + const request = useContext(ProductionGuardContext); + + return ( + + ); +} + +describe("ProductionGuardProvider", () => { + beforeEach(() => { + settingsState.delayEnabled = false; + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("allows immediate confirmation when the safety delay is disabled", () => { + render( + + + , + ); + + fireEvent.click(screen.getByRole("button", { name: "Request confirmation" })); + + expect(screen.getByRole("button", { name: "Run anyway" })).not.toBeDisabled(); + }); + + it("counts down production confirmation when the safety delay is enabled", () => { + settingsState.delayEnabled = true; + render( + + + , + ); + + fireEvent.click(screen.getByRole("button", { name: "Request confirmation" })); + expect(screen.getByRole("button", { name: "Run anyway (5)" })).toBeDisabled(); + + for (let second = 0; second < 5; second += 1) { + act(() => { + vi.advanceTimersByTime(1000); + }); + } + + expect(screen.getByRole("button", { name: "Run anyway" })).not.toBeDisabled(); + }); +}); diff --git a/tests/contexts/SettingsProvider.test.tsx b/tests/contexts/SettingsProvider.test.tsx index 9aff8f5f6..2ae50ba39 100644 --- a/tests/contexts/SettingsProvider.test.tsx +++ b/tests/contexts/SettingsProvider.test.tsx @@ -68,6 +68,7 @@ describe("SettingsProvider", () => { expect(result.current.settings.aiEnabled).toBe(false); expect(result.current.settings.aiProvider).toBeNull(); expect(result.current.settings.aiModel).toBeNull(); + expect(result.current.settings.safetyConfirmationDelayEnabled).toBe(false); }); it("should load settings from backend config", async () => { @@ -79,6 +80,7 @@ describe("SettingsProvider", () => { aiEnabled: true, aiProvider: "openai", aiModel: "gpt-4", + safetyConfirmationDelayEnabled: true, }; vi.mocked(invoke).mockImplementation((cmd: string) => { @@ -113,6 +115,7 @@ describe("SettingsProvider", () => { expect(result.current.settings.aiEnabled).toBe(true); expect(result.current.settings.aiProvider).toBe("openai"); expect(result.current.settings.aiModel).toBe("gpt-4"); + expect(result.current.settings.safetyConfirmationDelayEnabled).toBe(true); }); it("hydrates persisted settings even while language application is still pending", async () => { diff --git a/tests/hooks/useDangerousQueryGuard.test.ts b/tests/hooks/useDangerousQueryGuard.test.ts index 9a4498cdb..193a768bf 100644 --- a/tests/hooks/useDangerousQueryGuard.test.ts +++ b/tests/hooks/useDangerousQueryGuard.test.ts @@ -15,6 +15,18 @@ describe('useDangerousQueryGuard', () => { expect(result.current.isPending).toBe(false); }); + it('skips confirmation when the guard is disabled', async () => { + const { result } = renderHook(() => useDangerousQueryGuard(false)); + + let resolved: boolean | undefined; + await act(async () => { + resolved = await result.current.guardQuery('DROP TABLE users'); + }); + + expect(resolved).toBe(true); + expect(result.current.pending).toBeNull(); + }); + it('opens a pending confirmation for a destructive query with no WHERE', async () => { const { result } = renderHook(() => useDangerousQueryGuard()); diff --git a/tests/utils/environment.test.ts b/tests/utils/environment.test.ts new file mode 100644 index 000000000..d43f5de27 --- /dev/null +++ b/tests/utils/environment.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it } from "vitest"; +import { isProductionConnection } from "../../src/utils/environment"; + +describe("environment", () => { + describe("isProductionConnection", () => { + const connections = [ + { id: "dev", environment: "development" as const }, + { id: "prod", environment: "production" as const }, + { id: "plain" }, + ]; + + it("returns true for the matching production connection", () => { + expect(isProductionConnection(connections, "prod")).toBe(true); + }); + + it("returns false for non-production, missing, and empty connection ids", () => { + expect(isProductionConnection(connections, "dev")).toBe(false); + expect(isProductionConnection(connections, "missing")).toBe(false); + expect(isProductionConnection(connections, null)).toBe(false); + expect(isProductionConnection(connections, undefined)).toBe(false); + }); + }); +}); diff --git a/tests/utils/queryGuard.test.ts b/tests/utils/queryGuard.test.ts new file mode 100644 index 000000000..17c601435 --- /dev/null +++ b/tests/utils/queryGuard.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, it, vi } from "vitest"; +import { passQueryGuards } from "../../src/utils/queryGuard"; + +describe("passQueryGuards", () => { + it("runs the production guard before the standard dangerous-query guard", async () => { + const order: string[] = []; + + const result = await passQueryGuards({ + guardProduction: vi.fn(async () => { + order.push("production"); + return true; + }), + guardDangerousQuery: vi.fn(async () => { + order.push("dangerous-query"); + return true; + }), + }); + + expect(result).toBe(true); + expect(order).toEqual(["production", "dangerous-query"]); + }); + + it("does not run the standard guard when production blocks the query", async () => { + const guardDangerousQuery = vi.fn(async () => true); + + const result = await passQueryGuards({ + guardProduction: vi.fn(async () => false), + guardDangerousQuery, + }); + + expect(result).toBe(false); + expect(guardDangerousQuery).not.toHaveBeenCalled(); + }); + + it("returns false when the standard guard blocks a non-production query", async () => { + const result = await passQueryGuards({ + guardProduction: vi.fn(async () => true), + guardDangerousQuery: vi.fn(async () => false), + }); + + expect(result).toBe(false); + }); +}); From 16b5e2be5ee059d7c6fe59ec26aa6d7c1300492d Mon Sep 17 00:00:00 2001 From: Andrea Debernardi Date: Wed, 19 Aug 2026 08:21:09 +0200 Subject: [PATCH 2/2] test: cover unified query guard integration --- src/components/notebook/NotebookView.tsx | 35 ++----- src/hooks/useQueryGuards.ts | 52 ++++++++++ src/pages/Editor.tsx | 34 ++----- tests/hooks/useQueryGuards.test.ts | 122 +++++++++++++++++++++++ 4 files changed, 190 insertions(+), 53 deletions(-) create mode 100644 src/hooks/useQueryGuards.ts create mode 100644 tests/hooks/useQueryGuards.test.ts diff --git a/src/components/notebook/NotebookView.tsx b/src/components/notebook/NotebookView.tsx index b7bc4bfe5..31a7d8f48 100644 --- a/src/components/notebook/NotebookView.tsx +++ b/src/components/notebook/NotebookView.tsx @@ -60,16 +60,11 @@ import { import { useDatabase } from "../../hooks/useDatabase"; import { useSqlAutocompleteRegistration } from "../../hooks/useSqlAutocompleteRegistration"; import { usesMultiDatabaseLayout } from "../../utils/database"; -import { isProductionConnection } from "../../utils/environment"; -import { passQueryGuards } from "../../utils/queryGuard"; import { useSettings } from "../../hooks/useSettings"; import { useAlert } from "../../hooks/useAlert"; import { useKeybindings } from "../../hooks/useKeybindings"; -import { - useDangerousQueryGuard, - DANGEROUS_QUERY_I18N, -} from "../../hooks/useDangerousQueryGuard"; -import { useProductionGuard } from "../../hooks/useProductionGuard"; +import { DANGEROUS_QUERY_I18N } from "../../hooks/useDangerousQueryGuard"; +import { useQueryGuards } from "../../hooks/useQueryGuards"; import { ConfirmModal } from "../modals/ConfirmModal"; import { NotebookToolbar } from "./NotebookToolbar"; import { NotebookHistoryPanel } from "./NotebookHistoryPanel"; @@ -94,13 +89,8 @@ export function NotebookView({ isActive, }: NotebookViewProps) { const { t } = useTranslation(); - const { - activeSchema, - activeCapabilities, - selectedDatabases, - activeDriver, - connections, - } = useDatabase(); + const { activeSchema, activeCapabilities, selectedDatabases, activeDriver } = + useDatabase(); const isMultiDb = usesMultiDatabaseLayout(activeCapabilities, selectedDatabases); const effectiveSchema = tab.schema || activeSchema || (isMultiDb ? selectedDatabases[0] : null); @@ -109,18 +99,13 @@ export function NotebookView({ enabled: isActive, }); const { settings } = useSettings(); - const hasProductionConnection = isProductionConnection( - connections, - connectionId, - ); const { showAlert } = useAlert(); const { matchesShortcut } = useKeybindings(); const { pending: dangerousQuery, - guardQuery: guardDangerousQuery, + guardQuery: guardQueryExecution, resolve: resolveDangerousQuery, - } = useDangerousQueryGuard(!hasProductionConnection); - const guardProductionWrite = useProductionGuard(); + } = useQueryGuards(connectionId); // Local notebook state — loaded from store/disk, NOT from tab const [notebook, setNotebook] = useState(() => @@ -380,10 +365,7 @@ export function NotebookView({ return; } - const mayRun = await passQueryGuards({ - guardProduction: () => guardProductionWrite(connectionId, resolvedSql), - guardDangerousQuery: () => guardDangerousQuery(resolvedSql), - }); + const mayRun = await guardQueryExecution(resolvedSql); if (!mayRun) { updateCell(cellId, { isLoading: false }); return; @@ -443,8 +425,7 @@ export function NotebookView({ updateCell, params, activeDriver, - guardDangerousQuery, - guardProductionWrite, + guardQueryExecution, ], ); diff --git a/src/hooks/useQueryGuards.ts b/src/hooks/useQueryGuards.ts new file mode 100644 index 000000000..1a4233157 --- /dev/null +++ b/src/hooks/useQueryGuards.ts @@ -0,0 +1,52 @@ +import { useCallback } from "react"; +import { useDatabase } from "./useDatabase"; +import { + useDangerousQueryGuard, + type DangerousQueryInfo, +} from "./useDangerousQueryGuard"; +import { useProductionGuard } from "./useProductionGuard"; +import { isProductionConnection } from "../utils/environment"; +import { passQueryGuards } from "../utils/queryGuard"; + +interface QueryGuards { + pending: DangerousQueryInfo | null; + isPending: boolean; + guardQuery: (sqlOrQueries: string | string[]) => Promise; + resolve: (confirmed: boolean) => void; +} + +/** + * Composes production and dangerous-query confirmations into one ordered gate. + * On production connections, the production prompt replaces the standard + * dangerous-query prompt so a write never displays two confirmations. + */ +export function useQueryGuards( + connectionId: string | null | undefined, +): QueryGuards { + const { connections } = useDatabase(); + const isProduction = isProductionConnection(connections, connectionId); + const { + pending, + isPending, + guardQuery: guardDangerousQuery, + resolve, + } = useDangerousQueryGuard(!isProduction); + const guardProductionWrite = useProductionGuard(); + + const guardQuery = useCallback( + (sqlOrQueries: string | string[]) => { + const productionSql = Array.isArray(sqlOrQueries) + ? sqlOrQueries.join(";\n") + : sqlOrQueries; + + return passQueryGuards({ + guardProduction: () => + guardProductionWrite(connectionId, productionSql), + guardDangerousQuery: () => guardDangerousQuery(sqlOrQueries), + }); + }, + [connectionId, guardDangerousQuery, guardProductionWrite], + ); + + return { pending, isPending, guardQuery, resolve }; +} diff --git a/src/pages/Editor.tsx b/src/pages/Editor.tsx index 8aa4b43a2..649798a4b 100644 --- a/src/pages/Editor.tsx +++ b/src/pages/Editor.tsx @@ -15,14 +15,10 @@ import { usesMultiDatabaseLayout, } from "../utils/database"; import { isReadonly, supportsExplain } from "../utils/driverCapabilities"; -import { isProductionConnection } from "../utils/environment"; -import { passQueryGuards } from "../utils/queryGuard"; import { useClickOutside } from "../hooks/useClickOutside"; -import { - useDangerousQueryGuard, - DANGEROUS_QUERY_I18N, -} from "../hooks/useDangerousQueryGuard"; +import { DANGEROUS_QUERY_I18N } from "../hooks/useDangerousQueryGuard"; import { useProductionGuard } from "../hooks/useProductionGuard"; +import { useQueryGuards } from "../hooks/useQueryGuards"; import { generateTempId, initializeNewRow, @@ -238,10 +234,6 @@ export const Editor = ({ commandScopeId }: EditorProps) => { const driverReadonly = isReadonly(activeCapabilities); const driverSupportsExplain = supportsExplain(activeCapabilities); const activeDialect = activeCapabilities?.sql_dialect; - const hasProductionConnection = isProductionConnection( - connections, - activeConnectionId, - ); // Editor panes stay mounted (hidden with display:none) so Monaco never // remounts. Render them sorted by id, decoupled from the tab-strip order: @@ -417,9 +409,9 @@ export const Editor = ({ commandScopeId }: EditorProps) => { useState(false); const { pending: dangerousQuery, - guardQuery: guardDangerousQuery, + guardQuery: guardQueryExecution, resolve: resolveDangerousQuery, - } = useDangerousQueryGuard(!hasProductionConnection); + } = useQueryGuards(activeConnectionId); const guardProductionWrite = useProductionGuard(); const [isTabSwitcherOpen, setIsTabSwitcherOpen] = useState(false); const [isRunDropdownOpen, setIsRunDropdownOpen] = useState(false); @@ -935,11 +927,7 @@ export const Editor = ({ commandScopeId }: EditorProps) => { if (!textToRun || !textToRun.trim()) return; - const mayRun = await passQueryGuards({ - guardProduction: () => - guardProductionWrite(activeConnectionId, textToRun), - guardDangerousQuery: () => guardDangerousQuery(textToRun), - }); + const mayRun = await guardQueryExecution(textToRun); if (!mayRun) return; // Check for parameters @@ -1167,8 +1155,7 @@ export const Editor = ({ commandScopeId }: EditorProps) => { isMultiDb, activeDatabaseName, addHistoryEntry, - guardDangerousQuery, - guardProductionWrite, + guardQueryExecution, activeDialect, ], ); @@ -1181,11 +1168,7 @@ export const Editor = ({ commandScopeId }: EditorProps) => { const targetTab = tabsRef.current.find((t) => t.id === targetTabId); if (!targetTab) return; - const mayRun = await passQueryGuards({ - guardProduction: () => - guardProductionWrite(activeConnectionId, queries.join(";\n")), - guardDangerousQuery: () => guardDangerousQuery(queries), - }); + const mayRun = await guardQueryExecution(queries); if (!mayRun) return; // Collect all unique parameters across all queries @@ -1370,8 +1353,7 @@ export const Editor = ({ commandScopeId }: EditorProps) => { isMultiDb, activeDatabaseName, addHistoryEntry, - guardDangerousQuery, - guardProductionWrite, + guardQueryExecution, activeDialect, ], ); diff --git a/tests/hooks/useQueryGuards.test.ts b/tests/hooks/useQueryGuards.test.ts new file mode 100644 index 000000000..290aa84be --- /dev/null +++ b/tests/hooks/useQueryGuards.test.ts @@ -0,0 +1,122 @@ +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { useQueryGuards } from "../../src/hooks/useQueryGuards"; + +const guardState = vi.hoisted(() => ({ + connections: [] as Array<{ + id: string; + environment?: "development" | "staging" | "production"; + }>, + guardProduction: vi.fn( + async ( + _connectionId: string | null | undefined, + _sql?: string, + ): Promise => true, + ), +})); + +vi.mock("../../src/hooks/useDatabase", () => ({ + useDatabase: () => ({ connections: guardState.connections }), +})); + +vi.mock("../../src/hooks/useProductionGuard", () => ({ + useProductionGuard: () => guardState.guardProduction, +})); + +describe("useQueryGuards", () => { + beforeEach(() => { + guardState.connections = []; + guardState.guardProduction.mockReset(); + guardState.guardProduction.mockResolvedValue(true); + }); + + it("runs the production guard before opening a dangerous-query prompt", async () => { + guardState.connections = [ + { id: "dev-id", environment: "development" }, + ]; + const { result } = renderHook(() => useQueryGuards("dev-id")); + + let confirmation!: Promise; + await act(async () => { + confirmation = result.current.guardQuery("DROP TABLE users"); + await Promise.resolve(); + }); + + expect(guardState.guardProduction).toHaveBeenCalledWith( + "dev-id", + "DROP TABLE users", + ); + expect(result.current.pending?.kind).toBe("drop"); + + let allowed: boolean | undefined; + await act(async () => { + result.current.resolve(false); + allowed = await confirmation; + }); + expect(allowed).toBe(false); + }); + + it("stops before the dangerous-query guard when production blocks", async () => { + guardState.connections = [ + { id: "prod-id", environment: "production" }, + ]; + guardState.guardProduction.mockResolvedValue(false); + const { result } = renderHook(() => useQueryGuards("prod-id")); + + let allowed: boolean | undefined; + await act(async () => { + allowed = await result.current.guardQuery("DROP TABLE users"); + }); + + expect(allowed).toBe(false); + expect(result.current.pending).toBeNull(); + }); + + it("uses only the production guard for dangerous production queries", async () => { + guardState.connections = [ + { id: "prod-id", environment: "production" }, + ]; + const { result } = renderHook(() => useQueryGuards("prod-id")); + + let allowed: boolean | undefined; + await act(async () => { + allowed = await result.current.guardQuery("DELETE FROM users"); + }); + + expect(allowed).toBe(true); + expect(guardState.guardProduction).toHaveBeenCalledWith( + "prod-id", + "DELETE FROM users", + ); + expect(result.current.pending).toBeNull(); + }); + + it("serializes batch SQL for production while preserving statements for danger checks", async () => { + guardState.connections = [ + { id: "dev-id", environment: "development" }, + ]; + const { result } = renderHook(() => useQueryGuards("dev-id")); + const queries = ["SELECT 1", "DROP TABLE users"]; + + let confirmation!: Promise; + await act(async () => { + confirmation = result.current.guardQuery(queries); + await Promise.resolve(); + }); + + expect(guardState.guardProduction).toHaveBeenCalledWith( + "dev-id", + "SELECT 1;\nDROP TABLE users", + ); + expect(result.current.pending).toMatchObject({ + kind: "drop", + sql: "DROP TABLE users", + count: 1, + }); + + await act(async () => { + result.current.resolve(true); + await confirmation; + }); + }); +});