fix(security): remediate CodeQL ReDoS, SQL injection, and CLI-injection alerts

Addresses the remaining open code-scanning alerts on main. The 138 alerts
were only 5 rules; 118 were a single mechanical pattern.

polynomial-redos (118): almost all were s.matches(".*RE.*"). String.matches
already anchors, so the wrapping .* exists only to undo that anchoring, and
.* plus an alternation is what backtracks. Rewritten to
PatternUtil.containsPattern (find() over a cached Pattern) at 174 sites in
17 files -- more than the 118 flagged, since CodeQL only reports where taint
reaches, and the untainted ones are the same hazard. Equivalence was checked
by differential test over all 175 literals rather than by inspection.

This changes behavior on multi-line input: `.` does not cross a newline, so
the anchored form failed to match a keyword after a line break and find()
matches it. That is a fix for intent classifiers, and only affects callers
that do not pre-normalize.

The rest were compiled Pattern constants with ambiguous quantifiers, fixed
individually with possessive quantifiers or bounded gaps. PlanPatternLibrary
also had a latent bug: [^from]+ is a character class, so any column with f,
r, o or m in it (order_id) defeated the collapse.

sql-injection (15): three different cases. CardinalityEstimationService was
a real hole -- quoteIdentifier wrapped in quotes without doubling an embedded
quote, so a table named `x" ; DROP TABLE users; --` escaped it. Four of the
five other quoteIdentifier implementations here already escape correctly. It
now delegates to the dialect's SamplingProvider (removing an if/else on
dbType) and resolves both identifiers against information_schema first.
MySQLPrivilegeCheckProvider concatenated a database name into a literal; now
bound. QueryExecutorService and the EXPLAIN providers execute user SQL by
design -- that is the Editor, guarded by the policy layer, not parameterizable.

spring-disabled-csrf-protection: correct as-is; every route is STATELESS with
header-carried tokens, so there is no cookie session to forge. Documented in
place rather than changed.

command-line-injection: spawn already passed array args, but authorize_url
comes from a server response and the win32 branch goes through cmd. Now
scheme-validated to http/https.

Verified: full backend suite shows the same 13 failures / 4 errors as the
untouched baseline (confirmed by stashing) -- no new regressions. 25 new/
touched unit tests and 259 MCP tests pass.

Author: notSumit25

CLAUDE.md

