mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(e2e/oauth): gate Redux store exposure and fix loopback listener timeout race (#2670)
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
});
|
||||
|
||||
@@ -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<string> =>
|
||||
new Promise<string>((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<CallbackPayload>(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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user