fix(localdb): refuse cascading-FK parents at registration; make an entry's row+ledger writes atomic (0.3.3, ScadaBridge arch-review P4-7)

Two latent apply-path hazards from the ScadaBridge adversarial architecture
review. Neither has a live trigger in any current consumer — ScadaBridge's ten
replicated tables carry no foreign keys at all (verified against the running rig's
site-localdb.db) and OtOpcUa's LocalDb tables carry none either — but both are the
kind that surface as silent divergence rather than as an error, so they are closed
before a consumer's schema grows into them.

Wire-compatible: no proto change, no lib_schema_version bump, no on-disk schema
change. No consumer code change.

- RegisterReplicated now refuses a table that is the PARENT of a foreign key with
  a row-mutating ON DELETE action (CASCADE / SET NULL / SET DEFAULT), naming the
  offending child column and action. The apply path writes remote rows with
  INSERT OR REPLACE, whose PK-conflict resolution DELETES the existing row first;
  connections run PRAGMA foreign_keys=ON, so that implicit delete fires the child's
  ON DELETE action while the capture guard is raised. The child rows would change
  on the applying node only, produce no oplog entry, and diverge the pair with
  nothing able to detect it — LWW compares only rows it was told about. RESTRICT /
  NO ACTION stay allowed: they abort the statement instead of mutating rows behind
  capture's back, which the apply path already handles as per-entry poison. The
  check inspects every foreign key in the database and runs at registration;
  SQLite cannot add a foreign key to an existing table, so a child table created
  later is caught at the next startup's registration, not at the DDL. Documented
  on ILocalDb.RegisterReplicated and in the README's limitations list.

- An entry's row write and its ledger write are now atomic with each other. They
  are two statements and SQLite aborts only the failing one, so a statement-level
  fault between them — the case the per-entry poison catch exists for — committed
  a half-applied entry: a tombstone whose table row was gone while
  __localdb_row_version still called it live (SnapshotStreamer can only warn and
  skip that, so the peer never learns the row died), or an upsert whose new row
  carried the OLD version and therefore lost the next comparison it should have
  won. The pair now runs inside a per-entry SAVEPOINT, rolled back (and released,
  so nothing nests) before dead-lettering, leaving both writes or neither while
  the rest of the batch proceeds and commits.

Every new test was run against the unfixed code first and fails there with exactly
the divergence shape described: the tombstone case reads back null instead of the
live row, the upsert case reads back the remote row against an unchanged ledger,
and the registration cases throw nothing.

