🛡️ Sentinel: [CRITICAL] Fix SQL Injection in sqliteColumnExists#96
🛡️ Sentinel: [CRITICAL] Fix SQL Injection in sqliteColumnExists#96mattjoyce wants to merge 1 commit into
Conversation
Refactored sqliteColumnExists to use the parameterized table-valued function `pragma_table_info(?)` rather than dynamic string formatting with `PRAGMA table_info(%s)`, closing a potential SQL injection vector. Co-authored-by: mattjoyce <278869+mattjoyce@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. |
📝 WalkthroughWalkthroughThis PR addresses a SQL injection vulnerability in schema inspection. The ChangesSQL Injection Fix in Schema Inspection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.jules/sentinel.md:
- Around line 1-2: Change the first line to a top-level heading and add a blank
line after it: update the heading "2026-05-24 - [SQL Injection via String
Formatting in PRAGMA table_info]" to be a H1 (prefix with a single "#") and
ensure there is an empty line between that heading and the following paragraph
so the file conforms to markdownlint rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f0e5dfba-feeb-4cd4-ad93-6b4eb2194652
📒 Files selected for processing (2)
.jules/sentinel.mdinternal/storage/sqlite.go
| ## 2026-05-24 - [SQL Injection via String Formatting in PRAGMA table_info] | ||
| **Vulnerability:** Found a SQL injection risk in `internal/storage/sqlite.go` where `fmt.Sprintf("PRAGMA table_info(%s);", table)` was used to dynamically construct a schema inspection query. |
There was a problem hiding this comment.
Fix heading level and blank-line spacing to satisfy markdownlint.
Line 1 should be a top-level heading, and it should be followed by a blank line before the paragraph text.
Suggested diff
-## 2026-05-24 - [SQL Injection via String Formatting in PRAGMA table_info]
+# 2026-05-24 - [SQL Injection via String Formatting in PRAGMA table_info]
+
**Vulnerability:** Found a SQL injection risk in `internal/storage/sqlite.go` where `fmt.Sprintf("PRAGMA table_info(%s);", table)` was used to dynamically construct a schema inspection query.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 2026-05-24 - [SQL Injection via String Formatting in PRAGMA table_info] | |
| **Vulnerability:** Found a SQL injection risk in `internal/storage/sqlite.go` where `fmt.Sprintf("PRAGMA table_info(%s);", table)` was used to dynamically construct a schema inspection query. | |
| # 2026-05-24 - [SQL Injection via String Formatting in PRAGMA table_info] | |
| **Vulnerability:** Found a SQL injection risk in `internal/storage/sqlite.go` where `fmt.Sprintf("PRAGMA table_info(%s);", table)` was used to dynamically construct a schema inspection query. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.jules/sentinel.md around lines 1 - 2, Change the first line to a top-level
heading and add a blank line after it: update the heading "2026-05-24 - [SQL
Injection via String Formatting in PRAGMA table_info]" to be a H1 (prefix with a
single "#") and ensure there is an empty line between that heading and the
following paragraph so the file conforms to markdownlint rules.
…+rebase, review order code→docs
- #96 (ADR vocab sync) -> done. - #83 epic: luminary code review done (unanimous approve, zero blockers); Tier A+B folded; T7 finding (live host loads a vault -> compose-attestation active, but no accounts map -> privsep unconfined, enforce macOS-pending #95). Next: doc review. - #97: deferred non-blocking review follow-ups (T3, T5, T9, T15, vocab lint).
1 participant
🚨 Severity: CRITICAL
💡 Vulnerability: SQL Injection vector found in
sqliteColumnExists(internal/storage/sqlite.go). The functionsqliteColumnExistsdynamically interpolated a string argument directly into a SQLite query usingfmt.Sprintf("PRAGMA table_info(%s);", table).🎯 Impact: If an attacker could control the
tableinput parameter, they might be able to append additional SQL commands to be executed on the database or alter the scope of the original check (e.g., executing); DROP TABLE ...).🔧 Fix: Refactored the function to utilize SQLite's safer table-valued function alternative. By querying
SELECT cid, name, type, "notnull", dflt_value, pk FROM pragma_table_info(?)with a bound parameter, SQL syntax structure is strictly enforced, and string inputs are correctly escaped by the database driver. The"notnull"column was appropriately quoted because it is a SQL reserved keyword.✅ Verification: The codebase test suite successfully passes after applying this fix.
This commit fulfills Sentinel's daily security fix process.
PR created automatically by Jules for task 3263817608207373134 started by @mattjoyce
Summary by CodeRabbit