Skip to content

Fix migration SQL boundaries and tracking-table identifiers - #695

Merged
matteius merged 2 commits into
mainfrom
fix/sqlite-migration-boundaries
Oct 31, 2026
Merged

matteius merged 2 commits into
mainfrom
fix/sqlite-migration-boundaries

Conversation

@matteius

Copy link
Copy Markdown
Contributor

Migration validation could lose statement boundaries when quoted identifiers, comments or trigger names contained SQL-looking tokens. Configured tracking-table names were also interpolated as SQL fragments and truncated by fixed buffers.

Track quoted/comment tokens and the SQLite trigger terminator correctly before executing the migration. Quote tracking-table names as identifiers in all four statements while retaining bound version values.

  • Scope: one production file, one descriptive regression suite, existing CMake registration, and existing integration/sanitizer CI selections (five files).
  • Verified independently from main: 14 of 19 regression cases fail before the fix; all 19 pass under ASan/UBSan afterward. Tests use private temporary files and in-memory SQLite databases.
  • Covers valid quoted/long names, triggers and nested expressions, rejected statements before effects, unchanged unrelated tables, and normal apply/reload/rollback. Workflow selection validation and actionlint pass.

Test selection: ctest --test-dir build/migration-debug --output-on-failure -V -R '^test_sqlite_migration_boundaries$'.

Extracted from #693. No foundation framework, plan, inventory or generated contract changes are included. Production callers currently use a constant tracking-table name; this does not claim remotely controlled input.

Treat quoted identifiers, escaped delimiters and comments as tokens when
validating migration statements. Recognize trigger completion as ; END ;
so identifier names cannot hide a following disallowed statement. Quote
migration tracking table names with SQLite's identifier formatter instead
of interpolating them into fixed-size SQL buffers.

Add 19 real SQLite regressions for valid UP/DOWN round trips, trigger
boundaries, disallowed ATTACH rejection and tracking-name isolation.
Register the test in the existing unit build and integration/sanitizer CI.

Validation on origin/main 8927bc8: the new fixture fails 14/19 cases before
the fix and passes 19/19 under Debug ASan/UBSan afterward. No services or
legacy shared-path test fixtures were run.
@codecov

codecov Bot commented Oct 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.68293% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/database/sqlite_migrate.c 92.68% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@matteius
matteius merged commit 8d79dd9 into main Oct 31, 2026
13 checks passed
@matteius
matteius deleted the fix/sqlite-migration-boundaries branch October 31, 2026 14:40
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