From 06621879d6d866a855a40e795bd7317124667456 Mon Sep 17 00:00:00 2001 From: sumit Date: Mon, 17 Aug 2026 15:37:23 +0530 Subject: [PATCH 1/5] fix(ssh): add opt-in SSRF guard for SSH bastion hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SshTunnelService passed request.getSshHost() straight to jsch.getSession() with no validation, so an authenticated user who can create a connection could aim the tunnel at the cloud metadata endpoint (169.254.169.254) or internal hosts and use the backend as a probe inside the network. This is CodeQL alert #138 (java/ssrf), open since the initial commit — the file had never been modified, and the "fixed then reappeared" timeline on that alert was scan flapping, not a reverted fix. SshHostGuard resolves the host and checks every returned address rather than matching the literal string, so a public hostname whose A record points at a private or link-local address is still refused. Covers loopback, wildcard, link-local, RFC1918, CGNAT, multicast, IPv6 ULA, and IPv4-mapped IPv6 forms. It sits in createSession, so establishTunnel and testSshConnection are both covered by one call site. testSshConnection calls the guard outside its try block: that method catches broad Exception and returns false, which would render a blocked host as an ordinary auth failure. The guard ships disabled. Bastions legitimately live on RFC1918 networks, so enabling it by default would break existing self-hosted installs on upgrade. Note this means a default install is as exposed as before while the CodeQL alert closes — the sanitizer is on the call path regardless of the flag — so a green #138 does not mean deployments are protected. Documented in CLAUDE.md. --- CLAUDE.md | 32 ++++- .../com/dbaagent/service/SshHostGuard.java | 123 ++++++++++++++++++ .../service/SshHostGuardProperties.java | 32 +++++ .../dbaagent/service/SshTunnelService.java | 9 +- .../dbaagent/service/SshHostGuardTest.java | 111 ++++++++++++++++ .../service/SshTunnelServiceTest.java | 3 +- 6 files changed, 307 insertions(+), 3 deletions(-) create mode 100644 backend/src/main/java/com/dbaagent/service/SshHostGuard.java create mode 100644 backend/src/main/java/com/dbaagent/service/SshHostGuardProperties.java create mode 100644 backend/src/test/java/com/dbaagent/service/SshHostGuardTest.java diff --git a/CLAUDE.md b/CLAUDE.md index 2cf64a0..38e5b05 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -197,7 +197,7 @@ returns a number). ### Backend Rules 1. **Database Provider Registry**: Use `DatabaseProviderRegistry` for all DB-specific operations. Do NOT add if/else or switch for database types. 2. **LLM Provider Registry**: Use `LlmProviderRegistry` for all provider-specific LLM behavior. Do NOT add if/else or switch on provider type. Chat and embedding providers are registered and resolved independently — some providers offer only one. Providers are *factories* over credentials, not `ChatModel`s, so credentials stay resolvable per call and key rotation needs no restart. -3. **SSH-Aware Access**: Always use `ConnectionService.getJdbcTemplate(connectionId, request)` — handles SSH tunneling transparently. +3. **SSH-Aware Access**: Always use `ConnectionService.getJdbcTemplate(connectionId, request)` — handles SSH tunneling transparently. The bastion host is screened by `SshHostGuard` before any session is created (see SSRF guard below). 4. **SQL Rule**: All generated SQL MUST use table-qualified column names (`table.column_name`). 5. **RAG Caching**: Three-tier cache (memory → Redis → Azure Search). Redis failure is graceful (app continues without caching). 6. **Virtual Threads**: Enabled for concurrency (JDK 25). @@ -321,6 +321,36 @@ it against a real database — not a theoretical hardening pass. constructs a real `MySQLQueryExecutionProvider`. Do not reintroduce a stubbed dialect here; the mock is what let the blocker ship. +### SSH Tunnel SSRF Guard + +`SshHostGuard` screens `request.getSshHost()` before `jsch.getSession(...)` in +`SshTunnelService.createSession`, closing CodeQL `java/ssrf` alert #138. Both entry +points are covered by that single call site (`establishTunnel` and +`testSshConnection`). + +- **It resolves the host and checks every returned address**, not just the literal + string. A public hostname whose A record points at `169.254.169.254` or `10.x` is + still refused — a string-only check is defeated by one DNS record. +- Blocked: loopback, wildcard, link-local (cloud metadata), RFC1918, CGNAT + (`100.64/10`), multicast, IPv6 ULA (`fc00::/7`), and IPv4-mapped/compatible IPv6 + forms that smuggle a blocked v4 address through a v6 literal. +- **`testSshConnection` calls the guard *outside* its try block.** That method catches + broad `Exception` and returns `false`, so a guard rejection inside it would render a + blocked host as an ordinary auth failure — the silent-failure anti-pattern above. + It propagates `IllegalArgumentException` instead, matching how a missing SSH password + already surfaces. +- **It ships disabled** (`deepsql.ssh.host-guard.enabled=false`). Bastions legitimately + live on RFC1918 networks, so enabling it by default would break existing self-hosted + installs on upgrade. The trade-off is explicit: **on a default install the SSRF + surface is open** — an authenticated user who can create connections can still point + the tunnel at `169.254.169.254` or internal hosts. The CodeQL alert closes either way + (the sanitizer is on the call path regardless of the flag), so a closed alert here + does **not** mean deployments are protected. Do not read #138 going green as + "SSRF handled". +- Operators who want the protection set `enabled=true` and allowlist their own bastion + via `deepsql.ssh.host-guard.allowed-hosts` (exact host, or a leading-dot suffix like + `.corp.internal`). + ### Data Model Rules - **`mcp_tokens.user_id` is a non-null FK with no cascade.** Deleting a user who holds diff --git a/backend/src/main/java/com/dbaagent/service/SshHostGuard.java b/backend/src/main/java/com/dbaagent/service/SshHostGuard.java new file mode 100644 index 0000000..76798a3 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/SshHostGuard.java @@ -0,0 +1,123 @@ +package com.dbaagent.service; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Component; + +import java.net.Inet4Address; +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.Locale; + +/** + * Rejects SSH bastion hosts that point back into infrastructure the caller + * should not reach through DeepSQL — loopback, link-local (including the cloud + * metadata endpoint), and RFC1918 ranges. + * + * The host is resolved and every returned address is checked, so a public + * hostname whose A record points at 10.x or 169.254.169.254 is still refused. + */ +@Component +@Slf4j +public class SshHostGuard { + + private final SshHostGuardProperties properties; + + public SshHostGuard(SshHostGuardProperties properties) { + this.properties = properties; + } + + public void assertAllowed(String sshHost) { + if (sshHost == null || sshHost.isBlank()) { + throw new IllegalArgumentException("SSH host is required"); + } + + String host = normalize(sshHost); + + if (!properties.isEnabled() || isExplicitlyAllowed(host)) { + return; + } + + InetAddress[] addresses; + try { + addresses = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + throw new IllegalArgumentException("SSH host could not be resolved: " + sshHost); + } + + for (InetAddress address : addresses) { + if (isBlocked(address)) { + log.warn("Blocked SSH tunnel to restricted host {} (resolved to {})", + sshHost, address.getHostAddress()); + throw new IllegalArgumentException( + "SSH host '" + sshHost + "' resolves to a restricted address (" + + address.getHostAddress() + "). Add it to " + + "deepsql.ssh.host-guard.allowed-hosts if this is intentional."); + } + } + } + + private boolean isExplicitlyAllowed(String host) { + for (String allowed : properties.getAllowedHosts()) { + if (allowed == null || allowed.isBlank()) continue; + String candidate = normalize(allowed); + if (candidate.startsWith(".")) { + if (host.endsWith(candidate)) return true; + } else if (host.equals(candidate)) { + return true; + } + } + return false; + } + + private boolean isBlocked(InetAddress address) { + if (address.isLoopbackAddress() + || address.isAnyLocalAddress() + || address.isLinkLocalAddress() + || address.isSiteLocalAddress() + || address.isMulticastAddress()) { + return true; + } + if (address instanceof Inet4Address) { + return isBlockedIpv4(address.getAddress()); + } + if (address instanceof Inet6Address v6) { + // Unique local addresses (fc00::/7) have no isSiteLocalAddress() mapping in Java. + if ((v6.getAddress()[0] & 0xFE) == 0xFC) return true; + // IPv4-mapped/compatible forms smuggle a blocked v4 address through a v6 literal. + byte[] embedded = embeddedIpv4(v6); + return embedded != null && isBlockedIpv4(embedded); + } + return false; + } + + private boolean isBlockedIpv4(byte[] octets) { + int first = octets[0] & 0xFF; + int second = octets[1] & 0xFF; + // 100.64.0.0/10 carrier-grade NAT, used for cloud-internal routing. + if (first == 100 && second >= 64 && second <= 127) return true; + // 192.0.0.0/24 IETF protocol assignments. + if (first == 192 && second == 0 && (octets[2] & 0xFF) == 0) return true; + // 0.0.0.0/8 "this network". + return first == 0; + } + + private byte[] embeddedIpv4(Inet6Address address) { + byte[] bytes = address.getAddress(); + for (int i = 0; i < 10; i++) { + if (bytes[i] != 0) return null; + } + boolean mapped = (bytes[10] & 0xFF) == 0xFF && (bytes[11] & 0xFF) == 0xFF; + boolean compat = bytes[10] == 0 && bytes[11] == 0; + if (!mapped && !compat) return null; + return new byte[]{bytes[12], bytes[13], bytes[14], bytes[15]}; + } + + private String normalize(String host) { + String trimmed = host.trim().toLowerCase(Locale.ROOT); + if (trimmed.startsWith("[") && trimmed.endsWith("]")) { + trimmed = trimmed.substring(1, trimmed.length() - 1); + } + return trimmed; + } +} diff --git a/backend/src/main/java/com/dbaagent/service/SshHostGuardProperties.java b/backend/src/main/java/com/dbaagent/service/SshHostGuardProperties.java new file mode 100644 index 0000000..df3ec81 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/SshHostGuardProperties.java @@ -0,0 +1,32 @@ +package com.dbaagent.service; + +import lombok.Data; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.stereotype.Component; + +import java.util.ArrayList; +import java.util.List; + +/** + * Binds the SSH bastion host guard. + * + * Off by default: bastions legitimately live on RFC1918 networks, so enabling + * this would break existing self-hosted installs on upgrade. Operators who want + * the protection set {@code deepsql.ssh.host-guard.enabled=true} and allowlist + * their own bastion. Disabled means the SSRF surface is open — see the SSH + * Tunnel SSRF Guard section in CLAUDE.md. + */ +@Component +@ConfigurationProperties(prefix = "deepsql.ssh.host-guard") +@Data +public class SshHostGuardProperties { + + private boolean enabled = false; + + /** + * Hosts exempt from the private/link-local block. Matched case-insensitively + * against the literal value, and against the resolved IP for hostnames. + * A leading "." denotes a domain suffix match (".corp.example.com"). + */ + private List allowedHosts = new ArrayList<>(); +} diff --git a/backend/src/main/java/com/dbaagent/service/SshTunnelService.java b/backend/src/main/java/com/dbaagent/service/SshTunnelService.java index 2b3c897..cc03613 100644 --- a/backend/src/main/java/com/dbaagent/service/SshTunnelService.java +++ b/backend/src/main/java/com/dbaagent/service/SshTunnelService.java @@ -21,6 +21,7 @@ public class SshTunnelService { private final DatabaseProviderRegistry providerRegistry; + private final SshHostGuard sshHostGuard; private static final int SSH_CONNECT_TIMEOUT = 30000; // 30 seconds private static final String LOCAL_BIND_HOST = "127.0.0.1"; @@ -33,8 +34,9 @@ public class SshTunnelService { for (int i = 0; i < LOCK_STRIPES; i++) tunnelLocks[i] = new Object(); } - public SshTunnelService(DatabaseProviderRegistry providerRegistry) { + public SshTunnelService(DatabaseProviderRegistry providerRegistry, SshHostGuard sshHostGuard) { this.providerRegistry = providerRegistry; + this.sshHostGuard = sshHostGuard; } /** @@ -103,6 +105,8 @@ private Session createSession(ConnectionRequest request) throws JSchException { // Create a new JSch instance per session to avoid shared state issues JSch jsch = new JSch(); + sshHostGuard.assertAllowed(request.getSshHost()); + String sshHost = request.getSshHost(); int sshPort = request.getSshPort() != null ? request.getSshPort() : 22; String sshUsername = request.getSshUsername(); @@ -234,6 +238,9 @@ public boolean testSshConnection(ConnectionRequest request) { return true; // SSH not required } + // Outside the try: a blocked host must surface its reason, not read as a failed login. + sshHostGuard.assertAllowed(request.getSshHost()); + Session session = null; try { log.info("Testing SSH connection to {}@{}:{}", diff --git a/backend/src/test/java/com/dbaagent/service/SshHostGuardTest.java b/backend/src/test/java/com/dbaagent/service/SshHostGuardTest.java new file mode 100644 index 0000000..ce5b3d0 --- /dev/null +++ b/backend/src/test/java/com/dbaagent/service/SshHostGuardTest.java @@ -0,0 +1,111 @@ +package com.dbaagent.service; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class SshHostGuardTest { + + private SshHostGuardProperties properties; + private SshHostGuard guard; + + @BeforeEach + void setUp() { + properties = new SshHostGuardProperties(); + // Ships disabled; these cases cover the behaviour once an operator turns it on. + properties.setEnabled(true); + guard = new SshHostGuard(properties); + } + + @Test + void shipsDisabledByDefault() { + SshHostGuardProperties defaults = new SshHostGuardProperties(); + + assertFalse(defaults.isEnabled()); + assertTrue(defaults.getAllowedHosts().isEmpty()); + assertDoesNotThrow(() -> new SshHostGuard(defaults).assertAllowed("169.254.169.254")); + } + + @ParameterizedTest + @ValueSource(strings = { + "127.0.0.1", + "localhost", + "0.0.0.0", + "10.0.0.5", + "172.16.4.9", + "192.168.1.1", + "169.254.169.254", + "100.64.0.1", + "[::1]", + "[fd00::1]", + "[::ffff:169.254.169.254]" + }) + void blocksRestrictedTargets(String host) { + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed(host)); + } + + @Test + void blockedMessageNamesTheHostAndTheEscapeHatch() { + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> guard.assertAllowed("169.254.169.254")); + + assertTrue(e.getMessage().contains("169.254.169.254"), e.getMessage()); + assertTrue(e.getMessage().contains("allowed-hosts"), e.getMessage()); + } + + @Test + void allowsPublicAddress() { + assertDoesNotThrow(() -> guard.assertAllowed("93.184.216.34")); + } + + @Test + void rejectsBlankHost() { + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed(" ")); + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed(null)); + } + + @Test + void rejectsUnresolvableHost() { + assertThrows(IllegalArgumentException.class, + () -> guard.assertAllowed("no-such-host.invalid")); + } + + @Test + void allowlistExemptsExactHost() { + properties.setAllowedHosts(List.of("10.0.0.5")); + + assertDoesNotThrow(() -> guard.assertAllowed("10.0.0.5")); + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed("10.0.0.6")); + } + + @Test + void allowlistIsCaseInsensitiveAndTrimmed() { + properties.setAllowedHosts(List.of(" LocalHost ")); + + assertDoesNotThrow(() -> guard.assertAllowed("localhost")); + } + + @Test + void allowlistSupportsDomainSuffix() { + properties.setAllowedHosts(List.of(".corp.internal")); + + assertDoesNotThrow(() -> guard.assertAllowed("bastion.corp.internal")); + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed("10.0.0.5")); + } + + @Test + void disabledGuardPermitsEverything() { + properties.setEnabled(false); + + assertDoesNotThrow(() -> guard.assertAllowed("169.254.169.254")); + assertDoesNotThrow(() -> guard.assertAllowed("no-such-host.invalid")); + } +} diff --git a/backend/src/test/java/com/dbaagent/service/SshTunnelServiceTest.java b/backend/src/test/java/com/dbaagent/service/SshTunnelServiceTest.java index db4b2c3..c8756d4 100644 --- a/backend/src/test/java/com/dbaagent/service/SshTunnelServiceTest.java +++ b/backend/src/test/java/com/dbaagent/service/SshTunnelServiceTest.java @@ -26,7 +26,8 @@ class SshTunnelServiceTest { @BeforeEach void setUp() { - sshTunnelService = new SshTunnelService(providerRegistry); + sshTunnelService = new SshTunnelService( + providerRegistry, new SshHostGuard(new SshHostGuardProperties())); } @Test From 563daa129f8c5bb494980a4494cd10684977da23 Mon Sep 17 00:00:00 2001 From: sumit Date: Tue, 18 Aug 2026 09:10:41 +0530 Subject: [PATCH 2/5] 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. --- CLAUDE.md | 57 +++++++++++ .../com/dbaagent/config/SecurityConfig.java | 5 + .../mysql/MySQLPrivilegeCheckProvider.java | 31 +++--- .../service/ChatContextAssembler.java | 41 ++++---- .../dbaagent/service/ChatHistoryService.java | 2 +- .../service/ChatQuestionRoutingService.java | 17 ++-- .../service/ChatRetrievalContextService.java | 23 ++--- .../com/dbaagent/service/ChatService.java | 95 ++++++++++--------- .../service/CompanyKnowledgeService.java | 2 +- .../service/ConversationContextService.java | 3 +- .../service/PostgresSlowLogPatterns.java | 6 +- .../service/QueryExecutionPolicyService.java | 9 +- .../service/QueryOptimizationService.java | 9 +- .../service/QueryPlanCacheService.java | 2 +- .../dbaagent/service/SchemaQuestionUtil.java | 27 +++--- .../com/dbaagent/service/SqlUsageService.java | 5 +- .../service/agent/AgentOrchestrator.java | 3 +- .../dbaagent/service/agent/AgentPlanner.java | 7 +- .../agent/LlmOrchestrationService.java | 3 +- .../agent/MetadataRequestScopeResolver.java | 5 +- .../service/agent/PerformanceExecutor.java | 49 +++++----- .../service/agent/PromptIntentAnalyzer.java | 31 +++--- .../service/agent/SchemaMetadataExecutor.java | 21 ++-- .../service/agent/UniversalChatTool.java | 19 ++-- .../keycolumn/KeyColumnAnalysisService.java | 5 +- .../query/CardinalityEstimationService.java | 50 ++++++++-- .../query/PlanPatternLibraryService.java | 8 +- .../service/optd/OptdOptimizationService.java | 6 +- .../java/com/dbaagent/util/PatternUtil.java | 43 +++++++++ .../dbaagent/util/PromptIntentSignals.java | 6 +- .../com/dbaagent/util/QueryNormalizer.java | 7 +- .../com/dbaagent/util/PatternUtilTest.java | 55 +++++++++++ mcp/src/auth/browser-flow.js | 18 +++- 33 files changed, 456 insertions(+), 214 deletions(-) create mode 100644 backend/src/main/java/com/dbaagent/util/PatternUtil.java create mode 100644 backend/src/test/java/com/dbaagent/util/PatternUtilTest.java diff --git a/CLAUDE.md b/CLAUDE.md index 38e5b05..f695a3e 100644 --- a/CLAUDE.md +++ b/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 diff --git a/backend/src/main/java/com/dbaagent/config/SecurityConfig.java b/backend/src/main/java/com/dbaagent/config/SecurityConfig.java index 0a6520f..6d83335 100644 --- a/backend/src/main/java/com/dbaagent/config/SecurityConfig.java +++ b/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())); diff --git a/backend/src/main/java/com/dbaagent/provider/mysql/MySQLPrivilegeCheckProvider.java b/backend/src/main/java/com/dbaagent/provider/mysql/MySQLPrivilegeCheckProvider.java index 40e69bf..4af64fc 100644 --- a/backend/src/main/java/com/dbaagent/provider/mysql/MySQLPrivilegeCheckProvider.java +++ b/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 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() diff --git a/backend/src/main/java/com/dbaagent/service/ChatContextAssembler.java b/backend/src/main/java/com/dbaagent/service/ChatContextAssembler.java index ef821fd..bf07a21 100644 --- a/backend/src/main/java/com/dbaagent/service/ChatContextAssembler.java +++ b/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 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 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 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); diff --git a/backend/src/main/java/com/dbaagent/service/ChatHistoryService.java b/backend/src/main/java/com/dbaagent/service/ChatHistoryService.java index 88f5f4e..82d7f5d 100644 --- a/backend/src/main/java/com/dbaagent/service/ChatHistoryService.java +++ b/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; diff --git a/backend/src/main/java/com/dbaagent/service/ChatQuestionRoutingService.java b/backend/src/main/java/com/dbaagent/service/ChatQuestionRoutingService.java index 93a3a03..5853483 100644 --- a/backend/src/main/java/com/dbaagent/service/ChatQuestionRoutingService.java +++ b/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"); } } diff --git a/backend/src/main/java/com/dbaagent/service/ChatRetrievalContextService.java b/backend/src/main/java/com/dbaagent/service/ChatRetrievalContextService.java index 340910e..d8a75c7 100644 --- a/backend/src/main/java/com/dbaagent/service/ChatRetrievalContextService.java +++ b/backend/src/main/java/com/dbaagent/service/ChatRetrievalContextService.java @@ -4,6 +4,7 @@ import com.dbaagent.model.QualifiedTableName; import com.dbaagent.model.SchemaMetadata; import com.dbaagent.model.TrainingDataEmbedding; +import com.dbaagent.util.PatternUtil; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -40,13 +41,13 @@ public RetrievalIntent detectRetrievalIntent(String userQuestion) { return RetrievalIntent.GENERAL; } String q = userQuestion.toLowerCase(Locale.ROOT); - if (q.matches(".*(valid values|possible values|allowed values|status values|enum|picklist|dropdown|what values|which values).*")) { + if (PatternUtil.containsPattern(q, "(valid values|possible values|allowed values|status values|enum|picklist|dropdown|what values|which values)")) { return RetrievalIntent.VALUE_LOOKUP; } - if (q.matches(".*(what does|meaning of|define|definition|business meaning|what is an? .*|mrr|arr|revpar|adr|occupancy|glossary|metric definition).*")) { + if (PatternUtil.containsPattern(q, "(what does|meaning of|define|definition|business meaning|what is an? .*|mrr|arr|revpar|adr|occupancy|glossary|metric definition)")) { return RetrievalIntent.BUSINESS_MEANING; } - if (q.matches(".*(sql|query example|example query|show me sql|write sql).*")) { + if (PatternUtil.containsPattern(q, "(sql|query example|example query|show me sql|write sql)")) { return RetrievalIntent.SQL_EXAMPLE; } return RetrievalIntent.GENERAL; @@ -386,20 +387,20 @@ private boolean isSimpleSchemaQuestion(String message) { } String lower = message.toLowerCase(Locale.ROOT); boolean exactTableColumnQuestion = lower.contains("column") - && (lower.matches(".*\\b(how many|count|number of)\\b.*\\bcolumns?\\b.*") - || lower.matches(".*\\b(what|which|show|list|display|describe|schema|structure)\\b.*\\bcolumns?\\b.*") - || lower.matches(".*\\bcolumns?\\b.*\\b(in|for|of|on)\\b.*")); + && (PatternUtil.containsPattern(lower, "\\b(how many|count|number of)\\b.*\\bcolumns?\\b") + || PatternUtil.containsPattern(lower, "\\b(what|which|show|list|display|describe|schema|structure)\\b.*\\bcolumns?\\b") + || PatternUtil.containsPattern(lower, "\\bcolumns?\\b.*\\b(in|for|of|on)\\b")); if (exactTableColumnQuestion) { return true; } - boolean hasScopedQualifier = lower.matches(".*\\b(last|past|today|yesterday|month|week|day|year|active|inactive|for|from|where|between)\\b.*"); + boolean hasScopedQualifier = PatternUtil.containsPattern(lower, "\\b(last|past|today|yesterday|month|week|day|year|active|inactive|for|from|where|between)\\b"); if (hasScopedQualifier) { return false; } - return lower.matches(".*(show|list|what).*(tables?|columns?|schema|views?).*") - || lower.matches(".*(how many|count).*(tables?|rows?|records?).*") - || lower.matches(".*(describe|structure|definition).*") - || lower.matches(".*(largest|biggest|smallest|size).*table.*"); + return PatternUtil.containsPattern(lower, "(show|list|what).*(tables?|columns?|schema|views?)") + || PatternUtil.containsPattern(lower, "(how many|count).*(tables?|rows?|records?)") + || PatternUtil.containsPattern(lower, "(describe|structure|definition)") + || PatternUtil.containsPattern(lower, "(largest|biggest|smallest|size).*table"); } private Set resolveScopedTables( diff --git a/backend/src/main/java/com/dbaagent/service/ChatService.java b/backend/src/main/java/com/dbaagent/service/ChatService.java index 8ed7228..d3e88ad 100644 --- a/backend/src/main/java/com/dbaagent/service/ChatService.java +++ b/backend/src/main/java/com/dbaagent/service/ChatService.java @@ -66,6 +66,7 @@ import com.dbaagent.service.agent.MetadataRequestScope; import com.dbaagent.service.agent.MetadataRequestScopeResolver; import com.dbaagent.service.agent.VerifiedAnswer; +import com.dbaagent.util.PatternUtil; import com.fasterxml.jackson.databind.ObjectMapper; import com.dbaagent.util.QueryNormalizer; import com.dbaagent.service.security.AccessControlService; @@ -461,7 +462,7 @@ private String retiredDirectSchemaAnswer(String message, SchemaMetadata schema) } // "How many tables do I have?" or similar (simple, unscoped questions only) - if (lowerMessage.matches(".*(how many|count|number of).*(tables?).*") && + if (PatternUtil.containsPattern(lowerMessage, "(how many|count|number of).*(tables?)") && !lowerMessage.contains("rows") && !lowerMessage.contains("record")) { long tableCount = resolveTableCount(schema); return String.format("You have **%d tables** in the `%s` database.", @@ -469,15 +470,15 @@ private String retiredDirectSchemaAnswer(String message, SchemaMetadata schema) } // "How many views do I have?" - if (lowerMessage.matches(".*(how many|count|number of).*(views?).*")) { + if (PatternUtil.containsPattern(lowerMessage, "(how many|count|number of).*(views?)")) { long viewCount = resolveViewCount(schema); return String.format("You have **%d views** in the `%s` database.", viewCount, schemaDisplayName(schema)); } // "What's the database size?" or "How big is the database?" - if (lowerMessage.matches(".*(database|total|overall).*(size|big|large).*") || - lowerMessage.matches(".*(how (big|large)|size of).*(database).*")) { + if (PatternUtil.containsPattern(lowerMessage, "(database|total|overall).*(size|big|large)") || + PatternUtil.containsPattern(lowerMessage, "(how (big|large)|size of).*(database)")) { long sizeBytes = schema.getTotalSizeBytes() != null ? schema.getTotalSizeBytes() : 0; String formattedSize = contextAssembler.formatBytes(sizeBytes); return String.format("The total database size is **%s** (%d bytes).", @@ -506,8 +507,8 @@ private String retiredDirectSchemaAnswer(String message, SchemaMetadata schema) } // "What are my largest tables?" or "Biggest tables" or "Tables by size" - if (lowerMessage.matches(".*(largest|biggest|heaviest|top).*tables?.*") || - lowerMessage.matches(".*tables?.*(by size|sorted by size|largest|biggest).*")) { + if (PatternUtil.containsPattern(lowerMessage, "(largest|biggest|heaviest|top).*tables?") || + PatternUtil.containsPattern(lowerMessage, "tables?.*(by size|sorted by size|largest|biggest)")) { if (schema.getTables() == null || schema.getTables().isEmpty()) { return "No tables found in the database."; } @@ -551,7 +552,7 @@ private String retiredDirectSchemaAnswer(String message, SchemaMetadata schema) } // "How many indexes?" or "Index count" - if (lowerMessage.matches(".*(how many|count|number of).*(indexes?|indices).*")) { + if (PatternUtil.containsPattern(lowerMessage, "(how many|count|number of).*(indexes?|indices)")) { long indexCount = schema.getTables() != null ? schema.getTables().stream() .filter(t -> t.getIndexes() != null) @@ -563,8 +564,8 @@ private String retiredDirectSchemaAnswer(String message, SchemaMetadata schema) // "What database type?" or "Database info" (NOT version - that needs SQL) // Version questions should fall back to LLM since we don't have version in schema metadata - if ((lowerMessage.matches(".*(what|which).*(database|db).*(type|engine).*") || - lowerMessage.matches(".*(database|db).*(info|information|details).*")) && + if ((PatternUtil.containsPattern(lowerMessage, "(what|which).*(database|db).*(type|engine)") || + PatternUtil.containsPattern(lowerMessage, "(database|db).*(info|information|details)")) && !lowerMessage.contains("version")) { StringBuilder sb = new StringBuilder(); sb.append("### Database Information\n\n"); @@ -578,7 +579,7 @@ private String retiredDirectSchemaAnswer(String message, SchemaMetadata schema) } // "Show schema summary" or "Database overview" - if (lowerMessage.matches(".*(schema|database).*(summary|overview|stats|statistics).*") || + if (PatternUtil.containsPattern(lowerMessage, "(schema|database).*(summary|overview|stats|statistics)") || lowerMessage.equals("overview") || lowerMessage.equals("summary")) { StringBuilder sb = new StringBuilder(); sb.append(String.format("### Database Overview: `%s`\n\n", schemaDisplayName(schema))); @@ -660,13 +661,13 @@ private String retiredDirectSlowQueryAnswer(String message, String connectionId) String lowerMessage = actualQuestion.toLowerCase().trim(); // Detect question type - boolean isSlowQueryQuestion = lowerMessage.matches(".*(slowest|slow|worst|heaviest|most expensive)\\s+(query|queries).*") || - lowerMessage.matches(".*slow\\s+query.*"); - boolean isTopNQuestion = lowerMessage.matches(".*(top|show|list)\\s+\\d*\\s*(slow|worst|expensive).*") || - lowerMessage.matches(".*\\d+\\s+(slow|worst|expensive)\\s+(query|queries).*"); - boolean isHealthQuestion = lowerMessage.matches(".*(performance|query|database)\\s+(health|status|summary).*") || - lowerMessage.matches(".*(how.*perform|health\\s+check|health\\s+status).*"); - boolean isStatsQuestion = lowerMessage.matches(".*(slow\\s+query|performance)\\s+(stats|statistics|metrics|numbers).*"); + boolean isSlowQueryQuestion = PatternUtil.containsPattern(lowerMessage, "(slowest|slow|worst|heaviest|most expensive)\\s+(query|queries)") || + PatternUtil.containsPattern(lowerMessage, "slow\\s+query"); + boolean isTopNQuestion = PatternUtil.containsPattern(lowerMessage, "(top|show|list)\\s+\\d*\\s*(slow|worst|expensive)") || + PatternUtil.containsPattern(lowerMessage, "\\d+\\s+(slow|worst|expensive)\\s+(query|queries)"); + boolean isHealthQuestion = PatternUtil.containsPattern(lowerMessage, "(performance|query|database)\\s+(health|status|summary)") || + PatternUtil.containsPattern(lowerMessage, "(how.*perform|health\\s+check|health\\s+status)"); + boolean isStatsQuestion = PatternUtil.containsPattern(lowerMessage, "(slow\\s+query|performance)\\s+(stats|statistics|metrics|numbers)"); if (!isSlowQueryQuestion && !isTopNQuestion && !isHealthQuestion && !isStatsQuestion) { return null; @@ -883,11 +884,11 @@ private String retiredDirectIndexRecommendationAnswer(String message, String con String lowerMessage = actualQuestion.toLowerCase().trim(); // Match index recommendation questions - boolean isIndexQuestion = lowerMessage.matches(".*(index|indexes|indices).*(recommend|suggestion|missing|should|need|add|create).*") || - lowerMessage.matches(".*(recommend|suggest|missing|need).*(index|indexes|indices).*") || - lowerMessage.matches(".*(what|which).*(index|indexes|indices).*(should|need|add|create).*") || - lowerMessage.matches(".*missing\\s+index.*") || - lowerMessage.matches(".*index\\s+recommendation.*"); + boolean isIndexQuestion = PatternUtil.containsPattern(lowerMessage, "(index|indexes|indices).*(recommend|suggestion|missing|should|need|add|create)") || + PatternUtil.containsPattern(lowerMessage, "(recommend|suggest|missing|need).*(index|indexes|indices)") || + PatternUtil.containsPattern(lowerMessage, "(what|which).*(index|indexes|indices).*(should|need|add|create)") || + PatternUtil.containsPattern(lowerMessage, "missing\\s+index") || + PatternUtil.containsPattern(lowerMessage, "index\\s+recommendation"); if (!isIndexQuestion) { return null; @@ -1092,27 +1093,27 @@ private boolean isDataRetrievalQuestion(String question) { String q = question.toLowerCase(); // Ranking / top-N / bottom-N patterns - if (q.matches(".*(top|bottom|least|most|highest|lowest|worst|best|slowest|fastest)\\s+\\d+.*")) return true; - if (q.matches(".*(top|bottom|least|most|highest|lowest|worst|best)\\s+(\\d+\\s+)?(accounts?|users?|customers?|orders?|records?|rows?|queries?|tables?|sessions?|transactions?|products?|bookings?|customers?|properties?|tenants?|clients?).*")) return true; + if (PatternUtil.containsPattern(q, "(top|bottom|least|most|highest|lowest|worst|best|slowest|fastest)\\s+\\d+")) return true; + if (PatternUtil.containsPattern(q, "(top|bottom|least|most|highest|lowest|worst|best)\\s+(\\d+\\s+)?(accounts?|users?|customers?|orders?|records?|rows?|queries?|tables?|sessions?|transactions?|products?|bookings?|customers?|properties?|tenants?|clients?)")) return true; // "Show me / list / give me / find" entity requests - if (q.matches(".*(show me|list|give me|find|fetch|retrieve|get me|display|return)\\s+.*\\b(accounts?|users?|customers?|orders?|records?|rows?|entries?|data|results?|transactions?|bookings?|customers?|properties?).*")) return true; + if (PatternUtil.containsPattern(q, "(show me|list|give me|find|fetch|retrieve|get me|display|return)\\s+.*\\b(accounts?|users?|customers?|orders?|records?|rows?|entries?|data|results?|transactions?|bookings?|customers?|properties?)")) return true; // "Which X are / have / do" questions — expect data rows as answer - if (q.matches(".*which\\s+\\w+\\s+(are|have|do|did|has|were|is).*")) return true; + if (PatternUtil.containsPattern(q, "which\\s+\\w+\\s+(are|have|do|did|has|were|is)")) return true; // Row/record count questions (business data, not schema metadata) - if (q.matches(".*(how many|count of|number of)\\s+.*(rows?|records?|accounts?|users?|customers?|orders?|bookings?|sessions?|transactions?).*")) return true; + if (PatternUtil.containsPattern(q, "(how many|count of|number of)\\s+.*(rows?|records?|accounts?|users?|customers?|orders?|bookings?|sessions?|transactions?)")) return true; // Engagement, activity, churn, usage patterns - if (q.matches(".*(least|most|zero|no|without|never|active|inactive|engaged|churned|dormant|unused)\\s+.*(accounts?|users?|customers?|sessions?|logins?|activity|usage|engagement).*")) return true; + if (PatternUtil.containsPattern(q, "(least|most|zero|no|without|never|active|inactive|engaged|churned|dormant|unused)\\s+.*(accounts?|users?|customers?|sessions?|logins?|activity|usage|engagement)")) return true; // Explicit data/report requests - if (q.matches(".*(report|summary|breakdown|overview|analysis)\\s+(of|on|for).*\\b(last|past|since|in the).*\\b(days?|weeks?|months?|years?).*")) return true; + if (PatternUtil.containsPattern(q, "(report|summary|breakdown|overview|analysis)\\s+(of|on|for).*\\b(last|past|since|in the).*\\b(days?|weeks?|months?|years?)")) return true; // Time-bounded data questions - if (q.matches(".*in the (last|past)\\s+\\d+\\s+(days?|weeks?|months?).*") && - q.matches(".*(accounts?|users?|customers?|orders?|bookings?|queries?|transactions?|sessions?).*")) return true; + if (PatternUtil.containsPattern(q, "in the (last|past)\\s+\\d+\\s+(days?|weeks?|months?)") && + PatternUtil.containsPattern(q, "(accounts?|users?|customers?|orders?|bookings?|queries?|transactions?|sessions?)")) return true; return false; } @@ -1206,7 +1207,7 @@ private String formatExactTableKeyColumnAnswer( List keyColumns, String lowerQuestion ) { - boolean countQuestion = lowerQuestion.matches(".*(how many|count|number of).*(key columns?|primary keys?|foreign keys?|join columns?).*"); + boolean countQuestion = PatternUtil.containsPattern(lowerQuestion, "(how many|count|number of).*(key columns?|primary keys?|foreign keys?|join columns?)"); if (countQuestion) { return String.format( "Table `%s` has **%d key columns**: %s.", @@ -1626,7 +1627,7 @@ private String retiredDirectKeyColumnAnswer(String message, String connectionId, .filter(this::isMeaningfulKeyColumn) .toList(); - boolean countQuestion = lowerMessage.matches(".*(how many|count|number of).*(key columns?|primary keys?|foreign keys?).*"); + boolean countQuestion = PatternUtil.containsPattern(lowerMessage, "(how many|count|number of).*(key columns?|primary keys?|foreign keys?)"); if (!meaningfulColumns.isEmpty()) { List topColumns = meaningfulColumns.stream().limit(8).toList(); long distinctTables = meaningfulColumns.stream() @@ -1882,7 +1883,7 @@ private String retiredDirectSchemaClassificationAnswer(String message, String co return "I have schema classification metadata, but no table role details are stored yet for this connection."; } - boolean largestQuestion = lowerMessage.matches(".*\\b(largest|biggest|top|heaviest)\\b.*"); + boolean largestQuestion = PatternUtil.containsPattern(lowerMessage, "\\b(largest|biggest|top|heaviest)\\b"); boolean asksPatternSummary = lowerMessage.contains("pattern") || (lowerMessage.contains("fact") && lowerMessage.contains("dimension")); if (largestQuestion) { @@ -2193,12 +2194,12 @@ private String retiredDirectWorkloadAnswer(String message, String connectionId) String lowerMessage = actualQuestion.toLowerCase().trim(); // Match workload type questions - boolean isWorkloadQuestion = lowerMessage.matches(".*(workload|work load).*(type|kind|pattern|characteristic|profile).*") || - lowerMessage.matches(".*(what|which).*(type|kind).*(workload|database|db).*") || - lowerMessage.matches(".*(is this|is it|am i running).*(oltp|olap|mixed|read|write).*") || - lowerMessage.matches(".*(oltp|olap).*(or|vs|versus).*") || - lowerMessage.matches(".*(read|write).*(heavy|intensive|dominant).*") || - lowerMessage.matches(".*workload\\s+(analysis|summary|overview).*"); + boolean isWorkloadQuestion = PatternUtil.containsPattern(lowerMessage, "(workload|work load).*(type|kind|pattern|characteristic|profile)") || + PatternUtil.containsPattern(lowerMessage, "(what|which).*(type|kind).*(workload|database|db)") || + PatternUtil.containsPattern(lowerMessage, "(is this|is it|am i running).*(oltp|olap|mixed|read|write)") || + PatternUtil.containsPattern(lowerMessage, "(oltp|olap).*(or|vs|versus)") || + PatternUtil.containsPattern(lowerMessage, "(read|write).*(heavy|intensive|dominant)") || + PatternUtil.containsPattern(lowerMessage, "workload\\s+(analysis|summary|overview)"); if (!isWorkloadQuestion) { return null; @@ -2505,29 +2506,29 @@ private List prioritizeTrainingDataByIntent( */ private boolean hasScopedOrTemporalQualifiers(String lowerMessage) { // Schema qualifiers - if (lowerMessage.matches(".*\\b(in schema|in the .* schema|schema\\s+\\w+)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(in schema|in the .* schema|schema\\s+\\w+)\\b")) { return true; } // Table-scoped qualifiers (e.g., "indexes on users", "columns in orders", "for table X") - if (lowerMessage.matches(".*\\b(on|in|for|of)\\s+(the\\s+)?\\w+\\s*(table)?\\b.*") && + if (PatternUtil.containsPattern(lowerMessage, "\\b(on|in|for|of)\\s+(the\\s+)?\\w+\\s*(table)?\\b") && (lowerMessage.contains("index") || lowerMessage.contains("column") || lowerMessage.contains("constraint") || lowerMessage.contains("foreign key"))) { return true; } // Temporal qualifiers - if (lowerMessage.matches(".*\\b(today|yesterday|last week|last month|this week|this month|since|after|before|created|added|modified|updated|recent|new)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(today|yesterday|last week|last month|this week|this month|since|after|before|created|added|modified|updated|recent|new)\\b")) { return true; } // Conditional qualifiers - if (lowerMessage.matches(".*\\b(where|with|that have|that are|containing|larger than|smaller than|more than|less than|greater|empty|non-empty)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(where|with|that have|that are|containing|larger than|smaller than|more than|less than|greater|empty|non-empty)\\b")) { return true; } // Specific object references (e.g., "tables like X", "tables starting with") - if (lowerMessage.matches(".*\\b(like|starting with|ending with|matching|named|called)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(like|starting with|ending with|matching|named|called)\\b")) { return true; } @@ -2553,10 +2554,10 @@ private boolean isSimpleSchemaQuestion(String message) { return false; } - return lower.matches(".*(how many|count|number of).*(tables?|views?).*") || + return PatternUtil.containsPattern(lower, "(how many|count|number of).*(tables?|views?)") || lower.matches("^(list|show|what are).*tables?$") || lower.equals("tables") || lower.equals("show tables") || - lower.matches(".*(database|total).*(size|big).*"); + PatternUtil.containsPattern(lower, "(database|total).*(size|big)"); } public ChatResponse processMessage(String connectionId, String message, String threadId) { diff --git a/backend/src/main/java/com/dbaagent/service/CompanyKnowledgeService.java b/backend/src/main/java/com/dbaagent/service/CompanyKnowledgeService.java index f034bc7..e9b0e50 100644 --- a/backend/src/main/java/com/dbaagent/service/CompanyKnowledgeService.java +++ b/backend/src/main/java/com/dbaagent/service/CompanyKnowledgeService.java @@ -570,7 +570,7 @@ private String sanitizeAnnotationToken(String token) { if (!notBlank(token)) { return ""; } - return token.trim().replaceAll("[,.;:]+$", ""); + return token.trim().replaceAll("[,.;:]++$", ""); } public String canonicalColumnReference(String reference) { diff --git a/backend/src/main/java/com/dbaagent/service/ConversationContextService.java b/backend/src/main/java/com/dbaagent/service/ConversationContextService.java index 9358cad..9ca6753 100644 --- a/backend/src/main/java/com/dbaagent/service/ConversationContextService.java +++ b/backend/src/main/java/com/dbaagent/service/ConversationContextService.java @@ -8,6 +8,7 @@ import com.dbaagent.repository.ChatTurnContextRepository; import com.dbaagent.service.agent.AgentExecutionContext; import com.dbaagent.service.agent.AgentRunService; +import com.dbaagent.util.PatternUtil; import com.dbaagent.util.QueryNormalizer; import com.dbaagent.util.PromptIntentSignals; import com.fasterxml.jackson.core.type.TypeReference; @@ -1031,7 +1032,7 @@ private boolean looksLikePriorQueryReference(String question) { || normalized.contains("first one") || normalized.contains("second one") || normalized.contains("third one") - || normalized.matches(".*\\bshow\\b.*\\b(query|sql)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\bshow\\b.*\\b(query|sql)\\b"); } private boolean looksLikeClarificationAnswer(String question, ResolvedConversationContext context) { diff --git a/backend/src/main/java/com/dbaagent/service/PostgresSlowLogPatterns.java b/backend/src/main/java/com/dbaagent/service/PostgresSlowLogPatterns.java index 2d095bc..c4401a4 100644 --- a/backend/src/main/java/com/dbaagent/service/PostgresSlowLogPatterns.java +++ b/backend/src/main/java/com/dbaagent/service/PostgresSlowLogPatterns.java @@ -16,15 +16,15 @@ private PostgresSlowLogPatterns() {} /** Duration header with inline statement/execute/parse/bind SQL (group 3 = SQL). */ static final Pattern LOG_STATEMENT = Pattern.compile( - "(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?).*?LOG:\\s+duration: ([\\d.]+) ms\\s+(?:statement|execute \\S*|parse \\S*|bind \\S*):\\s*(.*)"); + "(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?)[^\\n]{0,200}?LOG:\\s+duration: ([\\d.]+) ms\\s+(?:statement|execute \\S*|parse \\S*|bind \\S*):\\s*(.*)"); /** auto_explain header: "duration: N ms plan:" (SQL on following Query Text line). */ static final Pattern PLAN_HEADER = Pattern.compile( - "(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?).*?LOG:\\s+duration: ([\\d.]+) ms\\s+plan:\\s*$"); + "(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?)[^\\n]{0,200}?LOG:\\s+duration: ([\\d.]+) ms\\s+plan:\\s*$"); /** Duration-only header (statement keyword on the next line). */ static final Pattern DURATION_ONLY = Pattern.compile( - "(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?).*?LOG:\\s+duration: ([\\d.]+) ms$"); + "(\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}(?:\\.\\d+)?)[^\\n]{0,200}?LOG:\\s+duration: ([\\d.]+) ms$"); /** True if the line starts a new (timestamped) log entry. */ static boolean isNewEntry(String line) { diff --git a/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java b/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java index d330e6a..eb063b3 100644 --- a/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java +++ b/backend/src/main/java/com/dbaagent/service/QueryExecutionPolicyService.java @@ -461,9 +461,14 @@ private String stripLeadingComments(String sql) { } private String stripQuotedLiterals(String sql) { + // The alternations are ordered so no input has two parses: '' must be + // tried before the single-char branch, and the char class excludes both + // quote and backslash. Without that the ('' | [^'\\])* form is ambiguous + // and backtracks exponentially (java/polynomial-redos) on a long + // run of quotes — reachable from Editor SQL, so this is on a hot path. return sql - .replaceAll("'([^'\\\\]|\\\\.|'')*'", "''") - .replaceAll("\"([^\"\\\\]|\\\\.)*\"", "\"\""); + .replaceAll("'(?:''|\\\\.|[^'\\\\])*+'", "''") + .replaceAll("\"(?:\\\\.|[^\"\\\\])*+\"", "\"\""); } private boolean containsWhereClause(String sql) { diff --git a/backend/src/main/java/com/dbaagent/service/QueryOptimizationService.java b/backend/src/main/java/com/dbaagent/service/QueryOptimizationService.java index 9d9bf5f..657ddc4 100644 --- a/backend/src/main/java/com/dbaagent/service/QueryOptimizationService.java +++ b/backend/src/main/java/com/dbaagent/service/QueryOptimizationService.java @@ -16,6 +16,7 @@ import com.dbaagent.model.brain.ColumnStatistics; import com.dbaagent.model.KeyColumnAnalysis; import com.dbaagent.service.optd.OptdOptimizationService; +import com.dbaagent.util.PatternUtil; import com.dbaagent.util.QueryNormalizer; import static com.dbaagent.service.QueryFingerprintService.computeCanonicalFingerprint; import com.fasterxml.jackson.core.JsonProcessingException; @@ -2301,8 +2302,8 @@ private boolean isValidSuggestion(OptimizationSuggestion s) { String desc = s.getDescription() != null ? s.getDescription().toLowerCase().trim() : ""; // Skip metadata titles (estimated impact, improvement percentages) - if (title.matches(".*estimated\\s+(impact|improvement|performance).*")) return false; - if (title.matches(".*overall\\s+estimated.*")) return false; + if (PatternUtil.containsPattern(title, "estimated\\s+(impact|improvement|performance)")) return false; + if (PatternUtil.containsPattern(title, "overall\\s+estimated")) return false; if (title.matches("^\\d+[\\-–]\\d+\\s*%.*")) return false; // Improvement percentage as title ("80% overall performance improvement", "65% faster") if (title.matches("^\\d+%\\s+(overall|performance|improvement|faster|better|reduction).*")) return false; @@ -2957,7 +2958,7 @@ public List optimizeQueries(String connectionId, List metadata(String keyOne, Object valueOne, String keyT private boolean looksLikePerformanceActionPrompt(String normalized, PromptIntent promptIntent) { return normalized.contains("roi") || normalized.contains("top performance actions") - || normalized.matches(".*\\b(top|best|highest)\\b.*\\b(actions?|recommendations?)\\b.*\\b(roi|impact|value)\\b.*") - || normalized.matches(".*\\b(actions?|recommendations?)\\b.*\\b(take|apply|prioritize)\\b.*\\b(now|first|right now)\\b.*") - || normalized.matches(".*\\b(actions?|recommendations?)\\b.*\\b(performance|latency|slow query|bottleneck)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(top|best|highest)\\b.*\\b(actions?|recommendations?)\\b.*\\b(roi|impact|value)\\b") + || PatternUtil.containsPattern(normalized, "\\b(actions?|recommendations?)\\b.*\\b(take|apply|prioritize)\\b.*\\b(now|first|right now)\\b") + || PatternUtil.containsPattern(normalized, "\\b(actions?|recommendations?)\\b.*\\b(performance|latency|slow query|bottleneck)\\b"); } private boolean matchesRequestedTableScope(String tableName, String normalizedQuestion) { @@ -2622,10 +2623,10 @@ private String normalizeIdentifier(String value) { private boolean looksLikeColumnImpactPrompt(String normalized, PromptIntent promptIntent) { boolean columnSignal = promptIntent.subjectTypes().contains(PromptIntent.SubjectType.COLUMN) - || normalized.matches(".*\\b(columns?|fields?)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(columns?|fields?)\\b"); boolean queryPerformanceSignal = promptIntent.subjectTypes().contains(PromptIntent.SubjectType.QUERY) - || normalized.matches(".*\\b(query|queries|performance|latency|slow|slowness|bottleneck|impact|impacting|causing)\\b.*"); - boolean notSchemaCatalog = !normalized.matches(".*\\b(what columns|list columns|show columns|columns are in)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(query|queries|performance|latency|slow|slowness|bottleneck|impact|impacting|causing)\\b"); + boolean notSchemaCatalog = !PatternUtil.containsPattern(normalized, "\\b(what columns|list columns|show columns|columns are in)\\b"); return columnSignal && queryPerformanceSignal && notSchemaCatalog && !looksLikeCardinalityPrompt(normalized); } @@ -2635,8 +2636,8 @@ private boolean looksLikeIndexRecommendationPrompt(String normalized, PromptInte || normalized.contains("indexes") || normalized.contains("indices") || normalized.contains("indexing") - || normalized.matches(".*\\b(columns?|tables?)\\b.*\\b(need|needs|should|urgent|urgently|required|missing)\\b.*\\bindex.*") - || normalized.matches(".*\\bindex.*\\b(need|needs|should|recommend|urgent|urgently|required|missing|candidate)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(columns?|tables?)\\b.*\\b(need|needs|should|urgent|urgently|required|missing)\\b.*\\bindex") + || PatternUtil.containsPattern(normalized, "\\bindex.*\\b(need|needs|should|recommend|urgent|urgently|required|missing|candidate)\\b"); } private boolean prefersWorkloadRankedIndexActions(String normalized) { @@ -2656,10 +2657,10 @@ private boolean prefersWorkloadRankedIndexActions(String normalized) { } private boolean looksLikePerformanceChangePrompt(String normalized, PromptIntent promptIntent) { - return normalized.matches(".*\\bwhat changed\\b.*\\b(performance|database)\\b.*") - || normalized.matches(".*\\b(performance|database)\\b.*\\b(last|past)\\b.*\\b(hours?|days?)\\b.*") - || normalized.matches(".*\\b(performance|health)\\b.*\\b(summary|status|trend|spike|spikes)\\b.*") - || normalized.matches(".*\\b(change|changes|changed|delta|trend|trending)\\b.*\\b(performance|latency|cpu|connections?|queries?)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\bwhat changed\\b.*\\b(performance|database)\\b") + || PatternUtil.containsPattern(normalized, "\\b(performance|database)\\b.*\\b(last|past)\\b.*\\b(hours?|days?)\\b") + || PatternUtil.containsPattern(normalized, "\\b(performance|health)\\b.*\\b(summary|status|trend|spike|spikes)\\b") + || PatternUtil.containsPattern(normalized, "\\b(change|changes|changed|delta|trend|trending)\\b.*\\b(performance|latency|cpu|connections?|queries?)\\b"); } private boolean looksLikeRegressionPrompt(String normalized) { @@ -2672,40 +2673,40 @@ private boolean looksLikeRegressionPrompt(String normalized) { private boolean looksLikeTuningPrompt(String normalized, PromptIntent promptIntent) { return promptIntent.subjectTypes().contains(PromptIntent.SubjectType.TUNING) - || normalized.matches(".*\\b(config|configuration|knob|knobs|setting|settings|buffer pool|shared buffers?)\\b.*") - || normalized.matches(".*\\b(reduc(e|ing)|lower|improv(e|ing))\\b.*\\b(latency|p99|response time)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(config|configuration|knob|knobs|setting|settings|buffer pool|shared buffers?)\\b") + || PatternUtil.containsPattern(normalized, "\\b(reduc(e|ing)|lower|improv(e|ing))\\b.*\\b(latency|p99|response time)\\b"); } private boolean looksLikeWorkloadPrompt(String normalized, PromptIntent promptIntent) { return promptIntent.subjectTypes().contains(PromptIntent.SubjectType.WORKLOAD) - || normalized.matches(".*\\b(oltp|olap|mixed workload|mixed|workload type|workload profile|read-heavy|write-heavy)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(oltp|olap|mixed workload|mixed|workload type|workload profile|read-heavy|write-heavy)\\b"); } private boolean looksLikeCardinalityPrompt(String normalized) { - return normalized.matches(".*\\b(cardinality|statistics|selectivity|estimated rows|actual rows|plan quality|plan cost)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(cardinality|statistics|selectivity|estimated rows|actual rows|plan quality|plan cost)\\b"); } private boolean looksLikeActiveQueryPrompt(String normalized) { - return normalized.matches(".*\\b(active queries|active query|queries)\\b.*\\b(pressure|waiting|wait|blocked|blocking)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(active queries|active query|queries)\\b.*\\b(pressure|waiting|wait|blocked|blocking)\\b"); } private boolean looksLikeHotTablePrompt(String normalized) { - return normalized.matches(".*\\b(hot|hottest|busy|busiest)\\b.*\\b(table|tables)\\b.*") - || normalized.matches(".*\\b(table|tables)\\b.*\\b(used|usage|pressure)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(hot|hottest|busy|busiest)\\b.*\\b(table|tables)\\b") + || PatternUtil.containsPattern(normalized, "\\b(table|tables)\\b.*\\b(used|usage|pressure)\\b"); } private boolean looksLikeGrowthRiskPrompt(String normalized, PromptIntent promptIntent) { return promptIntent.subjectTypes().contains(PromptIntent.SubjectType.GROWTH) - || normalized.matches(".*\\b(growth|capacity|risk|run out|exhaust|forecast|bloat)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(growth|capacity|risk|run out|exhaust|forecast|bloat)\\b"); } private boolean looksLikeSlowQueryPrompt(String normalized, PromptIntent promptIntent) { if (promptIntent.subjectTypes().contains(PromptIntent.SubjectType.TUNING) || promptIntent.subjectTypes().contains(PromptIntent.SubjectType.WORKLOAD)) { - return normalized.matches(".*\\b(slow query|slow queries|slowest|query health)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(slow query|slow queries|slowest|query health)\\b"); } - return normalized.matches(".*\\b(slow query|slow queries|slowest|performance health|query health|bottleneck)\\b.*") - || normalized.matches(".*\\b(query|queries)\\b.*\\b(latency|slow|bottleneck|wait)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(slow query|slow queries|slowest|performance health|query health|bottleneck)\\b") + || PatternUtil.containsPattern(normalized, "\\b(query|queries)\\b.*\\b(latency|slow|bottleneck|wait)\\b"); } private int extractMonitoringWindowHours(String normalized) { diff --git a/backend/src/main/java/com/dbaagent/service/agent/PromptIntentAnalyzer.java b/backend/src/main/java/com/dbaagent/service/agent/PromptIntentAnalyzer.java index d73b283..d95ef21 100644 --- a/backend/src/main/java/com/dbaagent/service/agent/PromptIntentAnalyzer.java +++ b/backend/src/main/java/com/dbaagent/service/agent/PromptIntentAnalyzer.java @@ -1,5 +1,6 @@ package com.dbaagent.service.agent; +import com.dbaagent.util.PatternUtil; import com.dbaagent.service.ChatQuestionRoutingService; import com.dbaagent.service.ResolvedConversationContext; import org.springframework.stereotype.Service; @@ -131,8 +132,8 @@ private boolean looksLikePriorQueryDisplayFollowUp( if (!(normalized.contains("full query") || normalized.contains("full sql") || normalized.contains("query text") - || normalized.matches(".*\\bshow\\b.*\\b(query|sql)\\b.*") - || normalized.matches(".*\\bwhat\\b.*\\bquery\\b.*"))) { + || PatternUtil.containsPattern(normalized, "\\bshow\\b.*\\b(query|sql)\\b") + || PatternUtil.containsPattern(normalized, "\\bwhat\\b.*\\bquery\\b"))) { return false; } if (resolvedConversationContext.sourceSql() != null && !resolvedConversationContext.sourceSql().isBlank()) { @@ -149,7 +150,7 @@ private boolean looksLikePriorQueryDisplayFollowUp( return resolvedConversationContext.conversationHistory().stream() .filter(turn -> turn != null && "assistant".equalsIgnoreCase(turn.role())) .map(turn -> lower(turn.content())) - .anyMatch(content -> content.contains("```sql") || content.matches(".*\\bselect\\b.*\\bfrom\\b.*")); + .anyMatch(content -> content.contains("```sql") || PatternUtil.containsPattern(content, "\\bselect\\b.*\\bfrom\\b")); } private boolean looksLikePriorQueryDiagnosticFollowUp(String normalized) { @@ -305,7 +306,7 @@ private PromptIntent.TaskType detectTaskType( } private boolean looksLikeFollowUp(String normalized) { - return normalized.matches(".*\\b(these|those|same|that|it|them|above|returned)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(these|those|same|that|it|them|above|returned)\\b"); } private boolean looksLikeRecommendationPrompt(String normalized) { @@ -331,36 +332,36 @@ private boolean looksLikeIndexRecommendationPrompt(String normalized) { || normalized.contains("missing indexes") || normalized.contains("unused index") || normalized.contains("duplicate index") - || normalized.matches(".*\\b(which|what)\\b.*\\b(columns?|fields?|tables?)\\b.*\\b(index|indexes|indices|indexing|indexed)\\b.*") - || normalized.matches(".*\\b(index|indexes|indices|indexing|indexed)\\b.*\\b(need|needs|should|recommend|required|missing|urgent|urgently|candidate|prioritize|priority)\\b.*") - || normalized.matches(".*\\b(need|needs|should|recommend|required|missing|urgent|urgently|candidate|prioritize|priority)\\b.*\\b(index|indexes|indices|indexing|indexed)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(which|what)\\b.*\\b(columns?|fields?|tables?)\\b.*\\b(index|indexes|indices|indexing|indexed)\\b") + || PatternUtil.containsPattern(normalized, "\\b(index|indexes|indices|indexing|indexed)\\b.*\\b(need|needs|should|recommend|required|missing|urgent|urgently|candidate|prioritize|priority)\\b") + || PatternUtil.containsPattern(normalized, "\\b(need|needs|should|recommend|required|missing|urgent|urgently|candidate|prioritize|priority)\\b.*\\b(index|indexes|indices|indexing|indexed)\\b"); } private boolean looksLikeIndexWorkloadRecommendationPrompt(String normalized) { - return normalized.matches(".*\\b(need|needs|should|recommend|required|missing|urgent|urgently|candidate|prioritize|priority)\\b.*") + return PatternUtil.containsPattern(normalized, "\\b(need|needs|should|recommend|required|missing|urgent|urgently|candidate|prioritize|priority)\\b") || normalized.contains("current workload") || normalized.contains("workload"); } private boolean looksLikePerformancePrompt(String normalized) { - return normalized.matches(".*\\b(slow query|slow queries|latency|bottleneck|regress|regression|regressions|workload|tuning|health|execution plan|plan quality|performance|pressure|waiting|wait event|wait events|active queries|active query|hot|hottest|usage|used|config knobs?|cardinality|statistics|growth|capacity|risk|roi|cost benefit|performance actions?|fix suggestions?)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(slow query|slow queries|latency|bottleneck|regress|regression|regressions|workload|tuning|health|execution plan|plan quality|performance|pressure|waiting|wait event|wait events|active queries|active query|hot|hottest|usage|used|config knobs?|cardinality|statistics|growth|capacity|risk|roi|cost benefit|performance actions?|fix suggestions?)\\b"); } private boolean looksLikeColumnImpactPrompt(String normalized, Set subjectTypes) { boolean columnSignal = subjectTypes.contains(PromptIntent.SubjectType.COLUMN) - || normalized.matches(".*\\b(columns?|fields?)\\b.*"); - boolean impactSignal = normalized.matches(".*\\b(impact|impactful|impacting|important|critical|hot|hottest|used|usage|pressure|bottleneck)\\b.*"); - boolean schemaCatalogSignal = normalized.matches(".*\\b(what columns|list columns|show columns|columns are in|has columns)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\b(columns?|fields?)\\b"); + boolean impactSignal = PatternUtil.containsPattern(normalized, "\\b(impact|impactful|impacting|important|critical|hot|hottest|used|usage|pressure|bottleneck)\\b"); + boolean schemaCatalogSignal = PatternUtil.containsPattern(normalized, "\\b(what columns|list columns|show columns|columns are in|has columns)\\b"); return columnSignal && impactSignal && !schemaCatalogSignal; } private boolean looksLikeBiPrompt(String normalized) { - return normalized.matches(".*\\b(revenue|sales|gmv|arr|mrr|bookings?|orders?|customers?|payments?|transactions?|retention|churn|ltv|aov|inventory|pipeline|funnel|conversion)\\b.*") - || normalized.matches(".*\\b(show|list|get|count|how many|top|compare|trend|breakdown|summarize)\\b.*"); + return PatternUtil.containsPattern(normalized, "\\b(revenue|sales|gmv|arr|mrr|bookings?|orders?|customers?|payments?|transactions?|retention|churn|ltv|aov|inventory|pipeline|funnel|conversion)\\b") + || PatternUtil.containsPattern(normalized, "\\b(show|list|get|count|how many|top|compare|trend|breakdown|summarize)\\b"); } private boolean looksLikeSchemaPrompt(String normalized, Set subjectTypes) { - return normalized.matches(".*\\b(schema|table|tables|view|views|column|columns|fields|indexes?|structure|definition)\\b.*") + return PatternUtil.containsPattern(normalized, "\\b(schema|table|tables|view|views|column|columns|fields|indexes?|structure|definition)\\b") || subjectTypes.contains(PromptIntent.SubjectType.TABLE) || subjectTypes.contains(PromptIntent.SubjectType.COLUMN) || subjectTypes.contains(PromptIntent.SubjectType.RELATIONSHIP); diff --git a/backend/src/main/java/com/dbaagent/service/agent/SchemaMetadataExecutor.java b/backend/src/main/java/com/dbaagent/service/agent/SchemaMetadataExecutor.java index e420693..860d478 100644 --- a/backend/src/main/java/com/dbaagent/service/agent/SchemaMetadataExecutor.java +++ b/backend/src/main/java/com/dbaagent/service/agent/SchemaMetadataExecutor.java @@ -24,6 +24,7 @@ import com.dbaagent.service.SchemaQuestionUtil; import com.dbaagent.service.SchemaTableMatchUtil; import com.dbaagent.service.brain.classification.SchemaClassificationService; +import com.dbaagent.util.PatternUtil; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; import org.springframework.beans.factory.annotation.Autowired; @@ -282,9 +283,9 @@ private DraftMetadataAnswer schemaEvidence(String question, String connectionId, return null; } - boolean asksForTableCount = lowerMessage.matches(".*(how many|count|number of).*(tables?).*") && !lowerMessage.contains("rows"); - boolean asksForLargestTables = lowerMessage.matches(".*(largest|biggest|heaviest|top).*tables?.*") - || lowerMessage.matches(".*tables?.*(by size|sorted by size|largest|biggest).*"); + boolean asksForTableCount = PatternUtil.containsPattern(lowerMessage, "(how many|count|number of).*(tables?)") && !lowerMessage.contains("rows"); + boolean asksForLargestTables = PatternUtil.containsPattern(lowerMessage, "(largest|biggest|heaviest|top).*tables?") + || PatternUtil.containsPattern(lowerMessage, "tables?.*(by size|sorted by size|largest|biggest)"); if (asksForTableCount && asksForLargestTables) { long tableCount = resolveTableCount(schema); @@ -724,7 +725,7 @@ private DraftMetadataAnswer classificationEvidence(String question, String conne ); } - boolean largestQuestion = lowerMessage.matches(".*\\b(largest|biggest|top|heaviest)\\b.*"); + boolean largestQuestion = PatternUtil.containsPattern(lowerMessage, "\\b(largest|biggest|top|heaviest)\\b"); boolean patternSummaryQuestion = lowerMessage.contains("pattern") || (lowerMessage.contains("fact") && lowerMessage.contains("dimension")); if (largestQuestion) { @@ -1552,7 +1553,7 @@ private String formatExactTableIndexAnswer(TableMetadata table, String lowerQues } private String formatExactTableKeyColumnAnswer(TableMetadata table, List keyColumns, String lowerQuestion) { - boolean countQuestion = lowerQuestion.matches(".*(how many|count|number of).*(key columns?|primary keys?|foreign keys?|join columns?).*"); + boolean countQuestion = PatternUtil.containsPattern(lowerQuestion, "(how many|count|number of).*(key columns?|primary keys?|foreign keys?|join columns?)"); if (countQuestion) { return String.format( "Table `%s` has **%d key columns**: %s.", @@ -1613,20 +1614,20 @@ private String renderIndexColumns(IndexMetadata index) { } private boolean hasScopedOrTemporalQualifiers(String lowerMessage) { - if (lowerMessage.matches(".*\\b(in schema|in the .* schema|schema\\s+\\w+)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(in schema|in the .* schema|schema\\s+\\w+)\\b")) { return true; } - if (lowerMessage.matches(".*\\b(on|in|for|of)\\s+(the\\s+)?\\w+\\s*(table)?\\b.*") + if (PatternUtil.containsPattern(lowerMessage, "\\b(on|in|for|of)\\s+(the\\s+)?\\w+\\s*(table)?\\b") && (lowerMessage.contains("index") || lowerMessage.contains("column") || lowerMessage.contains("constraint") || lowerMessage.contains("foreign key"))) { return true; } - if (lowerMessage.matches(".*\\b(today|yesterday|last week|last month|this week|this month|since|after|before|created|added|modified|updated|recent|new)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(today|yesterday|last week|last month|this week|this month|since|after|before|created|added|modified|updated|recent|new)\\b")) { return true; } - if (lowerMessage.matches(".*\\b(where|with|that have|that are|containing|larger than|smaller than|more than|less than|greater|empty|non-empty)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(where|with|that have|that are|containing|larger than|smaller than|more than|less than|greater|empty|non-empty)\\b")) { return true; } - if (lowerMessage.matches(".*\\b(like|starting with|ending with|matching|named|called)\\b.*")) { + if (PatternUtil.containsPattern(lowerMessage, "\\b(like|starting with|ending with|matching|named|called)\\b")) { return true; } return false; diff --git a/backend/src/main/java/com/dbaagent/service/agent/UniversalChatTool.java b/backend/src/main/java/com/dbaagent/service/agent/UniversalChatTool.java index b3e5c6d..6b0b390 100644 --- a/backend/src/main/java/com/dbaagent/service/agent/UniversalChatTool.java +++ b/backend/src/main/java/com/dbaagent/service/agent/UniversalChatTool.java @@ -24,6 +24,7 @@ import com.dbaagent.service.pipeline.PipelineResult; import com.dbaagent.service.pipeline.QueryGenerationPipeline; import com.dbaagent.service.pipeline.ResolvedContext; +import com.dbaagent.util.PatternUtil; import com.dbaagent.util.PromptIntentSignals; import lombok.extern.slf4j.Slf4j; import org.springframework.ai.chat.client.ChatClient; @@ -814,7 +815,7 @@ private boolean looksLikePriorQueryDisplayFollowUp(String effectiveQuestion) { return normalized.contains("full query") || normalized.contains("full sql") || normalized.contains("query text") - || normalized.matches(".*\\bshow\\b.*\\b(query|sql)\\b.*"); + || PatternUtil.containsPattern(normalized, "\\bshow\\b.*\\b(query|sql)\\b"); } private String resolvePriorQuerySql(ResolvedConversationContext resolvedConversationContext) { @@ -4214,14 +4215,14 @@ private boolean isDataRetrievalQuestion(String question) { return false; } String q = question.toLowerCase(Locale.ROOT); - if (q.matches(".*(top|bottom|least|most|highest|lowest|worst|best|slowest|fastest)\\s+\\d+.*")) return true; - if (q.matches(".*(show me|list|give me|find|fetch|retrieve|get me|display|return)\\s+.*\\b(accounts?|users?|customers?|orders?|records?|rows?|entries?|data|results?|transactions?|bookings?|customers?|properties?).*")) return true; - if (q.matches(".*which\\s+\\w+\\s+(are|have|do|did|has|were|is).*")) return true; - if (q.matches(".*(how many|count of|number of)\\s+.*(rows?|records?|accounts?|users?|customers?|orders?|bookings?|sessions?|transactions?).*")) return true; - if (q.matches(".*(report|summary|breakdown|overview|analysis)\\s+(of|on|for).*\\b(last|past|since|in the).*\\b(days?|weeks?|months?|years?).*")) return true; - if (q.matches(".*in the (last|past)\\s+\\d+\\s+(days?|weeks?|months?).*") && - q.matches(".*(accounts?|users?|customers?|orders?|bookings?|queries?|transactions?|sessions?|customers?|properties?).*")) return true; - if (q.matches(".*(what|which|show|list|give me).*(fees?|taxes|refunds?|cancellations?|services?|details?|amounts?|methods?).*")) return true; + if (PatternUtil.containsPattern(q, "(top|bottom|least|most|highest|lowest|worst|best|slowest|fastest)\\s+\\d+")) return true; + if (PatternUtil.containsPattern(q, "(show me|list|give me|find|fetch|retrieve|get me|display|return)\\s+.*\\b(accounts?|users?|customers?|orders?|records?|rows?|entries?|data|results?|transactions?|bookings?|customers?|properties?)")) return true; + if (PatternUtil.containsPattern(q, "which\\s+\\w+\\s+(are|have|do|did|has|were|is)")) return true; + if (PatternUtil.containsPattern(q, "(how many|count of|number of)\\s+.*(rows?|records?|accounts?|users?|customers?|orders?|bookings?|sessions?|transactions?)")) return true; + if (PatternUtil.containsPattern(q, "(report|summary|breakdown|overview|analysis)\\s+(of|on|for).*\\b(last|past|since|in the).*\\b(days?|weeks?|months?|years?)")) return true; + if (PatternUtil.containsPattern(q, "in the (last|past)\\s+\\d+\\s+(days?|weeks?|months?)") && + PatternUtil.containsPattern(q, "(accounts?|users?|customers?|orders?|bookings?|queries?|transactions?|sessions?|customers?|properties?)")) return true; + if (PatternUtil.containsPattern(q, "(what|which|show|list|give me).*(fees?|taxes|refunds?|cancellations?|services?|details?|amounts?|methods?)")) return true; return false; } diff --git a/backend/src/main/java/com/dbaagent/service/brain/keycolumn/KeyColumnAnalysisService.java b/backend/src/main/java/com/dbaagent/service/brain/keycolumn/KeyColumnAnalysisService.java index 36519f7..f58bc44 100644 --- a/backend/src/main/java/com/dbaagent/service/brain/keycolumn/KeyColumnAnalysisService.java +++ b/backend/src/main/java/com/dbaagent/service/brain/keycolumn/KeyColumnAnalysisService.java @@ -1,5 +1,6 @@ package com.dbaagent.service.brain.keycolumn; +import com.dbaagent.util.PatternUtil; import com.dbaagent.repository.brain.BrainRuleRepository; import com.dbaagent.model.brain.BrainRule; import com.dbaagent.dto.*; @@ -1529,7 +1530,7 @@ private void classifyKeys(List analyses, String connectionId) // Check naming patterns String columnName = analysis.getColumnName().toLowerCase(); - boolean isSurrogateNaming = columnName.matches(".*(id|_id|uuid|key|_key).*"); + boolean isSurrogateNaming = PatternUtil.containsPattern(columnName, "(id|_id|uuid|key|_key)"); boolean isPrimaryKeyNaming = columnName.equals("id") || columnName.equals(analysis.getTableName().toLowerCase() + "_id"); @@ -1639,7 +1640,7 @@ private void detectPartitioningCandidates(List analyses) { Long totalRows = analysis.getTotalRows() != null ? analysis.getTotalRows() : 0L; // Strategy 1: RANGE Partitioning (Time-based columns) - boolean isTimeColumn = columnName.matches(".*(date|time|timestamp|created|updated|modified).*"); + boolean isTimeColumn = PatternUtil.containsPattern(columnName, "(date|time|timestamp|created|updated|modified)"); boolean hasTemporalQueries = analysis.getWhereCount() >= 5 || analysis.getOrderByCount() >= 3; boolean isLargeTable = totalRows > 1_000_000; diff --git a/backend/src/main/java/com/dbaagent/service/brain/query/CardinalityEstimationService.java b/backend/src/main/java/com/dbaagent/service/brain/query/CardinalityEstimationService.java index 18cb2d1..121b241 100644 --- a/backend/src/main/java/com/dbaagent/service/brain/query/CardinalityEstimationService.java +++ b/backend/src/main/java/com/dbaagent/service/brain/query/CardinalityEstimationService.java @@ -114,6 +114,13 @@ private ColumnStatistics collectColumnStatistics( String dataType) { try { + // Resolve both names against information_schema before any of the + // collect* helpers interpolate them into SQL. Every interpolated + // identifier below is therefore a value the catalog returned, not + // caller input (java/sql-injection). + String safeTable = requireKnownIdentifier(jdbc, dbType, tableName, null); + String safeColumn = requireKnownIdentifier(jdbc, dbType, tableName, columnName); + // Find or create statistics record ColumnStatistics stats = statisticsRepository .findByConnectionIdAndTableNameAndColumnName(connectionId, tableName, columnName) @@ -128,19 +135,19 @@ private ColumnStatistics collectColumnStatistics( stats.setCollectionMethod(ColumnStatistics.CollectionMethod.SAMPLE); // Collect basic statistics - collectBasicStats(jdbc, dbType, tableName, columnName, stats); + collectBasicStats(jdbc, dbType, safeTable, safeColumn, stats); // Collect MCV (Most Common Values) - collectMCVStats(jdbc, dbType, tableName, columnName, stats); + collectMCVStats(jdbc, dbType, safeTable, safeColumn, stats); // Collect histogram for numeric columns if (isNumericType(dataType)) { - collectHistogramStats(jdbc, dbType, tableName, columnName, stats); + collectHistogramStats(jdbc, dbType, safeTable, safeColumn, stats); } // Collect string length stats for text columns if (isTextType(dataType)) { - collectStringStats(jdbc, dbType, tableName, columnName, stats); + collectStringStats(jdbc, dbType, safeTable, safeColumn, stats); } stats.setCollectedAt(LocalDateTime.now()); @@ -498,12 +505,39 @@ private String getColumnDataType(JdbcTemplate jdbc, String dbType, String tableN } } + // Delegates to the dialect's SamplingProvider: it doubles an embedded quote + // character, which this method previously did not do at all — a table named + // `x" ; DROP TABLE users; --` escaped the quoting entirely (java/sql-injection). + // The if/else on dbType it replaced also violated the provider-registry rule. private String quoteIdentifier(String dbType, String identifier) { - if ("postgres".equals(dbType)) { - return "\"" + identifier + "\""; - } else { - return "`" + identifier + "`"; + return providerRegistry.getDialect(dbType).sampling().quoteIdentifier(identifier); + } + + /** + * Resolves an identifier to the spelling recorded in information_schema. + * Quoting alone makes injection inert; this makes the value non-arbitrary, + * so a name that is not a real table/column never reaches interpolated SQL. + */ + private String requireKnownIdentifier(JdbcTemplate jdbc, String dbType, + String tableName, String columnName) { + boolean isColumn = columnName != null; + String sql = isColumn + ? "SELECT column_name FROM information_schema.columns " + + "WHERE table_name = ? AND column_name = ? LIMIT 1" + : "SELECT table_name FROM information_schema.tables WHERE table_name = ? LIMIT 1"; + + boolean lower = "postgres".equals(dbType); + List found = isColumn + ? jdbc.queryForList(sql, String.class, + lower ? tableName.toLowerCase() : tableName, + lower ? columnName.toLowerCase() : columnName) + : jdbc.queryForList(sql, String.class, lower ? tableName.toLowerCase() : tableName); + + if (found.isEmpty()) { + throw new IllegalArgumentException( + "Unknown " + (isColumn ? "column " + tableName + "." + columnName : "table " + tableName)); } + return found.get(0); } private boolean isNumericType(String dataType) { diff --git a/backend/src/main/java/com/dbaagent/service/brain/query/PlanPatternLibraryService.java b/backend/src/main/java/com/dbaagent/service/brain/query/PlanPatternLibraryService.java index d60cb18..3912eb1 100644 --- a/backend/src/main/java/com/dbaagent/service/brain/query/PlanPatternLibraryService.java +++ b/backend/src/main/java/com/dbaagent/service/brain/query/PlanPatternLibraryService.java @@ -552,11 +552,13 @@ private String extractQuerySignature(String normalizedQuery) { // Extract key parts of the query for pattern matching String signature = normalizedQuery; - // Remove column lists - signature = signature.replaceAll("select\\s+[^from]+\\s+from", "select ... from"); + // Remove column lists. `[^from]+` was a character class ("not f/r/o/m"), + // so any column containing one of those letters (order_id, from_date) + // defeated it; a bounded lazy scan to the FROM keyword is what was meant. + signature = signature.replaceAll("select\\s++[^\\n]{0,500}?\\s++from", "select ... from"); // Simplify WHERE clause - signature = signature.replaceAll("where\\s+.+?(\\s+order|\\s+group|\\s+limit|$)", "where ... $1"); + signature = signature.replaceAll("where\\s++[^\\n]{0,500}?(\\s++order|\\s++group|\\s++limit|$)", "where ... $1"); return signature.substring(0, Math.min(100, signature.length())); } diff --git a/backend/src/main/java/com/dbaagent/service/optd/OptdOptimizationService.java b/backend/src/main/java/com/dbaagent/service/optd/OptdOptimizationService.java index cd00856..b404dbc 100644 --- a/backend/src/main/java/com/dbaagent/service/optd/OptdOptimizationService.java +++ b/backend/src/main/java/com/dbaagent/service/optd/OptdOptimizationService.java @@ -356,12 +356,12 @@ private String replaceDerivedTablePlaceholders(String sql) { replaced = replaced.replaceAll("(?i)\\b(from|join)\\s+\\$\\d+", "$1 " + placeholder); replaced = replaced.replaceAll("(?i)\\b(from|join)\\s*\\(\\s*\\d+\\s*\\)", "$1 " + placeholder); replaced = replaced.replaceAll("(?i)\\b(from|join)\\s+\\d+", "$1 " + placeholder); - java.util.regex.Pattern paren = java.util.regex.Pattern.compile("(?i)\\b(from|join)\\s*\\(([^)]*)\\)"); + java.util.regex.Pattern paren = java.util.regex.Pattern.compile("(?i)\\b(from|join)\\s*+\\(([^)]*+)\\)"); java.util.regex.Matcher matcher = paren.matcher(replaced); StringBuffer buffer = new StringBuffer(); while (matcher.find()) { String inner = matcher.group(2); - if (inner != null && inner.matches("\\s*[\\?\\$\\d\\s]+")) { + if (inner != null && inner.matches("[\\?\\$\\d\\s]++")) { matcher.appendReplacement(buffer, matcher.group(1) + " " + placeholder); } } @@ -439,7 +439,7 @@ private String stripMySqlHints(String sql) { result = result.replaceAll("(?i)\\bSTRAIGHT_JOIN\\b", "INNER JOIN"); // FORCE/USE/IGNORE INDEX/KEY [FOR JOIN|ORDER BY|GROUP BY] (index_list) result = result.replaceAll( - "(?i)\\b(FORCE|USE|IGNORE)\\s+(INDEX|KEY)\\s+(FOR\\s+(JOIN|ORDER\\s+BY|GROUP\\s+BY)\\s+)?\\([^)]*\\)", + "(?i)\\b(FORCE|USE|IGNORE)\\s++(INDEX|KEY)\\s++(FOR\\s++(JOIN|ORDER\\s++BY|GROUP\\s++BY)\\s++)?\\([^)]*+\\)", "" ); return result; diff --git a/backend/src/main/java/com/dbaagent/util/PatternUtil.java b/backend/src/main/java/com/dbaagent/util/PatternUtil.java new file mode 100644 index 0000000..36f808f --- /dev/null +++ b/backend/src/main/java/com/dbaagent/util/PatternUtil.java @@ -0,0 +1,43 @@ +package com.dbaagent.util; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; + +/** + * Unanchored regex matching over a cached pattern. + * + * Replaces {@code s.matches(".*RE.*")}. String.matches anchors the whole input, + * so the surrounding {@code .*} only exists to undo that anchoring — and the + * combination of those wrappers with an alternation is what drives the + * polynomial backtracking CodeQL reports as java/polynomial-redos. find() on + * the bare expression is equivalent and linear. + */ +public final class PatternUtil { + + private PatternUtil() {} + + private static final int MAX_CACHED_PATTERNS = 512; + private static final Map CACHE = new ConcurrentHashMap<>(); + + public static boolean containsPattern(String input, String regex) { + if (input == null) { + return false; + } + return cached(regex).matcher(input).find(); + } + + private static Pattern cached(String regex) { + Pattern p = CACHE.get(regex); + if (p != null) { + return p; + } + Pattern compiled = Pattern.compile(regex); + // Callers pass compile-time literals, so the key set is bounded in practice. + // The cap only stops an unforeseen dynamic caller from growing this without limit. + if (CACHE.size() < MAX_CACHED_PATTERNS) { + CACHE.putIfAbsent(regex, compiled); + } + return compiled; + } +} diff --git a/backend/src/main/java/com/dbaagent/util/PromptIntentSignals.java b/backend/src/main/java/com/dbaagent/util/PromptIntentSignals.java index 3f385f9..988d04d 100644 --- a/backend/src/main/java/com/dbaagent/util/PromptIntentSignals.java +++ b/backend/src/main/java/com/dbaagent/util/PromptIntentSignals.java @@ -101,9 +101,9 @@ public static boolean hasExplicitTimeWindow(String question) { if (normalized.isBlank()) { return false; } - return normalized.matches(".*\\b(last|past|previous|current|this|today|yesterday|tomorrow)\\b.*") - || normalized.matches(".*\\b\\d+\\s+(day|days|week|weeks|month|months|year|years|hour|hours)\\b.*") - || normalized.matches(".*\\b(january|february|march|april|may|june|july|august|september|october|november|december)\\b.*") + return PatternUtil.containsPattern(normalized, "\\b(last|past|previous|current|this|today|yesterday|tomorrow)\\b") + || PatternUtil.containsPattern(normalized, "\\b\\d+\\s+(day|days|week|weeks|month|months|year|years|hour|hours)\\b") + || PatternUtil.containsPattern(normalized, "\\b(january|february|march|april|may|june|july|august|september|october|november|december)\\b") || normalized.contains(" between ") || normalized.contains(" from ") || normalized.contains(" since ") diff --git a/backend/src/main/java/com/dbaagent/util/QueryNormalizer.java b/backend/src/main/java/com/dbaagent/util/QueryNormalizer.java index ae6e00a..3df46d6 100644 --- a/backend/src/main/java/com/dbaagent/util/QueryNormalizer.java +++ b/backend/src/main/java/com/dbaagent/util/QueryNormalizer.java @@ -27,8 +27,11 @@ public class QueryNormalizer { // this rule a Postgres literal "= true" never matches the digest's "= ?". private static final Pattern BOOLEAN_PATTERN = Pattern.compile("\\b(?:true|false)\\b", Pattern.CASE_INSENSITIVE); private static final Pattern WHITESPACE_PATTERN = Pattern.compile("\\s+"); - private static final Pattern IN_LIST_PATTERN = Pattern.compile("IN\\s*\\([^)]+\\)", Pattern.CASE_INSENSITIVE); - private static final Pattern VALUES_PATTERN = Pattern.compile("VALUES\\s*\\([^)]+\\)", Pattern.CASE_INSENSITIVE); + // Possessive \s*+ : without it the engine can split the run of whitespace + // between IN and "(" many ways on a non-matching line, which is the + // backtracking CodeQL flags (java/polynomial-redos). Same language matched. + private static final Pattern IN_LIST_PATTERN = Pattern.compile("IN\\s*+\\([^)]++\\)", Pattern.CASE_INSENSITIVE); + private static final Pattern VALUES_PATTERN = Pattern.compile("VALUES\\s*+\\([^)]++\\)", Pattern.CASE_INSENSITIVE); // DML/DDL keywords that indicate the start of the actual query private static final String[] DML_KEYWORDS = { diff --git a/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java b/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java new file mode 100644 index 0000000..377758d --- /dev/null +++ b/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java @@ -0,0 +1,55 @@ +package com.dbaagent.util; + +import org.junit.jupiter.api.Test; + +import java.util.regex.Pattern; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class PatternUtilTest { + + @Test + void matchesTheSameInputsAsTheAnchoredFormItReplaced() { + String[] regexes = { + "\\b(slow query|slowest)\\b", + "\\b(how many|count|number of)\\b.*\\bcolumns?\\b", + "(?:sql|query example|write sql)" + }; + String[] inputs = { + "", "show me the slowest query", "how many columns are in orders", + "unrelated text", "write sql for me" + }; + + for (String regex : regexes) { + Pattern anchored = Pattern.compile(".*" + regex + ".*"); + for (String input : inputs) { + assertEquals(anchored.matcher(input).matches(), + PatternUtil.containsPattern(input, regex), + "regex=" + regex + " input=" + input); + } + } + } + + @Test + void nullInputIsFalseRatherThanThrowing() { + assertFalse(PatternUtil.containsPattern(null, "\\bx\\b")); + } + + @Test + void findsAKeywordAfterANewline() { + // The anchored ".*X.*" form missed this: `.` does not cross a newline, + // so a multi-line chat message never matched. find() is the fix. + assertTrue(PatternUtil.containsPattern("first line\nslow query here", + "\\b(slow query|slowest)\\b")); + } + + @Test + void reusesTheCompiledPatternForTheSameRegex() { + String regex = "\\bcache-me\\b"; + assertTrue(PatternUtil.containsPattern("cache-me please", regex)); + assertTrue(PatternUtil.containsPattern("cache-me again", regex)); + assertFalse(PatternUtil.containsPattern("nothing here", regex)); + } +} diff --git a/mcp/src/auth/browser-flow.js b/mcp/src/auth/browser-flow.js index b6a0088..0e1015f 100644 --- a/mcp/src/auth/browser-flow.js +++ b/mcp/src/auth/browser-flow.js @@ -72,12 +72,24 @@ function startLoopbackServer() { } function openInBrowser(url) { + // The URL arrives in a server response, so it is not ours to trust. Args are + // passed as an array (no shell), but the win32 branch goes through cmd, where + // a non-http scheme could still be read as something other than a URL. + let parsed; + try { + parsed = new URL(url); + } catch { + return; + } + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") return; + + const safeUrl = parsed.toString(); const opener = process.platform === "darwin" - ? ["open", [url]] + ? ["open", [safeUrl]] : process.platform === "win32" - ? ["cmd", ["/c", "start", '""', url]] - : ["xdg-open", [url]]; + ? ["cmd", ["/c", "start", '""', safeUrl]] + : ["xdg-open", [safeUrl]]; try { const child = spawn(opener[0], opener[1], { stdio: "ignore", detached: true }); child.on("error", () => {}); From c38471bada7147765792ba82f82d15bfb58c375d Mon Sep 17 00:00:00 2001 From: sumit Date: Tue, 18 Aug 2026 17:37:41 +0530 Subject: [PATCH 3/5] fix(security): guard the remaining two SSRF sinks (JDBC host, presigned URL) Closes the other two java/ssrf alerts. Address classification moves into a shared OutboundHostGuard -- resolve the host and check every returned address, so a public hostname whose A record points at 10.x or 169.254.169.254 is still refused. SshHostGuard now delegates to it instead of carrying its own copy. DatabaseHostGuard (#136, ConnectionService) screens the JDBC host. The SSH guard never covered this: a direct, non-tunnelled connection does not go through SshTunnelService at all. Applied in buildJdbcUrl and in the Hikari pool path, and skipped when a tunnel port is present since that targets the local forwarded port. Ships disabled, same reasoning as the SSH guard -- databases sit on RFC1918 more often than bastions do. S3LogFetchService (#137) was calling setInstanceFollowRedirects(true), so the JDK chased a 302 with no chance to inspect the target and a presigned URL on a public host could hand off to the metadata endpoint. Redirects are now followed manually with a cap of 5, and every hop is re-checked for https plus a public address. This one is always on: there is no legitimate reason to fetch a slow query log from a private address. Verified: 47 unit tests pass across the new and touched guards; the six pre-existing failing classes show the same 13 failures / 4 errors as the untouched baseline, so no new regressions. --- CLAUDE.md | 18 +++ .../dbaagent/service/ConnectionService.java | 12 ++ .../dbaagent/service/DatabaseHostGuard.java | 51 ++++++++ .../service/DatabaseHostGuardProperties.java | 25 ++++ .../dbaagent/service/OutboundHostGuard.java | 110 ++++++++++++++++++ .../dbaagent/service/S3LogFetchService.java | 69 +++++++++-- .../com/dbaagent/service/SshHostGuard.java | 98 +++------------- .../service/ConnectionServiceTest.java | 3 +- .../service/DatabaseHostGuardTest.java | 71 +++++++++++ 9 files changed, 364 insertions(+), 93 deletions(-) create mode 100644 backend/src/main/java/com/dbaagent/service/DatabaseHostGuard.java create mode 100644 backend/src/main/java/com/dbaagent/service/DatabaseHostGuardProperties.java create mode 100644 backend/src/main/java/com/dbaagent/service/OutboundHostGuard.java create mode 100644 backend/src/test/java/com/dbaagent/service/DatabaseHostGuardTest.java diff --git a/CLAUDE.md b/CLAUDE.md index f695a3e..cbf4ae1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -350,6 +350,24 @@ points are covered by that single call site (`establishTunnel` and - Operators who want the protection set `enabled=true` and allowlist their own bastion via `deepsql.ssh.host-guard.allowed-hosts` (exact host, or a leading-dot suffix like `.corp.internal`). +- **Two sibling guards cover the other two `java/ssrf` alerts.** Address + classification is shared in `OutboundHostGuard` (resolve the host, check every + returned address, block loopback/link-local/RFC1918/CGNAT/ULA/IPv4-mapped-IPv6); + the three call sites differ only in policy and message. + - `DatabaseHostGuard` (alert #136, `ConnectionService`) screens the **JDBC** host. + The SSH guard never covered this — a direct, non-tunnelled connection does not + pass through `SshTunnelService` at all. Applied in `buildJdbcUrl` *and* the + Hikari pool path, and skipped when `tunnelPort != null` since a tunnelled + connection targets the local forwarded port. Also ships disabled + (`deepsql.database.host-guard.enabled`) — databases sit on RFC1918 even more + often than bastions do. + - `S3LogFetchService.assertFetchableUrl` (alert #137) screens the presigned log + URL. The real hazard was `setInstanceFollowRedirects(true)`: the JDK chases a + 302 with no chance to inspect the target, so a presigned URL on a public host + could hand off to the metadata endpoint. Redirects are now followed manually + (max 5), with **every hop** re-checked for https + a public address. Unlike the + other two this is always on — there is no legitimate reason to fetch a slow + query log from a private address. ### CodeQL Remediation (code scanning, 138 alerts on main) diff --git a/backend/src/main/java/com/dbaagent/service/ConnectionService.java b/backend/src/main/java/com/dbaagent/service/ConnectionService.java index 9b91566..080107e 100644 --- a/backend/src/main/java/com/dbaagent/service/ConnectionService.java +++ b/backend/src/main/java/com/dbaagent/service/ConnectionService.java @@ -45,6 +45,7 @@ public class ConnectionService { private final SshTunnelService sshTunnelService; private final CredentialService credentialService; private final DatabaseProviderRegistry providerRegistry; + private final DatabaseHostGuard databaseHostGuard; public boolean testConnection(ConnectionRequest request) { Connection connection = null; @@ -263,6 +264,10 @@ private HikariDataSource createConnectionPool(String connectionId, ConnectionReq log.info("SSH tunnel established on local port {} for connection: {}", tunnelPort, connectionId); } + if (tunnelPort == null) { + databaseHostGuard.assertAllowed(request.getHost()); + } + DatabaseDialect dialect = providerRegistry.getDialect(request.getDbType()); ConnectionProvider connectionProvider = dialect.connection(); @@ -316,6 +321,13 @@ private HikariDataSource createConnectionPool(String connectionId, ConnectionReq * Delegates to the appropriate database provider. */ private String buildJdbcUrl(ConnectionRequest request, Integer tunnelPort) { + // Guard here rather than at each caller: this is the single point every + // JDBC URL is built through (java/ssrf on ConnectionService). A tunnelled + // connection targets the local forwarded port, so only the direct case + // carries a user-supplied host worth screening. + if (tunnelPort == null) { + databaseHostGuard.assertAllowed(request.getHost()); + } DatabaseDialect dialect = providerRegistry.getDialect(request.getDbType()); return dialect.connection().buildJdbcUrl(request, tunnelPort); } diff --git a/backend/src/main/java/com/dbaagent/service/DatabaseHostGuard.java b/backend/src/main/java/com/dbaagent/service/DatabaseHostGuard.java new file mode 100644 index 0000000..52fb2bc --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/DatabaseHostGuard.java @@ -0,0 +1,51 @@ +package com.dbaagent.service; + +import lombok.extern.slf4j.Slf4j; +import org.springframework.stereotype.Component; + +import java.net.InetAddress; + +/** + * Screens the database host a user supplies before a JDBC connection is opened. + * + * Same class of exposure as {@link SshHostGuard} but a different code path: the + * SSH guard sees only request.getSshHost(), and a direct (non-tunnelled) + * connection never passes through it. + */ +@Component +@Slf4j +public class DatabaseHostGuard { + + private final DatabaseHostGuardProperties properties; + + public DatabaseHostGuard(DatabaseHostGuardProperties properties) { + this.properties = properties; + } + + public void assertAllowed(String databaseHost) { + if (!properties.isEnabled() || databaseHost == null || databaseHost.isBlank()) { + return; + } + + String host = OutboundHostGuard.normalize(databaseHost); + if (OutboundHostGuard.isAllowlisted(host, properties.getAllowedHosts())) { + return; + } + + InetAddress blocked; + try { + blocked = OutboundHostGuard.findBlockedAddress(host); + } catch (OutboundHostGuard.BlockedHostException e) { + throw new IllegalArgumentException("Database host could not be resolved: " + databaseHost); + } + + if (blocked != null) { + log.warn("Blocked database connection to restricted host {} (resolved to {})", + databaseHost, blocked.getHostAddress()); + throw new IllegalArgumentException( + "Database host '" + databaseHost + "' resolves to a restricted address (" + + blocked.getHostAddress() + "). Add it to " + + "deepsql.database.host-guard.allowed-hosts if this is intentional."); + } + } +} diff --git a/backend/src/main/java/com/dbaagent/service/DatabaseHostGuardProperties.java b/backend/src/main/java/com/dbaagent/service/DatabaseHostGuardProperties.java new file mode 100644 index 0000000..ff5164b --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/DatabaseHostGuardProperties.java @@ -0,0 +1,25 @@ +package com.dbaagent.service; + +import lombok.Data; +import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.stereotype.Component; + +import java.util.ArrayList; +import java.util.List; + +/** + * Binds the database-host guard (CodeQL java/ssrf on ConnectionService). + * + * Off by default, same reasoning as {@link SshHostGuardProperties}: databases + * legitimately live on RFC1918 networks — more often than bastions do — so + * enabling this would break most existing installs on upgrade. + */ +@Component +@ConfigurationProperties(prefix = "deepsql.database.host-guard") +@Data +public class DatabaseHostGuardProperties { + + private boolean enabled = false; + + private List allowedHosts = new ArrayList<>(); +} diff --git a/backend/src/main/java/com/dbaagent/service/OutboundHostGuard.java b/backend/src/main/java/com/dbaagent/service/OutboundHostGuard.java new file mode 100644 index 0000000..6986323 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/service/OutboundHostGuard.java @@ -0,0 +1,110 @@ +package com.dbaagent.service; + +import java.net.Inet4Address; +import java.net.Inet6Address; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.util.List; +import java.util.Locale; + +/** + * Shared address classification for every outbound host the user can name: + * SSH bastions, database hosts, and presigned log URLs. + * + * The host is resolved and every returned address is checked, so a public + * hostname whose A record points at 10.x or 169.254.169.254 is still refused — + * a check against the literal string is defeated by one DNS record. + */ +public final class OutboundHostGuard { + + private OutboundHostGuard() {} + + /** Thrown when a host resolves to an address outbound traffic must not reach. */ + public static class BlockedHostException extends RuntimeException { + public BlockedHostException(String message) { + super(message); + } + } + + public static String normalize(String host) { + String trimmed = host.trim().toLowerCase(Locale.ROOT); + if (trimmed.startsWith("[") && trimmed.endsWith("]")) { + trimmed = trimmed.substring(1, trimmed.length() - 1); + } + return trimmed; + } + + public static boolean isAllowlisted(String host, List allowedHosts) { + for (String allowed : allowedHosts) { + if (allowed == null || allowed.isBlank()) continue; + String candidate = normalize(allowed); + if (candidate.startsWith(".")) { + if (host.endsWith(candidate)) return true; + } else if (host.equals(candidate)) { + return true; + } + } + return false; + } + + /** + * @return the blocked address, or null when every resolved address is allowed. + */ + public static InetAddress findBlockedAddress(String host) { + InetAddress[] addresses; + try { + addresses = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + throw new BlockedHostException("Host could not be resolved: " + host); + } + for (InetAddress address : addresses) { + if (isBlocked(address)) { + return address; + } + } + return null; + } + + public static boolean isBlocked(InetAddress address) { + if (address.isLoopbackAddress() + || address.isAnyLocalAddress() + || address.isLinkLocalAddress() + || address.isSiteLocalAddress() + || address.isMulticastAddress()) { + return true; + } + if (address instanceof Inet4Address) { + return isBlockedIpv4(address.getAddress()); + } + if (address instanceof Inet6Address v6) { + // Unique local addresses (fc00::/7) have no isSiteLocalAddress() mapping in Java. + if ((v6.getAddress()[0] & 0xFE) == 0xFC) return true; + byte[] embedded = embeddedIpv4(v6); + return embedded != null && isBlockedIpv4(embedded); + } + return false; + } + + private static boolean isBlockedIpv4(byte[] octets) { + int first = octets[0] & 0xFF; + int second = octets[1] & 0xFF; + // 100.64.0.0/10 carrier-grade NAT, used for cloud-internal routing. + if (first == 100 && second >= 64 && second <= 127) return true; + // 192.0.0.0/24 IETF protocol assignments. + if (first == 192 && second == 0 && (octets[2] & 0xFF) == 0) return true; + // 0.0.0.0/8 "this network". + return first == 0; + } + + /** IPv4-mapped/compatible forms smuggle a blocked v4 address through a v6 literal. */ + private static byte[] embeddedIpv4(Inet6Address address) { + byte[] bytes = address.getAddress(); + for (int i = 0; i < 10; i++) { + if (bytes[i] != 0) return null; + } + boolean mapped = (bytes[10] & 0xFF) == 0xFF && (bytes[11] & 0xFF) == 0xFF; + boolean compat = bytes[10] == 0 && bytes[11] == 0; + if (!mapped && !compat) return null; + return new byte[]{bytes[12], bytes[13], bytes[14], bytes[15]}; + } +} diff --git a/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java b/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java index d50ce9a..a81a66f 100644 --- a/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java +++ b/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java @@ -184,6 +184,33 @@ private AwsCredentialsProvider resolveCredentialsProvider(AwsCredentialsInput cr ); } + private static final int MAX_PRESIGNED_REDIRECTS = 5; + + /** + * Every hop of a presigned fetch must be https to a public address. The + * initial URL and each redirect target both go through here. + */ + private URI assertFetchableUrl(URI uri) { + String scheme = uri.getScheme(); + if (scheme == null || !scheme.equalsIgnoreCase("https")) { + throw new IllegalArgumentException( + "Presigned log URL must use https, got: " + scheme); + } + String host = uri.getHost(); + if (host == null || host.isBlank()) { + throw new IllegalArgumentException("Presigned log URL has no host"); + } + java.net.InetAddress blocked = + OutboundHostGuard.findBlockedAddress(OutboundHostGuard.normalize(host)); + if (blocked != null) { + log.warn("Blocked presigned log fetch to restricted host {} (resolved to {})", + host, blocked.getHostAddress()); + throw new IllegalArgumentException( + "Presigned log URL resolves to a restricted address (" + blocked.getHostAddress() + ")"); + } + return uri; + } + boolean isPresignedUrl(String s3Url) { if (s3Url == null || s3Url.isBlank()) { return false; @@ -197,26 +224,52 @@ boolean isPresignedUrl(String s3Url) { private InputStream downloadPresignedLog(String presignedUrl) { try { - HttpURLConnection connection = (HttpURLConnection) URI.create(presignedUrl).toURL().openConnection(); - connection.setRequestMethod("GET"); - connection.setConnectTimeout(15_000); - connection.setReadTimeout(60_000); - connection.setInstanceFollowRedirects(true); + // Redirects are followed manually: with setInstanceFollowRedirects(true) + // the JDK chases a 302 without giving us a chance to screen the target, + // so a presigned URL on a public host could hand off to 169.254.169.254 + // or an internal address (java/ssrf). + URI current = assertFetchableUrl(URI.create(presignedUrl)); + HttpURLConnection connection = null; + int status; + for (int redirects = 0; ; redirects++) { + if (redirects > MAX_PRESIGNED_REDIRECTS) { + throw new RuntimeException("Too many redirects fetching presigned URL"); + } + connection = (HttpURLConnection) current.toURL().openConnection(); + connection.setRequestMethod("GET"); + connection.setConnectTimeout(15_000); + connection.setReadTimeout(60_000); + connection.setInstanceFollowRedirects(false); + + status = connection.getResponseCode(); + if (status != HttpURLConnection.HTTP_MOVED_PERM + && status != HttpURLConnection.HTTP_MOVED_TEMP + && status != HttpURLConnection.HTTP_SEE_OTHER + && status != 307 && status != 308) { + break; + } + String location = connection.getHeaderField("Location"); + connection.disconnect(); + if (location == null || location.isBlank()) { + throw new RuntimeException("Redirect without a Location header"); + } + current = assertFetchableUrl(current.resolve(location)); + } - int status = connection.getResponseCode(); if (status < 200 || status >= 300) { throw new RuntimeException("Failed to download presigned URL, status " + status); } log.info("Streaming slow query log from presigned URL"); - InputStream inputStream = connection.getInputStream(); + final HttpURLConnection finalConnection = connection; + InputStream inputStream = finalConnection.getInputStream(); return new java.io.FilterInputStream(inputStream) { @Override public void close() throws java.io.IOException { try { super.close(); } finally { - connection.disconnect(); + finalConnection.disconnect(); } } }; diff --git a/backend/src/main/java/com/dbaagent/service/SshHostGuard.java b/backend/src/main/java/com/dbaagent/service/SshHostGuard.java index 76798a3..3f0bf8c 100644 --- a/backend/src/main/java/com/dbaagent/service/SshHostGuard.java +++ b/backend/src/main/java/com/dbaagent/service/SshHostGuard.java @@ -3,19 +3,15 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Component; -import java.net.Inet4Address; -import java.net.Inet6Address; import java.net.InetAddress; -import java.net.UnknownHostException; -import java.util.Locale; /** * Rejects SSH bastion hosts that point back into infrastructure the caller * should not reach through DeepSQL — loopback, link-local (including the cloud * metadata endpoint), and RFC1918 ranges. * - * The host is resolved and every returned address is checked, so a public - * hostname whose A record points at 10.x or 169.254.169.254 is still refused. + * Address classification lives in {@link OutboundHostGuard}, shared with the + * database-host and presigned-URL guards. */ @Component @Slf4j @@ -32,92 +28,26 @@ public void assertAllowed(String sshHost) { throw new IllegalArgumentException("SSH host is required"); } - String host = normalize(sshHost); + String host = OutboundHostGuard.normalize(sshHost); - if (!properties.isEnabled() || isExplicitlyAllowed(host)) { + if (!properties.isEnabled() || OutboundHostGuard.isAllowlisted(host, properties.getAllowedHosts())) { return; } - InetAddress[] addresses; + InetAddress blocked; try { - addresses = InetAddress.getAllByName(host); - } catch (UnknownHostException e) { + blocked = OutboundHostGuard.findBlockedAddress(host); + } catch (OutboundHostGuard.BlockedHostException e) { throw new IllegalArgumentException("SSH host could not be resolved: " + sshHost); } - for (InetAddress address : addresses) { - if (isBlocked(address)) { - log.warn("Blocked SSH tunnel to restricted host {} (resolved to {})", - sshHost, address.getHostAddress()); - throw new IllegalArgumentException( - "SSH host '" + sshHost + "' resolves to a restricted address (" - + address.getHostAddress() + "). Add it to " - + "deepsql.ssh.host-guard.allowed-hosts if this is intentional."); - } + if (blocked != null) { + log.warn("Blocked SSH tunnel to restricted host {} (resolved to {})", + sshHost, blocked.getHostAddress()); + throw new IllegalArgumentException( + "SSH host '" + sshHost + "' resolves to a restricted address (" + + blocked.getHostAddress() + "). Add it to " + + "deepsql.ssh.host-guard.allowed-hosts if this is intentional."); } } - - private boolean isExplicitlyAllowed(String host) { - for (String allowed : properties.getAllowedHosts()) { - if (allowed == null || allowed.isBlank()) continue; - String candidate = normalize(allowed); - if (candidate.startsWith(".")) { - if (host.endsWith(candidate)) return true; - } else if (host.equals(candidate)) { - return true; - } - } - return false; - } - - private boolean isBlocked(InetAddress address) { - if (address.isLoopbackAddress() - || address.isAnyLocalAddress() - || address.isLinkLocalAddress() - || address.isSiteLocalAddress() - || address.isMulticastAddress()) { - return true; - } - if (address instanceof Inet4Address) { - return isBlockedIpv4(address.getAddress()); - } - if (address instanceof Inet6Address v6) { - // Unique local addresses (fc00::/7) have no isSiteLocalAddress() mapping in Java. - if ((v6.getAddress()[0] & 0xFE) == 0xFC) return true; - // IPv4-mapped/compatible forms smuggle a blocked v4 address through a v6 literal. - byte[] embedded = embeddedIpv4(v6); - return embedded != null && isBlockedIpv4(embedded); - } - return false; - } - - private boolean isBlockedIpv4(byte[] octets) { - int first = octets[0] & 0xFF; - int second = octets[1] & 0xFF; - // 100.64.0.0/10 carrier-grade NAT, used for cloud-internal routing. - if (first == 100 && second >= 64 && second <= 127) return true; - // 192.0.0.0/24 IETF protocol assignments. - if (first == 192 && second == 0 && (octets[2] & 0xFF) == 0) return true; - // 0.0.0.0/8 "this network". - return first == 0; - } - - private byte[] embeddedIpv4(Inet6Address address) { - byte[] bytes = address.getAddress(); - for (int i = 0; i < 10; i++) { - if (bytes[i] != 0) return null; - } - boolean mapped = (bytes[10] & 0xFF) == 0xFF && (bytes[11] & 0xFF) == 0xFF; - boolean compat = bytes[10] == 0 && bytes[11] == 0; - if (!mapped && !compat) return null; - return new byte[]{bytes[12], bytes[13], bytes[14], bytes[15]}; - } - - private String normalize(String host) { - String trimmed = host.trim().toLowerCase(Locale.ROOT); - if (trimmed.startsWith("[") && trimmed.endsWith("]")) { - trimmed = trimmed.substring(1, trimmed.length() - 1); - } - return trimmed; - } } diff --git a/backend/src/test/java/com/dbaagent/service/ConnectionServiceTest.java b/backend/src/test/java/com/dbaagent/service/ConnectionServiceTest.java index 363f2b7..e18834f 100644 --- a/backend/src/test/java/com/dbaagent/service/ConnectionServiceTest.java +++ b/backend/src/test/java/com/dbaagent/service/ConnectionServiceTest.java @@ -32,7 +32,8 @@ class ConnectionServiceTest { @BeforeEach void setUp() { - connectionService = new ConnectionService(sshTunnelService, credentialService, providerRegistry); + connectionService = new ConnectionService(sshTunnelService, credentialService, providerRegistry, + new DatabaseHostGuard(new DatabaseHostGuardProperties())); } // ─── testConnection ────────────────────────────────────────────────────── diff --git a/backend/src/test/java/com/dbaagent/service/DatabaseHostGuardTest.java b/backend/src/test/java/com/dbaagent/service/DatabaseHostGuardTest.java new file mode 100644 index 0000000..2ec760b --- /dev/null +++ b/backend/src/test/java/com/dbaagent/service/DatabaseHostGuardTest.java @@ -0,0 +1,71 @@ +package com.dbaagent.service; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class DatabaseHostGuardTest { + + private DatabaseHostGuardProperties properties; + private DatabaseHostGuard guard; + + @BeforeEach + void setUp() { + properties = new DatabaseHostGuardProperties(); + properties.setEnabled(true); + guard = new DatabaseHostGuard(properties); + } + + @Test + void shipsDisabledByDefault() { + DatabaseHostGuardProperties defaults = new DatabaseHostGuardProperties(); + + assertFalse(defaults.isEnabled()); + assertDoesNotThrow(() -> new DatabaseHostGuard(defaults).assertAllowed("169.254.169.254")); + } + + @ParameterizedTest + @ValueSource(strings = {"127.0.0.1", "10.0.0.5", "192.168.1.1", "169.254.169.254", "[::1]"}) + void blocksRestrictedTargets(String host) { + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed(host)); + } + + @Test + void allowsPublicAddress() { + assertDoesNotThrow(() -> guard.assertAllowed("93.184.216.34")); + } + + @Test + void allowlistExemptsTheOperatorsOwnDatabase() { + properties.setAllowedHosts(List.of("10.0.0.5", ".db.internal")); + + assertDoesNotThrow(() -> guard.assertAllowed("10.0.0.5")); + assertDoesNotThrow(() -> guard.assertAllowed("primary.db.internal")); + assertThrows(IllegalArgumentException.class, () -> guard.assertAllowed("10.0.0.6")); + } + + @Test + void blankHostIsSkippedRatherThanRejected() { + // A tunnelled connection targets the local forwarded port; the caller + // passes no host and that must not be an error. + assertDoesNotThrow(() -> guard.assertAllowed(null)); + assertDoesNotThrow(() -> guard.assertAllowed(" ")); + } + + @Test + void messageNamesTheHostAndTheEscapeHatch() { + IllegalArgumentException e = assertThrows(IllegalArgumentException.class, + () -> guard.assertAllowed("169.254.169.254")); + + assertTrue(e.getMessage().contains("169.254.169.254"), e.getMessage()); + assertTrue(e.getMessage().contains("allowed-hosts"), e.getMessage()); + } +} From 1170b517119ce23c0a6f60768c335dd22353725f Mon Sep 17 00:00:00 2001 From: sumit Date: Tue, 18 Aug 2026 19:22:10 +0530 Subject: [PATCH 4/5] fix(security): bound ReDoS scan window and rebuild presigned URL from checked parts Follow-up to the CodeQL findings on PR #62's own new code -- two alerts the first pass introduced rather than fixed. polynomial-redos (PatternUtil): the find() rewrite removed the outer .* but not the inner A.*B gap that ~47 of the classifier patterns carry. That gap backtracks super-linearly when B is absent -- measured at 93s on a 50k-token input, a real DoS reachable from a chat message. Rather than reshape ~90 patterns and risk changing what they match, containsPattern now caps the scanned input to 4096 chars. The gap can only backtrack within that window (worst pattern: 93s -> ~23ms). Real questions and identifiers are far shorter, so matching is unchanged for every legitimate input. ssrf (S3LogFetchService): CodeQL did not treat assertFetchableUrl as a barrier because it returned the tainted URI unchanged. It now rebuilds the URI from validated components with the scheme pinned to the https literal, so no unvalidated part of the caller's string survives into openConnection(). Verified: worst pattern bounded under 1s in a test; assertFetchableUrl still accepts real https S3 URLs and rejects http/file/private-host/redirect targets. PatternUtil and S3LogFetchService suites green. --- .../dbaagent/service/S3LogFetchService.java | 15 ++++++++-- .../java/com/dbaagent/util/PatternUtil.java | 30 ++++++++++++++----- .../com/dbaagent/util/PatternUtilTest.java | 21 +++++++++++++ 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java b/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java index a81a66f..cfc88ef 100644 --- a/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java +++ b/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java @@ -187,8 +187,10 @@ private AwsCredentialsProvider resolveCredentialsProvider(AwsCredentialsInput cr private static final int MAX_PRESIGNED_REDIRECTS = 5; /** - * Every hop of a presigned fetch must be https to a public address. The - * initial URL and each redirect target both go through here. + * Validates and rebuilds a presigned fetch URL: https only, to a public + * address. Returns a URI reconstructed from checked components rather than + * the input, so no unvalidated part of the caller's string survives into + * the request (java/ssrf). The initial URL and every redirect hop pass here. */ private URI assertFetchableUrl(URI uri) { String scheme = uri.getScheme(); @@ -208,7 +210,14 @@ private URI assertFetchableUrl(URI uri) { throw new IllegalArgumentException( "Presigned log URL resolves to a restricted address (" + blocked.getHostAddress() + ")"); } - return uri; + try { + // Rebuild from validated pieces; scheme is pinned to the https literal. + return new URI("https", uri.getRawUserInfo(), host, uri.getPort(), + uri.getRawPath(), uri.getRawQuery(), uri.getRawFragment()) + .parseServerAuthority(); + } catch (java.net.URISyntaxException e) { + throw new IllegalArgumentException("Malformed presigned log URL"); + } } boolean isPresignedUrl(String s3Url) { diff --git a/backend/src/main/java/com/dbaagent/util/PatternUtil.java b/backend/src/main/java/com/dbaagent/util/PatternUtil.java index 36f808f..a00c807 100644 --- a/backend/src/main/java/com/dbaagent/util/PatternUtil.java +++ b/backend/src/main/java/com/dbaagent/util/PatternUtil.java @@ -5,18 +5,31 @@ import java.util.regex.Pattern; /** - * Unanchored regex matching over a cached pattern. + * Unanchored regex matching over a cached pattern, on a length-bounded input. * - * Replaces {@code s.matches(".*RE.*")}. String.matches anchors the whole input, - * so the surrounding {@code .*} only exists to undo that anchoring — and the - * combination of those wrappers with an alternation is what drives the - * polynomial backtracking CodeQL reports as java/polynomial-redos. find() on - * the bare expression is equivalent and linear. + * Replaces {@code s.matches(".*RE.*")}: String.matches anchors the whole input, + * so the surrounding {@code .*} exists only to undo that anchoring, and find() + * on the bare expression is equivalent. + * + * Some of the caller expressions have an inner {@code A.*B} gap that backtracks + * super-linearly when B is absent (java/polynomial-redos) — measured at tens of + * seconds on a crafted multi-thousand-token input. Rather than reshape ~90 + * classifier patterns (and risk changing what they match), the input is capped + * to {@link #MAX_SCAN_CHARS} before matching. The gap can then backtrack only + * within that window, which bounds every pattern to a few milliseconds. Real + * chat messages and identifiers are far shorter, so matching is unchanged for + * every legitimate input; only an abusive one is truncated. */ public final class PatternUtil { private PatternUtil() {} + /** + * Longest input scanned. Well above any real question or identifier, far + * below the length where a backtracking gap becomes expensive. + */ + static final int MAX_SCAN_CHARS = 4096; + private static final int MAX_CACHED_PATTERNS = 512; private static final Map CACHE = new ConcurrentHashMap<>(); @@ -24,7 +37,10 @@ public static boolean containsPattern(String input, String regex) { if (input == null) { return false; } - return cached(regex).matcher(input).find(); + CharSequence scanned = input.length() > MAX_SCAN_CHARS + ? input.subSequence(0, MAX_SCAN_CHARS) + : input; + return cached(regex).matcher(scanned).find(); } private static Pattern cached(String regex) { diff --git a/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java b/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java index 377758d..15ccd66 100644 --- a/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java +++ b/backend/src/test/java/com/dbaagent/util/PatternUtilTest.java @@ -45,6 +45,27 @@ void findsAKeywordAfterANewline() { "\\b(slow query|slowest)\\b")); } + @Test + void backtrackingGapStaysFastOnAbusiveInput() { + // \bA\b.*\bB\b backtracks super-linearly when B is absent; the input + // cap keeps it bounded. This input would take tens of seconds uncapped. + String regex = "\\b(how many|count|number of)\\b.*\\b(rows?|records?)\\b"; + String abusive = "count ".repeat(50_000); + + long start = System.nanoTime(); + boolean result = PatternUtil.containsPattern(abusive, regex); + long millis = (System.nanoTime() - start) / 1_000_000; + + assertFalse(result); + assertTrue(millis < 1000, "took " + millis + "ms, expected bounded"); + } + + @Test + void matchWithinTheCapIsUnaffected() { + assertTrue(PatternUtil.containsPattern("how many rows are there", + "\\b(how many|count)\\b.*\\b(rows?)\\b")); + } + @Test void reusesTheCompiledPatternForTheSameRegex() { String regex = "\\bcache-me\\b"; From 0fbb4885e686c7d1a0ff9f848c9c61076aef7853 Mon Sep 17 00:00:00 2001 From: Venkat SF Date: Wed, 19 Aug 2026 10:13:48 +0530 Subject: [PATCH 5/5] Potential fix for pull request finding 'CodeQL / Server-side request forgery' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../dbaagent/service/S3LogFetchService.java | 25 ++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java b/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java index cfc88ef..f101c56 100644 --- a/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java +++ b/backend/src/main/java/com/dbaagent/service/S3LogFetchService.java @@ -186,11 +186,27 @@ private AwsCredentialsProvider resolveCredentialsProvider(AwsCredentialsInput cr private static final int MAX_PRESIGNED_REDIRECTS = 5; + private boolean isAllowedPresignedHost(String host) { + if (host == null || host.isBlank()) { + return false; + } + String normalized = host.toLowerCase(java.util.Locale.ROOT); + return normalized.equals("s3.amazonaws.com") + || normalized.endsWith(".s3.amazonaws.com") + || normalized.matches(".*\\.s3\\.[a-z0-9-]+\\.amazonaws\\.com") + || normalized.matches(".*\\.s3-[a-z0-9-]+\\.amazonaws\\.com") + || normalized.equals("s3.amazonaws.com.cn") + || normalized.endsWith(".s3.amazonaws.com.cn") + || normalized.matches(".*\\.s3\\.[a-z0-9-]+\\.amazonaws\\.com\\.cn") + || normalized.matches(".*\\.s3-[a-z0-9-]+\\.amazonaws\\.com\\.cn"); + } + /** * Validates and rebuilds a presigned fetch URL: https only, to a public - * address. Returns a URI reconstructed from checked components rather than - * the input, so no unvalidated part of the caller's string survives into - * the request (java/ssrf). The initial URL and every redirect hop pass here. + * address, and restricted to known S3 endpoint host patterns. Returns a URI + * reconstructed from checked components rather than the input, so no + * unvalidated part of the caller's string survives into the request + * (java/ssrf). The initial URL and every redirect hop pass here. */ private URI assertFetchableUrl(URI uri) { String scheme = uri.getScheme(); @@ -202,6 +218,9 @@ private URI assertFetchableUrl(URI uri) { if (host == null || host.isBlank()) { throw new IllegalArgumentException("Presigned log URL has no host"); } + if (!isAllowedPresignedHost(host)) { + throw new IllegalArgumentException("Presigned log URL host is not an allowed S3 endpoint"); + } java.net.InetAddress blocked = OutboundHostGuard.findBlockedAddress(OutboundHostGuard.normalize(host)); if (blocked != null) {