diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index b5ae800..a205496 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -45,9 +45,13 @@ jobs: - name: Resolve version id: v + # The input reaches the script as an env var, never spliced into it, + # so a crafted version string can't run as shell. + env: + INPUT_VERSION: ${{ inputs.version }} run: | if [ "${{ github.event_name }}" = "workflow_dispatch" ]; then - VERSION="${{ inputs.version }}" + VERSION="$INPUT_VERSION" DRAFT=true else VERSION="${GITHUB_REF_NAME#v}" @@ -71,6 +75,11 @@ jobs: - run: flutter pub get + # A tag can point at any commit, including one CI never saw green. + # Nothing ships unless the same checks pass on exactly what is built. + - run: flutter analyze + - run: flutter test + - name: Build run: | flutter build macos --release \ diff --git a/.github/workflows/web.yml b/.github/workflows/web.yml index 68f9ccd..a219ce8 100644 --- a/.github/workflows/web.yml +++ b/.github/workflows/web.yml @@ -55,6 +55,11 @@ jobs: - run: flutter pub get + # This deploy runs beside CI, not after it, so it gates itself: a push + # that breaks analyze or the tests never reaches production. + - run: flutter analyze + - run: flutter test + # Each deploy keeps the pubspec version and bumps the build number, so # open tabs see a strictly newer version.json and offer "Refresh" # (lib/update/update_service.dart). The build lives under /b// so diff --git a/CHANGELOG.md b/CHANGELOG.md index 84e6f13..1659556 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ ## 2026-11-03 +- File ▸ Show crash log opens the folder with dbscope.log for bug reports (desktop) (2026-11-03) +- dbscope_pg_proxy can be pinned to one database with --allow-db-host, and caps each result at --max-rows rows (default 100000, flagged truncated) (2026-11-03) +- Releases and web deploys run analyze and the tests on exactly the commit they ship (2026-11-03) +- The web app is served with clickjacking, MIME-sniffing and HSTS security headers (2026-11-03) +- macOS self-update checks that the download is signed by the same Developer ID team as the running app, and gives up on a stalled download (2026-11-03) +- Exported connection links need a passphrase of at least 10 characters (2026-11-03) +- New connections to a database on another machine default to SSL mode require (2026-11-03) +- dbscope_pg_proxy now accepts WebSocket upgrades only from the hosted web app and localhost pages by default (--allow-origin replaces the list, --allow-any-origin turns the check off), so other web pages can't use it to reach your databases (2026-11-03) +- Fixed: one malformed message could crash dbscope_pg_proxy for every client (2026-11-03) +- Fixed: read-only mode was not enforced on PG Proxy connections; the database now refuses writes as it does on direct connections (2026-11-03) +- Fixed: Quit could hang when a database connection had gone away (2026-11-03) +- Fixed: a saved tab or connection that could not be read could stop dbscope from starting or hide every saved connection (2026-11-03) +- Uncaught errors are now written to ~/Library/Logs/dbscope/dbscope.log, and a widget that fails to draw shows a short message instead of a grey box (2026-11-03) - Cells in point, line, lseg, box, path, polygon and circle columns can be edited on Postgres; the save used to fail every time, and the editor now starts from Postgres syntax like (1,2) (2026-11-03) - SQLite reports 0 rows affected for DDL instead of the count from the previous write (2026-11-03) - DBSCOPE_AUTO_READ_ONLY (and DBSCOPE_AUTO_ASSIGN_IP) accept 1/0, yes/no, on/off in any case; =1 used to open the connection read-write (2026-11-03) diff --git a/TODO.md b/TODO.md index 5b75969..b55ce88 100644 --- a/TODO.md +++ b/TODO.md @@ -42,6 +42,22 @@ rg -n '\[SIMPLE\]' TODO.md +### **[P2]** Find out whether the release build still needs `disable-library-validation` + +- **Where.** `macos/Runner/Release.entitlements` (`com.apple.security.cs.disable-library-validation`) +- **Why.** With hardened runtime it lets the app load libraries signed by anyone, which undoes much of what signing buys. The release workflow signs every bundled dylib with our own identity, so it may no longer be needed; removing it needs a signed build launched on a Mac (sqlite3 and the plugin frameworks are the libraries to watch). +- **Effort.** `<1h` +- **Blocker.** needs a signed build and a Mac (see "Sign and notarize the macOS release" in `MANUAL_TODO.md`). +- **Noticed.** 2026-11-03, while auditing for production readiness. + +### **[P2]** Undo the Cloud SQL public IP after a crash + +- **Where.** `lib/db/cloudsql/cloudsql_proxy_manager.dart` +- **Why.** When the manager assigned a public IP, it removes it on the last release. If dbscope crashes or is force-quit first, the instance keeps the public IP (and the orphaned `cloud-sql-proxy` keeps running) until someone notices. Recording "we assigned it" on disk and offering to undo it on the next launch would close that. +- **Effort.** `<4h` +- **Blocker.** none +- **Noticed.** 2026-11-03, while auditing for production readiness. + ### **[P2]** Make Tab move to the next cell, and let multi-line cells take a newline, in grid inline editing - **Where.** `packages/data_grid/lib/src/internal/grid_cell.dart` (`_editor` shortcut bindings) diff --git a/docs/architecture.md b/docs/architecture.md index ca3c617..4f2f4ab 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -295,5 +295,6 @@ The driver (`postgres ^3.5.11`) does not constrain the floor further in practice ## Other notes +- **Uncaught errors and quitting:** `installErrorHandlers` (`lib/util/error_reporting.dart`, first call in `main` for every isolate) sends framework errors and uncaught async errors to the console and, on desktop, to `~/Library/Logs/dbscope/dbscope.log` (rotated at 1 MB; `/dbscope/` off macOS; File ▸ Show crash log opens the folder). Uncaught async errors are marked handled so one failed background task doesn't kill the window; release builds show a small placeholder instead of a grey box for a widget that threw. Each quit step (save tabs, close connections, stop the backend) is capped at 3 s and logged on failure, so a dead database socket can't hang Quit. Anything restored before the first frame (saved tabs, saved profiles) skips an entry it can't read instead of throwing; `ProfileStore` writes unreadable profile entries back unchanged on save. - **Table-open latency:** `TableLoader.loadTable` (`lib/query/runner.dart`) fires `listColumns`, `primaryKey`, `indexedColumns` together via `Future.wait` (the driver pipelines them); only the first-page `fetchNext` blocks on the catalog round. `TableTabView` also keeps a single-slot prefetch (`_nextPagePrefetch` / `_kickPrefetch`) so the next page is buffered before the user scrolls to it. The pager is not safe for concurrent `fetchNext` — the slot enforces one in-flight at a time, and `_load` / errors clear it. - **Relationship gutter:** `lib/shell/relationship_gutter.dart` routes each FK edge in its own vertical lane keyed by sorted index (so an edge's lane and color are stable across reloads). Adjust `laneSpacing` / `laneMin` / `cornerRadius` together — the lane count is derived from the available width up to `anchorX - cornerRadius`. diff --git a/firebase.json b/firebase.json index 5528c5f..63574dd 100644 --- a/firebase.json +++ b/firebase.json @@ -1 +1 @@ -{"hosting":{"public":"build/web","ignore":["firebase.json","**/.*","**/node_modules/**"],"rewrites":[{"source":"**","destination":"/index.html"}],"headers":[{"source":"**","headers":[{"key":"Cache-Control","value":"no-cache"}]},{"source":"/b/**","headers":[{"key":"Cache-Control","value":"public, max-age=31536000, immutable"}]},{"source":"/index.html","headers":[{"key":"Cache-Control","value":"no-cache, no-store, must-revalidate"}]}]},"flutter":{"platforms":{"dart":{"lib/firebase_options.dart":{"projectId":"dbscope-prod","configurations":{"web":"1:593558037618:web:eb7caea728dd21ba29ceeb"}}}}}} \ No newline at end of file +{"hosting":{"public":"build/web","ignore":["firebase.json","**/.*","**/node_modules/**"],"rewrites":[{"source":"**","destination":"/index.html"}],"headers":[{"source":"**","headers":[{"key":"X-Content-Type-Options","value":"nosniff"},{"key":"X-Frame-Options","value":"DENY"},{"key":"Content-Security-Policy","value":"frame-ancestors 'none'"},{"key":"Referrer-Policy","value":"strict-origin-when-cross-origin"},{"key":"Strict-Transport-Security","value":"max-age=31536000"},{"key":"Permissions-Policy","value":"camera=(), microphone=(), geolocation=(), payment=(), usb=()"}]},{"source":"**","headers":[{"key":"Cache-Control","value":"no-cache"}]},{"source":"/b/**","headers":[{"key":"Cache-Control","value":"public, max-age=31536000, immutable"}]},{"source":"/index.html","headers":[{"key":"Cache-Control","value":"no-cache, no-store, must-revalidate"}]}]},"flutter":{"platforms":{"dart":{"lib/firebase_options.dart":{"projectId":"dbscope-prod","configurations":{"web":"1:593558037618:web:eb7caea728dd21ba29ceeb"}}}}}} diff --git a/lib/connection/connection_dialog.dart b/lib/connection/connection_dialog.dart index b24a509..6c815d5 100644 --- a/lib/connection/connection_dialog.dart +++ b/lib/connection/connection_dialog.dart @@ -31,6 +31,10 @@ class _ConnectionDialogState extends State { /// touch it on a new profile, it follows [suggestReadOnly] as they type /// the name and host, and says why. var _readOnlyChosen = false; + + /// Same as [_readOnlyChosen], for the SSL mode: until the user picks one + /// on a new profile, it follows [suggestSslMode] as they type the host. + var _sslChosen = false; var _passwordVisible = false; var _forgetPassword = false; @@ -48,7 +52,8 @@ class _ConnectionDialogState extends State { _tenantValue = TextEditingController( text: i?.tenantFilter?.value?.toString() ?? '', ); - _sslMode = i?.sslMode ?? SslMode.disable; + _sslMode = i?.sslMode ?? suggestSslMode(_host.text); + _sslChosen = i != null; _readOnlyChosen = i != null; _readOnly = i?.readOnly ?? _suggestion.readOnly; _name.addListener(_followSuggestion); @@ -59,9 +64,13 @@ class _ConnectionDialogState extends State { suggestReadOnly(name: _name.text, host: _host.text); void _followSuggestion() { - if (_readOnlyChosen) return; - final suggested = _suggestion.readOnly; - if (suggested != _readOnly) setState(() => _readOnly = suggested); + final readOnly = _readOnlyChosen ? _readOnly : _suggestion.readOnly; + final ssl = _sslChosen ? _sslMode : suggestSslMode(_host.text); + if (readOnly == _readOnly && ssl == _sslMode) return; + setState(() { + _readOnly = readOnly; + _sslMode = ssl; + }); } @override @@ -199,6 +208,9 @@ class _ConnectionDialogState extends State { ), const SizedBox(height: 12.0), DropdownButtonFormField( + // Keyed on the value so a suggestion change re-seeds the field + // (`initialValue` is read only once per field state). + key: ValueKey(_sslMode), initialValue: _sslMode, decoration: const InputDecoration(labelText: 'SSL mode'), items: const [ @@ -215,7 +227,10 @@ class _ConnectionDialogState extends State { child: Text('verify-full'), ), ], - onChanged: (mode) => setState(() => _sslMode = mode ?? SslMode.disable), + onChanged: (mode) => setState(() { + _sslMode = mode ?? _sslMode; + _sslChosen = true; + }), ), const SizedBox(height: 12.0), SwitchListTile( diff --git a/lib/connection/connection_link_flow.dart b/lib/connection/connection_link_flow.dart index cfc5d51..814e22c 100644 --- a/lib/connection/connection_link_flow.dart +++ b/lib/connection/connection_link_flow.dart @@ -227,7 +227,17 @@ class _PassphraseDialogState extends State<_PassphraseDialog> { ), ), ), - validator: (text) => (text == null || text.isEmpty) ? 'required' : null, + validator: (text) { + if (text == null || text.isEmpty) return 'required'; + // Only when setting one: the file carries the database + // password, and anyone holding it can guess offline as long + // as they like. Importing must still accept whatever an + // older build exported. + if (widget.confirm && text.length < _kMinPassphraseLength) { + return 'use at least $_kMinPassphraseLength characters'; + } + return null; + }, onFieldSubmitted: (_) { if (!widget.confirm) _submit(); }, @@ -249,3 +259,6 @@ class _PassphraseDialogState extends State<_PassphraseDialog> { ); } } + +/// Shortest passphrase an exported connection link accepts. +const _kMinPassphraseLength = 10; diff --git a/lib/connection/profile.dart b/lib/connection/profile.dart index 851b0c4..a326f9c 100644 --- a/lib/connection/profile.dart +++ b/lib/connection/profile.dart @@ -26,13 +26,25 @@ final readOnlyHeuristic = RegExp( if (match != null) { return (readOnly: true, reason: 'the name says "${match.group(2)!.toLowerCase()}"'); } - final h = host.trim().toLowerCase(); - if (h.isEmpty || h == 'localhost' || h == '::1' || h.startsWith('127.')) { + if (isLocalHost(host)) { return (readOnly: false, reason: 'the database is on this machine'); } return (readOnly: true, reason: 'the database is on another server'); } +/// Whether [host] is this machine (blank counts: the driver's default is +/// localhost). +bool isLocalHost(String host) { + final h = host.trim().toLowerCase(); + return h.isEmpty || h == 'localhost' || h == '::1' || h.startsWith('127.'); +} + +/// The SSL default the connection dialog proposes for a new profile: +/// `disable` for a database on this machine (a local Postgres usually has no +/// certificate), `require` for any other server, so the password and every +/// row never cross a network in the clear unless the user asks for that. +SslMode suggestSslMode(String host) => isLocalHost(host) ? SslMode.disable : SslMode.require; + /// Built-in kind id for the direct-Postgres backend. External plugins /// pick their own. const kPostgresKind = 'postgres'; diff --git a/lib/connection/profile_store.dart b/lib/connection/profile_store.dart index 58e67c8..e5865a9 100644 --- a/lib/connection/profile_store.dart +++ b/lib/connection/profile_store.dart @@ -1,3 +1,5 @@ +import 'package:flutter/foundation.dart'; + import '../project/project_store.dart'; import 'profile.dart'; import 'secure_store.dart'; @@ -9,21 +11,39 @@ class ProfileStore { ProfileStore._(); static final instance = ProfileStore._(); + /// Saved entries this build can't parse (hand-edited, or written by a + /// newer build). They are left out of [load] and written back unchanged by + /// [save], so one bad entry neither hides every other profile nor gets + /// silently deleted by the next save. + final List> _unreadable = []; + Future> load() async { final projectDocument = await ProjectStore.instance.load(); final out = []; + _unreadable.clear(); for (final entry in projectDocument.profiles) { - final id = entry['id'] as String?; - if (id == null) continue; - final password = await SecureStore.instance.readPassword(id) ?? ''; - out.add(ConnectionProfile.fromJson(entry, password)); + final id = entry['id']; + if (id is! String) { + _unreadable.add(entry); + continue; + } + try { + final password = await SecureStore.instance.readPassword(id) ?? ''; + out.add(ConnectionProfile.fromJson(entry, password)); + } catch (e) { + debugPrint('Skipped saved connection "$id": it could not be read ($e).'); + _unreadable.add(entry); + } } return out; } Future save(List profiles) async { + final ids = {for (final profile in profiles) profile.id}; await ProjectStore.instance.setProfiles([ for (final profile in profiles) profile.toJson(), + for (final entry in _unreadable) + if (!ids.contains(entry['id'])) entry, ]); for (final profile in profiles) { if (profile.password.isNotEmpty) { diff --git a/lib/main.dart b/lib/main.dart index 347dd00..15aefad 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -29,6 +29,7 @@ import 'server/host.dart'; import 'shell/app_state.dart'; import 'shell/ui_prefs.dart'; import 'update/update_service.dart'; +import 'util/error_reporting.dart'; import 'util/shell_env.dart'; // Entrypoint for both the host (first) window and any subwindows spawned @@ -46,6 +47,11 @@ import 'util/shell_env.dart'; // set (no Postgres-direct, no local SQLite). Future main(List args) async { WidgetsFlutterBinding.ensureInitialized(); + installErrorHandlers( + isolateLabel: !kIsWeb && args.isNotEmpty && args.first == 'multi_window' + ? 'window ${args.length > 1 ? args[1] : '?'}' + : 'host', + ); // Firebase is wired for the web build only. firebase_options.dart is // configured for `web` (project dbscope-prod); desktop targets throw // UnsupportedError from DefaultFirebaseOptions, so the init is gated on @@ -181,13 +187,28 @@ class _HostQuitHook extends WidgetsBindingObserver { @override Future didRequestAppExit() async { - await AppState.instance.flushTabs(); - await ConnectionRegistry.instance.disconnectAll(); - await host.dispose(); + await _quitStep('save open tabs', AppState.instance.flushTabs); + await _quitStep('close connections', ConnectionRegistry.instance.disconnectAll); + await _quitStep('stop the backend', host.dispose); return ui.AppExitResponse.exit; } } +/// Longest one quit step may take. Closing a connection to a database that +/// went away can wait on a dead socket, and quitting must never hang on it. +const _kQuitStepTimeout = Duration(seconds: 3); + +/// Runs one cleanup step on quit. A step that throws or hangs is logged and +/// skipped, so the app always quits. +Future _quitStep(String name, Future Function() step) async { + try { + await step().timeout(_kQuitStepTimeout); + } catch (e, st) { + debugPrint('Quit: could not $name: $e'); + recordError(e, st, context: 'quit: $name'); + } +} + // Universal plugin registration — one shape for every contribution. // // Every entry here implements [DbscopePlugin]. Its `register(host)` @@ -227,7 +248,7 @@ void _registerPlugins() { class _SubwindowQuitHook extends WidgetsBindingObserver { @override Future didRequestAppExit() async { - await ConnectionRegistry.instance.disconnectAll(); + await _quitStep('close connections', ConnectionRegistry.instance.disconnectAll); return ui.AppExitResponse.exit; } } diff --git a/lib/plugins/builtin_plugin.dart b/lib/plugins/builtin_plugin.dart index a75143a..773fad4 100644 --- a/lib/plugins/builtin_plugin.dart +++ b/lib/plugins/builtin_plugin.dart @@ -1,4 +1,5 @@ import 'package:ai_broker/ai_broker.dart'; +import 'package:flutter/foundation.dart' show kIsWeb; import '../jobs/job_tab_kind.dart'; import '../json_viewer/json_tab_kind.dart' show jsonTabKind; @@ -6,6 +7,7 @@ import '../linked/linked_tab_kind.dart'; import '../navigator/navigator_tab_kind.dart'; import '../query/query_tab_kind.dart'; import '../schema/schema_tab_kind.dart'; +import '../shell/crash_log_menu.dart'; import '../shell/sidebar_table_actions_default.dart'; import '../table/table_tab_kind.dart'; import 'dbscope_plugin.dart'; @@ -23,6 +25,7 @@ import 'dbscope_plugin.dart'; /// another DB) /// - the three packaged AI brokers (OpenAI, Anthropic, Gemini) from /// `package:ai_broker` +/// - File ▸ Show crash log, on desktop class BuiltinPlugin extends DbscopePlugin { @override String get id => 'builtin'; @@ -45,6 +48,9 @@ class BuiltinPlugin extends DbscopePlugin { // Sidebar table-row actions. registerDefaultSidebarTableActions(); + // File ▸ Show crash log (desktop: the web build has no log file). + if (!kIsWeb) host.menus.register(crashLogMenuContribution); + // AI brokers. host.aiBrokers ..register(OpenAiBroker()) diff --git a/lib/shell/app_state.dart b/lib/shell/app_state.dart index b95d360..9473920 100644 --- a/lib/shell/app_state.dart +++ b/lib/shell/app_state.dart @@ -163,19 +163,27 @@ class AppState extends ChangeNotifier { final restored = []; for (final entry in list) { if (entry is! Map) continue; - final kindId = entry['kind'] as String?; - if (kindId == null) continue; - final kind = registry.lookupById(kindId); - final tab = kind?.deserialize?.call(entry); - if (tab == null) continue; - tab.pinned = entry['pinned'] as bool? ?? false; - restored.add(tab); + // This runs before the first frame, so one entry a newer or older + // build wrote in a shape this one can't read must cost that tab, not + // the launch: a throw here would fail every start from now on. + try { + final kindId = entry['kind']; + if (kindId is! String) continue; + final kind = registry.lookupById(kindId); + final tab = kind?.deserialize?.call(entry); + if (tab == null) continue; + tab.pinned = entry['pinned'] == true; + restored.add(tab); + } catch (e) { + debugPrint('Skipped a saved tab that could not be restored: $e'); + continue; + } if (restored.length >= maxTabs) break; } if (restored.isEmpty) return; tabs.addAll(restored); - final activeId = decoded['activeId'] as String?; - if (activeId != null) { + final activeId = decoded['activeId']; + if (activeId is String) { for (final tab in restored) { if (tab.id == activeId) { _active = tab; diff --git a/lib/shell/crash_log_menu.dart b/lib/shell/crash_log_menu.dart new file mode 100644 index 0000000..ba55916 --- /dev/null +++ b/lib/shell/crash_log_menu.dart @@ -0,0 +1,27 @@ +import 'package:flutter/material.dart'; +import 'package:url_launcher/url_launcher.dart'; + +import '../util/error_reporting.dart'; +import 'menu_spec.dart'; + +/// File ▸ Show crash log: opens the folder `installErrorHandlers` writes to, +/// so a user can attach `dbscope.log` to a bug report. Desktop only; the web +/// build logs to the browser console instead. +final crashLogMenuContribution = MenuContribution( + menu: 'File', + build: (context) => MenuItemSpec( + label: 'Show crash log', + onSelected: () => _showCrashLog(context), + ), +); + +Future _showCrashLog(BuildContext context) async { + final messenger = ScaffoldMessenger.maybeOf(context); + final folder = crashLogFolder(); + final opened = folder != null && await launchUrl(Uri.directory(folder)); + if (!opened) { + messenger?.showSnackBar( + SnackBar(content: Text('Could not open ${folder ?? 'the crash log folder'}.')), + ); + } +} diff --git a/lib/update/update_installer_io.dart b/lib/update/update_installer_io.dart index 5ef1b31..bc69749 100644 --- a/lib/update/update_installer_io.dart +++ b/lib/update/update_installer_io.dart @@ -45,6 +45,7 @@ Future installMacosUpdate(MacosRelease release, void Function(String stage if (runningId != newId) { throw UpdateInstallException('The download is a different app ($newId).'); } + await _checkSignedBySameTeam(appBundle, newBundle); onStage('Installing… dbscope will restart.'); final script = File('${work.path}/swap.sh'); @@ -82,8 +83,49 @@ void _checkCanReplace(Directory appBundle) { } } +/// The checksum and the zip come from the same release, so they only catch a +/// broken download, not a tampered release. A signed build therefore also +/// insists the new bundle carries a valid signature from the same Developer +/// ID team, which nobody holding only the GitHub repo can produce. An +/// unsigned build (no Apple secrets in CI) has no team to compare against. +Future _checkSignedBySameTeam(Directory running, Directory downloaded) async { + final team = await _teamId(running); + if (team == null) return; + final verify = await Process.run('/usr/bin/codesign', [ + '--verify', + '--deep', + '--strict', + downloaded.path, + ]); + if (verify.exitCode != 0) { + throw UpdateInstallException('The download is not validly signed: ${verify.stderr}'.trim()); + } + final newTeam = await _teamId(downloaded); + if (newTeam != team) { + throw UpdateInstallException( + 'The download is signed by ${newTeam ?? 'nobody'}, not by $team, the signer of this copy.', + ); + } +} + +/// The Developer ID team that signed [bundle], or null when it is unsigned +/// or ad-hoc signed. +Future _teamId(Directory bundle) async { + final result = await Process.run('/usr/bin/codesign', ['-dv', '--verbose=2', bundle.path]); + if (result.exitCode != 0) return null; + // codesign prints the details on stderr. + final match = RegExp(r'^TeamIdentifier=(\S+)$', multiLine: true).firstMatch('${result.stderr}'); + final id = match?.group(1); + return id == null || id == 'not set' ? null : id; +} + +/// Bounds for the two downloads, so a stalled connection fails the install +/// with a message instead of leaving "Downloading…" up forever. +const _kMetadataTimeout = Duration(seconds: 30); +const _kZipTimeout = Duration(minutes: 10); + Future _get(Uri url) async { - final response = await http.get(url); + final response = await http.get(url).timeout(_kMetadataTimeout); if (response.statusCode != 200) { throw UpdateInstallException('Download failed (HTTP ${response.statusCode}).'); } @@ -93,11 +135,15 @@ Future _get(Uri url) async { Future _download(Uri url, File to) async { final client = http.Client(); try { - final response = await client.send(http.Request('GET', url)); - if (response.statusCode != 200) { - throw UpdateInstallException('Download failed (HTTP ${response.statusCode}).'); + Future fetch() async { + final response = await client.send(http.Request('GET', url)); + if (response.statusCode != 200) { + throw UpdateInstallException('Download failed (HTTP ${response.statusCode}).'); + } + await response.stream.pipe(to.openWrite()); } - await response.stream.pipe(to.openWrite()); + + await fetch().timeout(_kZipTimeout); } finally { client.close(); } diff --git a/lib/util/crash_log.dart b/lib/util/crash_log.dart new file mode 100644 index 0000000..8bf5418 --- /dev/null +++ b/lib/util/crash_log.dart @@ -0,0 +1,2 @@ +// Conditional export: a log file on desktop, the browser console on web. +export 'crash_log_io.dart' if (dart.library.js_interop) 'crash_log_web.dart'; diff --git a/lib/util/crash_log_io.dart b/lib/util/crash_log_io.dart new file mode 100644 index 0000000..9eda60c --- /dev/null +++ b/lib/util/crash_log_io.dart @@ -0,0 +1,45 @@ +import 'dart:io'; + +/// Rotate once the log passes this size, keeping one previous file, so a +/// crash loop can't fill the disk. +const int _kMaxBytes = 1024 * 1024; + +String? _cachedPath; + +/// `~/Library/Logs/dbscope/dbscope.log` on macOS (where Console.app looks), +/// `/dbscope/dbscope.log` elsewhere. +String? crashLogPath() { + if (_cachedPath != null) return _cachedPath; + final home = Platform.environment['HOME']; + final dir = Platform.isMacOS && home != null && home.isNotEmpty + ? '$home/Library/Logs/dbscope' + : '${Directory.systemTemp.path}/dbscope'; + return _cachedPath = '$dir/dbscope.log'; +} + +/// Appends [entry] to the log. Never throws: failing to log must not turn +/// one error into two. +void appendCrashLog(String entry) { + final path = crashLogPath(); + if (path == null) return; + try { + final file = File(path); + file.parent.createSync(recursive: true); + if (file.existsSync() && file.lengthSync() > _kMaxBytes) { + file.renameSync('$path.1'); + } + file.writeAsStringSync(entry, mode: FileMode.append, flush: true); + } catch (_) {} +} + +/// The folder holding the log, created if missing so it can always be +/// opened, or null when it can't be created. +String? crashLogFolder() { + final path = crashLogPath(); + if (path == null) return null; + try { + return (File(path).parent..createSync(recursive: true)).path; + } catch (_) { + return null; + } +} diff --git a/lib/util/crash_log_web.dart b/lib/util/crash_log_web.dart new file mode 100644 index 0000000..f827ea2 --- /dev/null +++ b/lib/util/crash_log_web.dart @@ -0,0 +1,9 @@ +// Web: there is no file to write. The entry already went to the browser +// console through debugPrint, which is where a web user looks. + +/// Where [appendCrashLog] writes, or null when it writes nowhere. +String? crashLogPath() => null; + +void appendCrashLog(String entry) {} + +String? crashLogFolder() => null; diff --git a/lib/util/error_reporting.dart b/lib/util/error_reporting.dart new file mode 100644 index 0000000..ef2b203 --- /dev/null +++ b/lib/util/error_reporting.dart @@ -0,0 +1,77 @@ +import 'package:flutter/foundation.dart'; +import 'package:flutter/widgets.dart'; + +import 'crash_log.dart'; + +export 'crash_log.dart' show crashLogFolder, crashLogPath; + +/// Routes every error nothing else caught into one place: the console and, +/// on desktop, the crash log ([crashLogPath]). Call once per isolate, first +/// thing in `main`. +/// +/// Without this, an uncaught async error in a release build vanishes and a +/// widget that throws while building paints a blank grey box, so a user has +/// nothing to put in a bug report. +void installErrorHandlers({required String isolateLabel}) { + final presentError = FlutterError.onError; + FlutterError.onError = (details) { + presentError?.call(details); + recordError( + details.exception, + details.stack, + context: details.context?.toDescription() ?? 'Flutter framework', + isolateLabel: isolateLabel, + ); + }; + PlatformDispatcher.instance.onError = (error, stack) { + debugPrint('Uncaught error: $error\n$stack'); + recordError(error, stack, context: 'uncaught async error', isolateLabel: isolateLabel); + // Handled: keep the app running. One failed background task must not + // take every open tab down with it. + return true; + }; + if (kReleaseMode) { + ErrorWidget.builder = (details) => const _BrokenWidgetPlaceholder(); + } +} + +/// Writes one error to the crash log. Public so code that catches an error +/// it can't show anywhere can still leave a trace. +void recordError( + Object error, + StackTrace? stack, { + required String context, + String isolateLabel = '', +}) { + final when = DateTime.now().toIso8601String(); + final where = isolateLabel.isEmpty ? '' : ' [$isolateLabel]'; + appendCrashLog('--- $when$where $context\n$error\n${stack ?? ''}\n'); +} + +/// Shown in place of a widget that threw while building, in release builds. +/// Depends on nothing inherited (no Theme, no Directionality), because the +/// subtree it replaces may be the one that provides them. +class _BrokenWidgetPlaceholder extends StatelessWidget { + const _BrokenWidgetPlaceholder(); + + @override + Widget build(BuildContext context) { + final log = crashLogPath(); + return ColoredBox( + color: const Color(0x14FF0000), + child: Center( + child: Padding( + padding: const EdgeInsets.all(8.0), + child: Text( + log == null + ? 'Something went wrong showing this part of dbscope.' + : 'Something went wrong showing this part of dbscope. Details: $log', + textAlign: TextAlign.center, + textDirection: TextDirection.ltr, + style: const TextStyle(fontSize: 12.0, color: Color(0xFFCC4444)), + ), + ), + ), + ); + } +} diff --git a/packages/dbscope_pg_proxy/README.md b/packages/dbscope_pg_proxy/README.md index 59df00d..d36e04f 100644 --- a/packages/dbscope_pg_proxy/README.md +++ b/packages/dbscope_pg_proxy/README.md @@ -50,8 +50,20 @@ exactly like exposing Postgres itself. - **Require a shared secret.** Pass `--shared-secret SECRET`; the web client echoes it on every `open` message. Without one, any browser that can reach the proxy can connect to your DB. -- **Constrain the origin.** Pass `--allow-origin https://your.host` so - the proxy rejects WebSocket upgrades from other origins. +- **Constrain the origin.** Any web page you open can try a WebSocket to + `127.0.0.1`, so by default the proxy accepts upgrades only from the + hosted web app (`https://dbscope-prod.web.app`) and localhost pages. + If you serve dbscope from your own host, pass + `--allow-origin https://your.host` (repeatable); the list then replaces + the defaults. `--allow-any-origin` turns the check off: don't, unless + nothing but trusted pages can reach the proxy. +- **Pin the database.** A client names the host it wants in its `open` + message, so an exposed proxy can reach any Postgres its network can. + Pass `--allow-db-host db.internal:5432` (repeatable; a bare host allows + any port) to allow only that one. +- **Results are capped.** One result sends at most `--max-rows` rows + (default 100000); a longer one is cut and its reply says + `"truncated": true`. - **Use a least-privileged Postgres role.** The proxy never inspects or restricts the SQL it forwards. diff --git a/packages/dbscope_pg_proxy/bin/dbscope_pg_proxy.dart b/packages/dbscope_pg_proxy/bin/dbscope_pg_proxy.dart index 7a3a437..42afb4c 100644 --- a/packages/dbscope_pg_proxy/bin/dbscope_pg_proxy.dart +++ b/packages/dbscope_pg_proxy/bin/dbscope_pg_proxy.dart @@ -4,7 +4,7 @@ // dart run packages/dbscope_pg_proxy/bin/dbscope_pg_proxy.dart \ // --port 8765 \ // [--shared-secret SECRET] \ -// [--allow-origin https://your.web.host] +// [--allow-origin https://your.web.host] (repeatable) // // Wire protocol is JSON envelopes over a single WebSocket. One WebSocket // owns one Postgres connection for its lifetime. @@ -20,7 +20,7 @@ // server → client: // { "type": "ready" } // { "type": "result", "id": "...", "columns": [{name, typeName}], -// "rows": [[...]], "affected": 0 } +// "rows": [[...]], "affected": 0, "truncated"?: true } // { "type": "error", "id"?: "...", "code"?: "...", "message": "...", // "constraintName"?: "..." } // @@ -30,6 +30,12 @@ // (reverse proxy → wss://), and (c) set --shared-secret so clients // must echo it on `open`. Pair this with a least-privileged Postgres // role. +// +// Browsers let any web page open a WebSocket to 127.0.0.1, so by default +// the proxy only accepts upgrades whose Origin is the hosted dbscope web +// app or a localhost page (see `_kDefaultOrigins`), or exactly the +// `--allow-origin` list when one is given. Clients that send no +// Origin (scripts, curl) are not browsers and are let through. import 'dart:async'; import 'dart:convert'; @@ -47,43 +53,90 @@ Future main(List args) async { if (opts.sharedSecret != null) { stdout.writeln('shared-secret authentication required'); } - if (opts.allowOrigin != null) { - stdout.writeln('restricting Origin to: ${opts.allowOrigin}'); + if (opts.allowAnyOrigin) { + stdout.writeln('WARNING: accepting WebSocket upgrades from any Origin'); + } else if (opts.allowOrigins.isNotEmpty) { + stdout.writeln('accepting Origin: ${opts.allowOrigins.join(', ')}'); + } else { + stdout.writeln('accepting Origin: ${_kDefaultOrigins.join(', ')} and localhost pages'); } await for (final req in server) { - if (req.uri.path == '/health') { - req.response - ..statusCode = 200 - ..write('ok'); - await req.response.close(); - continue; - } - if (!WebSocketTransformer.isUpgradeRequest(req)) { - req.response - ..statusCode = 426 - ..headers.set('upgrade', 'websocket') - ..write('websocket required'); - await req.response.close(); - continue; - } - if (opts.allowOrigin != null) { - final origin = req.headers.value('origin'); - if (origin != opts.allowOrigin) { - req.response - ..statusCode = 403 - ..write('forbidden origin'); - await req.response.close(); - continue; - } + try { + await _route(req, opts); + } catch (e) { + // One bad request (a client that hung up mid-response, …) must not + // stop the accept loop for everyone else. + stderr.writeln('request failed: $e'); } - unawaited(_handleUpgrade(req, opts)); } } +Future _route(HttpRequest req, _Opts opts) async { + if (req.uri.path == '/health') { + req.response + ..statusCode = 200 + ..write('ok'); + await req.response.close(); + return; + } + if (!WebSocketTransformer.isUpgradeRequest(req)) { + req.response + ..statusCode = 426 + ..headers.set('upgrade', 'websocket') + ..write('websocket required'); + await req.response.close(); + return; + } + final origin = req.headers.value('origin'); + if (!_originAllowed(origin, opts)) { + stderr.writeln('rejected WebSocket upgrade from Origin $origin'); + req.response + ..statusCode = 403 + ..write('forbidden origin'); + await req.response.close(); + return; + } + unawaited(_handleUpgrade(req, opts)); +} + +/// Origins accepted when no `--allow-origin` is given: the hosted web app +/// (plus any localhost page, see [_originAllowed]). +const _kDefaultOrigins = { + 'https://dbscope-prod.web.app', + 'https://dbscope-prod.firebaseapp.com', +}; + +bool _originAllowed(String? origin, _Opts opts) { + if (opts.allowAnyOrigin || origin == null) return true; + // An explicit list replaces the defaults, so it can only narrow them. + if (opts.allowOrigins.isNotEmpty) return opts.allowOrigins.contains(origin); + if (_kDefaultOrigins.contains(origin)) return true; + final uri = Uri.tryParse(origin); + // A locally served web build (`flutter run -d chrome`, a static server). + return uri != null && + (uri.scheme == 'http' || uri.scheme == 'https') && + (uri.host == 'localhost' || uri.host == '127.0.0.1' || uri.host == '::1'); +} + +/// Drops sockets whose peer vanished without closing (laptop asleep, network +/// gone), so their database connections don't pile up. +const _kPingInterval = Duration(seconds: 30); + Future _handleUpgrade(HttpRequest req, _Opts opts) async { - final socket = await WebSocketTransformer.upgrade(req); + final WebSocket socket; + try { + socket = await WebSocketTransformer.upgrade(req); + } catch (e) { + stderr.writeln('WebSocket upgrade failed: $e'); + return; + } + socket.pingInterval = _kPingInterval; try { await _Session(socket, opts).run(); + } catch (e) { + // A session error ends that session only; unawaited, it would + // otherwise be an uncaught error that kills the whole proxy. + stderr.writeln('session ended with an error: $e'); } finally { // After the session has closed its DB connection. try { @@ -113,15 +166,27 @@ class _Session { continue; } final type = msg['type']; - switch (type) { - case 'open': - await _onOpen(msg); - case 'execute': - await _onExecute(msg); - case 'close': - return; - default: - await _send({'type': 'error', 'message': 'unknown type "$type"'}); + try { + switch (type) { + case 'open': + await _onOpen(msg); + case 'execute': + await _onExecute(msg); + case 'close': + return; + default: + await _send({'type': 'error', 'message': 'unknown type "$type"'}); + } + // The handlers read fields with `as` casts, so a field of the wrong + // type (`"port": "x"`) from this untrusted peer is a TypeError. + // Reject that message and keep the session. + // ignore: avoid_catching_errors + } on TypeError catch (e) { + await _send({ + 'type': 'error', + 'id': msg['id']?.toString(), + 'message': 'malformed message: $e', + }); } } } finally { @@ -134,7 +199,7 @@ class _Session { Future _onOpen(Map msg) async { if (opts.sharedSecret != null) { final got = msg['secret']; - if (got != opts.sharedSecret) { + if (got is! String || !_constantTimeEquals(got, opts.sharedSecret!)) { await _send({'type': 'error', 'message': 'bad shared-secret'}); await socket.close(); return; @@ -146,6 +211,14 @@ class _Session { } final host = msg['host'] as String? ?? ''; final port = (msg['port'] as num?)?.toInt() ?? 5432; + if (!_dbHostAllowed(host, port, opts)) { + await _send({ + 'type': 'error', + 'message': 'this proxy does not open $host:$port (see its --allow-db-host)', + }); + await socket.close(); + return; + } final db = msg['database'] as String? ?? ''; final user = msg['username'] as String? ?? ''; final pass = msg['password'] as String? ?? ''; @@ -237,9 +310,10 @@ class _Session { }, ], 'rows': [ - for (final row in result) [for (final v in row) _serialize(v)], + for (final row in result.take(opts.maxRows)) [for (final v in row) _serialize(v)], ], 'affected': result.affectedRows, + if (result.length > opts.maxRows) 'truncated': true, }); } catch (e) { String? code; @@ -303,24 +377,61 @@ class _Session { } } +/// Whether a client may open [host]:[port]. With no `--allow-db-host` any +/// database is allowed; with one, only the listed entries are, each either +/// `host` (any port) or `host:port`. Host names compare case-insensitively. +bool _dbHostAllowed(String host, int port, _Opts opts) { + if (opts.allowDbHosts.isEmpty) return true; + final h = host.trim().toLowerCase(); + return opts.allowDbHosts.contains(h) || opts.allowDbHosts.contains('$h:$port'); +} + +/// Compares secrets in time that depends only on their lengths, so a +/// caller can't learn the secret one character at a time by timing replies. +bool _constantTimeEquals(String a, String b) { + final x = utf8.encode(a); + final y = utf8.encode(b); + var diff = x.length ^ y.length; + for (var i = 0; i < x.length && i < y.length; i++) { + diff |= x[i] ^ y[i]; + } + return diff == 0; +} + class _Opts { _Opts({ required this.host, required this.port, required this.sharedSecret, - required this.allowOrigin, + required this.allowOrigins, + required this.allowAnyOrigin, + required this.allowDbHosts, + required this.maxRows, }); final String host; final int port; final String? sharedSecret; - final String? allowOrigin; + final Set allowOrigins; + final bool allowAnyOrigin; + final Set allowDbHosts; + + /// Most rows one result sends back; the rest are dropped and the reply + /// carries `"truncated": true`. Bounds the proxy's and the browser's + /// memory for an unbounded `SELECT *`. dbscope's own grids page far below + /// it (query tabs stop at 10 000 rows). + final int maxRows; } +const _kDefaultMaxRows = 100000; + _Opts _parseArgs(List args) { var host = '127.0.0.1'; var port = 8765; String? secret; - String? allow; + final allow = {}; + var allowAny = false; + final dbHosts = {}; + var maxRows = _kDefaultMaxRows; for (var i = 0; i < args.length; i++) { final a = args[i]; String next() { @@ -334,11 +445,29 @@ _Opts _parseArgs(List args) { case '--host': host = next(); case '--port': - port = int.parse(next()); + final raw = next(); + final parsed = int.tryParse(raw); + if (parsed == null || parsed < 0 || parsed > 65535) { + stderr.writeln('--port needs a number from 0 to 65535, got "$raw"'); + exit(2); + } + port = parsed; case '--shared-secret': secret = next(); case '--allow-origin': - allow = next(); + allow.add(next()); + case '--allow-any-origin': + allowAny = true; + case '--allow-db-host': + dbHosts.add(next().trim().toLowerCase()); + case '--max-rows': + final raw = next(); + final parsed = int.tryParse(raw); + if (parsed == null || parsed < 1) { + stderr.writeln('--max-rows needs a positive number, got "$raw"'); + exit(2); + } + maxRows = parsed; case '--help' || '-h': _printHelp(); exit(0); @@ -352,7 +481,10 @@ _Opts _parseArgs(List args) { host: host, port: port, sharedSecret: secret, - allowOrigin: allow, + allowOrigins: allow, + allowAnyOrigin: allowAny, + allowDbHosts: dbHosts, + maxRows: maxRows, ); } @@ -367,7 +499,16 @@ Options: --host HOST Interface to bind (default 127.0.0.1) --port PORT TCP port (default 8765) --shared-secret SECRET Require this in every `open` message - --allow-origin ORIGIN Require this Origin header (CORS-style hardening) + --allow-origin ORIGIN Accept WebSocket upgrades only from this Origin + (repeatable). Without it: the hosted web app and + localhost pages. + --allow-any-origin Accept any Origin. Any web page the user opens + can then reach your databases through the proxy. + --allow-db-host HOST[:PORT] + Only open this database (repeatable). Without it, + a client may open any host the proxy can reach. + --max-rows N Most rows one result returns (default 100000); + longer results are cut and flagged "truncated". -h, --help Show this message '''); } diff --git a/packages/dbscope_pg_proxy_plugin/lib/src/executor.dart b/packages/dbscope_pg_proxy_plugin/lib/src/executor.dart index dab6111..cbe9a79 100644 --- a/packages/dbscope_pg_proxy_plugin/lib/src/executor.dart +++ b/packages/dbscope_pg_proxy_plugin/lib/src/executor.dart @@ -88,6 +88,9 @@ class PgProxyExecutor implements DbExecutor { @override DbCapabilities get capabilities => const DbCapabilities( + // The proxy holds one Postgres session per socket, so a session setting + // sticks for every later statement, exactly as on a direct connection. + sessionReadOnly: true, cancelByClose: true, dml: true, ); @@ -143,10 +146,10 @@ class PgProxyExecutor implements DbExecutor { @override Future setReadOnly({required bool readOnly}) async { - // Proxy doesn't currently expose a session-toggle. Pretend it - // worked so the UI can reflect the requested state, but - // capabilities.sessionReadOnly is false so callers know not to - // depend on it. + // Same statement as PostgresExecutor.setReadOnly: the database itself + // refuses writes, so a read-only profile stays read-only whatever SQL + // the user (or the AI assistant) sends. + await execute('SET default_transaction_read_only = ${readOnly ? 'ON' : 'OFF'}'); _readOnly = readOnly; } diff --git a/packages/dbscope_pg_proxy_plugin/lib/src/profile_dialog.dart b/packages/dbscope_pg_proxy_plugin/lib/src/profile_dialog.dart index 685f8c5..2f12e7c 100644 --- a/packages/dbscope_pg_proxy_plugin/lib/src/profile_dialog.dart +++ b/packages/dbscope_pg_proxy_plugin/lib/src/profile_dialog.dart @@ -82,6 +82,10 @@ class _PgProxyDialogState extends State<_PgProxyDialog> { readOnly: _readOnly, extra: { 'proxyUrl': _proxyUrl.text.trim(), + // Kept in `extra`, not the keychain like the password: subwindows + // have no secure-storage plugin and get the password from the host + // over RPC, which carries only the password. On web, where this + // plugin is mostly used, secure storage is browser storage anyway. if (_sharedSecret.text.isNotEmpty) 'sharedSecret': _sharedSecret.text, }, ); diff --git a/test/production_readiness_test.dart b/test/production_readiness_test.dart new file mode 100644 index 0000000..a7a41b8 --- /dev/null +++ b/test/production_readiness_test.dart @@ -0,0 +1,62 @@ +// Contract: startup and connection defaults hold up against bad input. +// +// * suggestSslMode: a database on this machine defaults to `disable`, any +// other host to `require`, so a new remote profile never sends its +// password in the clear by default. +// * restorePersistedTabs: a saved tab entry this build can't read (wrong +// field types, unknown shape) is skipped; the readable tabs still come +// back and the call never throws. It runs before the first frame, so a +// throw would fail every launch. + +import 'dart:convert'; + +import 'package:dbscope/connection/profile.dart'; +import 'package:dbscope/query/query_tab_kind.dart'; +import 'package:dbscope/shell/app_state.dart'; +import 'package:dbscope/shell/ui_prefs.dart'; +import 'package:dbscope/tabs/tab_kind.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; + +void main() { + group('suggestSslMode', () { + test('local hosts default to disable', () { + for (final host in ['localhost', '127.0.0.1', '::1', '', ' LOCALHOST ']) { + expect(suggestSslMode(host), SslMode.disable, reason: host); + } + }); + + test('remote hosts default to require', () { + for (final host in ['db.example.com', '10.0.0.5', 'my-instance.rds.amazonaws.com']) { + expect(suggestSslMode(host), SslMode.require, reason: host); + } + }); + }); + + group('restorePersistedTabs', () { + setUp(() { + TestWidgetsFlutterBinding.ensureInitialized(); + if (TabKindRegistry.instance.lookupById(queryTabKind.id) == null) { + TabKindRegistry.instance.register(queryTabKind); + } + }); + + test('skips unreadable entries and keeps the rest', () async { + SharedPreferences.setMockInitialValues({ + 'ui.openTabs': jsonEncode({ + 'activeId': 42, + 'tabs': [ + {'kind': 7}, + {'kind': 'query', 'id': 'bad', 'sql': 12345}, + {'kind': 'query', 'id': 'good', 'connectionId': 'c', 'sql': 'select 1'}, + 'not a map', + ], + }), + }); + await UiPrefs.instance.init(); + + expect(AppState.instance.restorePersistedTabs, returnsNormally); + expect([for (final tab in AppState.instance.tabs) tab.id], ['good']); + }); + }); +}