diff --git a/Furious/Repository/AGENTS.md b/Furious/Repository/AGENTS.md index 92900a5..a9fa029 100644 --- a/Furious/Repository/AGENTS.md +++ b/Furious/Repository/AGENTS.md @@ -13,7 +13,8 @@ This scope owns restoration, migration, ordering, and persistence; workflows and - A restore failure remains observable. Automatic cleanup must not replace unreadable persisted bytes with an empty fallback; only an explicit successful replacement may do so. Root decoding, individual-record hydration, and later serialization are separate failure boundaries. Test malformed records inside a valid root as well as malformed - roots; the existing root fallback is not a guarantee that every record error is recoverable. + roots. Profile and subscription hydration publishes only a complete collection; an invalid record must not + expose a partially restored prefix that cleanup can serialize over the original document. - Stage fallible decode/migration before live mutation. Subscription reconciliation currently belongs to `Furious/Service/SubscriptionSync.py` and commits through the compatibility live collection: matched managed profiles retain object/profile identity and local metadata, removed profiles become stale, and unrelated groups remain diff --git a/Furious/Repository/Servers.py b/Furious/Repository/Servers.py index c933773..a35b604 100644 --- a/Furious/Repository/Servers.py +++ b/Furious/Repository/Servers.py @@ -147,25 +147,34 @@ class UserServers(Mixins.CleanupOnExit, StorageBackend): self._data = restore() self._list = [] - records = self._data.get('model', []) + profiles = [] - for index, value in enumerate(records): - record = dict(value) + try: + for index, value in enumerate(self._data.get('model', [])): + if not isinstance(value, Mapping): + raise TypeError('server repository records must be objects') - if 'connection' in record: - connection = configurationFromAny(record.get('connection', '')) - metadata = ProfileMetadata.fromMapping(record.get('metadata', {})) - else: - connection = configurationFromAny(record.pop('config', '')) - metadata = UserServer.metadataFromMapping(record) + record = dict(value) - self._list.append( - ServerProfile.fromConfiguration( - connection, - metadata, - index=index, + if 'connection' in record: + connection = configurationFromAny(record.get('connection', '')) + metadata = ProfileMetadata.fromMapping(record.get('metadata', {})) + else: + connection = configurationFromAny(record.pop('config', '')) + metadata = UserServer.metadataFromMapping(record) + + profiles.append( + ServerProfile.fromConfiguration(connection, metadata, index=index) ) - ) + except Exception as ex: + # Any non-exit exceptions + + # Keep the original persisted bytes recoverable, never a partial prefix. + self._restoreFailed = True + + logger.error('failed to restore server records (%s)', type(ex).__name__) + else: + self._list = profiles def sync(self): """Persist the current user servers data.""" diff --git a/Furious/Repository/Subscriptions.py b/Furious/Repository/Subscriptions.py index b1b2db3..54e637e 100644 --- a/Furious/Repository/Subscriptions.py +++ b/Furious/Repository/Subscriptions.py @@ -197,17 +197,32 @@ class UserSubs(Mixins.CleanupOnExit, StorageBackend): restored = restore() - self._data = restored + self._data = {} + normalized = {} - # Normalize legacy URL-only entries into the current group schema. The - # dictionary key remains the stable group ID used by existing profiles. - for order, (unique, value) in enumerate(tuple(self._data.items())): - group = SubscriptionGroup.fromMapping(unique, value) + # Publish only a completely hydrated collection. Failed startup cleanup + # must not serialize a partly normalized document over recoverable input. + try: + for order, (unique, value) in enumerate(restored.items()): + if not isinstance(value, Mapping): + raise TypeError('subscription repository records must be objects') - if not group.sortOrder: - group.sortOrder = order + group = SubscriptionGroup.fromMapping(unique, value) - self._data[unique] = group.toMapping() + if not group.sortOrder: + group.sortOrder = order + + normalized[unique] = group.toMapping() + except Exception as ex: + # Any non-exit exceptions + + self._restoreFailed = True + + logger.error( + 'failed to restore subscription records (%s)', type(ex).__name__ + ) + else: + self._data = normalized def sync(self): """Persist the current user subs data.""" diff --git a/tests/README.md b/tests/README.md index 2c1c91a..9d0afc8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -222,3 +222,8 @@ signal endpoint destruction orders, and seven transient editor families. Repeat A null protected-list count means Nuitka does not expose that diagnostic; inspect its installed package configuration and require zero live wrappers and registry entries instead. + +### Evolution regressions and CI + +`test_repository_contracts.py` verifies all-or-nothing hydration and preservation of +original stored bytes after malformed records or plugin parsing failures. diff --git a/tests/test_repository_contracts.py b/tests/test_repository_contracts.py index a95aa34..151375a 100644 --- a/tests/test_repository_contracts.py +++ b/tests/test_repository_contracts.py @@ -339,6 +339,85 @@ class RepositoryContractTest(unittest.TestCase): self.assertEqual(Storage.UserActivatedItemIndex(), -1) self.assertEqual(settings.value('ActivatedItemIndex'), 'not-an-index') + def testMalformedRecordsPreserveOriginalStorageDuringCleanup(self): + """Reject incomplete hydration without crashing or saving a partial prefix.""" + validProfile = {'config': '{}', 'remark': 'Valid profile'} + cases = ( + ('Configuration', UserServers, {'model': [validProfile, None]}, []), + ( + 'Configuration', + UserServers, + { + 'model': [ + validProfile, + {'config': '{}', 'profileMetadata': {'tags': 42}}, + ] + }, + [], + ), + ( + 'Configuration', + UserServers, + {'model': [validProfile, {'connection': {}, 'metadata': {'tags': 42}}]}, + [], + ), + ( + 'CustomSubscription', + UserSubs, + {'valid': {'remark': 'Valid group'}, 'broken': 42}, + {}, + ), + ( + 'CustomSubscription', + UserSubs, + {'valid': {'remark': 'Valid group'}, 'broken': None}, + {}, + ), + ( + 'CustomSubscription', + UserSubs, + {'valid': {'remark': 'Valid group'}, 'broken': []}, + {}, + ), + ) + + for setting, repositoryType, payload, empty in cases: + with self.subTest(setting=setting, payload=payload), isolatedSettings(): + encoded = PyBase64Encoder.encode(UJSONEncoder.encode(payload).encode()) + AppSettings.set(setting, encoded) + + with self.assertLogs(repositoryType.__module__, level='ERROR'): + repository = repositoryType() + + self.assertEqual(repository.data(), empty) + repository.cleanup() + repository.cleanup() + self.assertEqual(AppSettings.get(setting), encoded) + + def testRecordHydrationFailureDoesNotLogPrivateInput(self): + """Parser exceptions can contain secrets; diagnostics identify only the type.""" + module = importlib.import_module('Furious.Repository.Servers') + + with isolatedSettings(): + encoded = PyBase64Encoder.encode( + UJSONEncoder.encode({'model': [{'config': '{}'}]}).encode() + ) + AppSettings.set('Configuration', encoded) + + with ( + mock.patch.object( + module, + 'configurationFromAny', + side_effect=ValueError('private-token'), + ), + self.assertLogs(module.__name__, level='ERROR') as logged, + ): + repository = UserServers() + + self.assertNotIn('private-token', '\n'.join(logged.output)) + repository.cleanup() + self.assertEqual(AppSettings.get('Configuration'), encoded) + def testFailedRestoreIsNotOverwrittenByAutomaticCleanup(self): """Preserve recoverable persisted bytes when decoding fails at startup.""" corrupt = b'eA=='