diff --git a/app/src/store/index.ts b/app/src/store/index.ts index 9a279f2ec..698e9ecb3 100644 --- a/app/src/store/index.ts +++ b/app/src/store/index.ts @@ -11,7 +11,7 @@ import { REHYDRATE, } from 'redux-persist'; -import { IS_DEV } from '../utils/config'; +import { E2E_RESTART_APP_AS_RELOAD, IS_DEV } from '../utils/config'; import accountsReducer from './accountsSlice'; import agentProfileReducer from './agentProfileSlice'; import channelConnectionsReducer from './channelConnectionsSlice'; @@ -183,10 +183,12 @@ export const store = configureStore({ export const persistor = persistStore(store); // Expose the store on `window` so WDIO E2E specs can read Redux state directly -// to assert backing-state changes (see app/test/e2e/specs/*.spec.ts). The store -// holds no secrets that aren't already in the renderer's memory; this only -// surfaces the existing handle under a stable, namespaced key. -if (typeof window !== 'undefined') { +// to assert backing-state changes (see app/test/e2e/specs/*.spec.ts). Gated on +// the E2E build flag (`VITE_OPENHUMAN_E2E_RESTART_APP_AS_RELOAD`, baked by +// `app/scripts/e2e-build.sh`) so shipped production bundles do NOT expose the +// store handle — denying a same-origin attacker (compromised CDN, supply-chain +// asset, XSS) a one-call read/mutate path into full Redux state. +if (typeof window !== 'undefined' && (IS_DEV || E2E_RESTART_APP_AS_RELOAD)) { (window as unknown as { __OPENHUMAN_STORE__?: typeof store }).__OPENHUMAN_STORE__ = store; } diff --git a/app/src/utils/__tests__/loopbackOauthListener.test.ts b/app/src/utils/__tests__/loopbackOauthListener.test.ts index 449351d4f..07e326c91 100644 --- a/app/src/utils/__tests__/loopbackOauthListener.test.ts +++ b/app/src/utils/__tests__/loopbackOauthListener.test.ts @@ -109,7 +109,8 @@ describe('startLoopbackOauthListener', () => { try { mockInvoke .mockResolvedValueOnce({ redirectUri: 'http://127.0.0.1:53824/auth', state: 's' }) - .mockResolvedValueOnce(undefined); + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce({ redirectUri: 'http://127.0.0.1:53824/auth', state: 's2' }); const unlisten = vi.fn(); mockListen.mockResolvedValue(unlisten); @@ -121,9 +122,82 @@ describe('startLoopbackOauthListener', () => { await expect(callbackPromise).rejects.toThrow('Loopback OAuth listener timed out'); expect(unlisten).toHaveBeenCalledTimes(1); - // Drain the queued microtask that calls stop(). + // Drain the queued microtask that calls stop() await Promise.resolve(); expect(mockInvoke).toHaveBeenNthCalledWith(2, 'stop_loopback_oauth_listener'); + + // Ensure timeout cleanup removed activeUnlisten (starting a new listener + // should not invoke the previous unlisten again). + await startLoopbackOauthListener(); + expect(unlisten).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + + test('tears down late listen registration when timeout fires before listen() resolves', async () => { + vi.useFakeTimers(); + try { + mockInvoke + .mockResolvedValueOnce({ redirectUri: 'http://127.0.0.1:53824/auth', state: 's' }) + .mockResolvedValueOnce(undefined) + .mockResolvedValueOnce({ redirectUri: 'http://127.0.0.1:53824/auth', state: 's2' }); + + let resolveListen: ((fn: () => void) => void) | null = null; + const lateUnlisten = vi.fn(); + mockListen.mockImplementationOnce( + () => + new Promise(resolve => { + resolveListen = resolve; + }) + ); + + const handle = await startLoopbackOauthListener({ timeoutSecs: 1 }); + const callbackPromise = handle!.awaitCallback(); + vi.advanceTimersByTime(1000); + + await expect(callbackPromise).rejects.toThrow('Loopback OAuth listener timed out'); + await Promise.resolve(); + expect(mockInvoke).toHaveBeenNthCalledWith(2, 'stop_loopback_oauth_listener'); + + // listen() resolves after timeout: the returned unlisten must be called + // immediately and must not become the active global handle. + resolveListen!(lateUnlisten); + await Promise.resolve(); + expect(lateUnlisten).toHaveBeenCalledTimes(1); + + await startLoopbackOauthListener(); + expect(lateUnlisten).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + + test('ignores callback events that arrive after timeout', async () => { + vi.useFakeTimers(); + try { + mockInvoke + .mockResolvedValueOnce({ redirectUri: 'http://127.0.0.1:53824/auth', state: 's' }) + .mockResolvedValueOnce(undefined); + const unlisten = vi.fn(); + let registered: ((event: { payload: { url: string } }) => void) | null = null; + mockListen.mockImplementation((_event, handler) => { + registered = handler; + return Promise.resolve(unlisten); + }); + + const handle = await startLoopbackOauthListener({ timeoutSecs: 1 }); + const callbackPromise = handle!.awaitCallback(); + await Promise.resolve(); + vi.advanceTimersByTime(1000); + + await expect(callbackPromise).rejects.toThrow('Loopback OAuth listener timed out'); + expect(unlisten).toHaveBeenCalledTimes(1); + + // Late callback should be ignored by the timedOut guard. + registered!({ payload: { url: 'http://127.0.0.1:53824/auth?token=late&state=s' } }); + await Promise.resolve(); + expect(unlisten).toHaveBeenCalledTimes(1); } finally { vi.useRealTimers(); } @@ -133,16 +207,25 @@ describe('startLoopbackOauthListener', () => { describe('E2E build hook', () => { // Top-level side effect in loopbackOauthListener.ts: when the // VITE_OPENHUMAN_E2E_RESTART_APP_AS_RELOAD flag is set to 'true' at - // build time, the module exposes startLoopbackOauthListener on + // build time (surfaced via the `E2E_RESTART_APP_AS_RELOAD` constant in + // utils/config.ts per the CLAUDE.md "no direct import.meta.env" rule), + // the module exposes startLoopbackOauthListener on // window.__startLoopbackOauthListener so E2E spec helpers can drive // the real loopback flow. Exercise both branches so the conditional // assignment is covered. + // + // We mock `../config` directly rather than `vi.stubEnv` + `vi.resetModules` + // because the gate is now a derived constant, not a live read of + // `import.meta.env`. Stubbing the env after the module graph has already + // been loaded does not flip the already-evaluated constant unless EVERY + // transitive importer is also reset, which is brittle. Mocking the module + // export is direct and resilient to refactors of how the flag is derived. type WithE2eHook = Window & { __startLoopbackOauthListener?: typeof startLoopbackOauthListener }; test('exposes __startLoopbackOauthListener on window when the E2E build flag is set', async () => { vi.resetModules(); - vi.stubEnv('VITE_OPENHUMAN_E2E_RESTART_APP_AS_RELOAD', 'true'); + vi.doMock('../config', () => ({ E2E_RESTART_APP_AS_RELOAD: true })); delete (window as WithE2eHook).__startLoopbackOauthListener; try { const mod = await import('../loopbackOauthListener'); @@ -150,20 +233,20 @@ describe('E2E build hook', () => { mod.startLoopbackOauthListener ); } finally { - vi.unstubAllEnvs(); + vi.doUnmock('../config'); delete (window as WithE2eHook).__startLoopbackOauthListener; } }); test('does NOT expose the hook when the E2E build flag is absent', async () => { vi.resetModules(); - vi.stubEnv('VITE_OPENHUMAN_E2E_RESTART_APP_AS_RELOAD', ''); + vi.doMock('../config', () => ({ E2E_RESTART_APP_AS_RELOAD: false })); delete (window as WithE2eHook).__startLoopbackOauthListener; try { await import('../loopbackOauthListener'); expect((window as WithE2eHook).__startLoopbackOauthListener).toBeUndefined(); } finally { - vi.unstubAllEnvs(); + vi.doUnmock('../config'); delete (window as WithE2eHook).__startLoopbackOauthListener; } }); diff --git a/app/src/utils/loopbackOauthListener.ts b/app/src/utils/loopbackOauthListener.ts index 0693d2460..715d73dd1 100644 --- a/app/src/utils/loopbackOauthListener.ts +++ b/app/src/utils/loopbackOauthListener.ts @@ -1,6 +1,7 @@ import { invoke } from '@tauri-apps/api/core'; import { listen, type UnlistenFn } from '@tauri-apps/api/event'; +import { E2E_RESTART_APP_AS_RELOAD } from './config'; import { isTauri } from './tauriCommands/common'; /** @@ -97,14 +98,25 @@ export const startLoopbackOauthListener = async ( const awaitCallback = (): Promise => new Promise((resolve, reject) => { + // `timedOut` closes the race where `setTimeout` fires *before* the async + // `listen()` registration resolves: previously the just-registered + // unlisten handle was stored in module-global `activeUnlisten` after the + // promise had already rejected, leaving the listener armed until the + // next `startLoopbackOauthListener` call cleaned it up. + let timedOut = false; let unlisten: UnlistenFn | null = null; const timer = window.setTimeout(() => { - if (unlisten) unlisten(); + timedOut = true; + if (unlisten) { + unlisten(); + if (activeUnlisten === unlisten) activeUnlisten = null; + } void stop(); reject(new Error('Loopback OAuth listener timed out')); }, timeoutSecs * 1000); listen(CALLBACK_EVENT, event => { + if (timedOut) return; window.clearTimeout(timer); if (unlisten) { unlisten(); @@ -113,10 +125,18 @@ export const startLoopbackOauthListener = async ( resolve(event.payload.url); }) .then(fn => { + if (timedOut) { + // Timer already rejected the promise — tear down the + // just-registered handle so it does not leak into + // `activeUnlisten` and stay armed past the timeout. + fn(); + return; + } unlisten = fn; activeUnlisten = fn; }) .catch(err => { + if (timedOut) return; window.clearTimeout(timer); reject(err); }); @@ -135,10 +155,7 @@ const appendState = (uri: string, state: string): string => { // emit + frontend listener) without scripting the OAuth button UI itself. // Gated on the E2E-mode VITE flag baked in by app/scripts/e2e-build.sh so it // never leaks into release bundles. -if ( - typeof window !== 'undefined' && - import.meta.env.VITE_OPENHUMAN_E2E_RESTART_APP_AS_RELOAD === 'true' -) { +if (typeof window !== 'undefined' && E2E_RESTART_APP_AS_RELOAD) { type WithE2eHook = Window & { __startLoopbackOauthListener?: typeof startLoopbackOauthListener }; (window as WithE2eHook).__startLoopbackOauthListener = startLoopbackOauthListener; }