F-04 (High) + F-14: Append-only triggers and row-validity checks#4
Conversation
The library issues no UPDATE or DELETE, but nothing prevented them: the consumer's Durable Object holds the raw SqlStorage handle by design, so one stray UPDATE from application code could rewrite closed periods with no trace and no error. Row validity was also unconstrained on pluts_amounts (a row with type 'Debit' or a float amount would be silently excluded from every sum) and on pluts_entries.date. - BEFORE UPDATE/DELETE triggers on pluts_entries, pluts_amounts, and pluts_entry_keys RAISE(ABORT) — corrections must be reversing entries. The entries UPDATE trigger is column-scoped to the financial fields so future housekeeping columns stay backfillable. - BEFORE INSERT validation triggers enforce kind in (debit, credit), integer non-negative amounts, and yyyy-mm-dd dates on existing databases; matching CHECK constraints added to the CREATE TABLE definitions for fresh databases. - README porting notes: enable PRAGMA foreign_keys on plain SQLite (defaults OFF; Durable Objects enforce it already), and don't split SCHEMA_SQL on ';' now that trigger bodies contain semicolons. - Schema tests run the real DDL against SQLite via node:sqlite, with a minimal ambient type declaration (the project compiles against workers-types, not @types/node). Audit findings F-04 (High) and F-14 (Low). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09c6e10801
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
With SQLite's default recursive_triggers = OFF, INSERT OR REPLACE resolves a primary-key conflict by deleting the existing row WITHOUT firing the DELETE trigger, then inserting the replacement — so raw consumer SQL could silently rewrite a posted amount, entry, or key mapping despite the append-only triggers (caught by Codex review). BEFORE INSERT triggers fire before conflict resolution, so an existence guard on the primary key closes the path on all three tables. The entry-keys guard keeps "UNIQUE constraint failed" in its message: the repository's concurrent-post recovery string-matches that phrase when two posts race on the same idempotency key. Reproduced first as three failing tests against node:sqlite; the tests also pin that the original rows survive the attempt and that plain duplicate-key inserts still surface as unique-constraint failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77f5f5a4b8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The table CHECK and validate-insert trigger only matched the digit pattern, so raw SQL could store impossible dates like 2026-02-30 or 2026-13-01 — rows that lexicographic date-range filters then mis-bucket or silently drop from period reports. Both guards now also require date(x) round-tripping: SQLite normalizes 2026-02-30 to 2026-03-02 (breaking equality) and returns NULL for month 13, so only real calendar dates pass. Addresses PR #4 review (P2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…g test Main merged F-11 (repository test suite, PR #10) and F-16 (frozen entry arrays, PR #11) independently; combined, the in-memory aliasing test's push onto entry.debitAmounts now throws TypeError on the frozen array, failing on main itself and on every PR's merge CI. The freeze strengthens the invariant the test checks, so it now asserts the push throws and that later reads still come back clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71a4a619bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The no_replace triggers only checked the TEXT primary key, but all three append-only tables are rowid tables: raw SQL could INSERT OR REPLACE reusing an existing row's rowid with a different id, conflict on rowid instead, and (with recursive_triggers OFF) silently rewrite the row past the DELETE trigger. Each guard now also aborts when NEW.rowid matches an existing row. Auto-assigned rowids surface as NEW.rowid = -1 inside a BEFORE INSERT trigger, which matches no real row, so normal inserts are unaffected — verified against node:sqlite. Addresses PR #4 review (P2, rowid REPLACE). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 083f69fdcc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Raw SQL could store an amount above Number.MAX_SAFE_INTEGER — fine as a SQLite 64-bit integer, but unreadable across the SqlStorage JS-number boundary: every later read or SUM would throw in fromStorageInt, taking reports down. The fresh-table CHECK and the validate-insert trigger now enforce amount <= 9007199254740991, matching the ceiling the write path already guarantees via toStorageInt. Addresses PR #4 review (P2, unsafe amount integers). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb2b96b8db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The rowid REPLACE guards read NEW.rowid = -1 as SQLite's auto-assigned sentinel, but raw SQL can store a real row at rowid -1 — after which the sentinel matches an existing row and every ordinary insert aborts as append-only. AFTER INSERT triggers see the actually-assigned (always positive when automatic) rowid, so each guarded table now aborts any insert that lands on a negative rowid, making the poisoned state unstorable without touching normal inserts. Addresses PR #4 review (P2, rowid -1 poisoning). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c741c09e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The entries UPDATE trigger is column-scoped (UPDATE OF ...) so future migration columns stay backfillable — but UPDATE OF never matches a rowid assignment, so raw SQL could run UPDATE pluts_entries SET rowid = -1, bypassing every append-only trigger and poisoning the auto-rowid sentinel: all subsequent ordinary posts aborted. A companion trigger now fires on every UPDATE and aborts when the rowid changes. Amounts and entry_keys use bare UPDATE triggers and were already covered (verified by probe). Addresses PR #4 review (P2, rowid-only UPDATE). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c83029cc1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Databases provisioned before the append-only triggers can already hold rows at rowid -1; the AFTER INSERT negative-rowid guard is not retroactive, so such a row makes the no_replace sentinel match on every ordinary insert — bricking posts — while the new UPDATE triggers block any in-place repair. migrate() now reassigns negative rowids to fresh positive ones before the DDL loop creates the triggers. Safe: nothing references rowid (FKs use TEXT ids), and re-migrating a guarded database finds no negatives so no guarded UPDATE ever runs. Addresses PR #4 review (P2, legacy rowid -1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Findings
F-04 (High): append-only was convention, not enforcement. The library itself contains no
UPDATE/DELETE— but the deployment model hands the consumer the raw SQL handle (migrate(ctx.storage.sql)runs in their DO), so a strayUPDATE pluts_amounts SET amount = ...in application code rewrites the financial record silently: no trace, no error, and if it keeps the entry balanced, no trial-balance signal. GAAP-grade auditability requires corrections via reversing entries that leave a trail.F-14 (Low):
pluts_amounts.typeandamounthad no CHECK constraints, andpluts_entries.dateno format check. A row inserted withtype = 'Debit'via non-library SQL is silently excluded from everyWHERE type = 'debit'aggregate — quietly corrupting sums instead of failing loudly.Fix
pluts_entries,pluts_amounts,pluts_entry_keys:BEFORE UPDATE/BEFORE DELETE→RAISE(ABORT). The entries UPDATE trigger is column-scoped to the financial fields (id, description, date, posted_at) so future non-financial housekeeping columns (e.g. a journal-sequence backfill) remain writable.debit|credit, amount must be a non-negative integer (typeof(amount) = 'integer'also blocks float contamination at the DB level), date must beyyyy-mm-dd. Triggers apply to already-provisioned databases, where SQLite cannot retrofit CHECKs; equivalent CHECK constraints added to theCREATE TABLEdefinitions for fresh databases.PRAGMA foreign_keys = OFF(DO storage enforces FKs already) — ports must enable it; andSCHEMA_SQLmust not be split naively on;now that trigger bodies contain semicolons (migrate()is unaffected — it executes each statement whole).Testing
New
schema-hardening.spec.tsruns the real DDL against SQLite via Node's built-innode:sqlite(same engine family as DO storage): 9 tests written first and watched fail — UPDATE/DELETE blocked on all three tables, invalid kind / negative / float amounts rejected, malformed dates rejected, valid rows still accepted, schema still idempotent. Includes a minimal ambient type declaration fornode:sqlitesince the project compiles against@cloudflare/workers-types, not@types/node.Full suite: 89 passed.
tsc --noEmitandbiome checkclean.🤖 Generated with Claude Code