Skip to content

Commit 3fdffe2

Browse files
justrachclaude
andcommitted
fix: complete audit — fix all remaining 35 medium and low severity bugs
Storage layer (4 fixes): - parallel_wal: use writeAll instead of write to handle short writes - parallel_wal: add completed counter to prevent torn writes (flusher only flushes fully-written entries) - parallel_wal: remove fetchSub rollback on segment-full (eliminates race) - page: leafRead returns null instead of silently truncating Indexing subsystem (10 fixes): - codeindex: indexBatch caps at 64 entries (prevents stack overflow) - codeindex: indexBatch errdefer frees trigram slices on error - codeindex: SparseNgramIndex now dupes path strings (was use-after-free) - codeindex: SparseNgramIndex now has mutex for thread safety - codeindex: removeFileById cleans up path_to_id and id_to_path - fast_index: add mutex to FastTrigramIndex - trigram: change tombstone from 0 to maxInt(u64) (0 is valid doc_id) - disk_index: cap extractSparseNgrams loop at weights[1024] (OOB fix) - disk_index: validate mmap size in DiskIndex.open (corrupt file safety) - disk_index: add errdefer for mmap/fd cleanup on partial open failure Core features (6 fixes): - mvcc: GC rolls back on OOM instead of losing version entries - mvcc: beginRead uses cmpxchgWeak loop for atomic min_active_epoch update - auth: enabled flag is now atomic (eliminates auth bypass race window) - auth: addKey returns null when MAX_KEYS reached (was silent failure) - branch: use StringHashMap with actual key (was FNV-1a hash — collisions caused data loss) - collection: round-robin index queue assignment (prevents double-processing) CDC/query/errors (6 fixes): - server: scan/search adds "truncated":true when results exceed buffer - cdc: add events_dropped atomic counter (was silent event loss) - cdc: increase event buffers + add truncated flag - cdc: escape JSON in webhook payloads (was injection vector) - query: increase field name buffer from 256 to 1024 bytes - errors: escape detail string in JSON error responses Advanced data structures (6 fixes): - lsm: only drop tombstones at max level (prevents data resurrection) - lsm: add RwLock for flush/get synchronization (prevents torn reads) - compression: LZ4 hash table u16→u32 (fixes corruption >64KB) - art: hold write lock through prefix mismatch (closes TOCTOU window) - bwtree: epoch-based reclamation with 2-epoch grace period - turboquant: dequantize handles FWHT mode (was crash on empty slice) Replication (7 fixes): - integration: replace globals with per-instance context - integration: heap-allocate executor (was dangling ptr to optional field) - integration: bounds check in packTxnData (was stack overflow) - integration: free allocations on submit failure (was memory leak) - shard: registerPartition also updates hash ring - router: parseU16 uses checked arithmetic (was wrapping to wrong partition) - sequencer: owns_transactions flag prevents freeing stack memory Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent d9b3f7f commit 3fdffe2

24 files changed

Lines changed: 479 additions & 153 deletions

src/art.zig

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -490,9 +490,13 @@ pub const ART = struct {
490490
}
491491

