fix: replace empty catch blocks with specific exception types (#244)

* fix: replace empty catch blocks with specific exception types (#213)

- NamedRangeCommands.Operations.cs: Replace empty catch blocks with
  COMException handlers for COM interop resilience
- PowerQueryCommands.Lifecycle.cs: Replace generic Exception catch with
  COMException for accurate error handling
- ExcelComSmokeTests.cs: Replace empty catches with InvalidOperationException
  and Win32Exception handlers for process cleanup

Addresses CodeQL alerts: cs/empty-catch-block, cs/catch-of-all-exceptions

Fixes #213

* fix: replace additional empty catch blocks with specific exception types

- VbaCommands.cs: Replace empty catch and generic catch with SecurityException
  for registry access operations
- PowerQueryCommands.Lifecycle.cs: Replace empty catch with COMException for
  CommandText property access
- PivotTableCommands.Lifecycle.cs: Replace empty catches with COMException for
  field count access (RowFields, ColumnFields, DataFields, PageFields)
- DataModelCommands.Read.cs: Replace empty catches with COMException and
  RuntimeBinderException for FormatInformation/FormatString access

All catches now have specific exception types and explanatory comments.

* chore: configure CodeQL suppressions for intentional patterns

Suppress CodeQL alerts for legitimate exception handling patterns:

- MCP Tools: catch(Exception) required for JSON error responses (MCP protocol)
- CLI Commands: catch(Exception) for user-friendly error messages
- COM Interop: exception handling at session boundaries for cleanup
- Dynamic COM: late binding is standard pattern for Excel automation
- GC.Collect: required for COM object cleanup per Microsoft guidance
- Test paths: Path.Combine safe in isolated test directories
- Generated code: Regex generator output triggers false positives

These patterns are intentional by design, not bugs to fix.

Author: sbroenne

.github/codeql/codeql-config.yml

@@ -42,17 +42,90 @@ paths:
   - 'src/**'
   - 'tests/**'
 
-# Query filters - TEMPORARILY DISABLED to see all real issues
-# Will re-enable targeted suppressions after fixing legitimate bugs
-query-filters: []
-
-# SUPPRESSED RULES (to be re-added selectively after cleanup):
-# - cs/invalid-dynamic-call: COM interop dynamic calls (20 errors - false positive)
-# - cs/call-to-gc: Explicit GC for COM cleanup (66 warnings - intentional)
-# - cs/catch-of-all-exceptions: Need to audit - some legitimate, some not (1,349)
-# - cs/empty-catch-block: Need to audit - some legitimate COM cleanup, some bugs (162)
-# - cs/path-combine: Test code path handling (463)
-# - Style suggestions: nested-if, missed-ternary, linq suggestions, etc.
+# Query filters - Suppress intentional patterns
+query-filters:
+  # ============================================================================
+  # INTENTIONAL PATTERN: Generic Exception Catches in MCP Tools
+  # MCP protocol requires tools to return JSON responses for ALL errors.
+  # Throwing exceptions would break the MCP protocol contract.
+  # ============================================================================
+  - exclude:
+      id: cs/catch-of-all-exceptions
+      paths:
+        - src/ExcelMcp.McpServer/Tools/**
+
+  # ============================================================================
+  # INTENTIONAL PATTERN: Generic Exception Catches in CLI Commands
+  # CLI commands catch exceptions to display user-friendly error messages.
+  # This is the standard pattern for command-line applications.
+  # ============================================================================
+  - exclude:
+      id: cs/catch-of-all-exceptions
+      paths:
+        - src/ExcelMcp.CLI/Commands/**
+        - src/ExcelMcp.CLI/Program.cs
+
+  # ============================================================================
+  # INTENTIONAL PATTERN: COM Interop Exception Handling
+  # Excel COM automation requires catching exceptions at session/batch boundaries
+  # to ensure proper COM cleanup and resource management.
+  # ============================================================================
+  - exclude:
+      id: cs/catch-of-all-exceptions
+      paths:
+        - src/ExcelMcp.ComInterop/Session/**
+
+  # ============================================================================
+  # INTENTIONAL PATTERN: Dynamic COM Calls
+  # Excel COM interop uses late binding (dynamic) extensively.
+  # These are not invalid calls - they're the standard COM interop pattern.
+  # ============================================================================
+  - exclude:
+      id: cs/invalid-dynamic-call
+      paths:
+        - src/ExcelMcp.Core/**
+        - src/ExcelMcp.ComInterop/**
+
+  # ============================================================================
+  # INTENTIONAL PATTERN: Explicit GC for COM Cleanup
+  # GC.Collect() is required after releasing COM objects to ensure
+  # Excel process cleanup. This follows Microsoft guidance for COM interop.
+  # ============================================================================
+  - exclude:
+      id: cs/call-to-gc
+      paths:
+        - src/ExcelMcp.ComInterop/**
+
+  # ============================================================================
+  # TEST CODE: Path.Combine with User Input
+  # Test code constructs paths from test parameters. This is safe because
+  # tests run in isolated temp directories with controlled inputs.
+  # ============================================================================
+  - exclude:
+      id: cs/path-combine
+      paths:
+        - tests/**
+
+  # ============================================================================
+  # AUTO-GENERATED CODE: Regex Generator
+  # System.Text.RegularExpressions.Generator produces code with patterns
+  # that trigger CodeQL alerts. This code is compiler-generated and safe.
+  # ============================================================================
+  - exclude:
+      id: cs/useless-cast-to-self
+      paths:
+        - '**/obj/**'
+        - '**/*.g.cs'
+  - exclude:
+      id: cs/useless-assignment-to-local
+      paths:
+        - '**/obj/**'
+        - '**/*.g.cs'
+  - exclude:
+      id: cs/complex-block
+      paths:
+        - '**/obj/**'
+        - '**/*.g.cs'
 
 # External data sources (for taint tracking)
 # external-repository-token: ${{ secrets.GITHUB_TOKEN }}

src/ExcelMcp.Core/Commands/DataModel/DataModelCommands.Read.cs

@@ -178,14 +178,28 @@ public DataModelMeasureViewResult Read(IExcelBatch batch, string measureName)
                         {
                             result.FormatString = formatInfo.FormatString?.ToString();
                         }
-                        catch { /* FormatString may not be accessible */ }
+                        catch (System.Runtime.InteropServices.COMException)
+                        {
+                            // FormatString property may not be accessible in certain Excel versions
+                        }
+                        catch (Microsoft.CSharp.RuntimeBinder.RuntimeBinderException)
+                        {
+                            // FormatString property may not exist on this COM object type
+                        }
                         finally
                         {
                             ComUtilities.Release(ref formatInfo);
                         }
                     }
                 }
