test(notifications): explicitly verify NO whitelist for custom commands

Update notification tests to clearly document and verify that:
- Custom notification commands have NO whitelist and NO validation
- ANY executable path, binary name, or arguments are allowed
- Shell pipelines with redirects and pipes are fully supported

Also rename TTS terminology to generic notification terminology:
- resetTtsState → resetNotificationState
- getTtsQueueLength → getNotificationQueueLength
- getActiveTtsCount → getActiveNotificationCount
- clearTtsQueue → clearNotificationQueue
- getTtsMaxQueueSize → getNotificationMaxQueueSize
- Add onCommandCompleted alongside deprecated onTtsCompleted

This ensures the feature remains open-ended for user flexibility.
This commit is contained in:
Pedram Amini
2026-02-01 21:52:31 -06:00
parent 6b33e72a21
commit 33de2c15ab
4 changed files with 141 additions and 65 deletions

View File

@@ -1,6 +1,11 @@
/**
* Tests for notification IPC handlers
*
* IMPORTANT: Custom notification commands have NO WHITELIST and NO VALIDATION.
* Users have full control to specify ANY command, ANY path, ANY arguments.
* This is by design - the feature supports arbitrary shell pipelines for
* maximum flexibility (e.g., fabric | 11s, tee ~/log.txt | say, etc.)
*
* Note: Notification command tests are simplified due to the complexity of mocking
* child_process spawn with all the event listeners and stdin handling.
*/
@@ -83,11 +88,11 @@ vi.mock('child_process', async (importOriginal) => {
import {
registerNotificationsHandlers,
resetTtsState,
getTtsQueueLength,
getActiveTtsCount,
clearTtsQueue,
getTtsMaxQueueSize,
resetNotificationState,
getNotificationQueueLength,
getActiveNotificationCount,
clearNotificationQueue,
getNotificationMaxQueueSize,
parseNotificationCommand,
} from '../../../../main/ipc/handlers/notifications';
@@ -96,7 +101,7 @@ describe('Notification IPC Handlers', () => {
beforeEach(() => {
vi.clearAllMocks();
resetTtsState();
resetNotificationState();
handlers = new Map();
// Reset mocks
@@ -113,7 +118,7 @@ describe('Notification IPC Handlers', () => {
afterEach(() => {
vi.clearAllMocks();
resetTtsState();
resetNotificationState();
});
describe('handler registration', () => {
@@ -193,31 +198,44 @@ describe('Notification IPC Handlers', () => {
describe('notification state utilities', () => {
it('should track notification queue length', () => {
expect(getTtsQueueLength()).toBe(0);
expect(getNotificationQueueLength()).toBe(0);
});
it('should track active notification count', () => {
expect(getActiveTtsCount()).toBe(0);
expect(getActiveNotificationCount()).toBe(0);
});
it('should clear notification queue', () => {
clearTtsQueue();
expect(getTtsQueueLength()).toBe(0);
clearNotificationQueue();
expect(getNotificationQueueLength()).toBe(0);
});
it('should reset notification state', () => {
resetTtsState();
expect(getTtsQueueLength()).toBe(0);
expect(getActiveTtsCount()).toBe(0);
resetNotificationState();
expect(getNotificationQueueLength()).toBe(0);
expect(getActiveNotificationCount()).toBe(0);
});
it('should return max queue size', () => {
expect(getTtsMaxQueueSize()).toBe(10);
expect(getNotificationMaxQueueSize()).toBe(10);
});
});
describe('notification command parsing', () => {
it('should return default command when none provided', () => {
/**
* Custom notification command parsing tests
*
* CRITICAL: These tests verify that there is NO WHITELIST and NO VALIDATION
* on custom notification commands. Users have FULL CONTROL to specify:
* - ANY executable path (absolute or relative)
* - ANY binary name
* - ANY arguments and flags
* - ANY shell pipeline (pipes, redirects, etc.)
*
* This design allows maximum flexibility for users to integrate with
* any tooling they prefer (TTS engines, AI summarizers, logging, etc.)
*/
describe('custom notification command parsing - NO WHITELIST, ANY COMMAND ALLOWED', () => {
it('should return default command (say) when none provided', () => {
const result = parseNotificationCommand();
expect(result).toBe('say');
});
@@ -232,55 +250,79 @@ describe('Notification IPC Handlers', () => {
expect(result).toBe('say');
});
it('should accept any command - user has full control', () => {
const result = parseNotificationCommand('say');
expect(result).toBe('say');
// Explicit NO WHITELIST tests - any command should be passed through unchanged
it('should NOT validate or whitelist commands - any binary name is allowed', () => {
// These would have been blocked by a whitelist - verify they pass through
expect(parseNotificationCommand('my-custom-binary')).toBe('my-custom-binary');
expect(parseNotificationCommand('totally-unknown-command')).toBe('totally-unknown-command');
expect(parseNotificationCommand('arbitrary_executable')).toBe('arbitrary_executable');
});
it('should accept custom commands with full paths', () => {
const result = parseNotificationCommand('/usr/local/bin/my-tts');
expect(result).toBe('/usr/local/bin/my-tts');
});
it('should accept commands with arguments', () => {
const result = parseNotificationCommand('say -v Alex');
expect(result).toBe('say -v Alex');
});
it('should accept command chains with pipes', () => {
const result = parseNotificationCommand('tee ~/log.txt | say');
expect(result).toBe('tee ~/log.txt | say');
});
it('should accept fabric pattern commands', () => {
const result = parseNotificationCommand(
'/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini --raw 2>/dev/null | /Users/pedram/.local/bin/11s --voice NFQv27BRKPFgprCm0xgr'
it('should NOT validate or whitelist paths - any absolute path is allowed', () => {
expect(parseNotificationCommand('/usr/local/bin/my-custom-tool')).toBe(
'/usr/local/bin/my-custom-tool'
);
expect(result).toBe(
'/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini --raw 2>/dev/null | /Users/pedram/.local/bin/11s --voice NFQv27BRKPFgprCm0xgr'
expect(parseNotificationCommand('/Users/pedram/go/bin/fabric')).toBe(
'/Users/pedram/go/bin/fabric'
);
expect(parseNotificationCommand('/opt/homebrew/bin/anything')).toBe(
'/opt/homebrew/bin/anything'
);
expect(parseNotificationCommand('/some/deeply/nested/path/to/binary')).toBe(
'/some/deeply/nested/path/to/binary'
);
});
it('should trim leading and trailing whitespace', () => {
const result = parseNotificationCommand(' say ');
expect(result).toBe('say');
it('should NOT validate or whitelist arguments - any arguments are allowed', () => {
expect(parseNotificationCommand('say -v Alex')).toBe('say -v Alex');
expect(parseNotificationCommand('cmd --flag1 --flag2=value -x -y -z')).toBe(
'cmd --flag1 --flag2=value -x -y -z'
);
expect(parseNotificationCommand('binary arg1 arg2 arg3 "quoted arg"')).toBe(
'binary arg1 arg2 arg3 "quoted arg"'
);
});
it('should accept espeak command', () => {
const result = parseNotificationCommand('espeak');
expect(result).toBe('espeak');
it('should allow shell pipelines with any commands', () => {
expect(parseNotificationCommand('tee ~/log.txt | say')).toBe('tee ~/log.txt | say');
expect(parseNotificationCommand('cmd1 | cmd2 | cmd3')).toBe('cmd1 | cmd2 | cmd3');
});
it('should accept festival command with flags', () => {
const result = parseNotificationCommand('festival --tts');
expect(result).toBe('festival --tts');
it('should allow complex command chains with redirects and pipes', () => {
const complexCommand =
'/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini --raw 2>/dev/null | /Users/pedram/.local/bin/11s --voice NFQv27BRKPFgprCm0xgr';
expect(parseNotificationCommand(complexCommand)).toBe(complexCommand);
});
it('should trim leading and trailing whitespace only', () => {
expect(parseNotificationCommand(' say ')).toBe('say');
expect(parseNotificationCommand('\t/path/to/cmd\n')).toBe('/path/to/cmd');
});
// Common TTS commands work, but are NOT special-cased or whitelisted
it('should accept common TTS commands (not because whitelisted, but because any command works)', () => {
expect(parseNotificationCommand('say')).toBe('say');
expect(parseNotificationCommand('espeak')).toBe('espeak');
expect(parseNotificationCommand('espeak-ng')).toBe('espeak-ng');
expect(parseNotificationCommand('festival --tts')).toBe('festival --tts');
expect(parseNotificationCommand('flite')).toBe('flite');
expect(parseNotificationCommand('spd-say')).toBe('spd-say');
});
// Non-TTS commands are equally valid
it('should accept non-TTS commands for logging, processing, or other purposes', () => {
expect(parseNotificationCommand('tee ~/notifications.log')).toBe('tee ~/notifications.log');
expect(parseNotificationCommand('cat >> ~/log.txt')).toBe('cat >> ~/log.txt');
expect(parseNotificationCommand('curl -X POST https://webhook.example.com')).toBe(
'curl -X POST https://webhook.example.com'
);
});
});
describe('notification queue size limit', () => {
it('should reject requests when queue is full', async () => {
const handler = handlers.get('notification:speak')!;
const maxSize = getTtsMaxQueueSize();
const maxSize = getNotificationMaxQueueSize();
// The flow is:
// 1. First call: item added to queue, processNextNotification() shifts it out to process
@@ -305,7 +347,7 @@ describe('Notification IPC Handlers', () => {
await new Promise((resolve) => setTimeout(resolve, 10));
// Verify queue is at capacity
expect(getTtsQueueLength()).toBe(maxSize);
expect(getNotificationQueueLength()).toBe(maxSize);
// Now try to add one more - should be rejected immediately
// This will resolve immediately with error because queue >= maxSize check triggers
@@ -316,7 +358,7 @@ describe('Notification IPC Handlers', () => {
expect(result.error).toContain(`max ${maxSize}`);
// Clean up - reset all notification state including clearing the queue
resetTtsState();
resetNotificationState();
});
});
});

View File

@@ -1,5 +1,8 @@
/**
* Tests for notifications preload API
*
* IMPORTANT: Custom notification commands have NO WHITELIST and NO VALIDATION.
* Users have full control to specify ANY command, ANY path, ANY arguments.
*/
import { describe, it, expect, vi, beforeEach } from 'vitest';
@@ -47,8 +50,14 @@ describe('Notification Preload API', () => {
});
});
describe('speak', () => {
it('should invoke notification:speak with text', async () => {
/**
* Custom notification command tests - NO WHITELIST, ANY COMMAND ALLOWED
*
* The speak() method executes a custom command with text piped to stdin.
* There is NO validation or whitelist - users can specify ANY command.
*/
describe('speak - custom notification command (NO WHITELIST)', () => {
it('should invoke notification:speak with text and default command', async () => {
mockInvoke.mockResolvedValue({ success: true, ttsId: 123 });
const result = await api.speak('Hello world');
@@ -57,16 +66,17 @@ describe('Notification Preload API', () => {
expect(result).toEqual({ success: true, ttsId: 123 });
});
it('should invoke notification:speak with custom command', async () => {
it('should accept ANY command - no whitelist restriction', async () => {
mockInvoke.mockResolvedValue({ success: true, ttsId: 456 });
const result = await api.speak('Hello', 'espeak');
// Any command works - not just whitelisted TTS tools
const result = await api.speak('Hello', 'my-custom-tool');
expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'Hello', 'espeak');
expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'Hello', 'my-custom-tool');
expect(result.ttsId).toBe(456);
});
it('should accept complex command chains with pipes', async () => {
it('should accept complex command chains with pipes (shell pipeline)', async () => {
mockInvoke.mockResolvedValue({ success: true, ttsId: 789 });
const complexCommand = 'tee ~/log.txt | say';
@@ -76,9 +86,10 @@ describe('Notification Preload API', () => {
expect(result.ttsId).toBe(789);
});
it('should accept commands with full paths and arguments', async () => {
it('should accept ANY absolute path with ANY arguments', async () => {
mockInvoke.mockResolvedValue({ success: true, ttsId: 111 });
// Full paths to custom binaries are allowed
const fullPathCommand =
'/Users/pedram/go/bin/fabric --pattern ped_summarize_conversational --model gpt-5-mini';
const result = await api.speak('Test', fullPathCommand);
@@ -86,6 +97,17 @@ describe('Notification Preload API', () => {
expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'Test', fullPathCommand);
expect(result.success).toBe(true);
});
it('should accept non-TTS commands like curl, tee, cat, etc.', async () => {
mockInvoke.mockResolvedValue({ success: true, ttsId: 222 });
// Non-TTS commands are equally valid
const curlCommand = 'curl -X POST -d @- https://webhook.example.com';
const result = await api.speak('notification payload', curlCommand);
expect(mockInvoke).toHaveBeenCalledWith('notification:speak', 'notification payload', curlCommand);
expect(result.success).toBe(true);
});
});
describe('stopSpeak', () => {
@@ -99,7 +121,7 @@ describe('Notification Preload API', () => {
});
});
describe('onTtsCompleted', () => {
describe('onCommandCompleted (legacy: onTtsCompleted)', () => {
it('should register event listener and return cleanup function', () => {
const callback = vi.fn();
@@ -109,7 +131,7 @@ describe('Notification Preload API', () => {
expect(typeof cleanup).toBe('function');
});
it('should call callback when event is received', () => {
it('should call callback when notification command completes', () => {
const callback = vi.fn();
let registeredHandler: (event: unknown, notificationId: number) => void;

View File

@@ -22,7 +22,9 @@ export interface NotificationShowResponse {
*/
export interface NotificationCommandResponse {
success: boolean;
ttsId?: number; // Legacy name kept for backward compatibility
notificationId?: number;
/** @deprecated Use notificationId instead */
ttsId?: number;
error?: string;
}
@@ -59,6 +61,14 @@ export function createNotificationApi() {
* @param handler - Callback when a notification command completes
* @returns Cleanup function to unsubscribe
*/
onCommandCompleted: (handler: (notificationId: number) => void): (() => void) => {
const wrappedHandler = (_event: Electron.IpcRendererEvent, notificationId: number) =>
handler(notificationId);
ipcRenderer.on('notification:commandCompleted', wrappedHandler);
return () => ipcRenderer.removeListener('notification:commandCompleted', wrappedHandler);
},
/** @deprecated Use onCommandCompleted instead */
onTtsCompleted: (handler: (notificationId: number) => void): (() => void) => {
const wrappedHandler = (_event: Electron.IpcRendererEvent, notificationId: number) =>
handler(notificationId);

View File

@@ -1238,9 +1238,11 @@ interface MaestroAPI {
speak: (
text: string,
command?: string
) => Promise<{ success: boolean; ttsId?: number; error?: string }>;
stopSpeak: (ttsId: number) => Promise<{ success: boolean; error?: string }>;
onTtsCompleted: (handler: (ttsId: number) => void) => () => void;
) => Promise<{ success: boolean; notificationId?: number; ttsId?: number; error?: string }>;
stopSpeak: (notificationId: number) => Promise<{ success: boolean; error?: string }>;
onCommandCompleted: (handler: (notificationId: number) => void) => () => void;
/** @deprecated Use onCommandCompleted instead */
onTtsCompleted: (handler: (notificationId: number) => void) => () => void;
};
attachments: {
save: (