Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion Convos/Debug View/BackupDebugView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ struct BackupDebugView: View {
private func revokeVaultInstallationsAction() {
let vaultKeyStore = makeVaultKeyStore()
let vaultManager = session.vaultService as? VaultManager
runAction(title: "Revoke vault installations") { [environment] in
runAction(title: "Revoke all other vault installations") { [environment] in
let vaultIdentity = try await vaultKeyStore.loadAny()
let keepId: String? = await vaultManager?.vaultInstallationId
let count = try await XMTPInstallationRevoker.revokeOtherInstallations(
Expand Down Expand Up @@ -316,13 +316,24 @@ struct BackupDebugView: View {
)
let vaultManager = session.vaultService as? VaultManager

let capturedEnvironment = environment
let revoker: RestoreInstallationRevoker = { inboxId, signingKey, keepId in
try await XMTPInstallationRevoker.revokeOtherInstallations(
inboxId: inboxId,
signingKey: signingKey,
keepInstallationId: keepId,
environment: capturedEnvironment
)
}

return RestoreManager(
vaultKeyStore: vaultKeyStore,
identityStore: identityStore,
databaseManager: databaseManager,
archiveImporter: archiveImporter,
restoreLifecycleController: session as? any RestoreLifecycleControlling,
vaultManager: vaultManager,
installationRevoker: revoker,
environment: environment
)
}
Expand Down
47 changes: 33 additions & 14 deletions ConvosCore/Sources/ConvosCore/Backup/BackupManager.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
import Foundation
import GRDB

public enum BackupError: LocalizedError {
case broadcastKeysFailed(any Error)

public var errorDescription: String? {
switch self {
case .broadcastKeysFailed(let error):
return "Failed to broadcast conversation keys to vault: \(error.localizedDescription)"
}
}
}

public struct ConversationArchiveResult: Sendable {
public let inboxId: String
public let success: Bool
Expand Down Expand Up @@ -47,6 +58,7 @@ public actor BackupManager {
)

let outputURL = try writeToICloudOrLocal(bundleData: bundleData, metadata: metadata)
Log.info("[Backup] saved to \(outputURL.path)")
BackupBundle.cleanup(directory: stagingDir)
return outputURL
} catch {
Expand All @@ -64,23 +76,32 @@ public actor BackupManager {
try await archiveProvider.broadcastKeysToVault()
Log.info("[Backup] keys broadcast to vault")
} catch {
Log.warning("[Backup] failed to broadcast keys to vault (non-fatal): \(error)")
// Fail loud: if we can't broadcast keys to the vault, the archive may not
// contain every inbox's key. That would produce a backup that silently
// cannot be fully restored. Better to surface the failure now than create
// an incomplete bundle the user trusts.
Log.error("[Backup] failed to broadcast keys to vault: \(error)")
throw BackupError.broadcastKeysFailed(error)
}
Log.info("[Backup] creating vault archive")
try await createVaultArchive(encryptionKey: encryptionKey, in: stagingDir)
Log.info("[Backup] vault archive created")

Log.info("[Backup] creating conversation archives")
let conversationResults = await createConversationArchives(in: stagingDir)
let successCount = conversationResults.filter(\.success).count
let failedResults = conversationResults.filter { !$0.success }
Log.info("[Backup] conversation archives: \(successCount)/\(conversationResults.count) succeeded")
if !failedResults.isEmpty {
Log.warning("Failed to archive \(failedResults.count)/\(conversationResults.count) conversation(s)")
for result in failedResults {
Log.warning(" \(result.inboxId): \(result.error?.localizedDescription ?? "unknown error")")
Log.warning("[Backup] conversation archive failed for \(result.inboxId): \(result.error?.localizedDescription ?? "unknown error")")
}
}

Log.info("[Backup] copying database snapshot")
try copyDatabase(to: stagingDir)
Log.info("[Backup] database snapshot copied")

let successCount = conversationResults.filter(\.success).count
let metadata = BackupBundleMetadata(
deviceId: DeviceInfo.deviceIdentifier,
deviceName: DeviceInfo.deviceName,
Expand All @@ -90,16 +111,14 @@ public actor BackupManager {
try BackupBundleMetadata.write(metadata, to: stagingDir)

let bundleData = try BackupBundle.pack(directory: stagingDir, encryptionKey: encryptionKey)
let bundleSizeKB = bundleData.count / 1024
Log.info("[Backup] bundle packed: \(bundleSizeKB)KB, \(successCount) conversation(s), vault=true, db=true")
return (bundleData, metadata)
}

private func createVaultArchive(encryptionKey: Data, in directory: URL) async {
private func createVaultArchive(encryptionKey: Data, in directory: URL) async throws {
let archivePath = BackupBundle.vaultArchivePath(in: directory)
do {
try await archiveProvider.createVaultArchive(at: archivePath, encryptionKey: encryptionKey)
} catch {
Log.warning("[Backup] vault archive creation failed (non-fatal): \(error)")
}
try await archiveProvider.createVaultArchive(at: archivePath, encryptionKey: encryptionKey)
}

private func createConversationArchives(in directory: URL) async -> [ConversationArchiveResult] {
Expand All @@ -108,7 +127,7 @@ public actor BackupManager {
let repo = InboxesRepository(databaseReader: databaseReader)
inboxes = try repo.nonVaultUsedInboxes()
} catch {
Log.warning("Failed to load inboxes for backup: \(error)")
Log.warning("[Backup] failed to load inboxes: \(error)")
return []
}

Expand All @@ -118,7 +137,7 @@ public actor BackupManager {
do {
identity = try await identityStore.identity(for: inbox.inboxId)
} catch {
Log.warning("No identity found for inbox \(inbox.inboxId), skipping archive")
Log.warning("[Backup] no identity for inbox \(inbox.inboxId), skipping archive")
results.append(.init(inboxId: inbox.inboxId, success: false, error: error))
continue
}
Expand All @@ -132,7 +151,7 @@ public actor BackupManager {
)
results.append(.init(inboxId: inbox.inboxId, success: true, error: nil))
} catch {
Log.warning("Failed to archive conversation \(inbox.inboxId): \(error)")
Log.warning("[Backup] failed to archive conversation \(inbox.inboxId): \(error)")
results.append(.init(inboxId: inbox.inboxId, success: false, error: error))
}
}
Expand Down Expand Up @@ -179,7 +198,7 @@ public actor BackupManager {
.appendingPathComponent("backups", isDirectory: true)
.appendingPathComponent(deviceId, isDirectory: true)
try FileManager.default.createDirectory(at: localDir, withIntermediateDirectories: true)
Log.warning("iCloud container unavailable, backup saved locally")
Log.warning("[Backup] iCloud container unavailable, saved locally")
return localDir
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,11 @@ public struct ConvosRestoreArchiveImporter: RestoreArchiveImporter {
self.environment = environment
}

public func importConversationArchive(inboxId: String, path: String, encryptionKey: Data) async throws {
public func importConversationArchive(inboxId: String, path: String, encryptionKey: Data) async throws -> String {
// RestoreManager has already staged/wiped the local XMTP DBs for conversation
// inboxes before calling us, so no existing client can be reused here. Create
// a single fresh client and import the archive into it — any prior `Client.build`
// probe would register an extra installation on the network as a side effect.
let identity = try await identityStore.identity(for: inboxId)
let api = XMTPAPIOptionsBuilder.build(environment: environment)
let options = ClientOptions(
Expand All @@ -23,26 +27,29 @@ public struct ConvosRestoreArchiveImporter: RestoreArchiveImporter {
deviceSyncEnabled: false
)

do {
let client = try await Client.build(
publicIdentity: identity.keys.signingKey.identity,
options: options,
inboxId: inboxId
)
try? client.dropLocalDatabaseConnection()
Log.info("[Restore] conversation XMTP DB already exists for \(inboxId), skipping archive import")
return
} catch {
Log.info("[Restore] no existing XMTP DB for \(inboxId), importing archive")
}

let client = try await Client.create(
account: identity.keys.signingKey,
options: options
)
defer { try? client.dropLocalDatabaseConnection() }

try await client.importArchive(path: path, encryptionKey: encryptionKey)
Log.info("[Restore] conversation archive imported for \(inboxId)")

// After archive import, the XMTP SDK's consent state for restored
// groups may be 'unknown'. shouldProcessConversation in StreamProcessor
// drops messages from groups that aren't 'allowed', so the first
// incoming message after restore would be silently lost. Set consent
// to 'allowed' for all groups in this inbox's archive.
let groups = try client.conversations.listGroups()
for group in groups {
try await group.updateConsentState(state: .allowed)
}
if !groups.isEmpty {
Log.info("[Restore] set consent=allowed for \(groups.count) group(s) in \(inboxId)")
}

let newInstallationId = client.installationID
Comment on lines +43 to +51
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High Backup/ConvosRestoreArchiveImporter.swift:43

If listGroups() or any updateConsentState() call throws after client.importArchive() succeeds, the function throws without returning the installation ID. The caller in RestoreManager.importConversationArchives treats this as a failed import and excludes the inbox from imported, so stale installations aren't revoked and the new installation remains active on the network indefinitely. Consider wrapping the consent-update loop in a do-catch that logs errors but still returns newInstallationId to signal the archive import succeeded.

-        let groups = try client.conversations.listGroups()
-        for group in groups {
-            try await group.updateConsentState(state: .allowed)
-        }
-        if !groups.isEmpty {
-            Log.info("[Restore] set consent=allowed for \(groups.count) group(s) in \(inboxId)")
+        do {
+            let groups = try client.conversations.listGroups()
+            for group in groups {
+                try await group.updateConsentState(state: .allowed)
+            }
+            if !groups.isEmpty {
+                Log.info("[Restore] set consent=allowed for \(groups.count) group(s) in \(inboxId)")
+            }
+        } catch {
+            Log.error("[Restore] failed to set consent state for groups in \(inboxId): \(error)")
         }
         
         let newInstallationId = client.installationID
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file ConvosCore/Sources/ConvosCore/Backup/ConvosRestoreArchiveImporter.swift around lines 43-51:

If `listGroups()` or any `updateConsentState()` call throws after `client.importArchive()` succeeds, the function throws without returning the installation ID. The caller in `RestoreManager.importConversationArchives` treats this as a failed import and excludes the inbox from `imported`, so stale installations aren't revoked and the new installation remains active on the network indefinitely. Consider wrapping the consent-update loop in a `do-catch` that logs errors but still returns `newInstallationId` to signal the archive import succeeded.

Evidence trail:
ConvosCore/Sources/ConvosCore/Backup/ConvosRestoreArchiveImporter.swift lines 34-52 (REVIEWED_COMMIT): `importArchive()` at line 34, `listGroups()` at line 43 with `try`, `updateConsentState()` at lines 44-46 with `try await`, return at line 52.

ConvosCore/Sources/ConvosCore/Backup/RestoreManager.swift lines 464-472 (REVIEWED_COMMIT): do-catch wraps the call, inbox added to `imported` only on success (line 469), error logs warning but doesn't add to `imported`.

ConvosCore/Sources/ConvosCore/Backup/RestoreManager.swift lines 163-166 (REVIEWED_COMMIT): `importedInboxes` passed to `revokeStaleInstallationsForRestoredInboxes`.

ConvosCore/Sources/ConvosCore/Backup/RestoreManager.swift lines 485-514 (REVIEWED_COMMIT): function iterates only over `imported`, so missing inboxes won't have stale installations revoked.

Log.info("[Restore] conversation archive imported for \(inboxId) (new installationId=\(newInstallationId))")
return newInstallationId
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@ import Foundation
@preconcurrency import XMTPiOS

public struct ConvosVaultArchiveImporter: VaultArchiveImporter {
private let vaultKeyStore: VaultKeyStore
private let environment: AppEnvironment

public init(vaultKeyStore: VaultKeyStore, environment: AppEnvironment) {
self.vaultKeyStore = vaultKeyStore
self.environment = environment
}
Comment on lines 6 to 10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Low Backup/ConvosVaultArchiveImporter.swift:6

The vaultKeyStore parameter in ConvosVaultArchiveImporter.init is accepted but never stored or used. Callers pass a VaultKeyStore believing it is required for operation, but the implementation ignores it entirely — the property was removed while the parameter remains as dead code.

-    public init(vaultKeyStore: VaultKeyStore, environment: AppEnvironment) {
+    public init(environment: AppEnvironment) {
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file ConvosCore/Sources/ConvosCore/Backup/ConvosVaultArchiveImporter.swift around lines 6-10:

The `vaultKeyStore` parameter in `ConvosVaultArchiveImporter.init` is accepted but never stored or used. Callers pass a `VaultKeyStore` believing it is required for operation, but the implementation ignores it entirely — the property was removed while the parameter remains as dead code.

Evidence trail:
ConvosCore/Sources/ConvosCore/Backup/ConvosVaultArchiveImporter.swift at REVIEWED_COMMIT - lines 5-10 show the struct definition with only `environment` property and init accepting both `vaultKeyStore` and `environment` but only storing `environment`


public func importVaultArchive(from path: URL, encryptionKey: Data) async throws -> [VaultKeyEntry] {
Log.info("[Restore] loading vault identity")
let vaultIdentity = try await vaultKeyStore.loadAny()
public func importVaultArchive(
from path: URL,
encryptionKey: Data,
vaultIdentity: KeychainIdentity
) async throws -> [VaultKeyEntry] {
let api = XMTPAPIOptionsBuilder.build(environment: environment)

let codecs: [any ContentCodec] = [
Expand All @@ -26,28 +26,19 @@ public struct ConvosVaultArchiveImporter: VaultArchiveImporter {
TextCodec(),
]

let existingOptions = ClientOptions(
api: api,
codecs: codecs,
dbEncryptionKey: vaultIdentity.keys.databaseKey,
deviceSyncEnabled: false
)

if let existingClient = try? await Client.build(
publicIdentity: vaultIdentity.keys.signingKey.identity,
options: existingOptions,
inboxId: vaultIdentity.inboxId
) {
Log.info("[Restore] vault XMTP DB already exists, extracting keys from existing vault")
defer { try? existingClient.dropLocalDatabaseConnection() }
try await existingClient.conversations.sync()
return try await extractKeys(from: existingClient)
}

// Always import the archive from the backup into an isolated temp
// directory. Reusing an existing vault XMTP DB on disk is wrong
// when the keychain holds multiple vault identities (e.g. after
// iCloud Keychain sync) — loadAny() might return the restoring
// device's vault instead of the backup device's, and the existing
// DB would contain a different set of key messages.
let importDir = FileManager.default.temporaryDirectory
.appendingPathComponent("xmtp-vault-import-\(UUID().uuidString)")
try FileManager.default.createDirectory(at: importDir, withIntermediateDirectories: true)
Log.info("[Restore] no existing vault XMTP DB, importing archive into isolated directory")
defer {
try? FileManager.default.removeItem(at: importDir)
}
Log.info("[Restore] importing vault archive into isolated directory (inboxId=\(vaultIdentity.inboxId))")

let importOptions = ClientOptions(
api: api,
Expand All @@ -61,25 +52,25 @@ public struct ConvosVaultArchiveImporter: VaultArchiveImporter {
account: vaultIdentity.keys.signingKey,
options: importOptions
)
defer { try? client.dropLocalDatabaseConnection() }

Log.info("[Restore] importing vault archive (inboxId: \(client.inboxID))")
Log.info("[Restore] importing vault archive (client inboxId=\(client.inboxID))")
try await client.importArchive(path: path.path, encryptionKey: encryptionKey)
Log.info("[Restore] vault archive import succeeded")

let entries = try await extractKeys(from: client)
try? client.dropLocalDatabaseConnection()
try? FileManager.default.removeItem(at: importDir)
return entries
return try await extractKeys(from: client)
}

private func extractKeys(from client: Client) async throws -> [VaultKeyEntry] {
try await client.conversations.sync()
// Read groups and messages from the local DB only — no network sync.
// The archive import already populated the local DB with everything
// we need, and syncing would fail if the vault group was deactivated
// on the network by a previous restore's revocation step.
let groups = try client.conversations.listGroups()

Log.info("[Restore] reading messages from \(groups.count) vault group(s)")
var allMessages: [DecodedMessage] = []
for group in groups {
try await group.sync()
let messages = try await group.messages()
allMessages.append(contentsOf: messages)
}
Expand All @@ -95,7 +86,7 @@ public struct ConvosVaultArchiveImporter: VaultArchiveImporter {
}

let entries = VaultManager.extractKeyEntries(bundles: bundles, shares: shares)
Log.info("[Restore] extracted \(entries.count) key entries")
Log.info("[Restore] extracted \(entries.count) key entries from \(bundles.count) bundle(s) and \(shares.count) share(s)")
return entries
}
}
Loading
Loading