Suite: 213/213 green, build 0 warnings / 0 errors.
This commit is contained in:
Joseph Doherty
2026-08-20 08:40:14 -04:00
parent e9dc13689a
commit 122940c0e4
7 changed files with 329 additions and 10 deletions
+35 -2
View File
@@ -10,16 +10,40 @@ Change capture uses SQLite triggers writing a single HLC-stamped oplog inside th
consumer's own write transaction (crash-consistent, non-bypassable). Convergence is
last-writer-wins over a hybrid logical clock. All timestamps are UTC.
## Packages (all `0.3.2`)
## Packages (all `0.3.3`)
| Package | Contents |
|---|---|
| `ZB.MOM.WW.LocalDb` | Embedded DB host: `ILocalDb` SQL surface (`ExecuteAsync` / `QueryAsync` / transactions), `RegisterReplicated(table[, baselineExistingRows])` (PK validation + trigger install + oplog schema), `DeregisterReplicated(table)` (trigger removal + oplog/ledger prune), `HybridLogicalClock`, `AddZbLocalDb()` DI. |
| `ZB.MOM.WW.LocalDb` | Embedded DB host: `ILocalDb` SQL surface (`ExecuteAsync` / `QueryAsync` / transactions), `RegisterReplicated(table[, baselineExistingRows])` (PK + BLOB + foreign-key validation, trigger install, oplog schema), `DeregisterReplicated(table)` (trigger removal + oplog/ledger prune), `HybridLogicalClock`, `AddZbLocalDb()` DI. |
| `ZB.MOM.WW.LocalDb.Contracts` | `localdb_sync.v1` proto + generated gRPC types (packable; a future peer could be non-.NET). |
| `ZB.MOM.WW.LocalDb.Replication` | gRPC sync engine: passive `LocalDbSyncService`, initiating `SyncBackgroundService`, `MaintenanceBackgroundService`, LWW apply, watermark acks + pruning, snapshot resync, `ISyncStatus`, `LocalDbMetrics`, `AddZbLocalDbReplication()` / `MapZbLocalDbSync()`. |
> Status: **published to the Gitea feed; adopted by ScadaBridge (Phases 1+2) and OtOpcUa (Phase 1).**
>
> - `0.3.3` (2026-08-20) — two latent apply-path hazards from the ScadaBridge adversarial
> architecture review (P4-7). Neither had a live trigger in any current consumer; both are the
> kind that surface as silent divergence, not as an error. Wire-compatible: no proto change, no
> `lib_schema_version` bump, no on-disk schema change.
> - **A replicated table may no longer be the parent of a row-mutating `ON DELETE` foreign key.**
> The apply path writes remote rows with `INSERT OR REPLACE`, and REPLACE resolves a PK conflict
> by DELETING the existing row first. Connections run `PRAGMA foreign_keys=ON`, so that implicit
> delete fires the child's `ON DELETE CASCADE` / `SET NULL` / `SET DEFAULT` — while the capture
> guard is raised. The child rows change on the applying node ONLY, produce no oplog entry, and
> the pair diverges with nothing able to detect it (LWW compares only rows it was told about).
> `RegisterReplicated` now inspects every foreign key in the database and refuses such a table
> with a `LocalDbRegistrationException` naming the offending child column and action.
> `RESTRICT` / `NO ACTION` remain fine — they abort the statement instead of mutating rows
> behind capture's back, which the apply path already handles as per-entry poison. Checked at
> registration; SQLite cannot add a foreign key to an existing table, so a child table created
> later is caught at the next startup's registration rather than at the DDL.
> - **An entry's row write and its ledger write are now atomic with each other.** They are two
> statements, and SQLite aborts only the failing one — so a statement-level fault between them
> (the case the per-entry poison catch exists for) committed a half-applied entry: a tombstone
> whose table row was gone while `__localdb_row_version` still called it live, which the snapshot
> streamer can only warn about and skip, so the peer never learned the row died; or an upsert
> whose new row carried the OLD version and therefore lost the next comparison it should have
> won. The pair is wrapped in a per-entry `SAVEPOINT` that is rolled back before dead-lettering,
> so a poison entry leaves both writes or neither and the rest of the batch is untouched.
> - `0.3.2` (2026-08-19) — two Medium findings from the ScadaBridge adversarial architecture review
> (P4-1, P4-3). Wire-compatible with every earlier peer: no proto change, no `lib_schema_version`
> bump, no on-disk schema change — the keepalive is transport-level and the rest is local policy.
@@ -260,6 +284,15 @@ All options are validated at startup (`ZB.MOM.WW.Configuration` validators).
`RegisterReplicated`, because `json_object` cannot capture it). A *typeless* column can
still hold a BLOB value at runtime, which then surfaces as a write-time error, not a
registration error.
- **A replicated table may not be the parent of a row-mutating `ON DELETE` foreign key**
(`CASCADE` / `SET NULL` / `SET DEFAULT` from any other table, or from itself). Applying a
remote row uses `INSERT OR REPLACE`, whose implicit delete fires that action with change
capture suppressed — the child rows would change on the applying node only and never
replicate, diverging the pair undetectably. `RegisterReplicated` refuses such a table
(`0.3.3`); use `ON DELETE NO ACTION`/`RESTRICT` and delete children explicitly, or leave
the parent unreplicated. The check runs at registration and reads the whole database's
foreign keys, so a child table created *after* registration is caught at the next startup,
not at the moment of the `CREATE TABLE`.
- **CDC semantics** — rows that existed **before** `RegisterReplicated` are not captured
and are not snapshotted; only changes after registration are replicated. **Opt out with
`RegisterReplicated(table, baselineExistingRows: true)`**, which seeds the row-version