🔒 fix: block document-level deleteOne/updateOne on AuditLog

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.
This commit is contained in:
Dustin Healy 2026-06-01 21:54:04 -07:00 • committed by Danny Avila
parent 2612df9097
commit 5506281a1d
2 changed files with 31 additions and 0 deletions

View file

@ -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))!;

View file

@ -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.