Skip to content

Conversation

wqxoxo
Copy link

@wqxoxo wqxoxo commented Sep 21, 2025

Summary

Implements SQLite STRICT tables and security hardening for developer/testing environments as requested in #5390.

Changes

  • Automatically appends STRICT to CREATE TABLE statements when --developer flag is set (requires SQLite 3.37.0+)
  • Enables trusted_schema=OFF and cell_size_check=ON pragmas in developer mode
  • Adds VARCHAR→TEXT and INT→INTEGER type conversions to devtools/sql-rewrite.py for STRICT compatibility
  • Includes test to verify STRICT tables are created in developer mode

Resolves #5390

@wqxoxo wqxoxo force-pushed the feat/strict-tables-security branch 9 times, most recently from 415d91c to dec2ae7 Compare September 21, 2025 14:36
@rustyrussell
Copy link
Contributor

  1. STRICT Tables: Enable for developer/testing environments (SQLite 3.37.0+)

This seems good, except that it should be enabled by the --developer flag, not random environment variables. We get much of this checking already by using Postgresql as the backend, but it's definitely an improvement.

  1. Type Safety: Automatic conversion of VARCHAR→TEXT, BIGINT→INTEGER, BIGSERIAL→INTEGER, INT→INTEGER

This is done in the wrong place. We use devtools/sql-rewrite.py to translate our SQL statements into local dialects already. The exception is the sql plugin, but that's also up to the user.

  1. Security Hardening: Enable trusted_schema=OFF, cell_size_check=ON, secure_delete=ON

We don't load untrusted databases, but trusted_schema isn't harmful. cell_size_check will slow us down, and only helps if the db is corrupted (hopefully catching it earlier), so I like that one. secure_delete is not meaningful for us, that I can tell.

  1. Enhanced Error Messages: Clear constraint violation reporting for STRICT tables

Except that doesn't matter since only developers changing the code will ever see such messages, when they add an invalid sql statement.

  1. Memory Safety: Buffer overflow protection and proper error path cleanup

Hi ChatGPT? Or maybe claude?

To be honest, this entire thing should be a several-line patch, which enables the pragmas inside db/db_sqlite3.c

@wqxoxo wqxoxo force-pushed the feat/strict-tables-security branch 2 times, most recently from b01b22e to 55927b7 Compare September 22, 2025 13:58
@wqxoxo
Copy link
Author

wqxoxo commented Sep 22, 2025

Thank you for the feedback @rustyrussell . I've reworked the implementation based on your guidance, I hope this is more aligned with what you had in mind.

@madelinevibes madelinevibes added BOUNTY! 🫰 A bounty is available for this PR 25.09.1 Point release for 25.09 labels Sep 26, 2025
@rustyrussell rustyrussell removed the 25.09.1 Point release for 25.09 label Sep 29, 2025
@wqxoxo wqxoxo force-pushed the feat/strict-tables-security branch from 55927b7 to eac29a0 Compare October 2, 2025 20:58
Enable trusted_schema=OFF and cell_size_check=ON pragmas when
running in developer mode for enhanced security and debugging.

Add STRICT keyword to CREATE TABLE statements in developer mode
for improved type safety with SQLite 3.37.0+.

Add missing VARCHAR and INT type conversions in sql-rewrite.py.

Addresses feedback from issue ElementsProject#7913.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BOUNTY! 🫰 A bounty is available for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turn on strict tables for SQLITE3 when running integration tests
3 participants