diff --git a/README.md b/README.md index 93ebf49..fa27a2f 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

- Version + Version License Docker TypeScript diff --git a/backend/package.json b/backend/package.json index cef2ccd..d0ee2a3 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.24.3-beta", + "version": "0.24.4-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/services/mikrotik/DeviceCollector.ts b/backend/src/services/mikrotik/DeviceCollector.ts index 1e75079..9528c6f 100644 --- a/backend/src/services/mikrotik/DeviceCollector.ts +++ b/backend/src/services/mikrotik/DeviceCollector.ts @@ -30,7 +30,7 @@ const UPDATE_DOWNLOAD_TIMEOUT_MS = 10 * 60_000; * learned to abandon a timed-out connection, desynchronised the session (#137). */ const LOG_READ_TIMEOUT_MS = 120_000; -import { selectNewLogLines, highestStoredId, type RawLogLine } from '../../utils/logDedup'; +import { selectNewLogLines, highestStoredId, surrogateLogId, type RawLogLine } from '../../utils/logDedup'; import { alertService } from '../AlertService'; /** DB column limits for topology_links (see migrate.ts); reject oversize rows instead of silent truncation. */ @@ -852,17 +852,23 @@ export class DeviceCollector { const pending: unknown[][] = []; for (const log of fresh) { - const logId = (log['.id'] || '') as string; + const rawId = (log['.id'] || '') as string; + const topics = (log['topics'] as string) || ''; + const message = (log['message'] as string) || ''; const time = this.parseLogTime(log['time'] || ''); - const severity = this.mapLogSeverity((log['topics'] as string) || ''); + const severity = this.mapLogSeverity(topics); + // Never store a null log_id: the unique index that stops re-insertion + // does not constrain nulls, so a device whose lines carry no `.id` + // would accumulate the same entries for ever (#137). + const logId = rawId || surrogateLogId(log['time'] || '', topics, message); pending.push([ this.device.id, time.toISOString(), severity, - (log['topics'] as string) || null, - (log['message'] as string) || '', + topics || null, + message, JSON.stringify(log), - logId || null, + logId, ]); } diff --git a/backend/src/utils/__tests__/logDedup.test.ts b/backend/src/utils/__tests__/logDedup.test.ts index 2f9d204..d937c7e 100644 --- a/backend/src/utils/__tests__/logDedup.test.ts +++ b/backend/src/utils/__tests__/logDedup.test.ts @@ -1,5 +1,6 @@ import { - parseRosId, highestStoredId, isLogReset, selectNewLogLines, type RawLogLine, + parseRosId, highestStoredId, isLogReset, selectNewLogLines, + surrogateLogId, isSurrogateLogId, type RawLogLine, } from '../logDedup'; // Fixed clock so timestamp comparisons are deterministic. @@ -91,3 +92,49 @@ describe('selectNewLogLines', () => { expect(selectNewLogLines(logs, { lastStoredId: 0, latestStoredTime, parseTime })).toHaveLength(0); }); }); + +describe('surrogateLogId', () => { + const line = ['sep/08 02:38:10', 'system,info', 'router rebooted'] as const; + + it('is stable for the same line', () => { + expect(surrogateLogId(...line)).toBe(surrogateLogId(...line)); + }); + + it('differs when any field differs', () => { + const base = surrogateLogId(...line); + expect(surrogateLogId('sep/08 02:38:11', line[1], line[2])).not.toBe(base); + expect(surrogateLogId(line[0], 'system,error', line[2])).not.toBe(base); + expect(surrogateLogId(line[0], line[1], 'something else')).not.toBe(base); + }); + + // Field boundaries are separated, so shifting text between fields cannot + // produce the same key. + it('does not confuse field boundaries', () => { + expect(surrogateLogId('a', 'b', 'c')).not.toBe(surrogateLogId('ab', '', 'c')); + }); + + // The column is VARCHAR(20); an over-long key would throw on insert. + it('fits the log_id column', () => { + const id = surrogateLogId('x'.repeat(500), 'y'.repeat(500), 'z'.repeat(5000)); + expect(id.length).toBeLessThanOrEqual(20); + }); + + it('cannot be mistaken for a RouterOS id', () => { + const id = surrogateLogId(...line); + expect(id.startsWith('#')).toBe(true); + expect(isSurrogateLogId(id)).toBe(true); + expect(isSurrogateLogId('*CC75')).toBe(false); + expect(isSurrogateLogId(null)).toBe(false); + }); + + /** + * The watermark is a max over RouterOS's own hex ids. A synthesised id must + * not be parsed as one, or it would poison the high-water mark and make every + * real line look already-seen. + */ + it('is ignored by the id watermark', () => { + const id = surrogateLogId(...line); + expect(parseRosId(id)).toBe(0); + expect(highestStoredId([id, '*10'])).toBe(0x10); + }); +}); diff --git a/backend/src/utils/logDedup.ts b/backend/src/utils/logDedup.ts index ce53d57..6c424ff 100644 --- a/backend/src/utils/logDedup.ts +++ b/backend/src/utils/logDedup.ts @@ -7,6 +7,8 @@ * the entire buffer once a minute. */ +import { createHash } from 'crypto'; + export interface RawLogLine { '.id'?: string; time?: string; @@ -81,3 +83,35 @@ export function selectNewLogLines( } return out; } + +/** + * A stable identifier for a log line that RouterOS gave no `.id`. + * + * The events table has a unique index on (device_id, log_id) and relies on it + * to reject lines already stored. PostgreSQL treats NULLs as *distinct* in a + * unique index, so a NULL log_id conflicts with nothing: a device whose log + * entries carry no `.id` had no protection at all and re-inserted the same + * lines on every poll, without bound. + * + * Deriving the key from the line's own content restores that protection. The + * `#` prefix keeps it out of the space of real RouterOS ids, which are `*` + * followed by hex, so the two can never collide or be mistaken for each other. + * + * The trade, stated plainly: two genuinely separate lines with the same + * timestamp, topic and text collapse into one. RouterOS timestamps are + * second-resolution, so that is possible. Losing a repeat of an identical + * message is a far smaller loss than growing the table by the whole buffer + * every minute for ever. + */ +export function surrogateLogId(time: string, topics: string, message: string): string { + const hash = createHash('sha256') + .update(`${time}\u0000${topics}\u0000${message}`) + .digest('hex') + .slice(0, 16); + return `#${hash}`; // 17 chars, inside the column's VARCHAR(20) +} + +/** True for the ids this module synthesised, as opposed to RouterOS's own. */ +export function isSurrogateLogId(id: string | null | undefined): boolean { + return typeof id === 'string' && id.startsWith('#'); +} diff --git a/frontend/package.json b/frontend/package.json index b98ba30..9edff39 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.24.3-beta", + "version": "0.24.4-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/version.ts b/frontend/src/version.ts index d056b94..7b1d20b 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.24.3 Beta'; +export const APP_VERSION = 'v0.24.4 Beta';