-                catch { /* FormatInformation may not be available in all Excel versions */ }
+                catch (System.Runtime.InteropServices.COMException)
+                {
+                    // FormatInformation may not be available in older Excel versions
+                }
+                catch (Microsoft.CSharp.RuntimeBinder.RuntimeBinderException)
+                {
+                    // FormatInformation property may not exist on this COM object type
+                }
 
                 result.Success = true;
             }

src/ExcelMcp.Core/Commands/NamedRange/NamedRangeCommands.Operations.cs

@@ -1,3 +1,4 @@
+using System.Runtime.InteropServices;
 using Sbroenne.ExcelMcp.ComInterop;
 using Sbroenne.ExcelMcp.ComInterop.Session;
 using Sbroenne.ExcelMcp.Core.Models;
@@ -51,7 +52,11 @@ public List<NamedRangeInfo> List(IExcelBatch batch)
                                 valueType = rawValue?.GetType().Name ?? "null";
                             }
                         }
-                        catch { }
+                        catch (COMException)
+                        {
+                            // Named range may not have a valid RefersToRange (e.g., formula-based or external reference)
+                            // Continue with null value - this is expected for some named ranges
+                        }
 
                         namedRanges.Add(new NamedRangeInfo
                         {
@@ -61,7 +66,11 @@ public List<NamedRangeInfo> List(IExcelBatch batch)
                             ValueType = valueType
                         });
                     }
