From 88d91b10af73cb3f3bff6f3b29916b7e466648c0 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Mon, 11 May 2026 23:14:06 -0500 Subject: [PATCH] activity_log: stop the bleeding - writer-leak fix on 3 sites (activityLogger middleware, alert service, login route) + one-time backfill of 548 NULL-workspace rows via device.workspace_id or workspace_members lookup; activity.js route migration deferred to its own slice tomorrow. KNOWN REGRESSION (Phase 3 fix): platform_admin / superadmin no longer has cross-workspace 'see everything' view. Every route migrated tonight (2.2a-2.2m) deliberately removed the role-based bypass per design doc - cross-workspace visibility will come via dedicated admin endpoints in Phase 3, not magic role bypasses. Until Phase 3 ships, platform admins must switch-workspace to see other workspaces' data. --- server/db/database.js | 45 +++++++++++++++++++++++++++++++++++++ server/routes/auth.js | 11 +++++++-- server/services/activity.js | 18 +++++++++++---- server/services/alerts.js | 9 ++++---- 4 files changed, 73 insertions(+), 10 deletions(-) diff --git a/server/db/database.js b/server/db/database.js index 34d84f3..7167002 100644 --- a/server/db/database.js +++ b/server/db/database.js @@ -370,6 +370,51 @@ function migrateFolderWorkspaceIds() { migrateFolderWorkspaceIds(); +const PHASE_2_2_ACTIVITY_STOP_ID = 'phase_2_2_activity_log_stop_bleeding'; + +// One-time backfill of activity_log rows that were written between the +// Phase 1 schema migration and the writer-leak fix in this commit. Strategy: +// * Rows with device_id: derive workspace_id from devices.workspace_id +// (the activity is about a specific device, so this is unambiguous). +// * Rows with no device_id but a user_id: derive from the user's oldest +// workspace_members row (pre-flight confirmed 0 affected users have +// more than one workspace, so the choice is unambiguous). +// Rows with user_id IS NULL (auth:login_failed and similar pre-tenancy +// system events) are left alone - they have no tenant context. +function backfillActivityLogWorkspace() { + const already = db.prepare('SELECT 1 FROM schema_migrations WHERE id = ?').get(PHASE_2_2_ACTIVITY_STOP_ID); + if (already) return; + + const viaDevice = db.prepare(` + UPDATE activity_log SET workspace_id = ( + SELECT workspace_id FROM devices WHERE devices.id = activity_log.device_id + ) + WHERE workspace_id IS NULL AND device_id IS NOT NULL + AND EXISTS (SELECT 1 FROM devices WHERE devices.id = activity_log.device_id AND devices.workspace_id IS NOT NULL) + `); + + const viaMembers = db.prepare(` + UPDATE activity_log SET workspace_id = ( + SELECT wm.workspace_id FROM workspace_members wm + WHERE wm.user_id = activity_log.user_id + ORDER BY wm.joined_at ASC LIMIT 1 + ) + WHERE workspace_id IS NULL AND user_id IS NOT NULL AND device_id IS NULL + AND EXISTS (SELECT 1 FROM workspace_members wm WHERE wm.user_id = activity_log.user_id) + `); + + const tx = db.transaction(() => { + const d = viaDevice.run().changes; + const m = viaMembers.run().changes; + db.prepare('INSERT OR IGNORE INTO schema_migrations (id) VALUES (?)').run(PHASE_2_2_ACTIVITY_STOP_ID); + return { d, m }; + }); + const { d, m } = tx(); + if (d + m > 0) console.log(`activity_log backfill: ${d} via device.workspace_id, ${m} via workspace_members lookup`); +} + +backfillActivityLogWorkspace(); + // Prune old telemetry (keep last 24h worth at 15s intervals = ~5760, cap at 6000) function pruneTelemetry(deviceId) { db.prepare(` diff --git a/server/routes/auth.js b/server/routes/auth.js index 2bbfa70..23ccc0d 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -56,8 +56,15 @@ function logFailedLogin(email, ip, reason) { function logSuccessfulLogin(userId, email, ip) { try { - db.prepare('INSERT INTO activity_log (user_id, action, details, ip_address) VALUES (?, ?, ?, ?)') - .run(userId, 'auth:login_success', email, ip); + // Phase 2.2 writer-leak fix: stamp the user's oldest workspace so this + // login event is queryable in tenant-scoped activity views. Multi-workspace + // users still land on one row; the activity dashboard already shows + // per-user context separately from per-workspace context. + const ws = db.prepare( + 'SELECT workspace_id FROM workspace_members WHERE user_id = ? ORDER BY joined_at ASC LIMIT 1' + ).get(userId); + db.prepare('INSERT INTO activity_log (user_id, action, details, ip_address, workspace_id) VALUES (?, ?, ?, ?, ?)') + .run(userId, 'auth:login_success', email, ip, ws?.workspace_id || null); db.prepare("UPDATE users SET last_login = strftime('%s','now') WHERE id = ?").run(userId); } catch {} } diff --git a/server/services/activity.js b/server/services/activity.js index 9662380..b9a9ddd 100644 --- a/server/services/activity.js +++ b/server/services/activity.js @@ -27,11 +27,21 @@ function getClientIp(req) { return req.ip || null; } -function logActivity(userId, action, details = null, deviceId = null, ipAddress = null) { +// Phase 2.2 writer-leak fix: activity_log rows now stamp workspace_id so +// tenant-scoped queries don't miss new events. Callers pass the workspace +// when known; the middleware below sources it from resolveTenancy. When +// workspaceId is null but a device_id is provided, fall back to the device's +// workspace - matches the backfill rule for consistency. +function logActivity(userId, action, details = null, deviceId = null, ipAddress = null, workspaceId = null) { try { + let ws = workspaceId || null; + if (!ws && deviceId) { + const d = db.prepare('SELECT workspace_id FROM devices WHERE id = ?').get(deviceId); + ws = d?.workspace_id || null; + } db.prepare( - 'INSERT INTO activity_log (user_id, device_id, action, details, ip_address) VALUES (?, ?, ?, ?, ?)' - ).run(userId || null, deviceId || null, action, details || null, ipAddress || null); + 'INSERT INTO activity_log (user_id, device_id, action, details, ip_address, workspace_id) VALUES (?, ?, ?, ?, ?, ?)' + ).run(userId || null, deviceId || null, action, details || null, ipAddress || null, ws); } catch (e) { console.error('Activity log error:', e.message); } @@ -67,7 +77,7 @@ function activityLogger(req, res, next) { const userId = req.user?.id; const deviceId = req.params?.id || req.params?.deviceId || req.body?.device_id; const details = summarizeAction(req); - logActivity(userId, action, details, deviceId, getClientIp(req)); + logActivity(userId, action, details, deviceId, getClientIp(req), req.workspaceId || null); } return originalJson(data); }; diff --git a/server/services/alerts.js b/server/services/alerts.js index c4837e5..103593f 100644 --- a/server/services/alerts.js +++ b/server/services/alerts.js @@ -17,7 +17,7 @@ function checkOfflineDevices(io) { const threshold = 300; // 5 minutes offline const offlineDevices = db.prepare(` - SELECT d.id, d.name, d.user_id, d.last_heartbeat, d.status, + SELECT d.id, d.name, d.user_id, d.workspace_id, d.last_heartbeat, d.status, u.email as owner_email, u.name as owner_name, u.email_alerts FROM devices d LEFT JOIN users u ON d.user_id = u.id @@ -42,11 +42,12 @@ function checkOfflineDevices(io) { }); offlineNotified.set(device.id, now); - // Log activity + // Log activity. Phase 2.2 writer-leak fix: stamp workspace_id from the + // device so the row is tenant-queryable. try { db.prepare( - 'INSERT INTO activity_log (user_id, device_id, action, details) VALUES (?, ?, ?, ?)' - ).run(device.user_id, device.id, 'alert:device_offline', `${device.name} offline for ${offlineMinutes}m`); + 'INSERT INTO activity_log (user_id, device_id, action, details, workspace_id) VALUES (?, ?, ?, ?, ?)' + ).run(device.user_id, device.id, 'alert:device_offline', `${device.name} offline for ${offlineMinutes}m`, device.workspace_id || null); } catch {} } }