diff --git a/ui/desktop/src/components/ChatView.tsx b/ui/desktop/src/components/ChatView.tsx index 6c01d8955921..a7869b4b494b 100644 --- a/ui/desktop/src/components/ChatView.tsx +++ b/ui/desktop/src/components/ChatView.tsx @@ -43,6 +43,15 @@ export interface ChatType { messages: Message[]; } +interface _RecipeConfig { + id: string; + name: string; + description: string; + instructions?: string; + activities?: string[]; + [key: string]: unknown; +} + // Helper function to determine if a message is a user message const isUserMessage = (message: Message): boolean => { if (message.role === 'assistant') { @@ -244,18 +253,11 @@ function ChatContent({ // Create a new window for the recipe editor console.log('Opening recipe editor with config:', response.recipe); - const recipeConfig: { - id: string; - title: string; - description: string; - instructions: string; - activities: string[]; - prompt: string; - } = { + const recipeConfig = { id: response.recipe.title || 'untitled', - title: response.recipe.title, - description: response.recipe.description, - instructions: response.recipe.instructions, + name: response.recipe.title || 'Untitled Recipe', + description: response.recipe.description || '', + instructions: response.recipe.instructions || '', activities: response.recipe.activities || [], prompt: response.recipe.prompt || '', }; @@ -287,11 +289,8 @@ function ChatContent({ // Update chat messages when they change and save to sessionStorage useEffect(() => { - setChat((prevChat: ChatType) => { - const updatedChat = { ...prevChat, messages }; - return updatedChat; - }); - }, [messages, setChat]); + setChat({ ...chat, messages }); + }, [messages, setChat, chat]); useEffect(() => { if (messages.length > 0) { diff --git a/ui/desktop/src/components/more_menu/MoreMenu.tsx b/ui/desktop/src/components/more_menu/MoreMenu.tsx index 04e40aa4b378..a54ed55238f8 100644 --- a/ui/desktop/src/components/more_menu/MoreMenu.tsx +++ b/ui/desktop/src/components/more_menu/MoreMenu.tsx @@ -7,10 +7,11 @@ import { ViewOptions, View } from '../../App'; interface RecipeConfig { id: string; - title: string; + name: string; description: string; - instructions: string; - activities: string[]; + instructions?: string; + activities?: string[]; + [key: string]: unknown; } interface MenuButtonProps { @@ -252,7 +253,7 @@ export default function MoreMenu({ undefined, // dir undefined, // version undefined, // resumeSessionId - recipeConfig as RecipeConfig | undefined, // recipe config + recipeConfig as RecipeConfig, // recipe config 'recipeEditor' // view type ); }} diff --git a/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx b/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx index 99e28a9f25ca..358ab72ff6c3 100644 --- a/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx +++ b/ui/desktop/src/components/settings_v2/extensions/ExtensionsSection.tsx @@ -250,7 +250,7 @@ export default function ExtensionsSection({ {deepLinkConfigStateVar && showEnvVarsStateVar && ( = { provider_name: OllamaSubmitHandler, // example }; @@ -94,7 +98,7 @@ export default function ProviderConfigurationModal() { // Call onSubmit callback if provided (from modal props) if (modalProps.onSubmit) { - modalProps.onSubmit(configValues); + modalProps.onSubmit(configValues as FormValues); } } catch (error) { console.error('Failed to save configuration:', error); @@ -156,7 +160,7 @@ export default function ProviderConfigurationModal() { // Call onDelete callback if provided // This should trigger the refreshProviders function if (modalProps.onDelete) { - modalProps.onDelete(currentProvider.name as unknown); + modalProps.onDelete(currentProvider.name as FormValues); } // Reset the delete confirmation state before closing diff --git a/ui/desktop/src/goosed.ts b/ui/desktop/src/goosed.ts index 29dab4932e46..6501a4b269f8 100644 --- a/ui/desktop/src/goosed.ts +++ b/ui/desktop/src/goosed.ts @@ -70,17 +70,51 @@ export const startGoosed = async ( const homeDir = os.homedir(); const isWindows = process.platform === 'win32'; - // Ensure dir is properly normalized for the platform + // Ensure dir is properly normalized for the platform and validate it if (!dir) { dir = homeDir; } - dir = path.normalize(dir); + + // Sanitize and validate the directory path + dir = path.resolve(path.normalize(dir)); + + // Security check: Ensure the directory path doesn't contain suspicious characters + if (dir.includes('..') || dir.includes(';') || dir.includes('|') || dir.includes('&')) { + throw new Error(`Invalid directory path: ${dir}`); + } // Get the goosed binary path using the shared utility let goosedPath = getBinaryPath(app, 'goosed'); + + // Security validation: Ensure the binary path is safe + const resolvedGoosedPath = path.resolve(goosedPath); + + // Validate that the binary path doesn't contain suspicious characters or sequences + if (resolvedGoosedPath.includes('..') || + resolvedGoosedPath.includes(';') || + resolvedGoosedPath.includes('|') || + resolvedGoosedPath.includes('&') || + resolvedGoosedPath.includes('`') || + resolvedGoosedPath.includes('$')) { + throw new Error(`Invalid binary path detected: ${resolvedGoosedPath}`); + } + + // Ensure the binary path is within expected application directories + const appPath = app.getAppPath(); + const resourcesPath = process.resourcesPath; + const currentWorkingDir = process.cwd(); + + const isValidPath = resolvedGoosedPath.startsWith(path.resolve(appPath)) || + resolvedGoosedPath.startsWith(path.resolve(resourcesPath)) || + resolvedGoosedPath.startsWith(path.resolve(currentWorkingDir)); + + if (!isValidPath) { + throw new Error(`Binary path is outside of allowed directories: ${resolvedGoosedPath}`); + } + const port = await findAvailablePort(); - log.info(`Starting goosed from: ${goosedPath} on port ${port} in dir ${dir}`); + log.info(`Starting goosed from: ${resolvedGoosedPath} on port ${port} in dir ${dir}`); // Define additional environment variables const additionalEnv: GooseProcessEnv = { @@ -93,7 +127,7 @@ export const startGoosed = async ( // Set LOCAL_APPDATA for Windows LOCALAPPDATA: process.env.LOCALAPPDATA || path.join(homeDir, 'AppData', 'Local'), // Set PATH to include the binary directory - PATH: `${path.dirname(goosedPath)}${path.delimiter}${process.env.PATH || ''}`, + 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, @@ -115,20 +149,25 @@ export const startGoosed = async ( log.info(`Environment PATH: ${processEnv.PATH}`); // Ensure proper executable path on Windows - if (isWindows && !goosedPath.toLowerCase().endsWith('.exe')) { - goosedPath += '.exe'; + if (isWindows && !resolvedGoosedPath.toLowerCase().endsWith('.exe')) { + goosedPath = resolvedGoosedPath + '.exe'; + } else { + goosedPath = resolvedGoosedPath; } log.info(`Binary path resolved to: ${goosedPath}`); - // Verify binary exists + // 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); - log.info(`Binary exists: ${stats.isFile()}`); + 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 at ${goosedPath}:`, error); - throw new Error(`Binary not found at ${goosedPath}`); + log.error(`Binary not found or invalid at ${goosedPath}:`, error); + throw new Error(`Binary not found or invalid at ${goosedPath}`); } const spawnOptions = { @@ -139,15 +178,29 @@ export const startGoosed = async ( windowsHide: true, // Run detached on Windows only to avoid terminal windows detached: isWindows, - // Never use shell to avoid terminal windows + // Never use shell to avoid command injection - this is critical for security shell: false, }; - // Log spawn options for debugging - log.info('Spawn options:', JSON.stringify(spawnOptions, null, 2)); + // Log spawn options for debugging (excluding sensitive env vars) + const safeSpawnOptions = { + ...spawnOptions, + env: Object.keys(spawnOptions.env || {}).reduce((acc, key) => { + if (key.includes('SECRET') || key.includes('PASSWORD') || key.includes('TOKEN')) { + acc[key] = '[REDACTED]'; + } else { + acc[key] = spawnOptions.env![key]; + } + return acc; + }, {} as Record) + }; + log.info('Spawn options:', JSON.stringify(safeSpawnOptions, null, 2)); - // Spawn the goosed process - const goosedProcess: ChildProcess = spawn(goosedPath, ['agent'], spawnOptions); + // Security: Use only hardcoded, safe arguments + const safeArgs = ['agent']; // Only allow the 'agent' argument + + // Spawn the goosed process with validated inputs + const goosedProcess: ChildProcess = spawn(goosedPath, safeArgs, spawnOptions); // Only unref on Windows to allow it to run independently of the parent if (isWindows) { @@ -179,7 +232,12 @@ export const startGoosed = async ( try { if (isWindows) { // On Windows, use taskkill to forcefully terminate the process tree - spawn('taskkill', ['/pid', goosedProcess.pid?.toString() || "0", '/T', '/F']); + // 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?.(); } @@ -196,7 +254,13 @@ export const startGoosed = async ( try { if (isWindows) { // On Windows, use taskkill to forcefully terminate the process tree - spawn('taskkill', ['/pid', goosedProcess.pid?.toString() || "0", '/T', '/F']); + // 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?.(); } diff --git a/ui/desktop/src/sessionLinks.ts b/ui/desktop/src/sessionLinks.ts index f3a0493ae01a..09a3f8f88377 100644 --- a/ui/desktop/src/sessionLinks.ts +++ b/ui/desktop/src/sessionLinks.ts @@ -27,7 +27,7 @@ export async function openSharedSessionFromDeepLink( } // Extract the share token from the URL - const shareToken = url.replace('goose://sessions/', ''); + const shareToken: string = url.replace('goose://sessions/', ''); if (!shareToken || shareToken.trim() === '') { throw new Error('Invalid URL: Missing share token'); @@ -58,7 +58,7 @@ export async function openSharedSessionFromDeepLink( } // Fetch the shared session details - const sessionDetails = await fetchSharedSessionDetails(baseUrl, shareToken); + const sessionDetails = await fetchSharedSessionDetails(baseUrl!, shareToken); // Navigate to the shared session view setView('sharedSession', { diff --git a/ui/desktop/src/utils/binaryPath.ts b/ui/desktop/src/utils/binaryPath.ts index 9704a6770635..ff6cea4334d1 100644 --- a/ui/desktop/src/utils/binaryPath.ts +++ b/ui/desktop/src/utils/binaryPath.ts @@ -4,6 +4,21 @@ import Electron from 'electron'; import log from './logger'; export const getBinaryPath = (app: Electron.App, binaryName: string): string => { + // Security validation: Ensure binaryName doesn't contain suspicious characters + if (!binaryName || + typeof binaryName !== 'string' || + binaryName.includes('..') || + binaryName.includes('/') || + binaryName.includes('\\') || + binaryName.includes(';') || + binaryName.includes('|') || + binaryName.includes('&') || + binaryName.includes('`') || + binaryName.includes('$') || + binaryName.length > 50) { // Reasonable length limit + throw new Error(`Invalid binary name: ${binaryName}`); + } + const isWindows = process.platform === 'win32'; const possiblePaths: string[] = []; @@ -16,8 +31,26 @@ export const getBinaryPath = (app: Electron.App, binaryName: string): string => for (const binPath of possiblePaths) { try { - if (fs.existsSync(binPath)) { - return binPath; + // Security: Resolve the path and validate it's within expected directories + const resolvedPath = path.resolve(binPath); + + // Ensure the resolved path doesn't contain suspicious sequences + if (resolvedPath.includes('..') || + resolvedPath.includes(';') || + resolvedPath.includes('|') || + resolvedPath.includes('&')) { + log.error(`Suspicious path detected, skipping: ${resolvedPath}`); + continue; + } + + if (fs.existsSync(resolvedPath)) { + // Additional security check: ensure it's a regular file + const stats = fs.statSync(resolvedPath); + if (stats.isFile()) { + return resolvedPath; + } else { + log.error(`Path exists but is not a regular file: ${resolvedPath}`); + } } } catch (error) { log.error(`Error checking path ${binPath}:`, error); diff --git a/ui/desktop/src/utils/settings.ts b/ui/desktop/src/utils/settings.ts index b6e2d53131de..977c59e9b999 100644 --- a/ui/desktop/src/utils/settings.ts +++ b/ui/desktop/src/utils/settings.ts @@ -1,4 +1,4 @@ -import { app } from 'electron'; +import { app, MenuItem } from 'electron'; import fs from 'fs'; import path from 'path'; @@ -66,9 +66,9 @@ export function createEnvironmentMenu( return [ { label: 'Enable Memory Mode', - type: 'checkbox', + type: 'checkbox' as const, checked: envToggles.GOOSE_SERVER__MEMORY, - click: (menuItem: { checked: boolean }) => { + click: (menuItem: MenuItem) => { const newToggles = { ...envToggles, GOOSE_SERVER__MEMORY: menuItem.checked, @@ -78,9 +78,9 @@ export function createEnvironmentMenu( }, { label: 'Enable Computer Controller Mode', - type: 'checkbox', + type: 'checkbox' as const, checked: envToggles.GOOSE_SERVER__COMPUTER_CONTROLLER, - click: (menuItem: { checked: boolean }) => { + click: (menuItem: MenuItem) => { const newToggles = { ...envToggles, GOOSE_SERVER__COMPUTER_CONTROLLER: menuItem.checked,