-                    catch { }
+                    catch (COMException)
+                    {
+                        // Skip corrupted or inaccessible named ranges - continue listing remaining
+                        continue;
+                    }
                     finally
                     {
                         ComUtilities.Release(ref refersToRange);

src/ExcelMcp.Core/Commands/PivotTable/PivotTableCommands.Lifecycle.cs

@@ -92,25 +92,37 @@ public PivotTableListResult List(IExcelBatch batch)
                                 {
                                     rowFieldCount = pivot.RowFields.Count;
                                 }
-                                catch { /* Count might fail for certain configurations */ }
+                                catch (System.Runtime.InteropServices.COMException)
+                                {
+                                    // RowFields.Count may fail for Data Model or OLAP PivotTables
+                                }
 
                                 try
                                 {
                                     columnFieldCount = pivot.ColumnFields.Count;
                                 }
-                                catch { /* Count might fail for certain configurations */ }
+                                catch (System.Runtime.InteropServices.COMException)
+                                {
+                                    // ColumnFields.Count may fail for Data Model or OLAP PivotTables
+                                }
 
                                 try
                                 {
                                     valueFieldCount = pivot.DataFields.Count;
                                 }
-                                catch { /* Count might fail for certain configurations */ }
+                                catch (System.Runtime.InteropServices.COMException)
+                                {
+                                    // DataFields.Count may fail for Data Model or OLAP PivotTables
+                                }
 
                                 try
                                 {
                                     filterFieldCount = pivot.PageFields.Count;
                                 }
-                                catch { /* Count might fail for certain configurations */ }
+                                catch (System.Runtime.InteropServices.COMException)
+                                {
+                                    // PageFields.Count may fail for Data Model or OLAP PivotTables
+                                }
 
                                 var info = new PivotTableInfo
                                 {

src/ExcelMcp.Core/Commands/PowerQuery/PowerQueryCommands.Lifecycle.cs

@@ -1,3 +1,4 @@
+using System.Runtime.InteropServices;
 using Sbroenne.ExcelMcp.ComInterop;
 using Sbroenne.ExcelMcp.ComInterop.Session;
 using Sbroenne.ExcelMcp.Core.Models;
@@ -139,11 +140,11 @@ public PowerQueryListResult List(IExcelBatch batch)
                             IsConnectionOnly = isConnectionOnly
                         });
                     }
-                    catch (Exception)
+                    catch (COMException)
                     {
-                        // ✅ Skip query if any error occurs during processing
+                        // Skip query if COM error occurs during processing
                         // This allows listing to continue for remaining queries
-                        // Exceptions are rare - typically only corrupted queries or access issues
+                        // COM exceptions occur for corrupted queries or access issues
                         continue;
                     }
                     finally
@@ -239,7 +240,10 @@ public PowerQueryLoadConfigResult GetLoadConfig(IExcelBatch batch, string queryN
                                     else
                                         commandText = queryTable.CommandText?.ToString() ?? "";
                                 }
-                                catch { /* ignore */ }
+                                catch (System.Runtime.InteropServices.COMException)
+                                {
+                                    // CommandText property may not be accessible for certain QueryTable types
+                                }
 
                                 bool cmdMatches = commandText.Contains($"[{queryName}]", StringComparison.OrdinalIgnoreCase);
 

src/ExcelMcp.Core/Commands/Vba/VbaCommands.cs

@@ -37,13 +37,17 @@ private static bool IsVbaTrustEnabled()
                         return true;
                     }
                 }
-                catch { /* Try next path */ }
+                catch (System.Security.SecurityException)
+                {
+                    // Registry access denied for this path, try next Office version
+                }
             }
 
             return false; // Assume not enabled if cannot read registry
         }
-        catch
+        catch (System.Security.SecurityException)
         {
+            // Registry access completely denied, assume VBA trust not enabled
             return false;
         }
     }

tests/ExcelMcp.ComInterop.Tests/Integration/ExcelComSmokeTests.cs

@@ -43,12 +43,23 @@ public Task InitializeAsync()
                 _output.WriteLine($"Cleaning up {existingProcesses.Length} existing Excel processes...");
                 foreach (var p in existingProcesses)
                 {
-                    try { p.Kill(); p.WaitForExit(2000); } catch { }
+                    try { p.Kill(); p.WaitForExit(2000); }
+                    catch (InvalidOperationException)
+                    {
+                        // Process already exited - safe to ignore
+                    }
+                    catch (System.ComponentModel.Win32Exception)
+                    {
+                        // Access denied or process not available - safe to ignore in cleanup
+                    }
                 }
                 Thread.Sleep(2000);
             }
         }
-        catch { }
+        catch (InvalidOperationException)
+        {
+            // GetProcessesByName failed - safe to ignore in cleanup
+        }
 
         // Create test file
         _testFile = Path.Join(Path.GetTempPath(), $"excel-com-smoke-{Guid.NewGuid():N}.xlsx");