Skip to content

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

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

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

Conversation

@mattjoyce
Copy link
Copy Markdown
Owner

@mattjoyce mattjoyce commented May 27, 2026

🚨 Severity: CRITICAL
πŸ’‘ Vulnerability: A SQL injection vulnerability existed in internal/storage/sqlite.go where fmt.Sprintf was used to insert an unvalidated table name directly into a PRAGMA table_info SQL string.
🎯 Impact: If sqliteColumnExists was ever called with an untrusted table argument, an attacker could inject arbitrary SQL queries.
πŸ”§ Fix: Switched from using a direct PRAGMA statement to using SQLite's table-valued function pragma_table_info(?), which properly supports standard parameter binding (?) and entirely eliminates the injection vector. The column names were explicitly mapped to preserve compatibility. Added a critical entry to the .jules/sentinel.md journal.
βœ… Verification: Ran go test -v ./internal/storage/... successfully, confirming no regressions in SQLite database operations or schema validations.


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

Summary by CodeRabbit

  • Bug Fixes
    • Fixed a critical security vulnerability in database query handling to prevent potential injection attacks.

Review Change Stack

Replaced the unsafe string interpolation in `PRAGMA table_info(%s)` with a parameterized query using the `pragma_table_info(?)` table-valued function. This prevents SQL injection vulnerabilities if the table name is ever derived from an untrusted source. Also documented this critical finding in `.jules/sentinel.md`.

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 27, 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 6c9caaf Commit Preview URL

Branch Preview URL
May 27 2026, 11:56 AM

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. πŸŽ‰

ℹ️ Recent review info
βš™οΈ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2db76ef2-591d-48fd-a596-79f89574000f

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 1ed7ef3 and 6c9caaf.

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

πŸ“ Walkthrough

Walkthrough

This PR fixes a SQL injection vulnerability in the sqliteColumnExists function by replacing string interpolation with a parameterized SQL call using SQLite's pragma_table_info(?) function. A sentinel security note documents the issue, root cause, and prevention approach.

Changes

SQL Injection Prevention

Layer / File(s) Summary
SQL injection mitigation in sqliteColumnExists
internal/storage/sqlite.go, .jules/sentinel.md
sqliteColumnExists now queries column metadata via parameterized pragma_table_info(?) instead of string-interpolated PRAGMA table_info(%s). Security comments clarify the injection prevention intent. A sentinel entry documents the vulnerability, SQLite PRAGMA parameterization limitations, and the mitigation strategy.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A table name slipped through unguarded gates,
But now it binds where safety waits,
pragma_table_info holds its ground,
With quotes for fields, no SQL wound.

πŸš₯ 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 directly and clearly summarizes the main change: fixing a critical SQL injection vulnerability in sqliteColumnExists, which is the primary focus of the PR.
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-1210334762389177713

Warning

Review ran into problems

πŸ”₯ Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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

mattjoyce added a commit that referenced this pull request Jun 7, 2026
…o test, #105 FHS install artifact

#104 enforces the filesystem-layout ADR load-bearing invariant (age key never in a --scope config
backup) as a regression test. #105 turns the hand-run deploy-as-new into a repeatable v1.0 install
implementing the full ADR layout (/opt code, /etc/ductile/secret, /run, packaged binary+units).
mattjoyce added a commit that referenced this pull request Jun 7, 2026
Operator direction: convert fully to the new enforced/FHS ductile and migrate everything. #103
updated (all confinable, shared default, fabric last, establish via #105 packaging, 5-step migration
sequence). #106 = the unconfinable admin automation (docker/apt/perf/file_handler + notifies) gets
its own unconfined ductile instance β€” the ADR data-plane/admin split made concrete.
mattjoyce added a commit that referenced this pull request Jun 7, 2026
Enforced data-plane gateway live on :8081: vault carried, 5 keyless integrations enforced+attested
on default(1001), config/plugin lock, admission re-enabled, all wall-bites pass. #93 downgrade proven
live. Remaining carded as dev workstreams: #107 (HEAVY, secret-holders), #106 (admin instance), #108
(vault fail-open), #105 (FHS package). Old --user decommission deferred until #106+#107 land.
mattjoyce added a commit that referenced this pull request Jun 7, 2026
Idempotent root installer lays the ADR filesystem-layout package layer: service accounts +
the FHS dir skeleton (/etc/ductile + secret/ ductile-0700, /opt/ductile/plugins root-0755 world-rx,
/var/lib + /run via tmpfiles), the binary (root 0755 never-setuid), and the cap-only systemd unit.
Config/secrets/plugin-code stay operator data (runbook). Extended tmpfiles for /run/ductile. Turns
the hand-run deploy-as-new into "run the package", validated by the upcoming redeploy.
mattjoyce added a commit that referenced this pull request Jun 7, 2026
…105/#101/#108

deploy/install.sh proven idempotent + safe on the live host (no /var/lib chown, operator data
untouched, /run created, accounts intact); binary swap needs no re-lock; #101 valid-but-unconfined
warning fired live; #108 requires_vault gate in the binary (unit-test-proven).
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