From 76f11e0385613407a0286c678aa1f730e099669d Mon Sep 17 00:00:00 2001 From: abhishekp-bruno Date: Wed, 24 Jun 2026 16:53:08 +0530 Subject: [PATCH 1/2] FIXED proxy not refreshing from terminal RESOLVED pr conversations ADD test cases and os specific test casess REVERTE comment removed for windows refetch refactor: remove redundant afterEach cleanup in shell-env-proxy-refresh tests RENAME the test file and add more test for wider coverage among the test cases test: add integration tests for refreshShellEnvProxyVars delete: remove outdated system proxy integration test file fix: enhance proxy variable restoration on shell-env subprocess failure fix: implement timeout for shellEnv to prevent hanging on misconfigured shells feat: add refreshShellEnvProxyVars function to shell-env-state.ts and update shell environment initialization logic refactor: simplify shell environment refresh and update system proxy integration refactor: implement withTimeout utility for shell environment operations to prevent indefinite hangs --- .../bruno-electron/src/store/system-proxy.js | 9 +- packages/bruno-requests/src/index.ts | 2 +- .../system-proxy/refresh-integration.spec.ts | 282 ++++++++++++++++++ .../src/utils/shell-env-proxy-refresh.spec.ts | 201 +++++++++++++ .../bruno-requests/src/utils/shell-env.ts | 91 +++++- 5 files changed, 579 insertions(+), 6 deletions(-) create mode 100644 packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts create mode 100644 packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts diff --git a/packages/bruno-electron/src/store/system-proxy.js b/packages/bruno-electron/src/store/system-proxy.js index 3a40daa59..af071afa7 100644 --- a/packages/bruno-electron/src/store/system-proxy.js +++ b/packages/bruno-electron/src/store/system-proxy.js @@ -1,10 +1,13 @@ -const { getSystemProxy } = require('@usebruno/requests'); +const { getSystemProxy, refreshShellEnvProxyVars } = require('@usebruno/requests'); let cachedSystemProxy; let systemProxyPromise; -const loadSystemProxy = async () => { +const loadSystemProxy = async ({ refreshShellEnv = false } = {}) => { try { + if (refreshShellEnv) { + await refreshShellEnvProxyVars(); + } cachedSystemProxy = await getSystemProxy(); } catch (error) { console.error('Failed to initialize system proxy:', error); @@ -21,7 +24,7 @@ const loadSystemProxy = async () => { const fetchSystemProxy = ({ refresh = false } = {}) => { if (refresh || !systemProxyPromise) { - systemProxyPromise = loadSystemProxy(); + systemProxyPromise = loadSystemProxy({ refreshShellEnv: refresh }); } return systemProxyPromise; }; diff --git a/packages/bruno-requests/src/index.ts b/packages/bruno-requests/src/index.ts index e3598e93a..de638f557 100644 --- a/packages/bruno-requests/src/index.ts +++ b/packages/bruno-requests/src/index.ts @@ -8,7 +8,7 @@ export { transformProxyConfig } from './utils/proxy-util'; export { default as createVaultClient, VaultError } from './utils/node-vault'; export type { VaultClient, VaultConfig, VaultRequestOptions } from './utils/node-vault'; export { getHttpHttpsAgents, resolveAgentsFromPac, PatchedHttpsProxyAgent } from './utils/http-https-agents'; -export { initializeShellEnv } from './utils/shell-env'; +export { initializeShellEnv, refreshShellEnvProxyVars } from './utils/shell-env'; export { getOrCreateHttpsAgent, getOrCreateHttpAgent, clearAgentCache, getAgentCacheSize } from './utils/agent-cache'; export { getPacResolver, clearPacCache } from './utils/pac-resolver'; export type { PacWrapper, GetPacResolverParams } from './utils/pac-resolver'; diff --git a/packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts b/packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts new file mode 100644 index 000000000..961a0a870 --- /dev/null +++ b/packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts @@ -0,0 +1,282 @@ +/** + * Composition tests: refreshShellEnvProxyVars + getSystemProxy wired end-to-end. + * + * The individual contracts are covered by: + * - ./get-system-proxy.spec.ts (cross-platform detection + merge) + * - ../../utils/shell-env-proxy-refresh.spec.ts (refresh delete/re-populate + Windows early-return) + * + * This file exercises only the composition: after a refresh, does the resulting proxy config + * from getSystemProxy reflect the intended end-to-end behavior? + */ +export { }; // Mark this file as a module so top-level declarations don't collide with sibling specs. + +jest.mock('node:child_process', () => ({ execFile: jest.fn() })); +jest.mock('node:util', () => ({ promisify: jest.fn((fn) => fn) })); +jest.mock('node:fs', () => ({ existsSync: jest.fn() })); +jest.mock('node:fs/promises', () => ({ readFile: jest.fn(), readdir: jest.fn() })); + +// Config the mocked login shell would export. Reassigned per test. +// Prefix required by jest.mock's out-of-scope-variable guard (mock*/MOCK* allowed). +let mockShellEnvResult: Record = {}; + +jest.mock('shell-env', () => ({ + shellEnv: () => Promise.resolve(mockShellEnvResult) +})); + +const PROXY_ENV_KEYS = [ + 'http_proxy', + 'HTTP_PROXY', + 'https_proxy', + 'HTTPS_PROXY', + 'no_proxy', + 'NO_PROXY', + 'all_proxy', + 'ALL_PROXY' +]; + +interface FreshModules { + execFile: jest.MockedFunction; + existsSync: jest.MockedFunction; + refresh: () => Promise>; + getSystemProxy: () => Promise; +} + +/** + * Re-require the proxy modules with a mocked node:os platform. SystemProxyResolver reads + * os.platform() at construction, so this has to happen before requiring ./index. + * refreshShellEnvProxyVars branches on process.platform (not os.platform), so Windows tests + * must additionally override process.platform via Object.defineProperty. + */ +const loadForPlatform = (platform: string): FreshModules => { + jest.resetModules(); + jest.doMock('node:os', () => ({ platform: () => platform })); + + const { execFile } = require('node:child_process'); + const { existsSync } = require('node:fs'); + const { refreshShellEnvProxyVars } = require('../../utils/shell-env'); + const { getSystemProxy } = require('./index'); + + return { execFile, existsSync, refresh: refreshShellEnvProxyVars, getSystemProxy }; +}; + +/** + * Argument-aware execFile router. Matches on a substring of the joined command; first hit wins. + * Keeps tests decoupled from the resolver's internal call sequencing. + */ +const routeExecFile = ( + execFile: jest.MockedFunction, + routes: Array<[string, { stdout: string; stderr?: string } | Error]> +) => { + execFile.mockImplementation((file: string, args: string[]) => { + const command = [file, ...args].join(' '); + for (const [needle, response] of routes) { + if (command.includes(needle)) { + return response instanceof Error + ? Promise.reject(response) + : Promise.resolve({ stderr: '', ...response }); + } + } + return Promise.resolve({ stdout: '', stderr: '' }); + }); +}; + +let originalEnv: typeof process.env; + +beforeEach(() => { + originalEnv = { ...process.env }; + mockShellEnvResult = {}; + for (const key of PROXY_ENV_KEYS) { + delete process.env[key]; + } +}); + +afterEach(() => { + process.env = originalEnv; +}); + +describe('refresh + getSystemProxy composition', () => { + it('macOS: stale env purged on refresh, system layer wins alone', async () => { + // A proxy var lingering on process.env from a previous session — the user has since + // removed the export from .zshrc, so the login shell no longer emits it. + process.env.http_proxy = 'http://stale.usebruno.com:8080'; + mockShellEnvResult = {}; + + const { execFile, refresh, getSystemProxy } = loadForPlatform('darwin'); + execFile.mockResolvedValueOnce({ + stdout: ` { + HTTPEnable : 1 + HTTPPort : 8080 + HTTPProxy : sys-proxy.usebruno.com + }`, + stderr: '' + }); + + await refresh(); + const result = await getSystemProxy(); + + // Stale env value is gone, so system detection wins with no "+ environment" suffix. + expect(result.http_proxy).toBe('http://sys-proxy.usebruno.com:8080'); + expect(result.source).toBe('macos-system'); + }); + + it('macOS: profile export beats system detection after refresh', async () => { + // .zshrc now exports HTTP_PROXY (either newly added or edited). + mockShellEnvResult = { http_proxy: 'http://profile.usebruno.com:9090' }; + + const { execFile, refresh, getSystemProxy } = loadForPlatform('darwin'); + execFile.mockResolvedValueOnce({ + stdout: ` { + HTTPEnable : 1 + HTTPPort : 8080 + HTTPProxy : sys-proxy.usebruno.com + }`, + stderr: '' + }); + + await refresh(); + const result = await getSystemProxy(); + + expect(result.http_proxy).toBe('http://profile.usebruno.com:9090'); + expect(result.source).toBe('macos-system + environment'); + }); + + it('Linux: profile export merges with gsettings detection — env wins http, system supplies https + bypass', async () => { + // .zshrc exports an http proxy; gsettings reports a DIFFERENT http proxy plus https + bypass + // the profile does not set. After refresh, the env layer should win for http and combine + // with system detection for the rest. + mockShellEnvResult = { http_proxy: 'http://profile.usebruno.com:9090' }; + + const { execFile, existsSync, refresh, getSystemProxy } = loadForPlatform('linux'); + existsSync.mockReturnValue(false); // isolate the command-based (gsettings) path + routeExecFile(execFile, [ + ['org.gnome.system.proxy mode', { stdout: '\'manual\'' }], + ['org.gnome.system.proxy.http host', { stdout: '\'sys-proxy.usebruno.com\'' }], + ['org.gnome.system.proxy.http port', { stdout: '8080' }], + ['org.gnome.system.proxy.https host', { stdout: '\'secure-proxy.usebruno.com\'' }], + ['org.gnome.system.proxy.https port', { stdout: '8443' }], + ['org.gnome.system.proxy ignore-hosts', { stdout: '[\'localhost\', \'127.0.0.1\']' }] + ]); + + await refresh(); + const result = await getSystemProxy(); + + expect(result.http_proxy).toBe('http://profile.usebruno.com:9090'); // env wins + expect(result.https_proxy).toBe('http://secure-proxy.usebruno.com:8443'); // from system + expect(result.no_proxy).toBe('localhost,127.0.0.1'); // from system + expect(result.source).toBe('linux-system + environment'); + }); + + it('Linux: stale env purged on refresh, gsettings detection wins alone', async () => { + // A proxy var lingering on process.env from a previous session — the user has since + // removed the export from their profile, so the login shell no longer emits it. + process.env.http_proxy = 'http://stale.usebruno.com:1234'; + mockShellEnvResult = {}; + + const { execFile, existsSync, refresh, getSystemProxy } = loadForPlatform('linux'); + existsSync.mockReturnValue(false); + routeExecFile(execFile, [ + ['org.gnome.system.proxy mode', { stdout: '\'manual\'' }], + ['org.gnome.system.proxy.http host', { stdout: '\'sys-proxy.usebruno.com\'' }], + ['org.gnome.system.proxy.http port', { stdout: '8080' }] + ]); + + await refresh(); + const result = await getSystemProxy(); + + // Stale env value is gone, so system detection wins with no "+ environment" suffix. + expect(result.http_proxy).toBe('http://sys-proxy.usebruno.com:8080'); + expect(result.source).toBe('linux-system'); + }); + + it('Linux (KDE): profile export merges with kreadconfig5 detection — env wins http, system supplies https + bypass', async () => { + // gsettings is unavailable, so detection falls through to KDE (kreadconfig5). The profile + // exports an http proxy; KDE reports a DIFFERENT http proxy plus https + bypass. + mockShellEnvResult = { http_proxy: 'http://profile.usebruno.com:9090' }; + + const { execFile, existsSync, refresh, getSystemProxy } = loadForPlatform('linux'); + existsSync.mockReturnValue(false); // isolate the command-based path + execFile.mockImplementation((file: string, args: string[]) => { + const fullCommand = [file, ...args].join(' '); + if (fullCommand.includes('gsettings')) { + return Promise.reject(new Error('gsettings not available')); + } + if (fullCommand.includes('kreadconfig5')) { + // Lowercase args before matching so we are agnostic to the key casing the resolver uses. + const lowerArgs = args.join(' ').toLowerCase(); + if (lowerArgs.includes('proxytype')) return Promise.resolve({ stdout: '1', stderr: '' }); + if (lowerArgs.includes('httpproxy')) return Promise.resolve({ stdout: 'http://sys-proxy.usebruno.com:8080', stderr: '' }); + if (lowerArgs.includes('httpsproxy')) return Promise.resolve({ stdout: 'http://secure-proxy.usebruno.com:8443', stderr: '' }); + if (lowerArgs.includes('noproxyfor')) return Promise.resolve({ stdout: 'localhost,127.0.0.1', stderr: '' }); + } + return Promise.resolve({ stdout: '', stderr: '' }); + }); + + await refresh(); + const result = await getSystemProxy(); + + expect(result.http_proxy).toBe('http://profile.usebruno.com:9090'); // env wins + expect(result.https_proxy).toBe('http://secure-proxy.usebruno.com:8443'); // from system (KDE) + expect(result.no_proxy).toBe('localhost,127.0.0.1'); // from system (KDE) + expect(result.source).toBe('linux-system + environment'); + }); + + it('Windows: shell profile export is ignored, registry detection wins', async () => { + // fetchShellEnv branches on process.platform (not the mocked node:os), so we force it here. + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + // A shell profile that WOULD export a proxy — on Windows the refresh is a no-op, so it + // must not surface. Registry detection alone should decide the result. + mockShellEnvResult = { http_proxy: 'http://should-be-ignored.usebruno.com:9090' }; + + const { execFile, refresh, getSystemProxy } = loadForPlatform('win32'); + routeExecFile(execFile, [ + ['Internet Settings', { + stdout: ` +HKEY_CURRENT_USER\\Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings + ProxyEnable REG_DWORD 0x1 + ProxyServer REG_SZ sys-proxy.usebruno.com:8080 +` + }] + ]); + + await refresh(); + const result = await getSystemProxy(); + + // Registry value surfaces; the ignored shell value contributes nothing, so no "+ environment". + expect(result.http_proxy).toBe('http://sys-proxy.usebruno.com:8080'); + expect(result.source).toBe('windows-system'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); + + it('Windows: launcher-set env survives refresh and surfaces via getSystemProxy', async () => { + // fetchShellEnv branches on process.platform (not the mocked node:os), so we force it here. + const originalPlatform = process.platform; + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + + try { + // Set by a launcher script / parent shell / registry propagation — anything that reaches + // process.env before Bruno starts. Without the Windows early-return in + // refreshShellEnvProxyVars, this value would be stripped and never restored. + process.env.HTTP_PROXY = 'http://launcher.usebruno.com:8080'; + // A shell profile that WOULD export a different proxy — must not surface on Windows. + mockShellEnvResult = { http_proxy: 'http://should-be-ignored.usebruno.com:9090' }; + + const { execFile, refresh, getSystemProxy } = loadForPlatform('win32'); + // System registry has no proxy, so any surviving proxy in the final result must come + // from the env layer we preserved. + execFile.mockImplementation(() => Promise.resolve({ stdout: '', stderr: '' })); + + await refresh(); + const result = await getSystemProxy(); + + expect(result.http_proxy).toBe('http://launcher.usebruno.com:8080'); + expect(result.source).toContain('environment'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + } + }); +}); diff --git a/packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts b/packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts new file mode 100644 index 000000000..c8a43370c --- /dev/null +++ b/packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts @@ -0,0 +1,201 @@ +/** + * Regression tests for refreshShellEnvProxyVars. + * + * shell-env spawns a login shell (`$SHELL -i -c 'export -p'`) that INHERITS the parent + * process.env. So any proxy var still set on process.env when the shell is spawned would be + * inherited and re-exported — meaning a proxy the user removed from their profile would never + * go away. refreshShellEnvProxyVars guards against this by deleting all proxy vars BEFORE + * invoking the shell on POSIX platforms, and by early-returning on Windows (where fetchShellEnv + * is a no-op and the delete would strip values with nothing to restore them). + * + * The mock for shell-env models login-shell parent-env inheritance and captures a snapshot of + * process.env at invocation time, so tests can assert delete-before-fetch ordering directly. + */ +import { refreshShellEnvProxyVars, PROXY_ENV_KEYS } from './shell-env'; + +let mockShellEnvResult: Record = {}; + +// When set, the mocked shellEnv() rejects with this error. Lets us verify graceful degradation. +let mockShellEnvThrows: Error | null = null; + +// Snapshot of the proxy vars present in process.env when shellEnv() was invoked. Lets tests +// assert that refreshShellEnvProxyVars deleted stale vars BEFORE spawning the shell. +// Prefix required by jest.mock's out-of-scope-variable guard (mock*/MOCK* allowed). +let mockProxyVarsSeenByShell: Record = {}; + +jest.mock('shell-env', () => ({ + shellEnv: () => { + const PROXY_KEYS = [ + 'http_proxy', + 'HTTP_PROXY', + 'https_proxy', + 'HTTPS_PROXY', + 'no_proxy', + 'NO_PROXY', + 'all_proxy', + 'ALL_PROXY' + ]; + + // Capture what the subprocess would inherit from Bruno's process.env at spawn time. + mockProxyVarsSeenByShell = {}; + for (const key of PROXY_KEYS) { + if (process.env[key] !== undefined) { + mockProxyVarsSeenByShell[key] = process.env[key] as string; + } + } + + if (mockShellEnvThrows) { + return Promise.reject(mockShellEnvThrows); + } + + // Model a login shell: inherits from parent, then applies whatever the profile exports. + return Promise.resolve({ ...mockProxyVarsSeenByShell, ...mockShellEnvResult }); + } +})); + +describe('refreshShellEnvProxyVars — shell inheritance (POSIX)', () => { + beforeEach(() => { + mockProxyVarsSeenByShell = {}; + mockShellEnvResult = {}; + mockShellEnvThrows = null; + for (const key of PROXY_ENV_KEYS) { + delete process.env[key]; + } + }); + + test('updates proxy env vars from shell config', async () => { + process.env.http_proxy = 'http://old-proxy:8080'; + mockShellEnvResult = { + http_proxy: 'http://new-proxy:8080', + https_proxy: 'http://new-proxy:8443' + }; + + await refreshShellEnvProxyVars(); + + expect(process.env.http_proxy).toBe('http://new-proxy:8080'); + expect(process.env.https_proxy).toBe('http://new-proxy:8443'); + }); + + test('removes proxy env vars missing from shell config', async () => { + // Stale vars lingering from a previous session. + process.env.http_proxy = 'http://old-proxy:8080'; + process.env.no_proxy = 'localhost'; + // User removed the exports from their profile -> shell config no longer has them. + mockShellEnvResult = {}; + + await refreshShellEnvProxyVars(); + + expect(process.env.http_proxy).toBeUndefined(); + expect(process.env.no_proxy).toBeUndefined(); + }); + + test('treats an empty-string proxy value as removal', async () => { + process.env.http_proxy = 'http://old-proxy:8080'; + mockShellEnvResult = { http_proxy: '' }; + + await refreshShellEnvProxyVars(); + + expect(process.env.http_proxy).toBeUndefined(); + }); + + test('leaves non-proxy env vars untouched', async () => { + process.env.NON_PROXY_VAR = 'keep-me'; + mockShellEnvResult = { http_proxy: 'http://new-proxy:8080' }; + + await refreshShellEnvProxyVars(); + + expect(process.env.http_proxy).toBe('http://new-proxy:8080'); + expect(process.env.NON_PROXY_VAR).toBe('keep-me'); + + delete process.env.NON_PROXY_VAR; + }); + + test('handles uppercase proxy variants (HTTP_PROXY, HTTPS_PROXY, NO_PROXY, ALL_PROXY)', async () => { + process.env.HTTP_PROXY = 'http://old-upper:8080'; + process.env.ALL_PROXY = 'http://old-all:1080'; + mockShellEnvResult = { + HTTP_PROXY: 'http://new-upper:8080', + HTTPS_PROXY: 'http://new-upper:8443', + NO_PROXY: 'localhost,127.0.0.1', + ALL_PROXY: 'http://new-all:1080' + }; + + await refreshShellEnvProxyVars(); + + expect(process.env.HTTP_PROXY).toBe('http://new-upper:8080'); + expect(process.env.HTTPS_PROXY).toBe('http://new-upper:8443'); + expect(process.env.NO_PROXY).toBe('localhost,127.0.0.1'); + expect(process.env.ALL_PROXY).toBe('http://new-all:1080'); + }); + + test('restores prior proxy vars when the shell-env subprocess fails', async () => { + process.env.http_proxy = 'http://existing:8080'; + mockShellEnvThrows = new Error('shell subprocess failed'); + + // On subprocess failure we return {} but restore the snapshot taken before the + // delete, so the user is not left silently unproxied. + await expect(refreshShellEnvProxyVars()).resolves.toEqual({}); + expect(process.env.http_proxy).toBe('http://existing:8080'); + }); + + test('returns the raw shell env dict, including non-proxy keys', async () => { + mockShellEnvResult = { + http_proxy: 'http://p:8080', + PATH: '/foo:/bar', + EDITOR: 'vim' + }; + + const result = await refreshShellEnvProxyVars(); + + expect(result).toEqual({ + http_proxy: 'http://p:8080', + PATH: '/foo:/bar', + EDITOR: 'vim' + }); + }); +}); + +describe('refreshShellEnvProxyVars — Windows', () => { + const originalPlatform = process.platform; + + beforeAll(() => { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + }); + + afterAll(() => { + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + }); + + beforeEach(() => { + mockProxyVarsSeenByShell = {}; + mockShellEnvResult = {}; + mockShellEnvThrows = null; + for (const key of PROXY_ENV_KEYS) { + delete process.env[key]; + } + }); + + test('leaves process.env proxy vars untouched (registry / launcher / parent-shell values survive)', async () => { + // A proxy in process.env from any Windows source: registry propagation, launcher script, + // PowerShell $env, Git Bash export, etc. All look the same to us. + process.env.HTTP_PROXY = 'http://launcher-proxy:8080'; + process.env.https_proxy = 'http://another:9090'; + + await refreshShellEnvProxyVars(); + + expect(process.env.HTTP_PROXY).toBe('http://launcher-proxy:8080'); + expect(process.env.https_proxy).toBe('http://another:9090'); + }); + + test('does not invoke the shell subprocess on Windows', async () => { + // A shell profile that would export a proxy if it were consulted — it must not be. + mockShellEnvResult = { http_proxy: 'http://should-not-appear:8080' }; + + await refreshShellEnvProxyVars(); + + // mockProxyVarsSeenByShell is only populated when the shell-env mock is actually called. + // Empty means we never invoked shellEnv() — the Windows early-return short-circuited. + expect(mockProxyVarsSeenByShell).toEqual({}); + expect(process.env.http_proxy).toBeUndefined(); + }); +}); diff --git a/packages/bruno-requests/src/utils/shell-env.ts b/packages/bruno-requests/src/utils/shell-env.ts index 49c8ac231..29d9bc7c9 100644 --- a/packages/bruno-requests/src/utils/shell-env.ts +++ b/packages/bruno-requests/src/utils/shell-env.ts @@ -6,8 +6,40 @@ import path from 'path'; -const fetchShellEnv = async (): Promise> => { +export const PROXY_ENV_KEYS = [ + 'http_proxy', + 'HTTP_PROXY', + 'https_proxy', + 'HTTPS_PROXY', + 'no_proxy', + 'NO_PROXY', + 'all_proxy', + 'ALL_PROXY' +] as const; + +const TIMEOUT = Symbol('shell-env-timeout'); +const TIMEOUT_MS = 60_000; + +/** + * Races a promise against a timeout. Resolves to the TIMEOUT symbol if the + * timeout wins, so a misconfigured shell can't hang the caller indefinitely. + */ +const withTimeout = async (promise: Promise, timeoutMs: number): Promise => { + let timeoutId: ReturnType; + const timeoutPromise = new Promise((resolve) => { + timeoutId = setTimeout(() => resolve(TIMEOUT), timeoutMs); + }); + + try { + return await Promise.race([promise, timeoutPromise]); + } finally { + clearTimeout(timeoutId!); + } +}; + +const fetchShellEnv = async (): Promise | null> => { // Windows handles environment variables differently - skip + // everything related to windows proxy settings is handled by the system proxy resolver i.e getSystemProxy() if (process.platform === 'win32') { return {}; } @@ -18,7 +50,7 @@ const fetchShellEnv = async (): Promise> => { const env = await shellEnv(); return env; } catch (error) { - return {}; + return null; } }; @@ -30,6 +62,11 @@ const fetchShellEnv = async (): Promise> => { */ export const initializeShellEnv = async (): Promise> => { const shellEnvVars = await fetchShellEnv(); + + if (shellEnvVars === null) { + return {}; + } + for (const [key, value] of Object.entries(shellEnvVars)) { if (key === 'PATH' && process.env.PATH) { process.env.PATH = `${value}${path.delimiter}${process.env.PATH}`; @@ -39,3 +76,53 @@ export const initializeShellEnv = async (): Promise> => { } return shellEnvVars; }; + +/** + * Re-syncs proxy-related process.env values from the user's shell configuration. + * Used when refreshing system proxy settings without restarting the app. + * + * @returns The fetched shell environment variables + */ +export const refreshShellEnvProxyVars = async (): Promise> => { + // Windows handles environment variables differently - skip + // everything related to windows proxy settings is handled by the + // system proxy resolver i.e getSystemProxy() + if (process.platform === 'win32') { + return {}; + } + + // Snapshot and clear stale proxy vars first so shell-env does not inherit them + // into the login shell subprocess (removed .zshrc exports would otherwise persist). + const snapshot: Record = {}; + for (const key of PROXY_ENV_KEYS) { + snapshot[key] = process.env[key]; + delete process.env[key]; + } + + const restoreSnapshot = () => { + for (const key of PROXY_ENV_KEYS) { + if (snapshot[key] !== undefined) { + process.env[key] = snapshot[key] as string; + } + } + }; + + // Race the shell-env subprocess against a 60s timeout so a misconfigured shell + // can't hang the refresh indefinitely. + const result = await withTimeout(fetchShellEnv(), TIMEOUT_MS); + + if (result === TIMEOUT || result === null) { + // Timed out — restore prior values rather than leave the user unproxied. + restoreSnapshot(); + return {}; + } + + for (const key of PROXY_ENV_KEYS) { + const value = result[key]; + if (value) { + process.env[key] = value; + } + } + + return result; +}; From f6e31722391aa5b7378ec5bf0f26e6d9b35c93ce Mon Sep 17 00:00:00 2001 From: Sid Date: Wed, 8 Jul 2026 18:12:06 +0530 Subject: [PATCH 2/2] feat(system-proxy): implement refreshShellEnvProxyVars to sync proxy vars from shell config --- .../bruno-electron/src/store/system-proxy.js | 36 ++- .../src/store/tests/system-proxy.spec.js | 92 ++++++ packages/bruno-requests/src/index.ts | 2 +- .../system-proxy/refresh-integration.spec.ts | 282 ------------------ .../src/utils/shell-env-proxy-refresh.spec.ts | 201 ------------- .../bruno-requests/src/utils/shell-env.ts | 83 +----- 6 files changed, 129 insertions(+), 567 deletions(-) create mode 100644 packages/bruno-electron/src/store/tests/system-proxy.spec.js delete mode 100644 packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts delete mode 100644 packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts diff --git a/packages/bruno-electron/src/store/system-proxy.js b/packages/bruno-electron/src/store/system-proxy.js index af071afa7..023c32e30 100644 --- a/packages/bruno-electron/src/store/system-proxy.js +++ b/packages/bruno-electron/src/store/system-proxy.js @@ -1,8 +1,42 @@ -const { getSystemProxy, refreshShellEnvProxyVars } = require('@usebruno/requests'); +const { getSystemProxy, fetchShellEnv } = require('@usebruno/requests'); + +const PROXY_ENV_KEYS = ['http_proxy', 'HTTP_PROXY', 'https_proxy', 'HTTPS_PROXY', 'no_proxy', 'NO_PROXY', 'all_proxy', 'ALL_PROXY']; +const TIMEOUT_MS = 60_000; let cachedSystemProxy; let systemProxyPromise; +// Re-syncs proxy-related process.env values from the user's shell config, without restarting the app. +// ponytail: proxy vars are cleared before spawning the shell (not after fetching) because the child +// process inherits process.env — a removed .zshrc export would otherwise still be echoed back as "current". +const refreshShellEnvProxyVars = async () => { + if (process.platform === 'win32') return {}; + + const snapshot = {}; + for (const key of PROXY_ENV_KEYS) { + snapshot[key] = process.env[key]; + delete process.env[key]; + } + + let timer; + const timeout = new Promise((resolve) => { + timer = setTimeout(() => resolve(null), TIMEOUT_MS); + }); + const result = await Promise.race([fetchShellEnv(), timeout]).finally(() => clearTimeout(timer)); + + if (result === null) { + for (const key of PROXY_ENV_KEYS) { + if (snapshot[key] !== undefined) process.env[key] = snapshot[key]; + } + return {}; + } + + for (const key of PROXY_ENV_KEYS) { + if (result[key]) process.env[key] = result[key]; + } + return result; +}; + const loadSystemProxy = async ({ refreshShellEnv = false } = {}) => { try { if (refreshShellEnv) { diff --git a/packages/bruno-electron/src/store/tests/system-proxy.spec.js b/packages/bruno-electron/src/store/tests/system-proxy.spec.js new file mode 100644 index 000000000..776100f1f --- /dev/null +++ b/packages/bruno-electron/src/store/tests/system-proxy.spec.js @@ -0,0 +1,92 @@ +let mockFetchShellEnv; +let mockGetSystemProxy; + +jest.mock('@usebruno/requests', () => ({ + fetchShellEnv: (...args) => mockFetchShellEnv(...args), + getSystemProxy: (...args) => mockGetSystemProxy(...args) +})); + +const PROXY_ENV_KEYS = ['http_proxy', 'HTTP_PROXY', 'https_proxy', 'HTTPS_PROXY', 'no_proxy', 'NO_PROXY', 'all_proxy', 'ALL_PROXY']; + +describe('system-proxy refresh', () => { + let fetchSystemProxy; + const originalPlatform = process.platform; + + beforeEach(() => { + jest.resetModules(); + Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); + mockFetchShellEnv = jest.fn(() => Promise.resolve({})); + mockGetSystemProxy = jest.fn(() => Promise.resolve({ source: 'environment' })); + for (const key of PROXY_ENV_KEYS) delete process.env[key]; + ({ fetchSystemProxy } = require('../system-proxy')); + }); + + it('updates proxy env vars from shell config', async () => { + process.env.http_proxy = 'http://old-proxy:8080'; + mockFetchShellEnv = jest.fn(() => Promise.resolve({ http_proxy: 'http://new-proxy:8080' })); + ({ fetchSystemProxy } = require('../system-proxy')); + + await fetchSystemProxy({ refresh: true }); + + expect(process.env.http_proxy).toBe('http://new-proxy:8080'); + }); + + it('removes proxy env vars missing from shell config (user removed the export)', async () => { + process.env.http_proxy = 'http://old-proxy:8080'; + process.env.no_proxy = 'localhost'; + mockFetchShellEnv = jest.fn(() => Promise.resolve({})); + ({ fetchSystemProxy } = require('../system-proxy')); + + await fetchSystemProxy({ refresh: true }); + + expect(process.env.http_proxy).toBeUndefined(); + expect(process.env.no_proxy).toBeUndefined(); + }); + + it('restores prior proxy vars when the shell fetch fails', async () => { + // fetchShellEnv never rejects - it resolves null on subprocess failure (see shell-env.ts). + process.env.http_proxy = 'http://existing:8080'; + mockFetchShellEnv = jest.fn(() => Promise.resolve(null)); + ({ fetchSystemProxy } = require('../system-proxy')); + + await fetchSystemProxy({ refresh: true }); + + expect(process.env.http_proxy).toBe('http://existing:8080'); + }); + + it('does not touch process.env or invoke the shell on Windows', async () => { + Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); + process.env.HTTP_PROXY = 'http://launcher-proxy:8080'; + mockFetchShellEnv = jest.fn(() => Promise.resolve({ HTTP_PROXY: 'http://should-not-appear:8080' })); + ({ fetchSystemProxy } = require('../system-proxy')); + + await fetchSystemProxy({ refresh: true }); + + expect(mockFetchShellEnv).not.toHaveBeenCalled(); + expect(process.env.HTTP_PROXY).toBe('http://launcher-proxy:8080'); + }); + + it('does not refresh shell env when refresh is not requested', async () => { + await fetchSystemProxy(); + + expect(mockFetchShellEnv).not.toHaveBeenCalled(); + expect(mockGetSystemProxy).toHaveBeenCalledTimes(1); + }); + + describe('timeout', () => { + beforeEach(() => jest.useFakeTimers()); + afterEach(() => jest.useRealTimers()); + + it('restores prior proxy vars if the shell fetch hangs past 60s', async () => { + process.env.http_proxy = 'http://existing:8080'; + mockFetchShellEnv = jest.fn(() => new Promise(() => {})); // never resolves + ({ fetchSystemProxy } = require('../system-proxy')); + + const p = fetchSystemProxy({ refresh: true }); + jest.advanceTimersByTime(60_000); + await p; + + expect(process.env.http_proxy).toBe('http://existing:8080'); + }); + }); +}); diff --git a/packages/bruno-requests/src/index.ts b/packages/bruno-requests/src/index.ts index de638f557..98d623d4c 100644 --- a/packages/bruno-requests/src/index.ts +++ b/packages/bruno-requests/src/index.ts @@ -8,7 +8,7 @@ export { transformProxyConfig } from './utils/proxy-util'; export { default as createVaultClient, VaultError } from './utils/node-vault'; export type { VaultClient, VaultConfig, VaultRequestOptions } from './utils/node-vault'; export { getHttpHttpsAgents, resolveAgentsFromPac, PatchedHttpsProxyAgent } from './utils/http-https-agents'; -export { initializeShellEnv, refreshShellEnvProxyVars } from './utils/shell-env'; +export { initializeShellEnv, fetchShellEnv } from './utils/shell-env'; export { getOrCreateHttpsAgent, getOrCreateHttpAgent, clearAgentCache, getAgentCacheSize } from './utils/agent-cache'; export { getPacResolver, clearPacCache } from './utils/pac-resolver'; export type { PacWrapper, GetPacResolverParams } from './utils/pac-resolver'; diff --git a/packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts b/packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts deleted file mode 100644 index 961a0a870..000000000 --- a/packages/bruno-requests/src/network/system-proxy/refresh-integration.spec.ts +++ /dev/null @@ -1,282 +0,0 @@ -/** - * Composition tests: refreshShellEnvProxyVars + getSystemProxy wired end-to-end. - * - * The individual contracts are covered by: - * - ./get-system-proxy.spec.ts (cross-platform detection + merge) - * - ../../utils/shell-env-proxy-refresh.spec.ts (refresh delete/re-populate + Windows early-return) - * - * This file exercises only the composition: after a refresh, does the resulting proxy config - * from getSystemProxy reflect the intended end-to-end behavior? - */ -export { }; // Mark this file as a module so top-level declarations don't collide with sibling specs. - -jest.mock('node:child_process', () => ({ execFile: jest.fn() })); -jest.mock('node:util', () => ({ promisify: jest.fn((fn) => fn) })); -jest.mock('node:fs', () => ({ existsSync: jest.fn() })); -jest.mock('node:fs/promises', () => ({ readFile: jest.fn(), readdir: jest.fn() })); - -// Config the mocked login shell would export. Reassigned per test. -// Prefix required by jest.mock's out-of-scope-variable guard (mock*/MOCK* allowed). -let mockShellEnvResult: Record = {}; - -jest.mock('shell-env', () => ({ - shellEnv: () => Promise.resolve(mockShellEnvResult) -})); - -const PROXY_ENV_KEYS = [ - 'http_proxy', - 'HTTP_PROXY', - 'https_proxy', - 'HTTPS_PROXY', - 'no_proxy', - 'NO_PROXY', - 'all_proxy', - 'ALL_PROXY' -]; - -interface FreshModules { - execFile: jest.MockedFunction; - existsSync: jest.MockedFunction; - refresh: () => Promise>; - getSystemProxy: () => Promise; -} - -/** - * Re-require the proxy modules with a mocked node:os platform. SystemProxyResolver reads - * os.platform() at construction, so this has to happen before requiring ./index. - * refreshShellEnvProxyVars branches on process.platform (not os.platform), so Windows tests - * must additionally override process.platform via Object.defineProperty. - */ -const loadForPlatform = (platform: string): FreshModules => { - jest.resetModules(); - jest.doMock('node:os', () => ({ platform: () => platform })); - - const { execFile } = require('node:child_process'); - const { existsSync } = require('node:fs'); - const { refreshShellEnvProxyVars } = require('../../utils/shell-env'); - const { getSystemProxy } = require('./index'); - - return { execFile, existsSync, refresh: refreshShellEnvProxyVars, getSystemProxy }; -}; - -/** - * Argument-aware execFile router. Matches on a substring of the joined command; first hit wins. - * Keeps tests decoupled from the resolver's internal call sequencing. - */ -const routeExecFile = ( - execFile: jest.MockedFunction, - routes: Array<[string, { stdout: string; stderr?: string } | Error]> -) => { - execFile.mockImplementation((file: string, args: string[]) => { - const command = [file, ...args].join(' '); - for (const [needle, response] of routes) { - if (command.includes(needle)) { - return response instanceof Error - ? Promise.reject(response) - : Promise.resolve({ stderr: '', ...response }); - } - } - return Promise.resolve({ stdout: '', stderr: '' }); - }); -}; - -let originalEnv: typeof process.env; - -beforeEach(() => { - originalEnv = { ...process.env }; - mockShellEnvResult = {}; - for (const key of PROXY_ENV_KEYS) { - delete process.env[key]; - } -}); - -afterEach(() => { - process.env = originalEnv; -}); - -describe('refresh + getSystemProxy composition', () => { - it('macOS: stale env purged on refresh, system layer wins alone', async () => { - // A proxy var lingering on process.env from a previous session — the user has since - // removed the export from .zshrc, so the login shell no longer emits it. - process.env.http_proxy = 'http://stale.usebruno.com:8080'; - mockShellEnvResult = {}; - - const { execFile, refresh, getSystemProxy } = loadForPlatform('darwin'); - execFile.mockResolvedValueOnce({ - stdout: ` { - HTTPEnable : 1 - HTTPPort : 8080 - HTTPProxy : sys-proxy.usebruno.com - }`, - stderr: '' - }); - - await refresh(); - const result = await getSystemProxy(); - - // Stale env value is gone, so system detection wins with no "+ environment" suffix. - expect(result.http_proxy).toBe('http://sys-proxy.usebruno.com:8080'); - expect(result.source).toBe('macos-system'); - }); - - it('macOS: profile export beats system detection after refresh', async () => { - // .zshrc now exports HTTP_PROXY (either newly added or edited). - mockShellEnvResult = { http_proxy: 'http://profile.usebruno.com:9090' }; - - const { execFile, refresh, getSystemProxy } = loadForPlatform('darwin'); - execFile.mockResolvedValueOnce({ - stdout: ` { - HTTPEnable : 1 - HTTPPort : 8080 - HTTPProxy : sys-proxy.usebruno.com - }`, - stderr: '' - }); - - await refresh(); - const result = await getSystemProxy(); - - expect(result.http_proxy).toBe('http://profile.usebruno.com:9090'); - expect(result.source).toBe('macos-system + environment'); - }); - - it('Linux: profile export merges with gsettings detection — env wins http, system supplies https + bypass', async () => { - // .zshrc exports an http proxy; gsettings reports a DIFFERENT http proxy plus https + bypass - // the profile does not set. After refresh, the env layer should win for http and combine - // with system detection for the rest. - mockShellEnvResult = { http_proxy: 'http://profile.usebruno.com:9090' }; - - const { execFile, existsSync, refresh, getSystemProxy } = loadForPlatform('linux'); - existsSync.mockReturnValue(false); // isolate the command-based (gsettings) path - routeExecFile(execFile, [ - ['org.gnome.system.proxy mode', { stdout: '\'manual\'' }], - ['org.gnome.system.proxy.http host', { stdout: '\'sys-proxy.usebruno.com\'' }], - ['org.gnome.system.proxy.http port', { stdout: '8080' }], - ['org.gnome.system.proxy.https host', { stdout: '\'secure-proxy.usebruno.com\'' }], - ['org.gnome.system.proxy.https port', { stdout: '8443' }], - ['org.gnome.system.proxy ignore-hosts', { stdout: '[\'localhost\', \'127.0.0.1\']' }] - ]); - - await refresh(); - const result = await getSystemProxy(); - - expect(result.http_proxy).toBe('http://profile.usebruno.com:9090'); // env wins - expect(result.https_proxy).toBe('http://secure-proxy.usebruno.com:8443'); // from system - expect(result.no_proxy).toBe('localhost,127.0.0.1'); // from system - expect(result.source).toBe('linux-system + environment'); - }); - - it('Linux: stale env purged on refresh, gsettings detection wins alone', async () => { - // A proxy var lingering on process.env from a previous session — the user has since - // removed the export from their profile, so the login shell no longer emits it. - process.env.http_proxy = 'http://stale.usebruno.com:1234'; - mockShellEnvResult = {}; - - const { execFile, existsSync, refresh, getSystemProxy } = loadForPlatform('linux'); - existsSync.mockReturnValue(false); - routeExecFile(execFile, [ - ['org.gnome.system.proxy mode', { stdout: '\'manual\'' }], - ['org.gnome.system.proxy.http host', { stdout: '\'sys-proxy.usebruno.com\'' }], - ['org.gnome.system.proxy.http port', { stdout: '8080' }] - ]); - - await refresh(); - const result = await getSystemProxy(); - - // Stale env value is gone, so system detection wins with no "+ environment" suffix. - expect(result.http_proxy).toBe('http://sys-proxy.usebruno.com:8080'); - expect(result.source).toBe('linux-system'); - }); - - it('Linux (KDE): profile export merges with kreadconfig5 detection — env wins http, system supplies https + bypass', async () => { - // gsettings is unavailable, so detection falls through to KDE (kreadconfig5). The profile - // exports an http proxy; KDE reports a DIFFERENT http proxy plus https + bypass. - mockShellEnvResult = { http_proxy: 'http://profile.usebruno.com:9090' }; - - const { execFile, existsSync, refresh, getSystemProxy } = loadForPlatform('linux'); - existsSync.mockReturnValue(false); // isolate the command-based path - execFile.mockImplementation((file: string, args: string[]) => { - const fullCommand = [file, ...args].join(' '); - if (fullCommand.includes('gsettings')) { - return Promise.reject(new Error('gsettings not available')); - } - if (fullCommand.includes('kreadconfig5')) { - // Lowercase args before matching so we are agnostic to the key casing the resolver uses. - const lowerArgs = args.join(' ').toLowerCase(); - if (lowerArgs.includes('proxytype')) return Promise.resolve({ stdout: '1', stderr: '' }); - if (lowerArgs.includes('httpproxy')) return Promise.resolve({ stdout: 'http://sys-proxy.usebruno.com:8080', stderr: '' }); - if (lowerArgs.includes('httpsproxy')) return Promise.resolve({ stdout: 'http://secure-proxy.usebruno.com:8443', stderr: '' }); - if (lowerArgs.includes('noproxyfor')) return Promise.resolve({ stdout: 'localhost,127.0.0.1', stderr: '' }); - } - return Promise.resolve({ stdout: '', stderr: '' }); - }); - - await refresh(); - const result = await getSystemProxy(); - - expect(result.http_proxy).toBe('http://profile.usebruno.com:9090'); // env wins - expect(result.https_proxy).toBe('http://secure-proxy.usebruno.com:8443'); // from system (KDE) - expect(result.no_proxy).toBe('localhost,127.0.0.1'); // from system (KDE) - expect(result.source).toBe('linux-system + environment'); - }); - - it('Windows: shell profile export is ignored, registry detection wins', async () => { - // fetchShellEnv branches on process.platform (not the mocked node:os), so we force it here. - const originalPlatform = process.platform; - Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); - - try { - // A shell profile that WOULD export a proxy — on Windows the refresh is a no-op, so it - // must not surface. Registry detection alone should decide the result. - mockShellEnvResult = { http_proxy: 'http://should-be-ignored.usebruno.com:9090' }; - - const { execFile, refresh, getSystemProxy } = loadForPlatform('win32'); - routeExecFile(execFile, [ - ['Internet Settings', { - stdout: ` -HKEY_CURRENT_USER\\Software\\Microsoft\\Windows\\CurrentVersion\\Internet Settings - ProxyEnable REG_DWORD 0x1 - ProxyServer REG_SZ sys-proxy.usebruno.com:8080 -` - }] - ]); - - await refresh(); - const result = await getSystemProxy(); - - // Registry value surfaces; the ignored shell value contributes nothing, so no "+ environment". - expect(result.http_proxy).toBe('http://sys-proxy.usebruno.com:8080'); - expect(result.source).toBe('windows-system'); - } finally { - Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); - } - }); - - it('Windows: launcher-set env survives refresh and surfaces via getSystemProxy', async () => { - // fetchShellEnv branches on process.platform (not the mocked node:os), so we force it here. - const originalPlatform = process.platform; - Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); - - try { - // Set by a launcher script / parent shell / registry propagation — anything that reaches - // process.env before Bruno starts. Without the Windows early-return in - // refreshShellEnvProxyVars, this value would be stripped and never restored. - process.env.HTTP_PROXY = 'http://launcher.usebruno.com:8080'; - // A shell profile that WOULD export a different proxy — must not surface on Windows. - mockShellEnvResult = { http_proxy: 'http://should-be-ignored.usebruno.com:9090' }; - - const { execFile, refresh, getSystemProxy } = loadForPlatform('win32'); - // System registry has no proxy, so any surviving proxy in the final result must come - // from the env layer we preserved. - execFile.mockImplementation(() => Promise.resolve({ stdout: '', stderr: '' })); - - await refresh(); - const result = await getSystemProxy(); - - expect(result.http_proxy).toBe('http://launcher.usebruno.com:8080'); - expect(result.source).toContain('environment'); - } finally { - Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); - } - }); -}); diff --git a/packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts b/packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts deleted file mode 100644 index c8a43370c..000000000 --- a/packages/bruno-requests/src/utils/shell-env-proxy-refresh.spec.ts +++ /dev/null @@ -1,201 +0,0 @@ -/** - * Regression tests for refreshShellEnvProxyVars. - * - * shell-env spawns a login shell (`$SHELL -i -c 'export -p'`) that INHERITS the parent - * process.env. So any proxy var still set on process.env when the shell is spawned would be - * inherited and re-exported — meaning a proxy the user removed from their profile would never - * go away. refreshShellEnvProxyVars guards against this by deleting all proxy vars BEFORE - * invoking the shell on POSIX platforms, and by early-returning on Windows (where fetchShellEnv - * is a no-op and the delete would strip values with nothing to restore them). - * - * The mock for shell-env models login-shell parent-env inheritance and captures a snapshot of - * process.env at invocation time, so tests can assert delete-before-fetch ordering directly. - */ -import { refreshShellEnvProxyVars, PROXY_ENV_KEYS } from './shell-env'; - -let mockShellEnvResult: Record = {}; - -// When set, the mocked shellEnv() rejects with this error. Lets us verify graceful degradation. -let mockShellEnvThrows: Error | null = null; - -// Snapshot of the proxy vars present in process.env when shellEnv() was invoked. Lets tests -// assert that refreshShellEnvProxyVars deleted stale vars BEFORE spawning the shell. -// Prefix required by jest.mock's out-of-scope-variable guard (mock*/MOCK* allowed). -let mockProxyVarsSeenByShell: Record = {}; - -jest.mock('shell-env', () => ({ - shellEnv: () => { - const PROXY_KEYS = [ - 'http_proxy', - 'HTTP_PROXY', - 'https_proxy', - 'HTTPS_PROXY', - 'no_proxy', - 'NO_PROXY', - 'all_proxy', - 'ALL_PROXY' - ]; - - // Capture what the subprocess would inherit from Bruno's process.env at spawn time. - mockProxyVarsSeenByShell = {}; - for (const key of PROXY_KEYS) { - if (process.env[key] !== undefined) { - mockProxyVarsSeenByShell[key] = process.env[key] as string; - } - } - - if (mockShellEnvThrows) { - return Promise.reject(mockShellEnvThrows); - } - - // Model a login shell: inherits from parent, then applies whatever the profile exports. - return Promise.resolve({ ...mockProxyVarsSeenByShell, ...mockShellEnvResult }); - } -})); - -describe('refreshShellEnvProxyVars — shell inheritance (POSIX)', () => { - beforeEach(() => { - mockProxyVarsSeenByShell = {}; - mockShellEnvResult = {}; - mockShellEnvThrows = null; - for (const key of PROXY_ENV_KEYS) { - delete process.env[key]; - } - }); - - test('updates proxy env vars from shell config', async () => { - process.env.http_proxy = 'http://old-proxy:8080'; - mockShellEnvResult = { - http_proxy: 'http://new-proxy:8080', - https_proxy: 'http://new-proxy:8443' - }; - - await refreshShellEnvProxyVars(); - - expect(process.env.http_proxy).toBe('http://new-proxy:8080'); - expect(process.env.https_proxy).toBe('http://new-proxy:8443'); - }); - - test('removes proxy env vars missing from shell config', async () => { - // Stale vars lingering from a previous session. - process.env.http_proxy = 'http://old-proxy:8080'; - process.env.no_proxy = 'localhost'; - // User removed the exports from their profile -> shell config no longer has them. - mockShellEnvResult = {}; - - await refreshShellEnvProxyVars(); - - expect(process.env.http_proxy).toBeUndefined(); - expect(process.env.no_proxy).toBeUndefined(); - }); - - test('treats an empty-string proxy value as removal', async () => { - process.env.http_proxy = 'http://old-proxy:8080'; - mockShellEnvResult = { http_proxy: '' }; - - await refreshShellEnvProxyVars(); - - expect(process.env.http_proxy).toBeUndefined(); - }); - - test('leaves non-proxy env vars untouched', async () => { - process.env.NON_PROXY_VAR = 'keep-me'; - mockShellEnvResult = { http_proxy: 'http://new-proxy:8080' }; - - await refreshShellEnvProxyVars(); - - expect(process.env.http_proxy).toBe('http://new-proxy:8080'); - expect(process.env.NON_PROXY_VAR).toBe('keep-me'); - - delete process.env.NON_PROXY_VAR; - }); - - test('handles uppercase proxy variants (HTTP_PROXY, HTTPS_PROXY, NO_PROXY, ALL_PROXY)', async () => { - process.env.HTTP_PROXY = 'http://old-upper:8080'; - process.env.ALL_PROXY = 'http://old-all:1080'; - mockShellEnvResult = { - HTTP_PROXY: 'http://new-upper:8080', - HTTPS_PROXY: 'http://new-upper:8443', - NO_PROXY: 'localhost,127.0.0.1', - ALL_PROXY: 'http://new-all:1080' - }; - - await refreshShellEnvProxyVars(); - - expect(process.env.HTTP_PROXY).toBe('http://new-upper:8080'); - expect(process.env.HTTPS_PROXY).toBe('http://new-upper:8443'); - expect(process.env.NO_PROXY).toBe('localhost,127.0.0.1'); - expect(process.env.ALL_PROXY).toBe('http://new-all:1080'); - }); - - test('restores prior proxy vars when the shell-env subprocess fails', async () => { - process.env.http_proxy = 'http://existing:8080'; - mockShellEnvThrows = new Error('shell subprocess failed'); - - // On subprocess failure we return {} but restore the snapshot taken before the - // delete, so the user is not left silently unproxied. - await expect(refreshShellEnvProxyVars()).resolves.toEqual({}); - expect(process.env.http_proxy).toBe('http://existing:8080'); - }); - - test('returns the raw shell env dict, including non-proxy keys', async () => { - mockShellEnvResult = { - http_proxy: 'http://p:8080', - PATH: '/foo:/bar', - EDITOR: 'vim' - }; - - const result = await refreshShellEnvProxyVars(); - - expect(result).toEqual({ - http_proxy: 'http://p:8080', - PATH: '/foo:/bar', - EDITOR: 'vim' - }); - }); -}); - -describe('refreshShellEnvProxyVars — Windows', () => { - const originalPlatform = process.platform; - - beforeAll(() => { - Object.defineProperty(process, 'platform', { value: 'win32', configurable: true }); - }); - - afterAll(() => { - Object.defineProperty(process, 'platform', { value: originalPlatform, configurable: true }); - }); - - beforeEach(() => { - mockProxyVarsSeenByShell = {}; - mockShellEnvResult = {}; - mockShellEnvThrows = null; - for (const key of PROXY_ENV_KEYS) { - delete process.env[key]; - } - }); - - test('leaves process.env proxy vars untouched (registry / launcher / parent-shell values survive)', async () => { - // A proxy in process.env from any Windows source: registry propagation, launcher script, - // PowerShell $env, Git Bash export, etc. All look the same to us. - process.env.HTTP_PROXY = 'http://launcher-proxy:8080'; - process.env.https_proxy = 'http://another:9090'; - - await refreshShellEnvProxyVars(); - - expect(process.env.HTTP_PROXY).toBe('http://launcher-proxy:8080'); - expect(process.env.https_proxy).toBe('http://another:9090'); - }); - - test('does not invoke the shell subprocess on Windows', async () => { - // A shell profile that would export a proxy if it were consulted — it must not be. - mockShellEnvResult = { http_proxy: 'http://should-not-appear:8080' }; - - await refreshShellEnvProxyVars(); - - // mockProxyVarsSeenByShell is only populated when the shell-env mock is actually called. - // Empty means we never invoked shellEnv() — the Windows early-return short-circuited. - expect(mockProxyVarsSeenByShell).toEqual({}); - expect(process.env.http_proxy).toBeUndefined(); - }); -}); diff --git a/packages/bruno-requests/src/utils/shell-env.ts b/packages/bruno-requests/src/utils/shell-env.ts index 29d9bc7c9..d98bdd487 100644 --- a/packages/bruno-requests/src/utils/shell-env.ts +++ b/packages/bruno-requests/src/utils/shell-env.ts @@ -6,38 +6,7 @@ import path from 'path'; -export const PROXY_ENV_KEYS = [ - 'http_proxy', - 'HTTP_PROXY', - 'https_proxy', - 'HTTPS_PROXY', - 'no_proxy', - 'NO_PROXY', - 'all_proxy', - 'ALL_PROXY' -] as const; - -const TIMEOUT = Symbol('shell-env-timeout'); -const TIMEOUT_MS = 60_000; - -/** - * Races a promise against a timeout. Resolves to the TIMEOUT symbol if the - * timeout wins, so a misconfigured shell can't hang the caller indefinitely. - */ -const withTimeout = async (promise: Promise, timeoutMs: number): Promise => { - let timeoutId: ReturnType; - const timeoutPromise = new Promise((resolve) => { - timeoutId = setTimeout(() => resolve(TIMEOUT), timeoutMs); - }); - - try { - return await Promise.race([promise, timeoutPromise]); - } finally { - clearTimeout(timeoutId!); - } -}; - -const fetchShellEnv = async (): Promise | null> => { +export const fetchShellEnv = async (): Promise | null> => { // Windows handles environment variables differently - skip // everything related to windows proxy settings is handled by the system proxy resolver i.e getSystemProxy() if (process.platform === 'win32') { @@ -76,53 +45,3 @@ export const initializeShellEnv = async (): Promise> => { } return shellEnvVars; }; - -/** - * Re-syncs proxy-related process.env values from the user's shell configuration. - * Used when refreshing system proxy settings without restarting the app. - * - * @returns The fetched shell environment variables - */ -export const refreshShellEnvProxyVars = async (): Promise> => { - // Windows handles environment variables differently - skip - // everything related to windows proxy settings is handled by the - // system proxy resolver i.e getSystemProxy() - if (process.platform === 'win32') { - return {}; - } - - // Snapshot and clear stale proxy vars first so shell-env does not inherit them - // into the login shell subprocess (removed .zshrc exports would otherwise persist). - const snapshot: Record = {}; - for (const key of PROXY_ENV_KEYS) { - snapshot[key] = process.env[key]; - delete process.env[key]; - } - - const restoreSnapshot = () => { - for (const key of PROXY_ENV_KEYS) { - if (snapshot[key] !== undefined) { - process.env[key] = snapshot[key] as string; - } - } - }; - - // Race the shell-env subprocess against a 60s timeout so a misconfigured shell - // can't hang the refresh indefinitely. - const result = await withTimeout(fetchShellEnv(), TIMEOUT_MS); - - if (result === TIMEOUT || result === null) { - // Timed out — restore prior values rather than leave the user unproxied. - restoreSnapshot(); - return {}; - } - - for (const key of PROXY_ENV_KEYS) { - const value = result[key]; - if (value) { - process.env[key] = value; - } - } - - return result; -};