fix(api): software report FK failure + BullMQ enqueue inside held DB context#2613
Merged
Conversation
…context Fixes BREEZE-3: device_vulnerabilities.software_inventory_id had no ON DELETE action, so the agent software report's wipe-and-reinsert failed with 23503 for any device with correlated software findings, permanently freezing that device's inventory. The FK is now ON DELETE SET NULL (added NOT VALID + separate VALIDATE migration to avoid a blocking validation scan, plus a partial index so the per-row SET NULL trigger doesn't seq-scan the multi-tenant table), and the report route re-links each finding to the replacement row for the same (name, vendor), normalized lower/trim to match correlation semantics. Because a software finding can now legitimately carry a NULL link, the correlation resolve passes discriminate OS vs software findings by os_vulnerabilities facts instead of link nullness: correlateOrg resolves severed software findings (previously stranded open forever) and correlateOsVulns no longer falsely patches NULL-linked software findings on macOS. upsertDeviceVulnerability re-resolves the inventory id at write time via a scalar subquery so a concurrent inventory wipe degrades to a NULL link instead of aborting the org's correlation run on 23503. Fixes BREEZE-H: BullMQ enqueues on the agent WS command-result paths ran inside the held org-scoped transaction (#1105 tripwire). All seven call sites (monitor, SNMP orphaned + tracked, discovery x2, backup, DR reconcile) now wrap in runOutsideDbContext; this also relieves the pooled-connection pressure behind the CONNECT_TIMEOUT burst (BREEZE-J). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Deploying breeze with
|
| Latest commit: |
abaa9f7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://632661dd.breeze-9te.pages.dev |
| Branch Preview URL: | https://toddhebebrand-sentry-issues.breeze-9te.pages.dev |
There was a problem hiding this comment.
Pull request overview
Fixes two production-facing failure modes in the API: (1) agent software inventory wipes failing due to device_vulnerabilities → software_inventory FK enforcement, and (2) BullMQ enqueue calls occurring while an org-scoped DB transaction context is held during agent WS command-result handling (pool poisoning / tripwire #1105). The PR updates DB constraints + indexing, hardens correlation semantics for NULL-linked findings, and adds both unit + real-DB integration tests to pin the behavior.
Changes:
- Change
device_vulnerabilities.software_inventory_idFK toON DELETE SET NULL(with NOT VALID + validate migration) and add a supporting partial index. - Re-link device vulnerability findings to replacement
software_inventoryrows after wipe-and-reinsert; update correlation resolve passes to distinguish OS vs software findings viaos_vulnerabilitiesfacts (not link nullness). - Wrap all BullMQ enqueue call sites in
agentWs.tswithrunOutsideDbContextand add a static contract test to prevent regressions.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/api/vitest.integration.config.ts | Adds the new real-DB integration test to the integration runner include list. |
| apps/api/vitest.config.ts | Excludes the new real-DB integration test from the unit runner. |
| apps/api/src/services/vulnerabilityFleetAggregation.ts | Updates documentation comment to reflect transient NULL-link semantics post-SET-NULL FK. |
| apps/api/src/services/vulnerabilityCorrelation.ts | Adds os-fact discrimination helpers, adjusts resolve predicates, and re-resolves softwareInventoryId at write time. |
| apps/api/src/services/vulnerabilityCorrelation.integration.test.ts | Adds integration coverage for NULL-linked finding resolution and OS-pass behavior. |
| apps/api/src/routes/agentWs.ts | Wraps BullMQ enqueues in runOutsideDbContext within command_result processing paths; small refactors + extra Sentry capture. |
| apps/api/src/routes/agentWs.test.ts | Adds a unit test asserting enqueue happens inside runOutsideDbContext for monitor results. |
| apps/api/src/routes/agentWs.enqueueContract.test.ts | Adds a static scan contract ensuring all enqueue* call sites are wrapped. |
| apps/api/src/routes/agents/inventorySoftwareRelink.integration.test.ts | Adds real Postgres + RLS integration test for wipe-and-reinsert + re-link + SET NULL FK behavior. |
| apps/api/src/routes/agents/inventory.ts | Implements re-linking of findings to replacement inventory rows during software report ingest. |
| apps/api/src/routes/agents/inventory.test.ts | Adds unit tests for the re-link logic and transaction usage. |
| apps/api/src/db/schema/vulnerabilityManagement.ts | Updates Drizzle FK definition to onDelete: 'set null' and adds the partial index in schema. |
| apps/api/migrations/2026-07-17-a-device-vulnerabilities-software-fk-set-null.sql | Drops/re-adds FK as SET NULL (NOT VALID) and creates the partial index. |
| apps/api/migrations/2026-07-17-b-device-vulnerabilities-software-fk-validate.sql | Validates the NOT VALID FK in a follow-up migration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+149
to
+156
| // .returning() serializes up to 10k rows back over the wire — only pay | ||
| // for it when there are findings to re-link. | ||
| if (linkedFindings.length > 0) { | ||
| inserted = await tx.insert(softwareInventory).values(rows).returning({ | ||
| id: softwareInventory.id, | ||
| name: softwareInventory.name, | ||
| vendor: softwareInventory.vendor, | ||
| }); |
Collaborator
Author
There was a problem hiding this comment.
Done in abaa9f7 — insert now runs without .returning(), followed by a select of only the candidate replacement rows (lower(trim(name)) matching the distinct normalized names carried by linked findings; vendor matching stays in the existing map lookup). The emptied-inventory/severed-link warn branches were re-gated on data.software.length since the selected set no longer represents everything inserted. Unit + real-DB integration suites pass.
… full inventory Copilot review: .returning() on the software wipe-and-reinsert shipped the entire inventory (up to ~10k rows) back over the wire whenever the device had any linked vuln finding. Insert without returning, then select just the candidate replacement rows whose normalized name matches a linked finding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
software_inventoryrows still referenced bydevice_vulnerabilities.software_inventory_id. Migration changes the FK toON DELETE SET NULL(addedNOT VALIDthen validated in a separate transaction to avoid holding an exclusive lock), plus a supporting index for the referential trigger.software_inventoryrow after the wipe-and-reinsert, matched on normalized(name, vendor); severed/uninstalled software degrades to aNULLlink and is swept by the correlation resolve passes.vulnerabilityCorrelation.tsresolve passes (correlateOrg,correlateOsVulns) to discriminate OS vs software findings byos_vulnerabilitiesfacts rather than link-nullness alone, since a NULL link no longer uniquely identifies an OS finding.upsertDeviceVulnerabilitynow re-resolvessoftwareInventoryIdvia a scalar subquery at write time to avoid a race where a concurrent inventory wipe FK-violates the correlation write.enqueue*BullMQ calls inagentWs.ts(monitor, SNMP, discovery, backup, DR reconcile) inrunOutsideDbContext, sincecommand_resulthandling runs inside a held org-scoped DB transaction. Added a static contract test to catch future unwrapped call sites.Testing
inventory.test.ts) and the enqueue-context contract (agentWs.enqueueContract.test.ts,agentWs.test.ts).inventorySoftwareRelink.integration.test.ts(FK behavior + re-link under org-scoped RLS) and expandedvulnerabilityCorrelation.integration.test.tscases for NULL-linked finding resolution.vitest.config.ts(excluded from unit runner) andvitest.integration.config.ts(included).