diff --git a/apps/api/migrations/2026-07-17-a-device-vulnerabilities-software-fk-set-null.sql b/apps/api/migrations/2026-07-17-a-device-vulnerabilities-software-fk-set-null.sql new file mode 100644 index 0000000000..d5ec7ca260 --- /dev/null +++ b/apps/api/migrations/2026-07-17-a-device-vulnerabilities-software-fk-set-null.sql @@ -0,0 +1,51 @@ +-- device_vulnerabilities.software_inventory_id had no ON DELETE action, so the +-- agent software report (PUT /agents/:id/software), which wipes and reinserts a +-- device's software_inventory rows, failed with FK violation 23503 for any +-- device that had correlated software findings — freezing that device's +-- software inventory permanently (Sentry BREEZE-3). SET NULL matches the +-- correlation service's semantics: upsertDeviceVulnerability re-links the +-- finding to the current inventory row on the next scan, and the report route +-- re-links surviving software in the same transaction. +-- +-- The constraint is added NOT VALID to skip the full-table validation scan +-- while ALTER TABLE holds its lock on both tables (agent reports and +-- correlation writes would queue behind it during deploy). Existing rows are +-- guaranteed valid — the dropped constraint enforced the same reference — and +-- the companion -b- migration VALIDATEs in a separate transaction under the +-- weaker SHARE UPDATE EXCLUSIVE lock. +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'device_vulnerabilities_software_inventory_id_fkey' + AND conrelid = 'device_vulnerabilities'::regclass + AND confdeltype <> 'n' + ) THEN + ALTER TABLE device_vulnerabilities + DROP CONSTRAINT device_vulnerabilities_software_inventory_id_fkey; + END IF; + + IF NOT EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'device_vulnerabilities_software_inventory_id_fkey' + AND conrelid = 'device_vulnerabilities'::regclass + ) THEN + ALTER TABLE device_vulnerabilities + ADD CONSTRAINT device_vulnerabilities_software_inventory_id_fkey + FOREIGN KEY (software_inventory_id) REFERENCES software_inventory(id) + ON DELETE SET NULL + NOT VALID; + END IF; +END $$; + +-- The SET NULL referential trigger fires once per deleted software_inventory +-- row (up to 10k per software report) and executes +-- `UPDATE device_vulnerabilities SET software_inventory_id = NULL WHERE +-- software_inventory_id = $1` — RI triggers bypass RLS, so without an index +-- each firing is a scan of the whole multi-tenant table. The same index serves +-- the report route's finding-snapshot join. +CREATE INDEX IF NOT EXISTS device_vuln_software_inventory_idx + ON device_vulnerabilities (software_inventory_id) + WHERE software_inventory_id IS NOT NULL; diff --git a/apps/api/migrations/2026-07-17-b-device-vulnerabilities-software-fk-validate.sql b/apps/api/migrations/2026-07-17-b-device-vulnerabilities-software-fk-validate.sql new file mode 100644 index 0000000000..3e45500a58 --- /dev/null +++ b/apps/api/migrations/2026-07-17-b-device-vulnerabilities-software-fk-validate.sql @@ -0,0 +1,17 @@ +-- Companion to 2026-07-17-a: VALIDATE the NOT VALID FK in its own transaction. +-- VALIDATE CONSTRAINT takes SHARE UPDATE EXCLUSIVE, so agent reports and +-- correlation writes proceed during the scan (unlike a validating ADD +-- CONSTRAINT, which would hold the ALTER's exclusive lock for the duration). +DO $$ +BEGIN + IF EXISTS ( + SELECT 1 + FROM pg_constraint + WHERE conname = 'device_vulnerabilities_software_inventory_id_fkey' + AND conrelid = 'device_vulnerabilities'::regclass + AND NOT convalidated + ) THEN + ALTER TABLE device_vulnerabilities + VALIDATE CONSTRAINT device_vulnerabilities_software_inventory_id_fkey; + END IF; +END $$; diff --git a/apps/api/src/db/schema/vulnerabilityManagement.ts b/apps/api/src/db/schema/vulnerabilityManagement.ts index 8338400fad..e29918e4ad 100644 --- a/apps/api/src/db/schema/vulnerabilityManagement.ts +++ b/apps/api/src/db/schema/vulnerabilityManagement.ts @@ -127,7 +127,7 @@ export const deviceVulnerabilities = pgTable('device_vulnerabilities', { orgId: uuid('org_id').notNull().references(() => organizations.id), deviceId: uuid('device_id').notNull().references(() => devices.id), vulnerabilityId: uuid('vulnerability_id').notNull().references(() => vulnerabilities.id), - softwareInventoryId: uuid('software_inventory_id').references(() => softwareInventory.id), + softwareInventoryId: uuid('software_inventory_id').references(() => softwareInventory.id, { onDelete: 'set null' }), status: varchar('status', { length: 20 }).notNull().default('open'), // lifecycle: see VULN_STATUSES in @breeze/shared (source of truth: open|patched|mitigated|accepted) riskScore: numeric('risk_score', { precision: 5, scale: 2 }), // CVSS-primary matchConfidence: varchar('match_confidence', { length: 16 }), @@ -143,4 +143,7 @@ export const deviceVulnerabilities = pgTable('device_vulnerabilities', { orgDeviceIdx: index('device_vuln_org_device_idx').on(table.orgId, table.deviceId), statusIdx: index('device_vuln_status_idx').on(table.status), ticketIdx: index('device_vuln_ticket_id_idx').on(table.ticketId).where(sql`ticket_id IS NOT NULL`), + // Serves the ON DELETE SET NULL referential trigger, which fires per deleted + // software_inventory row (bypassing RLS) and would otherwise scan this table. + softwareInventoryIdx: index('device_vuln_software_inventory_idx').on(table.softwareInventoryId).where(sql`software_inventory_id IS NOT NULL`), })); diff --git a/apps/api/src/routes/agentWs.enqueueContract.test.ts b/apps/api/src/routes/agentWs.enqueueContract.test.ts new file mode 100644 index 0000000000..4b3cd32236 --- /dev/null +++ b/apps/api/src/routes/agentWs.enqueueContract.test.ts @@ -0,0 +1,39 @@ +/** + * Static contract (#1105, BREEZE-H): every BullMQ enqueue call site in + * agentWs.ts runs under runOutsideDbContext. The whole command_result pipeline + * executes inside a held org-scoped transaction (runWithAgentDbAccess), so an + * unwrapped enqueue pins a pooled Postgres connection idle-in-transaction + * across Redis round-trips — and for instrumented queues fires the + * assertOutsideHeldDbContext tripwire straight into Sentry. Seven sites were + * fixed in one pass; this scan keeps the next one from regressing silently. + */ +import { describe, expect, it } from 'vitest'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; + +const source = readFileSync(path.join(__dirname, 'agentWs.ts'), 'utf8'); + +describe('agentWs enqueue context contract (#1105)', () => { + it('every enqueue* call site is wrapped in runOutsideDbContext', () => { + const lines = source.split('\n'); + const callSites: number[] = []; + lines.forEach((line, idx) => { + if (!/\benqueue[A-Z]\w*\s*\(/.test(line)) return; + if (/^\s*import\b/.test(line) || /\bfrom '/.test(line) || /await import\(/.test(line)) return; + callSites.push(idx); + }); + + // Known sites: monitor, SNMP (orphaned + tracked), discovery x2, backup, + // DR reconcile. If the scan finds fewer, the regex rotted — fix the scan, + // don't delete the assertion. + expect(callSites.length).toBeGreaterThanOrEqual(7); + + for (const idx of callSites) { + const window = lines.slice(Math.max(0, idx - 3), idx + 1).join('\n'); + expect( + window.includes('runOutsideDbContext('), + `enqueue call at agentWs.ts:${idx + 1} must be wrapped in runOutsideDbContext (#1105):\n${lines[idx]}` + ).toBe(true); + } + }); +}); diff --git a/apps/api/src/routes/agentWs.test.ts b/apps/api/src/routes/agentWs.test.ts index bcc5d20418..e630aedbcc 100644 --- a/apps/api/src/routes/agentWs.test.ts +++ b/apps/api/src/routes/agentWs.test.ts @@ -187,7 +187,7 @@ vi.mock('../services/backupResultPersistence', async (importOriginal) => { }; }); -import { db } from '../db'; +import { db, runOutsideDbContext } from '../db'; import { devices } from '../db/schema'; import { createAgentWsHandlers, @@ -195,9 +195,11 @@ import { validateAgentToken, disconnectAgent, isAgentConnected, + sendCommandToAgent, __resetCrossTenantDropsForTest, AGENT_WS_CAPABILITIES, } from './agentWs'; +import { isRedisAvailable } from '../services/redis'; import { claimPendingCommandsForDevice } from '../services/commandDispatch'; import { writeAuditEvent } from '../services/auditEvents'; import { isAgentTenantActive } from '../services/tenantStatus'; @@ -770,6 +772,69 @@ describe('agent websocket command results', () => { expect(ws.send).toHaveBeenCalledWith(expect.stringContaining('"ack"')); }); + // BREEZE-H: the command_result handler runs inside a held org-scoped + // transaction; BullMQ enqueues must be wrapped in runOutsideDbContext so + // the #1105 tripwire in bullmqQueue passes and nested DB work routes to + // the pool (the wrap does not release the outer transaction's connection — + // dispatching after the context closes is the deeper #1105 fix). + it('enqueues an accepted monitor result via runOutsideDbContext (#1105, BREEZE-H)', async () => { + const preValidatedAgent = { deviceId: 'device-123', orgId: 'org-123' }; + + // onOpen: register the socket so sendCommandToAgent records the + // orphaned-result expectation the monitor result is matched against. + vi.mocked(db.select).mockReturnValue(selectAgentDevice([]) as any); + vi.mocked(db.update).mockReturnValue(updateResult() as any); + const handlers = createAgentWsHandlers('agent-123', preValidatedAgent); + const ws = wsMock(); + await handlers.onOpen({}, ws as any); + + sendCommandToAgent('agent-123', { + id: 'mon-monitor-1-123', + type: 'network_ping', + payload: { monitorId: 'monitor-1' }, + } as any); + + vi.mocked(isRedisAvailable).mockReturnValue(true); + let outsideDepth = 0; + vi.mocked(runOutsideDbContext).mockImplementation((fn: any) => { + outsideDepth++; + try { + return fn(); + } finally { + outsideDepth--; + } + }); + // Record (not assert) inside the mock: the handler catches enqueue errors, + // so a failing expect() inside the implementation would be swallowed. + let enqueuedOutsideContext: boolean | null = null; + vi.mocked(enqueueMonitorCheckResult).mockImplementation(async () => { + enqueuedOutsideContext = outsideDepth > 0; + return 'job-1'; + }); + + await handlers.onMessage({ + data: JSON.stringify({ + type: 'command_result', + commandId: 'mon-monitor-1-123', + status: 'completed', + result: { + monitorId: 'monitor-1', + status: 'online', + responseMs: 12 + } + }) + } as any, ws as any); + + expect(enqueueMonitorCheckResult).toHaveBeenCalledTimes(1); + expect(enqueueMonitorCheckResult).toHaveBeenCalledWith( + 'monitor-1', + expect.objectContaining({ monitorId: 'monitor-1', status: 'online', responseMs: 12 }), + expect.objectContaining({ source: 'route:agentWs:monitor-result' }) + ); + expect(enqueuedOutsideContext).toBe(true); + expect(ws.send).toHaveBeenCalledWith(expect.stringContaining('"ack"')); + }); + it('drops terminal output for sessions not owned by the connected agent', async () => { const preValidatedAgent = { deviceId: 'device-123', orgId: 'org-123' }; diff --git a/apps/api/src/routes/agentWs.ts b/apps/api/src/routes/agentWs.ts index fa2f9d9622..3af5d0d5d8 100644 --- a/apps/api/src/routes/agentWs.ts +++ b/apps/api/src/routes/agentWs.ts @@ -190,11 +190,14 @@ async function handleDiscoveryResult({ agentId, command, result }: Parameters enqueueDiscoveryResults( expectedJobId, job.orgId, job.siteId, - normalizeDiscoveryHosts(discoveryData.hosts), + normalizedHosts, discoveryData.hostsScanned ?? 0, discoveryData.hostsDiscovered ?? 0, undefined, @@ -204,7 +207,7 @@ async function handleDiscoveryResult({ agentId, command, result }: Parameters enqueueSnmpPollResults(expectedDeviceId, metrics)); } else { // Redis not available — log warning about dropped metrics and mark status console.warn(`[AgentWs] Redis unavailable, dropping ${snmpData.metrics.length} SNMP metrics for device ${expectedDeviceId}`); @@ -382,6 +388,7 @@ async function handleSnmpPollResult({ agentId, command, result }: Parameters + enqueueSnmpPollResults(snmpData.deviceId!, snmpData.metrics!, result.commandId) + ); } else { console.warn(`[AgentWs] Redis unavailable, dropping ${snmpData.metrics.length} SNMP metrics for device ${snmpData.deviceId}`); const { snmpDevices } = await import('../db/schema'); @@ -1046,32 +1057,33 @@ export async function processOrphanedCommandResult( } console.log(`[AgentWs] Processing monitor check result for monitor ${monitorData.monitorId} from agent ${agentId}`); try { - const status = normalizeMonitorStatus(monitorData.status); + const monitorId = monitorData.monitorId; + const checkResult = { + monitorId, + checkId: result.commandId, + status: normalizeMonitorStatus(monitorData.status), + responseMs: monitorData.responseMs ?? 0, + statusCode: monitorData.statusCode, + error: monitorData.error, + details: monitorData as Record + }; if (isRedisAvailable()) { - await enqueueMonitorCheckResult(monitorData.monitorId, { - monitorId: monitorData.monitorId, - checkId: result.commandId, - status, - responseMs: monitorData.responseMs ?? 0, - statusCode: monitorData.statusCode, - error: monitorData.error, - details: monitorData as Record - }, { - actorType: 'agent', - actorId: agentId, - source: 'route:agentWs:monitor-result', - }); + // Command results are processed inside a held org-scoped transaction + // (runWithAgentDbAccess). runOutsideDbContext exits the ALS context so + // instrumented-queue tripwires pass and any nested DB work routes to + // the pool — it does NOT release the outer transaction's connection, + // which stays held for the (normally short) Redis round-trips; the + // full fix is dispatching enqueues after the context closes (#1105). + await runOutsideDbContext(() => + enqueueMonitorCheckResult(monitorId, checkResult, { + actorType: 'agent', + actorId: agentId, + source: 'route:agentWs:monitor-result', + }) + ); } else { - console.warn(`[AgentWs] Redis unavailable, recording monitor result directly for ${monitorData.monitorId}`); - await recordMonitorCheckResult(monitorData.monitorId, { - monitorId: monitorData.monitorId, - checkId: result.commandId, - status, - responseMs: monitorData.responseMs ?? 0, - statusCode: monitorData.statusCode, - error: monitorData.error, - details: monitorData as Record - }); + console.warn(`[AgentWs] Redis unavailable, recording monitor result directly for ${monitorId}`); + await recordMonitorCheckResult(monitorId, checkResult); } } catch (err) { console.error(`[AgentWs] Failed to process monitor check result for ${agentId}:`, err); @@ -1284,11 +1296,14 @@ export async function processOrphanedCommandResult( } if (isRedisAvailable()) { - await enqueueDiscoveryResults( + const normalizedHosts = normalizeDiscoveryHosts(discoveryData.hosts); + // Exit the held org-scoped transaction context for the Redis + // round-trips (#1105) — see the note on the monitor-result branch. + await runOutsideDbContext(() => enqueueDiscoveryResults( discoveryJob.id, discoveryJob.orgId, discoveryJob.siteId, - normalizeDiscoveryHosts(discoveryData.hosts), + normalizedHosts, discoveryData.hostsScanned ?? 0, discoveryData.hostsDiscovered ?? 0, undefined, @@ -1298,7 +1313,7 @@ export async function processOrphanedCommandResult( actorId: agentId, source: 'route:agentWs:discovery-result', } - ); + )); } else { console.warn(`[AgentWs] Redis unavailable, cannot process ${discoveryData.hosts.length} discovery hosts for job ${discoveryJob.id}`); await db @@ -1422,7 +1437,9 @@ export async function processOrphanedCommandResult( : `Malformed backup result payload: ${parsedBackup.error.issues.map((issue) => issue.message).join(', ')}`; if (isRedisAvailable()) { - await enqueueBackupResults( + // Exit the held org-scoped transaction context for the Redis + // round-trips (#1105) — see the note on the monitor-result branch. + await runOutsideDbContext(() => enqueueBackupResults( backupJob.id, backupJob.orgId, backupJob.deviceId, @@ -1449,7 +1466,7 @@ export async function processOrphanedCommandResult( actorId: agentId, source: 'route:agentWs:backup-result', } - ); + )); } else { console.warn(`[AgentWs] Redis unavailable, marking backup job ${backupJob.id} with inline result`); const persisted = await applyBackupCommandResultToJob({ @@ -1741,7 +1758,10 @@ async function processCommandResult( try { const { enqueueDrExecutionReconcile } = await import('../jobs/drExecutionWorker'); - await enqueueDrExecutionReconcile(commandPayload.drExecutionId); + const drExecutionId = commandPayload.drExecutionId as string; + // Exit the held org-scoped transaction context for the Redis + // round-trips (#1105) — see the note on the monitor-result branch. + await runOutsideDbContext(() => enqueueDrExecutionReconcile(drExecutionId)); } catch (err) { console.error(`[AgentWs] Failed to enqueue DR reconciliation for ${result.commandId}:`, err); captureException(err); diff --git a/apps/api/src/routes/agents/inventory.test.ts b/apps/api/src/routes/agents/inventory.test.ts index fbb53d9abf..bd65ec2686 100644 --- a/apps/api/src/routes/agents/inventory.test.ts +++ b/apps/api/src/routes/agents/inventory.test.ts @@ -5,6 +5,7 @@ vi.mock('../../db', () => ({ db: { select: vi.fn(), insert: vi.fn(), + transaction: vi.fn(), }, runOutsideDbContext: vi.fn(async (fn: () => Promise) => fn()), withDbAccessContext: vi.fn(async (_ctx: unknown, fn: () => Promise) => fn()), @@ -184,3 +185,192 @@ describe('agent hardware inventory — warranty sync re-trigger (#1732)', () => expect(queueWarrantySyncForDevice).toHaveBeenCalledTimes(1); }); }); + +// The software report wipes and reinserts software_inventory rows. Vuln +// findings reference those rows (FK now ON DELETE SET NULL — BREEZE-3), and +// the fleet aggregation layer displays a NULL-linked finding as an OS finding, +// so the route must re-point each finding at the replacement row for the same +// (name, vendor). +describe('agent software inventory — vuln finding re-link (BREEZE-3)', () => { + type TxUpdateCall = { set?: Record; where?: unknown }; + + function mockSoftwareTx(opts: { + linkedFindings: Array<{ findingId: string; name: string; vendor: string | null }>; + replacementRows: Array<{ id: string; name: string; vendor: string | null }>; + }) { + const updateCalls: TxUpdateCall[] = []; + const deleteWhere = vi.fn().mockResolvedValue(undefined); + const insertValues = vi.fn().mockResolvedValue(undefined); + const tx = { + // Two select shapes: the linked-findings join + // (select().from().innerJoin().where()) and the post-insert replacement + // row lookup (select().from().where()). + select: vi.fn().mockReturnValue({ + from: vi.fn().mockReturnValue({ + innerJoin: vi.fn().mockReturnValue({ + where: vi.fn().mockResolvedValue(opts.linkedFindings), + }), + where: vi.fn().mockResolvedValue(opts.replacementRows), + }), + }), + delete: vi.fn().mockReturnValue({ where: deleteWhere }), + insert: vi.fn().mockReturnValue({ values: insertValues }), + update: vi.fn(() => { + const call: TxUpdateCall = {}; + updateCalls.push(call); + return { + set: vi.fn((set: Record) => { + call.set = set; + return { + where: vi.fn((where: unknown) => { + call.where = where; + return Promise.resolve(undefined); + }), + }; + }), + }; + }), + }; + vi.mocked(db.transaction).mockImplementation(async (fn: any) => fn(tx)); + return { tx, updateCalls, deleteWhere, insertValues }; + } + + async function putSoftware(app: Hono, software: Array>) { + return app.request('/agents/agent-1/software', { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ software }), + }); + } + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('re-links findings to the replacement rows matching (name, vendor)', async () => { + mockDeviceLookup({ id: 'device-1', orgId: 'org-1' }); + const { tx, updateCalls } = mockSoftwareTx({ + linkedFindings: [ + { findingId: 'finding-1', name: 'Google Chrome', vendor: 'Google LLC' }, + { findingId: 'finding-2', name: 'Google Chrome', vendor: 'Google LLC' }, + { findingId: 'finding-3', name: '7-Zip', vendor: null }, + ], + replacementRows: [ + { id: 'sw-new-1', name: 'Google Chrome', vendor: 'Google LLC' }, + { id: 'sw-new-2', name: '7-Zip', vendor: null }, + ], + }); + + const res = await putSoftware(makeApp(), [ + { name: 'Google Chrome', version: '127.0', vendor: 'Google LLC' }, + { name: '7-Zip', version: '24.06' }, + ]); + + expect(res.status).toBe(200); + expect(tx.delete).toHaveBeenCalledTimes(1); + expect(tx.insert).toHaveBeenCalledTimes(1); + // One UPDATE per replacement row; both Chrome findings batched together. + expect(updateCalls).toHaveLength(2); + const sets = updateCalls.map((c) => c.set?.softwareInventoryId).sort(); + expect(sets).toEqual(['sw-new-1', 'sw-new-2']); + }); + + it('re-links across casing/whitespace changes in name and vendor (correlation-normalized matching)', async () => { + mockDeviceLookup({ id: 'device-1', orgId: 'org-1' }); + const { updateCalls } = mockSoftwareTx({ + linkedFindings: [ + { findingId: 'finding-1', name: 'GOOGLE Chrome ', vendor: 'GOOGLE LLC' }, + ], + replacementRows: [ + { id: 'sw-new-1', name: 'Google Chrome', vendor: ' Google LLC' }, + ], + }); + + const res = await putSoftware(makeApp(), [ + { name: 'Google Chrome', vendor: ' Google LLC' }, + ]); + + expect(res.status).toBe(200); + expect(updateCalls).toHaveLength(1); + expect(updateCalls[0]!.set?.softwareInventoryId).toBe('sw-new-1'); + }); + + it('does not match on name alone when the vendor differs', async () => { + mockDeviceLookup({ id: 'device-1', orgId: 'org-1' }); + const { updateCalls } = mockSoftwareTx({ + linkedFindings: [ + { findingId: 'finding-1', name: 'Agent', vendor: 'Vendor A' }, + ], + replacementRows: [ + { id: 'sw-new-1', name: 'Agent', vendor: 'Vendor B' }, + ], + }); + + const res = await putSoftware(makeApp(), [{ name: 'Agent', vendor: 'Vendor B' }]); + + expect(res.status).toBe(200); + expect(updateCalls).toHaveLength(0); + }); + + it('leaves findings for uninstalled software unlinked (resolved by next correlation pass)', async () => { + mockDeviceLookup({ id: 'device-1', orgId: 'org-1' }); + const { updateCalls } = mockSoftwareTx({ + linkedFindings: [ + { findingId: 'finding-1', name: 'Old App', vendor: 'Gone Inc.' }, + ], + replacementRows: [ + { id: 'sw-new-1', name: 'Google Chrome', vendor: 'Google LLC' }, + ], + }); + + const res = await putSoftware(makeApp(), [ + { name: 'Google Chrome', vendor: 'Google LLC' }, + ]); + + expect(res.status).toBe(200); + expect(updateCalls).toHaveLength(0); + }); + + it('skips re-linking entirely when the device has no linked findings', async () => { + mockDeviceLookup({ id: 'device-1', orgId: 'org-1' }); + const { tx, updateCalls } = mockSoftwareTx({ + linkedFindings: [], + replacementRows: [{ id: 'sw-new-1', name: 'Google Chrome', vendor: 'Google LLC' }], + }); + + const res = await putSoftware(makeApp(), [ + { name: 'Google Chrome', vendor: 'Google LLC' }, + ]); + + expect(res.status).toBe(200); + expect(tx.insert).toHaveBeenCalledTimes(1); + expect(updateCalls).toHaveLength(0); + }); + + it('handles an empty software list: wipes rows, inserts nothing, re-links nothing', async () => { + mockDeviceLookup({ id: 'device-1', orgId: 'org-1' }); + const { tx, updateCalls } = mockSoftwareTx({ + linkedFindings: [ + { findingId: 'finding-1', name: 'Google Chrome', vendor: 'Google LLC' }, + ], + replacementRows: [], + }); + + const res = await putSoftware(makeApp(), []); + + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ success: true, count: 0 }); + expect(tx.delete).toHaveBeenCalledTimes(1); + expect(tx.insert).not.toHaveBeenCalled(); + expect(updateCalls).toHaveLength(0); + }); + + it('returns 404 without opening a transaction when the device is unknown', async () => { + mockDeviceLookup(null); + + const res = await putSoftware(makeApp(), [{ name: 'Google Chrome' }]); + + expect(res.status).toBe(404); + expect(db.transaction).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/api/src/routes/agents/inventory.ts b/apps/api/src/routes/agents/inventory.ts index 27d7ff0248..432401bba5 100644 --- a/apps/api/src/routes/agents/inventory.ts +++ b/apps/api/src/routes/agents/inventory.ts @@ -1,13 +1,14 @@ import { Hono } from 'hono'; import { bodyLimit } from 'hono/body-limit'; import { zValidator } from '../../lib/validation'; -import { eq } from 'drizzle-orm'; +import { and, eq, inArray, sql } from 'drizzle-orm'; import { db } from '../../db'; import { devices, deviceHardware, deviceDisks, deviceNetwork, + deviceVulnerabilities, softwareInventory, } from '../../db/schema'; import { @@ -105,26 +106,123 @@ inventoryRoutes.put('/:id/software', bodyLimit({ maxSize: 5 * 1024 * 1024, onErr } await db.transaction(async (tx) => { + // The wipe-and-reinsert below churns software_inventory row ids, and + // device_vulnerabilities.software_inventory_id references them (ON DELETE + // SET NULL). The fleet aggregation layer displays a NULL-linked finding as + // an OS finding (vulnerabilityFleetAggregation.ts groupKey), so without + // repair every software report would misclassify the device's software + // findings in the UI until the next correlation run. Capture what each + // finding pointed at, then re-link to the replacement row. + const linkedFindings = await tx + .select({ + findingId: deviceVulnerabilities.id, + name: softwareInventory.name, + vendor: softwareInventory.vendor, + }) + .from(deviceVulnerabilities) + .innerJoin(softwareInventory, eq(deviceVulnerabilities.softwareInventoryId, softwareInventory.id)) + .where(and( + eq(deviceVulnerabilities.deviceId, device.id), + eq(softwareInventory.deviceId, device.id) + )); + await tx .delete(softwareInventory) .where(eq(softwareInventory.deviceId, device.id)); + let replacementRows: { id: string; name: string; vendor: string | null }[] = []; if (data.software.length > 0) { const now = new Date(); - await tx.insert(softwareInventory).values( - data.software.map((item) => ({ - deviceId: device.id, - orgId: device.orgId, - name: item.name, - version: item.version || null, - vendor: item.vendor || null, - installDate: sanitizeDate(item.installDate), - installLocation: item.installLocation || null, - uninstallString: item.uninstallString || null, - fileHash: item.fileHash || null, - hashAlgorithm: item.hashAlgorithm || null, - lastSeen: now - })) + const rows = data.software.map((item) => ({ + deviceId: device.id, + orgId: device.orgId, + name: item.name, + version: item.version || null, + vendor: item.vendor || null, + installDate: sanitizeDate(item.installDate), + installLocation: item.installLocation || null, + uninstallString: item.uninstallString || null, + fileHash: item.fileHash || null, + hashAlgorithm: item.hashAlgorithm || null, + lastSeen: now + })); + await tx.insert(softwareInventory).values(rows); + // .returning() on the insert would serialize up to 10k rows back over + // the wire when only the handful of names carried by linked findings + // matter — select just the candidate replacement rows instead, + // normalized the same way the re-link match below is. + if (linkedFindings.length > 0) { + const findingNames = [...new Set(linkedFindings.map((f) => f.name.trim().toLowerCase()))]; + replacementRows = await tx + .select({ + id: softwareInventory.id, + name: softwareInventory.name, + vendor: softwareInventory.vendor, + }) + .from(softwareInventory) + .where(and( + eq(softwareInventory.deviceId, device.id), + inArray(sql`lower(trim(${softwareInventory.name}))`, findingNames) + )); + } + } + + if (linkedFindings.length > 0 && data.software.length > 0) { + // Match by (name, vendor), normalized the same way the correlation + // layer matches products (lower(trim(...)) — see the + // softwareProductResolutions join in vulnerabilityCorrelation.ts), so a + // casing/whitespace change between reports doesn't drop the link. + // Version is deliberately ignored: upgrades keep the link and the + // correlation job re-evaluates version ranges on its next run. First + // row wins for duplicate keys. Findings whose software is gone keep a + // NULL link; correlateOrg's resolve pass closes them. + const relinkKey = (name: string, vendor: string | null) => + JSON.stringify([name.trim().toLowerCase(), (vendor ?? '').trim().toLowerCase()]); + + const newRowByKey = new Map(); + for (const row of replacementRows) { + const key = relinkKey(row.name, row.vendor); + if (!newRowByKey.has(key)) newRowByKey.set(key, row.id); + } + + const findingIdsByNewRow = new Map(); + let severed = 0; + for (const finding of linkedFindings) { + const newRowId = newRowByKey.get(relinkKey(finding.name, finding.vendor)); + if (!newRowId) { + severed++; + continue; + } + const ids = findingIdsByNewRow.get(newRowId) ?? []; + ids.push(finding.findingId); + findingIdsByNewRow.set(newRowId, ids); + } + + for (const [newRowId, findingIds] of findingIdsByNewRow) { + await tx + .update(deviceVulnerabilities) + .set({ softwareInventoryId: newRowId, updatedAt: new Date() }) + .where(inArray(deviceVulnerabilities.id, findingIds)); + } + + if (severed > 0) { + // Expected for genuinely uninstalled software (the next correlation + // pass resolves those findings), but a spike of these fleet-wide is + // the signature of truncated agent-side collection silently converting + // open findings to patched — keep the trail. + console.warn( + `[Inventory] Software report for device ${device.id} severed ${severed} vuln finding link(s) with no replacement row (uninstalled or renamed software)` + ); + } + } + + if (linkedFindings.length > 0 && data.software.length === 0) { + // An empty report against a device with linked findings just severed + // every link in one statement. Legitimate only if all software was + // actually removed — it is also the signature of a truncated agent-side + // collection, so leave a trail. + console.warn( + `[Inventory] Software report for device ${device.id} emptied the inventory and detached ${linkedFindings.length} vuln finding link(s)` ); } }); diff --git a/apps/api/src/routes/agents/inventorySoftwareRelink.integration.test.ts b/apps/api/src/routes/agents/inventorySoftwareRelink.integration.test.ts new file mode 100644 index 0000000000..8cd31a2f92 --- /dev/null +++ b/apps/api/src/routes/agents/inventorySoftwareRelink.integration.test.ts @@ -0,0 +1,140 @@ +/** + * Integration test — software report wipe-and-reinsert with linked vuln + * findings, against real Postgres + RLS (Sentry BREEZE-3). + * + * The mocked unit suite (inventory.test.ts) encodes the route's assumptions; + * this suite checks them for real: the `device_vulnerabilities_software_ + * inventory_id_fkey` constraint must actually be ON DELETE SET NULL after the + * 2026-07-17-a/-b migrations (a constraint-name mismatch would silently leave + * the FK as NO ACTION and re-freeze inventories — the original shipped bug), + * and the re-link UPDATE on device_vulnerabilities must work under the same + * org-scoped RLS context agentAuthMiddleware gives the route (a silently + * 0-row RLS write would detach findings on every report while mocked tests + * stay green). + */ +import '../../__tests__/integration/setup'; +import { describe, it, expect } from 'vitest'; +import { Hono } from 'hono'; +import { eq } from 'drizzle-orm'; +import { db, withDbAccessContext, withSystemDbAccessContext, type DbAccessContext } from '../../db'; +import { devices, deviceVulnerabilities, softwareInventory, vulnerabilities } from '../../db/schema'; +import { setupTestEnvironment } from '../../__tests__/integration/db-utils'; +import { inventoryRoutes } from './inventory'; + +const runDb = it.runIf(!!process.env.DATABASE_URL); + +/** The exact RLS context `agentAuthMiddleware` sets up for org-scoped agent routes. */ +function agentRequestContext(orgId: string): DbAccessContext { + return { + scope: 'organization', + orgId, + accessibleOrgIds: [orgId], + accessiblePartnerIds: [], + currentPartnerId: null, + }; +} + +async function insertDevice(orgId: string, siteId: string): Promise<{ id: string; agentId: string }> { + const agentId = `agent-swrelink-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + return withSystemDbAccessContext(async () => { + const [row] = await db + .insert(devices) + .values({ + orgId, + siteId, + agentId, + hostname: `swrelink-${agentId}`, + osType: 'windows', + osVersion: '11', + architecture: 'x86_64', + agentVersion: '0.0.0-test', + status: 'online', + enrolledAt: new Date(), + }) + .returning({ id: devices.id }); + if (!row) throw new Error('insertDevice: no row'); + return { id: row.id, agentId }; + }); +} + +async function seedInventoryRow(orgId: string, deviceId: string, name: string, vendor: string): Promise { + return withSystemDbAccessContext(async () => { + const [row] = await db + .insert(softwareInventory) + .values({ orgId, deviceId, name, vendor, version: '1.0', lastSeen: new Date() }) + .returning({ id: softwareInventory.id }); + if (!row) throw new Error('seedInventoryRow: no row'); + return row.id; + }); +} + +async function seedLinkedFinding(orgId: string, deviceId: string, cveId: string, softwareInventoryId: string): Promise { + return withSystemDbAccessContext(async () => { + const [vuln] = await db + .insert(vulnerabilities) + .values({ cveId, source: 'nvd', description: 'x', rawPayload: { t: true } }) + .returning({ id: vulnerabilities.id }); + const [finding] = await db + .insert(deviceVulnerabilities) + .values({ orgId, deviceId, vulnerabilityId: vuln!.id, softwareInventoryId, status: 'open', detectedAt: new Date() }) + .returning({ id: deviceVulnerabilities.id }); + return finding!.id; + }); +} + +/** Submits a software report through the real Hono handler, under the same + * withDbAccessContext wrap agentAuthMiddleware applies in production. */ +async function submitSoftware(orgId: string, agentId: string, software: unknown[]) { + const app = new Hono(); + app.use('*', async (c: any, next: any) => { + c.set('agent', { orgId, agentId, role: 'agent' }); + await next(); + }); + app.route('/agents', inventoryRoutes); + return withDbAccessContext(agentRequestContext(orgId), async () => + app.request(`/agents/${agentId}/software`, { + method: 'PUT', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ software }), + }) + ); +} + +describe('software report with linked vuln findings (real FK + RLS, BREEZE-3)', () => { + runDb('wipe succeeds, surviving software is re-linked, uninstalled software degrades to NULL', async () => { + const env = await setupTestEnvironment({ scope: 'organization' }); + const orgId = env.organization.id; + const dev = await insertDevice(orgId, env.site.id); + + const chromeRowId = await seedInventoryRow(orgId, dev.id, 'Google Chrome', 'Google LLC'); + const goneRowId = await seedInventoryRow(orgId, dev.id, 'Old App', 'Gone Inc.'); + const chromeFindingId = await seedLinkedFinding(orgId, dev.id, 'CVE-2025-RELINK-1', chromeRowId); + const goneFindingId = await seedLinkedFinding(orgId, dev.id, 'CVE-2025-RELINK-2', goneRowId); + + // The report keeps Chrome (different casing — must still re-link) and + // drops Old App. Pre-fix this DELETE failed with FK 23503. + const res = await submitSoftware(orgId, dev.agentId, [ + { name: 'google chrome', vendor: 'Google LLC', version: '127.0' }, + ]); + expect(res.status).toBe(200); + expect(await res.json()).toEqual({ success: true, count: 1 }); + + const inventoryRows = await withSystemDbAccessContext(() => + db.select().from(softwareInventory).where(eq(softwareInventory.deviceId, dev.id))); + expect(inventoryRows).toHaveLength(1); + const newChromeRowId = inventoryRows[0]!.id; + expect(newChromeRowId).not.toBe(chromeRowId); + + const findings = await withSystemDbAccessContext(() => + db.select().from(deviceVulnerabilities).where(eq(deviceVulnerabilities.deviceId, dev.id))); + const byId = new Map(findings.map((f) => [f.id, f])); + // Surviving software: re-linked to the replacement row (real RLS UPDATE, + // not a silent 0-row write). + expect(byId.get(chromeFindingId)?.softwareInventoryId).toBe(newChromeRowId); + expect(byId.get(chromeFindingId)?.status).toBe('open'); + // Uninstalled software: the real SET NULL trigger fired (constraint name + // and delete action are correct on a fully-migrated database). + expect(byId.get(goneFindingId)?.softwareInventoryId).toBeNull(); + expect(byId.get(goneFindingId)?.status).toBe('open'); + }); +}); diff --git a/apps/api/src/services/vulnerabilityCorrelation.integration.test.ts b/apps/api/src/services/vulnerabilityCorrelation.integration.test.ts index f27c8c853b..d3f1a1e119 100644 --- a/apps/api/src/services/vulnerabilityCorrelation.integration.test.ts +++ b/apps/api/src/services/vulnerabilityCorrelation.integration.test.ts @@ -8,6 +8,7 @@ import { devices, deviceVulnerabilities, organizations, + osVulnerabilities, partners, sites, softwareInventory, @@ -16,7 +17,7 @@ import { softwareVulnerabilities, vulnerabilities, } from '../db/schema'; -import { correlateOrg } from './vulnerabilityCorrelation'; +import { correlateOrg, correlateOsVulns } from './vulnerabilityCorrelation'; import { refreshResolutionCache } from './cpeResolution'; const runDb = it.runIf(!!process.env.DATABASE_URL); @@ -36,13 +37,16 @@ beforeEach(async () => { await db.delete(deviceVulnerabilities); await db.delete(softwareProductResolutions); await db.delete(softwareVulnerabilities); + await db.delete(osVulnerabilities); await db.delete(softwareInventory); await db.delete(softwareProducts); await db.delete(vulnerabilities); }); }); -async function seedOrgWithDevice(): Promise<{ orgId: string; deviceId: string }> { +async function seedOrgWithDevice( + os: { osType: 'windows' | 'macos' | 'linux'; osVersion: string } = { osType: 'windows', osVersion: '11' } +): Promise<{ orgId: string; deviceId: string }> { return withSystemDbAccessContext(async () => { const unique = `${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; const [partner] = await db @@ -82,8 +86,8 @@ async function seedOrgWithDevice(): Promise<{ orgId: string; deviceId: string }> siteId: site.id, agentId: `vuln-correlation-agent-${unique}`, hostname: `vuln-correlation-host-${unique}`, - osType: 'windows', - osVersion: '11', + osType: os.osType, + osVersion: os.osVersion, architecture: 'x86_64', agentVersion: '0.0.0-test', status: 'offline', @@ -425,3 +429,136 @@ describe('correlation via resolver', () => { expect(rows[0]?.matchConfidence).toBe('curated'); }); }); + +// Since the software_inventory FK became ON DELETE SET NULL (BREEZE-3), a +// SOFTWARE finding can be NULL-linked: its inventory row was wiped by an agent +// software report and the route-level re-link missed (uninstalled software, +// renamed product, or a race). The resolve passes discriminate OS vs software +// findings by os_vulnerabilities facts, not link nullness — these tests pin +// that contract. +describe('NULL-linked finding resolution (SET NULL FK semantics)', () => { + async function seedSoftwareVulnFact(cveId: string, fixedBuild: string): Promise { + return withSystemDbAccessContext(async () => { + const [prod] = await db + .insert(softwareProducts) + .values({ normalizedName: 'acrobat_reader', normalizedVendor: 'adobe', cpe: 'cpe:2.3:a:adobe:acrobat_reader:*:*:*:*:*:*:*:*', cpeConfidence: 'authoritative' }) + .returning({ id: softwareProducts.id }); + const [vuln] = await db + .insert(vulnerabilities) + .values({ cveId, source: 'nvd', description: 'x', severity: 'high', cvssVersion: '3.1', cvssScore: '7.5', patchAvailable: true, rawPayload: { t: true } }) + .returning({ id: vulnerabilities.id }); + await db.insert(softwareVulnerabilities).values({ productId: prod!.id, vulnerabilityId: vuln!.id, versionEndExcluding: fixedBuild }); + return vuln!.id; + }); + } + + async function seedOsVulnFact(cveId: string, osLine: string, fixedVersion: string): Promise { + return withSystemDbAccessContext(async () => { + const [vuln] = await db + .insert(vulnerabilities) + .values({ cveId, source: 'apple', description: 'x', severity: 'high', cvssVersion: '3.1', cvssScore: '7.5', patchAvailable: true, rawPayload: { t: true } }) + .returning({ id: vulnerabilities.id }); + await db.insert(osVulnerabilities).values({ platform: 'macos', osLine, fixedVersion, vulnerabilityId: vuln!.id }); + return vuln!.id; + }); + } + + async function insertNullLinkedFinding(orgId: string, deviceId: string, vulnerabilityId: string): Promise { + return withSystemDbAccessContext(async () => { + const [row] = await db + .insert(deviceVulnerabilities) + .values({ orgId, deviceId, vulnerabilityId, softwareInventoryId: null, status: 'open', detectedAt: new Date() }) + .returning({ id: deviceVulnerabilities.id }); + return row!.id; + }); + } + + runDb('resolves a NULL-linked software finding whose software is gone (uninstalled)', async () => { + const { orgId, deviceId } = await seedOrgWithDevice(); + const vulnId = await seedSoftwareVulnFact('CVE-2025-GONE', '99.0'); + // No inventory row: the software was uninstalled and the report wiped it. + await insertNullLinkedFinding(orgId, deviceId, vulnId); + + await refreshResolutionCache(); + const res = await correlateOrg(orgId, { deviceIds: [deviceId] }); + + expect(res.resolved).toBe(1); + const rows = await withSystemDbAccessContext(() => + db.select().from(deviceVulnerabilities).where(eq(deviceVulnerabilities.deviceId, deviceId))); + expect(rows).toHaveLength(1); + expect(rows[0]?.status).toBe('patched'); + expect(rows[0]?.resolvedAt).not.toBeNull(); + }); + + runDb('re-links (not resolves) a NULL-linked finding whose software is still installed and vulnerable', async () => { + const { orgId, deviceId } = await seedOrgWithDevice(); + const vulnId = await seedSoftwareVulnFact('CVE-2025-RELINK', '99.0'); + await seedInventoryVendored(orgId, deviceId, 'Adobe Acrobat Reader DC', 'Adobe Inc.', '23.0'); + await insertNullLinkedFinding(orgId, deviceId, vulnId); + + await refreshResolutionCache(); + const res = await correlateOrg(orgId, { deviceIds: [deviceId] }); + + expect(res.resolved).toBe(0); + const rows = await withSystemDbAccessContext(() => + db.select().from(deviceVulnerabilities).where(eq(deviceVulnerabilities.deviceId, deviceId))); + expect(rows).toHaveLength(1); + expect(rows[0]?.status).toBe('open'); + expect(rows[0]?.softwareInventoryId).not.toBeNull(); + }); + + runDb('sweeps a severed finding while other software on the device re-matches (matchedIds non-empty)', async () => { + const { orgId, deviceId } = await seedOrgWithDevice(); + // Still-installed, still-vulnerable product: re-matched each pass, so the + // resolve sweep runs with matchedIds non-empty (the notInArray branch). + const matchedVulnId = await seedSoftwareVulnFact('CVE-2025-STILLHERE', '99.0'); + await seedInventoryVendored(orgId, deviceId, 'Adobe Acrobat Reader DC', 'Adobe Inc.', '23.0'); + const matchedFindingId = await insertNullLinkedFinding(orgId, deviceId, matchedVulnId); + // Uninstalled product: severed NULL-linked finding, must be swept even + // when the same device has re-matched findings this pass. + const goneVulnId = await withSystemDbAccessContext(async () => { + const [prod] = await db + .insert(softwareProducts) + .values({ normalizedName: '7-zip', normalizedVendor: 'igor pavlov', cpe: 'cpe:2.3:a:7-zip:7-zip:*:*:*:*:*:*:*:*', cpeConfidence: 'authoritative' }) + .returning({ id: softwareProducts.id }); + const [vuln] = await db + .insert(vulnerabilities) + .values({ cveId: 'CVE-2025-GONE2', source: 'nvd', description: 'x', severity: 'high', cvssVersion: '3.1', cvssScore: '7.5', patchAvailable: true, rawPayload: { t: true } }) + .returning({ id: vulnerabilities.id }); + await db.insert(softwareVulnerabilities).values({ productId: prod!.id, vulnerabilityId: vuln!.id, versionEndExcluding: '99.0' }); + return vuln!.id; + }); + const goneFindingId = await insertNullLinkedFinding(orgId, deviceId, goneVulnId); + + await refreshResolutionCache(); + const res = await correlateOrg(orgId, { deviceIds: [deviceId] }); + + expect(res.resolved).toBe(1); + const rows = await withSystemDbAccessContext(() => + db.select().from(deviceVulnerabilities).where(eq(deviceVulnerabilities.deviceId, deviceId))); + const byId = new Map(rows.map((r) => [r.id, r])); + expect(byId.get(matchedFindingId)?.status).toBe('open'); + expect(byId.get(matchedFindingId)?.softwareInventoryId).not.toBeNull(); + expect(byId.get(goneFindingId)?.status).toBe('patched'); + }); + + runDb('correlateOsVulns sweeps stale OS findings but never NULL-linked software findings', async () => { + const { orgId, deviceId } = await seedOrgWithDevice({ osType: 'macos', osVersion: '14.5' }); + // Stale OS finding: the device is already at/above the fixed version. + const osVulnId = await seedOsVulnFact('CVE-2025-OSSTALE', 'Sonoma 14', '14.4'); + const osFindingId = await insertNullLinkedFinding(orgId, deviceId, osVulnId); + // Software finding that lost its link to a wiped inventory row — must NOT + // be swept by the OS pass (it has no os_vulnerabilities facts). + const swVulnId = await seedSoftwareVulnFact('CVE-2025-SWNULL', '99.0'); + const swFindingId = await insertNullLinkedFinding(orgId, deviceId, swVulnId); + + const res = await correlateOsVulns(orgId, { deviceIds: [deviceId] }); + + expect(res.resolved).toBe(1); + const rows = await withSystemDbAccessContext(() => + db.select().from(deviceVulnerabilities).where(eq(deviceVulnerabilities.deviceId, deviceId))); + const byId = new Map(rows.map((r) => [r.id, r])); + expect(byId.get(osFindingId)?.status).toBe('patched'); + expect(byId.get(swFindingId)?.status).toBe('open'); + }); +}); diff --git a/apps/api/src/services/vulnerabilityCorrelation.ts b/apps/api/src/services/vulnerabilityCorrelation.ts index 7ffdadd69d..0c5614e9a9 100644 --- a/apps/api/src/services/vulnerabilityCorrelation.ts +++ b/apps/api/src/services/vulnerabilityCorrelation.ts @@ -1,4 +1,4 @@ -import { and, eq, inArray, isNotNull, isNull, notInArray, sql } from 'drizzle-orm'; +import { and, eq, exists, inArray, isNotNull, isNull, notExists, notInArray, or, sql } from 'drizzle-orm'; import { db, withSystemDbAccessContext } from '../db'; import { @@ -37,6 +37,35 @@ function isCriticalCvss(cvssScore: string | null): boolean { return cvssScore != null && Number(cvssScore) >= CRITICAL_CVSS_THRESHOLD; } +/** + * OS findings are created by {@link correlateOsVulns} for vulnerabilities that + * have `os_vulnerabilities` facts and always carry a NULL software link. A NULL + * link alone no longer identifies them: since the software_inventory FK became + * ON DELETE SET NULL, a SOFTWARE finding can legitimately be NULL-linked (its + * inventory row was wiped by an agent report and the route-level re-link missed + * — uninstalled software, renamed product, or a race). Both resolve passes must + * therefore discriminate on the vulnerability's os-facts, not link nullness. + * + * Caveat: a CVE carrying BOTH os facts and software facts (fact sources are + * disjoint today — apple platform facts vs msrc/nvd software facts — so this + * is currently unreachable) would be excluded from the software sweep and its + * severed finding would stay open on non-mac devices. That failure mode is + * deliberately conservative: an open stale finding beats a false "patched". + */ +const osVulnFactsSubquery = (platform?: string) => + db + .select({ one: sql`1` }) + .from(osVulnerabilities) + .where(platform == null + ? eq(osVulnerabilities.vulnerabilityId, deviceVulnerabilities.vulnerabilityId) + : and( + eq(osVulnerabilities.vulnerabilityId, deviceVulnerabilities.vulnerabilityId), + eq(osVulnerabilities.platform, platform) + )); + +const hasOsVulnFacts = (platform?: string) => exists(osVulnFactsSubquery(platform)); +const hasNoOsVulnFacts = () => notExists(osVulnFactsSubquery()); + /** Batched device → site map for event attribution, under one system-context read. */ async function lookupSiteIds(deviceIds: string[]): Promise> { const distinct = [...new Set(deviceIds)]; @@ -126,6 +155,17 @@ async function upsertDeviceVulnerability(args: { }): Promise<{ id: string; created: boolean }> { const riskScore = args.cvssScore == null ? null : String(args.cvssScore); const now = new Date(); + // The candidate inventory id was read earlier in this pass, and agent + // software reports wipe-and-reinsert inventory rows concurrently (the FK is + // ON DELETE SET NULL, so the wipe no longer blocks on existing links). A + // plain write of a since-deleted id would raise 23503 and abort the whole + // org's correlation transaction. The scalar subquery re-resolves the id at + // write time — deleted rows degrade to a NULL link, which the next pass + // repairs. This closes the practical window (a delete committing inside the + // write statement itself can still 23503; the next run covers that sliver). + const liveSoftwareInventoryId = args.softwareInventoryId == null + ? null + : sql`(select ${softwareInventory.id} from ${softwareInventory} where ${softwareInventory.id} = ${args.softwareInventoryId})`; const [existing] = await db .select({ id: deviceVulnerabilities.id, @@ -144,7 +184,7 @@ async function upsertDeviceVulnerability(args: { .update(deviceVulnerabilities) .set({ ...(existing.status === 'patched' ? { status: 'open', resolvedAt: null } : {}), - softwareInventoryId: args.softwareInventoryId, + softwareInventoryId: liveSoftwareInventoryId, riskScore, matchConfidence: args.matchConfidence, updatedAt: now, @@ -159,7 +199,7 @@ async function upsertDeviceVulnerability(args: { orgId: args.orgId, deviceId: args.deviceId, vulnerabilityId: args.vulnerabilityId, - softwareInventoryId: args.softwareInventoryId, + softwareInventoryId: liveSoftwareInventoryId, status: 'open', riskScore, matchConfidence: args.matchConfidence, @@ -324,7 +364,15 @@ export async function correlateOrg( const baseResolveWhere = and( eq(deviceVulnerabilities.orgId, orgId), eq(deviceVulnerabilities.status, 'open'), - isNotNull(deviceVulnerabilities.softwareInventoryId), + // Software findings: still-linked rows, plus rows whose link was severed + // by an inventory wipe (ON DELETE SET NULL) — identified by the absence + // of os_vulnerabilities facts so OS findings are never swept here. A + // severed software finding that wasn't re-matched above means the + // software is gone (or no longer vulnerable): resolve it. + or( + isNotNull(deviceVulnerabilities.softwareInventoryId), + hasNoOsVulnFacts() + ), findingDeviceFilter ); const resolveWhere = matchedIds.size > 0 @@ -438,6 +486,11 @@ export async function correlateOsVulns( eq(deviceVulnerabilities.orgId, orgId), eq(deviceVulnerabilities.status, 'open'), isNull(deviceVulnerabilities.softwareInventoryId), + // Only sweep true OS findings — and only those for the platform this + // pass evaluates. A NULL link is no longer sufficient: software + // findings can be NULL-linked after an inventory wipe (ON DELETE SET + // NULL) and must not be falsely patched by the OS pass. + hasOsVulnFacts('macos'), inArray(deviceVulnerabilities.deviceId, macDeviceIds) ) : undefined; diff --git a/apps/api/src/services/vulnerabilityFleetAggregation.ts b/apps/api/src/services/vulnerabilityFleetAggregation.ts index c17e4fa6a8..07d9c8c549 100644 --- a/apps/api/src/services/vulnerabilityFleetAggregation.ts +++ b/apps/api/src/services/vulnerabilityFleetAggregation.ts @@ -5,6 +5,10 @@ * across requests so `#software/` deep links work): * software finding: sw:| * OS finding: os: (softwareInventoryId IS NULL) + * NULL-link is only an approximation of "OS finding": since the + * software_inventory FK became ON DELETE SET NULL (BREEZE-3), a software + * finding can transiently be NULL-linked between an inventory wipe and the + * next correlation pass, and lands in the os:* group for that window. * The lower(trim(...)) normalization of the NAME deliberately matches the * correlation pipeline's JOIN (services/vulnerabilityCorrelation.ts), which * normalizes only the software name — so one product maps to one queue row diff --git a/apps/api/vitest.config.ts b/apps/api/vitest.config.ts index de8a454d4e..0496fa7c55 100644 --- a/apps/api/vitest.config.ts +++ b/apps/api/vitest.config.ts @@ -48,6 +48,11 @@ export default defineConfig({ // beforeAll), so the unit runner's no-DB environment fails the suite on // connect. Belongs to vitest.integration.config.ts. 'src/routes/agents/changes.integration.test.ts', + // Software-report re-link real-DB test (BREEZE-3): imports + // `__tests__/integration/setup` (real postgres pool + autoMigrate), so the + // no-DB unit runner would fail it on connect. Belongs to + // vitest.integration.config.ts. + 'src/routes/agents/inventorySoftwareRelink.integration.test.ts', // Auth-email worker real-DB test (SR2-22): imports `__tests__/integration/setup` // (real postgres pool + autoMigrate + real Redis) and lives in src/jobs/ — outside // the `src/__tests__/integration/**` glob above — so the no-DB unit runner would diff --git a/apps/api/vitest.integration.config.ts b/apps/api/vitest.integration.config.ts index cdc822cf80..509731b626 100644 --- a/apps/api/vitest.integration.config.ts +++ b/apps/api/vitest.integration.config.ts @@ -66,6 +66,11 @@ export default defineConfig({ // the mocked `changes.test.ts` unit suite, so this drives the real // `changesRoutes` handler + RLS insert/select policies against Postgres. 'src/routes/agents/changes.integration.test.ts', + // Co-located real-DB integration test for BREEZE-3: software report + // wipe-and-reinsert with linked vuln findings — proves the SET NULL FK + // (constraint name + delete action) and the re-link UPDATE under the + // org-scoped agent RLS context, which the mocked inventory.test.ts can't. + 'src/routes/agents/inventorySoftwareRelink.integration.test.ts', // Co-located real-DB integration test for the SR2-22 auth-email worker: // proves the OUT-OF-REQUEST worker's withSystemDbAccessContext wrap lets // it FIND a FORCE-RLS `users` row (a contextless read would be 0 rows =