From 0921146044409608e9979d3592822d9dd8f8c6bc Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Sun, 27 Sep 2026 08:35:18 +0200 Subject: [PATCH] Tests can no longer write the live desktop Firebase credential `flutter test` runs on a Linux host, so credentialStore()'s Platform.isLinux branch wrote $HOME/.config/screen_locker/firebase_auth.json -- the refresh token the screen-locker daemon signs in with. firebase_backend_google_test's "stores the account under the email Firebase reports" case overwrote it with the fixture token 'refresh' on every local run; the daemon only survived by self-healing from a cached token. Dart: - lib/services/desktop_config_dir.dart: one resolver, desktopConfigRoot(), for every desktop path (the credential file's read/write/delete and the ~/.config/crdt-sync account read). @visibleForTesting override; under FLUTTER_TEST it throws StateError if the root is, or is inside, the real $HOME/.config -- so a test that bypasses the override fails loudly instead of writing. HOME unset now yields null instead of a cwd-relative .config. - The crdt-sync read resolves the root outside its catch-all, so the guard cannot be swallowed as "not connected". - test/flutter_test_config.dart gives every test its own temp config root. - The Google sign-in test now asserts the token landed in that temp root. - path_provider is unbound under flutter test (MissingPluginException), so the getApplicationSupportDirectory callers cannot reach live dirs. Python (tests only): isolate_sync_token now patches every import-time binding of SYNC_TOKEN_FILE (_constants, _manual_push, _sync_status were missed), and test_home_isolation scans all runtime modules for any Path still bound inside the real ~/.config/screen_locker. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KFwA9nymhUdTrVF65kigTV --- screen_locker/tests/test_home_isolation.py | 30 +++++ screen_locker/tests/test_sync_fixtures.py | 23 +++- .../lib/services/desktop_config_dir.dart | 50 ++++++++ .../lib/services/firebase_backend.dart | 8 +- .../services/firebase_backend_desktop.dart | 25 ++-- .../test/firebase_backend_google_test.dart | 15 +++ .../workout_app/test/flutter_test_config.dart | 33 +++++ .../services/desktop_config_dir_test.dart | 118 ++++++++++++++++++ 8 files changed, 284 insertions(+), 18 deletions(-) create mode 100644 stronglift_replacement/workout_app/lib/services/desktop_config_dir.dart create mode 100644 stronglift_replacement/workout_app/test/flutter_test_config.dart create mode 100644 stronglift_replacement/workout_app/test/services/desktop_config_dir_test.dart diff --git a/screen_locker/tests/test_home_isolation.py b/screen_locker/tests/test_home_isolation.py index e205754..6a1a55b 100644 --- a/screen_locker/tests/test_home_isolation.py +++ b/screen_locker/tests/test_home_isolation.py @@ -17,11 +17,17 @@ which is the only warning that arrives before the credential is already gone. from __future__ import annotations from datetime import UTC, datetime, timedelta +import importlib import os from pathlib import Path +import pkgutil +import pwd +import sys from crdt_sync import FirebaseCredentials, credential_store_for +import screen_locker + def _fixture_credentials() -> FirebaseCredentials: """Return throwaway credentials shaped like a real cached session.""" @@ -73,3 +79,27 @@ def test_saving_a_credential_writes_only_inside_tmp_path(tmp_path: Path) -> None written = tmp_path / ".config" / "screen_locker" / "firebase_auth.json" assert written.is_file() assert "fixture-refresh-token" in written.read_text() + + +def test_no_module_binds_a_path_inside_the_real_app_config() -> None: + """Import-time bindings of ``~/.config/screen_locker`` must all be redirected. + + ``_isolate_home`` only reaches paths built from ``Path.home()`` at *call* + time. A module constant such as ``SYNC_TOKEN_FILE`` was bound at import, + before any fixture ran, and keeps the real path unless every module that + holds it is patched. This scans every runtime module for such a binding, + so a new one fails here instead of silently writing to the live dir. + """ + real_config = Path(pwd.getpwuid(os.getuid()).pw_dir) / ".config" / "screen_locker" + for info in pkgutil.walk_packages(screen_locker.__path__, "screen_locker."): + if ".tests" not in info.name: + importlib.import_module(info.name) + offenders = sorted( + f"{name}.{attr}" + for name, module in list(sys.modules.items()) + if name.startswith("screen_locker.") and ".tests" not in name + for attr, value in vars(module).items() + if isinstance(value, Path) + and (value == real_config or real_config in value.parents) + ) + assert offenders == [] diff --git a/screen_locker/tests/test_sync_fixtures.py b/screen_locker/tests/test_sync_fixtures.py index 5a25aaa..a69080a 100644 --- a/screen_locker/tests/test_sync_fixtures.py +++ b/screen_locker/tests/test_sync_fixtures.py @@ -7,6 +7,7 @@ for every test in this directory. from __future__ import annotations +from contextlib import ExitStack from typing import TYPE_CHECKING from unittest.mock import patch @@ -17,6 +18,17 @@ if TYPE_CHECKING: from pathlib import Path +# Every module that holds SYNC_TOKEN_FILE (~/.config/screen_locker/sync_token) +# by value, bound at import before any fixture could redirect Path.home(). +_SYNC_TOKEN_BINDINGS = ( + "_constants", + "_manual_push", + "_sync_client", + "_sync_status", + "_workout_sync", +) + + @pytest.fixture(autouse=True) def isolate_sync_token(tmp_path: Path) -> Iterator[None]: """Redirect SYNC_TOKEN_FILE to tmp_path so tests never see a real token. @@ -30,12 +42,13 @@ def isolate_sync_token(tmp_path: Path) -> Iterator[None]: """ # read_sync_token moved to _sync_client when _workout_sync was split; # _workout_sync still re-exports the constant, so both names must be - # redirected or a real token on the host leaks into the tests. + # redirected or a real token on the host leaks into the tests. The rest + # bind it by value too (only for messages today); patching every binding + # is what test_home_isolation's module scan checks. token = tmp_path / "sync_token" - with ( - patch("screen_locker._sync_client.SYNC_TOKEN_FILE", token), - patch("screen_locker._workout_sync.SYNC_TOKEN_FILE", token), - ): + with ExitStack() as stack: + for module in _SYNC_TOKEN_BINDINGS: + stack.enter_context(patch(f"screen_locker.{module}.SYNC_TOKEN_FILE", token)) yield diff --git a/stronglift_replacement/workout_app/lib/services/desktop_config_dir.dart b/stronglift_replacement/workout_app/lib/services/desktop_config_dir.dart new file mode 100644 index 0000000..27d6aad --- /dev/null +++ b/stronglift_replacement/workout_app/lib/services/desktop_config_dir.dart @@ -0,0 +1,50 @@ +/// Where the Linux desktop build keeps its per-app config (`~/.config`). +/// +/// Every desktop path this app reads or writes under the user's home goes +/// through [desktopConfigRoot]: the shared Firebase refresh token in +/// `screen_locker/firebase_auth.json` and the account in `crdt-sync/`. +/// +/// It exists as one choke point because `flutter test` runs on a Linux host, +/// so `Platform.isLinux` is true there too. Before this seam, a sign-in test +/// that stored its fixture refresh token wrote it straight over the live +/// credential the screen-locker daemon signs in with. +library; + +import 'dart:io'; + +import 'package:flutter/foundation.dart'; +import 'package:path/path.dart' as p; + +/// Replaces `$HOME/.config` for the rest of the process when non-null. +/// +/// `test/flutter_test_config.dart` points it at a temp directory for every +/// test file; nothing in the app sets it. +@visibleForTesting +String? desktopConfigRootOverride; + +/// Returns the config root (`$HOME/.config`), or null when HOME is unset. +/// +/// Throws [StateError] under `flutter test` (which exports `FLUTTER_TEST`) +/// when the root would be the real `$HOME/.config` or anywhere inside it: a +/// test that got here without the temp-dir override would otherwise read the +/// real account and overwrite the real refresh token, and still pass. +/// [environment] defaults to [Platform.environment]; tests pass their own. +String? desktopConfigRoot({Map? environment}) { + final env = environment ?? Platform.environment; + final home = env['HOME']; + final realRoot = (home == null || home.isEmpty) + ? null + : p.join(home, '.config'); + final root = desktopConfigRootOverride ?? realRoot; + if (env.containsKey('FLUTTER_TEST') && realRoot != null && root != null) { + if (p.equals(root, realRoot) || p.isWithin(realRoot, root)) { + throw StateError( + 'Refusing to use the real $realRoot under flutter test: that is the ' + 'live desktop credential store. Set desktopConfigRootOverride to a ' + 'temp directory (test/flutter_test_config.dart does this for every ' + 'test file).', + ); + } + } + return root; +} diff --git a/stronglift_replacement/workout_app/lib/services/firebase_backend.dart b/stronglift_replacement/workout_app/lib/services/firebase_backend.dart index 490acfe..55b5e63 100644 --- a/stronglift_replacement/workout_app/lib/services/firebase_backend.dart +++ b/stronglift_replacement/workout_app/lib/services/firebase_backend.dart @@ -20,6 +20,7 @@ import 'package:crdt_sync/crdt_sync.dart'; import 'package:flutter_secure_storage/flutter_secure_storage.dart'; import 'package:http/http.dart' as http; import 'package:path/path.dart' as p; +import 'package:workout_app/services/desktop_config_dir.dart'; import 'package:workout_app/services/google_sign_in_backend.dart'; part 'firebase_backend_desktop.dart'; @@ -58,11 +59,14 @@ SecureCredentialStore credentialStore() { return SecureCredentialStore( read: (key) async { final file = _desktopCredentialFile(); - if (!file.existsSync()) return null; + if (file == null || !file.existsSync()) return null; return await file.readAsString(); }, write: (key, value) async { final file = _desktopCredentialFile(); + if (file == null) { + throw StateError('HOME is unset: nowhere to store the session.'); + } await file.parent.create(recursive: true); await file.writeAsString(value); // The refresh token is the secret worth protecting; keep it 0600 the @@ -71,7 +75,7 @@ SecureCredentialStore credentialStore() { }, delete: (key) async { final file = _desktopCredentialFile(); - if (file.existsSync()) await file.delete(); + if (file != null && file.existsSync()) await file.delete(); }, ); } diff --git a/stronglift_replacement/workout_app/lib/services/firebase_backend_desktop.dart b/stronglift_replacement/workout_app/lib/services/firebase_backend_desktop.dart index d40ee35..04d21a9 100644 --- a/stronglift_replacement/workout_app/lib/services/firebase_backend_desktop.dart +++ b/stronglift_replacement/workout_app/lib/services/firebase_backend_desktop.dart @@ -16,14 +16,15 @@ part of 'firebase_backend.dart'; /// Deliberately the SAME file the PC's own sync already uses: the fleet holds /// one long-lived refresh token per app, and a second copy would drift out of /// date the first time either half refreshed it. -File _desktopCredentialFile() => File( - p.join( - Platform.environment['HOME'] ?? '', - '.config', - 'screen_locker', - 'firebase_auth.json', - ), -); +/// +/// Resolved through [desktopConfigRoot], which throws under `flutter test` +/// rather than let a test reach the live file. Null when HOME is unset; the +/// old `HOME ?? ''` silently resolved a cwd-relative `.config/` instead. +File? _desktopCredentialFile() { + final root = desktopConfigRoot(); + if (root == null) return null; + return File(p.join(root, 'screen_locker', 'firebase_auth.json')); +} /// Reads the shared desktop credential from `~/.config/crdt-sync/`. /// @@ -37,9 +38,11 @@ File _desktopCredentialFile() => File( /// Returns null when the files are absent or malformed; the caller then /// reports the device as not connected, exactly as an empty keystore would. Future _accountFromDesktopConfig() async { + // Resolved outside the try: its flutter-test guard must fail the test, not + // be logged and swallowed as "not connected" by the catch-all below. + final root = desktopConfigRoot(); try { - final home = Platform.environment['HOME']; - if (home == null || home.isEmpty) { + if (root == null) { log( 'WorkoutApp: HOME is unset — cannot locate ~/.config/crdt-sync, so ' 'this desktop is NOT connected to sync.', @@ -47,7 +50,7 @@ Future _accountFromDesktopConfig() async { ); return null; } - final dir = Directory(p.join(home, '.config', 'crdt-sync')); + final dir = Directory(p.join(root, 'crdt-sync')); final configFile = File(p.join(dir.path, 'firebase.json')); final passwordFile = File(p.join(dir.path, 'password')); if (!configFile.existsSync() || !passwordFile.existsSync()) { diff --git a/stronglift_replacement/workout_app/test/firebase_backend_google_test.dart b/stronglift_replacement/workout_app/test/firebase_backend_google_test.dart index 1bf6e3a..743889b 100644 --- a/stronglift_replacement/workout_app/test/firebase_backend_google_test.dart +++ b/stronglift_replacement/workout_app/test/firebase_backend_google_test.dart @@ -1,9 +1,12 @@ import 'dart:convert'; +import 'dart:io'; import 'package:crdt_sync/crdt_sync.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart' as http_testing; +import 'package:path/path.dart' as p; +import 'package:workout_app/services/desktop_config_dir.dart'; import 'package:workout_app/services/firebase_backend.dart'; import 'package:workout_app/services/google_sign_in_backend.dart'; @@ -66,6 +69,18 @@ void main() { // No password to store: this device authenticates with the refresh // token, and openFirebase() must not later try to sign in with ''. expect(saved.single.password, isEmpty); + // The session itself went through credentialStore(), which on this + // Linux host is the desktop file -- redirected by flutter_test_config. + // Before that redirect, this very test overwrote the live + // ~/.config/screen_locker/firebase_auth.json with 'refresh'. + final session = File( + p.join( + desktopConfigRootOverride!, + 'screen_locker', + 'firebase_auth.json', + ), + ); + expect(session.readAsStringSync(), contains('"refresh"')); client!.close(); }); diff --git a/stronglift_replacement/workout_app/test/flutter_test_config.dart b/stronglift_replacement/workout_app/test/flutter_test_config.dart new file mode 100644 index 0000000..0d462f2 --- /dev/null +++ b/stronglift_replacement/workout_app/test/flutter_test_config.dart @@ -0,0 +1,33 @@ +/// Suite-wide setup: applied by `flutter test` to every test file below. +/// +/// The test host is Linux, so every `Platform.isLinux` desktop branch runs +/// here -- including the one that stores the Firebase refresh token in +/// `~/.config/screen_locker/firebase_auth.json`, the live credential the +/// screen-locker daemon signs in with. A sign-in test once overwrote it with +/// fixture values and still passed. Each test now gets its own throwaway +/// config root instead; `desktopConfigRoot` refuses the real one under +/// `flutter test`, so a test that slipped past this fails loudly. +library; + +import 'dart:async'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:workout_app/services/desktop_config_dir.dart'; + +Future testExecutable(FutureOr Function() testMain) async { + // Also set up front, so code that runs while tests are being declared is + // redirected too, not only code inside a test body. + final declarationRoot = _freshRoot(); + desktopConfigRootOverride = declarationRoot.path; + tearDownAll(() => declarationRoot.deleteSync(recursive: true)); + setUp(() { + final root = _freshRoot(); + desktopConfigRootOverride = root.path; + addTearDown(() => root.deleteSync(recursive: true)); + }); + await testMain(); +} + +Directory _freshRoot() => + Directory.systemTemp.createTempSync('workout_app_test_config_'); diff --git a/stronglift_replacement/workout_app/test/services/desktop_config_dir_test.dart b/stronglift_replacement/workout_app/test/services/desktop_config_dir_test.dart new file mode 100644 index 0000000..490719b --- /dev/null +++ b/stronglift_replacement/workout_app/test/services/desktop_config_dir_test.dart @@ -0,0 +1,118 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:path/path.dart' as p; +import 'package:workout_app/services/desktop_config_dir.dart'; +import 'package:workout_app/services/firebase_backend.dart'; + +/// Runs [body] with the suite-wide override cleared, restoring it after. +void _withoutOverride(void Function() body) { + final saved = desktopConfigRootOverride; + desktopConfigRootOverride = null; + try { + body(); + } finally { + desktopConfigRootOverride = saved; + } +} + +void main() { + const home = '/home/someone'; + const testEnv = {'HOME': home, 'FLUTTER_TEST': 'true'}; + + group('suite isolation', () { + test('flutter test exports FLUTTER_TEST, which the guard keys on', () { + expect(Platform.environment.containsKey('FLUTTER_TEST'), isTrue); + }); + + test('flutter_test_config points every test at a temp root', () { + final root = desktopConfigRoot(); + expect(root, isNotNull); + expect(root, desktopConfigRootOverride); + expect(p.isWithin(Directory.systemTemp.path, root!), isTrue); + final realHome = Platform.environment['HOME']; + if (realHome != null && realHome.isNotEmpty) { + expect(p.isWithin(realHome, root), isFalse); + } + }); + + test('a stored session lands in the temp root, not ~/.config', () async { + // The write the 2026-09 incident performed: a sign-in test stored its + // fixture refresh token through credentialStore() on the Linux host. + await credentialStore().write('firebase.credentials', '{}'); + final written = File( + p.join( + desktopConfigRootOverride!, + 'screen_locker', + 'firebase_auth.json', + ), + ); + expect(written.existsSync(), isTrue); + }); + + test('without the override, storing a session fails loudly', () async { + final saved = desktopConfigRootOverride; + desktopConfigRootOverride = null; + addTearDown(() => desktopConfigRootOverride = saved); + await expectLater( + credentialStore().write('firebase.credentials', '{}'), + throwsStateError, + ); + }); + }); + + group('desktopConfigRoot', () { + test('is HOME/.config outside flutter test', () { + _withoutOverride(() { + expect( + desktopConfigRoot(environment: const {'HOME': home}), + '$home/.config', + ); + }); + }); + + test('is null when HOME is unset or empty', () { + _withoutOverride(() { + expect(desktopConfigRoot(environment: const {}), isNull); + expect(desktopConfigRoot(environment: const {'HOME': ''}), isNull); + expect( + desktopConfigRoot(environment: const {'FLUTTER_TEST': 'true'}), + isNull, + ); + }); + }); + + test('refuses the real ~/.config under flutter test', () { + _withoutOverride(() { + expect( + () => desktopConfigRoot(environment: testEnv), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('desktopConfigRootOverride'), + ), + ), + ); + }); + }); + + test('refuses an override inside the real ~/.config too', () { + final saved = desktopConfigRootOverride; + addTearDown(() => desktopConfigRootOverride = saved); + desktopConfigRootOverride = '$home/.config/elsewhere'; + expect(() => desktopConfigRoot(environment: testEnv), throwsStateError); + }); + + test('returns an override outside the real ~/.config', () { + final saved = desktopConfigRootOverride; + addTearDown(() => desktopConfigRootOverride = saved); + desktopConfigRootOverride = '/tmp/isolated'; + expect(desktopConfigRoot(environment: testEnv), '/tmp/isolated'); + expect( + desktopConfigRoot(environment: const {'HOME': home}), + '/tmp/isolated', + ); + }); + }); +}