From 2bbfe0b6635770df9e328de7db8eaa39c40a0290 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 25 Sep 2026 16:31:08 +0200 Subject: [PATCH] Desktop: launch Chrome with --password-store=basic so a locked keyring cannot blank the window Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 4 +- lib/desktop/browser_launcher.dart | 45 ++++++++---- pubspec.yaml | 2 +- test/desktop/browser_launcher_test.dart | 96 +++++++++++++++++++++++++ 4 files changed, 130 insertions(+), 17 deletions(-) create mode 100644 test/desktop/browser_launcher_test.dart diff --git a/AGENTS.md b/AGENTS.md index ee51e02..89077f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -117,8 +117,8 @@ rewrites links to `~/data/cloud/todo-images/…` so an agent can open them. ## Testing -- **The suite must stay quick and fully green.** It is currently **418 tests at - 100% line coverage** (2494 lines), ~30s on this machine with +- **The suite must stay quick and fully green.** It is currently **427 tests at + 100% line coverage** (2512 lines), ~30s on this machine with `--concurrency=3` (unbounded concurrency under `capped.sh`'s 4 GiB gets its test processes SIGTERMed while the wrapper still exits 0 — check the last line says "All tests passed"). Don't regress either. diff --git a/lib/desktop/browser_launcher.dart b/lib/desktop/browser_launcher.dart index 274f90f..b98f316 100644 --- a/lib/desktop/browser_launcher.dart +++ b/lib/desktop/browser_launcher.dart @@ -24,12 +24,14 @@ String profileDir(String home) => p.join(home, '.local', 'share', 'todo-desktop', 'profile'); /// Returns the first Chrome-family browser on this machine, or ''. -String findBrowser() { +/// +/// [override] defaults to `$TODO_BROWSER`; [exists] to a file check. +String findBrowser({String? override, bool Function(String path)? exists}) { // Ordered by preference, and deliberately broad: this machine runs Thorium // behind /opt/google/chrome, and has a policy that uninstalls the `chromium` // package, so assuming any single browser is wrong. TODO_BROWSER overrides. final candidates = [ - Platform.environment['TODO_BROWSER'] ?? '', + override ?? Platform.environment['TODO_BROWSER'] ?? '', '/opt/google/chrome/chrome', '/opt/thorium-browser/thorium-browser', '/usr/bin/google-chrome-stable', @@ -37,17 +39,40 @@ String findBrowser() { '/usr/bin/brave', ]; return candidates.firstWhere( - (path) => path.isNotEmpty && File(path).existsSync(), + (path) => path.isNotEmpty && (exists ?? (p) => File(p).existsSync())(path), orElse: () => '', ); } +/// Command-line arguments for the app window. +List browserArgs(String home) => [ + '--app=http://localhost:$kWrapperPort', + '--user-data-dir=${profileDir(home)}', + // Sets WM_CLASS, which the .desktop entry matches on via StartupWMClass. + // Without it the window inherits the browser's class and the taskbar shows + // a browser icon instead of todo's. + '--class=todo', + '--no-first-run', + // Keep Chrome off the GNOME keyring. While the keyring is locked, Chrome + // blocks every non-incognito page load on its first cookie access, and an + // "Unlock Keyring" prompt waits on another workspace: the window stays a + // blank grey. Nothing in this profile needs OS-level encryption (the + // GitHub token is in localStorage, not the password store). + '--password-store=basic', +]; + /// Launches the app in a Chrome-family browser with a **stable** profile /// directory, since the token and notes live in that profile. /// /// Returns true when the browser ran long enough to have owned the session. -Future launchBrowser(String home) async { - final browser = findBrowser(); +/// [browser] and [start] exist for tests; they default to [findBrowser] and +/// [Process.start]. +Future launchBrowser( + String home, { + String? browser, + Future Function(String executable, List args)? start, +}) async { + browser ??= findBrowser(); if (browser.isEmpty) { stderr.writeln( 'No Chrome-family browser found; open ' @@ -55,15 +80,7 @@ Future launchBrowser(String home) async { ); return false; } - final process = await Process.start(browser, [ - '--app=http://localhost:$kWrapperPort', - '--user-data-dir=${profileDir(home)}', - // Sets WM_CLASS, which the .desktop entry matches on via StartupWMClass. - // Without it the window inherits the browser's class and the taskbar shows - // a browser icon instead of todo's. - '--class=todo', - '--no-first-run', - ]); + final process = await (start ?? Process.start)(browser, browserArgs(home)); final started = DateTime.now(); await process.exitCode; diff --git a/pubspec.yaml b/pubspec.yaml index bd52d4b..63b3a82 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -16,7 +16,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev # https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html # In Windows, build-name is used as the major, minor, and patch parts # of the product and file versions while build-number is used as the build suffix. -version: 1.0.38+1 +version: 1.0.39+1 environment: sdk: ^3.12.2 diff --git a/test/desktop/browser_launcher_test.dart b/test/desktop/browser_launcher_test.dart new file mode 100644 index 0000000..84ca47a --- /dev/null +++ b/test/desktop/browser_launcher_test.dart @@ -0,0 +1,96 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:todo/desktop/browser_launcher.dart'; + +void main() { + group('browserArgs', () { + final args = browserArgs('/h'); + + test('opens the wrapper as an app window in the fixed profile', () { + expect(args, contains('--app=http://localhost:$kWrapperPort')); + expect( + args, + contains('--user-data-dir=/h/.local/share/todo-desktop/profile'), + ); + expect(args, contains('--class=todo')); + }); + + test('keeps Chrome off the keyring, whose lock blanks the window', () { + expect(args, contains('--password-store=basic')); + }); + }); + + group('findBrowser', () { + test('an override that exists wins', () { + expect(findBrowser(override: '/x/b', exists: (_) => true), '/x/b'); + }); + + test('falls through to the first installed candidate', () { + expect( + findBrowser( + override: '', + exists: (p) => p == '/usr/bin/google-chrome-stable', + ), + '/usr/bin/google-chrome-stable', + ); + }); + + test("returns '' when nothing is installed", () { + expect(findBrowser(override: '', exists: (_) => false), ''); + }); + + test(r'defaults to $TODO_BROWSER and real files', () { + expect(findBrowser(), isA()); + }); + }); + + group('launchBrowser', () { + test('defaults to the browser findBrowser picks', () async { + final expected = findBrowser(); + String? exe; + await launchBrowser( + '/h', + start: (e, _) { + exe = e; + return Process.start('true', []); + }, + ); + expect(exe, expected.isEmpty ? isNull : expected); + }); + + test('without a browser it reports false and starts nothing', () async { + var started = false; + final ok = await launchBrowser( + '/h', + browser: '', + start: (_, _) async { + started = true; + return Process.start('true', []); + }, + ); + expect(ok, isFalse); + expect(started, isFalse); + }); + + test( + 'starts the browser with browserArgs; a quick exit is false', + () async { + String? exe; + List? passed; + final ok = await launchBrowser( + '/h', + browser: '/x/chrome', + start: (e, a) { + exe = e; + passed = a; + return Process.start('true', []); + }, + ); + expect(exe, '/x/chrome'); + expect(passed, browserArgs('/h')); + expect(ok, isFalse, reason: 'ran under 5 s: did not own the session'); + }, + ); + }); +}