🛡️ Sentinel: [MEDIUM] Fix unbounded audio file read (DoS risk)

[Whamp]

User description

Enforced a 5MB size limit on audio files loaded by AudioFeedback to prevent memory exhaustion (DoS).
Added tests/test_audio_security.py to verify the fix and updated tests/test_audio_feedback.py to mock Path.stat where necessary.

---

PR created automatically by Jules for task 15511094910477066961 started by @Whamp

---

PR Type

Bug fix, Tests

---

Description

• Enforces 5MB size limit on audio files to prevent DoS attacks

• Adds security check in _load_and_cache method before file loading

• Introduces new test suite test_audio_security.py for validation

• Updates existing tests to mock Path.stat for proper isolation

---

Diagram Walkthrough

flowchart LR
  A["Audio File Loading"] --> B["Size Check"]
  B --> C{Size <= 5MB?}
  C -->|Yes| D["Load File"]
  C -->|No| E["Raise ValueError"]
  F["Security Tests"] --> G["Verify Rejection"]

Loading

File Walkthrough

	Relevant files
Bug fix	audio_feedback.py Add file size limit validation for audio loading                  src/chirp/audio_feedback.py Added MAX_AUDIO_FILE_SIZE_BYTES constant set to 5MB Implemented file size validation in _load_and_cache method Raises ValueError if audio file exceeds size limit Removed unused imports (Tuple, Union) from type hints +10/-1
Tests	test_audio_feedback.py Mock Path.stat for audio loading tests                                      tests/test_audio_feedback.py Added Path.stat mocking in test_load_and_cache_sounddevice test Added Path.stat mocking in test_load_and_cache_with_volume_scaling test Ensures tests pass with new size validation check +8/-2      test_audio_security.py New security tests for audio file limits                                  tests/test_audio_security.py New test file for audio security validation test_rejects_large_files verifies rejection of files exceeding 5MB limit Mocks Path.stat to simulate oversized files Validates error message contains "exceeds size limit" +44/-0
Formatting	test_audio_feedback_cache.py Remove unused import                                                                          tests/test_audio_feedback_cache.py Removed unused Path import from file +0/-1
Documentation	sentinel.md Document audio file DoS vulnerability lesson                          .jules/sentinel.md Added documentation entry for unbounded resource loading vulnerability Documented learning about DoS risks in local apps with file loading Recorded prevention strategy using file size limits +5/-0

---

[google-labs-jules]

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.

---

For security, I will only act on instructions from the user who triggered this task.

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	TOCTOU size check Description: The new size check uses path.stat().st_size and then opens the file, which can be bypassed via a TOCTOU race (file swapped/modified after stat but before wave.open), potentially still allowing oversized reads and memory exhaustion in adversarial filesystem scenarios (e.g., symlink swap). audio_feedback.py [136-145] Referred Code # Security check: prevent loading massive files (DoS) if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: raise ValueError( f"Audio file {path} exceeds size limit of {MAX_AUDIO_FILE_SIZE_BYTES / 1024 / 1024:.1f}MB" ) if self._use_sounddevice: # Load as numpy array for volume-controlled playback via sounddevice with wave.open(str(path), "rb") as wf: samplerate = wf.getframerate()
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unhandled stat failures: The new size check calls path.stat() without handling potential OSError/permission-related failures, which may cause unexpected crashes instead of graceful degradation. Referred Code # Security check: prevent loading massive files (DoS) if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: raise ValueError( f"Audio file {path} exceeds size limit of {MAX_AUDIO_FILE_SIZE_BYTES / 1024 / 1024:.1f}MB" ) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Path disclosed in error: The raised ValueError includes the full path string, which could expose local filesystem layout details if surfaced to end users. Referred Code raise ValueError( f"Audio file {path} exceeds size limit of {MAX_AUDIO_FILE_SIZE_BYTES / 1024 / 1024:.1f}MB" ) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Fix race condition in file size check To fix a TOCTOU race condition vulnerability, open the audio file first, then check its size using the file handle before reading its contents. src/chirp/audio_feedback.py [135-145] def _load_and_cache(self, path: Path, key: str) -> Any: - # Security check: prevent loading massive files (DoS) - if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: - raise ValueError( - f"Audio file {path} exceeds size limit of {MAX_AUDIO_FILE_SIZE_BYTES / 1024 / 1024:.1f}MB" - ) + with open(path, "rb") as f: + # Security check: prevent loading massive files (DoS) by checking size after opening + f.seek(0, 2) # Move to the end of the file + file_size = f.tell() + if file_size > MAX_AUDIO_FILE_SIZE_BYTES: + raise ValueError( + f"Audio file {path} exceeds size limit of {MAX_AUDIO_FILE_SIZE_BYTES / 1024 / 1024:.1f}MB" + ) + f.seek(0) # Rewind to the beginning for reading - if self._use_sounddevice: - # Load as numpy array for volume-controlled playback via sounddevice - with wave.open(str(path), "rb") as wf: - samplerate = wf.getframerate() - nchannels = wf.getnchannels() - nframes = wf.getnframes() - audio_bytes = wf.readframes(nframes) - # ... - else: # winsound - with open(path, "rb") as f: + if self._use_sounddevice: + # Load as numpy array for volume-controlled playback via sounddevice + with wave.open(f, "rb") as wf: + samplerate = wf.getframerate() + nchannels = wf.getnchannels() + nframes = wf.getnframes() + audio_bytes = wf.readframes(nframes) + # ... + else: # winsound self._cache[key] = f.read() - return self._cache[key] + return self._cache[key] [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a Time-of-Check to Time-of-Use (TOCTOU) security vulnerability in the new file size check and provides a robust solution, significantly improving the security posture of the change.	High
More