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
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,8 @@
**Vulnerability:** Control characters could be injected via `word_overrides` configuration, bypassing initial input sanitization.
**Learning:** Initial input sanitization is insufficient when configuration data (overrides) can re-introduce unsafe characters during processing.
**Prevention:** Implement "Output Sanitization" as a final step in data processing pipelines. Ensure sanitization logic is reusable and safe (e.g., does not unintentionally destroy formatting like trailing whitespace unless intended).

## 2025-05-21 - Security Documentation Drift
**Vulnerability:** A documented 5MB audio file size limit was missing in the actual implementation, exposing the app to DoS via memory exhaustion.
**Learning:** Security controls mentioned in documentation or developer memory may not exist or may have been regressed.
**Prevention:** Verify all documented security controls with automated tests. Trust code, not docs.
10 changes: 9 additions & 1 deletion src/chirp/audio_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import wave
from contextlib import contextmanager
from pathlib import Path
from typing import Any, Dict, Iterator, Optional, Tuple, Union
from typing import Any, Dict, Iterator, Optional

from importlib import resources

Expand All @@ -25,6 +25,9 @@
winsound = None # type: ignore[assignment]


MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024 # 5 MB


class AudioFeedback:
def __init__(
self,
Expand Down Expand Up @@ -130,6 +133,11 @@ def _play_sound(self, asset_name: str, override_path: Optional[str]) -> None:
self._logger.exception("Failed to play sound %s: %s", asset_name, exc)

def _load_and_cache(self, path: Path, key: str) -> Any:
if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES:
raise ValueError(
f"Audio file too large: {path} (max {MAX_AUDIO_FILE_SIZE_BYTES} bytes)"
)

if self._use_sounddevice:
# Load as numpy array for volume-controlled playback via sounddevice
with wave.open(str(path), "rb") as wf:
Expand Down
8 changes: 6 additions & 2 deletions tests/test_audio_feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,10 @@ def test_play_with_sounddevice_called(self, mock_sd):
@patch("chirp.audio_feedback.sd")
@patch("chirp.audio_feedback.np")
@patch("chirp.audio_feedback.wave")
def test_load_and_cache_sounddevice(self, mock_wave, mock_np, mock_sd):
@patch("pathlib.Path.stat")
def test_load_and_cache_sounddevice(self, mock_stat, mock_wave, mock_np, mock_sd):
"""_load_and_cache should read WAV and cache data."""
mock_stat.return_value.st_size = 1000 # Valid size
af = AudioFeedback(logger=self.mock_logger, enabled=True)

# Setup wave mock
Expand Down Expand Up @@ -85,8 +87,10 @@ def test_play_cached_sounddevice(self, mock_sd):
@patch("chirp.audio_feedback.np")
@patch("chirp.audio_feedback.wave")
@patch("chirp.audio_feedback.winsound", None)
def test_load_and_cache_with_volume_scaling(self, mock_wave, mock_np, mock_sd):
@patch("pathlib.Path.stat")
def test_load_and_cache_with_volume_scaling(self, mock_stat, mock_wave, mock_np, mock_sd):
"""_load_and_cache should scale audio when volume < 1.0."""
mock_stat.return_value.st_size = 1000 # Valid size
import numpy as np

# Create real numpy array for testing
Expand Down
1 change: 0 additions & 1 deletion tests/test_audio_feedback_cache.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import unittest
from unittest.mock import MagicMock, patch
from pathlib import Path
import logging

from chirp.audio_feedback import AudioFeedback
Expand Down
59 changes: 59 additions & 0 deletions tests/test_audio_security.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import logging
import unittest
from pathlib import Path
from unittest.mock import MagicMock, patch

from chirp.audio_feedback import AudioFeedback, MAX_AUDIO_FILE_SIZE_BYTES


class TestAudioSecurity(unittest.TestCase):
def setUp(self):
self.mock_logger = MagicMock(spec=logging.Logger)

@patch("chirp.audio_feedback.sd")
@patch("chirp.audio_feedback.np")
@patch("chirp.audio_feedback.wave")
@patch("pathlib.Path.stat")
def test_load_and_cache_large_file_raises_error(self, mock_stat, mock_wave, mock_np, mock_sd):
"""Should raise ValueError if audio file exceeds size limit."""
# Mock file size > 5MB
mock_stat.return_value.st_size = MAX_AUDIO_FILE_SIZE_BYTES + 1

af = AudioFeedback(logger=self.mock_logger, enabled=True)
# Force use_sounddevice to true to trigger the loading logic
af._use_sounddevice = True

with self.assertRaises(ValueError) as cm:
af._load_and_cache(Path("/fake/large_file.wav"), "key")

self.assertIn("Audio file too large", str(cm.exception))

@patch("chirp.audio_feedback.sd")
@patch("chirp.audio_feedback.np")
@patch("chirp.audio_feedback.wave")
@patch("pathlib.Path.stat")
def test_load_and_cache_valid_file_proceeds(self, mock_stat, mock_wave, mock_np, mock_sd):
"""Should proceed if audio file is within size limit."""
# Mock file size <= 5MB
mock_stat.return_value.st_size = MAX_AUDIO_FILE_SIZE_BYTES

# Mock wave open stuff
mock_wf = MagicMock()
mock_wave.open.return_value.__enter__.return_value = mock_wf
mock_wf.readframes.return_value = b""
mock_wf.getnchannels.return_value = 1
mock_wf.getframerate.return_value = 16000
mock_wf.getnframes.return_value = 0

mock_np.frombuffer.return_value = MagicMock()

af = AudioFeedback(logger=self.mock_logger, enabled=True)
af._use_sounddevice = True

try:
af._load_and_cache(Path("/fake/valid_file.wav"), "key")
except ValueError:
self.fail("_load_and_cache raised ValueError unexpectedly!")

if __name__ == "__main__":
unittest.main()