From 3e22ffeedfa901dc30363c6648af014f873dbb56 Mon Sep 17 00:00:00 2001 From: Yuhao Chen Date: Fri, 22 May 2026 21:22:13 +0800 Subject: [PATCH] fix(app): normalize cloud core RPC URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Normalize cloud core URLs so users can paste a core base URL like `https://example.trycloudflare.com` and still reach the JSON-RPC endpoint. - Apply the same normalization in the cloud-mode picker, persisted URL reads/writes, restored core mode state, and direct RPC probing. - Add regression coverage for Cloudflare-style base URLs, existing `/rpc` URLs, and previously persisted base URLs. ## Problem - Users connecting the desktop client to a self-hosted core through Cloudflare Tunnel may paste the tunnel base URL instead of the `/rpc` endpoint. - The core root is reachable, but JSON-RPC calls belong on `/rpc`; using the base URL can make the connection flow fail even though the tunnel itself is healthy. - The issue report surfaced this as a 405 during remote-core connection setup. ## Solution - Extend `normalizeRpcUrl` to append `/rpc` when the input URL has no path, while preserving existing `/rpc` URLs and non-root paths. - Reuse `normalizeRpcUrl` across `BootCheckGate`, `coreRpcClient`, `configPersistence`, and `coreModeSlice` so test connection, boot check, cached URL resolution, and localStorage restoration all agree. - Keep existing HTTP restrictions unchanged: public cloud URLs still require HTTPS, while local/private HTTP hosts remain allowed. ## Submission Checklist > If a section does not apply to this change, mark the item as `N/A` with a one-line reason. Do not delete items. - [x] Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement) - [x] **Diff coverage >= 80%** — focused Vitest coverage was added for the changed URL normalization paths; CI will enforce the merged diff-coverage gate. - [x] Coverage matrix updated — N/A: behaviour-only cloud URL normalization fix; no feature matrix row added/removed/renamed. - [x] All affected feature IDs from the matrix are listed in the PR description under `## Related` — N/A: no coverage-matrix feature ID touched. - [x] No new external network dependencies introduced (mock backend used per [Testing Strategy](../gitbooks/developing/testing-strategy.md#mock-policy)) - [x] Manual smoke checklist updated if this touches release-cut surfaces ([`docs/RELEASE-MANUAL-SMOKE.md`](../docs/RELEASE-MANUAL-SMOKE.md)) — N/A: no release-cut smoke checklist surface changed. - [x] Linked issue closed via `Closes #NNN` in the `## Related` section ## Impact - Runtime/platform: desktop/web app cloud-core connection setup and RPC URL resolution. - Compatibility: existing stored `/rpc` URLs continue to resolve unchanged; previously stored base URLs now self-heal on read. - Security: public HTTP cloud URLs are still rejected; no auth behavior or token storage behavior changes. ## Related - Closes #2467 - Follow-up PR(s)/TODOs: none --- ## AI Authored PR Metadata (required for Codex/Linear PRs) > Keep this section for AI-authored PRs. For human-only PRs, mark each field `N/A`. ### Linear Issue - Key: N/A - URL: N/A ### Commit & Branch - Branch: `yuhao/fix-remote-core-cloudflare-2467` - Commit SHA: `5e95aeed8a97acee5823d73b6dc8e92f04af00fb` ### Validation Run - [x] `pnpm --filter openhuman-app format:check` - [x] `pnpm typecheck` - [x] Focused tests: `pnpm --dir app exec vitest run --config test/vitest.config.ts src/services/__tests__/coreRpcClient.test.ts src/utils/__tests__/configPersistence.test.ts src/store/coreModeSlice.test.ts src/components/BootCheckGate/__tests__/BootCheckGate.test.tsx` — 200 passed - [x] Rust fmt/check (if changed): N/A: no Rust source changes; app format gate still ran Rust format checks. - [x] Tauri fmt/check (if changed): N/A: no Tauri shell source changes; app format gate still ran Tauri Rust format checks. ### Validation Blocked - `command:` N/A - `error:` N/A - `impact:` N/A ### Behavior Changes - Intended behavior change: cloud core base URLs with no path are normalized to `/rpc`. - User-visible effect: users can paste a Cloudflare Tunnel base URL into the cloud runtime picker without manually appending `/rpc`. ### Parity Contract - Legacy behavior preserved: existing `/rpc` URLs, auth token handling, RPC POST envelopes, and public-HTTP rejection behavior are unchanged. - Guard/fallback/dispatch parity checks: focused tests cover picker continuation, test connection, cached URL resolution, persisted URL reads/writes, and core-mode localStorage restoration. ### Duplicate / Superseded PR Handling - Duplicate PR(s): none found for #2467 by current open issue/PR review. - Canonical PR: this PR. - Resolution (closed/superseded/updated): N/A. ## Summary by CodeRabbit * **Bug Fixes** * Consistently normalize cloud RPC URLs: trims input, handles trailing slashes, and ensures the /rpc endpoint across input, storage, retrieval, and connection probes. * Safer RPC logging: credentials/query/hash are redacted for logged URLs. * **Tests** * Expanded coverage for URL normalization across connection flows, storage/readback, and boot checks. * **Localization** * Added German translations for subconscious and MCP server/settings UI strings. [![Review Change Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2480?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) Co-authored-by: YUHAO-corn Co-authored-by: M3gA-Mind --- .../BootCheckGate/BootCheckGate.tsx | 10 ++-- .../__tests__/BootCheckGate.test.tsx | 46 ++++++++++++++++++ .../services/__tests__/coreRpcClient.test.ts | 47 +++++++++++++++++++ app/src/services/coreRpcClient.ts | 18 +++---- app/src/store/coreModeSlice.test.ts | 19 +++++++- app/src/store/coreModeSlice.ts | 3 +- .../utils/__tests__/configPersistence.test.ts | 45 ++++++++++++++++-- app/src/utils/configPersistence.ts | 47 +++++++++++++++++-- app/src/utils/redactRpcUrlForLog.ts | 12 +++++ 9 files changed, 225 insertions(+), 22 deletions(-) create mode 100644 app/src/utils/redactRpcUrlForLog.ts diff --git a/app/src/components/BootCheckGate/BootCheckGate.tsx b/app/src/components/BootCheckGate/BootCheckGate.tsx index e531b6919..4df60a84a 100644 --- a/app/src/components/BootCheckGate/BootCheckGate.tsx +++ b/app/src/components/BootCheckGate/BootCheckGate.tsx @@ -26,6 +26,7 @@ import { clearStoredCoreMode, clearStoredCoreToken, isLocalOrPrivateNetworkHost, + normalizeRpcUrl, storeCoreMode, storeCoreToken, storeRpcUrl, @@ -122,13 +123,14 @@ function ModePicker({ onConfirm }: PickerProps) { * paths are passed through verbatim without the bearer value. */ const validateInputs = (): { url: string; token: string } | null => { - const trimmedUrl = cloudUrl.trim(); - if (!trimmedUrl) { + const rawUrl = cloudUrl.trim(); + if (!rawUrl) { setUrlError(t('bootCheck.invalidUrl')); return null; } + const normalizedUrl = normalizeRpcUrl(rawUrl); try { - const parsed = new URL(trimmedUrl); + const parsed = new URL(normalizedUrl); if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { setUrlError(t('bootCheck.urlMustStartWith')); return null; @@ -152,7 +154,7 @@ function ModePicker({ onConfirm }: PickerProps) { } setTokenError(null); - return { url: trimmedUrl, token: trimmedToken }; + return { url: normalizedUrl, token: trimmedToken }; }; const handleTestConnection = async () => { diff --git a/app/src/components/BootCheckGate/__tests__/BootCheckGate.test.tsx b/app/src/components/BootCheckGate/__tests__/BootCheckGate.test.tsx index cb754fa33..973e71806 100644 --- a/app/src/components/BootCheckGate/__tests__/BootCheckGate.test.tsx +++ b/app/src/components/BootCheckGate/__tests__/BootCheckGate.test.tsx @@ -187,6 +187,32 @@ describe('BootCheckGate — picker (unset mode)', () => { ); }); + it('normalizes a cloud core base URL to the /rpc endpoint before continuing', async () => { + mockRunBootCheck.mockResolvedValue({ kind: 'match' }); + + renderGate(); + fireEvent.click(screen.getByText('Run on the Cloud (Complex)')); + fireEvent.change(screen.getByPlaceholderText(/https:\/\/core\.example\.com/), { + target: { value: 'https://example.trycloudflare.com/' }, + }); + fireEvent.change(screen.getByPlaceholderText(/Bearer token/i), { + target: { value: 'tok-1234' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Continue' })); + + await waitFor(() => { + expect(screen.getByTestId('app-content')).toBeInTheDocument(); + }); + expect(mockRunBootCheck).toHaveBeenCalledWith( + expect.objectContaining({ + kind: 'cloud', + url: 'https://example.trycloudflare.com/rpc', + token: 'tok-1234', + }), + expect.any(Object) + ); + }); + it('rejects public HTTP cloud URLs', () => { renderGate(); @@ -274,6 +300,26 @@ describe('BootCheckGate — picker test connection', () => { ); }); + it('tests /rpc when the user enters a cloud core base URL', async () => { + mockTestCoreRpcConnection.mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ result: { ok: true } }), + } as unknown as Response); + + renderGate(); + fillCloudInputs('https://example.trycloudflare.com/'); + fireEvent.click(screen.getByRole('button', { name: 'Test Connection' })); + + await waitFor(() => { + expect(screen.getByTestId('test-status-ok')).toBeInTheDocument(); + }); + expect(mockTestCoreRpcConnection).toHaveBeenCalledWith( + 'https://example.trycloudflare.com/rpc', + 'tok-abc' + ); + }); + it('shows Auth failed on a 401 response', async () => { mockTestCoreRpcConnection.mockResolvedValue({ ok: false, diff --git a/app/src/services/__tests__/coreRpcClient.test.ts b/app/src/services/__tests__/coreRpcClient.test.ts index 166b367b1..707c853d5 100644 --- a/app/src/services/__tests__/coreRpcClient.test.ts +++ b/app/src/services/__tests__/coreRpcClient.test.ts @@ -582,6 +582,19 @@ describe('coreRpcClient', () => { }); }); + test('normalizes a supplied core base URL before probing', async () => { + vi.resetModules(); + vi.mocked(isTauri).mockReturnValue(false); + const { testCoreRpcConnection } = await import('../coreRpcClient'); + const fetchMock = vi.mocked(fetch); + fetchMock.mockResolvedValueOnce({ ok: true, status: 200 } as Response); + + await testCoreRpcConnection('https://example.trycloudflare.com/'); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(fetchMock.mock.calls[0][0]).toBe('https://example.trycloudflare.com/rpc'); + }); + test('omits Authorization header when no bearer token is available (non-Tauri)', async () => { vi.resetModules(); vi.mocked(isTauri).mockReturnValue(false); @@ -854,6 +867,11 @@ describe('coreRpcClient — typed errors + auth-expired event', () => { }); describe('getCoreRpcUrl', () => { + const normalizeMockRpcUrl = (url: string) => { + const trimmed = url.replace(/\/+$/, ''); + return trimmed.endsWith('/rpc') ? trimmed : `${trimmed}/rpc`; + }; + // Each test gets a fresh module so module-level caches are cleared beforeEach(() => { vi.resetModules(); @@ -865,6 +883,7 @@ describe('getCoreRpcUrl', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => 'http://custom-host:9999/rpc', getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(false); @@ -873,10 +892,24 @@ describe('getCoreRpcUrl', () => { expect(url).toBe('http://custom-host:9999/rpc'); }); + test('in web mode normalizes a stored core base URL', async () => { + vi.doMock('../../utils/configPersistence', () => ({ + peekStoredRpcUrl: () => 'https://example.trycloudflare.com/', + getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, + })); + vi.mocked(isTauri).mockReturnValue(false); + + const { getCoreRpcUrl: freshGetCoreRpcUrl } = await import('../coreRpcClient'); + const url = await freshGetCoreRpcUrl(); + expect(url).toBe('https://example.trycloudflare.com/rpc'); + }); + test('in web mode returns default CORE_RPC_URL when nothing is stored', async () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => null, getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(false); @@ -893,6 +926,7 @@ describe('getCoreRpcUrl', () => { return null; }, getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(false); @@ -909,6 +943,7 @@ describe('getCoreRpcUrl', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => storedValue, getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(false); @@ -930,6 +965,7 @@ describe('getCoreRpcUrl', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => null, getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); vi.mocked(invoke).mockImplementation(async (cmd: string) => { @@ -947,6 +983,7 @@ describe('getCoreRpcUrl', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => 'http://stored-override:4444/rpc', getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); vi.mocked(invoke).mockImplementation(async (cmd: string) => { @@ -968,6 +1005,7 @@ describe('getCoreRpcUrl', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => 'http://127.0.0.1:7788/rpc', getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); vi.mocked(invoke).mockImplementation(async (cmd: string) => { @@ -987,6 +1025,7 @@ describe('getCoreRpcUrl', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => null, getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); vi.mocked(invoke).mockRejectedValue(new Error('invoke failed')); @@ -999,6 +1038,11 @@ describe('getCoreRpcUrl', () => { }); describe('getCoreRpcToken (cloud-mode persistence)', () => { + const normalizeMockRpcUrl = (url: string) => { + const trimmed = url.replace(/\/+$/, ''); + return trimmed.endsWith('/rpc') ? trimmed : `${trimmed}/rpc`; + }; + beforeEach(() => { vi.resetModules(); vi.clearAllMocks(); @@ -1009,6 +1053,7 @@ describe('getCoreRpcToken (cloud-mode persistence)', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => 'https://core.example.com/rpc', getStoredCoreToken: () => 'cloud-token-abc', + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); vi.mocked(invoke).mockImplementation(async (cmd: string) => { @@ -1038,6 +1083,7 @@ describe('getCoreRpcToken (cloud-mode persistence)', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => 'https://core.example.com/rpc', getStoredCoreToken: () => storedToken, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); const fetchMock = vi.mocked(fetch); @@ -1065,6 +1111,7 @@ describe('getCoreRpcToken (cloud-mode persistence)', () => { vi.doMock('../../utils/configPersistence', () => ({ peekStoredRpcUrl: () => null, getStoredCoreToken: () => null, + normalizeRpcUrl: normalizeMockRpcUrl, })); vi.mocked(isTauri).mockReturnValue(true); vi.mocked(invoke).mockImplementation(async (cmd: string) => { diff --git a/app/src/services/coreRpcClient.ts b/app/src/services/coreRpcClient.ts index 610b3fecb..647c77cf1 100644 --- a/app/src/services/coreRpcClient.ts +++ b/app/src/services/coreRpcClient.ts @@ -3,7 +3,8 @@ import debug from 'debug'; import { dispatchLocalAiMethod } from '../lib/ai/localCoreAiMemory'; import { CORE_RPC_TIMEOUT_MS, CORE_RPC_URL } from '../utils/config'; -import { getStoredCoreToken, peekStoredRpcUrl } from '../utils/configPersistence'; +import { getStoredCoreToken, normalizeRpcUrl, peekStoredRpcUrl } from '../utils/configPersistence'; +import { redactRpcUrlForLog } from '../utils/redactRpcUrlForLog'; import { sanitizeError } from '../utils/sanitize'; import { isTauri as coreIsTauri } from '../utils/tauriCommands/common'; import { normalizeRpcMethod } from './rpcMethods'; @@ -278,7 +279,7 @@ export async function getCoreRpcUrl(): Promise { // null when nothing is stored, which lets us distinguish "user hasn't // chosen yet" from "user chose a value identical to the default". const storedUrl = peekStoredRpcUrl(); - resolvedCoreRpcUrl = storedUrl ?? CORE_RPC_URL; + resolvedCoreRpcUrl = normalizeRpcUrl(storedUrl ?? CORE_RPC_URL); return resolvedCoreRpcUrl; } @@ -296,8 +297,8 @@ export async function getCoreRpcUrl(): Promise { // cloud mode where no local sidecar is running. const storedUrl = peekStoredRpcUrl(); if (storedUrl) { - resolvedCoreRpcUrl = storedUrl; - return storedUrl; + resolvedCoreRpcUrl = normalizeRpcUrl(storedUrl); + return resolvedCoreRpcUrl; } const url = await invoke('core_rpc_url'); @@ -307,16 +308,16 @@ export async function getCoreRpcUrl(): Promise { fallback: CORE_RPC_URL, }); } - resolvedCoreRpcUrl = trimmed || CORE_RPC_URL; + resolvedCoreRpcUrl = normalizeRpcUrl(trimmed || CORE_RPC_URL); return resolvedCoreRpcUrl || CORE_RPC_URL; } catch (err) { // Tauri invoke failed — fall back to stored URL if any, then the // build-time default. Keep the underlying invoke failure visible so // port mismatches and shell misconfiguration are diagnosable. const storedUrl = peekStoredRpcUrl(); - resolvedCoreRpcUrl = storedUrl ?? CORE_RPC_URL; + resolvedCoreRpcUrl = normalizeRpcUrl(storedUrl ?? CORE_RPC_URL); coreRpcError('core_rpc_url invoke failed; using fallback RPC URL', { - fallback: resolvedCoreRpcUrl, + fallback: redactRpcUrlForLog(resolvedCoreRpcUrl), usedStoredUrl: Boolean(storedUrl), error: sanitizeError(err), }); @@ -397,12 +398,13 @@ export async function testCoreRpcConnection( tokenOverride?: string, init?: { signal?: AbortSignal } ): Promise { + const rpcUrl = normalizeRpcUrl(url); const token = tokenOverride?.trim() || (await getCoreRpcToken()); const headers: Record = { 'Content-Type': 'application/json' }; if (token) { headers.Authorization = `Bearer ${token}`; } - return fetch(url, { + return fetch(rpcUrl, { method: 'POST', headers, body: JSON.stringify({ jsonrpc: '2.0', id: 1, method: 'core.ping', params: {} }), diff --git a/app/src/store/coreModeSlice.test.ts b/app/src/store/coreModeSlice.test.ts index 05b91bb98..c40e7ed6e 100644 --- a/app/src/store/coreModeSlice.test.ts +++ b/app/src/store/coreModeSlice.test.ts @@ -67,7 +67,10 @@ describe('coreModeSlice — sync-localStorage-derived initial state', () => { it('uses local mode when the E2E default core mode config is local', async () => { localStorage.clear(); vi.resetModules(); - vi.doMock('../utils/config', () => ({ E2E_DEFAULT_CORE_MODE: 'local' })); + vi.doMock('../utils/config', () => ({ + CORE_RPC_URL: 'http://127.0.0.1:7788/rpc', + E2E_DEFAULT_CORE_MODE: 'local', + })); try { const mod = await import('./coreModeSlice'); const state = mod.default(undefined, { type: '@@INIT' }); @@ -100,6 +103,20 @@ describe('coreModeSlice — sync-localStorage-derived initial state', () => { }); }); + it('normalizes restored cloud base URLs to the /rpc endpoint', async () => { + localStorage.clear(); + localStorage.setItem('openhuman_core_mode', 'cloud'); + localStorage.setItem('openhuman_core_rpc_url', 'https://example.trycloudflare.com/'); + localStorage.setItem('openhuman_core_rpc_token', 'tok-abc'); + const mod = await freshImport(); + const state = mod.default(undefined, { type: '@@INIT' }); + expect(state.mode).toEqual({ + kind: 'cloud', + url: 'https://example.trycloudflare.com/rpc', + token: 'tok-abc', + }); + }); + it('falls back to unset when cloud marker exists but URL or token is missing', async () => { localStorage.clear(); localStorage.setItem('openhuman_core_mode', 'cloud'); diff --git a/app/src/store/coreModeSlice.ts b/app/src/store/coreModeSlice.ts index f68a3914d..8ece391b2 100644 --- a/app/src/store/coreModeSlice.ts +++ b/app/src/store/coreModeSlice.ts @@ -13,6 +13,7 @@ import { createSlice, type PayloadAction } from '@reduxjs/toolkit'; import { E2E_DEFAULT_CORE_MODE } from '../utils/config'; +import { normalizeRpcUrl } from '../utils/configPersistence'; export type CoreMode = | { kind: 'unset' } @@ -64,7 +65,7 @@ function deriveInitialMode(): CoreMode { if (mode === 'cloud') { const url = localStorage.getItem(RPC_URL_STORAGE_KEY)?.trim(); const token = localStorage.getItem(CORE_TOKEN_STORAGE_KEY)?.trim(); - if (url && token) return { kind: 'cloud', url, token }; + if (url && token) return { kind: 'cloud', url: normalizeRpcUrl(url), token }; } } catch { /* localStorage unavailable — fall through to unset */ diff --git a/app/src/utils/__tests__/configPersistence.test.ts b/app/src/utils/__tests__/configPersistence.test.ts index b1b9b537d..7354e9a12 100644 --- a/app/src/utils/__tests__/configPersistence.test.ts +++ b/app/src/utils/__tests__/configPersistence.test.ts @@ -17,6 +17,7 @@ import { isValidRpcUrl, normalizeRpcUrl, peekStoredRpcUrl, + redactRpcUrlForLog, storeCoreMode, storeCoreToken, storeRpcUrl, @@ -142,7 +143,7 @@ describe('configPersistence', () => { it('removes trailing slashes', () => { expect(normalizeRpcUrl('http://localhost:7788/rpc/')).toBe('http://localhost:7788/rpc'); - expect(normalizeRpcUrl('http://localhost:7788/')).toBe('http://localhost:7788'); + expect(normalizeRpcUrl('http://localhost:7788/')).toBe('http://localhost:7788/rpc'); }); it('handles multiple trailing slashes', () => { @@ -152,6 +153,28 @@ describe('configPersistence', () => { it('preserves URL without trailing slash', () => { expect(normalizeRpcUrl('http://localhost:7788/rpc')).toBe('http://localhost:7788/rpc'); }); + + it('preserves query and hash values when normalizing paths', () => { + expect(normalizeRpcUrl('https://host.example?next=/')).toBe( + 'https://host.example/rpc?next=/' + ); + expect(normalizeRpcUrl('https://host.example/#/')).toBe('https://host.example/rpc#/'); + expect(normalizeRpcUrl('https://host.example/rpc/?next=/#/')).toBe( + 'https://host.example/rpc?next=/#/' + ); + }); + }); + + describe('redactRpcUrlForLog', () => { + it('removes credentials, query, and hash values before logging', () => { + expect(redactRpcUrlForLog('https://user:pass@host.example/rpc?token=secret#/token')).toBe( + 'https://host.example/rpc' + ); + }); + + it('returns a sentinel for malformed URLs', () => { + expect(redactRpcUrlForLog('not a url')).toBe('[invalid-url]'); + }); }); describe('getDefaultRpcUrl', () => { @@ -258,8 +281,11 @@ describe('configPersistence', () => { }); describe('normalizeRpcUrl — edge cases', () => { - it('does not add /rpc suffix when missing (normalizeRpcUrl only strips, not appends)', () => { - expect(normalizeRpcUrl('http://127.0.0.1:7788')).toBe('http://127.0.0.1:7788'); + it('adds /rpc suffix when given a core base URL', () => { + expect(normalizeRpcUrl('http://127.0.0.1:7788')).toBe('http://127.0.0.1:7788/rpc'); + expect(normalizeRpcUrl('https://example.trycloudflare.com/')).toBe( + 'https://example.trycloudflare.com/rpc' + ); }); it('does not double-add /rpc — leaves existing /rpc alone', () => { @@ -285,6 +311,19 @@ describe('configPersistence', () => { }); describe('storeRpcUrl + getStoredRpcUrl — round-trip', () => { + it('stores normalized base core URLs as RPC endpoints', () => { + storeRpcUrl('https://remote.example.com'); + expect(localStorage.getItem(STORAGE_KEY)).toBe('https://remote.example.com/rpc'); + expect(getStoredRpcUrl()).toBe('https://remote.example.com/rpc'); + expect(peekStoredRpcUrl()).toBe('https://remote.example.com/rpc'); + }); + + it('normalizes previously persisted base core URLs on read', () => { + localStorage.setItem(STORAGE_KEY, 'https://old.example.com/'); + expect(getStoredRpcUrl()).toBe('https://old.example.com/rpc'); + expect(peekStoredRpcUrl()).toBe('https://old.example.com/rpc'); + }); + it('round-trips an HTTPS URL', () => { storeRpcUrl('https://remote.example.com/rpc'); expect(getStoredRpcUrl()).toBe('https://remote.example.com/rpc'); diff --git a/app/src/utils/configPersistence.ts b/app/src/utils/configPersistence.ts index 1e2e62b1c..3023cf007 100644 --- a/app/src/utils/configPersistence.ts +++ b/app/src/utils/configPersistence.ts @@ -4,9 +4,16 @@ * Handles storing/retrieving user preferences like RPC URL using * localStorage (web) or Tauri store (desktop). */ +import debug from 'debug'; + import { CORE_RPC_URL, E2E_DEFAULT_CORE_MODE } from './config'; +import { redactRpcUrlForLog } from './redactRpcUrlForLog'; import { isTauri } from './tauriCommands'; +export { redactRpcUrlForLog } from './redactRpcUrlForLog'; + +const log = debug('config-persistence'); + // Storage key for RPC URL preference const RPC_URL_STORAGE_KEY = 'openhuman_core_rpc_url'; @@ -43,7 +50,7 @@ export function getStoredRpcUrl(): string { try { const stored = localStorage.getItem(RPC_URL_STORAGE_KEY); if (stored && stored.trim().length > 0) { - return stored.trim(); + return normalizeRpcUrl(stored); } } catch { // localStorage might be unavailable in some environments @@ -68,7 +75,7 @@ export function peekStoredRpcUrl(): string | null { try { const stored = localStorage.getItem(RPC_URL_STORAGE_KEY); if (stored && stored.trim().length > 0) { - return stored.trim(); + return normalizeRpcUrl(stored); } } catch { console.warn('[configPersistence] Unable to access localStorage'); @@ -84,8 +91,9 @@ export function peekStoredRpcUrl(): string | null { export function storeRpcUrl(url: string): void { try { if (url && url.trim().length > 0) { - localStorage.setItem(RPC_URL_STORAGE_KEY, url.trim()); - console.debug('[configPersistence] Stored RPC URL:', { url: url.trim() }); + const normalized = normalizeRpcUrl(url); + localStorage.setItem(RPC_URL_STORAGE_KEY, normalized); + log('Stored RPC URL: %s', redactRpcUrlForLog(normalized)); } else { // Allow clearing the stored URL to reset to default localStorage.removeItem(RPC_URL_STORAGE_KEY); @@ -174,12 +182,41 @@ export function isAllowedCloudRpcUrl(url: string): boolean { /** * Normalize an RPC URL by trimming whitespace and trailing slashes. + * When the user provides a core base URL with no path, treat it as the + * JSON-RPC endpoint base and append `/rpc`. * * @param url - The URL to normalize * @returns The normalized URL */ export function normalizeRpcUrl(url: string): string { - return url.trim().replace(/\/+$/, ''); + const trimmed = url.trim(); + try { + // Parse before trimming path slashes so query/hash values such as ?next=/ + // or #/ stay byte-for-byte intact. + new URL(trimmed); + + const suffixStart = firstUrlSuffixIndex(trimmed); + const base = suffixStart === -1 ? trimmed : trimmed.slice(0, suffixStart); + const suffix = suffixStart === -1 ? '' : trimmed.slice(suffixStart); + const pathStart = base.indexOf('/', base.indexOf('://') + 3); + const origin = pathStart === -1 ? base : base.slice(0, pathStart); + const path = pathStart === -1 ? '' : base.slice(pathStart); + const pathWithoutTrailingSlashes = path.replace(/\/+$/, ''); + const normalizedPath = pathWithoutTrailingSlashes || '/rpc'; + + return `${origin}${normalizedPath}${suffix}`; + } catch { + // Validation reports malformed URLs. Keep this helper side-effect free. + } + return trimmed.replace(/\/+$/, ''); +} + +function firstUrlSuffixIndex(url: string): number { + const searchIndex = url.indexOf('?'); + const hashIndex = url.indexOf('#'); + if (searchIndex === -1) return hashIndex; + if (hashIndex === -1) return searchIndex; + return Math.min(searchIndex, hashIndex); } /** diff --git a/app/src/utils/redactRpcUrlForLog.ts b/app/src/utils/redactRpcUrlForLog.ts new file mode 100644 index 000000000..53da709ab --- /dev/null +++ b/app/src/utils/redactRpcUrlForLog.ts @@ -0,0 +1,12 @@ +export function redactRpcUrlForLog(url: string): string { + try { + const parsed = new URL(url); + parsed.username = ''; + parsed.password = ''; + parsed.search = ''; + parsed.hash = ''; + return parsed.origin + parsed.pathname; + } catch { + return '[invalid-url]'; + } +}