🛡️ Sentinel: Fix potential DoS via large audio file loading

[Whamp]

User description

Sentinel 🛡️ identified a potential Denial of Service (DoS) vulnerability where loading a large audio file (user-provided or malicious) could exhaust system memory.

This PR adds a 5MB file size limit to AudioFeedback._load_and_cache. Files exceeding this limit are logged and skipped, returning None instead of crashing the application.

A new test file tests/test_audio_security.py has been added to verify this behavior using mocked file stats. All existing tests pass.

---

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

---

PR Type

Bug fix, Tests

---

Description

• Enforce 5MB file size limit in AudioFeedback._load_and_cache to prevent DoS attacks

• Prevents memory exhaustion from loading large audio files

• Returns None for oversized files with warning log

• Added security test suite to verify file size validation

---

Diagram Walkthrough

flowchart LR
  A["Audio File Loading"] --> B{"File Size Check"}
  B -->|"≤ 5MB"| C["Load and Cache"]
  B -->|"> 5MB"| D["Log Warning"]
  D --> E["Return None"]
  C --> F["Play Sound"]
  E --> F

Loading

File Walkthrough

	Relevant files
Security enhancement	audio_feedback.py Add file size limit validation for audio loading                  src/chirp/audio_feedback.py Added 5MB file size validation in _load_and_cache method Checks path.stat().st_size before loading audio file Logs warning and returns None for oversized files Handles OSError gracefully for inaccessible files +12/-0
Tests	test_audio_security.py Add audio file size security tests                                              tests/test_audio_security.py New test file for audio security validation Tests that files larger than 5MB are rejected Verifies wave.open is not called for oversized files Mocks file system and dependencies for isolated testing +42/-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 size validation uses path.stat().st_size before opening the file, which can be bypassed via a TOCTOU race (e.g., swapping the path/symlink between stat() and wave.open() to a much larger file), potentially still enabling memory/CPU exhaustion during load. audio_feedback.py [133-148] Referred Code # Security: Enforce 5MB limit to prevent memory exhaustion MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024 try: if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: self._logger.warning( "Audio file %s too large (max 5MB), skipping.", path ) return None except OSError: # File might not exist or be inaccessible; let downstream handle it pass 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 Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. 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: Swallowed OSError: The new except OSError: pass silently suppresses file access/stat failures (and the newly introduced None return may require downstream handling), reducing diagnosability and potentially masking edge cases. Referred Code return None except OSError: # File might not exist or be inaccessible; let downstream handle it pass 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: Logs file path: The warning log includes the full path, which may contain user-specific or otherwise sensitive filesystem details depending on deployment and should be verified as acceptable for logs. Referred Code self._logger.warning( "Audio file %s too large (max 5MB), skipping.", path ) 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
High-level	Fix potential TOCTOU race condition To prevent a Time-of-Check to Time-of-Use (TOCTOU) race condition, open the audio file first and use its file handle to check the size before reading its contents. This ensures the checked file is the same one that is loaded into memory. Examples: src/chirp/audio_feedback.py [136-147] if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: self._logger.warning( "Audio file %s too large (max 5MB), skipping.", path ) return None except OSError: # File might not exist or be inaccessible; let downstream handle it pass if self._use_sounddevice: ... (clipped 2 lines) Solution Walkthrough: Before: def _load_and_cache(self, path: Path, key: str) -> Any: MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024 try: # 1. Check file size using path metadata. if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: self._logger.warning("Audio file too large, skipping.") return None except OSError: pass # 2. Open the file by path, which could have been swapped. with wave.open(str(path), "rb") as wf: # ... read file contents into memory ... After: def _load_and_cache(self, path: Path, key: str) -> Any: MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024 try: # 1. Open the file to get a secure handle. with path.open("rb") as f: # 2. Check size using the file handle. f.seek(0, 2) # Move to the end of the file file_size = f.tell() f.seek(0) # Rewind to the beginning if file_size > MAX_AUDIO_FILE_SIZE_BYTES: self._logger.warning("Audio file too large, skipping.") return None # 3. Use the same file handle to read contents. with wave.open(f, "rb") as wf: # ... read file contents into memory ... except OSError: # Handle file not found or other I/O errors return None Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a significant TOCTOU race condition vulnerability in the new security check, which is highly relevant given the PR's explicit goal of preventing a DoS attack.	Medium
Possible issue	Log and skip inaccessible files In the except OSError block, log a warning and return None instead of silently passing, which would cause a crash later. src/chirp/audio_feedback.py [141-143] -except OSError: - # File might not exist or be inaccessible; let downstream handle it - pass +except OSError as e: + self._logger.warning( + "Audio file %s inaccessible, skipping: %s", path, e + ) + return None Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a bug where an OSError is ignored, which would cause a subsequent crash, and provides a robust fix by logging the error and returning early.	Medium
General	Extract threshold to module constant Move the MAX_AUDIO_FILE_SIZE_BYTES constant from inside the _load_and_cache method to the module level. src/chirp/audio_feedback.py [133-134] -# Security: Enforce 5MB limit to prevent memory exhaustion +# module-level MAX_AUDIO_FILE_SIZE_BYTES = 5 * 1024 * 1024 +def _load_and_cache(...): + # Security: enforce file size limit + try: + if path.stat().st_size > MAX_AUDIO_FILE_SIZE_BYTES: + ... + [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 4 __ Why: This is a valid suggestion that improves code style and maintainability by moving a method-scoped constant to the module level, which is better practice.	Low
More