From 3117a01be793bbcf56cc2ff38fd8deee0e6706f3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 16 May 2026 02:11:32 +0100 Subject: [PATCH] fix: secure sqlite review regressions --- .../openclaw/app/gateway/DeviceAuthStore.kt | 36 ++++++++++++++----- .../app/gateway/DeviceAuthStoreTest.kt | 31 ++++++++++++++-- src/plugin-sdk/persistent-dedupe.ts | 23 ++++++++---- 3 files changed, 71 insertions(+), 19 deletions(-) diff --git a/apps/android/app/src/main/java/ai/openclaw/app/gateway/DeviceAuthStore.kt b/apps/android/app/src/main/java/ai/openclaw/app/gateway/DeviceAuthStore.kt index bc7ba9e0cf9..e473dcf2780 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/gateway/DeviceAuthStore.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/gateway/DeviceAuthStore.kt @@ -22,6 +22,7 @@ private data class PersistedDeviceAuthMetadata( private const val deviceAuthTokenPrefix = "gateway.deviceToken." private const val deviceAuthMetadataPrefix = "gateway.deviceTokenMeta." +private const val sqliteSecurePrefsTokenMarker = "__openclaw_secure_prefs__" interface DeviceAuthTokenStore { fun loadEntry( @@ -64,7 +65,16 @@ class DeviceAuthStore( val row = stateStore.readDeviceAuthToken(normalizedDevice, normalizedRole) ?: return migrateLegacyEntry(normalizedDevice, normalizedRole) - val token = row.token.trim().takeIf { it.isNotEmpty() } ?: return null + val token = + legacyPrefs + .getString(tokenKey(normalizedDevice, normalizedRole)) + ?.trim() + ?.takeIf { it.isNotEmpty() } + ?: row.token.trim().takeIf { it.isNotEmpty() && it != sqliteSecurePrefsTokenMarker }?.also { + legacyPrefs.putString(tokenKey(normalizedDevice, normalizedRole), it) + stateStore.upsertDeviceAuthToken(row.copy(token = sqliteSecurePrefsTokenMarker)) + } + ?: return null return DeviceAuthEntry( token = token, role = normalizedRole, @@ -92,20 +102,20 @@ class DeviceAuthStore( if (sqliteDeviceChanged) { stateStore.deleteAllDeviceAuthTokens() } + if (shouldDropLegacyAuth) { + removeAllLegacyEntries() + } + legacyPrefs.putString(tokenKey(normalizedDevice, normalizedRole), token.trim()) + removeLegacyMetadata(normalizedDevice, normalizedRole) stateStore.upsertDeviceAuthToken( OpenClawSQLiteDeviceAuthTokenRow( deviceId = normalizedDevice, role = normalizedRole, - token = token.trim(), + token = sqliteSecurePrefsTokenMarker, scopesJson = json.encodeToString(normalizedScopes), updatedAtMs = System.currentTimeMillis(), ), ) - if (shouldDropLegacyAuth) { - removeAllLegacyEntries() - } else { - removeLegacyEntry(normalizedDevice, normalizedRole) - } } override fun clearToken( @@ -144,15 +154,23 @@ class DeviceAuthStore( OpenClawSQLiteDeviceAuthTokenRow( deviceId = normalizedDevice, role = normalizedRole, - token = entry.token, + token = sqliteSecurePrefsTokenMarker, scopesJson = json.encodeToString(entry.scopes), updatedAtMs = entry.updatedAtMs, ), ) - removeLegacyEntry(normalizedDevice, normalizedRole) + legacyPrefs.putString(tokenKey(normalizedDevice, normalizedRole), entry.token) + removeLegacyMetadata(normalizedDevice, normalizedRole) return entry } + private fun removeLegacyMetadata( + normalizedDevice: String, + normalizedRole: String, + ) { + legacyPrefs.remove(metadataKey(normalizedDevice, normalizedRole)) + } + private fun removeLegacyEntry( normalizedDevice: String, normalizedRole: String, diff --git a/apps/android/app/src/test/java/ai/openclaw/app/gateway/DeviceAuthStoreTest.kt b/apps/android/app/src/test/java/ai/openclaw/app/gateway/DeviceAuthStoreTest.kt index 157cabe408e..86795c35b68 100644 --- a/apps/android/app/src/test/java/ai/openclaw/app/gateway/DeviceAuthStoreTest.kt +++ b/apps/android/app/src/test/java/ai/openclaw/app/gateway/DeviceAuthStoreTest.kt @@ -42,7 +42,7 @@ class DeviceAuthStoreTest { assertTrue((entry?.updatedAtMs ?: 0L) > 0L) val row = OpenClawSQLiteStateStore(app).readDeviceAuthToken("device-1", "operator") assertNotNull(row) - assertEquals("operator-token", row?.token) + assertEquals("__openclaw_secure_prefs__", row?.token) assertEquals("""["operator.read","operator.write"]""", row?.scopesJson) } @@ -75,10 +75,34 @@ class DeviceAuthStoreTest { assertEquals("operator", entry?.role) assertEquals(listOf("operator.read", "operator.write"), entry?.scopes) assertEquals(1700000000000L, entry?.updatedAtMs) - assertNull(prefs.getString("gateway.deviceToken.device-1.operator")) + assertEquals("operator-token", prefs.getString("gateway.deviceToken.device-1.operator")) assertNull(prefs.getString("gateway.deviceTokenMeta.device-1.operator")) assertEquals( - "operator-token", + "__openclaw_secure_prefs__", + OpenClawSQLiteStateStore(app).readDeviceAuthToken("device-1", "operator")?.token, + ) + } + + @Test + fun loadEntryMovesPlaintextSqliteTokenBackToSecurePrefs() { + val app = RuntimeEnvironment.getApplication() + val prefs = legacyPrefs(app) + OpenClawSQLiteStateStore(app).upsertDeviceAuthToken( + OpenClawSQLiteDeviceAuthTokenRow( + deviceId = "device-1", + role = "operator", + token = "operator-token", + scopesJson = """["operator.read"]""", + updatedAtMs = 1700000000000, + ), + ) + + val entry = DeviceAuthStore(app, legacyPrefsOverride = prefs).loadEntry("device-1", "operator") + + assertEquals("operator-token", entry?.token) + assertEquals("operator-token", prefs.getString("gateway.deviceToken.device-1.operator")) + assertEquals( + "__openclaw_secure_prefs__", OpenClawSQLiteStateStore(app).readDeviceAuthToken("device-1", "operator")?.token, ) } @@ -100,6 +124,7 @@ class DeviceAuthStoreTest { assertNull(prefs.getString("gateway.deviceToken.device-1.operator")) assertNull(prefs.getString("gateway.deviceTokenMeta.device-1.operator")) assertEquals("fresh-token", store.loadEntry("device-2", "operator")?.token) + assertEquals("fresh-token", prefs.getString("gateway.deviceToken.device-2.operator")) } private fun legacyPrefs(context: Context): SecurePrefs { diff --git a/src/plugin-sdk/persistent-dedupe.ts b/src/plugin-sdk/persistent-dedupe.ts index 1507ca54e17..ad720a32c15 100644 --- a/src/plugin-sdk/persistent-dedupe.ts +++ b/src/plugin-sdk/persistent-dedupe.ts @@ -145,17 +145,26 @@ export function createPersistentDedupe(options: PersistentDedupeOptions): Persis try { const scopeKey = options.resolveScopeKey(namespace); const storeKey = resolveStoreKey(scopeKey, namespace, key); - const existing = PERSISTENT_DEDUPE_STORE.lookup(storeKey); - const existingSeenAt = existing?.seenAt; - if (isRecentTimestamp(existingSeenAt, ttlMs, now)) { - memory.check(scopedKey, existingSeenAt); - return false; - } - PERSISTENT_DEDUPE_STORE.register( + const inserted = PERSISTENT_DEDUPE_STORE.registerIfAbsent( storeKey, { scopeKey, namespace, key, seenAt: now }, ttlMs > 0 ? { ttlMs } : undefined, ); + if (!inserted) { + const existingSeenAt = PERSISTENT_DEDUPE_STORE.lookup(storeKey)?.seenAt; + if (!isRecentTimestamp(existingSeenAt, ttlMs, now)) { + PERSISTENT_DEDUPE_STORE.register( + storeKey, + { scopeKey, namespace, key, seenAt: now }, + ttlMs > 0 ? { ttlMs } : undefined, + ); + prunePersistentRows(scopeKey, now, ttlMs, maxEntries); + memory.check(scopedKey, now); + return true; + } + memory.check(scopedKey, existingSeenAt); + return false; + } prunePersistentRows(scopeKey, now, ttlMs, maxEntries); memory.check(scopedKey, now); return true;