Hermes Agent Code Review — PR #12850

Verdict: Request Changes

Important security fix. One gap: the security path _is_authorized_media_path has no dedicated tests.

🔴 Critical — No Tests for _is_authorized_media_path

File: gateway/platforms/base.py:830

The _is_authorized_media_path() function is the security boundary that prevents prompt injection attacks from exfiltrating arbitrary files. It has zero dedicated tests — only the behavior change in extract_local_files is indirectly tested (if at all).

A function with this security role needs direct unit tests covering:

1. Authorized paths pass: Paths inside IMAGE_CACHE_DIR, AUDIO_CACHE_DIR, DOCUMENT_CACHE_DIR, and the screenshots dir should return True
2. Unauthorized paths fail: Paths like /etc/passwd, /home/pi/.ssh/id_rsa, or ../../etc/passwd should return False
3. Symlink traversal: A symlink pointing outside authorized dirs (if any exist on the system) should return False
4. Invalid paths: Non-existent paths, permission-denied paths, and garbage input should return False safely (not raise)

Suggested fix:

• Add a TestMediaPathAuthorization test class in tests/gateway/test_base.py with explicit test methods for each of the above cases
• Use tmp_path fixtures to create temporary cache directories for testing authorized paths

✅ Looks Good

• Clear, well-documented security boundary with explicit comment explaining the threat model
• Correct use of Path.resolve() (not expanduser) for symlink resolution — important for security
• Graceful error handling with try/except (ValueError, OSError) — paths that can't be resolved safely return False
• Correct combination: os.path.isfile(expanded) AND _is_authorized_media_path(expanded) — both conditions must pass

Reviewed by Hermes Agent