Skip to content
Open
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
18 changes: 15 additions & 3 deletions src/notification.behavior.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,14 @@ describe('NotificationPlugin Behavior', () => {
process.platform === 'darwin' ? '/System/Library/Sounds/Glass.aiff' : undefined;
expect(fallback).toBeUndefined();
});

it('should use system sound for Windows fallback', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });
const fallbackCommand = `powershell -Command "[System.Media.SystemSounds]::Exclamation.Play()"`;
expect(fallbackCommand).toBe(
'powershell -Command "[System.Media.SystemSounds]::Exclamation.Play()"',
);
Comment on lines +240 to +243

Copilot AI Feb 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should verify that SystemSounds.Exclamation.Play() uses PlaySync() instead of Play() to match the correct implementation. The asynchronous Play() method will cause the sound to be cut off when the PowerShell process exits immediately.

Copilot uses AI. Check for mistakes.
});
});

describe('Sound command construction', () => {
Expand All @@ -257,10 +265,14 @@ describe('NotificationPlugin Behavior', () => {
expect(fallbackCommand).toBe('aplay /path/to/sound.wav');
});

it('should have no command for Windows', () => {
it('should use PowerShell for Windows', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });
const hasCommand = process.platform === 'darwin' || process.platform === 'linux';
expect(hasCommand).toBe(false);
const soundPath = 'C:\\Users\\Test\\sound.wav';
const escapedPath = soundPath.replace(/'/g, "''");
const command = `powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
expect(command).toBe(
'powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new(\'C:\\Users\\Test\\sound.wav\').Play()"',
);
Comment on lines +272 to +275

Copilot AI Feb 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test contains the same incorrect assembly reference issue as the implementation. The test should verify the correct PowerShell command without the unnecessary Add-Type -AssemblyName System.Windows.Forms; part, and should use PlaySync() instead of Play().

Copilot uses AI. Check for mistakes.
});

Copilot AI Feb 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test verifies the command construction but doesn't actually test path escaping edge cases that could lead to command injection vulnerabilities. The test uses a simple Windows path without special characters.

Consider adding tests for paths with special PowerShell characters like single quotes, backticks, dollar signs, semicolons, etc. to ensure the escaping logic is robust. For example:

  • Path with single quotes: C:\test's folder\sound.wav
  • Path with semicolons: C:\test; rm -rf \sound.wav
  • Path with backticks: C:\testfolder\sound.wav`
Suggested change
});
});
it('should escape single quotes in Windows PowerShell command', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });
const soundPath = "C:\\test's folder\\sound.wav";
const escapedPath = soundPath.replace(/'/g, "''");
const command = `powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
expect(command).toBe(
"powershell -Command \"Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('C:\\\\test''s folder\\\\sound.wav').Play()\"",
);
});
it('should handle semicolons in Windows PowerShell command', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });
const soundPath = 'C:\\test; rm -rf \\sound.wav';
const escapedPath = soundPath.replace(/'/g, "''");
const command = `powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
expect(command).toBe(
"powershell -Command \"Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('C:\\\\test; rm -rf \\\\sound.wav').Play()\"",
);
});
it('should handle backticks and dollar signs in Windows PowerShell command', () => {
Object.defineProperty(process, 'platform', { value: 'win32' });
const soundPath = 'C:\\test`folder\\$sound.wav';
const escapedPath = soundPath.replace(/'/g, "''");
const command = `powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
expect(command).toBe(
"powershell -Command \"Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('C:\\\\test`folder\\\\$sound.wav').Play()\"",
);
});

Copilot uses AI. Check for mistakes.
});

Expand Down
8 changes: 6 additions & 2 deletions src/notification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ export const NotificationPlugin: Plugin = async (ctx) => {
await $`aplay ${soundPath}`;
}
} else if (process.platform === 'win32') {
log.warn('Windows sound playback not yet supported', { soundPath });
// Use PowerShell with SoundPlayer - works on all Windows versions
// Escape single quotes in path for PowerShell
const escapedPath = soundPath.replace(/'/g, "''");
await $`powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
Comment on lines +162 to +164

Copilot AI Feb 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path escaping is incomplete and could lead to command injection vulnerabilities. The current implementation only escapes single quotes, but PowerShell has many other special characters that could break the command context, including backticks (`), dollar signs ($), double quotes ("), and semicolons (;).

For example, a path like C:\test'; Stop-Process -Name powershell; ' would be escaped to C:\test''; Stop-Process -Name powershell; '' which still allows command injection.

Consider using a more robust escaping approach or passing the path as a parameter to avoid injection risks. One safer alternative would be to use the -File parameter with a temporary PowerShell script file, or use proper PowerShell parameter binding.

Suggested change
// Escape single quotes in path for PowerShell
const escapedPath = soundPath.replace(/'/g, "''");
await $`powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
// Pass the path as an argument to avoid manual escaping and injection risks
await $`powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new($args[0]).Play()" -- ${soundPath}`;

Copilot uses AI. Check for mistakes.

Copilot AI Feb 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The System.Media.SoundPlayer class is available in the default assemblies loaded by PowerShell and doesn't require explicitly loading System.Windows.Forms. The Add-Type -AssemblyName System.Windows.Forms; portion can be removed to simplify the command.

Additionally, use PlaySync() instead of Play() to ensure the sound completes before PowerShell exits. The asynchronous Play() method returns immediately, which may cause the sound to be cut off when the PowerShell process terminates.

Suggested change
await $`powershell -Command "Add-Type -AssemblyName System.Windows.Forms; [System.Media.SoundPlayer]::new('${escapedPath}').Play()"`;
await $`powershell -Command "[System.Media.SoundPlayer]::new('${escapedPath}').PlaySync()"`;

Copilot uses AI. Check for mistakes.
}
} else {
// Play fallback sound if primary sound is missing
Expand All @@ -170,7 +173,8 @@ export const NotificationPlugin: Plugin = async (ctx) => {
log.warn('Primary sound not found, using system sound', { soundPath });
await $`canberra-gtk-play --id=message`;
} else if (process.platform === 'win32') {
log.warn('Windows sound playback not yet supported', { soundPath });
log.warn('Primary sound not found, using system sound', { soundPath });
await $`powershell -Command "[System.Media.SystemSounds]::Exclamation.Play()"`;

Copilot AI Feb 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Play() method is asynchronous and returns immediately without waiting for the sound to finish. This means the PowerShell process may exit before the sound completes playing, resulting in the sound being cut off or not playing at all.

Use PlaySync() instead to ensure the sound plays completely before the PowerShell process exits.

Copilot uses AI. Check for mistakes.
}
await showMissingSoundToast(filename);
}
Expand Down
Loading