From 59bf69f1d5200731ca5ba5f863b251bdec8b5014 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Tue, 20 Oct 2026 00:11:31 +0600 Subject: [PATCH] fix(mobile): hide server addresses and tokens in shared logs (#32287) * fix(mobile): hide server addresses and tokens in shared logs * fix(mobile): stop writing server urls and oauth values to the log * fix(mobile): redact logs where they are read * fix(mobile): redact logs when they are written * fix(mobile): hide ipv6 addresses and name labels after their settings (cherry picked from commit 89d2de1274a3e1a70a6169f4cfe1feebfb327791) --- mobile/lib/domain/services/log.service.dart | 82 ++++++++- .../lib/pages/common/splash_screen.page.dart | 18 +- .../providers/app_life_cycle.provider.dart | 3 +- mobile/lib/services/auth.service.dart | 7 +- mobile/lib/services/oauth.service.dart | 4 +- mobile/lib/utils/bootstrap.dart | 2 + .../domain/services/log_service_test.dart | 171 +++++++++++++++++- .../app_life_cycle_provider_test.dart | 1 + 8 files changed, 260 insertions(+), 28 deletions(-) diff --git a/mobile/lib/domain/services/log.service.dart b/mobile/lib/domain/services/log.service.dart index b612b3ce9..bcb84e1eb 100644 --- a/mobile/lib/domain/services/log.service.dart +++ b/mobile/lib/domain/services/log.service.dart @@ -1,12 +1,16 @@ import 'dart:async'; +import 'package:http/http.dart' show ClientException; import 'package:immich_mobile/constants/constants.dart'; import 'package:immich_mobile/domain/models/log.model.dart'; import 'package:immich_mobile/domain/models/settings_key.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; import 'package:immich_mobile/infrastructure/repositories/log.repository.dart'; import 'package:immich_mobile/infrastructure/repositories/settings.repository.dart'; import 'package:immich_mobile/utils/debug_print.dart'; import 'package:logging/logging.dart'; +import 'package:openapi/api.dart' show ApiException; /// Service responsible for handling application logging. /// @@ -14,8 +18,15 @@ import 'package:logging/logging.dart'; /// writes them to a persistent [LogRepository], and manages log levels via /// [SettingsRepository]. class LogService { + static final _schemeAndHost = RegExp(r'''https?://[^\s/?#'"<>(),]*'''); + static final _ipAddress = RegExp( + r'\b\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}\b|' + r'\[?\b(?:(?:[0-9a-f]{1,4}:){7}[0-9a-f]{1,4}|(?:[0-9a-f]{1,4}:)+(?::[0-9a-f]{1,4})+)(?:%\w+)?\]?', + ); + final LogRepository _logRepository; final SettingsRepository _settingsRepository; + final StoreService _storeService; final List _msgBuffer = []; @@ -39,11 +50,13 @@ class LogService { static Future init({ required LogRepository logRepository, required SettingsRepository settingsRepository, + required StoreService storeService, bool shouldBuffer = true, }) async { _instance ??= await create( logRepository: logRepository, settingsRepository: settingsRepository, + storeService: storeService, shouldBuffer: shouldBuffer, ); return _instance!; @@ -52,16 +65,17 @@ class LogService { static Future create({ required LogRepository logRepository, required SettingsRepository settingsRepository, + required StoreService storeService, bool shouldBuffer = true, }) async { - final instance = LogService._(logRepository, settingsRepository, shouldBuffer); + final instance = LogService._(logRepository, settingsRepository, storeService, shouldBuffer); await logRepository.truncate(limit: kLogTruncateLimit); final level = instance._settingsRepository.appConfig.logLevel; Logger.root.level = Level.LEVELS.elementAtOrNull(level.index) ?? Level.INFO; return instance; } - LogService._(this._logRepository, this._settingsRepository, this._shouldBuffer) { + LogService._(this._logRepository, this._settingsRepository, this._storeService, this._shouldBuffer) { _logSubscription = Logger.root.onRecord.listen(_handleLogRecord); } @@ -74,12 +88,12 @@ class LogService { ); final record = LogMessage( - message: r.message, + message: _redact(r.message), level: r.level.toLogLevel(), createdAt: r.time, logger: r.loggerName, - error: r.error?.toString(), - stack: r.stackTrace?.toString(), + error: r.error == null ? null : _redactError(r.error!), + stack: r.stackTrace == null ? null : _redact(r.stackTrace.toString()), ); if (_shouldBuffer) { @@ -100,6 +114,64 @@ class LogService { return [..._msgBuffer.reversed, ...logsFromDb]; } + String _redact(String text) { + var redacted = text.replaceAllMapped(_schemeAndHost, (match) => _urlLabel(Uri.tryParse(match[0]!))); + final accessToken = _storeService.tryGet(StoreKey.accessToken); + if (accessToken != null) { + redacted = redacted.replaceAll(accessToken, '[ACCESS TOKEN]'); + } + // as a header map prints it, so a short value like "1" can't match other text + for (final header in _settingsRepository.appConfig.network.customHeaders.entries) { + redacted = redacted.replaceAll( + RegExp('${RegExp.escape(header.key)}:\\s+${RegExp.escape(header.value)}'), + '${header.key}: [HEADER VALUE]', + ); + } + return redacted; + } + + String _redactError(Object error) { + final redacted = _redact(error.toString()); + final httpError = error is ApiException ? error.innerException : error; + if (httpError is! ClientException) { + return redacted; + } + + // ok_http also prints the host and its ip on their own: "Failed to connect to host/203.0.113.9:443" + final url = httpError.uri; + final withoutHost = url == null || url.host.isEmpty + ? redacted + : redacted.replaceAll(RegExp('\\b${RegExp.escape(url.host)}\\b'), _urlLabel(url)); + return withoutHost.replaceAll(_ipAddress, '[IP ADDRESS]'); + } + + String _urlLabel(Uri? url) { + if (url == null) { + return '[OTHER URL]'; + } + + // a server url that is also a switching entry gets the entry's label + final network = _settingsRepository.appConfig.network; + for (var i = 0; i < network.externalEndpointList.length; i++) { + if (_isSameAddress(url, network.externalEndpointList[i])) { + return '[EXTERNAL ENDPOINT ${i + 1}]'; + } + } + if (_isSameAddress(url, network.localEndpoint)) { + return '[LOCAL ENDPOINT]'; + } + if (_isSameAddress(url, _storeService.tryGet(StoreKey.serverUrl)) || + _isSameAddress(url, _storeService.tryGet(StoreKey.serverEndpoint))) { + return '[SERVER URL]'; + } + return '[OTHER URL]'; + } + + bool _isSameAddress(Uri url, String? savedUrl) { + final saved = savedUrl == null ? null : Uri.tryParse(savedUrl); + return saved != null && saved.scheme == url.scheme && saved.host == url.host && saved.port == url.port; + } + Future clearLogs() async { _flushTimer?.cancel(); _flushTimer = null; diff --git a/mobile/lib/pages/common/splash_screen.page.dart b/mobile/lib/pages/common/splash_screen.page.dart index 4cffd3bd7..57e07fbc1 100644 --- a/mobile/lib/pages/common/splash_screen.page.dart +++ b/mobile/lib/pages/common/splash_screen.page.dart @@ -288,21 +288,7 @@ class SplashScreenPageState extends ConsumerState { @override void initState() { super.initState(); - unawaited( - ref - .read(authProvider.notifier) - .setOpenApiServiceEndpoint() - .then(logConnectionInfo) - .whenComplete(() => resumeSession()), - ); - } - - void logConnectionInfo(String? endpoint) { - if (endpoint == null) { - return; - } - - log.info("Resuming session at $endpoint"); + unawaited(ref.read(authProvider.notifier).setOpenApiServiceEndpoint().whenComplete(() => resumeSession())); } Future resumeSession() async { @@ -356,7 +342,7 @@ class SplashScreenPageState extends ConsumerState { } }, onError: (exception) { - log.severe('Failed to update auth info with access token: $accessToken'); + log.severe('Failed to update auth info', exception); if (!mounted) { return; } diff --git a/mobile/lib/providers/app_life_cycle.provider.dart b/mobile/lib/providers/app_life_cycle.provider.dart index 4f4f20efd..e1f3c41c0 100644 --- a/mobile/lib/providers/app_life_cycle.provider.dart +++ b/mobile/lib/providers/app_life_cycle.provider.dart @@ -84,8 +84,7 @@ class AppLifeCycleNotifier extends StateNotifier { // Needs to be logged in if (isAuthenticated) { // switch endpoint if needed - final endpoint = await _ref.read(authProvider.notifier).setOpenApiServiceEndpoint(); - _log.info("Using server URL: $endpoint"); + await _ref.read(authProvider.notifier).setOpenApiServiceEndpoint(); await _ref.read(serverInfoProvider.notifier).getServerVersion(); } diff --git a/mobile/lib/services/auth.service.dart b/mobile/lib/services/auth.service.dart index f22777d52..0c05923d2 100644 --- a/mobile/lib/services/auth.service.dart +++ b/mobile/lib/services/auth.service.dart @@ -162,6 +162,7 @@ class AuthService { final localEndpoint = _authRepository.getLocalEndpoint(); if (localEndpoint != null) { await _apiService.resolveAndSetEndpoint(localEndpoint); + _log.info("Using local endpoint"); return localEndpoint; } } catch (error, stackTrace) { @@ -181,9 +182,11 @@ class AuthService { return null; } - for (final endpoint in endpointList) { + for (var i = 0; i < endpointList.length; i++) { try { - return await _apiService.resolveAndSetEndpoint(endpoint.url); + final url = await _apiService.resolveAndSetEndpoint(endpointList[i].url); + _log.info("Using external endpoint ${i + 1}"); + return url; } on ApiException catch (error) { _log.severe("Cannot resolve endpoint", error); continue; diff --git a/mobile/lib/services/oauth.service.dart b/mobile/lib/services/oauth.service.dart index d8b1604e0..0f5415eba 100644 --- a/mobile/lib/services/oauth.service.dart +++ b/mobile/lib/services/oauth.service.dart @@ -26,7 +26,7 @@ class OAuthService { ); final authUrl = dto?.url; - log.info('Received Authorization URL: $authUrl'); + log.info('Received Authorization URL'); return authUrl; } @@ -34,7 +34,7 @@ class OAuthService { Future oAuthLogin(String oauthUrl, String state, String codeVerifier) async { String result = await FlutterWebAuth2.authenticate(url: oauthUrl, callbackUrlScheme: callbackUrlScheme); - log.info('Received OAuth callback: $result'); + log.info('Received OAuth callback'); if (result.startsWith('app.immich:/oauth-callback')) { result = result.replaceAll('app.immich:/oauth-callback', 'app.immich:///oauth-callback'); diff --git a/mobile/lib/utils/bootstrap.dart b/mobile/lib/utils/bootstrap.dart index d4753c07c..7e5488945 100644 --- a/mobile/lib/utils/bootstrap.dart +++ b/mobile/lib/utils/bootstrap.dart @@ -3,6 +3,7 @@ import 'package:immich_mobile/constants/constants.dart'; import 'package:immich_mobile/data/data_controller.dart'; import 'package:immich_mobile/domain/models/store.model.dart'; import 'package:immich_mobile/domain/services/log.service.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; import 'package:immich_mobile/entities/store.entity.dart'; import 'package:immich_mobile/generated/translations.g.dart'; import 'package:immich_mobile/infrastructure/repositories/log.repository.dart'; @@ -72,6 +73,7 @@ abstract final class Bootstrap { await LogService.init( logRepository: LogRepository(dataController.logDb), settingsRepository: settingsRepo, + storeService: StoreService.I, shouldBuffer: shouldBufferLogs, ); diff --git a/mobile/test/domain/services/log_service_test.dart b/mobile/test/domain/services/log_service_test.dart index 6a82d1dce..8b9262ca6 100644 --- a/mobile/test/domain/services/log_service_test.dart +++ b/mobile/test/domain/services/log_service_test.dart @@ -1,13 +1,23 @@ import 'package:collection/collection.dart'; +import 'package:drift/drift.dart'; +import 'package:drift/native.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' show ClientException; import 'package:immich_mobile/constants/constants.dart'; +import 'package:immich_mobile/data/db/main/database.dart'; import 'package:immich_mobile/domain/models/config/app_config.dart'; +import 'package:immich_mobile/domain/models/config/network_config.dart'; import 'package:immich_mobile/domain/models/log.model.dart'; import 'package:immich_mobile/domain/models/settings_key.dart'; +import 'package:immich_mobile/domain/models/store.model.dart'; import 'package:immich_mobile/domain/services/log.service.dart'; +import 'package:immich_mobile/domain/services/store.service.dart'; import 'package:immich_mobile/infrastructure/repositories/log.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/settings.repository.dart'; +import 'package:immich_mobile/infrastructure/repositories/store.repository.dart'; import 'package:logging/logging.dart'; import 'package:mocktail/mocktail.dart'; +import 'package:openapi/api.dart' show ApiException; import '../../infrastructure/repository.mock.dart'; import '../../test_utils.dart'; @@ -30,6 +40,16 @@ void main() { late LogService sut; late LogRepository mockLogRepo; late MockSettingsRepository mockSettingsRepository; + late Drift db; + + setUpAll(() async { + db = Drift(DatabaseConnection(NativeDatabase.memory(), closeStreamsSynchronously: true)); + await StoreService.init(storeRepository: StoreRepository(db), listenUpdates: false); + }); + + tearDownAll(() async { + await db.close(); + }); setUp(() async { mockLogRepo = MockLogRepository(); @@ -45,11 +65,16 @@ void main() { when(() => mockLogRepo.insert(any())).thenAnswer((_) async => true); when(() => mockLogRepo.insertAll(any())).thenAnswer((_) async => true); - sut = await LogService.create(logRepository: mockLogRepo, settingsRepository: mockSettingsRepository); + sut = await LogService.create( + logRepository: mockLogRepo, + settingsRepository: mockSettingsRepository, + storeService: StoreService.I, + ); }); tearDown(() async { await sut.dispose(); + await StoreService.I.clear(); }); group("Log Service Init:", () { @@ -87,6 +112,7 @@ void main() { sut = await LogService.create( logRepository: mockLogRepo, settingsRepository: mockSettingsRepository, + storeService: StoreService.I, shouldBuffer: true, ); @@ -105,6 +131,7 @@ void main() { sut = await LogService.create( logRepository: mockLogRepo, settingsRepository: mockSettingsRepository, + storeService: StoreService.I, shouldBuffer: true, ); @@ -126,6 +153,7 @@ void main() { sut = await LogService.create( logRepository: mockLogRepo, settingsRepository: mockSettingsRepository, + storeService: StoreService.I, shouldBuffer: false, ); @@ -160,6 +188,7 @@ void main() { sut = await LogService.create( logRepository: mockLogRepo, settingsRepository: mockSettingsRepository, + storeService: StoreService.I, shouldBuffer: true, ); @@ -176,4 +205,144 @@ void main() { }); }); }); + + group("Log Service Redaction:", () { + const accessToken = 'Kq3Zr8WbN2xYp5aLm7cT9vQ4sD6fH1jE0gU2iO5wA8y'; + + test('Replaces urls, the access token and header values when a line is logged', () async { + await StoreService.I.put(StoreKey.serverUrl, 'https://immich.example.com'); + await StoreService.I.put(StoreKey.serverEndpoint, 'https://api.example.com/api'); + await StoreService.I.put(StoreKey.accessToken, accessToken); + when(() => mockSettingsRepository.appConfig).thenReturn( + const AppConfig( + network: NetworkConfig( + localEndpoint: 'http://192.168.1.5:2283', + externalEndpointList: ['https://immich.example.com', 'http://nas.lan:2283'], + customHeaders: {'X-Proxy-Key': 'cf-secret-abcdef0123', 'X-Debug': '1'}, + ), + ), + ); + const rows = { + 'Resuming session at https://immich.example.com/api': 'Resuming session at [EXTERNAL ENDPOINT 1]/api', + 'GET http://nas.lan:2283/api/server/ping': 'GET [EXTERNAL ENDPOINT 2]/api/server/ping', + 'GET http://192.168.1.5:2283/api/server/ping': 'GET [LOCAL ENDPOINT]/api/server/ping', + 'Using server URL: https://api.example.com/api': 'Using server URL: [SERVER URL]/api', + 'Received Authorization URL: https://auth.example.org/authorize': + 'Received Authorization URL: [OTHER URL]/authorize', + 'Causing statement: INSERT INTO "store_entity", parameters: 11, $accessToken': + 'Causing statement: INSERT INTO "store_entity", parameters: 11, [ACCESS TOKEN]', + 'Could not enqueue task DownloadTask{headers: {X-Proxy-Key: cf-secret-abcdef0123, X-Debug: 1}, retries: 1}': + 'Could not enqueue task DownloadTask{headers: {X-Proxy-Key: [HEADER VALUE], X-Debug: [HEADER VALUE]}, retries: 1}', + 'X-Proxy-Key:\tcf-secret-abcdef0123': 'X-Proxy-Key: [HEADER VALUE]', + 'Uploaded 1 assets in 1.2s': 'Uploaded 1 assets in 1.2s', + }; + for (final row in rows.keys) { + Logger('Test').info(row); + } + await sut.flush(); + + final stored = verify(() => mockLogRepo.insertAll(captureAny())).captured.single as List; + expect(stored.map((m) => m.message), rows.values); + }); + + test('Hides the host and ip in http client errors', () async { + when( + () => mockSettingsRepository.appConfig, + ).thenReturn(const AppConfig(network: NetworkConfig(externalEndpointList: ['http://immich:2283']))); + final connectError = ClientException( + 'java.net.ConnectException: Failed to connect to immich/192.168.1.5:2283', + Uri.parse('http://immich:2283/api/server/ping'), + ); + final unknownHostError = ClientException( + 'java.net.UnknownHostException: Unable to resolve host "typo.example.org": No address associated with hostname', + Uri.parse('https://alice:secret@typo.example.org/api/users/me'), + ); + + Logger('Test').severe( + 'Error while checking server availability', + ApiException.withInner( + 400, + 'HTTP connection failed: GET /server/ping', + connectError, + StackTrace.fromString('#0 ApiService._isEndpointAvailable (package:immich_mobile/services/api.service.dart)'), + ), + ); + Logger('Test').severe('Error validating auxiliary endpoint', unknownHostError); + await sut.flush(); + + final stored = verify(() => mockLogRepo.insertAll(captureAny())).captured.single as List; + expect( + stored.first.error, + 'ApiException 400: HTTP connection failed: GET /server/ping (Inner exception: ClientException: ' + 'java.net.ConnectException: Failed to connect to [EXTERNAL ENDPOINT 1]/[IP ADDRESS]:2283, ' + 'uri=[EXTERNAL ENDPOINT 1]/api/server/ping)\n\n' + '#0 ApiService._isEndpointAvailable (package:immich_mobile/services/api.service.dart)', + ); + expect( + stored.last.error, + 'ClientException: java.net.UnknownHostException: Unable to resolve host "[OTHER URL]": ' + 'No address associated with hostname, uri=[OTHER URL]/api/users/me', + ); + }); + + test('Hides ipv6 addresses in http client errors', () async { + const rows = { + 'java.net.ConnectException: Failed to connect to immich.example.com/[2001:db8::5]:443': + 'java.net.ConnectException: Failed to connect to [OTHER URL]/[IP ADDRESS]:443', + 'java.net.ConnectException: Failed to connect to immich.example.com/2001:db8::5:443': + 'java.net.ConnectException: Failed to connect to [OTHER URL]/[IP ADDRESS]', + 'java.net.SocketTimeoutException: failed to connect to immich.example.com/2001:db8::5 (port 443) ' + 'from /2001:db8:1234:5678:9abc:def0:1234:5678 (port 41234) after 30000ms': + 'java.net.SocketTimeoutException: failed to connect to [OTHER URL]/[IP ADDRESS] (port 443) ' + 'from /[IP ADDRESS] (port 41234) after 30000ms', + 'java.net.ConnectException: Failed to connect to /[fe80::5%wlan0]:2283': + 'java.net.ConnectException: Failed to connect to /[IP ADDRESS]:2283', + 'at 12:34:56': 'at 12:34:56', + '0:00:01.500000': '0:00:01.500000', + 'deadbeef': 'deadbeef', + '#0 ApiClient.invokeAPI (package:openapi/api_client.dart:131:7)': + '#0 ApiClient.invokeAPI (package:openapi/api_client.dart:131:7)', + }; + for (final row in rows.keys) { + Logger('Test').severe('Upload error', ClientException(row, Uri.parse('https://immich.example.com/api/assets'))); + } + await sut.flush(); + + final stored = verify(() => mockLogRepo.insertAll(captureAny())).captured.single as List; + expect(stored.map((m) => m.error), [ + for (final row in rows.values) 'ClientException: $row, uri=[OTHER URL]/api/assets', + ]); + }); + + test('Keeps the labels of a line logged before a logout', () async { + final settings = await SettingsRepository.ensureInitialized(db); + addTearDown(SettingsRepository.reset); + await settings.write(SettingsKey.networkExternalEndpointList, ['https://immich.example.com']); + await StoreService.I.put(StoreKey.accessToken, accessToken); + await sut.dispose(); + sut = await LogService.create( + logRepository: mockLogRepo, + settingsRepository: settings, + storeService: StoreService.I, + ); + + Logger('Test').info('Resuming session at https://immich.example.com/api with $accessToken'); + // the store and settings changes of AuthService.clearLocalData + await StoreService.I.delete(StoreKey.accessToken); + await settings.clear(const [ + .networkAutoEndpointSwitching, + .networkPreferredWifiName, + .networkLocalEndpoint, + .networkExternalEndpointList, + ]); + Logger('Test').info('Resuming session at https://immich.example.com/api'); + await sut.flush(); + + final stored = verify(() => mockLogRepo.insertAll(captureAny())).captured.single as List; + expect(stored.map((m) => m.message), [ + 'Resuming session at [EXTERNAL ENDPOINT 1]/api with [ACCESS TOKEN]', + 'Resuming session at [OTHER URL]/api', + ]); + }); + }); } diff --git a/mobile/test/providers/app_life_cycle_provider_test.dart b/mobile/test/providers/app_life_cycle_provider_test.dart index abc7721a1..438d1864f 100644 --- a/mobile/test/providers/app_life_cycle_provider_test.dart +++ b/mobile/test/providers/app_life_cycle_provider_test.dart @@ -118,6 +118,7 @@ void main() { logService = await LogService.init( logRepository: logRepository, settingsRepository: settingsRepository, + storeService: MockStoreService(), shouldBuffer: false, ); });