rename workflow

.envrc

@@ -1,9 +0,0 @@
-# vi: ft=sh
-# shellcheck shell=bash
-
-if ! has zvm_direnv_version || ! zvm_direnv_version 2.0.0; then
-  source_url "https://git.lerch.org/lobo/zvm-direnv/raw/tag/2.0.0/direnvrc" "sha256-8Umzxj32hFU6G0a7Wrq0KTNDQ8XEuje2A3s2ljh/hFY="
-fi
-
-use zig 0.13.0
-use flake

.forgejo/workflows/build.yaml

@@ -0,0 +1,70 @@
+name: Generic zig build
+on:
+  workflow_dispatch:
+  push:
+    branches:
+      - '*'
+      - '!zig-develop*'
+jobs:
+  build:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v4
+      - name: Setup Zig
+        uses: https://codeberg.org/mlugg/setup-zig@v2.2.1
+      - name: Install dependencies (gitea)
+        if: env.GITEA_ACTIONS == 'true'
+        run: apt-get update && apt-get install --no-install-recommends -y libnotmuch-dev libgmime-3.0-dev libglib2.0-dev
+      - name: Build project
+        run: zig build --summary all
+      - name: Run tests
+        run: zig build test --summary all
+      - name: Notify
+        uses: https://git.lerch.org/lobo/action-notify-ntfy@v2
+        if: always() && env.GITEA_ACTIONS == 'true'
+        with:
+          host: ${{ secrets.NTFY_HOST }}
+          topic: ${{ secrets.NTFY_TOPIC }}
+          status: ${{ job.status }}
+          user: ${{ secrets.NTFY_USER }}
+          password: ${{ secrets.NTFY_PASSWORD }}
+  deploy:
+    runs-on: ubuntu-latest
+    container:
+      image: ghcr.io/catthehacker/ubuntu:act-22.04
+    needs: build
+    # Only publish images from the default branch. Other branches still
+    # build + test in the job above, but must not move :latest.
+    if: github.ref == 'refs/heads/master'
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v4
+      - name: Get short ref
+        id: vars
+        run: echo "shortsha=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
+      - name: Login to Gitea
+        uses: docker/login-action@v2
+        with:
+          registry: git.lerch.org
+          username: ${{ github.actor }}
+          password: ${{ secrets.PACKAGE_PUSH }}
+      # Builds the repo-root multi-stage Dockerfile (installs libnotmuch/gmime,
+      # compiles the zig binary, and produces the Debian runtime image).
+      - name: Build and push
+        uses: docker/build-push-action@v6
+        with:
+          context: .
+          push: true
+          tags: |
+            git.lerch.org/${{ github.repository }}:${{ steps.vars.outputs.shortsha }}
+            git.lerch.org/${{ github.repository }}:latest
+      - name: Notify
+        uses: https://git.lerch.org/lobo/action-notify-ntfy@v2
+        if: always() && env.GITEA_ACTIONS == 'true'
+        with:
+          host: ${{ secrets.NTFY_HOST }}
+          topic: ${{ secrets.NTFY_TOPIC }}
+          status: ${{ job.status }}
+          user: ${{ secrets.NTFY_USER }}
+          password: ${{ secrets.NTFY_PASSWORD }}

.gitea/workflows/zig-build.yaml

@@ -1,31 +0,0 @@
-name: Generic zig build
-on:
-  workflow_dispatch:
-  push:
-    branches:
-      - '*'
-      - '!zig-develop*'
-jobs:
-  build:
-    runs-on: ubuntu-latest
-    steps:
-      - name: Check out repository code
-        uses: actions/checkout@v4
-      - name: Setup Zig
-        uses: https://codeberg.org/mlugg/setup-zig@v2.2.1
-      - name: Install dependencies (gitea)
-        if: env.GITEA_ACTIONS == 'true'
-        run: apt-get update && apt-get install --no-install-recommends -y libnotmuch-dev libgmime-3.0-dev libglib2.0-dev
-      - name: Build project
-        run: zig build --summary all
-      - name: Run tests
-        run: zig build test --summary all
-      - name: Notify
-        uses: https://git.lerch.org/lobo/action-notify-ntfy@v2
-        if: always() && env.GITEA_ACTIONS == 'true'
-        with:
-          host: ${{ secrets.NTFY_HOST }}
-          topic: ${{ secrets.NTFY_TOPIC }}
-          status: ${{ job.status }}
-          user: ${{ secrets.NTFY_USER }}
-          password: ${{ secrets.NTFY_PASSWORD }}

