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-03-28 - [CRITICAL] Fix SQL injection risk in dynamic database queries
**Vulnerability:** SQL injection via bypass of Python's `assert` in `**kwargs` unpacking for dynamic query construction.
**Learning:** `streamrip/db.py` used `assert` to validate dictionary keys (`**kwargs` passed to `contains`) against allowed schema columns to build an SQL query string safely. However, `assert` statements can be optimized away entirely when Python is run with the `-O` flag. Since `**kwargs` allows any identifier, a malicious caller (or corrupted input) could bypass validation and inject arbitrary SQL column names (which are concatenated directly into the query string) if `-O` is used. Furthermore, `remove` did not validate `**kwargs` keys at all, presenting a similar SQL injection vulnerability.
**Prevention:** Explicitly validate keys against the allowed schema columns using `raise ValueError` instead of `assert` to ensure validation always runs regardless of execution flags. Ensure all dynamic query construction pathways (e.g., `remove`, `contains`) sanitize/validate their input keys.
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}"
# πŸ›‘οΈ Sentinel: Use ValueError instead of assert to prevent bypass via python -O
if not all(key in allowed_keys for key in items.keys()):
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())
# πŸ›‘οΈ Sentinel: Validate keys to prevent SQL injection via **kwargs
if not all(key in allowed_keys for key in items.keys()):
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