Skip to content
Draft
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
4 changes: 4 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
## 2024-05-18 - Prevent SQL Injection by Avoiding Assert for Validation
**Vulnerability:** The `streamrip/db.py` module relied on Python's `assert` statement to validate keyword argument keys against expected columns before unpacking them into SQL queries.
**Learning:** Python strips `assert` statements entirely when executed with optimizations enabled (`-O`). An attacker could supply malicious keyword argument keys (which bypass Python's strict identifier restrictions) leading to SQL injection in queries dynamically constructed from those keys.
**Prevention:** Never use `assert` for security-critical validation or control flow logic. Always use explicit `if` statements and raise standard exceptions like `ValueError` to ensure the validation code remains active under all runtime conditions.
110 changes: 78 additions & 32 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 8 additions & 3 deletions streamrip/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,9 @@ def contains(self, **items) -> bool:
:rtype: bool
"""
allowed_keys = set(self.structure.keys())
assert all(
key in allowed_keys for key in items.keys()
), f"Invalid key. Valid keys: {allowed_keys}"
if not all(key in allowed_keys for key in items.keys()):
# 🛡️ Sentinel: Prevent SQL injection via kwargs bypassing assert in optimized mode
raise ValueError(f"Invalid key. Valid keys: {allowed_keys}")

items = {k: str(v) for k, v in items.items()}

Expand Down Expand Up @@ -155,6 +155,11 @@ def remove(self, **items):

:param items:
"""
allowed_keys = set(self.structure.keys())
if not all(key in allowed_keys for key in items.keys()):
# 🛡️ Sentinel: Prevent SQL injection via kwargs unpacking
raise ValueError(f"Invalid key. Valid keys: {allowed_keys}")

conditions = " AND ".join(f"{key}=?" for key in items.keys())
command = f"DELETE FROM {self.name} WHERE {conditions}"

Expand Down