src/Email.zig

@@ -368,7 +368,7 @@ pub const Message = struct {
     }
 
     fn collectAttachments(part: *gmime.GMimeObject, list: *std.ArrayList(AttachmentInfo), allocator: std.mem.Allocator) !void {
-        // Check if this is a multipart
+        // Recurse into multipart containers.
         if (gmime.g_type_check_instance_is_a(@as(*gmime.GTypeInstance, @ptrCast(part)), gmime.g_mime_multipart_get_type()) != 0) {
             const multipart: *gmime.GMimeMultipart = @ptrCast(part);
             const count_i = gmime.g_mime_multipart_get_count(multipart);
@@ -384,31 +384,46 @@ pub const Message = struct {
             return;
         }
 
-        // Check if this part is an attachment
+        // Only parts carrying an attachment/inline content-disposition are
+        // treated as attachments.
         const disposition = gmime.g_mime_object_get_content_disposition(part);
-        if (disposition != null) {
+        if (disposition == null) return;
-            const disp_str = gmime.g_mime_content_disposition_get_disposition(disposition);
+        const disp_str = gmime.g_mime_content_disposition_get_disposition(disposition);
-            if (disp_str != null and (std.mem.eql(u8, std.mem.span(disp_str), "attachment") or
+        if (disp_str == null) return;
-                std.mem.eql(u8, std.mem.span(disp_str), "inline")))
+        if (!std.mem.eql(u8, std.mem.span(disp_str), "attachment") and
-            {
+            !std.mem.eql(u8, std.mem.span(disp_str), "inline")) return;
-                const filename_c = gmime.g_mime_part_get_filename(@as(*gmime.GMimePart, @ptrCast(part)));
-                if (filename_c != null) {
-                    const content_type_obj = gmime.g_mime_object_get_content_type(part);
-                    const mime_type = if (content_type_obj != null)
-                        gmime.g_mime_content_type_get_mime_type(content_type_obj)
-                    else
-                        null;
 
-                    try list.append(allocator, .{
+        const content_type_obj = gmime.g_mime_object_get_content_type(part);
-                        .filename = try allocator.dupe(u8, std.mem.span(filename_c)),
+        const mime_type = if (content_type_obj != null)
-                        .content_type = if (mime_type != null)
+            gmime.g_mime_content_type_get_mime_type(content_type_obj)
-                            try allocator.dupe(u8, std.mem.span(mime_type))
+        else
-                        else
+            null;
-                            try allocator.dupe(u8, "application/octet-stream"),
+        defer if (mime_type != null) gmime.g_free(mime_type);
-                    });
+
-                }
+        // g_mime_part_get_filename() asserts GMIME_IS_PART(part). Calling it on
-            }
+        // a non-part -- notably a message/rfc822 GMimeMessagePart, which is
+        // common in forwarded mail (e.g. Yahoo) -- prints a gmime-CRITICAL
+        // assertion failure and returns NULL, silently dropping the
+        // attachment. Check the concrete GObject type before dispatching.
+        var filename: ?[]const u8 = null;
+        if (gmime.g_type_check_instance_is_a(@as(*gmime.GTypeInstance, @ptrCast(part)), gmime.g_mime_part_get_type()) != 0) {
+            const filename_c = gmime.g_mime_part_get_filename(@as(*gmime.GMimePart, @ptrCast(part)));
+            if (filename_c != null) filename = std.mem.span(filename_c);
+        } else if (gmime.g_type_check_instance_is_a(@as(*gmime.GTypeInstance, @ptrCast(part)), gmime.g_mime_message_part_get_type()) != 0) {
+            // Attached/forwarded message (message/rfc822): surface it as a
+            // single .eml attachment rather than recursing into it (which would
+            // scramble attachment indices used by /api/attachment/:id/:num).
+            filename = "attached-message.eml";
         }
+        if (filename == null) return;
+
+        try list.append(allocator, .{
+            .filename = try allocator.dupe(u8, filename.?),
+            .content_type = if (mime_type != null)
+                try allocator.dupe(u8, std.mem.span(mime_type))
+            else
+                try allocator.dupe(u8, "application/octet-stream"),
+        });
     }
 };
 

src/main.zig

@@ -5,12 +5,6 @@ const auth = @import("auth.zig");
 
 const version = @import("build_options").git_revision;
 
-fn exitAfterDelay() void {
-    std.Thread.sleep(500 * std.time.ns_per_ms);
-    std.log.err("Notmuch search is in unrecoverable state: exiting", .{});
-    std.process.exit(1);
-}
-
 pub fn main() !u8 {
     var gpa = std.heap.GeneralPurposeAllocator(.{}){};
     defer _ = gpa.deinit();
@@ -22,7 +16,7 @@ pub fn main() !u8 {
     const stdout = &stdout_f.interface;
     // SAFETY: buffer to be used by writer
     var stderr_buffer: [1024]u8 = undefined;
-    var stderr_f = std.fs.File.stdout().writer(&stderr_buffer);
+    var stderr_f = std.fs.File.stderr().writer(&stderr_buffer);
     const stderr = &stderr_f.interface;
 
     errdefer stdout.flush() catch @panic("could not flush stdout");
@@ -136,7 +130,7 @@ pub fn main() !u8 {
     return 0;
 }
 
-fn indexHandler(db: *root.NotmuchDb, _: *httpz.Request, res: *httpz.Response) !void {
+fn indexHandler(_: *root.NotmuchDb, _: *httpz.Request, res: *httpz.Response) !void {
     const file = std.fs.cwd().openFile("static/index.html", .{}) catch {
         res.status = 500;
         res.body = "Error loading index.html";
@@ -144,7 +138,7 @@ fn indexHandler(db: *root.NotmuchDb, _: *httpz.Request, res: *httpz.Response) !v
     };
     defer file.close();
 
-    const content = file.readToEndAlloc(db.allocator, 1024 * 1024) catch {
+    const content = file.readToEndAlloc(res.arena, 1024 * 1024) catch {
         res.status = 500;
         res.body = "Error reading index.html";
         return;
@@ -155,7 +149,7 @@ fn indexHandler(db: *root.NotmuchDb, _: *httpz.Request, res: *httpz.Response) !v
     res.body = content;
 }
 
-fn staticHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response) !void {
+fn staticHandler(_: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response) !void {
     const path = req.url.path;
 
     const file_path = if (std.mem.eql(u8, path, "/style.css"))
@@ -175,7 +169,7 @@ fn staticHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response)
     };
     defer file.close();
 
-    const content = file.readToEndAlloc(db.allocator, 1024 * 1024) catch {
+    const content = file.readToEndAlloc(res.arena, 1024 * 1024) catch {
         res.status = 500;
         res.body = "Error reading file";
         return;
@@ -202,6 +196,12 @@ const SecurityHeaders = struct {
 };
 
 fn queryHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response) !void {
+    // notmuch/Xapian is not thread-safe and the DB is touched lazily during
+    // res.json below, so hold the lock across the whole handler. See
+    // root.NotmuchDb.mutex.
+    db.mutex.lock();
+    defer db.mutex.unlock();
+
     const encoded_query = req.url.path[11..]; // Skip "/api/query/"
     if (encoded_query.len == 0) {
         res.status = 400;
@@ -216,9 +216,7 @@ fn queryHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response)
     var threads = db.search(query) catch |err| {
         if (err == error.CouldNotSearchThreads) {
             res.status = 503;
-            try res.json(.{ .@"error" = "CouldNotSearchThreads", .fatal = true }, .{});
+            try res.json(.{ .@"error" = "CouldNotSearchThreads" }, .{});
-            const exit_thread = std.Thread.spawn(.{}, exitAfterDelay, .{}) catch @panic("could not spawn thread to kill process");
-            exit_thread.detach();
             return;
         }
         res.status = 500;
@@ -272,6 +270,9 @@ fn queryHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response)
 }
 
 fn threadHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response) !void {
+    db.mutex.lock();
+    defer db.mutex.unlock();
+
     const thread_id = req.param("thread_id") orelse {
         res.status = 400;
         try res.json(.{ .@"error" = "Thread ID required" }, .{});
@@ -289,6 +290,9 @@ fn threadHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response)
 }
 
 fn messageHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response) !void {
+    db.mutex.lock();
+    defer db.mutex.unlock();
+
     const message_id = req.param("message_id") orelse {
         res.status = 400;
         try res.json(.{ .@"error" = "Message ID required" }, .{});
@@ -306,6 +310,9 @@ fn messageHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response
 }
 
 fn attachmentHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Response) !void {
+    db.mutex.lock();
+    defer db.mutex.unlock();
+
     const message_id = req.param("message_id") orelse {
         res.status = 400;
         try res.json(.{ .@"error" = "Message ID required" }, .{});
@@ -338,8 +345,13 @@ fn attachmentHandler(db: *root.NotmuchDb, req: *httpz.Request, res: *httpz.Respo
     }
 
     const att = msg.attachments[num];
-    res.header("Content-Type", att.content_type);
+    // httpz stores header values by reference and serializes them after this
-    res.header("Content-Disposition", try std.fmt.allocPrint(db.allocator, "attachment; filename=\"{s}\"", .{att.filename}));
+    // handler returns -- i.e. after `defer msg.deinit` has freed att.* and
+    // without ever freeing anything allocated here. Copy both values into the
+    // per-request arena (reset by httpz after the response is written) to avoid
+    // a use-after-free on Content-Type and a leak on Content-Disposition.
+    res.header("Content-Type", try res.arena.dupe(u8, att.content_type));
+    res.header("Content-Disposition", try std.fmt.allocPrint(res.arena, "attachment; filename=\"{s}\"", .{att.filename}));
 
     // TODO: Actually retrieve and send attachment content
     // For now, just send metadata
@@ -418,3 +430,39 @@ test "threadHandler with valid thread" {
     try threadHandler(&db, t.req, t.res);
     try std.testing.expect(t.res.status != 404);
 }
+
+test "concurrent query handlers are thread-safe (regression: notmuch/Xapian SIGSEGV)" {
+    // Before NotmuchDb.mutex, httpz's handler thread pool called handlers on
+    // the single shared notmuch/Xapian handle concurrently. notmuch is not
+    // thread-safe, so concurrent requests corrupted Xapian's memory and the
+    // process segfaulted inside libxapian.so (exit 139). Hammer the *real*
+    // handler from many threads sharing one NotmuchDb: without the lock this
+    // reliably crashes; with it, it passes. Because it drives the actual
+    // handler, a future handler that forgets to take the lock also fails here.
+    const allocator = std.testing.allocator;
+    var db = try root.openNotmuchDb(allocator, "mail", null);
+    defer db.close();
+
+    const Worker = struct {
+        fn run(shared: *root.NotmuchDb) void {
+            var n: usize = 0;
+            while (n < 100) : (n += 1) {
+                var t = httpz.testing.init(.{});
+                defer t.deinit();
+                t.url("/api/query/*");
+                // A returned error just means that request produced an error
+                // status; a locking bug shows up as a segfault, not an error.
+                queryHandler(shared, t.req, t.res) catch {};
+            }
+        }
+    };
+
+    const thread_count = 16;
+    var threads: [thread_count]std.Thread = undefined;
+    var started: usize = 0;
+    while (started < thread_count) : (started += 1) {
+        threads[started] = std.Thread.spawn(.{}, Worker.run, .{&db}) catch break;
+    }
+    for (threads[0..started]) |th| th.join();
+    try std.testing.expect(started > 0);
+}

src/notmuch.zig

@@ -81,6 +81,19 @@ pub const Db = struct {
         db.handle = undefined;
     }
 
+    /// Refresh the read-only view of the database.
+    ///
+    /// A notmuch read handle sees a snapshot of the Xapian database as it was
+    /// at open time. When another process modifies the database (e.g. mail
+    /// delivery running `notmuch new`), operations on the stale handle can
+    /// begin returning Xapian exceptions. Calling reopen refreshes the
+    /// snapshot to the latest committed revision. The caller is responsible
+    /// for ensuring no other thread is using the handle concurrently.
+    pub fn reopen(db: *Db) !void {
+        const status = c.notmuch_database_reopen(db.handle, c.NOTMUCH_DATABASE_MODE_READ_ONLY);
+        if (status != c.NOTMUCH_STATUS_SUCCESS) return error.CouldNotReopenDatabase;
+    }
+
     //
     // Execute a query for threads, returning a notmuch_threads_t object
     // which can be used to iterate over the results. The returned threads

src/root.zig

@@ -168,6 +168,20 @@ pub const NotmuchDb = struct {
     /// parameter.
     email_owned: bool,
 
+    /// Serializes all access to the notmuch/Xapian handle.
+    ///
+    /// notmuch and its underlying Xapian database are NOT thread-safe: a
+    /// single database handle -- and everything derived from it (queries,
+    /// threads, messages, tags) -- must not be used from multiple threads
+    /// concurrently, or Xapian corrupts its own memory and the process
+    /// segfaults. httpz dispatches requests across a thread pool (32 threads
+    /// by default), so every handler that touches the database must hold this
+    /// mutex for the FULL duration of the request. In particular the lock must
+    /// span res.json(): Threads/Thread jsonStringify iterate the database
+    /// lazily while serializing, so releasing the lock after search()/
+    /// getThread() returns would still race.
+    mutex: std.Thread.Mutex = .{},
+
     pub fn close(self: *NotmuchDb) void {
         self.db.close();
         self.db.deinit();
@@ -175,11 +189,30 @@ pub const NotmuchDb = struct {
         if (self.email_owned) self.email.deinit();
     }
 
+    /// Run a threads query, recovering once from a stale database snapshot.
+    ///
+    /// A read-only notmuch handle sees a snapshot of the database as of open
+    /// time. Mail delivery runs `notmuch new`, and a query issued against a
+    /// snapshot that has since been rewritten can fail with a Xapian error.
+    /// When that happens we reopen to the latest revision and retry once,
+    /// rather than killing the whole process as the previous code did (it
+    /// called std.process.exit and leaned on the container restart policy). If
+    /// the reopen itself fails we surface the original query error.
+    ///
+    /// Callers must hold `self.mutex`.
+    fn openThreads(self: *NotmuchDb, query_z: [:0]const u8) !notmuch.Db.ThreadIterator {
+        return self.db.searchThreads(query_z) catch |first_err| {
+            self.db.reopen() catch return first_err;
+            return self.db.searchThreads(query_z);
+        };
+    }
+
     pub fn search(self: *NotmuchDb, query: []const u8) !Threads {
         var query_buf: [1024:0]u8 = undefined;
         const query_z = try std.fmt.bufPrintZ(&query_buf, "{s}", .{query});
         const ti = try self.allocator.create(notmuch.Db.ThreadIterator);
-        ti.* = try self.db.searchThreads(query_z);
+        errdefer self.allocator.destroy(ti);
+        ti.* = try self.openThreads(query_z);
         return Threads.init(self.allocator, ti);
     }
 
@@ -188,7 +221,7 @@ pub const NotmuchDb = struct {
         const query_z = try std.fmt.bufPrintZ(&query_buf, "thread:{s}", .{thread_id});
         const iter_ptr = try self.allocator.create(notmuch.Db.ThreadIterator);
         errdefer self.allocator.destroy(iter_ptr);
-        iter_ptr.* = try self.db.searchThreads(query_z);
+        iter_ptr.* = try self.openThreads(query_z);
         errdefer iter_ptr.deinit();
 
         const thread = iter_ptr.next();
@@ -235,7 +268,7 @@ pub const NotmuchDb = struct {
     pub fn getMessage(self: *NotmuchDb, message_id: []const u8) !MessageDetail {
         var query_buf: [1024:0]u8 = undefined;
         const query_z = try std.fmt.bufPrintZ(&query_buf, "mid:{s}", .{message_id});
-        var thread_iter = try self.db.searchThreads(query_z);
+        var thread_iter = try self.openThreads(query_z);
         defer thread_iter.deinit();
 
         const thread = thread_iter.next() orelse return error.MessageNotFound;