@@ -351,6 +351,63 @@ points are covered by that single call site (`establishTunnel` and
   via `deepsql.ssh.host-guard.allowed-hosts` (exact host, or a leading-dot suffix like
   `.corp.internal`).
 
+### CodeQL Remediation (code scanning, 138 alerts on main)
+
+The 138 open alerts were only **5 rules**, and the counter badly overstates the
+work: 118 were one mechanical pattern. What was fixed and what was not:
+
+- **`java/polynomial-redos` (118).** Nearly all were `s.matches(".*RE.*")`.
+  `String.matches` anchors the whole input, so the wrapping `.*` exists only to
+  undo that anchoring — and `.*` + alternation is the backtracking. Rewritten to
+  `PatternUtil.containsPattern(s, "RE")` (`find()` over a cached compiled
+  Pattern) at **174 sites in 17 files** — more than the 118 flagged, since
+  CodeQL only reports where taint reaches. Equivalence was verified by
+  differential test over all 175 literals, not by inspection.
+  - **This is a behavior change on multi-line input.** `.` does not cross a
+    newline, so the old form *failed* to match a keyword after a line break;
+    `find()` matches it. That is a bug fix for intent classifiers, and it only
+    affects callers that do not pre-normalize — `PromptIntentSignals.normalize`
+    already collapses newlines, `ChatContextAssembler` does not.
+  - The remainder were compiled `Pattern` constants with genuinely ambiguous
+    quantifiers, fixed individually with possessive quantifiers / bounded gaps
+    (`PostgresSlowLogPatterns`, `QueryNormalizer`, `OptdOptimizationService`,
+    `SqlUsageService`, `QueryPlanCacheService`, `ChatHistoryService`,
+    `CompanyKnowledgeService`, `QueryExecutionPolicyService`).
+  - `PlanPatternLibraryService` also carried a real latent bug: `[^from]+` is a
+    character class ("not f/r/o/m"), so any column containing those letters
+    (`order_id`, `from_date`) defeated the collapse. Now a bounded lazy scan.
+- **`java/sql-injection` (15).** Not one bug — three distinct cases:
+  - `CardinalityEstimationService` (6) was a **real vulnerability**:
+    `quoteIdentifier` wrapped in quotes but never doubled an embedded quote, so
+    a table named `x" ; DROP TABLE users; --` escaped the quoting. Four of the
+    other five `quoteIdentifier` implementations in this repo already escape
+    correctly — this one was the outlier. It now delegates to the dialect's
+    `SamplingProvider` (also removing an if/else on `dbType`), and both
+    identifiers are resolved against `information_schema` first, so only
+    catalog-returned names ever reach interpolated SQL.
+  - `MySQLPrivilegeCheckProvider` (1) concatenated a database name into a
+    string literal; now a bind parameter.
+  - `QueryExecutorService` (3) and the EXPLAIN providers (4) execute
+    user-authored SQL **by design** — that is the Editor feature. They are not
+    parameterizable; their protection is the guard layer in the SQL Editor Guard
+    Rules above. Do not "fix" these by mangling the SQL.
+- **`java/spring-disabled-csrf-protection` (1).** Correct as-is and documented
+  in `SecurityConfig`: every route is `STATELESS` with header-carried JWT/MCP
+  tokens, so there is no ambient cookie session to forge. Re-enable CSRF the
+  moment any cookie-based auth appears.
+- **`js/command-line-injection` (1).** `spawn` already used array args (no
+  shell), but `authorize_url` comes from a server response and the win32 branch
+  routes through `cmd`. Now scheme-validated to http/https before opening.
+
+**Scan-flapping, confirmed.** Analyses on `main` report 137 results
+consistently — except commit `8b47c67`, which reported **3**. That is the commit
+GitHub labelled "Fixed in branch main"; the next healthy scan re-found
+everything and it showed as "Reappeared". Nothing was fixed or reverted. Before
+concluding an alert is resolved, check `results_count` on the analysis
+(`gh api repos/.../code-scanning/analyses?ref=...`) — a partial scan reads as a
+clean one. Note also that PR-triggered scans are diff-scoped and legitimately
+report 0 for untouched files.
+
 ### Data Model Rules
 
 - **`mcp_tokens.user_id` is a non-null FK with no cascade.** Deleting a user who holds

backend/src/main/java/com/dbaagent/config/SecurityConfig.java

@@ -58,6 +58,11 @@ public class SecurityConfig {
     @Bean
     public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Exception {
         http
+                // Deliberate, not an oversight (CodeQL java/spring-disabled-csrf-protection):
+                // every authenticated route is SessionCreationPolicy.STATELESS and carries
+                // its credential in an Authorization / MCP token header, which a browser
+                // does not attach automatically. With no ambient cookie session there is no
+                // CSRF to forge. Re-enable the moment any cookie-based auth is introduced.
                 .csrf(csrf -> csrf.disable())
                 .cors(cors -> cors.configurationSource(corsConfigurationSource()));
 

backend/src/main/java/com/dbaagent/provider/mysql/MySQLPrivilegeCheckProvider.java

@@ -6,6 +6,7 @@
 import org.springframework.stereotype.Component;
 
 import java.sql.Connection;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
@@ -50,22 +51,28 @@ private ConnectionTestResult.PrivilegeCheck checkSelectOnTables(
         Connection connection,
         String database
     ) {
-        try (Statement stmt = connection.createStatement()) {
-            stmt.setQueryTimeout(10);
+        boolean scopedToDatabase = database != null && !database.isEmpty();
+        String listTablesQuery = scopedToDatabase
+            ? "SELECT table_name FROM information_schema.tables "
+                + "WHERE table_schema = ? AND table_type = 'BASE TABLE' LIMIT 5"
+            : "SELECT table_name FROM information_schema.tables "
+                + "WHERE table_type = 'BASE TABLE' LIMIT 5";
 
-            String listTablesQuery = database != null && !database.isEmpty()
-                ? "SELECT table_name FROM information_schema.tables " +
-                    "WHERE table_schema = '" + database + "' " +
-                    "AND table_type = 'BASE TABLE' LIMIT 5"
-                : "SELECT table_name FROM information_schema.tables " +
-                    "WHERE table_type = 'BASE TABLE' LIMIT 5";
+        // Bound rather than concatenated: a database name containing an
+        // apostrophe closed the literal and appended arbitrary SQL
+        // (java/sql-injection).
+        try (PreparedStatement stmt = connection.prepareStatement(listTablesQuery)) {
+            stmt.setQueryTimeout(10);
+            if (scopedToDatabase) {
+                stmt.setString(1, database);
+            }
 
-            ResultSet tablesRs = stmt.executeQuery(listTablesQuery);
             List<String> tables = new ArrayList<>();
-            while (tablesRs.next()) {
-                tables.add(tablesRs.getString(1));
+            try (ResultSet tablesRs = stmt.executeQuery()) {
+                while (tablesRs.next()) {
+                    tables.add(tablesRs.getString(1));
+                }
             }
-            tablesRs.close();
 
             if (tables.isEmpty()) {
                 return ConnectionTestResult.PrivilegeCheck.builder()

backend/src/main/java/com/dbaagent/service/ChatContextAssembler.java

@@ -35,6 +35,7 @@
 import com.dbaagent.repository.ColumnValueCacheRepository;
 import com.dbaagent.repository.SchemaDocumentationRepository;
 import com.dbaagent.service.brain.classification.SchemaClassificationService;
+import com.dbaagent.util.PatternUtil;
 import com.dbaagent.util.TokenEstimator;
 import com.dbaagent.service.SchemaObjectNameUtil;
 import com.dbaagent.service.SchemaTableMatchUtil;
@@ -146,10 +147,10 @@ public Set<ContextType> determineNeededContext(String message) {
         needed.add(ContextType.RELATIONSHIPS);
 
         // Simple schema/structure questions - minimal context needed
-        boolean isSimpleSchemaQuestion = lowerMessage.matches(".*(show|list|what).*(tables?|columns?|schema|views?).*") ||
-            lowerMessage.matches(".*(how many|count).*(tables?|rows?|records?).*") ||
-            lowerMessage.matches(".*(describe|structure|definition).*") ||
-            lowerMessage.matches(".*(largest|biggest|smallest|size).*table.*");
+        boolean isSimpleSchemaQuestion = PatternUtil.containsPattern(lowerMessage, "(show|list|what).*(tables?|columns?|schema|views?)") ||
+            PatternUtil.containsPattern(lowerMessage, "(how many|count).*(tables?|rows?|records?)") ||
+            PatternUtil.containsPattern(lowerMessage, "(describe|structure|definition)") ||
+            PatternUtil.containsPattern(lowerMessage, "(largest|biggest|smallest|size).*table");
 
         if (isSimpleSchemaQuestion) {
             // For simple questions, only add relationships (minimal context)
@@ -160,9 +161,9 @@ public Set<ContextType> determineNeededContext(String message) {
         needed.add(ContextType.SEMANTIC_MODEL);
 
         // Performance-related questions (tight patterns to avoid false positives on data queries)
-        if (lowerMessage.matches(".*(slow quer|performance|optimize|speed up|latency|execution time|response time).*") ||
-            lowerMessage.matches(".*(why.{0,20}(slow|taking|long)|taking too long|how long.{0,10}(quer|execut)).*") ||
-            lowerMessage.matches(".*(query.{0,10}(slow|fast|quick|seconds|minutes)|timeout|timed? out).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(slow quer|performance|optimize|speed up|latency|execution time|response time)") ||
+            PatternUtil.containsPattern(lowerMessage, "(why.{0,20}(slow|taking|long)|taking too long|how long.{0,10}(quer|execut))") ||
+            PatternUtil.containsPattern(lowerMessage, "(query.{0,10}(slow|fast|quick|seconds|minutes)|timeout|timed? out)")) {
             needed.add(ContextType.SLOW_QUERIES);
             needed.add(ContextType.REGRESSIONS);
             needed.add(ContextType.INDEX_RECOMMENDATIONS);
@@ -171,46 +172,46 @@ public Set<ContextType> determineNeededContext(String message) {
         }
 
         // Tuning/configuration questions - Brain ML insights
-        if (lowerMessage.matches(".*(tun(e|ing)|config|parameter|knob|setting|memory|buffer|cache).*") ||
-            lowerMessage.matches(".*(workload|oltp|olap|batch|throughput|qps).*") ||
-            lowerMessage.matches(".*(cardinality|selectivity|statistic|estimate|plan|cost).*") ||
-            lowerMessage.matches(".*(recommend|suggestion|improve|better).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(tun(e|ing)|config|parameter|knob|setting|memory|buffer|cache)") ||
+            PatternUtil.containsPattern(lowerMessage, "(workload|oltp|olap|batch|throughput|qps)") ||
+            PatternUtil.containsPattern(lowerMessage, "(cardinality|selectivity|statistic|estimate|plan|cost)") ||
+            PatternUtil.containsPattern(lowerMessage, "(recommend|suggestion|improve|better)")) {
             needed.add(ContextType.BRAIN_INSIGHTS);
         }
 
         // Index-related questions
-        if (lowerMessage.matches(".*(index|indexes|indexed|indexing).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(index|indexes|indexed|indexing)")) {
             needed.add(ContextType.INDEX_RECOMMENDATIONS);
             needed.add(ContextType.KEY_COLUMNS);
         }
 
         // Value dictionary / enum / filter-value questions
-        if (lowerMessage.matches(".*(valid values|allowed values|possible values|status values|enum|picklist|dropdown).*") ||
-            lowerMessage.matches(".*(what values|which values|acceptable values).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(valid values|allowed values|possible values|status values|enum|picklist|dropdown)") ||
+            PatternUtil.containsPattern(lowerMessage, "(what values|which values|acceptable values)")) {
             needed.add(ContextType.KEY_COLUMNS);
             needed.add(ContextType.CLASSIFICATION);
         }
 
         // Join-specific questions also get classification context
-        if (lowerMessage.matches(".*(join|relationship|foreign key|fk|reference|connect|link).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(join|relationship|foreign key|fk|reference|connect|link)")) {
             needed.add(ContextType.CLASSIFICATION);
         }
 
         // Growth/scaling questions
-        if (lowerMessage.matches(".*(grow|growth|scale|scaling|storage|disk|bloat|archive).*") ||
-            lowerMessage.matches(".*(partition|shard).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(grow|growth|scale|scaling|storage|disk|bloat|archive)") ||
+            PatternUtil.containsPattern(lowerMessage, "(partition|shard)")) {
             needed.add(ContextType.GROWTH);
             needed.add(ContextType.CLASSIFICATION);
         }
 
         // Analysis/review/audit questions - full context
-        if (lowerMessage.matches(".*(analyze|analysis|review|audit|health|diagnose|assessment).*") ||
-            lowerMessage.matches(".*(what.*wrong|issue|problem|bottleneck).*")) {
+        if (PatternUtil.containsPattern(lowerMessage, "(analyze|analysis|review|audit|health|diagnose|assessment)") ||
+            PatternUtil.containsPattern(lowerMessage, "(what.*wrong|issue|problem|bottleneck)")) {
             needed.addAll(EnumSet.allOf(ContextType.class));
         }
 
         // Complex SQL generation - add helpful context
-        if (lowerMessage.matches(".*(select|insert|update|delete|query).*") && lowerMessage.length() > 50) {
+        if (PatternUtil.containsPattern(lowerMessage, "(select|insert|update|delete|query)") && lowerMessage.length() > 50) {
             needed.add(ContextType.KEY_COLUMNS);
             needed.add(ContextType.RELATIONSHIPS);
             needed.add(ContextType.CLASSIFICATION);

backend/src/main/java/com/dbaagent/service/ChatHistoryService.java

@@ -254,7 +254,7 @@ private String summarizeTitleFromMessage(String message) {
             return "New chat";
         }
 
-        String punctuationTrimmed = normalized.replaceAll("[\\s?.!,;:]+$", "");
+        String punctuationTrimmed = normalized.replaceAll("[\\s?.!,;:]++$", "");
         String candidate = punctuationTrimmed.isBlank() ? normalized : punctuationTrimmed;
         if (candidate.length() <= AUTO_TITLE_MAX_LENGTH) {
             return candidate;

backend/src/main/java/com/dbaagent/service/ChatQuestionRoutingService.java

@@ -1,5 +1,6 @@
 package com.dbaagent.service;
 
+import com.dbaagent.util.PatternUtil;
 import org.springframework.stereotype.Service;
 
 import java.util.ArrayList;
@@ -226,7 +227,7 @@ private boolean looksLikeBusinessPerformancePrompt(String normalized) {
         if (normalized == null || normalized.isBlank() || !normalized.contains("performance")) {
             return false;
         }
-        return normalized.matches(".*\\b(payment|payments|gateway|refund|refunds|revenue|booking|bookings|customer|customers|customer|customers|account|accounts|billing|transaction|transactions)\\b.*");
+        return PatternUtil.containsPattern(normalized, "\\b(payment|payments|gateway|refund|refunds|revenue|booking|bookings|customer|customers|customer|customers|account|accounts|billing|transaction|transactions)\\b");
     }
 
     private boolean looksLikeExactSchemaQuestion(String normalized) {
@@ -238,20 +239,20 @@ private boolean looksLikeExactSchemaQuestion(String normalized) {
             || SchemaQuestionUtil.looksLikeExactTableColumnQuestion(normalized)) {
             return true;
         }
-        if (!normalized.matches(".*\\b(table|tables|view|views|column|columns|fields|schema|structure|describe|definition)\\b.*")) {
+        if (!PatternUtil.containsPattern(normalized, "\\b(table|tables|view|views|column|columns|fields|schema|structure|describe|definition)\\b")) {
             return false;
         }
         // Superlatives and rankings are not exact-schema lookups; neither is design advice.
         // "Which tables should I use to build an accounts module?" names tables but wants
         // reasoning over the schema, not a listing of it — answering it from cached
         // metadata drops exactly the part the user asked for.
-        if (normalized.matches(".*\\b(least|most|best|worst|top|bottom|largest|smallest|used|unused|slow|growth|performance|fact|dimension|pattern|relationship|join)\\b.*")
-            || normalized.matches(".*\\b(should|build|design|model|recommend|suggest|architect)\\b.*")) {
+        if (PatternUtil.containsPattern(normalized, "\\b(least|most|best|worst|top|bottom|largest|smallest|used|unused|slow|growth|performance|fact|dimension|pattern|relationship|join)\\b")
+            || PatternUtil.containsPattern(normalized, "\\b(should|build|design|model|recommend|suggest|architect)\\b")) {
             return false;
         }
-        return normalized.matches(".*\\b(what|which|show|list|display|describe|structure|schema)\\b.*\\b(columns?|fields?|tables?|views?)\\b.*")
-            || normalized.matches(".*\\b(columns?|fields?)\\b.*\\b(in|for|of|on)\\b.*")
-            || normalized.matches(".*\\bdescribe\\b.*\\b(table|view)\\b.*")
-            || normalized.matches(".*\\b(schema|structure|definition)\\b.*\\b(of|for)\\b.*\\b(table|view)\\b.*");
+        return PatternUtil.containsPattern(normalized, "\\b(what|which|show|list|display|describe|structure|schema)\\b.*\\b(columns?|fields?|tables?|views?)\\b")
+            || PatternUtil.containsPattern(normalized, "\\b(columns?|fields?)\\b.*\\b(in|for|of|on)\\b")
+            || PatternUtil.containsPattern(normalized, "\\bdescribe\\b.*\\b(table|view)\\b")
+            || PatternUtil.containsPattern(normalized, "\\b(schema|structure|definition)\\b.*\\b(of|for)\\b.*\\b(table|view)\\b");
     }
 }