diff --git a/CHANGELOG.md b/CHANGELOG.md index 212c018..cba2a58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ All notable changes to VaultSync are documented here. ### Fixed +- **Cloud Relay no longer continues from malformed saved Relay device IDs** ([#161](https://github.com/psimaker/vaultsync/issues/161)) — VaultSync reports that Cloud Relay provisioning did not complete and sends no provisioning request, without rewriting or deleting the stored value. Valid existing JSON and legacy records remain unchanged. - **Push and Cloud Relay registration details now survive failed secure-storage updates** ([#148](https://github.com/psimaker/vaultsync/issues/148)) — VaultSync keeps the last valid value when a replacement cannot be saved and reports the failure instead of continuing as if registration succeeded. - **Folder access now stays intact when reconnecting or syncing in the background** ([#147](https://github.com/psimaker/vaultsync/issues/147)) — reselecting the same Obsidian folder no longer accumulates access claims. Switching folders takes effect only after the new location is readable, scanned, and its permission is saved; any failure keeps the previous folder connected. Background runs release only their own access on completion, restart, or cancellation. - **Background sync no longer reports unfinished work as completed** ([#146](https://github.com/psimaker/vaultsync/issues/146)) — continued processing now reports success only after every expected vault is confirmed fully idle. If the sync engine stops, vault status cannot be read, a vault reports an error, the run expires or is cancelled, or the app returns to the foreground, the background run reports failure instead; conflict checks happen only after idle is proven. diff --git a/ios/VaultSync/Services/SubscriptionManager.swift b/ios/VaultSync/Services/SubscriptionManager.swift index d09ed0f..d7743ac 100644 --- a/ios/VaultSync/Services/SubscriptionManager.swift +++ b/ios/VaultSync/Services/SubscriptionManager.swift @@ -117,14 +117,23 @@ enum RelayDeviceIDStorage { } static func decodeStoredValue(_ stored: String) -> LoadResult { - if stored.hasPrefix("[") { + let ids: [String] + if stored.trimmingCharacters(in: .whitespacesAndNewlines).hasPrefix("[") { guard let data = stored.data(using: .utf8), - let ids = try? JSONDecoder().decode([String].self, from: data) else { + let decoded = try? JSONDecoder().decode([String].self, from: data) else { return .failed } - return .loaded(ids) + ids = decoded + } else { + ids = stored.components(separatedBy: ",") + } + + guard ids.allSatisfy({ SyncthingDeviceID.canonicalize($0) != nil }) else { + return .failed } - return .loaded(stored.components(separatedBy: ",").filter { !$0.isEmpty }) + // Validation must not become an implicit Keychain migration: preserve + // every valid stored value exactly as read. + return .loaded(ids) } static func remainingFailedIDs( diff --git a/ios/VaultSyncTests/KeychainServiceTests.swift b/ios/VaultSyncTests/KeychainServiceTests.swift index f330b80..8441346 100644 --- a/ios/VaultSyncTests/KeychainServiceTests.swift +++ b/ios/VaultSyncTests/KeychainServiceTests.swift @@ -301,6 +301,25 @@ struct APNsDeviceTokenRegistrationTests { @MainActor @Suite("Relay device ID persistence (#148)") struct RelayDeviceIDStorageTests { + private static let firstValidDeviceID = TestSupport.samplePeerDeviceID + private static let secondValidDeviceID = + "P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ2" + private static let invalidDeviceID = + "P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ3" + + @MainActor + private final class ProvisionRecorder { + var deviceIDs: [String] = [] + + func provision(deviceID: String, token: String, signedTransaction: String) async throws { + deviceIDs.append(deviceID) + } + } + + private static func json(_ deviceIDs: [String]) throws -> String { + String(decoding: try JSONEncoder().encode(deviceIDs), as: UTF8.self) + } + @Test("Encoding failure never calls the Keychain writer") func issue148RelayEncodingFailureSkipsKeychain() { var writeCalls: [String] = [] @@ -373,6 +392,127 @@ struct RelayDeviceIDStorageTests { #expect(RelayDeviceIDStorage.decodeStoredValue("[not-json") == .failed) } + @Test("Valid JSON storage loads every device ID unchanged (#161)") + func issue161ValidJSONStorageLoads() throws { + let stored = try Self.json([Self.firstValidDeviceID, Self.secondValidDeviceID]) + + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .loaded([ + Self.firstValidDeviceID, + Self.secondValidDeviceID, + ])) + } + + @Test("Valid JSON with leading whitespace loads unchanged (#161)") + func issue161WhitespacePrefixedJSONStorageLoads() throws { + let stored = " \n\t" + (try Self.json([ + Self.firstValidDeviceID, + Self.secondValidDeviceID, + ])) + + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .loaded([ + Self.firstValidDeviceID, + Self.secondValidDeviceID, + ])) + } + + @Test("A JSON array containing only an invalid device ID fails closed (#161)") + func issue161InvalidJSONMemberFailsClosed() throws { + let stored = try Self.json([Self.invalidDeviceID]) + + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .failed) + } + + @Test("A mixed-validity JSON array fails as one indivisible read (#161)") + func issue161MixedJSONFailsClosed() throws { + let stored = try Self.json([Self.firstValidDeviceID, Self.invalidDeviceID]) + + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .failed) + } + + @Test("Valid legacy comma-separated storage loads every device ID unchanged (#161)") + func issue161ValidLegacyStorageLoads() { + let stored = [Self.firstValidDeviceID, Self.secondValidDeviceID].joined(separator: ",") + + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .loaded([ + Self.firstValidDeviceID, + Self.secondValidDeviceID, + ])) + } + + @Test("An invalid legacy member fails the entire read closed (#161)") + func issue161InvalidLegacyMemberFailsClosed() { + let stored = [Self.firstValidDeviceID, Self.invalidDeviceID].joined(separator: ",") + + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .failed) + } + + @Test("Any empty legacy member fails the entire read closed (#161)") + func issue161EmptyLegacyMemberFailsClosed() { + let malformedValues = [ + "", + ",\(Self.firstValidDeviceID)", + "\(Self.firstValidDeviceID),", + "\(Self.firstValidDeviceID),,\(Self.secondValidDeviceID)", + ",,,", + ] + + for stored in malformedValues { + #expect(RelayDeviceIDStorage.decodeStoredValue(stored) == .failed) + } + } + + @Test("Invalid stored IDs cannot be rewritten or provisioned (#161)") + func issue161InvalidStorageSkipsWriteAndProvisioning() async throws { + let stored = try Self.json([Self.firstValidDeviceID, Self.invalidDeviceID]) + var encodeCount = 0 + var writeCount = 0 + let environment = RelayDeviceIDStorage.Environment( + load: { RelayDeviceIDStorage.decodeStoredValue(stored) }, + encode: { _ in + encodeCount += 1 + return "encoded" + }, + write: { _ in + writeCount += 1 + return true + } + ) + + let persistence = RelayDeviceIDStorage.persist( + [Self.secondValidDeviceID], + environment: environment + ) + let preparation = RelayDeviceIDStorage.prepareTarget( + source: .storedDeviceIDs, + persist: { _ in + Issue.record("A stored-device-ID target must not invoke persistence") + return .failed(.keychain) + }, + load: environment.load + ) + + let provisionRecorder = ProvisionRecorder() + if case .ready(let deviceIDs) = preparation { + let entitlement = try #require(RelayVerifiedEntitlement( + signedTransaction: "header.payload.signature" + )) + _ = await RelayReprovisioning.run( + trigger: .tokenRotation, + deviceIDs: deviceIDs, + statuses: [:], + entitlement: .verified(entitlement), + apnsToken: "token", + provision: provisionRecorder.provision + ) + } + + #expect(persistence == .failed(.read)) + #expect(preparation == .skip(nil)) + #expect(encodeCount == 0) + #expect(writeCount == 0) + #expect(provisionRecorder.deviceIDs.isEmpty) + } + @Test("Every caller context blocks its production target after storage failure") func issue148EveryRelayCallerHandlesPersistenceFailure() { #expect(RelayDeviceIDStorage.Context.allCases.count == 5) diff --git a/ios/VaultSyncTests/SubscriptionManagerTests.swift b/ios/VaultSyncTests/SubscriptionManagerTests.swift index 2d45232..c127c5d 100644 --- a/ios/VaultSyncTests/SubscriptionManagerTests.swift +++ b/ios/VaultSyncTests/SubscriptionManagerTests.swift @@ -5,6 +5,9 @@ import Testing @MainActor @Suite("Relay Provision State Machine", .serialized) struct SubscriptionManagerTests { + private static let invalidStoredDeviceJSON = + "[\"P56IOI7-MZJNU2Y-IQGDREY-DM2MGTI-MGL3BXN-PQ6W5BM-TBBZ4TJ-XZWICQ3\"]" + @Test("RelayProvisionStatus exposes stable summary contract") func relayProvisionStatusContract() { #expect(RelayProvisionStatus.notAttempted.stateKey == "not_attempted") @@ -66,4 +69,39 @@ struct SubscriptionManagerTests { ) #expect(manager.relayProvisioningNeedsAttention) } + + @Test("Invalid stored IDs use the existing generic storage failure without rewrite (#161)") + func issue161InvalidStoredIDsSurfaceGenericFailure() async { + TestSupport.resetRelayState() + defer { TestSupport.resetRelayState() } + + var encodeCount = 0 + var writeCount = 0 + let manager = SubscriptionManager( + relayDeviceIDStorageEnvironment: RelayDeviceIDStorage.Environment( + load: { + RelayDeviceIDStorage.decodeStoredValue(Self.invalidStoredDeviceJSON) + }, + encode: { _ in + encodeCount += 1 + return "encoded" + }, + write: { _ in + writeCount += 1 + return true + } + ), + startsLiveWork: false + ) + + await manager.retryRelayProvisioning(homeserverDeviceIDs: []) + + #expect( + manager.relayDeviceIDStorageErrorMessage + == L10n.tr("Cloud Relay provisioning did not complete.") + ) + #expect(manager.relayProvisioningNeedsAttention) + #expect(encodeCount == 0) + #expect(writeCount == 0) + } }