From c5d1ef1c422af58eee6e65f448567d104a285930 Mon Sep 17 00:00:00 2001 From: M57 <41645758+Hy4ri@users.noreply.github.com> Date: Fri, 10 Jul 2026 14:02:45 +0300 Subject: [PATCH] fix(network): scope management REST calls with ?profile= (fixes #528) (#540) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(network): scope management REST calls with ?profile= (fixes #528) Mobile fired all management REST calls with NO profile scope, so every profile read/wrote the backend's implicit default profile — multi-profile setups silently corrupted config/skills/toolsets/mcp/model/env. Add ProfileScopeInterceptor (OkHttp) that appends ?profile= to the same endpoint families the desktop/web dashboard scopes (web/src/lib/api.ts PROFILE_SCOPED_PREFIXES), respecting: - no active profile -> pass through (legacy default behavior) - explicit ?profile= on the URL wins over the global scope - only scoped families rewritten; ops/pairing/cron/profiles stay global Wired into ApiClient.buildService(). 5 MockWebServer tests cover scoped append, explicit-param precedence, no-profile passthrough, non-scoped skip, and skills scoping. Mirrors desktop/web contract derived from audit of /opt/hermes-agent. * fix(ci): stub getSelectedProfileId in ApiClientTest + harden prefix match (fixes PR #540) - ApiClientTest: stub AuthManager.getSelectedProfileId() to null. The new ProfileScopeInterceptor (registered in ApiClient.buildService) calls it, and the unstubbed real method threw IllegalStateException at AuthManager.kt:56, breaking all 4 auth-interceptor tests. - ProfileScopeInterceptor: replace startsWith(prefix) with whole-segment match (path == prefix || path.startsWith(prefix/)) so lookalike paths like /api/statusXYZ are NOT wrongly scoped (Sourcery review, PR #540). - ProfileScopeInterceptorTest: add lookalikePath_notScoped + scopedSubPath_isScoped to lock the segment-boundary fix. --- .../hermescontrol/data/remote/ApiClient.kt | 1 + .../data/remote/ProfileScopeInterceptor.kt | 77 +++++++++++ .../data/remote/ApiClientTest.kt | 5 + .../remote/ProfileScopeInterceptorTest.kt | 129 ++++++++++++++++++ 4 files changed, 212 insertions(+) create mode 100644 app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt create mode 100644 app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt index 6ae07356..fc6ed5e4 100644 --- a/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ApiClient.kt @@ -117,6 +117,7 @@ object ApiClient { .base .newBuilder() .addInterceptor(authInterceptor) + .addInterceptor(ProfileScopeInterceptor) .addInterceptor(logging) .authenticator(TokenRefreshAuthenticator) .certificatePinner(certificatePinner) diff --git a/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt b/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt new file mode 100644 index 00000000..75ec2592 --- /dev/null +++ b/app/src/main/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptor.kt @@ -0,0 +1,77 @@ +package com.m57.hermescontrol.data.remote + +import com.m57.hermescontrol.data.local.AuthManager +import okhttp3.Interceptor +import okhttp3.Response + +/** + * Appends `?profile=` to profile-scoped management endpoints, + * mirroring the desktop/web dashboard contract + * (`web/src/lib/api.ts` → `PROFILE_SCOPED_PREFIXES` + `withManagementProfile`). + * + * Without this, mobile fired all management REST calls with NO profile scope, + * so every profile read/wrote the backend's implicit default profile — + * multi-profile setups silently corrupted config/skills/toolsets/mcp/model/env + * (issue #528). + * + * Rules (copied from the desktop contract): + * - If no profile is active, pass through unchanged (legacy "default" behavior). + * - If the URL already carries an explicit `profile=` query, leave it + * untouched — explicit beats global. + * - Only the endpoint families below honor `?profile=` on the backend; + * everything else (ops, pairing, cron, profiles themselves) is + * machine-global or self-scoped and must NOT be rewritten. + */ +object ProfileScopeInterceptor : Interceptor { + private val PROFILE_SCOPED_PREFIXES = + listOf( + "/api/status", + "/api/gateway", + "/api/analytics", + "/api/skills", + "/api/tools/toolsets", + "/api/config", + "/api/env", + "/api/mcp", + "/api/messaging/platforms", + "/api/messaging/telegram/onboarding", + "/api/messaging/whatsapp/onboarding", + "/api/model/info", + "/api/model/set", + "/api/model/auxiliary", + "/api/model/moa", + "/api/model/options", + ) + + /** + * True only when [path] matches a scoped prefix as a whole path segment — + * e.g. `/api/status` or `/api/status/health`, but NOT `/api/statusXYZ` + * or `/api/gatewayExtra` (Sourcery review, PR #540). + */ + private fun isProfileScopedPath(path: String): Boolean = + PROFILE_SCOPED_PREFIXES.any { prefix -> path == prefix || path.startsWith("$prefix/") } + + override fun intercept(chain: Interceptor.Chain): Response { + val request = chain.request() + val profile = + AuthManager.getSelectedProfileId() + ?: return chain.proceed(request) + + val url = request.url + if (url.queryParameter("profile") != null) { + return chain.proceed(request) // explicit param wins + } + + if (!isProfileScopedPath(url.encodedPath)) { + return chain.proceed(request) + } + + val scopedUrl = + url + .newBuilder() + .addQueryParameter("profile", profile) + .build() + + return chain.proceed(request.newBuilder().url(scopedUrl).build()) + } +} diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt index d21aeecc..a1c9d591 100644 --- a/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/ApiClientTest.kt @@ -43,6 +43,11 @@ class ApiClientTest { every { AuthManager.baseUrl() } returns mockWebServer.url("/").toString() every { AuthManager.getHost() } returns "127.0.0.1" every { AuthManager.getPort() } returns 9119 + + // PR #540 registers ProfileScopeInterceptor in ApiClient.buildService(), + // which calls getSelectedProfileId(). Stub it to null so the interceptor + // short-circuits (no profile scope) and these auth tests stay focused. + every { AuthManager.getSelectedProfileId() } returns null } @AfterEach diff --git a/app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt b/app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt new file mode 100644 index 00000000..f87675b2 --- /dev/null +++ b/app/src/test/java/com/m57/hermescontrol/data/remote/ProfileScopeInterceptorTest.kt @@ -0,0 +1,129 @@ +package com.m57.hermescontrol.data.remote + +import com.m57.hermescontrol.data.local.AuthManager +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.unmockkAll +import okhttp3.HttpUrl +import okhttp3.OkHttpClient +import okhttp3.Request +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test + +class ProfileScopeInterceptorTest { + private lateinit var server: MockWebServer + + @Before + fun setUp() { + mockkObject(AuthManager) + server = MockWebServer() + server.start() + } + + @After + fun tearDown() { + server.shutdown() + unmockkAll() + } + + /** Builds a real client with the interceptor + a stubbed AuthManager profile. */ + private fun clientFor(profile: String?): OkHttpClient { + every { AuthManager.getSelectedProfileId() } returns profile + return OkHttpClient + .Builder() + .addInterceptor(ProfileScopeInterceptor) + .build() + } + + private fun lastRequestedPath(client: OkHttpClient): HttpUrl { + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/config")).build() + client.newCall(req).execute().close() + return server.takeRequest().requestUrl!! + } + + @Test + fun scopedEndpoint_appendsProfile() { + val client = clientFor("work") + val url = lastRequestedPath(client) + assertEquals("work", url.queryParameter("profile")) + assertEquals("/api/config", url.encodedPath) + } + + @Test + fun noActiveProfile_passesThrough() { + val client = clientFor(null) + val url = lastRequestedPath(client) + assertNull(url.queryParameter("profile")) + } + + @Test + fun explicitProfileParam_wins() { + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = + Request + .Builder() + .url(server.url("api/config?profile=other")) + .build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertEquals("other", url.queryParameter("profile")) + } + + @Test + fun nonScopedEndpoint_untouched() { + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/cron/jobs")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertNull(url.queryParameter("profile")) + assertEquals("/api/cron/jobs", url.encodedPath) + } + + @Test + fun skillsEndpoint_isScoped() { + val client = clientFor("alpha") + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/skills")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertEquals("alpha", url.queryParameter("profile")) + } + + @Test + fun lookalikePath_notScoped() { + // Sourcery review (PR #540): `startsWith` must not match non-segment + // suffixes like /api/statusXYZ or /api/gatewayExtra. + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/statusXYZ")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertNull(url.queryParameter("profile")) + assertEquals("/api/statusXYZ", url.encodedPath) + } + + @Test + fun scopedSubPath_isScoped() { + // A scoped prefix with a trailing segment (/api/status/health) MUST + // still receive the profile param. + every { AuthManager.getSelectedProfileId() } returns "work" + val client = OkHttpClient.Builder().addInterceptor(ProfileScopeInterceptor).build() + server.enqueue(MockResponse().setResponseCode(200).setBody("{}")) + val req = Request.Builder().url(server.url("api/status/health")).build() + client.newCall(req).execute().close() + val url = server.takeRequest().requestUrl!! + assertEquals("work", url.queryParameter("profile")) + assertEquals("/api/status/health", url.encodedPath) + } +}