🛡️ Sentinel: [CRITICAL] Fix DuckDB SQL injection via f-strings - #112
🛡️ Sentinel: [CRITICAL] Fix DuckDB SQL injection via f-strings#112aperetti wants to merge 1 commit into
Conversation
- Replaced unsafe f-string concatenation of user inputs (`start_time` and `end_time`) with standard SQL parameterization `?` in `DuckDBMeterDataRepository`. - Applied fix across all data access methods inside the adapter, including wrapper queries like `EXPLAIN ANALYZE`. - Refactored stale test assertions in `test_duckdb_adapter.py` that were improperly coupled to old query parameter signatures. - Appended a critical security entry to `.jules/sentinel.md` documenting this finding and prevention protocol. Co-authored-by: aperetti <482392+aperetti@users.noreply.github.com>
|
👋 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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: User-provided inputs (
start_timeandend_time) were directly interpolated into DuckDB SQL queries via f-strings inDuckDBMeterDataRepository. String replacement (like.replace("'", "''")) was used in an attempt to sanitize, but this is inherently unsafe and a known vector for SQL injection.🎯 Impact: Remote execution of arbitrary SQL commands, unauthorized data access, or denial of service by exploiting the analytical query endpoints on DuckDB instances.
🔧 Fix: Refactored all DuckDB
conn.execute(...)calls to use standard parameterized inputs with?placeholders, passing variables via the parameter list array (e.g.,conn.execute(..., [start_time, end_time])). Cleaned up stale, breaking test cases intest_duckdb_adapter.py. Logged finding to Sentinel security journal.✅ Verification: Executed test suite (
test_duckdb_adapter.py) against updated query format to confirm functional correctness and parameter transmission.PR created automatically by Jules for task 4880246826587057914 started by @aperetti