492492
if (mismatch < prefix_len) {
493-
// Prefix mismatch: split the node
494-
hdr.writeUnlock();
495-
const new_node = try Node4.create(self.alloc);
493+
// Prefix mismatch: split the node.
494+
// Keep write lock held through the entire operation to prevent
495+
// TOCTOU races (another thread modifying prefix between unlock/relock).
496+
const new_node = Node4.create(self.alloc) catch |err| {
497+
hdr.writeUnlock();
498+
return err;
499+
};
496500

497501
// New node's prefix is the common part
498502
new_node.header.prefix_len = @intCast(mismatch);
@@ -508,7 +512,6 @@ pub const ART = struct {
508512
}
509513

510514
// Adjust existing node's prefix (remove consumed part)
511-
hdr.writeLock();
512515
const remaining = prefix_len - mismatch - 1;
513516
if (remaining > 0 and mismatch + 1 < MAX_PREFIX_LEN) {
514517
var dst: [MAX_PREFIX_LEN]u8 = [_]u8{0} ** MAX_PREFIX_LEN;

src/auth.zig

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -44,34 +44,36 @@ pub const AuthContext = struct {
4444
pub const AuthStore = struct {
4545
keys: [MAX_KEYS]KeyEntry = undefined,
4646
count: u32 = 0,
47-
enabled: bool = false,
47+
enabled: std.atomic.Value(bool) = std.atomic.Value(bool).init(false),
4848
lock: std.Thread.RwLock = .{},
4949

50-
/// Add an API key. Returns the BLAKE3 hash for storage.
51-
pub fn addKey(self: *AuthStore, raw_key: []const u8, name: []const u8, perm: Permission) [32]u8 {
50+
/// Add an API key. Returns the BLAKE3 hash if added, null if at MAX_KEYS.
51+
pub fn addKey(self: *AuthStore, raw_key: []const u8, name: []const u8, perm: Permission) ?[32]u8 {
5252
return self.addKeyForTenant(raw_key, name, "default", perm);
5353
}
5454

55-
pub fn addKeyForTenant(self: *AuthStore, raw_key: []const u8, name: []const u8, tenant_id: []const u8, perm: Permission) [32]u8 {
55+
/// Add an API key for a specific tenant. Returns the BLAKE3 hash if added,
56+
/// null if the key store is full (MAX_KEYS reached).
57+
pub fn addKeyForTenant(self: *AuthStore, raw_key: []const u8, name: []const u8, tenant_id: []const u8, perm: Permission) ?[32]u8 {
5658
self.lock.lock();
5759
defer self.lock.unlock();
5860

61+
if (self.count >= MAX_KEYS) return null;
62+
5963
const hash = crypto.blake3(raw_key);
60-
if (self.count < MAX_KEYS) {
61-
var entry = KeyEntry{
62-
.hash = hash,
63-
.name = undefined,
64-
.name_len = @intCast(@min(name.len, 64)),
65-
.tenant_id = undefined,
66-
.tenant_id_len = @intCast(@min(tenant_id.len, 64)),
67-
.perm = perm,
68-
};
69-
@memcpy(entry.name[0..entry.name_len], name[0..entry.name_len]);
70-
@memcpy(entry.tenant_id[0..entry.tenant_id_len], tenant_id[0..entry.tenant_id_len]);
71-
self.keys[self.count] = entry;
72-
self.count += 1;
73-
self.enabled = true;
74-
}
64+
var entry = KeyEntry{
65+
.hash = hash,
66+
.name = undefined,
67+
.name_len = @intCast(@min(name.len, 64)),
68+
.tenant_id = undefined,
69+
.tenant_id_len = @intCast(@min(tenant_id.len, 64)),
70+
.perm = perm,
71+
};
72+
@memcpy(entry.name[0..entry.name_len], name[0..entry.name_len]);
73+
@memcpy(entry.tenant_id[0..entry.tenant_id_len], tenant_id[0..entry.tenant_id_len]);
74+
self.keys[self.count] = entry;
75+
self.count += 1;
76+
self.enabled.store(true, .release);
7577
return hash;
7678
}
7779

@@ -82,7 +84,9 @@ pub const AuthStore = struct {
8284
}
8385

8486
pub fn resolve(self: *AuthStore, raw_key: []const u8) ?AuthContext {
85-
if (!self.enabled) {
87+
// Fast-path: if no keys have been added, skip hashing and locking.
88+
// Uses acquire to see the latest store from addKeyForTenant.
89+
if (!self.enabled.load(.acquire)) {
8690
return AuthContext{
8791
.perm = .admin,
8892
.tenant_id = [_]u8{0} ** 64,
@@ -108,7 +112,7 @@ pub const AuthStore = struct {
108112

109113
/// Check if auth is enabled.
110114
pub fn isEnabled(self: *AuthStore) bool {
111-
return self.enabled;
115+
return self.enabled.load(.acquire);
112116
}
113117

114118
/// Extract API key from HTTP headers.

src/branch.zig

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,12 @@ pub const Branch = struct {
2222
agent_id: [64]u8, // which agent owns this branch
2323
agent_id_len: u8,
2424

25-
// Branch-local storage: key_hash -> value (only modified keys)
25+
// Branch-local storage: actual key string -> value (only modified keys)
2626
// This is the CoW layer — unmodified keys fall through to main
27-
writes: std.AutoHashMap(u64, BranchWrite),
27+
writes: std.StringHashMap(BranchWrite),
2828
allocator: Allocator,
2929

3030
pub const BranchWrite = struct {
31-
key: []const u8, // owned copy
3231
value: []const u8, // owned copy
3332
deleted: bool, // true = tombstone (deleted on branch)
3433
epoch: u64, // when this write happened
@@ -45,28 +44,30 @@ pub const Branch = struct {
4544
pub fn deinit(self: *Branch) void {
4645
var it = self.writes.iterator();
4746
while (it.next()) |entry| {
48-
if (entry.value_ptr.key.len > 0) self.allocator.free(entry.value_ptr.key);
4947
if (entry.value_ptr.value.len > 0) self.allocator.free(entry.value_ptr.value);
48+
self.allocator.free(@constCast(entry.key_ptr.*));
5049
}
5150
self.writes.deinit();
5251
}
5352

5453
/// Write a key-value pair on this branch (CoW — only stores the delta)
5554
pub fn write(self: *Branch, key: []const u8, value: []const u8, epoch: u64) !void {
56-
const key_hash = fnv1a(key);
5755
// Allocate new copies BEFORE freeing old ones — if alloc fails,
5856
// the existing entry stays valid.
5957
const owned_key = try self.allocator.dupe(u8, key);
6058
errdefer self.allocator.free(owned_key);
6159
const owned_val = try self.allocator.dupe(u8, value);
6260
errdefer self.allocator.free(owned_val);
6361
// Free old write if exists (safe — new copies already allocated)
64-
if (self.writes.getPtr(key_hash)) |old| {
65-
if (old.key.len > 0) self.allocator.free(old.key);
62+
if (self.writes.getPtr(key)) |old| {
6663
if (old.value.len > 0) self.allocator.free(old.value);
64+
// Free the old key that was used as the map key.
65+
const old_map_key = self.writes.getKey(key).?;
66+
self.allocator.free(@constCast(old_map_key));
67+
// Remove old entry so we can insert with new owned key.
68+
_ = self.writes.remove(key);
6769
}
68-
try self.writes.put(key_hash, .{
69-
.key = owned_key,
70+
try self.writes.put(owned_key, .{
7071
.value = owned_val,
7172
.deleted = false,
7273
.epoch = epoch,
@@ -75,14 +76,14 @@ pub const Branch = struct {
7576

7677
/// Mark a key as deleted on this branch
7778
pub fn delete(self: *Branch, key: []const u8, epoch: u64) !void {
78-
const key_hash = fnv1a(key);
79-
if (self.writes.getPtr(key_hash)) |old| {
80-
if (old.key.len > 0) self.allocator.free(old.key);
79+
if (self.writes.getPtr(key)) |old| {
8180
if (old.value.len > 0) self.allocator.free(old.value);
81+
const old_map_key = self.writes.getKey(key).?;
82+
self.allocator.free(@constCast(old_map_key));
83+
_ = self.writes.remove(key);
8284
}
8385
const owned_key = try self.allocator.dupe(u8, key);
84-
try self.writes.put(key_hash, .{
85-
.key = owned_key,
86+
try self.writes.put(owned_key, .{
8687
.value = &.{},
8788
.deleted = true,
8889
.epoch = epoch,
@@ -91,8 +92,7 @@ pub const Branch = struct {
9192

9293
/// Read a key on this branch. Returns branch-local value or null (fall through to main).
9394
pub fn read(self: *const Branch, key: []const u8) ?BranchRead {
94-
const key_hash = fnv1a(key);
95-
if (self.writes.get(key_hash)) |w| {
95+
if (self.writes.get(key)) |w| {
9696
if (w.deleted) return .{ .deleted = true, .value = null };
9797
return .{ .deleted = false, .value = w.value };
9898
}
@@ -111,7 +111,7 @@ pub const Branch = struct {
111111
while (it.next()) |entry| {
112112
const w = entry.value_ptr.*;
113113
try entries.append(alloc, .{
114-
.key = w.key,
114+
.key = entry.key_ptr.*,
115115
.value = w.value,
116116
.deleted = w.deleted,
117117
.epoch = w.epoch,
@@ -168,7 +168,7 @@ pub fn compareBranches(branches: []*const Branch, alloc: Allocator) !CompareResu
168168
for (branches) |br| {
169169
var it = br.writes.iterator();
170170
while (it.next()) |entry| {
171-
try all_keys.put(entry.value_ptr.key, {});
171+
try all_keys.put(entry.key_ptr.*, {});
172172
}
173173
}
174174

@@ -186,7 +186,7 @@ pub fn compareBranches(branches: []*const Branch, alloc: Allocator) !CompareResu
186186
.agent_id = br.getAgentId(),
187187
.value = r.value,
188188
.deleted = r.deleted,
189-
.epoch = if (br.writes.get(fnv1a(key))) |w| w.epoch else 0,
189+
.epoch = if (br.writes.get(key)) |w| w.epoch else 0,
190190
});
191191
}
192192
}
@@ -435,7 +435,7 @@ pub const BranchManager = struct {
435435
.status = .active,
436436
.agent_id = undefined,
437437
.agent_id_len = aid_len,
438-
.writes = std.AutoHashMap(u64, Branch.BranchWrite).init(self.allocator),
438+
.writes = std.StringHashMap(Branch.BranchWrite).init(self.allocator),
439439
.allocator = self.allocator,
440440
};
441441
@memcpy(branch.name[0..name_len], name_arg[0..name_len]);

src/bwtree.zig

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,17 +49,21 @@ pub const BwTree = struct {
4949
next_page_id: std.atomic.Value(usize),
5050
allocator: std.mem.Allocator,
5151
// Deferred reclamation: old chains retired after consolidation are parked here
52-
// and freed on the next consolidation or deinit (simple two-phase approach).
52+
// and freed once they are at least 2 epochs old (epoch-based safe reclamation).
5353
retired: std.ArrayList(*Page),
54+
retired_epochs: std.ArrayList(u64),
5455
retired_mu: std.Thread.Mutex,
56+
epoch: u64,
5557

5658
pub fn init(allocator: std.mem.Allocator) !BwTree {
5759
var tree: BwTree = undefined;
5860
tree.allocator = allocator;
5961
tree.root_pid = 0;
6062
tree.next_page_id = std.atomic.Value(usize).init(1);
6163
tree.retired = std.ArrayList(*Page).init(allocator);
64+
tree.retired_epochs = std.ArrayList(u64).init(allocator);
6265
tree.retired_mu = .{};
66+
tree.epoch = 0;
6367

6468
// Zero out all mapping slots
6569
var i: usize = 0;
@@ -78,9 +82,10 @@ pub const BwTree = struct {
7882
}
7983

8084
pub fn deinit(self: *BwTree) void {
81-
// Free all retired chains first
82-
self.drainRetired();
85+
// Free all retired chains unconditionally on shutdown
86+
self.drainAllRetired();
8387
self.retired.deinit();
88+
self.retired_epochs.deinit();
8489
var i: usize = 0;
8590
while (i < MAX_PAGES) : (i += 1) {
8691
const ptr_val = self.mapping[i].load(.acquire);
@@ -107,12 +112,32 @@ pub const BwTree = struct {
107112

108113
/// Drain the retired list — frees chains that were parked on previous consolidations.
109114
fn drainRetired(self: *BwTree) void {
115+
self.retired_mu.lock();
116+
defer self.retired_mu.unlock();
117+
const current_epoch = self.epoch;
118+
// Free entries that are at least 2 epochs old -- any concurrent reader
119+
// that started before the retirement has completed by now.
120+
var i: usize = 0;
121+
while (i < self.retired.items.len) {
122+
if (current_epoch -| self.retired_epochs.items[i] >= 2) {
123+
self.freeChain(self.retired.items[i]);
124+
_ = self.retired.swapRemove(i);
125+
_ = self.retired_epochs.swapRemove(i);
126+
} else {
127+
i += 1;
128+
}
129+
}
130+
}
131+
132+
/// Drain ALL retired entries unconditionally (used by deinit).
133+
fn drainAllRetired(self: *BwTree) void {
110134
self.retired_mu.lock();
111135
defer self.retired_mu.unlock();
112136
for (self.retired.items) |page| {
113137
self.freeChain(page);
114138
}
115139
self.retired.clearRetainingCapacity();
140+
self.retired_epochs.clearRetainingCapacity();
116141
}
117142

118143
// ─── allocPage ───────────────────────────────────────────────────────
@@ -245,8 +270,10 @@ pub const BwTree = struct {
245270

246271
/// When delta chain exceeds MAX_DELTA_CHAIN, merge into a new base page.
247272
pub fn consolidate(self: *BwTree, page_id: usize) void {
248-
// Drain previously retired chains — they've survived at least one full
249-
// consolidation cycle, so readers from the previous epoch are done.
273+
// Advance global epoch so drainRetired can age out old entries.
274+
self.epoch +%= 1;
275+
276+
// Drain previously retired chains that are old enough.
250277
self.drainRetired();
251278

252279
const old = self.mapping[page_id].load(.acquire);
@@ -290,11 +317,16 @@ pub const BwTree = struct {
290317
) == null) {
291318
// Success — park old chain head for deferred reclamation.
292319
// Concurrent readers may still be traversing it; it will be freed
293-
// on the next consolidation cycle (two-phase epoch approach).
320+
// once the entry is at least 2 epochs old.
294321
self.retired_mu.lock();
295322
defer self.retired_mu.unlock();
296323
self.retired.append(page) catch {
297-
// If we can't track it, leak it — better than use-after-free.
324+
// If we can't track it, leak it -- better than use-after-free.
325+
return;
326+
};
327+
self.retired_epochs.append(self.epoch) catch {
328+
// Keep arrays in sync: undo the page append on epoch failure.
329+
_ = self.retired.pop();
298330
};
299331
} else {
300332
// Another thread consolidated first; discard our work

0 commit comments

Comments
 (0)