fix: exclude xml/binary blobs from with_severity_counts (prod crash) (#713)

* fix: exclude xml/binary blobs from with_severity_counts

The with_severity_counts scope used select("table.*") which loaded
ALL columns including multi-MB xml blobs on Stig and SRG models.
On index pages this blew Heroku dyno memory (R14/R15) and caused
30s timeouts (H12), crashing the app.

Fix: auto-detect columns of type xml/binary and exclude them from
the SELECT in with_severity_counts. This is a DRY fix at the
concern level — all models (Stig, SRG, Component) benefit without
per-controller workarounds. Models without heavy columns are
unaffected (all columns loaded as before).

The xml column is still loaded via Stig.find(id) for export/download
endpoints that need it.

Authored by: Aaron Lippold<lippold@gmail.com>

* fix: address Copilot review — use abstract column type, update docs, dynamic test assertions

- Use c.type (ActiveRecord abstract) instead of c.sql_type (adapter-
  specific) for heavy column detection. Catches Postgres bytea, MySQL
  blob, etc.
- Update concern header docs to reflect auto-generated scope (was
  stale "define your own" instruction)
- Spec column assertions now dynamically check all non-blob columns
  instead of hardcoded subset

Signed-off-by: Will Dower <will@dower.dev>

---------

Signed-off-by: Will Dower <will@dower.dev>
Co-authored-by: Will Dower <will@dower.dev>

Author: aaronlippold

app/models/concerns/severity_counts.rb

@@ -4,15 +4,14 @@
 # SeverityCounts - Shared concern for models with severity-categorized rules
 #
 # Provides severity aggregation methods for Component, STIG, and SRG models.
-# Each including model must define its own `with_severity_counts` scope
-# due to model-specific SQL conditions.
+# Auto-generates a `with_severity_counts` scope based on model type.
+# Heavy columns (xml, binary) are excluded from the SELECT to prevent
+# memory blowout on index pages.
 #
 # Usage:
 #   class Component < ApplicationRecord
 #     include SeverityCounts
-#
-#     scope :with_severity_counts, -> { ... }  # Model-specific SQL
-#     def rules_association; rules; end         # Define association to query
+#     def rules_association; rules; end  # Define association to query
 #   end
 #
 module SeverityCounts
@@ -29,11 +28,22 @@ def as_json(options = {})
     super.merge({ severity_counts: severity_counts_hash })
   end
 
+  # Column types that are never needed on index/list pages and can be
+  # multi-MB per row. Excluding them from `with_severity_counts` prevents
+  # Heroku dyno memory blowout (R14/R15) on endpoints like GET /stigs.
+  # Models without these column types are unaffected (all columns loaded).
+  # Uses ActiveRecord's abstract `type` (not adapter-specific `sql_type`)
+  # so it works across databases (Postgres xml/bytea, MySQL blob, etc.).
+  HEAVY_COLUMN_TYPES = %i[xml binary].freeze
+
   included do
     ##
     # Auto-generate with_severity_counts scope based on model type
     #
-    # Detects model type and creates appropriate SQL subqueries
+    # Detects model type and creates appropriate SQL subqueries.
+    # Automatically excludes heavy columns (xml, binary) from the SELECT
+    # to prevent memory blowout on index pages. Use `Stig.find(id)` (no
+    # scope) when you need the full record including xml for export/download.
     scope :with_severity_counts, lambda {
       table = table_name
       rule_type = "#{name.gsub('SecurityRequirementsGuide', 'Srg')}Rule" # Component->ComponentRule, Stig->StigRule, SRG->SrgRule
@@ -54,8 +64,14 @@ def as_json(options = {})
       # Add type filter for STI (not needed for Component since it uses component_id)
       type_condition = name == 'Component' ? '' : "AND base_rules.type = '#{rule_type}' "
 
+      # Select only lightweight columns — exclude xml/binary blobs that can be
+      # multi-MB per row. This is the DRY fix: all models using this concern
+      # automatically benefit without per-controller workarounds.
+      lightweight_cols = columns.reject { |c| HEAVY_COLUMN_TYPES.include?(c.type) }
+                                .map { |c| "#{table}.#{c.name}" }
+
       select(
-        "#{table}.*",
+        *lightweight_cols,
         "(SELECT COUNT(*) FROM base_rules WHERE #{fk_condition} #{type_condition}" \
         "AND base_rules.rule_severity = 'high') AS severity_high_count",
         "(SELECT COUNT(*) FROM base_rules WHERE #{fk_condition} #{type_condition}" \

spec/models/concerns/severity_counts_spec.rb

@@ -153,4 +153,64 @@ def actual_counts(record)
       expect(json[:severity_counts]).to eq(expected)
     end
   end
+
+  describe 'heavy column exclusion' do
+    # REQUIREMENT: with_severity_counts must NOT load xml/binary columns.
+    # These columns store multi-MB STIG/SRG XML documents. Loading them on
+    # index pages blows Heroku dyno memory (R14/R15) and causes 30s timeouts (H12).
+    # The xml column is only needed for export/download, not for listing.
+
+    it 'excludes xml columns from STIG queries' do
+      stig = create(:stig)
+      loaded = Stig.with_severity_counts.find(stig.id)
+
+      # The record should NOT have the xml attribute loaded
+      expect(loaded.has_attribute?(:xml)).to be false
+    end
+
+    it 'excludes xml columns from SRG queries' do
+      loaded = SecurityRequirementsGuide.with_severity_counts.find(srg.id)
+
+      expect(loaded.has_attribute?(:xml)).to be false
+    end
+
+    it 'still includes all non-blob columns for STIGs' do
+      stig = create(:stig)
+      loaded = Stig.with_severity_counts.find(stig.id)
+
+      expected_cols = Stig.columns.reject { |c| SeverityCounts::HEAVY_COLUMN_TYPES.include?(c.type) }
+                          .map(&:name)
+      expected_cols.each do |col|
+        expect(loaded.has_attribute?(col)).to be(true), "Expected #{col} to be loaded"
+      end
+    end
+
+    it 'still includes all non-blob columns for SRGs' do
+      loaded = SecurityRequirementsGuide.with_severity_counts.find(srg.id)
+
+      expected_cols = SecurityRequirementsGuide.columns
+                                               .reject { |c| SeverityCounts::HEAVY_COLUMN_TYPES.include?(c.type) }
+                                               .map(&:name)
+      expected_cols.each do |col|
+        expect(loaded.has_attribute?(col)).to be(true), "Expected #{col} to be loaded"
+      end
+    end
+
+    it 'does not affect models without heavy columns (Component)' do
+      loaded = Component.with_severity_counts.find(component.id)
+
+      # Component has no xml column — all columns should be present
+      expect(loaded.has_attribute?(:id)).to be true
+      expect(loaded.has_attribute?(:name)).to be true
+      expect(loaded.has_attribute?(:description)).to be true
+    end
+
+    it 'xml is still loadable via explicit query (for export/download)' do
+      stig = create(:stig)
+
+      # Direct find without scope — loads everything including xml
+      full_stig = Stig.find(stig.id)
+      expect(full_stig.has_attribute?(:xml)).to be true
+    end
+  end
 end