From ad73b63fac3766de736f11fd67fd5549484bd09f Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 12 Aug 2026 14:07:01 -0500 Subject: [PATCH 1/5] fix: fail closed without vss auth Co-authored-by: Cursor --- Bitkit/Services/LightningService.swift | 24 ++++++++++-------------- Bitkit/Services/VssBackupClient.swift | 22 ++++++++++------------ Bitkit/Utilities/Errors.swift | 6 ++++++ changelog.d/next/fail-closed.security.md | 1 + 4 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 changelog.d/next/fail-closed.security.md diff --git a/Bitkit/Services/LightningService.swift b/Bitkit/Services/LightningService.swift index 16cb2a0ae..d3c2058ec 100644 --- a/Bitkit/Services/LightningService.swift +++ b/Bitkit/Services/LightningService.swift @@ -160,21 +160,17 @@ class LightningService { builder.setEntropyBip39Mnemonic(mnemonic: mnemonic, passphrase: passphrase) + guard !lnurlAuthServerUrl.isEmpty else { + throw CustomServiceError.vssAuthRequired + } + try await ServiceQueue.background(.ldk) { - if !lnurlAuthServerUrl.isEmpty { - self.node = try builder.buildWithVssStore( - vssUrl: vssUrl, - storeId: storeId, - lnurlAuthServerUrl: lnurlAuthServerUrl, - fixedHeaders: [:] - ) - } else { - self.node = try builder.buildWithVssStoreAndFixedHeaders( - vssUrl: vssUrl, - storeId: storeId, - fixedHeaders: [:] - ) - } + self.node = try builder.buildWithVssStore( + vssUrl: vssUrl, + storeId: storeId, + lnurlAuthServerUrl: lnurlAuthServerUrl, + fixedHeaders: [:] + ) } shouldReleaseLightningLock = false diff --git a/Bitkit/Services/VssBackupClient.swift b/Bitkit/Services/VssBackupClient.swift index dc7b7cf60..4f226ab45 100644 --- a/Bitkit/Services/VssBackupClient.swift +++ b/Bitkit/Services/VssBackupClient.swift @@ -87,18 +87,16 @@ class VssBackupClient { let vssUrl = Env.vssServerUrl Logger.debug("Building VSS client with vssUrl: '\(vssUrl)'", context: "VssBackupClient") - if let params = try await getLnurlAuthParams(walletIndex: walletIndex) { - try await vssNewClientWithLnurlAuth( - baseUrl: params.vssUrl, - storeId: params.storeId, - mnemonic: params.mnemonic, - passphrase: params.passphrase, - lnurlAuthServerUrl: params.lnurlAuthServerUrl - ) - } else { - let storeId = try await VssStoreIdProvider.shared.getVssStoreId(walletIndex: walletIndex) - try await vssNewClient(baseUrl: vssUrl, storeId: storeId) + guard let params = try await getLnurlAuthParams(walletIndex: walletIndex) else { + throw CustomServiceError.vssAuthRequired } + try await vssNewClientWithLnurlAuth( + baseUrl: params.vssUrl, + storeId: params.storeId, + mnemonic: params.mnemonic, + passphrase: params.passphrase, + lnurlAuthServerUrl: params.lnurlAuthServerUrl + ) Logger.info("VSS client setup with server: '\(vssUrl)'", context: "VssBackupClient") } } catch { @@ -110,7 +108,7 @@ class VssBackupClient { /// Lazily initializes the LDK VSS client (used only by the debug screen). Only runs when lnurl auth is configured. private func setupLdk(walletIndex: Int = 0) async throws { guard let params = try await getLnurlAuthParams(walletIndex: walletIndex) else { - throw AppError(message: "LDK VSS requires lnurl auth", debugMessage: "lnurlAuthServerUrl is not set") + throw CustomServiceError.vssAuthRequired } do { try await withTimeout(seconds: 30) { diff --git a/Bitkit/Utilities/Errors.swift b/Bitkit/Utilities/Errors.swift index 0e9cb5476..95e062490 100644 --- a/Bitkit/Utilities/Errors.swift +++ b/Bitkit/Utilities/Errors.swift @@ -6,6 +6,7 @@ enum CustomServiceError: LocalizedError { case nodeNotStarted case onchainWalletNotInitialized case mnemonicNotFound + case vssAuthRequired case nodeStillRunning case onchainWalletStillRunning case invalidNodeSigningMessage @@ -23,6 +24,8 @@ enum CustomServiceError: LocalizedError { return "Onchain wallet not created" case .mnemonicNotFound: return "Mnemonic not found" + case .vssAuthRequired: + return "VSS requires LNURL-auth" case .nodeStillRunning: return "Node is still running" case .onchainWalletStillRunning: @@ -144,6 +147,9 @@ struct AppError: LocalizedError { case .mnemonicNotFound: message = "Mnemonic not found" debugMessage = nil + case .vssAuthRequired: + message = "VSS requires LNURL-auth" + debugMessage = nil case .nodeStillRunning: message = "Node is still running" debugMessage = nil diff --git a/changelog.d/next/fail-closed.security.md b/changelog.d/next/fail-closed.security.md new file mode 100644 index 000000000..b4a989b09 --- /dev/null +++ b/changelog.d/next/fail-closed.security.md @@ -0,0 +1 @@ +Wallet backups no longer fall back to unauthenticated VSS when LNURL-auth is missing. From 4b65e310987a6052335039b222aef7dc923d8e13 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 12 Aug 2026 14:10:00 -0500 Subject: [PATCH 2/5] chore: rename changelog fragment Co-authored-by: Cursor --- changelog.d/next/{fail-closed.security.md => 665.security.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/next/{fail-closed.security.md => 665.security.md} (100%) diff --git a/changelog.d/next/fail-closed.security.md b/changelog.d/next/665.security.md similarity index 100% rename from changelog.d/next/fail-closed.security.md rename to changelog.d/next/665.security.md From 0ba9e6819471a4af4a35e663960b756ec2e9d372 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 12 Aug 2026 14:29:18 -0500 Subject: [PATCH 3/5] fix: treat blank vss auth urls as missing Co-authored-by: Cursor --- Bitkit/Services/LightningService.swift | 2 +- Bitkit/Services/VssBackupClient.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Bitkit/Services/LightningService.swift b/Bitkit/Services/LightningService.swift index d3c2058ec..3dec793eb 100644 --- a/Bitkit/Services/LightningService.swift +++ b/Bitkit/Services/LightningService.swift @@ -149,7 +149,7 @@ class LightningService { let storeId = try await VssStoreIdProvider.shared.getVssStoreId(walletIndex: walletIndex) let vssUrl = Env.vssServerUrl - let lnurlAuthServerUrl = Env.lnurlAuthServerUrl + let lnurlAuthServerUrl = Env.lnurlAuthServerUrl.trimmingCharacters(in: .whitespacesAndNewlines) Logger.debug("Building ldk-node with vssUrl: '\(vssUrl)'") Logger.debug("Building ldk-node with lnurlAuthServerUrl: '\(lnurlAuthServerUrl)'") diff --git a/Bitkit/Services/VssBackupClient.swift b/Bitkit/Services/VssBackupClient.swift index 4f226ab45..7392889d2 100644 --- a/Bitkit/Services/VssBackupClient.swift +++ b/Bitkit/Services/VssBackupClient.swift @@ -69,7 +69,7 @@ class VssBackupClient { private func getLnurlAuthParams(walletIndex: Int) async throws -> (vssUrl: String, storeId: String, mnemonic: String, passphrase: String?, lnurlAuthServerUrl: String)? { - let lnurlAuthServerUrl = Env.lnurlAuthServerUrl + let lnurlAuthServerUrl = Env.lnurlAuthServerUrl.trimmingCharacters(in: .whitespacesAndNewlines) guard !lnurlAuthServerUrl.isEmpty else { return nil } guard let mnemonic = try Keychain.loadString(key: .bip39Mnemonic(index: walletIndex)) else { throw CustomServiceError.mnemonicNotFound From 1073cd5ed8bdd3fad194d227f9576477fdc00b7b Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 13 Aug 2026 09:01:32 -0500 Subject: [PATCH 4/5] docs: clarify vss auth nil path Co-authored-by: Cursor --- Bitkit/Services/VssBackupClient.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Bitkit/Services/VssBackupClient.swift b/Bitkit/Services/VssBackupClient.swift index 7392889d2..3c7995d2f 100644 --- a/Bitkit/Services/VssBackupClient.swift +++ b/Bitkit/Services/VssBackupClient.swift @@ -65,7 +65,7 @@ class VssBackupClient { await ldkSetupCoordinator.reset() } - /// Returns lnurl auth params when lnurl is configured; nil otherwise. + /// Returns LNURL-auth params when configured. Callers fail closed if this returns nil. private func getLnurlAuthParams(walletIndex: Int) async throws -> (vssUrl: String, storeId: String, mnemonic: String, passphrase: String?, lnurlAuthServerUrl: String)? { From 8b05ef6590a8f42e600d9dbad78ff50264158373 Mon Sep 17 00:00:00 2001 From: benk10 Date: Mon, 17 Aug 2026 07:59:57 -0500 Subject: [PATCH 5/5] fix: retain pending channel migration on setup failure --- Bitkit/Services/MigrationsService.swift | 13 +++- Bitkit/ViewModels/WalletViewModel.swift | 30 ++++---- .../ChannelMigrationPersistenceTests.swift | 69 +++++++++++++++++++ 3 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 BitkitTests/ChannelMigrationPersistenceTests.swift diff --git a/Bitkit/Services/MigrationsService.swift b/Bitkit/Services/MigrationsService.swift index 0a248d858..1872e186a 100644 --- a/Bitkit/Services/MigrationsService.swift +++ b/Bitkit/Services/MigrationsService.swift @@ -291,7 +291,7 @@ enum RNKeychainKey { // MARK: - Channel Migration Data -struct PendingChannelMigration: Codable { +struct PendingChannelMigration: Codable, Equatable { let channelManager: Data let channelMonitors: [Data] } @@ -401,6 +401,17 @@ class MigrationsService: ObservableObject { set { setCodable(newValue, forKey: Self.rnPendingChannelMigrationKey) } } + func withPendingChannelMigration( + _ operation: (PendingChannelMigration?) async throws -> Void + ) async rethrows { + let migration = pendingChannelMigration + try await operation(migration) + + if pendingChannelMigration == migration { + pendingChannelMigration = nil + } + } + /// Stored activity data from RN remote backup for reapplying metadata after sync (persisted) var pendingRemoteActivityData: [RNActivityItem]? { get { getCodable(forKey: Self.rnPendingRemoteActivityDataKey) } diff --git a/Bitkit/ViewModels/WalletViewModel.swift b/Bitkit/ViewModels/WalletViewModel.swift index f4739765d..60ddecdab 100644 --- a/Bitkit/ViewModels/WalletViewModel.swift +++ b/Bitkit/ViewModels/WalletViewModel.swift @@ -162,23 +162,23 @@ class WalletViewModel: ObservableObject { let electrumServerUrl = electrumConfigService.getCurrentServer().fullUrl let rgsServerUrl = rgsConfigService.getCurrentServerUrl() - var channelMigration: ChannelDataMigration? - if let migration = MigrationsService.shared.pendingChannelMigration { - channelMigration = ChannelDataMigration( - channelManager: [UInt8](migration.channelManager), - channelMonitors: migration.channelMonitors.map { [UInt8]($0) } - ) - MigrationsService.shared.pendingChannelMigration = nil - } + try await MigrationsService.shared.withPendingChannelMigration { migration in + let channelMigration = migration.map { + ChannelDataMigration( + channelManager: [UInt8]($0.channelManager), + channelMonitors: $0.channelMonitors.map { [UInt8]($0) } + ) + } - await runLegacyNetworkGraphCleanupIfNeeded() + await runLegacyNetworkGraphCleanupIfNeeded() - try await lightningService.setup( - walletIndex: walletIndex, - electrumServerUrl: electrumServerUrl, - rgsServerUrl: rgsServerUrl.isEmpty ? nil : rgsServerUrl, - channelMigration: channelMigration - ) + try await lightningService.setup( + walletIndex: walletIndex, + electrumServerUrl: electrumServerUrl, + rgsServerUrl: rgsServerUrl.isEmpty ? nil : rgsServerUrl, + channelMigration: channelMigration + ) + } try await lightningService.start(onEvent: { event in Task { @MainActor in // Notify all event handlers diff --git a/BitkitTests/ChannelMigrationPersistenceTests.swift b/BitkitTests/ChannelMigrationPersistenceTests.swift new file mode 100644 index 000000000..8b262d44b --- /dev/null +++ b/BitkitTests/ChannelMigrationPersistenceTests.swift @@ -0,0 +1,69 @@ +@testable import Bitkit +import XCTest + +final class ChannelMigrationPersistenceTests: XCTestCase { + private enum SetupError: Error { + case failed + } + + private let migrations = MigrationsService.shared + + override func setUp() { + super.setUp() + migrations.pendingChannelMigration = nil + } + + override func tearDown() { + migrations.pendingChannelMigration = nil + super.tearDown() + } + + func testPendingMigrationIsRetainedWhenSetupFails() async { + let migration = makeMigration(seed: 1) + migrations.pendingChannelMigration = migration + + do { + try await migrations.withPendingChannelMigration { pendingMigration in + XCTAssertEqual(pendingMigration, migration) + throw SetupError.failed + } + XCTFail("Expected setup to fail") + } catch SetupError.failed { + } catch { + XCTFail("Unexpected error: \(error)") + } + + XCTAssertEqual(migrations.pendingChannelMigration, migration) + } + + func testPendingMigrationIsClearedAfterSetupSucceeds() async { + let migration = makeMigration(seed: 2) + migrations.pendingChannelMigration = migration + + await migrations.withPendingChannelMigration { pendingMigration in + XCTAssertEqual(pendingMigration, migration) + } + + XCTAssertNil(migrations.pendingChannelMigration) + } + + func testNewPendingMigrationIsNotClearedAfterSetupSucceeds() async { + let migration = makeMigration(seed: 3) + let replacement = makeMigration(seed: 4) + migrations.pendingChannelMigration = migration + + await migrations.withPendingChannelMigration { pendingMigration in + XCTAssertEqual(pendingMigration, migration) + migrations.pendingChannelMigration = replacement + } + + XCTAssertEqual(migrations.pendingChannelMigration, replacement) + } + + private func makeMigration(seed: UInt8) -> PendingChannelMigration { + PendingChannelMigration( + channelManager: Data([seed]), + channelMonitors: [Data([seed, seed &+ 1])] + ) + } +}