diff --git a/Memory/ARCHITECTURE.md b/Memory/ARCHITECTURE.md index 3623575d..af7f75fe 100644 --- a/Memory/ARCHITECTURE.md +++ b/Memory/ARCHITECTURE.md @@ -591,7 +591,11 @@ the application models without entering the local mutation path. Web Search and Terminal preferences are migrated from UserDefaults once, then mirrored there for existing runtime readers. Active agent runs retain their configuration snapshots. Conversations, approval mode, local MCP listener settings and other app preferences -are excluded. Only explicitly selected settings are projected, never all defaults. +are excluded. The listener's enabled state, port and exposed-tool selection remain in +device-local `UserDefaults`; its bearer token uses a dedicated this-device-only Data +Protection Keychain path. Older synchronized listener tokens are copied and verified +locally before their iCloud Keychain copy is removed. Only explicitly selected settings +are projected, never all defaults. The journal retains tombstones and unresolved references across partial fetches. Preexisting defaults use baseline versions and yield to cloud records. Built-in @@ -599,14 +603,14 @@ skills use stable IDs; migration preserves customized skills and records deleted built-ins. Duplicate skill names or MCP namespaces retain their data but only the lowest UUID is eligible for execution until the conflict is resolved. -AI credentials use `AISecretStore`. First Settings sync checks the iCloud account -before migrating local Keychain entries. A verified cloud copy and durable local +AI provider and remote MCP client credentials use `AISecretStore`. First Settings sync +checks the iCloud account before migrating local Keychain entries. A verified cloud copy and durable local storage marker precede local-copy cleanup. Migrated entries remain in iCloud Keychain when configuration sync pauses. Production reads do not cache secrets; missing synchronized credentials may arrive later. Pending credential deletion markers prevent delayed Keychain deliveries from reviving a cleared value. -Explicit credentials stay out of CloudKit; custom MCP headers and custom terminal -configuration are ordinary user-authored settings. The private-account sync +The built-in MCP listener token never enters this path. Explicit credentials stay out +of CloudKit; custom MCP headers and custom terminal configuration are ordinary user-authored settings. The private-account sync adapter is the only additional trusted source of skill-library snapshots. ## Mobile presentation and workspace activities diff --git a/Memory/SETTINGS_SYNC_RELEASE.md b/Memory/SETTINGS_SYNC_RELEASE.md index eaeb9479..f41620f4 100644 --- a/Memory/SETTINGS_SYNC_RELEASE.md +++ b/Memory/SETTINGS_SYNC_RELEASE.md @@ -27,6 +27,9 @@ Before shipping the app and privacy policy 1.4 together: independent. Repeat with the master disabled and after Pro expiration. - Check light/dark themes, fonts, custom terminal text and shell-integration changes on existing windows; ensure conversations and approval mode stay local. +- With different built-in MCP listener settings on both Macs, verify that enabled + state, port, exposed tools and bearer token remain independent. An older synchronized + listener token must be verified locally before its iCloud Keychain copy is removed. - Match the privacy policy effective date to the actual release date before publishing the website. The prepared date is September 10, 2026. diff --git a/TFM/App/AppDependencies.swift b/TFM/App/AppDependencies.swift index 7a147c94..2b6c87c6 100644 --- a/TFM/App/AppDependencies.swift +++ b/TFM/App/AppDependencies.swift @@ -17,6 +17,7 @@ struct AppDependencies { let settingsSync: SettingsSyncCoordinator? let vaultSync: VaultSyncCoordinator? let forwardingDefaults: UserDefaults? + let mcpServerSecrets: any SecretStoring init( hostRepository: any HostRepository, outboxRunner: OutboxRunner?, @@ -25,7 +26,8 @@ struct AppDependencies { agentRunRepository: (any AgentRunPersisting)? = nil, vaultSync: VaultSyncCoordinator? = nil, settingsSync: SettingsSyncCoordinator? = nil, - forwardingDefaults: UserDefaults? = nil + forwardingDefaults: UserDefaults? = nil, + mcpServerSecrets: any SecretStoring = InMemorySecretStore() ) { self.hostRepository = hostRepository self.outboxRunner = outboxRunner @@ -35,6 +37,7 @@ struct AppDependencies { self.vaultSync = vaultSync self.settingsSync = settingsSync self.forwardingDefaults = forwardingDefaults + self.mcpServerSecrets = mcpServerSecrets } static func inMemory(hosts: [Host], groups: [HostGroup]) -> Self { diff --git a/TFM/App/AppModel.swift b/TFM/App/AppModel.swift index c76fc40d..451a65a7 100644 --- a/TFM/App/AppModel.swift +++ b/TFM/App/AppModel.swift @@ -203,7 +203,8 @@ final class AppModel { ) outboxRunner = dependencies.outboxRunner mcpServer = MCPServerController( - ai: ai, hosts: agentHosts, metrics: serverMetrics, connections: agentConnections, + secrets: dependencies.mcpServerSecrets, hosts: agentHosts, metrics: serverMetrics, + connections: agentConnections, authentication: agentAuthentication, background: agentBackgroundCommands, externalAuthentication: externalAuthentication, history: externalToolCalls, canUsePro: { proAccess.canUse(.mcpServer) }, diff --git a/TFM/App/DesktopSettingsKey.swift b/TFM/App/DesktopSettingsKey.swift index f261fa8e..4b804bdd 100644 --- a/TFM/App/DesktopSettingsKey.swift +++ b/TFM/App/DesktopSettingsKey.swift @@ -18,6 +18,7 @@ nonisolated enum DesktopSettingsKey { /// App-wide appearance. `system` inherits changes from macOS. static let appAppearance = "app.appearance" + // Built-in MCP listener preferences are device-local and never enter Settings sync. static let mcpServerEnabled = "mcpServer.enabled" static let mcpServerPort = "mcpServer.port" static let mcpServerDisabledTools = "mcpServer.disabledTools" diff --git a/TFM/App/MCPServerController.swift b/TFM/App/MCPServerController.swift index 0d46c0f0..c2d0c99c 100644 --- a/TFM/App/MCPServerController.swift +++ b/TFM/App/MCPServerController.swift @@ -6,8 +6,6 @@ import TermindRemoteInfrastructure @MainActor @Observable final class MCPServerController { - static let tokenReference = "termind.ai.mcpserver.token" - enum Status: Equatable { case stopped, starting case ready(port: UInt16) @@ -25,7 +23,7 @@ final class MCPServerController { } private let defaults: UserDefaults - private let ai: AIConfigurationModel + private let tokenStore: MCPServerTokenStore private let metrics: any MetricsSnapshotReading private let hosts: AgentHostResolver private let connections: AgentConnectionPool @@ -43,9 +41,11 @@ final class MCPServerController { private(set) var isEnabled: Bool private(set) var port: Int private(set) var disabledTools: Set + private(set) var token = "" + private(set) var tokenStorageError: String? init( - ai: AIConfigurationModel, hosts: AgentHostResolver, metrics: any MetricsSnapshotReading, + secrets: any SecretStoring, hosts: AgentHostResolver, metrics: any MetricsSnapshotReading, connections: AgentConnectionPool, authentication: AgentSSHAuthenticationRegistry, background: AgentBackgroundCommands, @@ -55,7 +55,7 @@ final class MCPServerController { requestPro: @escaping @MainActor () -> Bool = { true }, defaults: UserDefaults = .standard ) { - self.ai = ai + tokenStore = MCPServerTokenStore(secrets: secrets) self.metrics = metrics self.hosts = hosts self.connections = connections @@ -79,6 +79,7 @@ final class MCPServerController { history: history, disabledTools: initialDisabledTools ) protocolServer = MCPServerProtocol(tools: tools) + reloadToken() Task { [weak self, protocolServer] in await protocolServer.setClientCountHandler { [weak self] count in Task { @MainActor in self?.connectedClients = count } @@ -87,10 +88,6 @@ final class MCPServerController { if isEnabled, canUsePro() { Task { [weak self] in await self?.start() } } } - var token: String { - ai.secret(for: Self.tokenReference) ?? "" - } - func setEnabled(_ enabled: Bool) { if enabled, !requestPro() { return } isEnabled = enabled @@ -135,9 +132,13 @@ final class MCPServerController { status = .failed(String(localized: "Unable to generate a secure token.")) return } - ai.setSecret(generated, for: Self.tokenReference) - guard ai.secretStorageError == nil else { - status = .failed(ai.secretStorageError ?? String(localized: "Unable to store the token.")) + do { + try tokenStore.save(generated) + token = generated + tokenStorageError = nil + } catch { + tokenStorageError = error.localizedDescription + status = .failed(error.localizedDescription) return } Task { [weak self] in @@ -148,6 +149,8 @@ final class MCPServerController { } func ensureToken() -> String? { + if token.isEmpty { reloadToken() } + guard tokenStorageError == nil else { return nil } if !token.isEmpty { return token } rotateToken() return token.isEmpty ? nil : token @@ -204,6 +207,16 @@ final class MCPServerController { .intersection(MCPServerTools.exportedNames) } + private func reloadToken() { + do { + token = try tokenStore.load() ?? "" + tokenStorageError = nil + } catch { + tokenStorageError = error.localizedDescription + status = .failed(error.localizedDescription) + } + } + private static func makeToken() throws -> String { var bytes = [UInt8](repeating: 0, count: 32) guard SecRandomCopyBytes(kSecRandomDefault, bytes.count, &bytes) == errSecSuccess else { diff --git a/TFM/App/TermindApp.swift b/TFM/App/TermindApp.swift index f7d90b03..240ef7eb 100644 --- a/TFM/App/TermindApp.swift +++ b/TFM/App/TermindApp.swift @@ -104,7 +104,7 @@ struct TermindApp: App { context: container.mainContext ), vaultSync: vaultSync, settingsSync: settingsSync, - forwardingDefaults: .standard + forwardingDefaults: .standard, mcpServerSecrets: secrets ), proAccess: proAccess, appReview: .live() diff --git a/TFM/Features/Settings/MCPServerSettings.swift b/TFM/Features/Settings/MCPServerSettings.swift index 45cba595..64152d52 100644 --- a/TFM/Features/Settings/MCPServerSettings.swift +++ b/TFM/Features/Settings/MCPServerSettings.swift @@ -70,6 +70,11 @@ struct MCPServerSettings: View { .foregroundStyle(.secondary) } } + if let error = server.tokenStorageError { + Text(error) + .font(.caption) + .foregroundStyle(.red) + } } clientSetupSection diff --git a/TFM/Infrastructure/Security/MCPServerTokenStore.swift b/TFM/Infrastructure/Security/MCPServerTokenStore.swift new file mode 100644 index 00000000..f6b2f752 --- /dev/null +++ b/TFM/Infrastructure/Security/MCPServerTokenStore.swift @@ -0,0 +1,54 @@ +import Foundation +import TermindApplication +import TermindDomain + +/// Keeps the built-in MCP server credential on this Mac, independently of AI +/// provider and remote MCP client credential sync. +nonisolated struct MCPServerTokenStore: Sendable { + static let reference = "termind.ai.mcpserver.token" + + private let secrets: any SecretStoring + + init(secrets: any SecretStoring) { + self.secrets = secrets + } + + /// Moves the value written by older releases out of iCloud Keychain. + /// + /// A local value always wins when both locations contain a token. When only + /// the synchronized copy exists, it is verified locally before that source + /// is removed. A failed copy or delete is safe to retry on the next read. + func load() throws -> String? { + let local = try secrets.string(for: Self.reference, storage: .thisDevice) + guard let synchronized = try secrets.string(for: Self.reference, storage: .iCloud) else { + return local + } + + let value: String + if let local { + value = local + } else { + try secrets.setString(synchronized, for: Self.reference, storage: .thisDevice) + guard try secrets.string(for: Self.reference, storage: .thisDevice) == synchronized else { + throw CocoaError(.coderInvalidValue) + } + value = synchronized + } + + try secrets.setString(nil, for: Self.reference, storage: .iCloud) + return value + } + + func save(_ value: String) throws { + let previous = try load() + do { + try secrets.setString(value, for: Self.reference, storage: .thisDevice) + guard try secrets.string(for: Self.reference, storage: .thisDevice) == value else { + throw CocoaError(.coderInvalidValue) + } + } catch { + try? secrets.setString(previous, for: Self.reference, storage: .thisDevice) + throw error + } + } +} diff --git a/TFMTests/ArchitectureBoundaryTests.swift b/TFMTests/ArchitectureBoundaryTests.swift index 0c719cbb..d6d2fe82 100644 --- a/TFMTests/ArchitectureBoundaryTests.swift +++ b/TFMTests/ArchitectureBoundaryTests.swift @@ -87,9 +87,16 @@ struct ArchitectureBoundaryTests { let records = try String( contentsOf: sourceRoot.appending(path: "../Frameworks/TermindDomain/Settings/SettingsSyncTypes.swift"), encoding: .utf8) - for excluded in ["AgentPermissionMode", "AgentRun", "AgentConversation", "mcpServerPort", "mcpServerEnabled"] { + for excluded in [ + "AgentPermissionMode", "AgentRun", "AgentConversation", "mcpServerPort", "mcpServerEnabled", + "mcpServerDisabledTools", "mcpserver.token", + ] { #expect(!records.contains(excluded)) } + let localServer = try String( + contentsOf: sourceRoot.appending(path: "App/MCPServerController.swift"), encoding: .utf8) + #expect(localServer.contains("MCPServerTokenStore")) + #expect(!localServer.contains("AIConfigurationModel")) let terminal = try String( contentsOf: sourceRoot.appending( path: "../Frameworks/TermindPersistence/TerminalPreferences+Defaults.swift"), diff --git a/TFMTests/MCPServerTokenStoreTests.swift b/TFMTests/MCPServerTokenStoreTests.swift new file mode 100644 index 00000000..5bf294da --- /dev/null +++ b/TFMTests/MCPServerTokenStoreTests.swift @@ -0,0 +1,108 @@ +import Foundation +import TermindApplication +import TermindDomain +import Testing + +@testable import Termind + +@Suite +struct MCPServerTokenStoreTests { + @Test + func savesNewTokensOnlyOnThisDevice() throws { + let secrets = MCPServerTokenTestSecrets() + let store = MCPServerTokenStore(secrets: secrets) + + try store.save("local-token") + + #expect(secrets.value(storage: .thisDevice) == "local-token") + #expect(secrets.value(storage: .iCloud) == nil) + #expect(secrets.synchronizedValuesWritten.isEmpty) + } + + @Test + func migratesACloudTokenOnlyAfterVerifyingTheLocalCopy() throws { + let secrets = MCPServerTokenTestSecrets(cloud: "legacy-token") + let store = MCPServerTokenStore(secrets: secrets) + + #expect(try store.load() == "legacy-token") + #expect(secrets.value(storage: .thisDevice) == "legacy-token") + #expect(secrets.value(storage: .iCloud) == nil) + } + + @Test + func localTokenWinsWhenBothLocationsContainValues() throws { + let secrets = MCPServerTokenTestSecrets(local: "device-token", cloud: "shared-token") + let store = MCPServerTokenStore(secrets: secrets) + + #expect(try store.load() == "device-token") + #expect(secrets.value(storage: .thisDevice) == "device-token") + #expect(secrets.value(storage: .iCloud) == nil) + } + + @Test + func failedLocalCopyKeepsTheOnlyCloudToken() { + let secrets = MCPServerTokenTestSecrets(cloud: "legacy-token") + secrets.failLocalWrites = true + let store = MCPServerTokenStore(secrets: secrets) + + #expect(throws: (any Error).self) { try store.load() } + #expect(secrets.value(storage: .thisDevice) == nil) + #expect(secrets.value(storage: .iCloud) == "legacy-token") + } + + @Test + func failedCloudCleanupRetainsBothCopiesAndCanRetry() throws { + let secrets = MCPServerTokenTestSecrets(cloud: "legacy-token") + secrets.failCloudDeletes = true + let store = MCPServerTokenStore(secrets: secrets) + + #expect(throws: (any Error).self) { try store.load() } + #expect(secrets.value(storage: .thisDevice) == "legacy-token") + #expect(secrets.value(storage: .iCloud) == "legacy-token") + + secrets.failCloudDeletes = false + #expect(try store.load() == "legacy-token") + #expect(secrets.value(storage: .iCloud) == nil) + } +} + +private nonisolated final class MCPServerTokenTestSecrets: SecretStoring, @unchecked Sendable { + private let lock = NSLock() + private var values: [PasswordStorage: String] = [:] + private(set) var synchronizedValuesWritten: [String] = [] + var failLocalWrites = false + var failCloudDeletes = false + + init(local: String? = nil, cloud: String? = nil) { + values[.thisDevice] = local + values[.iCloud] = cloud + } + + func value(storage: PasswordStorage) -> String? { + lock.withLock { values[storage] } + } + + func string(for account: String, storage: PasswordStorage) throws -> String? { + lock.withLock { values[storage] } + } + + func setString(_ value: String?, for account: String, storage: PasswordStorage) throws { + try lock.withLock { + if storage == .thisDevice, value != nil, failLocalWrites { + throw CocoaError(.fileWriteNoPermission) + } + if storage == .iCloud, value == nil, failCloudDeletes { + throw CocoaError(.fileWriteNoPermission) + } + values[storage] = value + if storage == .iCloud, let value { synchronizedValuesWritten.append(value) } + } + } + + func keyCredential(for account: String, storage: KeyStorage) throws -> StoredSSHKeyCredential? { nil } + func setKeyCredential(_ value: StoredSSHKeyCredential?, for account: String, storage: KeyStorage) throws {} + func removePassword(_ account: String, storage: PasswordStorage) throws { + try setString(nil, for: account, storage: storage) + } + func removeKeyCredential(_ account: String, storage: KeyStorage) throws {} +} diff --git a/TFMTests/SettingsSyncTests.swift b/TFMTests/SettingsSyncTests.swift index 18f97609..194dff33 100644 --- a/TFMTests/SettingsSyncTests.swift +++ b/TFMTests/SettingsSyncTests.swift @@ -204,6 +204,7 @@ struct SettingsSyncTests { let json = String(decoding: data, as: UTF8.self) #expect(!json.contains("permissionMode")) #expect(!json.contains("lastConnectedAt")) + #expect(!json.contains(MCPServerTokenStore.reference)) let record = try SettingsSyncCoordinator.makeRecord( SettingsOutgoingEntry(envelope: value), recordID: CKRecord.ID( @@ -426,6 +427,24 @@ struct SettingsSyncTests { #expect(try secrets.string(for: ref) == nil) } + @Test + func remoteMCPTokenMigratesWithoutSelectingTheBuiltInServerToken() throws { + let (container, _, _) = try fixture() + let keychain = SyncTestSecrets() + let secrets = AISecretStore(context: container.mainContext, keychain: keychain) + let remoteReference = "termind.ai.mcp.remote" + try secrets.setString("remote", for: remoteReference) + try keychain.setString( + "listener", for: MCPServerTokenStore.reference, storage: .thisDevice) + + secrets.uploadsAuthorized = true + try secrets.migrate([remoteReference]) + + #expect(try keychain.string(for: remoteReference, storage: .iCloud) == "remote") + #expect(try keychain.string(for: MCPServerTokenStore.reference, storage: .thisDevice) == "listener") + #expect(try keychain.string(for: MCPServerTokenStore.reference, storage: .iCloud) == nil) + } + @Test func remoteKeyCanArriveAfterConfigurationWithoutUsingStaleLocalCopy() throws { let (container, _, _) = try fixture()