Make the client more secure (#3742)

Co-authored-by: Douwe Osinga <douwe@squareup.com>
Co-authored-by: Michael Neale <michael.neale@gmail.com>

ui/desktop/src/agent/index.ts

@@ -1,4 +1,4 @@
-import { getApiUrl, getSecretKey } from '../config';
+import { getApiUrl } from '../config';
 
 interface initializeAgentProps {
   model: string;
@@ -10,7 +10,7 @@ export async function initializeAgent({ model, provider }: initializeAgentProps)
     method: 'POST',
     headers: {
       'Content-Type': 'application/json',
-      'X-Secret-Key': getSecretKey(),
+      'X-Secret-Key': await window.electron.getSecretKey(),
     },
     body: JSON.stringify({
       provider: provider.toLowerCase().replace(/ /g, '_'),

ui/desktop/src/components/ConfigContext.tsx

@@ -9,7 +9,6 @@ import {
   removeExtension as apiRemoveExtension,
   providers,
 } from '../api';
-import { client } from '../api/client.gen';
 import type {
   ConfigResponse,
   UpsertConfigQuery,
@@ -18,8 +17,9 @@ import type {
   ProviderDetails,
   ExtensionQuery,
   ExtensionConfig,
-} from '../api/types.gen';
+} from '../api';
 import { removeShims } from './settings/extensions/utils';
+import { ensureClientInitialized } from '../utils';
 
 export type { ExtensionConfig } from '../api/types.gen';
 
@@ -28,15 +28,6 @@ export type FixedExtensionEntry = ExtensionConfig & {
   enabled: boolean;
 };
 
-// Initialize client configuration
-client.setConfig({
-  baseUrl: window.appConfig.get('GOOSE_API_HOST') + ':' + window.appConfig.get('GOOSE_PORT'),
-  headers: {
-    'Content-Type': 'application/json',
-    'X-Secret-Key': window.appConfig.get('secretKey'),
-  },
-});
-
 interface ConfigContextType {
   config: ConfigResponse['config'];
   providersList: ProviderDetails[];
@@ -184,6 +175,7 @@ export const ConfigProvider: React.FC<ConfigProviderProps> = ({ children }) => {
   useEffect(() => {
     // Load all configuration data and providers on mount
     (async () => {
+      await ensureClientInitialized();
       // Load config
       const configResponse = await readAllConfig();
       setConfig(configResponse.data?.config || {});

ui/desktop/src/components/ProviderGuard.tsx

@@ -6,6 +6,7 @@ import { startOpenRouterSetup } from '../utils/openRouterSetup';
 import WelcomeGooseLogo from './WelcomeGooseLogo';
 import { initializeSystem } from '../utils/providerUtils';
 import { toastService } from '../toasts';
+import { ensureClientInitialized } from '../utils';
 
 interface ProviderGuardProps {
   children: React.ReactNode;
@@ -95,6 +96,8 @@ export default function ProviderGuard({ children }: ProviderGuardProps) {
   useEffect(() => {
     const checkProvider = async () => {
       try {
+        await ensureClientInitialized();
+
         const config = window.electron.getConfig();
         console.log('ProviderGuard - Full config:', config);
 

ui/desktop/src/components/common/ActivityHeatmap.tsx

@@ -1,6 +1,6 @@
 import { useEffect, useState } from 'react';
 import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from '../ui/Tooltip';
-import { getApiUrl, getSecretKey } from '../../config';
+import { getApiUrl } from '../../config';
 
 interface ActivityHeatmapCell {
   week: number;
@@ -40,7 +40,7 @@ export function ActivityHeatmap() {
           headers: {
             Accept: 'application/json',
             'Content-Type': 'application/json',
-            'X-Secret-Key': getSecretKey(),
+            'X-Secret-Key': await window.electron.getSecretKey(),
           },
         });
 

ui/desktop/src/components/sessions/SessionsInsights.tsx

@@ -1,7 +1,7 @@
 import { useEffect, useState } from 'react';
 import { Card, CardContent, CardDescription } from '../ui/card';
 // import { Folder } from 'lucide-react';
-import { getApiUrl, getSecretKey } from '../../config';
+import { getApiUrl } from '../../config';
 import { Greeting } from '../common/Greeting';
 import { fetchSessions, fetchSessionDetails, type Session } from '../../sessions';
 // import { fetchProjects, type ProjectMetadata } from '../../projects';
@@ -36,7 +36,7 @@ export function SessionInsights() {
           headers: {
             Accept: 'application/json',
             'Content-Type': 'application/json',
-            'X-Secret-Key': getSecretKey(),
+            'X-Secret-Key': await window.electron.getSecretKey(),
           },
         });
 

ui/desktop/src/components/settings/app/AppSettingsSection.tsx

@@ -5,7 +5,7 @@ import { Settings, RefreshCw, ExternalLink } from 'lucide-react';
 import { Dialog, DialogContent, DialogFooter, DialogHeader, DialogTitle } from '../../ui/dialog';
 import UpdateSection from './UpdateSection';
 import { COST_TRACKING_ENABLED, UPDATES_ENABLED } from '../../../updates';
-import { getApiUrl, getSecretKey } from '../../../config';
+import { getApiUrl } from '../../../config';
 import { Card, CardContent, CardDescription, CardHeader, CardTitle } from '../../ui/card';
 import ThemeSelector from '../../GooseSidebar/ThemeSelector';
 import BlockLogoBlack from './icons/block-lockup_black.png';
@@ -71,7 +71,7 @@ export default function AppSettingsSection({ scrollToSection }: AppSettingsSecti
   const checkPricingStatus = async () => {
     try {
       const apiUrl = getApiUrl('/config/pricing');
-      const secretKey = getSecretKey();
+      const secretKey = await window.electron.getSecretKey();
 
       const headers: HeadersInit = { 'Content-Type': 'application/json' };
       if (secretKey) {
@@ -100,7 +100,7 @@ export default function AppSettingsSection({ scrollToSection }: AppSettingsSecti
     setIsRefreshing(true);
     try {
       const apiUrl = getApiUrl('/config/pricing');
-      const secretKey = getSecretKey();
+      const secretKey = await window.electron.getSecretKey();
 
       const headers: HeadersInit = { 'Content-Type': 'application/json' };
       if (secretKey) {

ui/desktop/src/components/settings/extensions/agent-api.ts

@@ -1,5 +1,5 @@
 import { ExtensionConfig } from '../../../api/types.gen';
-import { getApiUrl, getSecretKey } from '../../../config';
+import { getApiUrl } from '../../../config';
 import { toastService, ToastServiceOptions } from '../../../toasts';
 import { replaceWithShims } from './utils';
 
@@ -46,7 +46,7 @@ export async function extensionApiCall(
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
-        'X-Secret-Key': getSecretKey(),
+        'X-Secret-Key': await window.electron.getSecretKey(),
       },
       body: JSON.stringify(payload),
     });

ui/desktop/src/components/settings/tool_selection_strategy/ToolSelectionStrategySection.tsx

@@ -1,6 +1,6 @@
 import { useEffect, useState, useCallback } from 'react';
 import { useConfig } from '../../ConfigContext';
-import { getApiUrl, getSecretKey } from '../../../config';
+import { getApiUrl } from '../../../config';
 
 interface ToolSelectionStrategy {
   key: string;
@@ -56,7 +56,7 @@ export const ToolSelectionStrategySection = () => {
           method: 'POST',
           headers: {
             'Content-Type': 'application/json',
-            'X-Secret-Key': getSecretKey(),
+            'X-Secret-Key': await window.electron.getSecretKey(),
           },
         });
 

ui/desktop/src/config.ts

@@ -7,7 +7,3 @@ export const getApiUrl = (endpoint: string): string => {
   const cleanEndpoint = endpoint.startsWith('/') ? endpoint : `/${endpoint}`;
   return `${baseUrl}${cleanEndpoint}`;
 };
-
-export const getSecretKey = (): string => {
-  return String(window.appConfig.get('secretKey') || '');
-};

ui/desktop/src/extensions.tsx

@@ -1,4 +1,4 @@
-import { getApiUrl, getSecretKey } from './config';
+import { getApiUrl } from './config';
 import { toast } from 'react-toastify';
 import { safeJsonParse } from './utils/jsonUtils';
 
@@ -100,7 +100,7 @@ export async function addExtension(
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
-        'X-Secret-Key': getSecretKey(),
+        'X-Secret-Key': await window.electron.getSecretKey(),
       },
       body: JSON.stringify(config),
     });
@@ -177,7 +177,7 @@ export async function removeExtension(name: string, silent: boolean = false): Pr
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
-        'X-Secret-Key': getSecretKey(),
+        'X-Secret-Key': await window.electron.getSecretKey(),
       },
       body: JSON.stringify(sanitizeName(name)),
     });

ui/desktop/src/goosed.ts

@@ -94,6 +94,7 @@ interface GooseProcessEnv {
 
 export const startGoosed = async (
   app: App,
+  serverSecret: string,
   dir: string | null = null,
   env: Partial<GooseProcessEnv> = {}
 ): Promise<[number, string, ChildProcess]> => {
@@ -182,7 +183,7 @@ export const startGoosed = async (
     PATH: `${path.dirname(resolvedGoosedPath)}${path.delimiter}${process.env.PATH || ''}`,
     // start with the port specified
     GOOSE_PORT: String(port),
-    GOOSE_SERVER__SECRET_KEY: process.env.GOOSE_SERVER__SECRET_KEY,
+    GOOSE_SERVER__SECRET_KEY: serverSecret,
     // Add any additional environment variables passed in
     ...env,
   } as GooseProcessEnv;
@@ -208,20 +209,6 @@ export const startGoosed = async (
   }
   log.info(`Binary path resolved to: ${goosedPath}`);
 
-  // Verify binary exists and is a regular file
-  try {
-    // eslint-disable-next-line @typescript-eslint/no-var-requires
-    const fs = require('fs');
-    const stats = fs.statSync(goosedPath);
-    if (!stats.isFile()) {
-      throw new Error(`Path is not a regular file: ${goosedPath}`);
-    }
-    log.info(`Binary exists and is a regular file: ${stats.isFile()}`);
-  } catch (error) {
-    log.error(`Binary not found or invalid at ${goosedPath}:`, error);
-    throw new Error(`Binary not found or invalid at ${goosedPath}`);
-  }
-
   const spawnOptions = {
     cwd: dir,
     env: processEnv,
@@ -282,16 +269,11 @@ export const startGoosed = async (
   // Wait for the server to be ready
   const isReady = await checkServerStatus(port);
   log.info(`Goosed isReady ${isReady}`);
-  if (!isReady) {
+
-    log.error(`Goosed server failed to start on port ${port}`);
+  const try_kill_goose = () => {
     try {
       if (isWindows) {
-        // On Windows, use taskkill to forcefully terminate the process tree
-        // Security: Validate PID is numeric and use safe arguments
         const pid = goosedProcess.pid?.toString() || '0';
-        if (!/^\d+$/.test(pid)) {
-          throw new Error(`Invalid PID: ${pid}`);
-        }
         spawn('taskkill', ['/pid', pid, '/T', '/F'], { shell: false });
       } else {
         goosedProcess.kill?.();
@@ -299,6 +281,11 @@ export const startGoosed = async (
     } catch (error) {
       log.error('Error while terminating goosed process:', error);
     }
+  };
+
+  if (!isReady) {
+    log.error(`Goosed server failed to start on port ${port}`);
+    try_kill_goose();
     throw new Error(`Goosed server failed to start on port ${port}`);
   }
 
@@ -306,22 +293,7 @@ export const startGoosed = async (
   // TODO will need to do it at tab level next
   app.on('will-quit', () => {
     log.info('App quitting, terminating goosed server');
-    try {
+    try_kill_goose();
-      if (isWindows) {
-        // On Windows, use taskkill to forcefully terminate the process tree
-        // Security: Validate PID is numeric and use safe arguments
-        const pid = goosedProcess.pid?.toString() || '0';
-        if (!/^\d+$/.test(pid)) {
-          log.error(`Invalid PID for termination: ${pid}`);
-          return;
-        }
-        spawn('taskkill', ['/pid', pid, '/T', '/F'], { shell: false });
-      } else {
-        goosedProcess.kill?.();
-      }
-    } catch (error) {
-      log.error('Error while terminating goosed process:', error);
-    }
   });
 
   log.info(`Goosed server successfully started on port ${port}`);

ui/desktop/src/hooks/useMessageStream.ts

@@ -1,7 +1,6 @@
-import { useState, useCallback, useEffect, useRef, useId, useReducer } from 'react';
+import { useCallback, useEffect, useId, useReducer, useRef, useState } from 'react';
 import useSWR from 'swr';
-import { getSecretKey } from '../config';
+import { createUserMessage, hasCompletedToolCalls, Message } from '../types/message';
-import { Message, createUserMessage, hasCompletedToolCalls } from '../types/message';
 import { getSessionHistory } from '../api';
 import { ChatState } from '../types/chatState';
 
@@ -382,9 +381,7 @@ export function useMessageStream({
                       break; // Don't throw error, just add the message
                     }
 
-                    // For non-token-limit errors, still throw the error
+                    throw new Error(parsedEvent.error);
-                    const error = new Error(parsedEvent.error);
-                    throw error;
                   }
 
                   case 'Finish': {
@@ -478,7 +475,7 @@ export function useMessageStream({
           method: 'POST',
           headers: {
             'Content-Type': 'application/json',
-            'X-Secret-Key': getSecretKey(),
+            'X-Secret-Key': await window.electron.getSecretKey(),
             ...extraMetadataRef.current.headers,
           },
           body: JSON.stringify({

ui/desktop/src/hooks/useWhisper.ts

@@ -1,6 +1,6 @@
 import { useState, useRef, useCallback, useEffect } from 'react';
 import { useConfig } from '../components/ConfigContext';
-import { getApiUrl, getSecretKey } from '../config';
+import { getApiUrl } from '../config';
 import { useDictationSettings } from './useDictationSettings';
 import { safeJsonParse } from '../utils/jsonUtils';
 
@@ -117,7 +117,7 @@ export const useWhisper = ({ onTranscription, onError, onSizeWarning }: UseWhisp
         let endpoint = '';
         let headers: Record<string, string> = {
           'Content-Type': 'application/json',
-          'X-Secret-Key': getSecretKey(),
+          'X-Secret-Key': await window.electron.getSecretKey(),
         };
         let body: Record<string, string> = {
           audio: base64Audio,

ui/desktop/src/main.ts

@@ -1,4 +1,4 @@
-import type { OpenDialogReturnValue, OpenDialogOptions } from 'electron';
+import type { OpenDialogOptions, OpenDialogReturnValue } from 'electron';
 import {
   app,
   App,
@@ -25,7 +25,7 @@ import os from 'node:os';
 import { spawn } from 'child_process';
 import 'dotenv/config';
 import { startGoosed } from './goosed';
-import { getBinaryPath, expandTilde } from './utils/pathUtils';
+import { expandTilde, getBinaryPath } from './utils/pathUtils';
 import { loadShellEnv } from './utils/loadEnv';
 import log from './utils/logger';
 import { ensureWinShims } from './utils/winShims';
@@ -463,12 +463,6 @@ const getGooseProvider = () => {
   ];
 };
 
-const generateSecretKey = () => {
-  const key = process.env.GOOSE_EXTERNAL_BACKEND ? 'test' : crypto.randomBytes(32).toString('hex');
-  process.env.GOOSE_SERVER__SECRET_KEY = key;
-  return key;
-};
-
 const getSharingUrl = () => {
   // checks app env for sharing url
   loadShellEnv(app.isPackaged); // will try to take it from the zshrc file
@@ -484,11 +478,15 @@ const getVersion = () => {
   return process.env.GOOSE_VERSION;
 };
 
-let [provider, model, predefinedModels] = getGooseProvider();
+const [provider, model, predefinedModels] = getGooseProvider();
 
-let sharingUrl = getSharingUrl();
+const sharingUrl = getSharingUrl();
 
-let gooseVersion = getVersion();
+const gooseVersion = getVersion();
+
+const SERVER_SECRET = process.env.GOOSE_EXTERNAL_BACKEND
+  ? 'test'
+  : crypto.randomBytes(32).toString('hex');
 
 let appConfig = {
   GOOSE_DEFAULT_PROVIDER: provider,
@@ -499,7 +497,6 @@ let appConfig = {
   GOOSE_WORKING_DIR: '',
   // If GOOSE_ALLOWLIST_WARNING env var is not set, defaults to false (strict blocking mode)
   GOOSE_ALLOWLIST_WARNING: process.env.GOOSE_ALLOWLIST_WARNING === 'true',
-  secretKey: generateSecretKey(),
 };
 
 // Track windows by ID
@@ -559,7 +556,12 @@ const createChat = async (
     const envVars = {
       GOOSE_SCHEDULER_TYPE: process.env.GOOSE_SCHEDULER_TYPE,
     };
-    const [newPort, newWorkingDir, newGoosedProcess] = await startGoosed(app, dir, envVars);
+    const [newPort, newWorkingDir, newGoosedProcess] = await startGoosed(
+      app,
+      SERVER_SECRET,
+      dir,
+      envVars
+    );
     port = newPort;
     working_dir = newWorkingDir;
     goosedProcess = newGoosedProcess;
@@ -1038,14 +1040,17 @@ ipcMain.handle('directory-chooser', (_event, replace: boolean = false) => {
 // Handle scheduling engine settings
 ipcMain.handle('get-settings', () => {
   try {
-    const settings = loadSettings();
+    return loadSettings();
-    return settings;
   } catch (error) {
     console.error('Error getting settings:', error);
     return null;
   }
 });
 
+ipcMain.handle('get-secret-key', () => {
+  return SERVER_SECRET;
+});
+
 ipcMain.handle('set-scheduling-engine', async (_event, engine: string) => {
   try {
     const settings = loadSettings();
@@ -1614,8 +1619,7 @@ ipcMain.handle('list-files', async (_event, dirPath, extension) => {
 
 // Handle message box dialogs
 ipcMain.handle('show-message-box', async (_event, options) => {
-  const result = await dialog.showMessageBox(options);
+  return dialog.showMessageBox(options);
-  return result;
 });
 
 ipcMain.handle('get-allowed-extensions', async () => {

ui/desktop/src/preload.ts

@@ -77,6 +77,7 @@ type ElectronAPI = {
   setDockIcon: (show: boolean) => Promise<boolean>;
   getDockIconState: () => Promise<boolean>;
   getSettings: () => Promise<unknown | null>;
+  getSecretKey: () => Promise<string>;
   setSchedulingEngine: (engine: string) => Promise<boolean>;
   setQuitConfirmation: (show: boolean) => Promise<boolean>;
   getQuitConfirmationState: () => Promise<boolean>;
@@ -171,6 +172,7 @@ const electronAPI: ElectronAPI = {
   setDockIcon: (show: boolean) => ipcRenderer.invoke('set-dock-icon', show),
   getDockIconState: () => ipcRenderer.invoke('get-dock-icon-state'),
   getSettings: () => ipcRenderer.invoke('get-settings'),
+  getSecretKey: () => ipcRenderer.invoke('get-secret-key'),
   setSchedulingEngine: (engine: string) => ipcRenderer.invoke('set-scheduling-engine', engine),
   setQuitConfirmation: (show: boolean) => ipcRenderer.invoke('set-quit-confirmation', show),
   getQuitConfirmationState: () => ipcRenderer.invoke('get-quit-confirmation-state'),

ui/desktop/src/utils.ts

@@ -1,5 +1,6 @@
 import { clsx, type ClassValue } from 'clsx';
 import { twMerge } from 'tailwind-merge';
+import { client } from './api/client.gen';
 
 export function cn(...inputs: ClassValue[]) {
   return twMerge(clsx(inputs));
@@ -16,3 +17,19 @@ export function patchConsoleLogging() {
   // Intercept console methods
   return;
 }
+
+// This needs to be called before any API calls are made, but since we're using the client
+// in multiple useEffect locations, we can't be sure who goes first.
+let clientInitialized = false;
+
+export async function ensureClientInitialized() {
+  if (clientInitialized) return;
+  client.setConfig({
+    baseUrl: window.appConfig.get('GOOSE_API_HOST') + ':' + window.appConfig.get('GOOSE_PORT'),
+    headers: {
+      'Content-Type': 'application/json',
+      'X-Secret-Key': await window.electron.getSecretKey(),
+    },
+  });
+  clientInitialized = true;
+}

ui/desktop/src/utils/costDatabase.ts

@@ -1,5 +1,5 @@
 // Import the proper type from ConfigContext
-import { getApiUrl, getSecretKey } from '../config';
+import { getApiUrl } from '../config';
 import { safeJsonParse } from './jsonUtils';
 
 export interface ModelCostInfo {
@@ -31,7 +31,7 @@ async function fetchPricingForModel(
   }
 
   const apiUrl = getApiUrl('/config/pricing');
-  const secretKey = getSecretKey();
+  const secretKey = await window.electron.getSecretKey();
 
   const headers: HeadersInit = { 'Content-Type': 'application/json' };
   if (secretKey) {
@@ -217,7 +217,7 @@ export async function refreshPricing(): Promise<boolean> {
 
     // The actual refresh happens on the backend when we call with configured_only: false
     const apiUrl = getApiUrl('/config/pricing');
-    const secretKey = getSecretKey();
+    const secretKey = await window.electron.getSecretKey();
 
     const headers: HeadersInit = { 'Content-Type': 'application/json' };
     if (secretKey) {

ui/desktop/src/utils/providerUtils.ts

@@ -1,4 +1,4 @@
-import { getApiUrl, getSecretKey } from '../config';
+import { getApiUrl } from '../config';
 import { FullExtensionConfig } from '../extensions';
 import { initializeAgent } from '../agent';
 import {
@@ -98,7 +98,7 @@ export const updateSystemPromptWithParameters = async (
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
-        'X-Secret-Key': getSecretKey(),
+        'X-Secret-Key': await window.electron.getSecretKey(),
       },
       body: JSON.stringify({
         extension: `${desktopPromptBot}\nIMPORTANT instructions for you to operate as agent:\n${substitutedInstructions}`,
@@ -135,8 +135,6 @@ export const updateSystemPromptWithParameters = async (
  * NOTE: This logic can be removed eventually when enough versions have passed
  * We leave the existing user settings in localStorage, in case users downgrade
  * or things need to be reverted.
- *
- * @param addExtension Function to add extension to config.yaml
  */
 export const migrateExtensionsToSettingsV3 = async () => {
   console.log('need to perform extension migration v3');
@@ -227,7 +225,7 @@ export const initializeSystem = async (
       method: 'POST',
       headers: {
         'Content-Type': 'application/json',
-        'X-Secret-Key': getSecretKey(),
+        'X-Secret-Key': await window.electron.getSecretKey(),
       },
       body: JSON.stringify({
         extension: prompt,
@@ -247,7 +245,7 @@ export const initializeSystem = async (
         method: 'POST',
         headers: {
           'Content-Type': 'application/json',
-          'X-Secret-Key': getSecretKey(),
+          'X-Secret-Key': await window.electron.getSecretKey(),
         },
         body: JSON.stringify({
           response: responseConfig,

ui/desktop/vite.renderer.config.mts

@@ -9,4 +9,8 @@ export default defineConfig({
   },
 
   plugins: [tailwindcss()],
+
+  build: {
+    target: 'esnext'
+  }
 });