fix(db2-delta): fold's cycle guard checks direction, not step count

- wal.c: wo_wal_fold_row_at now refuses any delta back-pointer that
  does not point strictly earlier than the record naming it
  (back_off >= cur), instead of capping total hops at off/13+1
- this is the real invariant, not a proxy for it: a step-count bound
  lets a forward-pointing back-pointer through in one hop whenever it
  happens to land on a genuine record, returning a plausible-but-wrong
  row instead of refusing it
- removes the 13-byte-record magic number entirely; no arithmetic
  tied to record framing remains in the guard
- wal.h: docblock updated to describe the direction invariant
- test_wal.c: two new tests — self-pointing back-pointer (boundary
  case, back_off == cur) and forward-pointing back-pointer to a real
  future record for the same row (the actual gap: verified failing
  against the old step-count guard, passing after the fix)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit 173dbf28a42d45a8c7f9fe430355f61f70c81935)

database/src/wal.c

@@ -877,15 +877,6 @@ int wo_wal_read_row_at(wo_wal *w, wo_db *db, wo_rt *rt, uint64_t off,
  * and compaction all call this one function — never a second copy. */
 int wo_wal_fold_row_at(wo_wal *w, wo_db *db, uint64_t off, uint32_t *class_out,
                        uint64_t *id_out, uint64_t *out_vals, const char **msg) {
-    /* Cycle guard: every legitimate back-pointer names a record already on
-     * disk before [off], so the chain cannot be longer than the number of
-     * minimal-sized records the bytes up to [off] could hold. 13 = the
-     * smallest an on-disk record can ever be (scan_record refuses len == 0,
-     * so 8-byte header + 1-byte payload + 4-byte mark). A malicious or
-     * corrupt back-pointer — even a record pointing at itself — still
-     * terminates here, loudly, instead of spinning forever. */
-    uint64_t max_steps = off / 13u + 1u;
-
     uint32_t cid = 0;
     uint64_t id = 0;
     uint32_t field_cnt = 0;
@@ -893,13 +884,9 @@ int wo_wal_fold_row_at(wo_wal *w, wo_db *db, uint64_t off, uint32_t *class_out,
     uint64_t *resolved_val = NULL; /* field idx -> its remembered engine value */
     uint64_t cur = off;
     int rc = 0;
+    int first = 1;
 
-    for (uint64_t step = 0;; step++) {
+    for (;;) {
-        if (step >= max_steps) {
-            *msg = "delta chain exceeds what the log could hold (cycle?)";
-            rc = -1;
-            break;
-        }
         uint32_t len;
         uint8_t *payload;
         if (scan_record(w->fd, cur, &len, &payload) != 0) {
@@ -917,7 +904,8 @@ int wo_wal_fold_row_at(wo_wal *w, wo_db *db, uint64_t off, uint32_t *class_out,
             rc = -1;
             break;
         }
-        if (step == 0) {
+        if (first) {
+            first = 0;
             cid = rec_cid;
             id = rec_id;
             field_cnt = db->classes[cid].field_cnt;
@@ -945,6 +933,21 @@ int wo_wal_fold_row_at(wo_wal *w, wo_db *db, uint64_t off, uint32_t *class_out,
                 rc = -1;
                 break;
             }
+            /* THE cycle/forgery guard: a back-pointer names the row's
+             * PREVIOUS record, which by construction is earlier in the
+             * (append-only) log than the delta naming it. Anything else —
+             * a self-pointer, a forward pointer, a pointer that only forms
+             * a cycle several hops later — is corruption or forgery, and
+             * this is what actually rules all of those out: not a bound on
+             * how many records could exist, which a forward pointer to a
+             * genuine record satisfies trivially and a bound would then let
+             * straight through. */
+            if (back_off >= cur) {
+                free(payload);
+                *msg = "delta back-pointer does not point earlier in the log";
+                rc = -1;
+                break;
+            }
             const wo_classdesc *c = &db->classes[cid];
             uint64_t v;
             if (dec_val(&r, db, c->kinds[field_idx], &v) != 0 ||

database/src/wal.h

@@ -294,9 +294,12 @@ int wo_wal_read_row_at(wo_wal *w, wo_db *db, wo_rt *rt, uint64_t off,
  * against EVERY record touched — a chain that disagrees about whose row it
  * is is corruption, not a new row.
  *
- * A cycle in the back-pointers is corruption, possibly malicious: the walk
+ * A back-pointer must name something STRICTLY EARLIER in the log than the
- * refuses to look at more records than the log at [off] could possibly
+ * record holding it — the row's PREVIOUS record, by construction, always
- * hold, and fails loudly instead of spinning.
+ * is. Anything else (a self-pointer, a forward pointer, corruption or
+ * forgery of any shape) is refused on the very hop that violates it, which
+ * also rules out a cycle: a walk that only ever moves to a lower offset
+ * cannot revisit one.
  *
  * 0 ok, -1 no intact/malformed/corrupt record anywhere in the chain (or a
  * REMOVE tombstone reached mid-chain), -2 out of memory (*msg set). */

runtime/test/test_wal.c

@@ -761,6 +761,101 @@ static void test_delta_fold_same_field_newest_wins(void) {
     wo_rt_destroy(&rt);
 }
 
+/* keys-resident delta updates, Task 2 (review follow-up): a back-pointer
+ * naming ITS OWN offset is the boundary case of the fold's invariant —
+ * every hop must land on a STRICTLY earlier offset than the record naming
+ * it. back_off == cur violates that on the very first hop and must be
+ * refused immediately, not walked. */
+static void test_fold_refuses_self_pointing_delta(void) {
+    char path[128];
+    snprintf(path, sizeof path, "%s/foldself.wal", g_dir);
+    wo_rt rt;
+    T_EQ(wo_rt_init(&rt, 1 << 20, DELTA_CLASSES, 1), 0);
+    wo_db db;
+    T_EQ(wo_db_init(&db, DELTA_CLASSES, 1, 0, 1), 0);
+    wo_wal w;
+    T_EQ(wo_wal_open(&w, path, 1 << 16), 0);
+    db.rt = &rt;
+    rt.wal = &w;
+    rt.db = &db;
+    const char *msg = "";
+
+    uint64_t vals[3] = {10, 20, 30};
+    uint64_t id = wo_row_insert(&db, 0, vals, &msg, NULL);
+    T_CHECK(id != 0);
+
+    /* a delta whose back-pointer names ITS OWN offset */
+    uint64_t delta_off = wo_wal_next_offset(&w);
+    T_EQ(wo_wal_append_delta(&w, &db, 0, id, 0, delta_off, 999), 0);
+    T_EQ(wo_wal_commit(&w), 0);
+    T_EQ(wo_row_drop_payload(&db, 0, id, delta_off), 0);
+
+    T_CHECK(wo_row_borrow(&db, 0, id, &msg) == NULL);
+
+    wo_wal_close(&w);
+    wo_db_destroy(&db);
+    wo_rt_destroy(&rt);
+}
+
+/* The case a mere chain-length bound cannot rule out: a back-pointer that
+ * points FORWARD to a real, valid record for the SAME row. Nothing about
+ * this loops, so a cap on chain length would let it straight through in
+ * one hop and return a plausible-but-wrong answer. Only checking that
+ * every hop moves to a STRICTLY earlier offset catches it, immediately. */
+static void test_fold_refuses_forward_pointing_delta(void) {
+    char path[128];
+    snprintf(path, sizeof path, "%s/foldfwd.wal", g_dir);
+    wo_rt rt;
+    T_EQ(wo_rt_init(&rt, 1 << 20, DELTA_CLASSES, 1), 0);
+    wo_db db;
+    T_EQ(wo_db_init(&db, DELTA_CLASSES, 1, 0, 1), 0);
+    wo_wal w;
+    T_EQ(wo_wal_open(&w, path, 1 << 16), 0);
+    db.rt = &rt;
+    rt.wal = &w;
+    rt.db = &db;
+    const char *msg = "";
+
+    /* filler row/record, same reason as test_delta_record's: a fresh WAL's
+       first record sits at offset 0, which would make the forged delta's
+       own offset indistinguishable from a zeroed field either way. */
+    uint64_t filler_vals[3] = {1, 2, 3};
+    uint64_t filler_id = wo_row_insert(&db, 0, filler_vals, &msg, NULL);
+    T_CHECK(filler_id != 0);
+    T_EQ(wo_wal_append_insert(&w, &db, 0, filler_id), 0);
+    T_EQ(wo_wal_commit(&w), 0);
+
+    uint64_t vals[3] = {10, 20, 30};
+    uint64_t id = wo_row_insert(&db, 0, vals, &msg, NULL);
+    T_CHECK(id != 0);
+
+    /* forge a delta BEFORE the row's real base record exists, naming the
+       offset the base record WILL occupy right after it. A scalar-field
+       delta payload is kind|class|id|field_idx|back_off|value(u64) = 33
+       bytes (established by test_delta_record); the frame is 8+33+4 = 45. */
+    uint64_t delta_off = wo_wal_next_offset(&w);
+    uint64_t insert_off = delta_off + 45u;
+    T_EQ(wo_wal_append_delta(&w, &db, 0, id, 0, insert_off, 999), 0);
+    T_EQ(wo_wal_commit(&w), 0);
+    T_EQ(wo_wal_next_offset(&w), insert_off); /* the hand-computed frame size held */
+
+    /* the row's TRUE base record, landing exactly where the forged delta
+       claimed — the row is still in RAM, so this is an ordinary insert-log */
+    T_EQ(wo_wal_append_insert(&w, &db, 0, id), 0);
+    T_EQ(wo_wal_commit(&w), 0);
+
+    /* point the row at the forged, forward-pointing delta */
+    T_EQ(wo_row_drop_payload(&db, 0, id, delta_off), 0);
+
+    /* the fold must refuse — not silently return {999, 20, 30} by walking
+       forward into the base record the forged back-pointer named */
+    T_CHECK(wo_row_borrow(&db, 0, id, &msg) == NULL);
+
+    wo_wal_close(&w);
+    wo_db_destroy(&db);
+    wo_rt_destroy(&rt);
+}
+
 /* databasev2 2 (5c): boot. A keys-resident store must come back from replay
  * with its rows readable FROM THE LOG — the map rebuilt to offsets, not slabs.
  * This is the half the round-trip test cannot cover: it runs in a fresh db,
@@ -1407,6 +1502,8 @@ int main(void) {
     test_delta_record();
     test_delta_fold_two_fields();
     test_delta_fold_same_field_newest_wins();
+    test_fold_refuses_self_pointing_delta();
+    test_fold_refuses_forward_pointing_delta();
     test_keys_resident_replay();
     test_keys_resident_survives_compaction();
     test_keys_resident_delete();