From 1be659d8f27fca107a1b62d63f461eb924a0b23f Mon Sep 17 00:00:00 2001 From: lagostimgomes Date: Mon, 25 May 2026 18:00:07 -0400 Subject: [PATCH 1/2] Tighten parsing, error handling, and Keychain hygiene - help_search_service: add 120s timeout to OpenAI response body read (same bug the notebook_search_service fix already addressed) - backup_models.BackupRunManifest.fromJson: raise a clear FormatException when createdAt is missing or non-string instead of throwing TypeError; reuse the parsed value for completedAt fallback - backup_models.RenderNotebook.fromJson: tolerate missing/invalid createdAt using DateTime.tryParse + epoch fallback - backup_models.RenderNode/RenderPart.fromJson: null-safe id casts so a corrupt render_notebook.json no longer breaks the whole load - labarchives_client.downloadNotebookBackup: wrap partial.delete() cleanup in try/catch so a failed unlink can't replace the original download error with an unrelated cleanup error - backup_service._classifyBackupError: replace lower.contains('auth') (which also matched 'author', 'authority') with the narrower 'authoriz', 'auth_code', and 'auth code' tokens - backup_service._withBackupLock: wrap lock.unlock() so a failure can't skip lock.close() and leak the file handle - ios AppDelegate: pin kSecAttrSynchronizable=false so credentials never sync to iCloud Keychain Co-Authored-By: Claude Sonnet 4.6 --- ios/Runner/AppDelegate.swift | 4 +++- lib/src/backup_models.dart | 20 ++++++++++++++------ lib/src/backup_service.dart | 10 ++++++++-- lib/src/help_search_service.dart | 6 +++++- lib/src/labarchives_client.dart | 8 ++++++-- 5 files changed, 36 insertions(+), 12 deletions(-) diff --git a/ios/Runner/AppDelegate.swift b/ios/Runner/AppDelegate.swift index dc029dc..b57fb59 100644 --- a/ios/Runner/AppDelegate.swift +++ b/ios/Runner/AppDelegate.swift @@ -55,10 +55,12 @@ import UIKit } private func keychainQuery(service: String, account: String) -> [String: Any] { + // Pin synchronizable=false so credentials never sync to iCloud Keychain. return [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, - kSecAttrAccount as String: account + kSecAttrAccount as String: account, + kSecAttrSynchronizable as String: false ] } diff --git a/lib/src/backup_models.dart b/lib/src/backup_models.dart index a77f3a9..7152e04 100644 --- a/lib/src/backup_models.dart +++ b/lib/src/backup_models.dart @@ -380,12 +380,18 @@ class BackupRunManifest { .whereType>() .map(BackupNotebookOutcome.fromJson) .toList(); + final createdAtRaw = json['createdAt']; + if (createdAtRaw is! String) { + throw const FormatException( + 'Backup run manifest is missing required date "createdAt".', + ); + } + final createdAt = DateTime.parse(createdAtRaw); return BackupRunManifest( id: json['id'] as String? ?? '', - createdAt: DateTime.parse(json['createdAt'] as String), + createdAt: createdAt, completedAt: - DateTime.tryParse(json['completedAt'] as String? ?? '') ?? - DateTime.parse(json['createdAt'] as String), + DateTime.tryParse(json['completedAt'] as String? ?? '') ?? createdAt, totalNotebookCount: json['totalNotebookCount'] as int? ?? json['notebookCount'] as int? ?? @@ -724,7 +730,9 @@ class RenderNotebook { final rawNodes = json['nodes'] as List? ?? const []; return RenderNotebook( name: json['name'] as String? ?? 'Untitled notebook', - createdAt: DateTime.parse(json['createdAt'] as String), + createdAt: + DateTime.tryParse(json['createdAt'] as String? ?? '') ?? + DateTime.fromMillisecondsSinceEpoch(0, isUtc: true), archivePath: json['archivePath'] as String? ?? '', sourceLayout: json['sourceLayout'] as String? ?? 'json', nodes: rawNodes @@ -766,7 +774,7 @@ class RenderNode { static RenderNode fromJson(Map json) { final rawParts = json['parts'] as List? ?? const []; return RenderNode( - id: json['id'] as int, + id: json['id'] as int? ?? 0, parentId: json['parentId'] as int? ?? 0, title: json['title'] as String? ?? 'Untitled', isPage: json['isPage'] as bool? ?? false, @@ -848,7 +856,7 @@ class RenderPart { static RenderPart fromJson(Map json) { return RenderPart( - id: json['id'] as int, + id: json['id'] as int? ?? 0, kindCode: json['kindCode'] as int? ?? -1, kindLabel: json['kindLabel'] as String? ?? 'Entry part', renderText: json['renderText'] as String? ?? '', diff --git a/lib/src/backup_service.dart b/lib/src/backup_service.dart index 3222aa0..5d3b66b 100644 --- a/lib/src/backup_service.dart +++ b/lib/src/backup_service.dart @@ -2281,7 +2281,11 @@ class BackupService { return await action(); } finally { if (locked) { - await lock.unlock(); + try { + await lock.unlock(); + } catch (_) { + // Still close the file handle below even if unlock fails. + } } await lock.close(); } @@ -2517,7 +2521,9 @@ class BackupService { if (lower.contains('authorization failed') || lower.contains('missing local credentials') || lower.contains('missing labarchives uid') || - lower.contains('auth')) { + lower.contains('authoriz') || + lower.contains('auth_code') || + lower.contains('auth code')) { return _BackupFailure( category: BackupFailureCategory.authorization, message: _oneLine(message), diff --git a/lib/src/help_search_service.dart b/lib/src/help_search_service.dart index 8c980fb..7c18982 100644 --- a/lib/src/help_search_service.dart +++ b/lib/src/help_search_service.dart @@ -312,7 +312,11 @@ class HelpSearchService { final response = await request.close().timeout( const Duration(seconds: 90), ); - final responseBody = await utf8.decoder.bind(response).join(); + // Timeout the body read independently; response.close() only covers headers. + final responseBody = await utf8.decoder + .bind(response) + .join() + .timeout(const Duration(seconds: 120)); if (response.statusCode < 200 || response.statusCode >= 300) { throw AiApiHttpException(response.statusCode, responseBody); } diff --git a/lib/src/labarchives_client.dart b/lib/src/labarchives_client.dart index b8bf732..55c3c23 100644 --- a/lib/src/labarchives_client.dart +++ b/lib/src/labarchives_client.dart @@ -182,8 +182,12 @@ class LabArchivesClient { } catch (_) { // Best-effort cleanup after an interrupted response. } - if (await partial.exists()) { - await partial.delete(); + try { + if (await partial.exists()) { + await partial.delete(); + } + } catch (_) { + // Don't let cleanup mask the original download failure. } rethrow; } From 49c09dde4462a4ca935366bd30b7e88a9814cc1c Mon Sep 17 00:00:00 2001 From: Xinlian Liu Date: Mon, 25 May 2026 18:11:04 -0400 Subject: [PATCH 2/2] Preserve strict render metadata parsing --- lib/src/backup_models.dart | 38 +++++++++++++++++++++---- test/backup_models_test.dart | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 5 deletions(-) diff --git a/lib/src/backup_models.dart b/lib/src/backup_models.dart index 7152e04..92e8c82 100644 --- a/lib/src/backup_models.dart +++ b/lib/src/backup_models.dart @@ -730,9 +730,7 @@ class RenderNotebook { final rawNodes = json['nodes'] as List? ?? const []; return RenderNotebook( name: json['name'] as String? ?? 'Untitled notebook', - createdAt: - DateTime.tryParse(json['createdAt'] as String? ?? '') ?? - DateTime.fromMillisecondsSinceEpoch(0, isUtc: true), + createdAt: _requiredCreatedAt(json), archivePath: json['archivePath'] as String? ?? '', sourceLayout: json['sourceLayout'] as String? ?? 'json', nodes: rawNodes @@ -742,6 +740,16 @@ class RenderNotebook { ); } + static DateTime _requiredCreatedAt(Map json) { + final value = json['createdAt']; + if (value is String) { + return DateTime.parse(value); + } + throw const FormatException( + 'Render notebook is missing required date "createdAt".', + ); + } + String toPrettyJson() => const JsonEncoder.withIndent(' ').convert(toJson()); } @@ -774,7 +782,7 @@ class RenderNode { static RenderNode fromJson(Map json) { final rawParts = json['parts'] as List? ?? const []; return RenderNode( - id: json['id'] as int? ?? 0, + id: _requiredId(json), parentId: json['parentId'] as int? ?? 0, title: json['title'] as String? ?? 'Untitled', isPage: json['isPage'] as bool? ?? false, @@ -785,6 +793,16 @@ class RenderNode { .toList(), ); } + + static int _requiredId(Map json) { + final value = json['id']; + if (value is int) { + return value; + } + throw const FormatException( + 'Render node is missing required integer "id".', + ); + } } class RenderPart { @@ -856,7 +874,7 @@ class RenderPart { static RenderPart fromJson(Map json) { return RenderPart( - id: json['id'] as int? ?? 0, + id: _requiredId(json), kindCode: json['kindCode'] as int? ?? -1, kindLabel: json['kindLabel'] as String? ?? 'Entry part', renderText: json['renderText'] as String? ?? '', @@ -874,6 +892,16 @@ class RenderPart { attachmentOriginalVersion: json['attachmentOriginalVersion'] as int?, ); } + + static int _requiredId(Map json) { + final value = json['id']; + if (value is int) { + return value; + } + throw const FormatException( + 'Render part is missing required integer "id".', + ); + } } class RenderComment { diff --git a/test/backup_models_test.dart b/test/backup_models_test.dart index f5c6edb..038485c 100644 --- a/test/backup_models_test.dart +++ b/test/backup_models_test.dart @@ -73,4 +73,58 @@ void main() { expect(records, hasLength(1)); expect(records.single.id, 'run_001_demo_lab'); }); + + test('render notebooks reject missing or invalid createdAt', () { + final valid = { + 'name': 'Demo Lab Notebook', + 'createdAt': '2026-05-14T12:00:00.000Z', + 'archivePath': 'notebooks/demo_lab/2026/05/14/run_001/notebook.7z', + 'nodes': const [], + }; + + final missingCreatedAt = Map.of(valid) + ..remove('createdAt'); + final invalidCreatedAt = Map.of(valid) + ..['createdAt'] = 'not-a-date'; + + expect( + () => RenderNotebook.fromJson(missingCreatedAt), + throwsA(isA()), + ); + expect( + () => RenderNotebook.fromJson(invalidCreatedAt), + throwsA(isA()), + ); + }); + + test('render nodes and parts reject missing structural IDs', () { + final validNode = { + 'id': 42, + 'parentId': 0, + 'title': 'Zebrafish hypoxia assay', + 'isPage': true, + 'position': 1, + 'parts': const [], + }; + final validPart = { + 'id': 7, + 'kindCode': 2, + 'kindLabel': 'Attachment', + 'renderText': 'Raw imaging export', + 'position': 1, + 'comments': const [], + }; + + final missingNodeId = Map.of(validNode)..remove('id'); + final missingPartId = Map.of(validPart)..remove('id'); + + expect( + () => RenderNode.fromJson(missingNodeId), + throwsA(isA()), + ); + expect( + () => RenderPart.fromJson(missingPartId), + throwsA(isA()), + ); + }); }