Skip to content

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

Open
mattjoyce wants to merge 1 commit into
mainfrom
sentinel-fix-sql-injection-4157878790136162875
Open

πŸ›‘οΈ Sentinel: [CRITICAL] Fix SQL injection in sqliteColumnExists#102
mattjoyce wants to merge 1 commit into
mainfrom
sentinel-fix-sql-injection-4157878790136162875

Conversation

@mattjoyce
Copy link
Copy Markdown
Owner

@mattjoyce mattjoyce commented May 26, 2026

🚨 Severity: CRITICAL
πŸ’‘ Vulnerability: The sqliteColumnExists function in internal/storage/sqlite.go was constructing a SQL query using fmt.Sprintf directly with the table string variable instead of utilizing parameterization.
🎯 Impact: If an attacker can control or manipulate the table variable that is passed into this function, they could inject arbitrary SQL commands.
πŸ”§ Fix: Replaced the fmt.Sprintf("PRAGMA table_info(%s);", table) query with the parameterized SQLite table-valued function equivalent: SELECT 1 FROM pragma_table_info(?) WHERE name = ? LIMIT 1;. This change safely parametrizes the query while also reducing iteration complexity.
βœ… Verification: Run go test -v ./... and go test -v ./internal/storage/... to verify that no core logic or tests were broken. Tests passed successfully.


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

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a SQL injection vulnerability in database column existence checks that could have allowed unauthorized SQL code injection through improperly handled table identifiers.

Review Change Stack

Replaces string interpolation with a parameterized query in `sqliteColumnExists` to prevent SQL injection vulnerabilities.

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 26, 2026

Deploying with Β Cloudflare Workers Β Cloudflare Workers

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

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
ductile 0131b85 May 26 2026, 11:50 AM

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

πŸ“ Walkthrough

Walkthrough

This PR addresses a SQL injection vulnerability in the sqliteColumnExists function by replacing string interpolation with parameterized query binding, and documents the fix in the project sentinel. The implementation now safely passes the table identifier as a bound parameter to PRAGMA table_info(?) rather than constructing the query via fmt.Sprintf.

Changes

SQL Injection Vulnerability Fix

Layer / File(s) Summary
Parameterized PRAGMA query and sentinel documentation
internal/storage/sqlite.go, .jules/sentinel.md
sqliteColumnExists replaces string-formatted PRAGMA table_info(<table>) with a parameterized PRAGMA table_info(?) query and Scan-based column detection. Sentinel documentation records the SQL injection vulnerability and parameterized mitigation approach.

🎯 2 (Simple) | ⏱️ ~12 minutes

🐰 A query once forged with careless strings,
Now safely boundβ€”no injection stings!
Parameterized praises for safer gates,
Where table names don't seal their fates. ✨

πŸš₯ 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 describes the main change: fixing a critical SQL injection vulnerability in sqliteColumnExists through parameterized queries.
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-sql-injection-4157878790136162875

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: The sentinel entry heading "## 2024-05-26 - [Fix SQL Injection
Vulnerability in sqliteColumnExists]" has the wrong year and lacks the required
blank line; update the date in that H2 to 2026-05-26 and insert a single blank
line immediately after the heading so the file starts with a proper H2 followed
by an empty line to satisfy markdownlint and correct the metadata for the
sqliteColumnExists fix.
πŸͺ„ 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: ae92b36b-62df-4677-8bff-a9cb6d0f4e12

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between eb5db7e and 0131b85.

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

Comment thread .jules/sentinel.md
@@ -0,0 +1,4 @@
## 2024-05-26 - [Fix SQL Injection Vulnerability in sqliteColumnExists]
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

Fix the sentinel entry heading metadata.

The entry is dated 2024-05-26, but this PR was opened on May 26, 2026. Line 1 also triggers the markdownlint warnings in the static analysis output because the file starts with an H2 and has no blank line after the heading.

πŸ“ Proposed fix
-## 2024-05-26 - [Fix SQL Injection Vulnerability in sqliteColumnExists]
+# 2026-05-26 - [Fix SQL Injection Vulnerability in sqliteColumnExists]
+
πŸ“ 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-26 - [Fix SQL Injection Vulnerability in sqliteColumnExists]
# 2026-05-26 - [Fix SQL Injection Vulnerability in sqliteColumnExists]
🧰 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, The sentinel entry heading "## 2024-05-26 -
[Fix SQL Injection Vulnerability in sqliteColumnExists]" has the wrong year and
lacks the required blank line; update the date in that H2 to 2026-05-26 and
insert a single blank line immediately after the heading so the file starts with
a proper H2 followed by an empty line to satisfy markdownlint and correct the
metadata for the sqliteColumnExists fix.

mattjoyce added a commit that referenced this pull request Jun 7, 2026
…#101, v1.0 line #102

Kay Γ— Victor session split: privsep is mechanism (v1.0 must be true/correct/documented),
explain is comprehension (v1.x). #99 broadened to the explain verb family (privsep + vault).
#100 promotes the T3 ResolvedAccount sum-type (the one correctness deferral) to the v1.0 line.
#101 is the cheap anti-footgun (valid != enforcing, loud at boot + config check). #102 tracks
the v1.0 ship line. Epic #83 open-list + v1.0 framing updated.
mattjoyce added a commit that referenced this pull request Jun 7, 2026
… records it + macOS deploy-vs-build note

#94 priority β†’ High, tagged v1.0: API bearer tokens are secrets and must move into the vault
before v1.0 (ADR Β§8.5), on a separate branch (vault/API loader work, not privsep). #102 v1.0 line
updated, and clarified that macOS enforce is deploy+verify (mechanism compiles/runs on Darwin as
root) rather than mechanism-pending.
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