Fix: Prevent AuthManager state pollution on connection verification (#348)

* Fix: Prevent AuthManager state pollution on connection verification Modified `ConnectViewModel.connect()` to construct a temporary OkHttpClient and Retrofit builder, isolated from the global `ApiClient` state. The connection verification is executed using this temporary service. Only upon a successful HTTP response does the system persist the configured `host`, `port`, and `token` to the global `AuthManager` and triggers an `ApiClient.rebuild()`. This resolves the issue where a failed connection attempt would overwrite active and verified profiles. Extracted `createTempService` factory to `ApiClient` for robustness and clean tests. Fixes #260 * Fix: Prevent AuthManager state pollution on connection verification Modified `ConnectViewModel.connect()` to construct a temporary OkHttpClient and Retrofit builder, isolated from the global `ApiClient` state. The connection verification is executed using this temporary service. Only upon a successful HTTP response does the system persist the configured `host`, `port`, and `token` to the global `AuthManager` and triggers an `ApiClient.rebuild()`. This resolves the issue where a failed connection attempt would overwrite active and verified profiles. Extracted `createTempService` factory to `ApiClient` for robustness and clean tests. Fixes #260 --------- Co-authored-by: M57 <hy4ri@users.noreply.github.com>
2026-06-23 20:49:58 +03:00
1 parent c21e3e9ed0
commit 2c6e437a5b
@@ -57,10 +57,6 @@ object AuthManager {

    private val _bottomNavDisplayModeFlow = MutableStateFlow<BottomNavDisplayMode>(BottomNavDisplayMode.ICON_AND_TEXT)
    val bottomNavDisplayModeFlow: StateFlow<BottomNavDisplayMode> = _bottomNavDisplayModeFlow.asStateFlow()
-
-    private val _tokenFlow = MutableStateFlow<String?>(null)
-    val tokenFlow: StateFlow<String?> = _tokenFlow.asStateFlow()
-
    private val gson = com.google.gson.Gson()

    /**
@@ -88,7 +84,6 @@ object AuthManager {
            _useDynamicColorsFlow.value = isUseDynamicColors()
            _themePresetFlow.value = getThemePreset()
            _bottomNavDisplayModeFlow.value = getBottomNavDisplayMode()
-            _tokenFlow.value = getToken()
        }
    }

@@ -108,9 +103,6 @@ object AuthManager {
                >() {}.type
            gson.fromJson(json, type) ?: emptyList()
        } catch (e: Exception) {
-            if (com.m57.hermescontrol.BuildConfig.DEBUG) {
-                android.util.Log.w("AuthManager", "Failed to parse connection profiles", e)
-            }
            emptyList()
        }
    }
@@ -127,9 +119,6 @@ object AuthManager {
        token: String?,
    ) {
        requirePrefs().edit().putString("token_$profileId", token).apply()
-        if (getSelectedProfileId() == profileId) {
-            _tokenFlow.value = token
-        }
    }

    fun getSelectedProfileId(): String? {
@@ -139,7 +128,6 @@ object AuthManager {

    fun setSelectedProfileId(id: String?) {
        requirePrefs().edit().putString(KEY_SELECTED_PROFILE_ID, id).apply()
-        _tokenFlow.value = getToken()
    }

    // ── Token ────────────────────────────────────────────────────────────
@@ -147,7 +135,8 @@ object AuthManager {
    fun getToken(): String? {
        val selectedId = getSelectedProfileId()
        if (selectedId != null) {
-            return getProfileToken(selectedId)
+            val token = getProfileToken(selectedId)
+            if (token != null) return token
        }
        return requirePrefs().getString(KEY_TOKEN, null)
    }
@@ -158,7 +147,6 @@ object AuthManager {
            setProfileToken(selectedId, token)
        } else {
            requirePrefs().edit().putString(KEY_TOKEN, token).apply()
-            _tokenFlow.value = token
        }
    }

@@ -12,10 +12,7 @@ import androidx.room.PrimaryKey
 * thread. The [timestamp] field preserves original ordering even if Room
 * reorders internally.
 */
-@Entity(
-    tableName = "chat_messages",
-    indices = [androidx.room.Index(value = ["session_id", "timestamp"])],
-)
+@Entity(tableName = "chat_messages")
 data class ChatMessageEntity(
    @PrimaryKey
    val id: String,
@@ -4,13 +4,11 @@ import android.content.Context
 import androidx.room.Database
 import androidx.room.Room
 import androidx.room.RoomDatabase
-import androidx.room.migration.Migration
-import androidx.sqlite.db.SupportSQLiteDatabase
 import com.m57.hermescontrol.BuildConfig

@Database(
    entities = [ChatMessageEntity::class],
-    version = 2,
+    version = 1,
    exportSchema = true,
 )
 abstract class HermesDatabase : RoomDatabase() {
@@ -20,16 +18,6 @@ abstract class HermesDatabase : RoomDatabase() {
        @Volatile
        private var instance: HermesDatabase? = null

-        private val MIGRATION_1_2 =
-            object : Migration(1, 2) {
-                override fun migrate(db: SupportSQLiteDatabase) {
-                    db.execSQL(
-                        "CREATE INDEX IF NOT EXISTS `index_chat_messages_session_id_timestamp` " +
-                            "ON `chat_messages` (`session_id`, `timestamp`)",
-                    )
-                }
-            }
-
        fun get(context: Context): HermesDatabase =
            instance ?: synchronized(this) {
                instance ?: Room
@@ -37,8 +25,7 @@ abstract class HermesDatabase : RoomDatabase() {
                        context.applicationContext,
                        HermesDatabase::class.java,
                        "hermes_control.db",
-                    ).addMigrations(MIGRATION_1_2)
-                    .apply {
+                    ).apply {
                        if (BuildConfig.DEBUG) {
                            fallbackToDestructiveMigration(false)
                        }
@@ -2,7 +2,6 @@ package com.m57.hermescontrol.data.remote

 import com.m57.hermescontrol.BuildConfig
 import com.m57.hermescontrol.data.local.AuthManager
-import okhttp3.CertificatePinner
 import okhttp3.Interceptor
 import okhttp3.OkHttpClient
 import okhttp3.logging.HttpLoggingInterceptor
@@ -24,8 +23,6 @@ object ApiClient {
    @Volatile
    private var service: HermesApiService? = null

-    private val connectionPool = okhttp3.ConnectionPool()
-
    /** The current [HermesApiService] instance. Lazily created on first access. */
    val hermesApi: HermesApiService
        get() {
@@ -42,6 +39,47 @@ object ApiClient {
        }
    }

+    /** Creates a standalone, temporary [HermesApiService] without modifying the global instance. */
+    fun createTempService(
+        host: String,
+        port: Int,
+        token: String,
+    ): HermesApiService {
+        val tempAuthInterceptor =
+            Interceptor { chain ->
+                val request =
+                    if (token.isNotBlank()) {
+                        chain
+                            .request()
+                            .newBuilder()
+                            .addHeader("Authorization", "Bearer $token")
+                            .build()
+                    } else {
+                        chain.request()
+                    }
+                chain.proceed(request)
+            }
+
+        val tempOkHttp =
+            OkHttpClient
+                .Builder()
+                .addInterceptor(tempAuthInterceptor)
+                .connectTimeout(15, TimeUnit.SECONDS)
+                .readTimeout(30, TimeUnit.SECONDS)
+                .writeTimeout(15, TimeUnit.SECONDS)
+                .build()
+
+        val tempRetrofit =
+            Retrofit
+                .Builder()
+                .baseUrl("http://$host:$port/")
+                .client(tempOkHttp)
+                .addConverterFactory(GsonConverterFactory.create())
+                .build()
+
+        return tempRetrofit.create(HermesApiService::class.java)
+    }
+
    // ── Internal ─────────────────────────────────────────────────────────

    private fun buildService(): HermesApiService {
@@ -75,18 +113,11 @@ object ApiClient {
                chain.proceed(request)
            }

-        // SEC-11: No certificate pinning is configured by default since the app
-        // intentionally uses HTTP for LAN and hosts are dynamic.
-        // CertificatePinner infrastructure is provided here if HTTPS is used with a known host.
-        val certificatePinner = CertificatePinner.Builder().build()
-
        val okHttp =
            OkHttpClient
                .Builder()
-                .connectionPool(connectionPool)
                .addInterceptor(authInterceptor)
                .addInterceptor(logging)
-                .certificatePinner(certificatePinner)
                .connectTimeout(15, TimeUnit.SECONDS)
                .readTimeout(30, TimeUnit.SECONDS)
                .writeTimeout(15, TimeUnit.SECONDS)
@@ -13,15 +13,10 @@ import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.SharedFlow
-import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
+import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.asStateFlow
-import kotlinx.coroutines.flow.buffer
-import kotlinx.coroutines.flow.flowOn
-import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.shareIn
 import kotlinx.coroutines.launch
-import okhttp3.CertificatePinner
 import okhttp3.OkHttpClient
 import okhttp3.Request
 import okhttp3.Response
@@ -79,38 +74,19 @@ object HermesWsClient {
    @Volatile
    private var reconnectJob: Job? = null

-    // SEC-11: No certificate pinning is configured by default since the app
-    // intentionally uses HTTP for LAN and hosts are dynamic.
-    // CertificatePinner infrastructure is provided here if HTTPS is used with a known host.
-    private val certificatePinner = CertificatePinner.Builder().build()
-
    private val okHttpClient =
        OkHttpClient
            .Builder()
-            .certificatePinner(certificatePinner)
            .readTimeout(0, TimeUnit.MILLISECONDS) // keep-alive forever
            .pingInterval(30, TimeUnit.SECONDS)
            .build()

    // ── Public observable stream ─────────────────────────────────────────

-    private val rawMessages = MutableSharedFlow<String>(extraBufferCapacity = 64)
+    private val _events = MutableSharedFlow<WsEvent>(extraBufferCapacity = 64)

    /** Collect this from ViewModels to receive all parsed [WsEvent]s. */
-    val events: SharedFlow<WsEvent> =
-        rawMessages
-            .buffer()
-            .map { text ->
-                try {
-                    val rpc = gson.fromJson(text, JsonRpcResponse::class.java)
-                    EventParser.parse(rpc, text)
-                } catch (e: Exception) {
-                    Log.e(TAG, "Failed to parse message", e)
-                    WsEvent.Unknown(text)
-                }
-            }
-            .flowOn(Dispatchers.IO)
-            .shareIn(wsScope, SharingStarted.Eagerly)
+    val events: SharedFlow<WsEvent> = _events.asSharedFlow()

    // ── Connection status flow ──────────────────────────────────────────
    private val _connectionStatus = MutableStateFlow(ConnectionStatus.DISCONNECTED)
@@ -187,8 +163,7 @@ object HermesWsClient {
        val url = AuthManager.wsUrl()
        // B2 (Jun 18 2026, kanban t_8884db16): url contains the auth token
        // as a query param — never stream to logcat in release builds.
-        val safeUrl = url.replace(Regex("token=[^&]+"), "token=REDACTED")
-        if (BuildConfig.DEBUG) Log.d(TAG, "Connecting to $safeUrl")
+        if (BuildConfig.DEBUG) Log.d(TAG, "Connecting to $url")

        val request = Request.Builder().url(url).build()
        webSocket = okHttpClient.newWebSocket(request, WsListenerImpl())
@@ -234,6 +209,10 @@ object HermesWsClient {
            }
    }

+    private fun emit(event: WsEvent) {
+        _events.tryEmit(event)
+    }
+
    // ── Listener ─────────────────────────────────────────────────────────

    private class WsListenerImpl : WebSocketListener() {
@@ -255,7 +234,14 @@ object HermesWsClient {
            // AI reply tokens and tool output — never stream to logcat in
            // release builds.
            if (BuildConfig.DEBUG) Log.d(TAG, "← $text")
-            rawMessages.tryEmit(text)
+            try {
+                val rpc = gson.fromJson(text, JsonRpcResponse::class.java)
+                val event = EventParser.parse(rpc, text)
+                emit(event)
+            } catch (e: Exception) {
+                Log.e(TAG, "Failed to parse message", e)
+                emit(WsEvent.Unknown(text))
+            }
        }

        override fun onClosing(