diff --git a/README.md b/README.md index 8558995..e652453 100644 --- a/README.md +++ b/README.md @@ -16,8 +16,9 @@ Start with the document that matches the work: - Operators: [operator guide](docs/operations.md), [production deployment](docs/deployment.md), [invite codes](docs/invite-codes.md), - [Comail](docs/comail.md), and the - [account takedown runbook](docs/account-takedown-runbook.md) + [Comail](docs/comail.md), the + [account takedown runbook](docs/account-takedown-runbook.md), and the + [2026-08-18 empty-MST-node incident](docs/incident-2026-08-18-empty-mst-node.md) - Account work: [create-account flow](docs/create-account-flow.md), [account security](docs/account-security.md), and [passkeys](docs/passkeys.md) - Protocol work: [architecture](docs/architecture.md), diff --git a/build.zig.zon b/build.zig.zon index 9d837fb..9342e7e 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -5,8 +5,8 @@ .minimum_zig_version = "0.16.0", .dependencies = .{ .zat = .{ - .url = "https://tangled.org/zat.dev/zat/archive/v0.3.29.tar.gz", - .hash = "zat-0.3.29-5PuC7vruDAD--Cox4-pdVRHS7QaLBDWjmUPlmuV1Ze9_", + .url = "https://tangled.org/zat.dev/zat/archive/v0.4.3.tar.gz", + .hash = "zat-0.4.3-5PuC7jGfDAAWck7re9WVY2M0YCionu5ymA0ewyto_Ooy", }, .zqlite = .{ .url = "git+https://github.com/karlseguin/zqlite.zig?ref=master#05a88d6758753e1c63fdd45b211dde2057094b0c", diff --git a/docs/incident-2026-08-18-empty-mst-node.md b/docs/incident-2026-08-18-empty-mst-node.md new file mode 100644 index 0000000..7a70fe9 --- /dev/null +++ b/docs/incident-2026-08-18-empty-mst-node.md @@ -0,0 +1,298 @@ +# incident: the 7-byte block that took down writes + +**2026-08-18** — every `com.atproto.repo.createRecord` on `pds.zat.dev` returned 500 for +two accounts, and `com.atproto.sync.getRepo` returned 404 for the same two. The cause was +a single missing 7-byte block, planted six days earlier by a bug that had already been +fixed upstream before it ever ran here. + +Two defects compose. Neither is dangerous alone. + +| | defect | where | status | +|---|---|---|---| +| **A** | delete leaves an emptied subtree node in the tree instead of pruning the pointer | zat ≤ v0.3.18 | fixed in [`f7d0816`](#citations), shipped v0.3.19 | +| **B** | `collectBlocks` never emits an empty node's block, so it is referenced but never stored | zat, current | **still live** | + +A *created* the bad node. B *hid* it, then turned it into an outage. + +--- + +## symptoms + +``` +error failed to serve /xrpc/com.atproto.repo.createRecord: PartialTree +``` + +- `createRecord` → 500 for **any** collection, including unknown ones +- `getRepo` → 404 `RepoNotFound` +- `listRepos`, `getLatestCommit`, `describeRepo` → **fine** + +That split is the first clue. The working endpoints read the `records` and `commits` +tables. The failing ones walk the MST. The repo metadata was intact; the tree was not. + +--- + +## the tree + +birds.place, commit `8730` → `8732`, deleting `place.birds.sighting/3msvg56t43skv` +— the sole occupant of the subtree hanging off the `speciesPhoto` entry. + +``` + BEFORE (8730) AFTER (8732, zat v0.3.10) + root, layer 1 root, layer 1 + ├── h1 app.bsky.actor.profile/self ├── h1 app.bsky.actor.profile/self + │ └── node (4 entries) │ └── node (4 entries) + ├── h1 …/place.birds.speciesPhoto ├── h1 …/place.birds.speciesPhoto + │ └── node (1 entry) │ └── {"e":[],"l":null} ← empty node, + │ └── place.birds.sighting/… │ left behind + ├── h1 sh.tangled.actor.profile/self ├── h1 sh.tangled.actor.profile/self + │ └── node (1 entry) │ └── node (1 entry) + └── h1 sh.tangled.repo/birds.place └── h1 sh.tangled.repo/birds.place + + CANONICAL: the pointer should be null, + and the node should not exist at all. +``` + +The MST is content-addressed, so a node that should not exist changes every ancestor CID. +From `8732` onward the repo served a root no other implementation would compute for that +keyset. + +--- + +## defect A — the missing prune + +zat v0.3.10, `deleteFromNode`, the descent path. The entry is removed from the child, the +parent is marked dirty, and **nothing drops the now-empty child**: + +```zig +// v0.3.10 src/internal/repo/mst.zig:389-393 +const child_ref = childAtIndex(node, entryLowerBound(node.entries.items, key)); +if (try self.ensureChildNode(child_ref)) |sub| { + const prev = try self.deleteFromNode(sub, layer - 1, key); + if (prev != null) node.dirty = true; // ← and that's all + return prev; +} +``` + +Current zat adds one line, plus a comment that states the stakes exactly: + +```zig +// HEAD src/internal/repo/mst.zig:382-386 +const prev = try self.deleteFromNode(sub, layer - 1, key); +if (prev != null) { + node.dirty = true; + pruneIfEmpty(child_ref); // ← f7d0816 +} +``` + +> Drop a subtree pointer whose node no longer holds anything. MST nodes are +> content-addressed, so an emptied node left in place serializes as a real block and +> changes every ancestor CID — the tree would no longer match the one that never +> contained the deleted key, breaking commit-proof inversion. +> +> — `pruneIfEmpty` doc comment, mst.zig:391-394 + +### the subtlety worth keeping + +`pruneIfEmpty` does **not** prune every childless node: + +```zig +// mst.zig:395-398 +fn pruneIfEmpty(child_ref: *?*Node) void { + const child = child_ref.* orelse return; + if (child.entries.items.len == 0 and child.left == null) child_ref.* = null; +} +``` + +The `and child.left == null` matters. A node with no entries but a surviving left child is +a legitimate **pass-through** that bridges a height gap; dropping it would promote the leaf +a level and produce a non-canonical root. zat tests both directions — that the emptied +subtree is pruned (mst.zig:1992) and that the height-gap pass-through is kept (mst.zig:2079). + +So the fully-empty encoding `{"e":[],"l":null}` — the 7 bytes `A2 6165 80 616C F6`, +CID `bafyreie5737gdxlw5i64vzichcalba3z2v5n6icifvx5xytvske7mr3hpm` — is canonical in +exactly one position: **the root of an empty repo**. Anywhere else it is corruption. + +--- + +## defect B — the block that never gets written + +zds derives the root CID and *then* collects blocks: + +```zig +// zds src/storage/store.zig:2140-2141 +const data_cid = try tree.rootCid(); +try writeMstBlocks(&tree, &mst_blocks); +``` + +`rootCid()` serializes the root, which recurses through `nodeCid` on every child. `nodeCid` +caches the encoding, **clears `dirty`, and does not emit a block** (mst.zig:492-516). That +is fine for ordinary nodes: `collectNodeBlocks` visits them afterwards and emits from the +cached encoding. The cache exists precisely to make that safe. + +An empty node is the one shape that never reaches the emit path: + +```zig +// mst.zig:518-519 +fn collectNodeBlocks(self: *Mst, node: *Node, …) MstError!void { + if (isUnloadedStub(node)) return; // ← bails here + +// mst.zig:786-788 +fn isUnloadedStub(node: *const Node) bool { + return !node.dirty and node.cid != null and node.left == null and node.entries.items.len == 0; +} +``` + +Once `nodeCid` has cleaned it, an empty node satisfies every clause: not dirty, has a CID, +no left, no entries. It is now **structurally indistinguishable from an unloaded stub** — +and skipping stubs is correct, because a stub's block is already in the store. This one's +never was. + +zat names the ambiguity in a test title of its own — *"An unloaded stub is indistinguishable +from an empty node by inspection … emptiness may only be judged after loading"* +(mst.zig:2102-2108). The delete path was hardened against it. `collectNodeBlocks` was not. + +``` + nodeCid() → empty node: dirty=false, cid set, encoded cached, NO BLOCK EMITTED + collectNodeBlocks() → isUnloadedStub() == true → return + repo_blocks → block absent + next tree walk → reader.get(cid) == null → error.PartialTree +``` + +`PartialTree` propagates out of `applyWrites` through the `else => return err` arm of +`createRecord` (src/atproto/repo.zig:49) as a 500 — which is why the collection in the +request was irrelevant. The tree could not be loaded at all. + +--- + +## timeline + +``` +2026-07-02 zat v0.3.10 released ← no pruneIfEmpty +2026-07-25 f7d0816 "mst: prune subtree nodes emptied by delete" + → shipped in v0.3.19; zds still pinned v0.3.10 +2026-08-12 15:13:07 commit 8730 + place.birds.sighting/3msvg56t43skv + 15:13:37 commit 8732 − same key → EMPTY NODE CREATED (defect A) + ... zat.dev acquires one the same way, under io.atcr.manifest/… +2026-08-13 04:54 zds 4cb15aa bumps zat v0.3.10 → v0.3.29 + defect A fixed — 13h41m too late. the bad node persists. +2026-08-18 05:14 / 05:16 last successful commits on both repos + ~05:37 first `PartialTree` — a walk finally reaches the empty node + → all writes down on zat.dev and birds.place +``` + +The gap is the interesting part. The bad node sat inert for six days because nothing had to +traverse that particular branch. Defect B guarantees the block is missing; it just takes a +walk that reaches it to convert that into an outage. + +--- + +## reproduction + +Feeding the exact 10-key set and the single delete into **zat v0.3.10** reproduces the +production commit bit-for-bit: + +``` +8732 stored (production) 830b8e14f23565d4029bb079a46be67188d81d1264a4e695301f03b963c99207 +8732 rebuilt on v0.3.10 830b8e14f23565d4029bb079a46be67188d81d1264a4e695301f03b963c99207 ✓ +canonical (fresh build) 6b68cca937ce9cf58e6047b4fa2d4ad8b831f444e5ebb157f5980bcac027bd5d +``` + +The same input on current zat produces `canonical`. The base tree at `8730` was verified +canonical first, so the delete is the sole cause. + +A replay through the **full zds write path** — real SQLite, `loadLazy`, `RepoBlockReader`, +`applyWrites` — on current code produces zero missing blocks and zero empty nodes. Defect A +is genuinely fixed here. + +--- + +## mitigation + +The empty node is a repo-independent constant, so restoring availability needed no commit +rewrite, no re-signing, and no firehose replay: + +```sql +INSERT INTO repo_blocks (did, cid, data, repo_rev) +VALUES (?, 'bafyreie5737gdxlw5i64vzichcalba3z2v5n6icifvx5xytvske7mr3hpm', + x'A2616580616CF6', NULL); +``` + +Applied to both DIDs. Both repos served again immediately. + +birds.place then **healed itself**: its bot's normal create/delete cycle on +`place.birds.sighting/*` lands in exactly that subtree, and the delete now runs through the +fixed `pruneIfEmpty`. Its served root is now identical to a canonical fresh build. + +zat.dev has not written since, so it still carries its empty node under +`io.atcr.manifest/…/l` and still serves a non-canonical root. + +### post-incident sweep + +All nine repos, walked from their served roots: + +``` +did:plc:b64lsctzqnzpv6vd4ry3qktw records=54 missing=0 empty=0 +did:plc:siv7zbedip4vcet4v67piibr records=84 missing=0 empty=0 +did:plc:mkqt76xvfgxuemlwlx6ruc3w records=384 missing=0 empty=1 ← zat.dev, non-canonical +did:plc:zbq37dlgttwafiqadqikhjgk records=2 missing=0 empty=0 +did:plc:whuetkjivofavwkqtmry5vpm records=1 missing=0 empty=0 +did:plc:w4p4bumx22vgnjmctoqjj7sy records=24 missing=0 empty=0 ← birds.place, healed +did:plc:lm5j7vhcqvmmalcosgujgkrh records=1 missing=0 empty=0 +did:plc:n3qjsl7p5xfgvirr5t7tnvdm records=0 missing=0 empty=1 ← empty repo, canonical +did:plc:lho2vo4lk3pqey4vl7zlvvvh records=0 missing=0 empty=1 ← empty repo, canonical +``` + +The two `records=0` repos have the empty node **at their root**, which is the one canonical +position for it. `collectBlocksInto` emits it there through a dedicated `self.root == null` +branch (mst.zig:466-479) — which is why those two DIDs had the block stored all along, and +why the same 7 bytes were present under other DIDs while missing under the broken ones. + +--- + +## outstanding + +- **defect B fix written, not yet released.** `collectNodeBlocks` now separates a stub from + an empty node by comparing the node's cid to the empty-node cid, and emits the 7 bytes when + they match. Two regression tests cover it — one on the closure invariant, one on the + production repair path — and both fail without the fix. Needs a zat release and a zds bump. + The fix is **self-healing**: `nodeCid` returns a stub's cid without loading it, so a write + that does not descend into the stranded branch still succeeds, and the collect pass puts + the missing block back. Any repo carrying a pre-v0.3.19 artifact repairs itself on its + next write. +- **zat.dev needs a canonical rebuild** — new root, new rev, re-signed commit. The defect B + fix restores availability but preserves the non-canonical shape; only a rebuild (or a + delete that trips `pruneIfEmpty`, which is how birds.place healed) makes the root agree + with what other implementations compute. +- Unrelated, found in the same logs: an httpz worker panic, + `access of union field 'http' while field 'retired' is active` in `collectTimedOut` + (worker.zig:1076), SIGABRT'd the process at 05:16:47. Separate availability bug, upstream. + +--- + +## citations + +Everything below was read at the versions named, not recalled. + +**zat** (`~/tangled.org/zat.dev/zat`) +- `f7d08169b9dd5afd34dbb8c6016d42ebafad763e` — *"mst: prune subtree nodes emptied by delete"*, 2026-07-25 01:56:38 -0500 +- first tag containing it: `v0.3.19`. `git merge-base --is-ancestor f7d0816 v0.3.10` → **NO** +- `git show v0.3.10:src/internal/repo/mst.zig | grep -c pruneIfEmpty` → **0** +- HEAD `src/internal/repo/mst.zig`: `pruneIfEmpty` call 385, definition 395-398; + `nodeCid` 492; `collectNodeBlocks` 518-519; `isUnloadedStub` 786-788; + `collectBlocksInto` 466-479 +- tests: empty-root reference CID 1564; subtree prune 1992; height-gap pass-through 2079; + stub-vs-empty ambiguity 2102 + +**zds** (`~/tangled.org/zat.dev/zds`) +- `src/storage/store.zig`: `loadLazy` 2111, `deleteReturn` 2134, `rootCid` 2140, + `writeMstBlocks` 2141, `repoBlockDataFrom` (DID-scoped reader) 485 +- `src/atproto/repo.zig:48` — the `else => return err` that surfaces `PartialTree` as 500 +- `build.zig.zon` pinned `zat v0.3.10` from `29fd5e2` (2026-07-02) until `4cb15aa` + (2026-08-12 23:54:39 -0500 = 2026-08-13 04:54 UTC), which bumped it to `v0.3.29` + +**production** (`zds-pds`, fly.io, `/data/zds.sqlite3`) +- birds.place `did:plc:w4p4bumx22vgnjmctoqjj7sy`, commits 8730 / 8732, revs + `3msvg56t43sku` / `3msvg63musrkw` +- stored roots `bafyreibaq3sxp4vuib5g7xzt63lua7b3okpy53hu4ds6znbfa2wpjhhd6a` (8730) and + `bafyreiedbohbj4rvmxkafg5qpgsgxztrrdmb2eteuttjkma7ao4whsmsa4` (8732) +- the missing block, verbatim from `repo_blocks`: `A2616580616CF6`, 7 bytes diff --git a/src/internal/cbor_json.zig b/src/internal/cbor_json.zig index d97aadb..d768986 100644 --- a/src/internal/cbor_json.zig +++ b/src/internal/cbor_json.zig @@ -19,6 +19,9 @@ fn writeValue(allocator: std.mem.Allocator, writer: anytype, value: zat.cbor.Val try writer.print("{{\"$bytes\":{f}}}", .{std.json.fmt(encoded, .{})}); }, .text => |text| try writer.print("{f}", .{std.json.fmt(text, .{})}), + // zat rejects NaN/Infinity at decode and asserts them unreachable at + // encode, so anything reaching here is finite and renders as JSON. + .float => |v| try writer.print("{f}", .{std.json.fmt(v, .{})}), .array => |items| { try writer.writeByte('['); for (items, 0..) |item, idx| { @@ -44,3 +47,24 @@ fn writeValue(allocator: std.mem.Allocator, writer: anytype, value: zat.cbor.Val ), } } + +test "float values render as JSON numbers" { + const a = std.testing.allocator; + const cases = [_]struct { v: f64, want: []const u8 }{ + .{ .v = 1.5, .want = "1.5" }, + .{ .v = -0.25, .want = "-0.25" }, + .{ .v = 2.0, .want = "2e0" }, + }; + for (cases) |c| { + const out = try writeAlloc(a, .{ .float = c.v }); + defer a.free(out); + // whatever the spelling, it must parse back as the same number + const parsed = try std.json.parseFromSlice(std.json.Value, a, out, .{}); + defer parsed.deinit(); + try std.testing.expectEqual(c.v, switch (parsed.value) { + .float => |f| f, + .integer => |i| @as(f64, @floatFromInt(i)), + else => return error.NotANumber, + }); + } +}