mirror of
https://github.com/kuhyx/screen-locker.git
synced 2026-11-10 19:27:15 +01:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KFwA9nymhUdTrVF65kigTV
This commit is contained in:
parent
7ae4c13407
commit
0921146044
@ -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 == []
|
||||
|
||||
@ -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
|
||||
|
||||
|
||||
|
||||
@ -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<String, String>? 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;
|
||||
}
|
||||
@ -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();
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
@ -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<FirebaseAccount?> _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<FirebaseAccount?> _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()) {
|
||||
|
||||
@ -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();
|
||||
});
|
||||
|
||||
|
||||
@ -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<void> testExecutable(FutureOr<void> 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_');
|
||||
@ -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<StateError>().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',
|
||||
);
|
||||
});
|
||||
});
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user