lightning icon indicating copy to clipboard operation
lightning copied to clipboard

db: implement SQLite STRICT tables and security hardening

Open wqxoxo opened this issue 3 months ago • 2 comments

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 avatar Sep 21 '25 10:09 wqxoxo

  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

rustyrussell avatar Sep 22 '25 00:09 rustyrussell

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.

wqxoxo avatar Sep 22 '25 14:09 wqxoxo