Skip to content

Commit 4194d90

Browse files
Yerazeclaude
andauthored
fix: database audit phase 2 — PG/MySQL crash prevention and schema alignment (#2386)
- Fix getAuditStatsAsync using wrong column names (user_id → userId) for PG/MySQL - Add PG/MySQL guards to 6 maintenance methods that crashed via this.db.prepare() - Add repo methods: cleanupOldRouteSegments (traceroutes), cleanupOldNeighborInfo (neighbors) - Make async wrappers properly delegate to repos on PG/MySQL instead of calling sync methods - Add getDatabaseSizeAsync with pg_database_size()/information_schema for PG/MySQL - Add vacuumAsync with native VACUUM for PostgreSQL - Add grantedAt/grantedBy to PG/MySQL permission Drizzle schemas and baselines - Add valueBefore/valueAfter to PG/MySQL audit_log baselines - Remove auth repo branching that stripped grantedAt/grantedBy on PG/MySQL Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 15cabd1 commit 4194d90

File tree

6 files changed

+170
-27
lines changed

6 files changed

+170
-27
lines changed

src/db/repositories/auth.ts

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -91,9 +91,9 @@ export interface DbPermission {
9191
canViewOnMap: boolean;
9292
canRead: boolean;
9393
canWrite: boolean;
94-
canDelete?: boolean; // PostgreSQL only
95-
grantedAt?: number; // SQLite only
96-
grantedBy?: number | null; // SQLite only
94+
canDelete?: boolean; // PostgreSQL/MySQL only
95+
grantedAt?: number;
96+
grantedBy?: number | null;
9797
}
9898

9999
/**
@@ -105,9 +105,9 @@ export interface CreatePermissionInput {
105105
canViewOnMap?: boolean;
106106
canRead?: boolean;
107107
canWrite?: boolean;
108-
canDelete?: boolean; // PostgreSQL only
109-
grantedAt?: number; // SQLite only
110-
grantedBy?: number | null; // SQLite only
108+
canDelete?: boolean; // PostgreSQL/MySQL only
109+
grantedAt?: number;
110+
grantedBy?: number | null;
111111
}
112112

113113
/**
@@ -309,31 +309,28 @@ export class AuthRepository extends BaseRepository {
309309

310310
/**
311311
* Create permission.
312-
* Keeps branching: different columns per dialect (SQLite has grantedAt/grantedBy, others have canDelete).
312+
* Keeps branching: SQLite doesn't have canDelete, PG/MySQL do.
313+
* All backends now support grantedAt/grantedBy.
313314
*/
314315
async createPermission(permission: CreatePermissionInput): Promise<number> {
315316
const { permissions } = this.tables;
317+
const permissionWithGrantedAt = {
318+
...permission,
319+
grantedAt: permission.grantedAt ?? Date.now(),
320+
};
316321
if (this.isSQLite()) {
317322
const db = this.getSqliteDb();
318-
// SQLite requires grantedAt, doesn't have canDelete
319-
const { canDelete, ...rest } = permission;
320-
const sqlitePermission = {
321-
...rest,
322-
grantedAt: permission.grantedAt ?? Date.now(),
323-
};
324-
const result = await db.insert(permissions).values(sqlitePermission);
323+
// SQLite doesn't have canDelete
324+
const { canDelete, ...rest } = permissionWithGrantedAt;
325+
const result = await db.insert(permissions).values(rest);
325326
return Number(result.lastInsertRowid);
326327
} else if (this.isMySQL()) {
327328
const db = this.getMysqlDb();
328-
// MySQL doesn't have grantedAt/grantedBy but has canDelete
329-
const { grantedAt, grantedBy, ...mysqlPermission } = permission;
330-
const result = await db.insert(permissions).values(mysqlPermission);
329+
const result = await db.insert(permissions).values(permissionWithGrantedAt);
331330
return Number(result[0].insertId);
332331
} else {
333332
const db = this.getPostgresDb();
334-
// PostgreSQL doesn't have grantedAt/grantedBy
335-
const { grantedAt, grantedBy, ...postgresPermission } = permission;
336-
const result = await db.insert(permissions).values(postgresPermission).returning({ id: permissionsPostgres.id });
333+
const result = await db.insert(permissions).values(permissionWithGrantedAt).returning({ id: permissionsPostgres.id });
337334
return result[0].id;
338335
}
339336
}

src/db/repositories/neighbors.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* Handles neighbor info database operations.
55
* Supports SQLite, PostgreSQL, and MySQL through Drizzle ORM.
66
*/
7-
import { eq, desc, and, gte, sql, count } from 'drizzle-orm';
7+
import { eq, desc, and, gte, lt, sql, count } from 'drizzle-orm';
88
import { BaseRepository, DrizzleDatabase } from './base.js';
99
import { DatabaseType, DbNeighborInfo } from '../types.js';
1010

@@ -136,6 +136,23 @@ export class NeighborsRepository extends BaseRepository {
136136
await this.db.delete(neighborInfo);
137137
}
138138

139+
/**
140+
* Delete neighbor info records older than the specified number of days
141+
*/
142+
async cleanupOldNeighborInfo(days: number = 30): Promise<number> {
143+
const cutoff = Date.now() - (days * 24 * 60 * 60 * 1000);
144+
const { neighborInfo } = this.tables;
145+
const toDelete = await this.db
146+
.select({ count: count() })
147+
.from(neighborInfo)
148+
.where(lt(neighborInfo.timestamp, cutoff));
149+
const total = Number(toDelete[0].count);
150+
if (total > 0) {
151+
await this.db.delete(neighborInfo).where(lt(neighborInfo.timestamp, cutoff));
152+
}
153+
return total;
154+
}
155+
139156
/**
140157
* Get direct neighbor RSSI statistics from zero-hop packets
141158
*

src/db/repositories/traceroutes.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,24 @@ export class TraceroutesRepository extends BaseRepository {
316316
return total;
317317
}
318318

319+
/**
320+
* Delete old route segments that are not record holders
321+
*/
322+
async cleanupOldRouteSegments(days: number = 30): Promise<number> {
323+
const cutoff = this.now() - (days * 24 * 60 * 60 * 1000);
324+
const { routeSegments } = this.tables;
325+
const toDelete = await this.db
326+
.select({ count: count() })
327+
.from(routeSegments)
328+
.where(and(lt(routeSegments.timestamp, cutoff), eq(routeSegments.isRecordHolder, false)));
329+
const total = Number(toDelete[0].count);
330+
if (total > 0) {
331+
await this.db.delete(routeSegments)
332+
.where(and(lt(routeSegments.timestamp, cutoff), eq(routeSegments.isRecordHolder, false)));
333+
}
334+
return total;
335+
}
336+
319337
/**
320338
* Delete all route segments
321339
*/

src/db/schema/auth.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ export const permissionsPostgres = pgTable('permissions', {
6969
canRead: pgBoolean('canRead').notNull().default(false),
7070
canWrite: pgBoolean('canWrite').notNull().default(false),
7171
canDelete: pgBoolean('canDelete').notNull().default(false),
72+
grantedAt: pgBigint('grantedAt', { mode: 'number' }),
73+
grantedBy: pgInteger('grantedBy').references(() => usersPostgres.id, { onDelete: 'set null' }),
7274
});
7375

7476
// ============ SESSIONS ============
@@ -172,6 +174,8 @@ export const permissionsMysql = mysqlTable('permissions', {
172174
canRead: myBoolean('canRead').notNull().default(false),
173175
canWrite: myBoolean('canWrite').notNull().default(false),
174176
canDelete: myBoolean('canDelete').notNull().default(false),
177+
grantedAt: myBigint('grantedAt', { mode: 'number' }),
178+
grantedBy: myInt('grantedBy').references(() => usersMysql.id, { onDelete: 'set null' }),
175179
});
176180

177181
export const sessionsMysql = mysqlTable('sessions', {

src/server/migrations/001_v37_baseline.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -959,7 +959,9 @@ export async function runMigration001Postgres(client: PoolClient): Promise<void>
959959
"canViewOnMap" BOOLEAN NOT NULL DEFAULT false,
960960
"canRead" BOOLEAN NOT NULL DEFAULT false,
961961
"canWrite" BOOLEAN NOT NULL DEFAULT false,
962-
"canDelete" BOOLEAN NOT NULL DEFAULT false
962+
"canDelete" BOOLEAN NOT NULL DEFAULT false,
963+
"grantedAt" BIGINT,
964+
"grantedBy" INTEGER REFERENCES users(id) ON DELETE SET NULL
963965
);
964966
965967
CREATE TABLE IF NOT EXISTS sessions (
@@ -977,6 +979,8 @@ export async function runMigration001Postgres(client: PoolClient): Promise<void>
977979
details TEXT,
978980
"ipAddress" TEXT,
979981
"userAgent" TEXT,
982+
"valueBefore" TEXT,
983+
"valueAfter" TEXT,
980984
timestamp BIGINT NOT NULL
981985
);
982986
@@ -1494,7 +1498,10 @@ export async function runMigration001Mysql(pool: MySQLPool): Promise<void> {
14941498
canRead BOOLEAN NOT NULL DEFAULT false,
14951499
canWrite BOOLEAN NOT NULL DEFAULT false,
14961500
canDelete BOOLEAN NOT NULL DEFAULT false,
1497-
FOREIGN KEY (userId) REFERENCES users(id) ON DELETE CASCADE
1501+
grantedAt BIGINT,
1502+
grantedBy INT,
1503+
FOREIGN KEY (userId) REFERENCES users(id) ON DELETE CASCADE,
1504+
FOREIGN KEY (grantedBy) REFERENCES users(id) ON DELETE SET NULL
14981505
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci`,
14991506

15001507
`CREATE TABLE IF NOT EXISTS sessions (
@@ -1553,6 +1560,8 @@ export async function runMigration001Mysql(pool: MySQLPool): Promise<void> {
15531560
details TEXT,
15541561
ipAddress VARCHAR(255),
15551562
userAgent TEXT,
1563+
valueBefore TEXT,
1564+
valueAfter TEXT,
15561565
timestamp BIGINT NOT NULL,
15571566
FOREIGN KEY (userId) REFERENCES users(id) ON DELETE SET NULL,
15581567
INDEX idx_audit_log_timestamp (timestamp)

src/services/database.ts

Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7369,6 +7369,16 @@ class DatabaseService {
73697369
}
73707370

73717371
cleanupOldRouteSegments(days: number = 30): number {
7372+
// For PostgreSQL/MySQL, fire-and-forget async cleanup
7373+
if (this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') {
7374+
if (this.traceroutesRepo) {
7375+
this.traceroutesRepo.cleanupOldRouteSegments(days).catch(err => {
7376+
logger.debug('Failed to cleanup old route segments:', err);
7377+
});
7378+
}
7379+
return 0;
7380+
}
7381+
73727382
const cutoff = Date.now() - (days * 24 * 60 * 60 * 1000);
73737383
const stmt = this.db.prepare(`
73747384
DELETE FROM route_segments
@@ -7382,6 +7392,16 @@ class DatabaseService {
73827392
* Delete traceroutes older than the specified number of days
73837393
*/
73847394
cleanupOldTraceroutes(days: number = 30): number {
7395+
// For PostgreSQL/MySQL, fire-and-forget async cleanup
7396+
if (this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') {
7397+
if (this.traceroutesRepo) {
7398+
this.traceroutesRepo.cleanupOldTraceroutes(days * 24).catch(err => {
7399+
logger.debug('Failed to cleanup old traceroutes:', err);
7400+
});
7401+
}
7402+
return 0;
7403+
}
7404+
73857405
const cutoff = Date.now() - (days * 24 * 60 * 60 * 1000);
73867406
const stmt = this.db.prepare('DELETE FROM traceroutes WHERE timestamp < ?');
73877407
const result = stmt.run(cutoff);
@@ -7392,6 +7412,16 @@ class DatabaseService {
73927412
* Delete neighbor info records older than the specified number of days
73937413
*/
73947414
cleanupOldNeighborInfo(days: number = 30): number {
7415+
// For PostgreSQL/MySQL, fire-and-forget async cleanup
7416+
if (this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') {
7417+
if (this.neighborsRepo) {
7418+
this.neighborsRepo.cleanupOldNeighborInfo(days).catch(err => {
7419+
logger.debug('Failed to cleanup old neighbor info:', err);
7420+
});
7421+
}
7422+
return 0;
7423+
}
7424+
73957425
const cutoff = Date.now() - (days * 24 * 60 * 60 * 1000);
73967426
const stmt = this.db.prepare('DELETE FROM neighbor_info WHERE timestamp < ?');
73977427
const result = stmt.run(cutoff);
@@ -7403,6 +7433,23 @@ class DatabaseService {
74037433
* This can take a while on large databases and temporarily doubles disk usage
74047434
*/
74057435
vacuum(): void {
7436+
// For PostgreSQL/MySQL, use native vacuum/optimize
7437+
if (this.drizzleDbType === 'postgres') {
7438+
logger.info('🧹 Running VACUUM on PostgreSQL database...');
7439+
this.postgresPool!.query('VACUUM').then(() => {
7440+
logger.info('✅ PostgreSQL VACUUM complete');
7441+
}).catch(err => {
7442+
logger.error('Failed to VACUUM PostgreSQL:', err);
7443+
});
7444+
return;
7445+
}
7446+
if (this.drizzleDbType === 'mysql') {
7447+
logger.info('🧹 Running OPTIMIZE TABLE on MySQL database...');
7448+
// MySQL OPTIMIZE TABLE requires table names; skip for now as it's not critical
7449+
logger.info('✅ MySQL OPTIMIZE TABLE skipped (not critical)');
7450+
return;
7451+
}
7452+
74067453
logger.info('🧹 Running VACUUM on database...');
74077454
this.db.exec('VACUUM');
74087455
logger.info('✅ VACUUM complete');
@@ -7412,6 +7459,17 @@ class DatabaseService {
74127459
* Get the current database file size in bytes
74137460
*/
74147461
getDatabaseSize(): number {
7462+
// For PostgreSQL, use pg_database_size()
7463+
if (this.drizzleDbType === 'postgres') {
7464+
// Return 0 from sync context; use getDatabaseSizeAsync for accurate results
7465+
return 0;
7466+
}
7467+
// For MySQL, use information_schema
7468+
if (this.drizzleDbType === 'mysql') {
7469+
// Return 0 from sync context; use getDatabaseSizeAsync for accurate results
7470+
return 0;
7471+
}
7472+
74157473
const stmt = this.db.prepare('SELECT page_count * page_size as size FROM pragma_page_count(), pragma_page_size()');
74167474
const result = stmt.get() as { size: number } | undefined;
74177475
return result?.size ?? 0;
@@ -8469,8 +8527,8 @@ class DatabaseService {
84698527
[cutoff]
84708528
);
84718529
const userStats = await client.query(
8472-
`SELECT u.username, COUNT(*) as count FROM audit_log al LEFT JOIN users u ON al.user_id = u.id
8473-
WHERE al.timestamp >= $1 GROUP BY al.user_id, u.username ORDER BY count DESC LIMIT 10`,
8530+
`SELECT u.username, COUNT(*) as count FROM audit_log al LEFT JOIN users u ON al."userId" = u.id
8531+
WHERE al.timestamp >= $1 GROUP BY al."userId", u.username ORDER BY count DESC LIMIT 10`,
84748532
[cutoff]
84758533
);
84768534
const dailyStats = await client.query(
@@ -8497,8 +8555,8 @@ class DatabaseService {
84978555
[cutoff]
84988556
);
84998557
const [userRows] = await pool.query(
8500-
`SELECT u.username, COUNT(*) as count FROM audit_log al LEFT JOIN users u ON al.user_id = u.id
8501-
WHERE al.timestamp >= ? GROUP BY al.user_id ORDER BY count DESC LIMIT 10`,
8558+
`SELECT u.username, COUNT(*) as count FROM audit_log al LEFT JOIN users u ON al.userId = u.id
8559+
WHERE al.timestamp >= ? GROUP BY al.userId ORDER BY count DESC LIMIT 10`,
85028560
[cutoff]
85038561
);
85048562
const [dailyRows] = await pool.query(
@@ -8521,6 +8579,16 @@ class DatabaseService {
85218579

85228580
// Cleanup old audit logs
85238581
cleanupAuditLogs(days: number): number {
8582+
// For PostgreSQL/MySQL, fire-and-forget async cleanup
8583+
if (this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') {
8584+
if (this.authRepo) {
8585+
this.authRepo.cleanupOldAuditLogs(days).catch(err => {
8586+
logger.debug('Failed to cleanup old audit logs:', err);
8587+
});
8588+
}
8589+
return 0;
8590+
}
8591+
85248592
const cutoff = Date.now() - (days * 24 * 60 * 60 * 1000);
85258593
const stmt = this.db.prepare('DELETE FROM audit_log WHERE timestamp < ?');
85268594
const result = stmt.run(cutoff);
@@ -10132,14 +10200,23 @@ class DatabaseService {
1013210200
}
1013310201

1013410202
async cleanupOldTraceroutesAsync(days: number = 30): Promise<number> {
10203+
if ((this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') && this.traceroutesRepo) {
10204+
return this.traceroutesRepo.cleanupOldTraceroutes(days * 24);
10205+
}
1013510206
return this.cleanupOldTraceroutes(days);
1013610207
}
1013710208

1013810209
async cleanupOldRouteSegmentsAsync(days: number = 30): Promise<number> {
10210+
if ((this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') && this.traceroutesRepo) {
10211+
return this.traceroutesRepo.cleanupOldRouteSegments(days);
10212+
}
1013910213
return this.cleanupOldRouteSegments(days);
1014010214
}
1014110215

1014210216
async cleanupOldNeighborInfoAsync(days: number = 30): Promise<number> {
10217+
if ((this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') && this.neighborsRepo) {
10218+
return this.neighborsRepo.cleanupOldNeighborInfo(days);
10219+
}
1014310220
return this.cleanupOldNeighborInfo(days);
1014410221
}
1014510222

@@ -10152,14 +10229,35 @@ class DatabaseService {
1015210229
}
1015310230

1015410231
async cleanupAuditLogsAsync(days: number): Promise<number> {
10232+
if ((this.drizzleDbType === 'postgres' || this.drizzleDbType === 'mysql') && this.authRepo) {
10233+
return this.authRepo.cleanupOldAuditLogs(days);
10234+
}
1015510235
return this.cleanupAuditLogs(days);
1015610236
}
1015710237

1015810238
async vacuumAsync(): Promise<void> {
10239+
if (this.drizzleDbType === 'postgres') {
10240+
await this.postgresPool!.query('VACUUM');
10241+
return;
10242+
}
10243+
if (this.drizzleDbType === 'mysql') {
10244+
// MySQL auto-manages space; OPTIMIZE TABLE requires table names
10245+
return;
10246+
}
1015910247
return this.vacuum();
1016010248
}
1016110249

1016210250
async getDatabaseSizeAsync(): Promise<number> {
10251+
if (this.drizzleDbType === 'postgres') {
10252+
const result = await this.postgresPool!.query('SELECT pg_database_size(current_database()) as size');
10253+
return Number(result.rows[0]?.size ?? 0);
10254+
}
10255+
if (this.drizzleDbType === 'mysql') {
10256+
const [rows] = await this.mysqlPool!.query(
10257+
`SELECT SUM(data_length + index_length) as size FROM information_schema.tables WHERE table_schema = DATABASE()`
10258+
);
10259+
return Number((rows as any[])[0]?.size ?? 0);
10260+
}
1016310261
return this.getDatabaseSize();
1016410262
}
1016510263

0 commit comments

Comments
 (0)