diff --git a/apps/desktop/electron/connection-config.test.ts b/apps/desktop/electron/connection-config.test.ts index 404997d124..7b61b3bf4f 100644 --- a/apps/desktop/electron/connection-config.test.ts +++ b/apps/desktop/electron/connection-config.test.ts @@ -41,6 +41,7 @@ import { profileSshOverride, remoteRequestMatchesBaseUrl, resolveAuthMode, + resolveProfileApiRequest, resolveProfileBackendRoute, resolveRemoteSshDashboardProfile, resolveTestWsUrl, @@ -354,9 +355,15 @@ const ROUTES = [ expected: { backend: 'pool', descriptorProfile: null, scopePath: false } }, { - name: 'a local non-primary profile gets its own pooled backend', + name: 'an unscoped local profile request keeps its pooled backend', profile: 'coder', - opts: { primaryProfile: 'default', globalRemote: false, profileRemoteOverride: false }, + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'POST', + requestPath: '/api/memory/reset' + }, expected: { backend: 'pool', descriptorProfile: null, scopePath: false } }, { @@ -382,6 +389,44 @@ const ROUTES = [ ownEntry: true }, expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + }, + { + name: 'a profile-aware local REST request reuses the primary backend', + profile: 'coder', + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'GET', + requestPath: '/api/config' + }, + expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true } + }, + { + name: 'a profile-management request uses the primary without a query scope', + profile: 'coder', + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'DELETE', + requestPath: '/api/profiles/worker' + }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'a stored local profile never reuses a remote primary for an eligible REST route', + profile: 'coder', + opts: { + primaryProfile: 'default', + globalRemote: false, + profileRemoteOverride: false, + primaryRemoteActive: true, + ownEntry: true, + requestMethod: 'GET', + requestPath: '/api/config' + }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } } ] @@ -547,6 +592,27 @@ test('translateSelfProfileQuery no-ops when alias and backend profile agree or a assert.equal(translateSelfProfileQuery('/api/cron/jobs?profile=mara', '', 'default'), '/api/cron/jobs?profile=mara') }) +test('pathWithGlobalRemoteProfile appends local-primary profile scope only for eligible routes', () => { + assert.equal( + pathWithGlobalRemoteProfile('/api/config', 'iris', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'GET', + requestPath: '/api/config' + }), + '/api/config?profile=iris' + ) + assert.equal( + pathWithGlobalRemoteProfile('/api/memory/reset', 'iris', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'POST', + requestPath: '/api/memory/reset' + }), + '/api/memory/reset' + ) +}) + test('pathWithGlobalRemoteProfile skips empty profile/path safely', () => { assert.equal( pathWithGlobalRemoteProfile('/api/model/info', '', { @@ -564,6 +630,130 @@ test('pathWithGlobalRemoteProfile skips empty profile/path safely', () => { ) }) +// --- resolveProfileApiRequest --- + +test('resolveProfileApiRequest keeps eligible local REST on the primary backend', () => { + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/config?view=desktop', { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: 'GET' + }), + { + backendProfile: null, + requestPath: '/api/config?view=desktop&profile=iris' + } + ) +}) + +test('resolveProfileApiRequest keeps unscoped destructive routes on the profile backend', () => { + for (const [method, path] of [ + ['POST', '/api/memory/reset'], + ['POST', '/api/curator/run'], + ['PUT', '/api/curator/paused'], + ['POST', '/api/webhooks'] + ]) { + assert.deepEqual( + resolveProfileApiRequest('iris', path, { + globalRemote: false, + profileRemoteOverride: false, + requestMethod: method + }), + { backendProfile: 'iris', requestPath: path } + ) + } +}) + +test('resolveProfileApiRequest uses exact method and path eligibility for mixed families', () => { + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/skills', { + requestMethod: 'GET' + }), + { backendProfile: null, requestPath: '/api/skills?profile=iris' } + ) + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/skills', { + requestMethod: 'POST' + }), + { backendProfile: 'iris', requestPath: '/api/skills' } + ) + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/config/defaults', { + requestMethod: 'GET' + }), + { backendProfile: 'iris', requestPath: '/api/config/defaults' } + ) + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/model/recommended-default?provider=nous', { + requestMethod: 'GET' + }), + { + backendProfile: 'iris', + requestPath: '/api/model/recommended-default?provider=nous' + } + ) +}) + +test('resolveProfileApiRequest scopes complete safe families according to their contracts', () => { + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/tools/toolsets/image_gen/config', { + requestMethod: 'GET' + }), + { + backendProfile: null, + requestPath: '/api/tools/toolsets/image_gen/config?profile=iris' + } + ) + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/profiles/worker', { + requestMethod: 'DELETE' + }), + { + backendProfile: null, + requestPath: '/api/profiles/worker' + } + ) +}) + +test('resolveProfileApiRequest preserves remote routing precedence', () => { + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/memory/reset', { + globalRemote: true, + profileRemoteOverride: false, + requestMethod: 'POST' + }), + { + backendProfile: null, + requestPath: '/api/memory/reset?profile=iris' + } + ) + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/config', { + globalRemote: true, + profileRemoteOverride: true, + requestMethod: 'GET' + }), + { + backendProfile: 'iris', + requestPath: '/api/config' + } + ) +}) + +test('resolveProfileApiRequest keeps a stored local profile off a remote primary', () => { + assert.deepEqual( + resolveProfileApiRequest('iris', '/api/config', { + primaryRemoteActive: true, + ownEntry: true, + requestMethod: 'GET' + }), + { + backendProfile: 'iris', + requestPath: '/api/config' + } + ) +}) + // --- normalizeRemoteBaseUrl --- test('normalizeRemoteBaseUrl strips trailing slashes, hash, and query', () => { diff --git a/apps/desktop/electron/connection-config.ts b/apps/desktop/electron/connection-config.ts index 395be771e9..88c3ae2e00 100644 --- a/apps/desktop/electron/connection-config.ts +++ b/apps/desktop/electron/connection-config.ts @@ -506,6 +506,8 @@ export interface ProfileRouteOptions { primaryRemoteActive?: boolean /** A stored per-profile entry exists for this profile (local or remote). */ ownEntry?: boolean + requestMethod?: null | string + requestPath?: null | string } export interface ProfileBackendRoute { @@ -521,16 +523,83 @@ export interface ProfileBackendRoute { scopePath: boolean } +const LOCAL_PRIMARY_SCOPED_ROUTES = new Set([ + 'GET /api/config', + 'PUT /api/config', + 'GET /api/config/raw', + 'PUT /api/config/raw', + 'GET /api/config/schema', + 'DELETE /api/env', + 'GET /api/env', + 'PUT /api/env', + 'POST /api/env/reveal', + 'GET /api/model/auxiliary', + 'GET /api/model/info', + 'GET /api/model/moa', + 'PUT /api/model/moa', + 'GET /api/model/options', + 'POST /api/model/set', + 'GET /api/skills', + 'GET /api/skills/content', + 'PUT /api/skills/toggle', + 'POST /api/skills/hub/install', + 'GET /api/skills/hub/preview', + 'GET /api/skills/hub/scan', + 'GET /api/skills/hub/search', + 'GET /api/skills/hub/sources', + 'POST /api/skills/hub/uninstall', + 'POST /api/skills/hub/update' +]) + +function localPrimaryRequestScope(opts: ProfileRouteOptions): boolean | null { + const rawPath = String(opts.requestPath || '') + + if (!rawPath) { + return null + } + + let pathname + + try { + pathname = new URL(rawPath, 'https://example.invalid').pathname + } catch { + return null + } + + const method = String(opts.requestMethod || 'GET').toUpperCase() + + if (LOCAL_PRIMARY_SCOPED_ROUTES.has(`${method} ${pathname}`)) { + return true + } + + // Every current /api/tools handler accepts `profile`; every /api/profiles + // handler either aggregates profiles or names its target in the path/body. + // These are the only whole families safe to route through the primary. + if (pathname === '/api/tools' || pathname.startsWith('/api/tools/')) { + return true + } + + if (pathname === '/api/profiles' || pathname.startsWith('/api/profiles/')) { + return false + } + + return null +} + /** * The one place that answers "which backend serves profile P, and does its - * REST path need a profile scope?". Four routes, in precedence order: + * REST path need a profile scope?". Six routes, in precedence order: * * 1. The primary profile owns the window backend outright. * 2. A profile with its own remote override gets a pooled descriptor for that * host, which is already scoped to it. * 3. A profile inheriting the app-global remote shares the primary backend — * one host serves every profile — so it is scoped per request instead. - * 4. Any other local profile gets its own pooled backend, spawned with + * 4. An unknown profile under a remote primary shares that remote backend. + * A stored local profile remains isolated in its own backend. + * 5. A local profile REST request that the primary backend can safely scope + * reuses that backend, with `?profile=` when the handler accepts it. + * 6. Any other local profile gets its own pooled backend, spawned with * `--profile`, so its `HERMES_HOME` scopes it. * * Routing used to be spread across three overlapping predicates that each @@ -553,12 +622,28 @@ function resolveProfileBackendRoute(profile, opts: ProfileRouteOptions = {}): Pr return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true } } - if (opts.primaryRemoteActive && !opts.ownEntry) { - // The primary profile's own backend is a remote gateway (per-profile - // override or env) and this sub-profile has no stored entry of its own. - // Route through that gateway with profile scoping instead of spawning a - // fresh local backend that shares nothing but the name (#88296). - return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true } + if (opts.primaryRemoteActive) { + if (!opts.ownEntry) { + // The primary profile's own backend is a remote gateway (per-profile + // override or env) and this sub-profile has no stored entry of its own. + // Route through that gateway with profile scoping instead of spawning a + // fresh local backend that shares nothing but the name (#88296). + return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true } + } + + // A stored local profile must not be redirected into the remote primary, + // even when its REST endpoint supports profile scoping. + return { backend: 'pool', descriptorProfile: null, scopePath: false } + } + + const localScope = localPrimaryRequestScope(opts) + + if (localScope !== null) { + return { + backend: 'primary', + descriptorProfile: localScope ? scopedProfile : null, + scopePath: localScope + } } return { backend: 'pool', descriptorProfile: null, scopePath: false } @@ -680,6 +765,29 @@ function apiRequestRegistryConnectionId(request): null | string { return id } +export interface ProfileApiRequestRoute { + /** Profile passed to ensureBackend; null selects the primary backend. */ + backendProfile: null | string + requestPath: string +} + +/** + * Resolve the two decisions made by the `hermes:api` IPC handler from the same + * routing table: which backend serves the request, and whether its URL needs a + * profile query scope. + */ +function resolveProfileApiRequest(profile, path, opts: ProfileRouteOptions = {}): ProfileApiRequestRoute { + const scopedProfile = connectionScopeKey(profile) + const requestPath = String(path || '') + const routeOpts = { ...opts, requestPath } + const route = resolveProfileBackendRoute(scopedProfile, routeOpts) + + return { + backendProfile: route.backend === 'pool' ? scopedProfile : null, + requestPath: pathWithGlobalRemoteProfile(requestPath, scopedProfile, routeOpts) + } +} + function tokenPreview(value) { const raw = String(value || '') @@ -822,6 +930,7 @@ export { profileSshOverride, remoteRequestMatchesBaseUrl, resolveAuthMode, + resolveProfileApiRequest, resolveProfileBackendRoute, resolveRemoteSshDashboardProfile, resolveTestWsUrl, diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 25c64f9bc6..be6cb891e9 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -86,6 +86,7 @@ import { profileSshOverride, remoteRequestMatchesBaseUrl, resolveAuthMode, + resolveProfileApiRequest, resolveProfileBackendRoute, resolveRemoteSshDashboardProfile, resolveTestWsUrl, @@ -9484,7 +9485,7 @@ function primaryProfileKey() { } // Options describing the current connection setup for `resolveProfileBackendRoute`. -function profileRouteOptions(profile) { +function profileRouteOptions(profile, request?) { const config = readDesktopConnectionConfig() const sshOverride = profileSshOverride(config, profile) const key = connectionScopeKey(profile) || primaryProfileKey() @@ -9502,7 +9503,9 @@ function profileRouteOptions(profile) { primaryRemoteActive: primaryBackendIsRemote(), // A stored per-profile entry (local or remote) — pins this profile to // its own backend; absent entries inherit the primary's remote. - ownEntry: Boolean((readDesktopConnectionConfig().profiles || {})[key]) + ownEntry: Boolean((config.profiles || {})[key]), + requestMethod: request?.method, + requestPath: request?.path } } @@ -13228,13 +13231,17 @@ async function handleHermesApiRequest(request) { // backend instead of spawning a fresh pool backend. A freshly spawned // backend calls ensure_hermes_home() which recreates the profile directory, // defeating the deletion and leaving a zombie process. - const routeProfile = resolveRouteProfile(tornDownProfile, profile) + // + // Safe local-profile REST calls also stay on the primary dashboard and carry + // ?profile=. Endpoints that cannot honor that scope retain their pooled + // backend so a destructive call can never fall through to the primary home. + const apiRoute = resolveProfileApiRequest(profile, request.path, profileRouteOptions(profile, request)) + const routeProfile = resolveRouteProfile(tornDownProfile, apiRoute.backendProfile) + const connection = await ensureBackend(routeProfile) const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS) - const requestPath = pathWithGlobalRemoteProfile(request.path, profile, profileRouteOptions(profile)) - - const url = `${connection.baseUrl}${requestPath}` + const url = `${connection.baseUrl}${apiRoute.requestPath}` // OAuth gateways authenticate REST via EITHER a native bearer token // (cookieless RFC 8252 flow) OR the HttpOnly session cookie held in the OAuth diff --git a/apps/desktop/electron/profile-delete-routing.test.ts b/apps/desktop/electron/profile-delete-routing.test.ts index 301c5c88db..df8ff09a07 100644 --- a/apps/desktop/electron/profile-delete-routing.test.ts +++ b/apps/desktop/electron/profile-delete-routing.test.ts @@ -147,3 +147,7 @@ test('localProfilePoolKeys returns every local process scope for one profile', ( assert.deepEqual(localProfilePoolKeys('Selena'), ['selena', 'conn:local::selena']) assert.deepEqual(localProfilePoolKeys(''), []) }) + +test('resolveRouteProfile preserves a primary-backend route from another routing policy', () => { + assert.equal(resolveRouteProfile(null, null), null) +}) diff --git a/apps/desktop/electron/profile-delete-routing.ts b/apps/desktop/electron/profile-delete-routing.ts index 3fefba9d91..55e79cb12e 100644 --- a/apps/desktop/electron/profile-delete-routing.ts +++ b/apps/desktop/electron/profile-delete-routing.ts @@ -180,7 +180,7 @@ export function decideProfileDeleteAction( */ export function resolveRouteProfile( tornDownProfile: string | null, - profile: string | undefined + profile: string | null | undefined ): string | null | undefined { return tornDownProfile ? null : profile } diff --git a/apps/desktop/src/hermes.ts b/apps/desktop/src/hermes.ts index a80d895d0f..cf44f0e478 100644 --- a/apps/desktop/src/hermes.ts +++ b/apps/desktop/src/hermes.ts @@ -249,9 +249,10 @@ export class HermesGateway extends JsonRpcGatewayClient { // Profile that profile-scoped REST settings (config/env/skills/tools/model/…) // should target. Mirrors $activeGatewayProfile, pushed in from the store via // setApiRequestProfile so this module needs no store import (avoids a cycle). -// Electron main consumes request.profile to pick which backend *process* serves -// the call; each pooled backend already has its own HERMES_HOME, so no backend -// change is needed. Null → primary, so single-profile users are unaffected. +// Electron main consumes request.profile as request scope. Local calls whose +// REST handlers accept profile reuse the primary dashboard via ?profile=; +// unscoped handlers retain a profile backend. Remote overrides still route to +// their owning backend. Null → primary, so single-profile users are unaffected. let _apiProfile: null | string = null export function setApiRequestProfile(profile: null | string): void { @@ -678,10 +679,8 @@ export async function listSidebarSessions(req: SidebarSessionsRequest): Promise< } } -// Mutations take the owning `profile` so Electron routes them to that profile's -// backend (remote pool or local primary) via request.profile — matching the -// read path. A remote session's row lives only on its remote host, so a mutation -// that hit the local primary would no-op or 404. Omit for the current/default. +// Mutations take the owning `profile` so Electron can route them to the correct +// remote backend or local profile scope. Omit for the current/default profile. export function setSessionArchived(id: string, archived: boolean, profile?: string | null): Promise<{ ok: boolean }> { return window.hermesDesktop.api<{ ok: boolean }>({ ...(profile ? { profile } : {}),