From 5506281a1dfe8f84ae8d7aa61c5ddc883477db49 Mon Sep 17 00:00:00 2001
From: Dustin Healy <54083382+dustinhealy@users.noreply.github.com>
Date: Mon, 1 Jun 2026 21:54:04 -0700
Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20fix:=20block=20document-level=20?=
 =?UTF-8?q?deleteOne/updateOne=20on=20AuditLog?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Mongoose registers deleteOne and updateOne pre-hooks as query middleware
by default. The query-level append-only block on AuditLog therefore did
not cover Document.prototype.deleteOne() or Document.prototype.updateOne(),
leaving a path where a caller that had already loaded an audit row via
findOne could call .deleteOne() or .updateOne() on the instance and bypass
the schema contract.

Explicit { document: true, query: false } registrations close the holes,
and the spec now covers both code paths against a real in-memory Mongo.
---
 .../data-schemas/src/methods/auditLog.spec.ts    | 16 ++++++++++++++++
 packages/data-schemas/src/schema/auditLog.ts     | 15 +++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/packages/data-schemas/src/methods/auditLog.spec.ts b/packages/data-schemas/src/methods/auditLog.spec.ts
index 79344d4c03..fe2361d25e 100644
--- a/packages/data-schemas/src/methods/auditLog.spec.ts
+++ b/packages/data-schemas/src/methods/auditLog.spec.ts
@@ -213,6 +213,22 @@ describe('auditLog methods', () => {
       await expect(AuditLog.deleteMany({})).rejects.toThrow(/append-only/);
     });
 
+    it('rejects Document.prototype.deleteOne() on a loaded audit entry', async () => {
+      const doc = await methods.recordAuditEntry(baseInput());
+      const persisted = (await AuditLog.findById(doc!._id))!;
+      await expect(persisted.deleteOne()).rejects.toThrow(/append-only/);
+      const stillThere = await AuditLog.findById(doc!._id);
+      expect(stillThere).not.toBeNull();
+    });
+
+    it('rejects Document.prototype.updateOne() on a loaded audit entry', async () => {
+      const doc = await methods.recordAuditEntry(baseInput());
+      const persisted = (await AuditLog.findById(doc!._id))!;
+      await expect(persisted.updateOne({ capability: 'tampered' })).rejects.toThrow(/append-only/);
+      const unchanged = await AuditLog.findById(doc!._id);
+      expect(unchanged?.capability).toBe('manage:users');
+    });
+
     it('rejects a second save() on an existing document', async () => {
       const doc = await methods.recordAuditEntry(baseInput());
       const persisted = (await AuditLog.findById(doc!._id))!;
diff --git a/packages/data-schemas/src/schema/auditLog.ts b/packages/data-schemas/src/schema/auditLog.ts
index 91d9b1aa5a..461016e6cb 100644
--- a/packages/data-schemas/src/schema/auditLog.ts
+++ b/packages/data-schemas/src/schema/auditLog.ts
@@ -90,6 +90,21 @@ auditLogSchema.pre(['deleteOne', 'deleteMany', 'findOneAndDelete'], function (ne
   next(new Error(APPEND_ONLY_MESSAGE));
 });
 
+/**
+ * Mongoose registers `deleteOne` / `updateOne` pre-hooks as query middleware
+ * by default, which leaves `Document.prototype.deleteOne()` and
+ * `Document.prototype.updateOne()` as escape hatches around the append-only
+ * contract: a caller that has already loaded a doc could invoke either method
+ * on the instance and bypass the query-level hooks above. The explicit
+ * `{ document: true, query: false }` registrations below close both holes.
+ */
+auditLogSchema.pre('deleteOne', { document: true, query: false }, function (next) {
+  next(new Error(APPEND_ONLY_MESSAGE));
+});
+auditLogSchema.pre('updateOne', { document: true, query: false }, function (next) {
+  next(new Error(APPEND_ONLY_MESSAGE));
+});
+
 /**
  * Document-level `save` is allowed for new docs only. A second save on an
  * existing document would mutate it, so reject that case explicitly.