diff --git a/packages/agent/src/audit-trail/README.md b/packages/agent/src/audit-trail/README.md index 5c6c64ad1f..1ea194c95b 100644 --- a/packages/agent/src/audit-trail/README.md +++ b/packages/agent/src/audit-trail/README.md @@ -212,6 +212,20 @@ audit read; a record deleted in between is treated as a request starting a momen treated it, and one moved out of the caller's scope in between is refused. The extra read only happens for a scoped caller — with no scope there is nothing to withhold. +Both reads count, and neither can clear the other, as in agent-ruby. A record gone at either read is +treated as gone: an id freed by a delete and taken by another record since answers for itself, never +for the rows of the record that held it before. And the re-read runs even for a record already gone +at the first check, so an id since taken by a record the caller cannot read is refused (404). + +**An id freed by a delete and taken since has two lives.** A live record in the caller's scope says +nothing about the rows filed under its id before the id's last confirmed `delete`: they are an +earlier record's. Those rows go through the same withholding as a record gone for good — on the +history route (with `search` and `fields` matched against the served values, so the count and the +authors cannot leak them either), on the correlation lookups, and on `/state`, whose reconstruction +at an instant before that `delete` is tested against the scope (strictly before: at the delete's own +instant the state already includes a replacement `create` sharing it). A `pending` delete frees nothing, since it may +never have landed. + That test only runs when the snapshot can actually answer it. The capture keeps the writable columns (plus the packed record id), so a scope reaching for anything else — a read-only column, a relation, a field stored redacted — has no honest answer in the snapshot and the values are withheld rather @@ -311,11 +325,12 @@ the second read of the record decides the withholding, so the SQL-matched answer the history scanned the same way. Matching the *serialized* text rather than a structural walk of the parsed value is cheap and still -correct for "keys and scalar values" — but two things follow from it. A punctuation-only term (`,`, -`:`, `{`) matches almost any row whose diff has more than one key, since those characters are JSON -structure rather than content. And a value containing a double quote or a backslash can't be found -by searching for it literally — `5"` is stored in the JSON text as `5\"`, so searching `5"` never -matches the row that holds it. Neither is severe, and both are inherent to the approach. +correct for "keys and scalar values". The term is escaped the way JSON escaped the values, so `5"` +finds the row whose JSON text holds `5\"`, and a bare quote can't match the document's own +structure. The redaction mask is removed before matching, so `search=redacted` confirms nothing. +Both hold on the served-value path too, and both mirror agent-ruby. One thing remains inherent to +the approach: a punctuation-only term (`,`, `:`, `{`) matches almost any row whose diff has more +than one key, since those characters are JSON structure rather than content. Defensive parsing: diff --git a/packages/agent/src/audit-trail/earlier-life.ts b/packages/agent/src/audit-trail/earlier-life.ts new file mode 100644 index 0000000000..bccdf039d1 --- /dev/null +++ b/packages/agent/src/audit-trail/earlier-life.ts @@ -0,0 +1,33 @@ +import type { AuditRecord, AuditStore } from './types'; + +// A delete frees the id, and a later record can take it. A live record under the caller's scope says +// nothing about the rows filed under that id before its last delete: they are another record's, and +// its values go through the same withholding as a record gone for good. + +/** The id's latest confirmed `delete`, or null when the id has never been freed. */ +export async function lastDeleteOf( + store: AuditStore, + collection: string, + recordId: string, +): Promise { + const deletes = await store.listByRecord({ + collection, + recordId, + operations: ['delete'], + order: 'desc', + }); + + // A pending delete may never have landed, so it frees nothing. Filtered here too, not just in + // the query: a store that ignores `operations` must not turn any row into the boundary. + return deletes.find(row => row.operation === 'delete' && row.status !== 'pending') ?? null; +} + +/** Whether a row was filed at or before the boundary delete, in the trail's (timestamp, id) order. */ +export function belongsToEarlierLife(row: AuditRecord, lastDelete: AuditRecord | null): boolean { + if (!lastDelete) return false; + + return ( + row.timestamp < lastDelete.timestamp || + (row.timestamp === lastDelete.timestamp && row.id <= lastDelete.id) + ); +} diff --git a/packages/agent/src/audit-trail/index.ts b/packages/agent/src/audit-trail/index.ts index f5e590afe2..12ae8d2a1c 100644 --- a/packages/agent/src/audit-trail/index.ts +++ b/packages/agent/src/audit-trail/index.ts @@ -11,6 +11,7 @@ export { ensureAuditStorage, defineAuditLogModel, fieldsChangedCondition, + jsonEscaped, searchCondition, toRow, fromRow, diff --git a/packages/agent/src/audit-trail/record-visibility.ts b/packages/agent/src/audit-trail/record-visibility.ts index e759e6ff28..de7f154de9 100644 --- a/packages/agent/src/audit-trail/record-visibility.ts +++ b/packages/agent/src/audit-trail/record-visibility.ts @@ -77,9 +77,13 @@ export default async function checkRecordVisibility( * The record can be deleted — or moved out of the caller's permission scope — between the check * that authorized the request and the audit read that answers it: the audit trail lives in its own * database, often its own engine, so no single snapshot spans both. Re-reads once the rows are in - * hand, and returns null when there is nothing to re-read: a caller with no scope has nothing to - * withhold, and a record already gone at the first check has already had the withholding applied — - * an id recreated in the meantime would only widen what is served, never what is hidden. + * hand, and returns null only for a caller with no scope, who has nothing to withhold. + * + * Both reads count, and neither can clear the other, as in agent-ruby. The first is the only one that + * saw the record whose rows these are, so an id freed by a delete and taken by another record since + * answers for itself, never for the earlier life: gone at either read means gone. The second is the + * only one that sees a record deleted since, and the one that answers 404 when the id now belongs to + * a record this caller cannot read — even if it was already gone at the first. */ export async function recheckRecordVisibility( collection: Collection, @@ -88,7 +92,9 @@ export async function recheckRecordVisibility( permissionScope: ConditionTree | null, wasGoneEntirely: boolean, ): Promise { - if (!permissionScope || wasGoneEntirely) return null; + if (!permissionScope) return null; - return checkRecordVisibility(collection, packedId, context, permissionScope); + const now = await checkRecordVisibility(collection, packedId, context, permissionScope); + + return { visible: now.visible, goneEntirely: wasGoneEntirely || now.goneEntirely }; } diff --git a/packages/agent/src/audit-trail/sql-store.ts b/packages/agent/src/audit-trail/sql-store.ts index 9459be3d8f..77baf29a4f 100644 --- a/packages/agent/src/audit-trail/sql-store.ts +++ b/packages/agent/src/audit-trail/sql-store.ts @@ -12,6 +12,7 @@ import type { Model, ModelStatic } from 'sequelize'; import { DataTypes, Op, Sequelize } from 'sequelize'; +import { REDACTED } from './instrument'; import { runAuditMigrations } from './migrations'; export const DEFAULT_SCHEMA = 'forest'; @@ -167,12 +168,20 @@ function jsonColumnAsText(sequelize: Sequelize, column: string): string { return column; } +// What serializing the term would have produced, minus its own surrounding quotes. +export function jsonEscaped(term: string): string { + return JSON.stringify(term).slice(1, -1); +} + // A free-text search against the *serialized* JSON text naturally covers "keys and scalar values // at any depth" without walking the structure by hand: both a key and a value appear as a quoted // literal substring of that text. This runs as a SQL WHERE clause (not an in-memory scan of -// fetched rows), so it composes with pagination and COUNT the same way every other filter does. A -// redacted value can't match a search for the real value — the real value already isn't in this -// text, `redactValues` replaced it before the row was ever written. +// fetched rows), so it composes with pagination and COUNT the same way every other filter does. +// +// Two things keep it honest about what the text holds. The term is escaped the way JSON wrote the +// values, so `15" monitor` (stored as `15\" monitor`) is found, and a bare quote can't match the +// document's own structure. And the redaction mask is removed before matching, so `search=redacted` +// can't confirm which rows hold a masked value. Both mirror agent-ruby's `Sql::TextSearch`. export function searchCondition(sequelize: Sequelize, term: string) { // `~` rather than the standard `\`: MySQL/MariaDB treat backslash as a string-literal escape // character under the default sql_mode (no NO_BACKSLASH_ESCAPES), so a bare `ESCAPE '\'` is @@ -180,21 +189,25 @@ export function searchCondition(sequelize: Sequelize, term: string) { // literal. `~` has no special meaning to any of the four supported dialects' string literals, so // it sidesteps that interaction entirely. Escapes the two LIKE wildcards plus itself, so a term // containing `%`/`_`/`~` is matched literally instead of behaving like a pattern. - const escaped = term.replace(/[~%_]/g, char => `~${char}`); - const pattern = sequelize.escape(`%${escaped}%`); - - const columns = [ - 'action_name', - 'user_first_name', - 'user_last_name', - 'user_email', - jsonColumnAsText(sequelize, 'previous_values'), - jsonColumnAsText(sequelize, 'new_values'), + const like = (expression: string, text: string) => { + const escaped = text.replace(/[~%_]/g, char => `~${char}`); + + return `LOWER(${expression}) LIKE LOWER(${sequelize.escape(`%${escaped}%`)}) ESCAPE '~'`; + }; + + const valuesAsText = (column: string) => + `REPLACE(${jsonColumnAsText(sequelize, column)}, ${sequelize.escape(REDACTED)}, '')`; + + const clauses = [ + ...['action_name', 'user_first_name', 'user_last_name', 'user_email'].map(column => + like(column, term), + ), + ...['previous_values', 'new_values'].map(column => + like(valuesAsText(column), jsonEscaped(term)), + ), ]; - return Sequelize.literal( - `(${columns.map(column => `LOWER(${column}) LIKE LOWER(${pattern}) ESCAPE '~'`).join(' OR ')})`, - ); + return Sequelize.literal(`(${clauses.join(' OR ')})`); } function buildHistoryWhereClause( diff --git a/packages/agent/src/routes/access/audit-trail-correlation.ts b/packages/agent/src/routes/access/audit-trail-correlation.ts index 17aef48285..d9b3b4d3ea 100644 --- a/packages/agent/src/routes/access/audit-trail-correlation.ts +++ b/packages/agent/src/routes/access/audit-trail-correlation.ts @@ -7,6 +7,7 @@ import type { Context } from 'koa'; import { ValidationError } from '@forestadmin/datasource-toolkit'; +import { belongsToEarlierLife, lastDeleteOf } from '../../audit-trail/earlier-life'; import checkRecordVisibility, { recheckRecordVisibility, } from '../../audit-trail/record-visibility'; @@ -110,14 +111,27 @@ export default class AuditTrailCorrelationRoute extends BaseRoute { const gone = after ? after.goneEntirely : target.goneEntirely; - if (!target.permissionScope || !gone) return entries; + if (!target.permissionScope) return entries; - return withholdOutsidePermissionScope(entries, { + // Live now, but the rows up to the id's last delete may be an earlier record's. + const lastDelete = gone + ? null + : await lastDeleteOf(this.options.auditTrail.store, target.collection, target.recordId); + + if (!gone && !lastDelete) return entries; + + const withheld = withholdOutsidePermissionScope(entries, { collection: target.collectionObject, permissionScope: target.permissionScope, timezone: QueryStringParser.parseCaller(context, { defaultTimezone: 'UTC' }).timezone, logger: this.options.logger, }); + + return gone + ? withheld + : entries.map((entry, index) => + belongsToEarlierLife(entry, lastDelete) ? withheld[index] : entry, + ); } // Returns null (after issuing the 404) when a configured record-level permission scope excludes the id — diff --git a/packages/agent/src/routes/access/audit-trail.ts b/packages/agent/src/routes/access/audit-trail.ts index 94edab757a..95365f392a 100644 --- a/packages/agent/src/routes/access/audit-trail.ts +++ b/packages/agent/src/routes/access/audit-trail.ts @@ -16,7 +16,8 @@ import { ValidationError, } from '@forestadmin/datasource-toolkit'; -import { revertRecord } from '../../audit-trail'; +import { REDACTED, jsonEscaped, revertRecord } from '../../audit-trail'; +import { belongsToEarlierLife, lastDeleteOf } from '../../audit-trail/earlier-life'; import { parseDateBoundary, parseFields, @@ -96,8 +97,8 @@ export default class AuditTrailRoute extends CollectionRoute { const filtersOnValues = Boolean(fields || search); - const serveMatchedValues = async (scope: ConditionTree) => { - const matched = await this.scanServedValues(context, scope, rowFilters, { + const serveMatchedValues = async (withholdServed: (rows: AuditRecord[]) => AuditRecord[]) => { + const matched = await this.scanServedValues(context, withholdServed, rowFilters, { fields, search, order, @@ -116,8 +117,23 @@ export default class AuditTrailRoute extends CollectionRoute { }; }; + const withholdAll = (rows: AuditRecord[]) => + permissionScope ? this.withhold(rows, permissionScope, context) : rows; + if (permissionScope && goneEntirely && filtersOnValues) { - await serveMatchedValues(permissionScope); + await serveMatchedValues(withholdAll); + + // Gone at the check, so everything stays withheld whatever the re-read finds; it is asked + // only for the 404, when the id has since been taken by a record this caller cannot read. + const recheck = await recheckRecordVisibility( + this.collection, + context.params.id, + context, + permissionScope, + goneEntirely, + ); + + if (recheck && !recheck.visible) context.throw(HttpCode.NotFound, 'Record does not exists'); return; } @@ -150,10 +166,27 @@ export default class AuditTrailRoute extends CollectionRoute { const gone = after ? after.goneEntirely : goneEntirely; - // Gone between the check and the read: this answer withholds, so the rows, count and authors - // matched in SQL above must not decide what is served either. - if (permissionScope && gone && filtersOnValues) { - await serveMatchedValues(permissionScope); + // Live now, but the id may have been freed by a delete and taken since: the rows up to that + // delete are the earlier record's, which the current one's scope says nothing about. + const lastDelete = + permissionScope && !gone + ? await lastDeleteOf(store, this.collection.name, context.params.id) + : null; + + const withholdServed = gone + ? withholdAll + : (rows: AuditRecord[]) => { + const withheld = withholdAll(rows); + + return rows.map((row, index) => + belongsToEarlierLife(row, lastDelete) ? withheld[index] : row, + ); + }; + + // Gone between the check and the read, or carrying an earlier life: this answer withholds, so + // the rows, count and authors matched in SQL above must not decide what is served either. + if (permissionScope && (gone || lastDelete) && filtersOnValues) { + await serveMatchedValues(withholdServed); return; } @@ -162,9 +195,8 @@ export default class AuditTrailRoute extends CollectionRoute { // existence against — but create/update/delete rows still carry captured column values from // when the record existed. If those values themselves would have failed the caller's permission scope, // withhold them while still surfacing that the row happened, by whom and when: that part - // stays visible regardless. - const data = - permissionScope && gone ? this.withhold(rawData, permissionScope, context) : rawData; + // stays visible regardless. The same holds for an earlier record that held this id. + const data = permissionScope && (gone || lastDelete) ? withholdServed(rawData) : rawData; context.response.body = { data: data.map(({ previousRecordId, ...served }) => served), @@ -180,7 +212,7 @@ export default class AuditTrailRoute extends CollectionRoute { // bounded at the instant the scan starts so an id taken since cannot keep it chasing new rows. private async scanServedValues( context: Context, - permissionScope: ConditionTree, + withholdServed: (rows: AuditRecord[]) => AuditRecord[], rowFilters: Omit, { fields, @@ -210,7 +242,7 @@ export default class AuditTrailRoute extends CollectionRoute { ...(after && { after: { timestamp: after.timestamp, id: after.id } }), }); - const matched = this.withhold(rows, permissionScope, context).filter(entry => + const matched = withholdServed(rows).filter(entry => AuditTrailRoute.matchesServedValues(entry, fields, search), ); @@ -248,15 +280,19 @@ export default class AuditTrailRoute extends CollectionRoute { sides.some(values => fields.some(field => Object.prototype.hasOwnProperty.call(values, field)), ); - const texts = [ - entry.actionName, - entry.userFirstName, - entry.userLastName, - entry.userEmail, - ...sides.map(values => JSON.stringify(values)), - ]; - - return touchesField && (!term || texts.some(text => text?.toLowerCase().includes(term))); + + if (!touchesField) return false; + if (!term) return true; + + const identity = [entry.actionName, entry.userFirstName, entry.userLastName, entry.userEmail]; + const valueTerm = jsonEscaped(term); + + return ( + identity.some(text => text?.toLowerCase().includes(term)) || + sides.some(values => + JSON.stringify(values).split(REDACTED).join('').toLowerCase().includes(valueTerm), + ) + ); } private withhold( @@ -326,6 +362,16 @@ export default class AuditTrailRoute extends CollectionRoute { const goneNow = after ? after.goneEntirely : goneEntirely; + // A live record whose id was freed by a delete and taken since: a state before that delete is + // the earlier record's, which the current one's scope says nothing about. Strictly before: at + // the delete's own instant the state already reflects every row there, including a replacement + // `create` sharing it. + const lastDelete = + permissionScope && !goneNow + ? await lastDeleteOf(store, this.collection.name, context.params.id) + : null; + const ofEarlierLife = Boolean(lastDelete && at < lastDelete.timestamp); + // `startTimestamp` is an inclusive lower bound, so an entry timestamped exactly `at` comes // back too — but the record already reflects that entry's change at instant `at`, so it must // be kept rather than reverted (which would wrongly return the state just *before* it). @@ -364,7 +410,7 @@ export default class AuditTrailRoute extends CollectionRoute { // id is merged in first for the same reason it is on a row: a read-only primary key never lands // in the capture, so a scope on the id would blank the very record it names. if ( - goneNow && + (goneNow || ofEarlierLife) && !permissionScopeAccepts( // `false`: the reconstruction may sit on the far side of a primary-key move this route // cannot see, so the requested id does not answer for a key the trail redacted. diff --git a/packages/agent/test/audit-trail/sql-store.test.ts b/packages/agent/test/audit-trail/sql-store.test.ts index c13d2b0fab..d6206849c0 100644 --- a/packages/agent/test/audit-trail/sql-store.test.ts +++ b/packages/agent/test/audit-trail/sql-store.test.ts @@ -869,7 +869,7 @@ describe('createSqlAuditStore (sqlite round-trip)', () => { await close(); }); - it('search does not match a value that was redacted before it was ever stored', async () => { + it('search confirms neither a redacted value nor the mask that replaced it', async () => { const { store, close } = createSqlAuditStore({ connectionString: 'sqlite::memory:' }); // Simulates what instrument.ts's redactValues does before a redacted row ever reaches the @@ -888,7 +888,40 @@ describe('createSqlAuditStore (sqlite round-trip)', () => { ).resolves.toEqual([]); await expect( store.listByRecord({ collection: 'accounts', recordId: '1', search: 'redacted' }), - ).resolves.toHaveLength(1); + ).resolves.toEqual([]); + + await close(); + }); + + it('search finds a value holding a quote or a backslash, as JSON serialized it', async () => { + const { store, close } = createSqlAuditStore({ connectionString: 'sqlite::memory:' }); + await seed(store, record({ recordId: '1', newValues: { name: '15" monitor' } })); + await seed(store, record({ recordId: '1', newValues: { path: 'C:\\temp' } })); + + const byQuote = await store.listByRecord({ + collection: 'accounts', + recordId: '1', + search: '15" monitor', + }); + const byBackslash = await store.listByRecord({ + collection: 'accounts', + recordId: '1', + search: 'C:\\temp', + }); + + expect(byQuote.map(entry => entry.newValues)).toEqual([{ name: '15" monitor' }]); + expect(byBackslash.map(entry => entry.newValues)).toEqual([{ path: 'C:\\temp' }]); + + await close(); + }); + + it('search does not let a bare quote match the JSON structure', async () => { + const { store, close } = createSqlAuditStore({ connectionString: 'sqlite::memory:' }); + await seed(store, record({ recordId: '1', newValues: { status: 'open' } })); + + await expect( + store.listByRecord({ collection: 'accounts', recordId: '1', search: '"status"' }), + ).resolves.toEqual([]); await close(); }); @@ -1070,8 +1103,8 @@ describe('searchCondition', () => { const sqlite = new Sequelize('sqlite::memory:', { logging: false }); const mssql = new Sequelize('mssql://user:pwd@localhost/db', { logging: false }); - expect(searchCondition(sqlite, 'Lyon').val).toMatch(/LOWER\(previous_values\)/); - expect(searchCondition(mssql, 'Lyon').val).toMatch(/LOWER\(previous_values\)/); + expect(searchCondition(sqlite, 'Lyon').val).toMatch(/LOWER\(REPLACE\(previous_values, /); + expect(searchCondition(mssql, 'Lyon').val).toMatch(/LOWER\(REPLACE\(previous_values, /); }); it('matches against the identity columns as well as the JSON columns', () => { diff --git a/packages/agent/test/routes/access/audit-trail-correlation.test.ts b/packages/agent/test/routes/access/audit-trail-correlation.test.ts index 32b5554e7a..9dad2fc5c8 100644 --- a/packages/agent/test/routes/access/audit-trail-correlation.test.ts +++ b/packages/agent/test/routes/access/audit-trail-correlation.test.ts @@ -17,7 +17,7 @@ describe('AuditTrailCorrelationRoute', () => { }), ]); const store = { - listByRecord: jest.fn(), + listByRecord: jest.fn().mockResolvedValue([]), countByRecord: jest.fn(), listByCorrelation: jest.fn().mockResolvedValue(history), listByCorrelations: jest.fn().mockResolvedValue(history), @@ -200,7 +200,7 @@ describe('AuditTrailCorrelationRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone + .mockResolvedValue([]); // bare check: genuinely gone, and at the re-read const route = new AuditTrailCorrelationRoute(services, options, dataSource); const context = contextWith({ timezone: 'Europe/Paris', @@ -244,6 +244,32 @@ describe('AuditTrailCorrelationRoute', () => { }); }); + test('withholds an entry from before the id was freed and taken by a record in scope', async () => { + const freed = { + id: 2, + timestamp: '2026-01-02T00:00:00.000Z', + operation: 'delete', + recordId: '2', + correlationKey: 'req-1', + previousValues: { title: 'Secret' }, + newValues: {}, + }; + const { services, dataSource, options, store } = setup([freed]); + store.listByRecord.mockResolvedValue([freed]); + (services.authorization.getScope as jest.Mock).mockResolvedValue( + new ConditionTreeLeaf('id', 'Equal', 1), + ); + jest.spyOn(dataSource.getCollection('books'), 'list').mockResolvedValue([{ id: 2 }]); + const route = new AuditTrailCorrelationRoute(services, options, dataSource); + const context = contextWith({ timezone: 'Europe/Paris', collection: 'books', recordId: '2' }); + + await route.handleHistory(context); + + expect((context.response.body as { data: unknown[] }).data).toEqual([ + { ...freed, previousValues: {} }, + ]); + }); + test('allows an id inside a restrictive scope through to the store', async () => { const history = [{ operation: 'update', correlationKey: 'req-1' }]; const { services, dataSource, options, store } = setup(history); @@ -304,7 +330,7 @@ describe('AuditTrailCorrelationRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone + .mockResolvedValue([]); // bare check: genuinely gone, and at the re-read const route = new AuditTrailCorrelationRoute(services, options, dataSource); const context = contextWith({ timezone: 'Europe/Paris', @@ -489,7 +515,7 @@ describe('AuditTrailCorrelationRoute', () => { const services = factories.forestAdminHttpDriverServices.build(); const dataSource = buildDataSource(); const store = { - listByRecord: jest.fn(), + listByRecord: jest.fn().mockResolvedValue([]), countByRecord: jest.fn(), listByCorrelation: jest.fn(), listByCorrelations: jest.fn(), diff --git a/packages/agent/test/routes/access/audit-trail.test.ts b/packages/agent/test/routes/access/audit-trail.test.ts index bd737cdf4e..7e5c5da4f8 100644 --- a/packages/agent/test/routes/access/audit-trail.test.ts +++ b/packages/agent/test/routes/access/audit-trail.test.ts @@ -823,7 +823,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone, not just out of scope + .mockResolvedValue([]); // bare check: genuinely gone, not just out of scope, and at the re-read const route = new AuditTrailRoute(services, options, dataSource, 'books'); const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, @@ -925,6 +925,23 @@ describe('AuditTrailRoute', () => { }); }); + test('does not confirm a masked value by searching the mask', async () => { + const { body } = await searched( + [kept({ previousValues: { ownerId: 1, ssn: '[redacted]' } })], + { search: 'redacted' }, + ); + + expect(body.data).toEqual([]); + }); + + test('finds a served value holding a quote, as JSON serialized it', async () => { + const quoted = kept({ previousValues: { ownerId: 1, title: '15" monitor' } }); + + const { body } = await searched([quoted], { search: '15" monitor' }); + + expect(body.data).toEqual([quoted]); + }); + test('does not match a field only a withheld side touched', async () => { const { body } = await searched([secretDelete()], { fields: 'title' }); @@ -1040,7 +1057,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone, not just out of scope + .mockResolvedValue([]); // bare check: genuinely gone, not just out of scope, and at the re-read const route = new AuditTrailRoute(services, options, dataSource, 'books'); const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, @@ -1070,7 +1087,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone + .mockResolvedValue([]); // bare check: genuinely gone, and at the re-read const route = new AuditTrailRoute(services, options, dataSource, 'books'); const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, @@ -1135,7 +1152,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone, not just out of scope + .mockResolvedValue([]); // bare check: genuinely gone, not just out of scope, and at the re-read const route = new AuditTrailRoute(services, options, dataSource, 'books'); const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, @@ -1166,7 +1183,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped check: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone, not just out of scope + .mockResolvedValue([]); // bare check: genuinely gone, not just out of scope, and at the re-read const route = new AuditTrailRoute(services, options, dataSource, 'books'); const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, @@ -1284,10 +1301,101 @@ describe('AuditTrailRoute', () => { expect(services.authorization.getScope).toHaveBeenCalledTimes(1); }); - test('does not ask again for a record that was already gone at the first check', async () => { - const { list } = await raceWith([], []); + test('refuses when an id gone at the first check now belongs to a record the caller cannot read', async () => { + // gone at the check (scoped and bare), then the id was taken by someone else's record + const { context } = await raceWith([], [], [], [{ id: 2 }]); + + expect(context.throw).toHaveBeenCalledWith(404, 'Record does not exists'); + }); + + test('keeps withholding when an in-scope record took the id after the first check', async () => { + // gone at the check; the replacement answers for itself, not for the rows of the earlier life + const { context } = await raceWith([], [], [{ id: 2 }]); + + expect(context.throw).not.toHaveBeenCalled(); + expect((context.response.body as { data: unknown[] }).data).toEqual([ + { operation: 'delete', recordId: '2', previousValues: {}, newValues: {} }, + ]); + }); + }); + + describe('an id freed by a delete and taken by a record in scope since', () => { + const row = (over: Record) => ({ + recordId: '2', + userId: 7, + userFirstName: null, + userLastName: null, + userEmail: 'jane@acme.io', + actionName: null, + ...over, + }); + const current = row({ + id: 3, + timestamp: '2026-01-03T00:00:00.000Z', + operation: 'create', + previousValues: {}, + newValues: { ownerId: 1, title: 'Mine' }, + }); + const freed = row({ + id: 2, + timestamp: '2026-01-02T00:00:00.000Z', + operation: 'delete', + previousValues: { ownerId: 2, title: 'Secret' }, + newValues: {}, + }); + const earlier = row({ + id: 1, + timestamp: '2026-01-01T00:00:00.000Z', + operation: 'update', + previousValues: { ownerId: 2, title: 'Old secret' }, + newValues: { ownerId: 2, title: 'Secret' }, + }); + + const readHistory = async (history: unknown[], query: Record = {}) => { + const { services, dataSource, options } = setup(history); + (services.authorization.getScope as jest.Mock).mockResolvedValue( + new ConditionTreeLeaf('ownerId', 'Equal', 1), + ); + jest.spyOn(dataSource.getCollection('books'), 'list').mockResolvedValue([{ id: 2 }]); + const route = new AuditTrailRoute(services, options, dataSource, 'books'); + const context = createMockContext({ + state: { user: { email: 'john.doe@domain.com' } }, + customProperties: { query: { timezone: 'Europe/Paris', ...query }, params: { id: '2' } }, + }); + + await route.handleHistory(context); + + return context.response.body as { data: unknown[]; meta: unknown }; + }; + + test('withholds the earlier record values and serves the current record', async () => { + const body = await readHistory([current, freed, earlier]); + + expect(body.data).toEqual([ + current, + { ...freed, previousValues: {} }, + { ...earlier, previousValues: {}, newValues: {} }, + ]); + }); + + test('matches a search on the values served, so it cannot find the earlier record values', async () => { + const body = await readHistory([current, freed, earlier], { search: 'secret' }); + + expect(body).toEqual({ data: [], meta: { count: 0, availableUsers: [] } }); + }); + + test('does not treat a pending delete as freeing the id, since it may never have landed', async () => { + const pending = { ...freed, status: 'pending' }; + + const body = await readHistory([current, pending, earlier]); + + expect(body.data).toEqual([current, pending, earlier]); + }); + + test('serves the whole history of an id that was never freed', async () => { + const body = await readHistory([current, earlier]); - expect(list).toHaveBeenCalledTimes(2); + expect(body.data).toEqual([current, earlier]); }); }); @@ -2145,6 +2253,84 @@ describe('AuditTrailRoute', () => { // The history route withholds a gone record's captured values from a caller whose scope they // fail; this route is those same values reassembled, so it has to answer the same way or the // withheld values are one request away. + describe('an id freed by a delete and taken by a record in scope since', () => { + const history = [ + { + id: 3, + timestamp: '2026-06-20T00:00:00.000Z', + operation: 'create', + previousValues: {}, + newValues: { id: 2, status: 'mine', name: 'New' }, + }, + { + id: 2, + timestamp: '2026-06-19T00:00:00.000Z', + operation: 'delete', + previousValues: { id: 2, status: 'secret', name: 'Old' }, + newValues: {}, + }, + ]; + + const stateAt = async ( + at: string, + entries = history, + scope = new ConditionTreeLeaf('status', 'Equal', 'mine'), + ) => { + const { services, dataSource, store, route } = setupBooks(); + store.listByRecord.mockImplementation( + async ({ + startTimestamp, + operations, + }: { + startTimestamp?: string; + operations?: string[]; + }) => + entries.filter( + entry => + (!startTimestamp || entry.timestamp >= startTimestamp) && + (!operations || operations.includes(entry.operation)), + ), + ); + (services.authorization.getScope as jest.Mock).mockResolvedValue(scope); + jest + .spyOn(dataSource.getCollection('books'), 'list') + .mockResolvedValue([{ id: 2, status: 'mine', name: 'New' }]); + const context = createMockContext({ + state: { user: { email: 'john.doe@domain.com' } }, + customProperties: { query: { timezone: 'UTC', at }, params: { id: '2' } }, + }); + + await route.handleStateAt(context); + + return context; + }; + + test('withholds a state from before the delete when the earlier record fails the scope', async () => { + const context = await stateAt('2026-06-18'); + + expect(context.response.body).toEqual({ data: null }); + }); + + test('serves a state from after the id was taken', async () => { + const context = await stateAt('2026-06-21'); + + expect(context.response.body).toEqual({ data: { id: 2, status: 'mine', name: 'New' } }); + }); + + // A scope on a column the capture never kept cannot be answered from a reconstruction, so + // testing this state at all would withhold the replacement the caller was just shown to read. + test('serves the state at the instant of a delete that a replacement create shares', async () => { + const sameInstant = '2026-06-19T00:00:00.000Z'; + const context = await stateAt( + sameInstant, + [{ ...history[0], timestamp: sameInstant }, history[1]], + new ConditionTreeLeaf('displayName', 'Equal', 'New'), + ); + + expect(context.response.body).toEqual({ data: { id: 2, status: 'mine', name: 'New' } }); + }); + }); + describe('a genuinely gone record, read by a scoped caller', () => { const reconstructFor = async (scope: ConditionTreeLeaf, status: string | null = 'closed') => { const history = [ @@ -2159,7 +2345,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped fetch: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone, not just out of scope + .mockResolvedValue([]); // bare check: genuinely gone, not just out of scope, and at the re-read const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, customProperties: { @@ -2222,7 +2408,7 @@ describe('AuditTrailRoute', () => { .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([{ id: 2, status: 'closed', name: 'Acme' }]) // present and in scope .mockResolvedValueOnce([]) // re-read, scoped: gone - .mockResolvedValueOnce([]); // re-read, bare: genuinely gone + .mockResolvedValue([]); // re-read, bare: genuinely gone, and at the re-read const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, customProperties: { @@ -2278,7 +2464,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped fetch: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone + .mockResolvedValue([]); // bare check: genuinely gone, and at the re-read const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, customProperties: { @@ -2303,7 +2489,7 @@ describe('AuditTrailRoute', () => { jest .spyOn(dataSource.getCollection('books'), 'list') .mockResolvedValueOnce([]) // scoped fetch: not found - .mockResolvedValueOnce([]); // bare check: genuinely gone + .mockResolvedValue([]); // bare check: genuinely gone, and at the re-read const context = createMockContext({ state: { user: { email: 'john.doe@domain.com' } }, customProperties: {