diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 00000000..3537cd81 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1 @@ +## 2026-03-31 - Prevent SQL Injection via Dictionary Unpacking in SQLite Operations\n**Vulnerability:** SQL injection vulnerability in `streamrip/db.py`'s `contains` and `remove` methods where user-provided `**items` keys were unvalidated or validated using `assert` (which is stripped under python -O), allowing arbitrary strings to be interpolated directly into SQL query text (`f"{key}=?"`).\n**Learning:** Dictionary unpacking (`**kwargs`) circumvents typical Python identifier naming constraints, meaning dictionary keys containing SQL fragments could be passed directly into SQL query generation if not rigorously validated against a schema allowlist.\n**Prevention:** Always explicitly validate dynamic keys against an allowed schema structure using a hard `raise ValueError` before incorporating them into SQL text construction, rather than relying on assertions. diff --git a/streamrip/db.py b/streamrip/db.py index a3558559..b49cf204 100644 --- a/streamrip/db.py +++ b/streamrip/db.py @@ -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}" + for key in items.keys(): + if key not in allowed_keys: + raise ValueError(f"Invalid key. Valid keys: {allowed_keys}") items = {k: str(v) for k, v in items.items()} @@ -155,6 +155,11 @@ def remove(self, **items): :param items: """ + allowed_keys = set(self.structure.keys()) + for key in items.keys(): + if key not in allowed_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}"