Skip to content

πŸ›‘οΈ Sentinel: [CRITICAL] Fix SQL Injection in sqliteColumnExists#110

Open
mattjoyce wants to merge 1 commit into
mainfrom
sentinel-fix-sqli-sqlite-column-exists-8740818745908940614
Open

πŸ›‘οΈ Sentinel: [CRITICAL] Fix SQL Injection in sqliteColumnExists#110
mattjoyce wants to merge 1 commit into
mainfrom
sentinel-fix-sqli-sqlite-column-exists-8740818745908940614

Conversation

@mattjoyce
Copy link
Copy Markdown
Owner

@mattjoyce mattjoyce commented May 31, 2026

🚨 Severity: CRITICAL
πŸ’‘ Vulnerability: fmt.Sprintf was used to interpolate a table name directly into an SQLite PRAGMA query in sqliteColumnExists, allowing potential SQL injection.
🎯 Impact: An attacker could potentially manipulate table names to execute arbitrary SQL or extract sensitive data.
πŸ”§ Fix: Replaced the interpolated PRAGMA statement with a parameterized query using the pragma_table_info(?) table-valued function. Extracted necessary columns explicitly to match the Scan logic, properly quoting the "notnull" keyword.
βœ… Verification: Ran go test -v ./... to ensure no regressions and verified correct column selection and mapping.


PR created automatically by Jules for task 8740818745908940614 started by @mattjoyce

Summary by CodeRabbit

  • Bug Fixes

    • Refactored SQLite schema metadata query execution to utilize parameterized parameter binding instead of direct string composition, improving robustness of database interactions.
  • Documentation

    • Added documentation detailing security enhancements to database query mechanisms and safer approaches for parameter handling in database operations.

Replaced unsafe string interpolation `fmt.Sprintf("PRAGMA table_info(%s);", table)` with a parameterized query using SQLite's `pragma_table_info(?)` table-valued function. Extracted the needed columns explicitly, ensuring `"notnull"` is properly quoted.

Co-authored-by: mattjoyce <278869+mattjoyce@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

πŸ‘‹ 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 31, 2026

Deploying with Β Cloudflare Workers Β Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
βœ… Deployment successful!
View logs
ductile b995e58 Commit Preview URL

Branch Preview URL
May 31 2026, 11:51 AM

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 31, 2026

Review Change Stack

πŸ“ Walkthrough

Walkthrough

This PR fixes a SQL injection vulnerability in SQLite schema metadata queries. The fix replaces unsafe string interpolation in a PRAGMA table_info call with a parameterized query using pragma_table_info(?), and documents the security issue and resolution.

Changes

SQL Injection Prevention in SQLite Schema Queries

Layer / File(s) Summary
Parameterized pragma_table_info query with security documentation
.jules/sentinel.md, internal/storage/sqlite.go
sqliteColumnExists now queries schema metadata using parameterized db.QueryContext against pragma_table_info(?) instead of string-interpolated PRAGMA table_info(%s). Security note documents the unsafe pattern and specifies the safer table-valued pragma_table_info(?) form.

🎯 2 (Simple) | ⏱️ ~8 minutes

A rabbit hops with glee,
No SQL injections shall be!
Parameterized queries bind tight,
Securing the schema in flight. 🐰✨

πŸš₯ Pre-merge checks | βœ… 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
βœ… Passed checks (4 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title accurately identifies the main change: fixing a critical SQL injection vulnerability in sqliteColumnExists using parameterized queries instead of string interpolation.
Linked Issues check βœ… Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check βœ… Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch sentinel-fix-sqli-sqlite-column-exists-8740818745908940614

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:
- Line 1: Update the header date on the sentinel entry for "Prevent SQL
Injection via string interpolation in PRAGMA table_info" from "2024-05-18" to
the actual PR creation date "2026-05-31" so the changelog reflects the correct
timeline; locate the heading line in .jules/sentinel.md and replace only the
date portion while preserving the rest of the header text.
πŸͺ„ 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: 090e9646-8bdb-412d-b4e5-906e5fb11d5c

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between cf887ea and b995e58.

πŸ“’ Files selected for processing (2)
  • .jules/sentinel.md
  • internal/storage/sqlite.go

Comment thread .jules/sentinel.md
@@ -0,0 +1,4 @@
## 2024-05-18 - Prevent SQL Injection via string interpolation in PRAGMA table_info
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟑 Minor | ⚑ Quick win

Correct the date to match PR creation.

The date "2024-05-18" predates this PR by over two years. Since this is a new file created in this PR (2026-05-31), the date should reflect when this fix was actually made.

πŸ“… Proposed fix
-## 2024-05-18 - Prevent SQL Injection via string interpolation in PRAGMA table_info
+## 2026-05-31 - Prevent SQL Injection via string interpolation in PRAGMA table_info
πŸ“ 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.

Suggested change
## 2024-05-18 - Prevent SQL Injection via string interpolation in PRAGMA table_info
## 2026-05-31 - Prevent SQL Injection via string interpolation in PRAGMA table_info
🧰 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 at line 1, Update the header date on the sentinel entry
for "Prevent SQL Injection via string interpolation in PRAGMA table_info" from
"2024-05-18" to the actual PR creation date "2026-05-31" so the changelog
reflects the correct timeline; locate the heading line in .jules/sentinel.md and
replace only the date portion while preserving the rest of the header text.

mattjoyce added a commit that referenced this pull request Jun 7, 2026
…on card

#110 captures the proven notify model (on-event + on-hook, real Discord posts under enforce), the
bucket classification, and the 2 findings (notify_on_complete gating; each notify route needs its own
vault_principal'd instance β€” corrects the trivial-carry assumption). Epic #83 records Phase 3:
discord_notify + ap_canary vault-native + fail-closed live, full pipeline model proven, in-binary
hardening (#100/#101/#104/#108) redeployed, plugin code @ a1934e5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant