Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 15 additions & 16 deletions ui/desktop/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand Down Expand Up @@ -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 || '',
};
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 5 additions & 4 deletions ui/desktop/src/components/more_menu/MoreMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
);
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ export default function ExtensionsSection({
{deepLinkConfigStateVar && showEnvVarsStateVar && (
<ExtensionModal
title="Add custom extension"
initialData={extensionToFormData({ ...deepLinkConfig, enabled: true })}
initialData={extensionToFormData({ ...deepLinkConfig, enabled: true } as FixedExtensionEntry)}
onClose={handleModalClose}
onSubmit={handleAddExtension}
submitLabel="Add Extension"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ import { useConfig } from '../../../ConfigContext';
import { AlertTriangle } from 'lucide-react';
import { getCurrentModelAndProvider } from '../../models'; // Import the utility

interface FormValues {
[key: string]: string | number | boolean | null;
}

const customSubmitHandlerMap: Record<string, unknown> = {
provider_name: OllamaSubmitHandler, // example
};
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
98 changes: 81 additions & 17 deletions ui/desktop/src/goosed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
Expand All @@ -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 = {
Expand All @@ -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<string, string>)
};
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);

Check failure

Code scanning / Semgrep OSS

Command Injection via child_process

Command Injection via child_process

// Only unref on Windows to allow it to run independently of the parent
if (isWindows) {
Expand Down Expand Up @@ -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?.();
}
Expand All @@ -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?.();
}
Expand Down
4 changes: 2 additions & 2 deletions ui/desktop/src/sessionLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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', {
Expand Down
37 changes: 35 additions & 2 deletions ui/desktop/src/utils/binaryPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] = [];
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions ui/desktop/src/utils/settings.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { app } from 'electron';
import { app, MenuItem } from 'electron';
import fs from 'fs';
import path from 'path';

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down