From 7900ff4731d522a3dd884906809d7b95afa604c6 Mon Sep 17 00:00:00 2001 From: Yuval Dinodia <102706514+yetval@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:49:46 -0400 Subject: [PATCH] fix(macos): preserve device identity storage (#101105) * fix(macos): preserve device identity storage Avoid selecting App Group identity storage when the running app lacks the matching application-groups entitlement, so unentitled macOS builds fall back to the legacy writable identity directory. Existing unrecognized identity files are now preserved instead of being replaced by a newly generated identity, matching the no-overwrite behavior already used for recognized invalid identity data. * fix(macos): repair identity CI coverage Limit the App Group entitlement probe to macOS builds so iOS keeps its existing App Group path behavior. Update the state-dir identity regression to assert the new no-overwrite contract for invalid identity-shaped files. * fix(macos): migrate existing app group identity on legacy fallback Unentitled macOS builds now migrate a readable App Group device identity into the selected legacy store before falling back, so an upgrade preserves the existing device id instead of minting a new one. * fix(macos): migrate device auth store with identity on app group fallback The app group to legacy identity migration now carries the sibling device-auth store file, so an unentitled build keeps its stored device tokens (keyed by the migrated deviceId) instead of re-pairing. The process-immutable entitlement check is resolved once per process rather than creating a SecTask on every state-dir lookup, and the migration source API returns an identity+auth pair struct. --------- Co-authored-by: Peter Steinberger --- .../Sources/OpenClawKit/DeviceIdentity.swift | 131 +++++++++++- .../DeviceIdentityStoreTests.swift | 186 ++++++++++++++++++ src/infra/device-identity.state-dir.test.ts | 26 +-- src/infra/device-identity.test.ts | 26 +++ src/infra/device-identity.ts | 4 + 5 files changed, 355 insertions(+), 18 deletions(-) diff --git a/apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift b/apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift index 16765d6304cb..475bfb6a8bd3 100644 --- a/apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift +++ b/apps/shared/OpenClawKit/Sources/OpenClawKit/DeviceIdentity.swift @@ -1,5 +1,8 @@ import CryptoKit import Foundation +#if canImport(Security) +import Security +#endif public enum GatewayDeviceIdentityProfile: String, Sendable { case primary @@ -46,11 +49,18 @@ public struct DeviceIdentity: Codable, Sendable { enum DeviceIdentityPaths { private static let stateDirEnv = ["OPENCLAW_STATE_DIR"] + /// Entitlements are baked into the code signature, so resolve the gate once per process. + /// Every identity load and DeviceAuthStore read/write resolves the state dir through here; + /// re-creating a SecTask each time is wasted work for a process-immutable fact. + private static let appGroupStateDirAvailable = + DeviceIdentityPaths.hasAppGroupEntitlement(OpenClawAppGroup.identifier) + static func stateDirURL() -> URL { self.stateDirURL( overrideURL: self.stateDirOverrideURL(), legacyStateDirURL: self.legacyStateDirURL(), appGroupStateDirURL: self.appGroupStateDirURL(), + appGroupStateDirAvailable: self.appGroupStateDirAvailable, temporaryDirectory: FileManager.default.temporaryDirectory) } @@ -58,12 +68,13 @@ enum DeviceIdentityPaths { overrideURL: URL?, legacyStateDirURL: URL?, appGroupStateDirURL: URL?, + appGroupStateDirAvailable: Bool = true, temporaryDirectory: URL) -> URL { if let overrideURL { return overrideURL } - if let appGroupStateDirURL { + if appGroupStateDirAvailable, let appGroupStateDirURL { return appGroupStateDirURL } if let legacyStateDirURL { @@ -91,6 +102,30 @@ enum DeviceIdentityPaths { return nil } + private static func hasAppGroupEntitlement(_ identifier: String) -> Bool { + // macOS resolves containerURL(forSecurityApplicationGroupIdentifier:) even without the + // App Groups entitlement, but macOS 15+ gates actual access behind a user consent prompt. + // Unentitled builds (the shipped mac app) must not depend on that container. iOS requires + // the entitlement for containerURL to resolve at all, so the gate is macOS-only. + #if os(macOS) && canImport(Security) + guard + let task = SecTaskCreateFromSelf(nil), + let value = SecTaskCopyValueForEntitlement( + task, + "com.apple.security.application-groups" as CFString, + nil) + else { + return false + } + guard let groups = value as? [String] else { + return false + } + return groups.contains(identifier) + #else + return true + #endif + } + private static func appGroupStateDirURL() -> URL? { guard let containerURL = FileManager.default @@ -100,6 +135,40 @@ enum DeviceIdentityPaths { } return containerURL.appendingPathComponent("OpenClaw", isDirectory: true) } + + /// Files a one-time fallback migration may carry from the App Group container into the + /// selected store. Stored device tokens are keyed by deviceId, so the identity file is + /// only useful together with its auth sibling; migrating one without the other forces + /// an unnecessary re-pair even though the deviceId survived. + struct AppGroupMigrationSource { + let identityURL: URL + let authURL: URL + } + + static func appGroupMigrationSource( + profile: GatewayDeviceIdentityProfile) -> AppGroupMigrationSource? + { + self.appGroupMigrationSource( + appGroupStateDirURL: self.appGroupStateDirURL(), + appGroupStateDirAvailable: self.appGroupStateDirAvailable, + profile: profile) + } + + /// Non-nil only for unentitled builds whose store selection fell back to legacy storage; + /// entitled builds keep using the App Group container and must never migrate out of it. + static func appGroupMigrationSource( + appGroupStateDirURL: URL?, + appGroupStateDirAvailable: Bool, + profile: GatewayDeviceIdentityProfile) -> AppGroupMigrationSource? + { + guard !appGroupStateDirAvailable, let appGroupStateDirURL else { + return nil + } + let identityDirURL = appGroupStateDirURL.appendingPathComponent("identity", isDirectory: true) + return AppGroupMigrationSource( + identityURL: identityDirURL.appendingPathComponent(profile.identityFileName, isDirectory: false), + authURL: identityDirURL.appendingPathComponent(profile.authFileName, isDirectory: false)) + } } public enum DeviceIdentityStore { @@ -117,25 +186,75 @@ public enum DeviceIdentityStore { } public static func loadOrCreate(profile: GatewayDeviceIdentityProfile) -> DeviceIdentity { - self.loadOrCreate(fileURL: self.fileURL(profile: profile)) + self.loadOrCreate( + fileURL: self.fileURL(profile: profile), + migrationSource: DeviceIdentityPaths.appGroupMigrationSource(profile: profile)) } - static func loadOrCreate(fileURL url: URL) -> DeviceIdentity { + static func loadOrCreate( + fileURL url: URL, + migrationSource: DeviceIdentityPaths.AppGroupMigrationSource? = nil) -> DeviceIdentity + { if let data = try? Data(contentsOf: url) { switch self.decodeStoredIdentity(data) { case let .identity(decoded): return decoded - case .recognizedInvalid: + case .recognizedInvalid, .unknown: + // Existing bytes may hold a newer schema or recoverable key material; never + // overwrite them. Callers run with a transient identity instead. return self.generate() - case .unknown: - break } } + if FileManager.default.fileExists(atPath: url.path) { + return self.generate() + } + if let migrated = self.migratedIdentity(from: migrationSource, to: url) { + return migrated + } let identity = self.generate() self.save(identity, to: url) return identity } + /// One-time upgrade path for builds that lost App Group storage: it runs only while the + /// selected store has no identity file, so steady state never re-reads the old container. + private static func migratedIdentity( + from source: DeviceIdentityPaths.AppGroupMigrationSource?, + to destinationURL: URL) -> DeviceIdentity? + { + guard + let source, + let data = try? Data(contentsOf: source.identityURL), + case let .identity(identity) = self.decodeStoredIdentity(data) + else { + return nil + } + self.save(identity, to: destinationURL) + // Stored device tokens only load when their store's deviceId matches (DeviceAuthStore), + // so they must move together with the identity or the install re-pairs for no reason. + // A mismatched copy is inert behind that same check; no validation needed here. + self.copyAuthStoreFile( + from: source.authURL, + toDirectory: destinationURL.deletingLastPathComponent()) + return identity + } + + private static func copyAuthStoreFile(from sourceURL: URL, toDirectory directoryURL: URL) { + let fileManager = FileManager.default + let destinationURL = directoryURL + .appendingPathComponent(sourceURL.lastPathComponent, isDirectory: false) + guard + !fileManager.fileExists(atPath: destinationURL.path), + fileManager.fileExists(atPath: sourceURL.path) + else { + return + } + try? fileManager.copyItem(at: sourceURL, to: destinationURL) + try? fileManager.setAttributes( + [.posixPermissions: 0o600], + ofItemAtPath: destinationURL.path) + } + private enum DecodeResult { case identity(DeviceIdentity) case recognizedInvalid diff --git a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift index efddc3de178a..939f271de2c6 100644 --- a/apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift +++ b/apps/shared/OpenClawKit/Tests/OpenClawKitTests/DeviceIdentityStoreTests.swift @@ -196,6 +196,24 @@ struct DeviceIdentityStoreTests { #expect(!FileManager.default.fileExists(atPath: sharedDeviceURL.path)) } + @Test + func `legacy app support storage wins when app group storage is not available`() { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + let legacyURL = tempDir.appendingPathComponent("legacy", isDirectory: true) + let sharedURL = tempDir.appendingPathComponent("shared", isDirectory: true) + + let selected = DeviceIdentityPaths.stateDirURL( + overrideURL: nil, + legacyStateDirURL: legacyURL, + appGroupStateDirURL: sharedURL, + appGroupStateDirAvailable: false, + temporaryDirectory: tempDir) + + #expect(selected == legacyURL) + } + @Test(.stateDirectoryIsolated) func `secondary profiles use separate identity and auth files`() throws { let tempDir = FileManager.default.temporaryDirectory @@ -326,6 +344,174 @@ struct DeviceIdentityStoreTests { #expect(try String(contentsOf: identityURL, encoding: .utf8) == before) } + @Test + func `does not overwrite an existing unrecognized identity file`() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + let identityURL = tempDir + .appendingPathComponent("identity", isDirectory: true) + .appendingPathComponent("device.json", isDirectory: false) + defer { try? FileManager.default.removeItem(at: tempDir) } + try FileManager.default.createDirectory( + at: identityURL.deletingLastPathComponent(), + withIntermediateDirectories: true) + let stored = """ + { + "schema": "future-openclaw-device-identity", + "stableDeviceId": "app-group-device-id" + } + """ + try stored.write(to: identityURL, atomically: true, encoding: .utf8) + let before = try String(contentsOf: identityURL, encoding: .utf8) + + let identity = DeviceIdentityStore.loadOrCreate(fileURL: identityURL) + + #expect(identity.deviceId != "app-group-device-id") + #expect(try String(contentsOf: identityURL, encoding: .utf8) == before) + } + + @Test + func `migrates an existing app group identity and auth store when falling back to legacy storage`() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + let appGroupURL = tempDir.appendingPathComponent("shared", isDirectory: true) + let appGroupIdentityURL = appGroupURL + .appendingPathComponent("identity", isDirectory: true) + .appendingPathComponent("device.json", isDirectory: false) + let appGroupAuthURL = appGroupIdentityURL + .deletingLastPathComponent() + .appendingPathComponent("device-auth.json", isDirectory: false) + let legacyIdentityURL = tempDir + .appendingPathComponent("legacy", isDirectory: true) + .appendingPathComponent("identity", isDirectory: true) + .appendingPathComponent("device.json", isDirectory: false) + let legacyAuthURL = legacyIdentityURL + .deletingLastPathComponent() + .appendingPathComponent("device-auth.json", isDirectory: false) + try FileManager.default.createDirectory( + at: appGroupIdentityURL.deletingLastPathComponent(), + withIntermediateDirectories: true) + let stored = try Self.identityJSON( + publicKeyPem: Self.pem( + label: "PUBLIC KEY", + body: "MCowBQYDK2VwAyEAA6EHv/POEL4dcN0Y50vAmWfk1jCbpQ1fHdyGZBJVMbg="), + privateKeyPem: Self.pem( + label: "PRIVATE KEY", + body: "MC4CAQAwBQYDK2VwBCIEIAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZGhscHR4f")) + try stored.write(to: appGroupIdentityURL, atomically: true, encoding: .utf8) + let storedAuth = #"{"version":1,"deviceId":"app-group-device-id","tokens":{}}"# + try storedAuth.write(to: appGroupAuthURL, atomically: true, encoding: .utf8) + let appGroupBefore = try String(contentsOf: appGroupIdentityURL, encoding: .utf8) + + let migrationSource = DeviceIdentityPaths.appGroupMigrationSource( + appGroupStateDirURL: appGroupURL, + appGroupStateDirAvailable: false, + profile: .primary) + let identity = DeviceIdentityStore.loadOrCreate( + fileURL: legacyIdentityURL, + migrationSource: migrationSource) + + #expect(identity.deviceId == "56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c") + #expect(FileManager.default.fileExists(atPath: legacyIdentityURL.path)) + let reloaded = DeviceIdentityStore.loadOrCreate(fileURL: legacyIdentityURL) + #expect(reloaded.deviceId == identity.deviceId) + #expect(try String(contentsOf: appGroupIdentityURL, encoding: .utf8) == appGroupBefore) + #expect(try String(contentsOf: legacyAuthURL, encoding: .utf8) == storedAuth) + #expect(try String(contentsOf: appGroupAuthURL, encoding: .utf8) == storedAuth) + } + + @Test + func `does not clobber an existing auth store when migrating an app group identity`() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + let appGroupIdentityDirURL = tempDir + .appendingPathComponent("shared", isDirectory: true) + .appendingPathComponent("identity", isDirectory: true) + let legacyIdentityDirURL = tempDir + .appendingPathComponent("legacy", isDirectory: true) + .appendingPathComponent("identity", isDirectory: true) + try FileManager.default.createDirectory(at: appGroupIdentityDirURL, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: legacyIdentityDirURL, withIntermediateDirectories: true) + let stored = try Self.identityJSON( + publicKeyPem: Self.pem( + label: "PUBLIC KEY", + body: "MCowBQYDK2VwAyEAA6EHv/POEL4dcN0Y50vAmWfk1jCbpQ1fHdyGZBJVMbg="), + privateKeyPem: Self.pem( + label: "PRIVATE KEY", + body: "MC4CAQAwBQYDK2VwBCIEIAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZGhscHR4f")) + try stored.write( + to: appGroupIdentityDirURL.appendingPathComponent("device.json", isDirectory: false), + atomically: true, + encoding: .utf8) + try #"{"version":1,"deviceId":"app-group-device-id","tokens":{}}"#.write( + to: appGroupIdentityDirURL.appendingPathComponent("device-auth.json", isDirectory: false), + atomically: true, + encoding: .utf8) + let legacyAuthURL = legacyIdentityDirURL.appendingPathComponent("device-auth.json", isDirectory: false) + let existingLegacyAuth = #"{"version":1,"deviceId":"legacy-device-id","tokens":{}}"# + try existingLegacyAuth.write(to: legacyAuthURL, atomically: true, encoding: .utf8) + + let identity = DeviceIdentityStore.loadOrCreate( + fileURL: legacyIdentityDirURL.appendingPathComponent("device.json", isDirectory: false), + migrationSource: DeviceIdentityPaths.appGroupMigrationSource( + appGroupStateDirURL: tempDir.appendingPathComponent("shared", isDirectory: true), + appGroupStateDirAvailable: false, + profile: .primary)) + + #expect(identity.deviceId == "56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c") + #expect(try String(contentsOf: legacyAuthURL, encoding: .utf8) == existingLegacyAuth) + } + + @Test + func `keeps an existing legacy identity instead of migrating the app group copy`() throws { + let tempDir = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + defer { try? FileManager.default.removeItem(at: tempDir) } + let appGroupURL = tempDir.appendingPathComponent("shared", isDirectory: true) + let appGroupIdentityURL = appGroupURL + .appendingPathComponent("identity", isDirectory: true) + .appendingPathComponent("device.json", isDirectory: false) + let legacyIdentityURL = tempDir + .appendingPathComponent("legacy", isDirectory: true) + .appendingPathComponent("identity", isDirectory: true) + .appendingPathComponent("device.json", isDirectory: false) + try FileManager.default.createDirectory( + at: appGroupIdentityURL.deletingLastPathComponent(), + withIntermediateDirectories: true) + let stored = try Self.identityJSON( + publicKeyPem: Self.pem( + label: "PUBLIC KEY", + body: "MCowBQYDK2VwAyEAA6EHv/POEL4dcN0Y50vAmWfk1jCbpQ1fHdyGZBJVMbg="), + privateKeyPem: Self.pem( + label: "PRIVATE KEY", + body: "MC4CAQAwBQYDK2VwBCIEIAABAgMEBQYHCAkKCwwNDg8QERITFBUWFxgZGhscHR4f")) + try stored.write(to: appGroupIdentityURL, atomically: true, encoding: .utf8) + + let existingLegacy = DeviceIdentityStore.loadOrCreate(fileURL: legacyIdentityURL) + let migrationSource = DeviceIdentityPaths.appGroupMigrationSource( + appGroupStateDirURL: appGroupURL, + appGroupStateDirAvailable: false, + profile: .primary) + let identity = DeviceIdentityStore.loadOrCreate( + fileURL: legacyIdentityURL, + migrationSource: migrationSource) + + #expect(identity.deviceId == existingLegacy.deviceId) + #expect(identity.deviceId != "56475aa75463474c0285df5dbf2bcab73da651358839e9b77481b2eab107708c") + } + + @Test + func `provides no app group migration source when the entitlement is present`() { + let appGroupURL = FileManager.default.temporaryDirectory + .appendingPathComponent(UUID().uuidString, isDirectory: true) + #expect(DeviceIdentityPaths.appGroupMigrationSource( + appGroupStateDirURL: appGroupURL, + appGroupStateDirAvailable: true, + profile: .primary) == nil) + } + private static func base64UrlDecode(_ value: String) -> Data? { let normalized = value .replacingOccurrences(of: "-", with: "+") diff --git a/src/infra/device-identity.state-dir.test.ts b/src/infra/device-identity.state-dir.test.ts index a5ed622d0012..968ab6d623cd 100644 --- a/src/infra/device-identity.state-dir.test.ts +++ b/src/infra/device-identity.state-dir.test.ts @@ -51,24 +51,26 @@ describe("device identity state dir defaults", () => { }); }); - it("regenerates the identity when the stored file is invalid", async () => { + it("returns a transient identity without overwriting an invalid stored file", async () => { await withStateDirEnv("openclaw-identity-state-", async ({ stateDir }) => { const identityPath = path.join(stateDir, "identity", "device.json"); await fs.mkdir(path.dirname(identityPath), { recursive: true }); - await fs.writeFile(identityPath, '{"version":1,"deviceId":"broken"}\n', "utf8"); + const before = [ + "{", + ' "version": 1,', + ' "deviceId": "broken",', + ' "publicKeyPem": "not-a-valid-public-key",', + ' "privateKeyPem": "not-a-valid-private-key"', + "}", + "", + ].join("\n"); + await fs.writeFile(identityPath, before, "utf8"); const regenerated = loadOrCreateDeviceIdentity(); - const stored = JSON.parse(await fs.readFile(identityPath, "utf8")) as { - version?: number; - deviceId?: string; - publicKeyPem?: string; - privateKeyPem?: string; - }; + const stored = await fs.readFile(identityPath, "utf8"); - expect(stored.version).toBe(1); - expect(stored.deviceId).toBe(regenerated.deviceId); - expect(stored.publicKeyPem).toBe(regenerated.publicKeyPem); - expect(stored.privateKeyPem).toBe(regenerated.privateKeyPem); + expect(regenerated.deviceId).not.toBe("broken"); + expect(stored).toBe(before); }); }); }); diff --git a/src/infra/device-identity.test.ts b/src/infra/device-identity.test.ts index f672ce75295f..f5f7336051be 100644 --- a/src/infra/device-identity.test.ts +++ b/src/infra/device-identity.test.ts @@ -141,6 +141,32 @@ describe("device identity crypto helpers", () => { }); }); + it("does not overwrite existing unrecognized identity files", async () => { + await withTempDir("openclaw-device-identity-unrecognized-", async (dir) => { + const identityPath = path.join(dir, "identity", "device.json"); + fs.mkdirSync(path.dirname(identityPath), { recursive: true }); + fs.writeFileSync( + identityPath, + `${JSON.stringify( + { + schema: "future-openclaw-device-identity", + stableDeviceId: "app-group-device-id", + }, + null, + 2, + )}\n`, + "utf8", + ); + const before = fs.readFileSync(identityPath, "utf8"); + + expect(loadDeviceIdentityIfPresent(identityPath)).toBeNull(); + const loaded = loadOrCreateDeviceIdentity(identityPath); + + expect(loaded.deviceId).not.toBe("app-group-device-id"); + expect(fs.readFileSync(identityPath, "utf8")).toBe(before); + }); + }); + it("does not migrate Swift raw-key identity files with mismatched key material", async () => { await withTempDir("openclaw-device-identity-swift-invalid-", async (dir) => { const identityPath = path.join(dir, "identity", "device.json"); diff --git a/src/infra/device-identity.ts b/src/infra/device-identity.ts index 5b91c184b2cc..e3e50e575070 100644 --- a/src/infra/device-identity.ts +++ b/src/infra/device-identity.ts @@ -244,6 +244,10 @@ export function loadOrCreateDeviceIdentity( // Avoid overwriting recognizable but invalid identity files; callers can still use a fresh key. return generateIdentity(); } + if (identityFileExists(filePath)) { + // Unrecognized existing files may hold a newer schema; never overwrite them either. + return generateIdentity(); + } } catch { if (identityFileExists(filePath)) { return generateIdentity();