From c1c762e4132d23fd0b4738b9bfad085f3ca3d5d3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 13 Aug 2026 11:19:19 +0000 Subject: [PATCH 1/2] =?UTF-8?q?fix(security):=20close=20OSS=20Criticals=20?= =?UTF-8?q?S1=E2=80=93S5?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kill-session pid validation + prepared SQL; connection ACL on apply/kill and related controllers; Hermes/compose loopback binds; Valkey requirepass; Actuator lockdown; JWT fail-closed under prod/auth. Adds SECURITY.md and smoke assertions. Co-authored-by: Venkat SF --- .env.example | 14 ++- SECURITY.md | 100 +++++++++--------- .../com/dbaagent/config/SecurityConfig.java | 5 +- .../controller/ActiveQueryController.java | 14 +++ .../controller/ConfigurationController.java | 13 +++ .../GrowthMonitoringController.java | 10 ++ .../IndexRecommendationController.java | 21 ++++ .../controller/LockContentionController.java | 13 +++ .../controller/PlaybookController.java | 9 ++ .../controller/SavedQueryController.java | 20 ++++ .../controller/SlowLogSourceController.java | 19 ++++ .../java/com/dbaagent/security/JwtUtil.java | 31 +++++- .../dbaagent/service/ActiveQueryService.java | 41 ++++--- .../service/DatabaseConfigurationService.java | 11 +- .../service/IndexRecommendationService.java | 11 +- .../service/LockContentionService.java | 56 +++++----- .../service/PlaybookExecutionService.java | 8 +- .../com/dbaagent/service/PlaybookService.java | 8 +- .../com/dbaagent/util/SessionKillSupport.java | 24 +++++ .../resources/application-prod.properties | 8 +- .../src/main/resources/application.properties | 4 +- .../security/JwtUtilFailClosedTest.java | 46 ++++++++ .../dbaagent/util/SessionKillSupportTest.java | 25 +++++ docker-compose.yml | 37 +++++-- docs/oss-ux/OSS_SECURITY_REVIEW.md | 2 + scripts/self-host/install.sh | 6 +- scripts/self-host/setup-agent.sh | 7 +- scripts/self-host/smoke-test.sh | 25 +++++ 28 files changed, 461 insertions(+), 127 deletions(-) create mode 100644 backend/src/main/java/com/dbaagent/util/SessionKillSupport.java create mode 100644 backend/src/test/java/com/dbaagent/security/JwtUtilFailClosedTest.java create mode 100644 backend/src/test/java/com/dbaagent/util/SessionKillSupportTest.java diff --git a/.env.example b/.env.example index 75ed05b..be8ea9e 100644 --- a/.env.example +++ b/.env.example @@ -33,8 +33,12 @@ SECURITY_JWT_SECRET= ENCRYPTION_KEY= ENCRYPTION_KEY_ID=self-hosted-key-1 -# Vault database password -DB_PASSWORD=postgres +# Vault database password — leave empty / change-me-* so install.sh generates one. +# Never leave this as the literal "postgres" on a networked host. +DB_PASSWORD=change-me-db-password + +# Valkey/Redis password (compose --requirepass). Generated by install.sh when placeholder. +DEEPSQL_VALKEY_PASSWORD=change-me-valkey-password # Self-host should always use the hardened production Spring profile. SPRING_PROFILES_ACTIVE=prod @@ -138,13 +142,17 @@ EMBEDDING_FAIL_OPEN=false # AGENT_PROVISIONER_URL Per-user profile provisioner. Compose default: # http://deepsql-agent:8788/provision # AGENT_PROVISION_SECRET Shared secret between backend and agent (required). -# DEEPSQL_AGENT_PORT / DEEPSQL_AGENT_PROVISIONER_PORT — host port mappings. +# DEEPSQL_AGENT_PORT / DEEPSQL_AGENT_PROVISIONER_PORT — host port mappings +# (compose binds these to 127.0.0.1 only; public path is nginx /agent-api). +# Native (non-Compose) agent: HERMES_WEBUI_HOST defaults to 127.0.0.1 in +# setup-agent.sh — do not set 0.0.0.0 on internet-facing hosts. # #AGENT_WEBUI_URL=http://deepsql-agent:8787 #AGENT_PROVISIONER_URL=http://deepsql-agent:8788/provision AGENT_PROVISION_SECRET=change-me-agent-provision-secret #DEEPSQL_AGENT_PORT=8787 #DEEPSQL_AGENT_PROVISIONER_PORT=8788 +#HERMES_WEBUI_HOST=127.0.0.1 #DEEPSQL_SMOKE_AGENT=1 # ── Demo Data Seeding ─────────────────────────────────────────────────────── diff --git a/SECURITY.md b/SECURITY.md index 5eeca88..7e3d27f 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,69 +1,67 @@ -# Security Policy +# DeepSQL Security -DeepSQL stores database credentials in an encrypted vault, holds an AES-GCM key -whose loss is unrecoverable, and enforces read-only SQL execution as a guardrail. -We treat reports against those paths as our highest priority. +## Threat model (be honest) -## Reporting a vulnerability +| Posture | Who is trusted | Network | +|---------|----------------|---------| +| **Private / single-admin** | One admin (or a fully trusted ops team) | Private network, Tailscale, or SSH tunnel. Prefer only `:3000` reachable from clients. | +| **Internet multi-user** | Untrusted tenants sharing one install | Requires ACL on every connection-scoped API, hardened compose binds, Actuator lockdown, Hermes behind nginx only, JWT fail-closed — see [`docs/oss-ux/OSS_SECURITY_REVIEW.md`](docs/oss-ux/OSS_SECURITY_REVIEW.md). | -Report privately via **Security → Report a vulnerability** on this repository. -Do not open a public issue, and do not describe the problem in a pull request. +DeepSQL is **not** marketed as multi-tenant SaaS until the Criticals in that review are closed and High findings (SSRF, SET allowlist, share-password defaults) are addressed. -We acknowledge reports within **48 hours**, provide an assessment within -**5 business days**, and aim to ship a fix within **90 days**. We credit -reporters in the published advisory unless you prefer otherwise. +## Required network layout -If a report is time-critical and you have had no acknowledgement within 48 hours, -open a public issue containing no technical detail — just a request that a -maintainer check private reports — and we will pick it up. +```text +Internet / LAN clients + │ + ▼ + :3000 frontend (nginx) + ├── /api/* → backend:8080 (auth cookies / JWT) + └── /agent-api/* → deepsql-agent:8787 (auth_request → /api/auth/me) + +Host loopback only (not WAN): + 127.0.0.1:5432 postgres + 127.0.0.1:6379 valkey (--requirepass) + 127.0.0.1:8080 backend (debug / health probes) + 127.0.0.1:8787 agent API + 127.0.0.1:8788 agent provisioner +``` -## Supported versions +Do **not** publish Postgres, Valkey, backend, or Hermes on `0.0.0.0` on a cloud VM. -The latest tagged release receives security fixes. Older tags do not. +## Secrets checklist -## In scope +| Secret | Purpose | +|--------|---------| +| `SECURITY_JWT_SECRET` | Session token signing (≥32 bytes). **Required** under `prod` / auth-on — boot fails closed if missing. | +| `ENCRYPTION_KEY` / `ENCRYPTION_KEYS` | Vault credential encryption | +| `DB_PASSWORD` | Vault Postgres (never leave as `postgres` on a networked host) | +| `DEEPSQL_VALKEY_PASSWORD` | Valkey `--requirepass` | +| `DEEPSQL_CHAT_*` / embedding keys | LLM | +| `AGENT_PROVISION_SECRET` | Backend ↔ agent provisioner | -- Credential-vault encryption and key handling -- Read-only SQL execution enforcement, and any bypass of it -- Authentication, JWT handling, and MCP token authorisation -- The admin bootstrap endpoint -- The dashboard sandbox iframe and its read-only query bridge, including the - public share path -- SSH tunnelling -- Reachable dependency vulnerabilities +`./scripts/self-host/install.sh` generates JWT, encryption, DB, Valkey, bootstrap, and provision secrets when placeholders remain. -## Out of scope +## Post-install -These are by design, and reporting them will get a courteous decline: +1. Disable admin bootstrap (`SECURITY_ADMIN_BOOTSTRAP_ENABLED=false`) after the first admin exists. +2. Rotate `ADMIN_BOOTSTRAP_SECRET` if it was ever logged or shared. +3. Confirm `SPRING_PROFILES_ACTIVE=prod` and `SECURITY_AUTH_ENABLED` is not forced off. -- Behaviour when `SECURITY_AUTH_ENABLED=false`. This is a development-only - shortcut and is documented as such. -- The hand-written SQL editor's ability to mutate data for a confirming admin. - A DBA tool that cannot run `UPDATE` is not a DBA tool; the guardrail governs - *generated* and *agent-issued* SQL, not a human who has explicitly confirmed. -- The localhost-only bootstrap endpoint when deliberately enabled. -- Anything requiring prior host compromise. -- Missing hardening headers with no demonstrated impact. +## MCP tokens -## How fixes are handled +MCP tokens are **full-account PATs** (same authority as the minting user). Revoke on logout / staff exit. Prefer short-lived tokens; connection-scoped tokens are a roadmap item. -Fixes are developed in a private fork through GitHub Security Advisories. The -advisory and the patched release are published simultaneously. A vulnerability is -never fixed in a normal public pull request: on a repository anyone can watch, -that commit is a roadmap to the bug for everyone still running the old version. +## Public dashboard links -## A note on review requirements +A public share token authorizes **read-only SQL** against the dashboard’s connection while `is_public` is true. Treat share URLs like credentials; revoke by deleting the share / flipping `is_public`. Prefer password-protected shares once that control ships. -`.github/CODEOWNERS` routes changes under the vault, authentication and -SQL-execution paths to the security owners, so the right people are *required* -reviewers. It cannot, however, require a larger *number* of approvals on those -paths specifically — GitHub carries a single repo-wide approval count. The -two-approval rule on security-critical paths is therefore a maintainer -convention, enforced by reviewers rather than by the platform. Treat a -security-path pull request carrying only one approval as not yet ready. +## Reporting vulnerabilities -## Please do not +Please report security issues privately to the maintainers (GitHub Security Advisory on [DeepSQLAI/deepsql](https://github.com/DeepSQLAI/deepsql) preferred). Do not open public issues that include exploit details until a fix is available. -- Test against infrastructure you do not own. -- Include real credentials, API keys, or `ENCRYPTION_KEY` values in a report. - Redact them; we can reproduce from a description. +## Explicit non-goals (until complete) + +- Guaranteeing every legacy controller has connection ACL (track remaining High/Medium in the security review) +- Scoping MCP tokens to a single connection +- Hardening every SSRF-capable webhook / LLM endpoint tester diff --git a/backend/src/main/java/com/dbaagent/config/SecurityConfig.java b/backend/src/main/java/com/dbaagent/config/SecurityConfig.java index d8d8f4b..0a6520f 100644 --- a/backend/src/main/java/com/dbaagent/config/SecurityConfig.java +++ b/backend/src/main/java/com/dbaagent/config/SecurityConfig.java @@ -89,7 +89,10 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti "/auth/cli/exchange", "/auth/cli/device/code", "/auth/cli/device/token", - "/actuator/**", + // Actuator: only liveness/readiness without auth. metrics/prometheus + // require an authenticated session (see application-prod.properties). + "/actuator/health", + "/actuator/health/**", "/error", "/admin/bootstrap/link", "/users/admin/bootstrap", diff --git a/backend/src/main/java/com/dbaagent/controller/ActiveQueryController.java b/backend/src/main/java/com/dbaagent/controller/ActiveQueryController.java index bebd1d9..da752a1 100644 --- a/backend/src/main/java/com/dbaagent/controller/ActiveQueryController.java +++ b/backend/src/main/java/com/dbaagent/controller/ActiveQueryController.java @@ -2,6 +2,7 @@ import com.dbaagent.model.ActiveQuery; import com.dbaagent.service.ActiveQueryService; +import com.dbaagent.service.security.AccessControlService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.ResponseEntity; @@ -18,6 +19,7 @@ public class ActiveQueryController { private final ActiveQueryService activeQueryService; + private final AccessControlService accessControlService; /** * Capture current active queries from the database @@ -27,6 +29,7 @@ public class ActiveQueryController { public ResponseEntity> captureActiveQueries(@PathVariable String connectionId) { log.info("API: Capturing active queries for connection {}", connectionId); try { + accessControlService.assertCanManageConnectionContent(connectionId); List queries = activeQueryService.captureActiveQueries(connectionId); return ResponseEntity.ok(queries); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -45,6 +48,7 @@ public ResponseEntity> captureActiveQueries(@PathVariable Stri public ResponseEntity> getLatestQueries(@PathVariable String connectionId) { log.info("API: Getting latest queries for connection {}", connectionId); try { + accessControlService.assertCanReadConnectionContent(connectionId); List queries = activeQueryService.getLatestQueries(connectionId); return ResponseEntity.ok(queries); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -66,6 +70,7 @@ public ResponseEntity> getQueriesByFilter( @RequestParam String value) { log.info("API: Getting queries for connection {} filtered by {}={}", connectionId, type, value); try { + accessControlService.assertCanReadConnectionContent(connectionId); List queries = activeQueryService.getQueriesByFilter(connectionId, type, value); return ResponseEntity.ok(queries); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -84,6 +89,7 @@ public ResponseEntity> getQueriesByFilter( public ResponseEntity> getStatistics(@PathVariable String connectionId) { log.info("API: Getting query statistics for connection {}", connectionId); try { + accessControlService.assertCanReadConnectionContent(connectionId); Map stats = activeQueryService.getStatistics(connectionId); return ResponseEntity.ok(stats); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -102,6 +108,7 @@ public ResponseEntity> getStatistics(@PathVariable String co public ResponseEntity>> getFilterOptions(@PathVariable String connectionId) { log.info("API: Getting filter options for connection {}", connectionId); try { + accessControlService.assertCanReadConnectionContent(connectionId); Map> options = activeQueryService.getFilterOptions(connectionId); return ResponseEntity.ok(options); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -122,11 +129,17 @@ public ResponseEntity> killQuery( @PathVariable String pid) { log.info("API: Killing query {} on connection {}", pid, connectionId); try { + accessControlService.assertCanManageConnectionContent(connectionId); activeQueryService.killQuery(connectionId, pid); return ResponseEntity.ok(Map.of( "message", "Successfully killed query " + pid, "pid", pid )); + } catch (IllegalArgumentException e) { + return ResponseEntity.badRequest().body(Map.of( + "error", e.getMessage(), + "pid", pid + )); } catch (org.springframework.web.server.ResponseStatusException e) { throw e; } catch (Exception e) { @@ -146,6 +159,7 @@ public ResponseEntity> killQuery( public ResponseEntity> cleanupOldSnapshots(@PathVariable String connectionId) { log.info("API: Cleaning up old snapshots for connection {}", connectionId); try { + accessControlService.assertCanManageConnectionContent(connectionId); int deleted = activeQueryService.cleanupOldSnapshots(connectionId); return ResponseEntity.ok(Map.of( "message", "Cleanup completed", diff --git a/backend/src/main/java/com/dbaagent/controller/ConfigurationController.java b/backend/src/main/java/com/dbaagent/controller/ConfigurationController.java index 1ce119f..987bf68 100644 --- a/backend/src/main/java/com/dbaagent/controller/ConfigurationController.java +++ b/backend/src/main/java/com/dbaagent/controller/ConfigurationController.java @@ -2,6 +2,7 @@ import com.dbaagent.model.ConfigurationRecommendation; import com.dbaagent.service.DatabaseConfigurationService; +import com.dbaagent.service.security.AccessControlService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.ResponseEntity; @@ -21,6 +22,7 @@ public class ConfigurationController { private final DatabaseConfigurationService configurationService; + private final AccessControlService accessControlService; /** * Analyze database configuration and generate tuning recommendations @@ -28,6 +30,7 @@ public class ConfigurationController { @PostMapping("/analyze/{connectionId}") public ResponseEntity analyzeConfiguration(@PathVariable String connectionId) { log.info("Analyzing configuration for connection: {}", connectionId); + accessControlService.assertCanManageConnectionContent(connectionId); try { List recommendations = configurationService.analyzeConfiguration(connectionId); @@ -59,6 +62,7 @@ public ResponseEntity analyzeConfiguration(@PathVariable String @GetMapping("/{connectionId}") public ResponseEntity> getRecommendations(@PathVariable String connectionId) { log.info("Fetching configuration recommendations for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List recommendations = configurationService.getRecommendations(connectionId); return ResponseEntity.ok(recommendations); @@ -70,6 +74,7 @@ public ResponseEntity> getRecommendations(@Pat @GetMapping("/pending/{connectionId}") public ResponseEntity> getPendingRecommendations(@PathVariable String connectionId) { log.info("Fetching pending configuration recommendations for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List recommendations = configurationService.getPendingRecommendations(connectionId); return ResponseEntity.ok(recommendations); @@ -83,6 +88,8 @@ public ResponseEntity markAsApplied(@PathVariable S log.info("Marking configuration recommendation as applied: {}", id); try { + ConfigurationRecommendation existing = configurationService.requireById(id); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); ConfigurationRecommendation recommendation = configurationService.markAsApplied(id); return ResponseEntity.ok(recommendation); } catch (IllegalArgumentException e) { @@ -104,6 +111,8 @@ public ResponseEntity dismissRecommendation(@PathVa log.info("Dismissing configuration recommendation: {}", id); try { + ConfigurationRecommendation existing = configurationService.requireById(id); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); ConfigurationRecommendation recommendation = configurationService.dismissRecommendation(id); return ResponseEntity.ok(recommendation); } catch (IllegalArgumentException e) { @@ -125,11 +134,15 @@ public ResponseEntity> deleteRecommendation(@PathVariable St log.info("Deleting configuration recommendation: {}", id); try { + ConfigurationRecommendation existing = configurationService.requireById(id); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); configurationService.deleteRecommendation(id); return ResponseEntity.ok(Map.of( "success", "true", "message", "Recommendation deleted successfully" )); + } catch (IllegalArgumentException e) { + return ResponseEntity.notFound().build(); } catch (org.springframework.web.server.ResponseStatusException e) { throw e; } catch (Exception e) { diff --git a/backend/src/main/java/com/dbaagent/controller/GrowthMonitoringController.java b/backend/src/main/java/com/dbaagent/controller/GrowthMonitoringController.java index 0f215fa..42feca8 100644 --- a/backend/src/main/java/com/dbaagent/controller/GrowthMonitoringController.java +++ b/backend/src/main/java/com/dbaagent/controller/GrowthMonitoringController.java @@ -8,6 +8,7 @@ import com.dbaagent.repository.TableStatsHistoryRepository; import com.dbaagent.service.GrowthDataCleanupService; import com.dbaagent.service.TableGrowthMonitoringService; +import com.dbaagent.service.security.AccessControlService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatus; @@ -33,6 +34,7 @@ public class GrowthMonitoringController { private final GrowthAlertConfigurationRepository configRepository; private final TableGrowthMonitoringService monitoringService; private final GrowthDataCleanupService cleanupService; + private final AccessControlService accessControlService; /** * Get growth history for tables @@ -43,6 +45,7 @@ public ResponseEntity> getGrowthHistory( @PathVariable String connectionId, @RequestParam(required = false) String tableName, @RequestParam(defaultValue = "7") int days) { + accessControlService.assertCanReadConnectionContent(connectionId); Map response = new HashMap<>(); @@ -84,6 +87,8 @@ public ResponseEntity> getAnomalies( @RequestParam(required = false) String tableName, @RequestParam(defaultValue = "false") boolean unacknowledgedOnly, @RequestParam(defaultValue = "30") int days) { + accessControlService.assertCanReadConnectionContent(connectionId); + Map response = new HashMap<>(); @@ -153,6 +158,7 @@ public ResponseEntity> acknowledgeAnomaly( } GrowthAnomaly anomaly = anomalyOpt.get(); + accessControlService.assertCanManageConnectionContent(anomaly.getConnectionId()); String acknowledgedBy = body.getOrDefault("acknowledgedBy", "system"); anomaly.acknowledge(acknowledgedBy); @@ -181,6 +187,7 @@ public ResponseEntity> acknowledgeAnomaly( public ResponseEntity> getConfiguration( @PathVariable String connectionId, @RequestParam(required = false) String tableName) { + accessControlService.assertCanReadConnectionContent(connectionId); Map response = new HashMap<>(); @@ -223,6 +230,7 @@ public ResponseEntity> getConfiguration( @PostMapping("/config") public ResponseEntity> saveConfiguration( @RequestBody GrowthAlertConfiguration config) { + accessControlService.assertCanManageConnectionContent(config.getConnectionId()); Map response = new HashMap<>(); @@ -278,6 +286,7 @@ public ResponseEntity> getGrowthTrends( @PathVariable String connectionId, @RequestParam(required = false) String tableName, @RequestParam(defaultValue = "30") int days) { + accessControlService.assertCanReadConnectionContent(connectionId); Map response = new HashMap<>(); @@ -348,6 +357,7 @@ public ResponseEntity> getGrowthTrends( */ @PostMapping("/capture/{connectionId}") public ResponseEntity> manualCapture(@PathVariable String connectionId) { + accessControlService.assertCanManageConnectionContent(connectionId); Map response = new HashMap<>(); try { diff --git a/backend/src/main/java/com/dbaagent/controller/IndexRecommendationController.java b/backend/src/main/java/com/dbaagent/controller/IndexRecommendationController.java index 247d800..656b532 100644 --- a/backend/src/main/java/com/dbaagent/controller/IndexRecommendationController.java +++ b/backend/src/main/java/com/dbaagent/controller/IndexRecommendationController.java @@ -4,6 +4,7 @@ import com.dbaagent.model.IndexRecommendationEvidence; import com.dbaagent.service.IndexRecommendationApplyService; import com.dbaagent.service.IndexRecommendationService; +import com.dbaagent.service.security.AccessControlService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.ResponseEntity; @@ -24,6 +25,7 @@ public class IndexRecommendationController { private final IndexRecommendationService recommendationService; private final IndexRecommendationApplyService applyService; + private final AccessControlService accessControlService; /** * Generate new index recommendations based on query history @@ -31,6 +33,7 @@ public class IndexRecommendationController { @PostMapping("/generate/{connectionId}") public ResponseEntity generateRecommendations(@PathVariable String connectionId) { log.info("Generating index recommendations for connection: {}", connectionId); + accessControlService.assertCanManageConnectionContent(connectionId); try { List recommendations = recommendationService.generateRecommendations(connectionId); @@ -62,6 +65,7 @@ public ResponseEntity generateRecommendations(@PathVariable St @GetMapping("/{connectionId}") public ResponseEntity> getRecommendations(@PathVariable String connectionId) { log.info("Fetching all recommendations for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List recommendations = recommendationService.getRecommendations(connectionId); return ResponseEntity.ok(recommendations); @@ -73,6 +77,7 @@ public ResponseEntity> getRecommendations(@PathV @GetMapping("/pending/{connectionId}") public ResponseEntity> getPendingRecommendations(@PathVariable String connectionId) { log.info("Fetching pending recommendations for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List recommendations = recommendationService.getPendingRecommendations(connectionId); return ResponseEntity.ok(recommendations); @@ -94,6 +99,7 @@ public ResponseEntity> getTopRecommendations( @RequestParam(value = "limit", required = false, defaultValue = "5") int limit ) { log.info("Fetching top {} recommendations for connection: {}", limit, connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List body = recommendationService .getTopRecommendationsWithEvidence(connectionId, limit) .stream() @@ -110,6 +116,8 @@ public ResponseEntity markAsApplied(@PathVariable Str log.info("Marking recommendation as applied: {}", id); try { + IndexRecommendationEntity existing = recommendationService.requireById(id); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); IndexRecommendationEntity recommendation = recommendationService.markAsApplied(id); return ResponseEntity.ok(recommendation); } catch (IllegalArgumentException e) { @@ -131,6 +139,8 @@ public ResponseEntity dismissRecommendation(@PathVari log.info("Dismissing recommendation: {}", id); try { + IndexRecommendationEntity existing = recommendationService.requireById(id); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); IndexRecommendationEntity recommendation = recommendationService.dismissRecommendation(id); return ResponseEntity.ok(recommendation); } catch (IllegalArgumentException e) { @@ -172,6 +182,13 @@ public ResponseEntity applyRecommen } catch (IllegalArgumentException e) { return ResponseEntity.badRequest().build(); } + IndexRecommendationEntity existing; + try { + existing = recommendationService.requireById(id); + } catch (IllegalArgumentException e) { + return ResponseEntity.notFound().build(); + } + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); IndexRecommendationApplyService.ApplyOptions opts = new IndexRecommendationApplyService.ApplyOptions(concurrent); IndexRecommendationApplyService.ApplyResult result = applyService.apply(id, m, confirm, opts); return ResponseEntity.ok(result); @@ -185,11 +202,15 @@ public ResponseEntity> deleteRecommendation(@PathVariable St log.info("Deleting recommendation: {}", id); try { + IndexRecommendationEntity existing = recommendationService.requireById(id); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); recommendationService.deleteRecommendation(id); return ResponseEntity.ok(Map.of( "success", "true", "message", "Recommendation deleted successfully" )); + } catch (IllegalArgumentException e) { + return ResponseEntity.notFound().build(); } catch (org.springframework.web.server.ResponseStatusException e) { throw e; } catch (Exception e) { diff --git a/backend/src/main/java/com/dbaagent/controller/LockContentionController.java b/backend/src/main/java/com/dbaagent/controller/LockContentionController.java index 17fcbdc..17a3762 100644 --- a/backend/src/main/java/com/dbaagent/controller/LockContentionController.java +++ b/backend/src/main/java/com/dbaagent/controller/LockContentionController.java @@ -2,6 +2,7 @@ import com.dbaagent.model.LockContention; import com.dbaagent.service.LockContentionService; +import com.dbaagent.service.security.AccessControlService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.ResponseEntity; @@ -18,6 +19,7 @@ public class LockContentionController { private final LockContentionService lockContentionService; + private final AccessControlService accessControlService; /** * Detect current lock contentions in the database @@ -27,6 +29,7 @@ public class LockContentionController { public ResponseEntity> detectLockContentions(@PathVariable String connectionId) { log.info("API: Detecting lock contentions for connection {}", connectionId); try { + accessControlService.assertCanManageConnectionContent(connectionId); List contentions = lockContentionService.detectLockContentions(connectionId); return ResponseEntity.ok(contentions); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -45,6 +48,7 @@ public ResponseEntity> detectLockContentions(@PathVariable public ResponseEntity> getActiveContentions(@PathVariable String connectionId) { log.info("API: Getting active contentions for connection {}", connectionId); try { + accessControlService.assertCanReadConnectionContent(connectionId); List contentions = lockContentionService.getActiveContentions(connectionId); return ResponseEntity.ok(contentions); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -63,6 +67,7 @@ public ResponseEntity> getActiveContentions(@PathVariable S public ResponseEntity> getAllContentions(@PathVariable String connectionId) { log.info("API: Getting all contentions for connection {}", connectionId); try { + accessControlService.assertCanReadConnectionContent(connectionId); List contentions = lockContentionService.getAllContentions(connectionId); return ResponseEntity.ok(contentions); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -81,6 +86,7 @@ public ResponseEntity> getAllContentions(@PathVariable Stri public ResponseEntity> getStatistics(@PathVariable String connectionId) { log.info("API: Getting contention statistics for connection {}", connectionId); try { + accessControlService.assertCanReadConnectionContent(connectionId); Map stats = lockContentionService.getContentionStatistics(connectionId); return ResponseEntity.ok(stats); } catch (org.springframework.web.server.ResponseStatusException e) { @@ -101,11 +107,17 @@ public ResponseEntity> killBlockingSession( @PathVariable String pid) { log.info("API: Killing blocking session {} on connection {}", pid, connectionId); try { + accessControlService.assertCanManageConnectionContent(connectionId); lockContentionService.killBlockingSession(connectionId, pid); return ResponseEntity.ok(Map.of( "message", "Successfully killed session " + pid, "pid", pid )); + } catch (IllegalArgumentException e) { + return ResponseEntity.badRequest().body(Map.of( + "error", e.getMessage(), + "pid", pid + )); } catch (org.springframework.web.server.ResponseStatusException e) { throw e; } catch (Exception e) { @@ -125,6 +137,7 @@ public ResponseEntity> killBlockingSession( public ResponseEntity> cleanupOldContentions(@PathVariable String connectionId) { log.info("API: Cleaning up old contentions for connection {}", connectionId); try { + accessControlService.assertCanManageConnectionContent(connectionId); int deleted = lockContentionService.cleanupOldContentions(connectionId); return ResponseEntity.ok(Map.of( "message", "Cleanup completed", diff --git a/backend/src/main/java/com/dbaagent/controller/PlaybookController.java b/backend/src/main/java/com/dbaagent/controller/PlaybookController.java index 0a99d1b..7d8583d 100644 --- a/backend/src/main/java/com/dbaagent/controller/PlaybookController.java +++ b/backend/src/main/java/com/dbaagent/controller/PlaybookController.java @@ -5,6 +5,7 @@ import com.dbaagent.model.PlaybookRun; import com.dbaagent.service.PlaybookExecutionService; import com.dbaagent.service.PlaybookService; +import com.dbaagent.service.security.AccessControlService; import lombok.Data; import lombok.RequiredArgsConstructor; import org.springframework.http.HttpStatus; @@ -23,6 +24,7 @@ public class PlaybookController { private final PlaybookService playbookService; private final PlaybookExecutionService playbookExecutionService; + private final AccessControlService accessControlService; // ========== PLAYBOOK MANAGEMENT ========== @@ -176,6 +178,7 @@ public ResponseEntity> executePlaybook( Map response = new HashMap<>(); try { + accessControlService.assertCanManageConnectionContent(request.getConnectionId()); PlaybookRun run = playbookExecutionService.executePlaybook( playbookId, request.getConnectionId() @@ -207,6 +210,7 @@ public ResponseEntity> getRunHistory( Map response = new HashMap<>(); try { + accessControlService.assertCanReadConnectionContent(connectionId); List runs = playbookExecutionService.getRunHistory(connectionId, limit); Map stats = playbookExecutionService.getRunStatistics(connectionId); @@ -229,6 +233,8 @@ public ResponseEntity> cancelRun(@PathVariable String runId) Map response = new HashMap<>(); try { + PlaybookRun run = playbookExecutionService.requireRunById(runId); + accessControlService.assertCanManageConnectionContent(run.getConnectionId()); playbookExecutionService.cancelRun(runId); response.put("success", true); response.put("message", "Run cancelled successfully"); @@ -252,6 +258,7 @@ public ResponseEntity> getAlerts( Map response = new HashMap<>(); try { + accessControlService.assertCanReadConnectionContent(connectionId); List alerts = unacknowledgedOnly ? playbookService.getUnacknowledgedAlerts(connectionId) : playbookService.getAlertsForConnection(connectionId); @@ -280,6 +287,8 @@ public ResponseEntity> acknowledgeAlert( Map response = new HashMap<>(); try { + PlaybookAlert existing = playbookService.requireAlertById(alertId); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); String acknowledgedBy = request != null ? request.getAcknowledgedBy() : "user"; PlaybookAlert alert = playbookService.acknowledgeAlert(alertId, acknowledgedBy); diff --git a/backend/src/main/java/com/dbaagent/controller/SavedQueryController.java b/backend/src/main/java/com/dbaagent/controller/SavedQueryController.java index f408f5b..68f3bfb 100644 --- a/backend/src/main/java/com/dbaagent/controller/SavedQueryController.java +++ b/backend/src/main/java/com/dbaagent/controller/SavedQueryController.java @@ -2,6 +2,7 @@ import com.dbaagent.model.SavedQuery; import com.dbaagent.service.SavedQueryService; +import com.dbaagent.service.security.AccessControlService; import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.HttpStatus; @@ -21,6 +22,9 @@ public class SavedQueryController { @Autowired private SavedQueryService savedQueryService; + @Autowired + private AccessControlService accessControlService; + /** * Create a new saved query */ @@ -28,6 +32,7 @@ public class SavedQueryController { public ResponseEntity> createQuery(@RequestBody SavedQuery savedQuery) { try { log.info("Creating saved query: {} for connection: {}", savedQuery.getName(), savedQuery.getConnectionId()); + accessControlService.assertCanManageConnectionContent(savedQuery.getConnectionId()); SavedQuery created = savedQueryService.saveQuery(savedQuery); @@ -55,6 +60,7 @@ public ResponseEntity> createQuery(@RequestBody SavedQuery s public ResponseEntity> getQueriesByConnection(@PathVariable String connectionId) { try { log.info("Fetching saved queries for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List queries = savedQueryService.getQueriesByConnection(connectionId); @@ -85,6 +91,7 @@ public ResponseEntity> getQueryById(@PathVariable UUID id) { return savedQueryService.getQueryById(id) .map(query -> { + accessControlService.assertCanReadConnectionContent(query.getConnectionId()); Map response = new HashMap<>(); response.put("success", true); response.put("savedQuery", query); @@ -114,6 +121,9 @@ public ResponseEntity> getQueryById(@PathVariable UUID id) { public ResponseEntity> updateQuery(@PathVariable UUID id, @RequestBody SavedQuery updates) { try { log.info("Updating saved query: {}", id); + SavedQuery existing = savedQueryService.getQueryById(id) + .orElseThrow(() -> new IllegalArgumentException("Query not found: " + id)); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); SavedQuery updated = savedQueryService.updateQuery(id, updates); @@ -147,6 +157,9 @@ public ResponseEntity> updateQuery(@PathVariable UUID id, @R public ResponseEntity> deleteQuery(@PathVariable UUID id) { try { log.info("Deleting saved query: {}", id); + SavedQuery existing = savedQueryService.getQueryById(id) + .orElseThrow(() -> new IllegalArgumentException("Query not found: " + id)); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); savedQueryService.deleteQuery(id); @@ -173,6 +186,9 @@ public ResponseEntity> deleteQuery(@PathVariable UUID id) { public ResponseEntity> toggleFavorite(@PathVariable UUID id) { try { log.info("Toggling favorite for query: {}", id); + SavedQuery existing = savedQueryService.getQueryById(id) + .orElseThrow(() -> new IllegalArgumentException("Query not found: " + id)); + accessControlService.assertCanManageConnectionContent(existing.getConnectionId()); SavedQuery updated = savedQueryService.toggleFavorite(id); @@ -206,6 +222,7 @@ public ResponseEntity> toggleFavorite(@PathVariable UUID id) public ResponseEntity> getFavoriteQueries(@PathVariable String connectionId) { try { log.info("Fetching favorite queries for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List queries = savedQueryService.getFavoriteQueries(connectionId); @@ -235,6 +252,7 @@ public ResponseEntity> getQueriesByFolder( @PathVariable String folder) { try { log.info("Fetching queries in folder: {} for connection: {}", folder, connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List queries = savedQueryService.getQueriesByFolder(connectionId, folder); @@ -264,6 +282,7 @@ public ResponseEntity> searchQueries( @RequestParam String q) { try { log.info("Searching queries for connection: {} with term: {}", connectionId, q); + accessControlService.assertCanReadConnectionContent(connectionId); List queries = savedQueryService.searchQueries(connectionId, q); @@ -291,6 +310,7 @@ public ResponseEntity> searchQueries( public ResponseEntity> getFolders(@PathVariable String connectionId) { try { log.info("Fetching folders for connection: {}", connectionId); + accessControlService.assertCanReadConnectionContent(connectionId); List folders = savedQueryService.getFolders(connectionId); diff --git a/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java b/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java index cec3563..c5858df 100644 --- a/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java +++ b/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java @@ -7,6 +7,7 @@ import com.dbaagent.dto.SlowLogSourceConfigResponse; import com.dbaagent.service.SlowLogIngestionService; import com.dbaagent.service.SlowLogSourceConfigService; +import com.dbaagent.service.security.AccessControlService; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.http.HttpStatus; @@ -25,9 +26,11 @@ public class SlowLogSourceController { private final SlowLogSourceConfigService configService; private final SlowLogIngestionService ingestionService; private final BatchIngestionService batchIngestionService; + private final AccessControlService accessControlService; @GetMapping("/{connectionId}") public ResponseEntity getConfig(@PathVariable String connectionId) { + accessControlService.assertCanReadConnectionContent(connectionId); try { SlowLogSourceConfigResponse response = configService.getResponse(connectionId); if (response == null) { @@ -46,6 +49,7 @@ public ResponseEntity getConfig(@PathVariable Strin public ResponseEntity upsertConfig( @RequestBody SlowLogSourceConfigRequest request ) { + accessControlService.assertCanManageConnectionContent(request.getConnectionId()); return ResponseEntity.ok(configService.upsertConfig(request)); } @@ -65,6 +69,8 @@ public ResponseEntity cloneFrom( @PathVariable String targetConnectionId, @PathVariable String sourceConnectionId ) { + accessControlService.assertCanReadConnectionContent(sourceConnectionId); + accessControlService.assertCanManageConnectionContent(targetConnectionId); try { SlowLogSourceConfigResponse cloned = configService.cloneFrom(sourceConnectionId, targetConnectionId); if (cloned == null) { @@ -86,6 +92,7 @@ public ResponseEntity cloneFrom( public ResponseEntity> ingestNow( @RequestBody SlowLogIngestRequest request ) { + accessControlService.assertCanManageConnectionContent(request.getConnectionId()); SlowLogIngestionService.IngestResult result = ingestionService.ingestNowWithDetails(request); if (!result.success()) { return ResponseEntity.ok(Map.of( @@ -109,6 +116,7 @@ public ResponseEntity> ingestNow( public ResponseEntity> startAsyncIngestion( @RequestBody SlowLogIngestRequest request ) { + accessControlService.assertCanManageConnectionContent(request.getConnectionId()); try { // Check if there's already an active job for this connection Optional activeJob = batchIngestionService.getActiveJobForConnection(request.getConnectionId()); @@ -163,6 +171,7 @@ public ResponseEntity getJobStatus(@PathVariable String jo if (status.isEmpty()) { return ResponseEntity.notFound().build(); } + accessControlService.assertCanReadConnectionContent(status.get().connectionId()); return ResponseEntity.ok(status.get()); } catch (NumberFormatException e) { log.warn("Invalid job ID format: {}", jobId); @@ -175,6 +184,7 @@ public ResponseEntity getJobStatus(@PathVariable String jo */ @GetMapping("/ingest/active/{connectionId}") public ResponseEntity getActiveJob(@PathVariable String connectionId) { + accessControlService.assertCanReadConnectionContent(connectionId); Optional job = batchIngestionService.getActiveJobForConnection(connectionId); if (job.isEmpty()) { return ResponseEntity.noContent().build(); @@ -189,6 +199,10 @@ public ResponseEntity getActiveJob(@PathVariable String co public ResponseEntity> cancelJob(@PathVariable String jobId) { try { Long executionId = Long.parseLong(jobId); + Optional status = batchIngestionService.getJobStatus(executionId); + if (status.isPresent()) { + accessControlService.assertCanManageConnectionContent(status.get().connectionId()); + } boolean stopped = batchIngestionService.stopJob(executionId); if (!stopped) { return ResponseEntity.ok(Map.of( @@ -217,6 +231,10 @@ public ResponseEntity> cancelJob(@PathVariable String jobId) public ResponseEntity> resumeJob(@PathVariable String jobId) { try { Long executionId = Long.parseLong(jobId); + Optional status = batchIngestionService.getJobStatus(executionId); + if (status.isPresent()) { + accessControlService.assertCanManageConnectionContent(status.get().connectionId()); + } BatchIngestionStatus job = batchIngestionService.restartJob(executionId); return ResponseEntity.ok(Map.of( "success", true, @@ -247,6 +265,7 @@ public ResponseEntity> getJobHistory( @PathVariable String connectionId, @RequestParam(defaultValue = "10") int limit ) { + accessControlService.assertCanReadConnectionContent(connectionId); List jobs = batchIngestionService.getJobHistory(connectionId, limit); return ResponseEntity.ok(jobs); } diff --git a/backend/src/main/java/com/dbaagent/security/JwtUtil.java b/backend/src/main/java/com/dbaagent/security/JwtUtil.java index 633b408..9a3d206 100644 --- a/backend/src/main/java/com/dbaagent/security/JwtUtil.java +++ b/backend/src/main/java/com/dbaagent/security/JwtUtil.java @@ -15,6 +15,7 @@ import javax.crypto.SecretKey; import java.nio.charset.StandardCharsets; import java.time.Duration; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -30,6 +31,12 @@ public class JwtUtil { @Value("${security.jwt.secret:}") private String jwtSecret; + @Value("${security.auth.enabled:true}") + private boolean authEnabled; + + @Value("${spring.profiles.active:}") + private String activeProfiles; + @Value("${security.session.access-minutes:15}") private long accessTokenMinutes; @@ -37,12 +44,30 @@ public class JwtUtil { @PostConstruct public void initialize() { - if (jwtSecret == null || jwtSecret.isBlank()) { + byte[] secretBytes = jwtSecret == null || jwtSecret.isBlank() + ? new byte[0] + : jwtSecret.getBytes(StandardCharsets.UTF_8); + + boolean prodProfile = Arrays.stream(activeProfiles.split(",")) + .map(String::trim) + .filter(p -> !p.isEmpty()) + .anyMatch(p -> p.equalsIgnoreCase("prod")); + + // Fail closed whenever auth is on or under prod — never ship an ephemeral signing key. + if (secretBytes.length < 32) { + if (authEnabled || prodProfile) { + throw new IllegalStateException( + "SECURITY_JWT_SECRET must be set to at least 32 bytes when auth is enabled " + + "or SPRING_PROFILES_ACTIVE includes prod (got " + + secretBytes.length + " bytes)" + ); + } secretKey = Keys.secretKeyFor(SignatureAlgorithm.HS256); log.warn("JWT secret not configured; generated ephemeral key (tokens will reset on restart)."); - } else { - secretKey = Keys.hmacShaKeyFor(jwtSecret.getBytes(StandardCharsets.UTF_8)); + return; } + + secretKey = Keys.hmacShaKeyFor(secretBytes); } public String extractUsername(String token) { diff --git a/backend/src/main/java/com/dbaagent/service/ActiveQueryService.java b/backend/src/main/java/com/dbaagent/service/ActiveQueryService.java index 82633c4..49c2ff4 100644 --- a/backend/src/main/java/com/dbaagent/service/ActiveQueryService.java +++ b/backend/src/main/java/com/dbaagent/service/ActiveQueryService.java @@ -2,7 +2,9 @@ import com.dbaagent.model.ActiveQuery; import com.dbaagent.model.ConnectionRequest; +import com.dbaagent.provider.DatabaseProviderRegistry; import com.dbaagent.repository.ActiveQueryRepository; +import com.dbaagent.util.SessionKillSupport; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -21,6 +23,7 @@ public class ActiveQueryService { private final ActiveQueryRepository activeQueryRepository; private final CredentialService credentialService; private final ConnectionService connectionService; + private final DatabaseProviderRegistry providerRegistry; /** * Capture current active queries from the database @@ -32,12 +35,12 @@ public List captureActiveQueries(String connectionId) { try { ConnectionRequest connRequest = credentialService.getDecryptedConnection(connectionId); try (Connection conn = connectionService.getConnection(connectionId, connRequest)) { - String dbType = connRequest.getDbType().toUpperCase(); + String dbType = providerRegistry.getCanonicalName(connRequest.getDbType()); List queries; - if ("POSTGRESQL".equals(dbType)) { + if ("postgres".equals(dbType)) { queries = capturePostgreSQLQueries(connectionId, conn); - } else if ("MYSQL".equals(dbType)) { + } else if ("mysql".equals(dbType)) { queries = captureMySQLQueries(connectionId, conn); } else { log.warn("Active query monitoring not supported for database type: {}", dbType); @@ -191,29 +194,33 @@ private List captureMySQLQueries(String connectionId, Connection co */ @Transactional public void killQuery(String connectionId, String pid) { - log.info("Killing query {} on connection {}", pid, connectionId); + long backendPid = SessionKillSupport.requireNumericPid(pid); + log.info("Killing query {} on connection {}", backendPid, connectionId); try { ConnectionRequest connRequest = credentialService.getDecryptedConnection(connectionId); try (Connection conn = connectionService.getConnection(connectionId, connRequest)) { - String dbType = connRequest.getDbType().toUpperCase(); - String killQuery; - - if ("POSTGRESQL".equals(dbType)) { - killQuery = "SELECT pg_terminate_backend(" + pid + ")"; - } else if ("MYSQL".equals(dbType)) { - killQuery = "KILL QUERY " + pid; + String dbType = providerRegistry.getCanonicalName(connRequest.getDbType()); + + if ("postgres".equals(dbType)) { + try (PreparedStatement ps = conn.prepareStatement("SELECT pg_terminate_backend(?)")) { + ps.setLong(1, backendPid); + ps.execute(); + } + } else if ("mysql".equals(dbType)) { + // MySQL KILL is not reliably parameterizable across drivers; pid is digit-only. + try (Statement stmt = conn.createStatement()) { + stmt.execute("KILL QUERY " + backendPid); + } } else { throw new IllegalArgumentException("Kill query not supported for database type: " + dbType); } - - try (Statement stmt = conn.createStatement()) { - stmt.execute(killQuery); - log.info("Successfully killed query {}", pid); - } + log.info("Successfully killed query {}", backendPid); } + } catch (IllegalArgumentException e) { + throw e; } catch (Exception e) { - log.error("Error killing query {}", pid, e); + log.error("Error killing query {}", backendPid, e); throw new RuntimeException("Failed to kill query: " + e.getMessage(), e); } } diff --git a/backend/src/main/java/com/dbaagent/service/DatabaseConfigurationService.java b/backend/src/main/java/com/dbaagent/service/DatabaseConfigurationService.java index 2e728e0..b0d551c 100644 --- a/backend/src/main/java/com/dbaagent/service/DatabaseConfigurationService.java +++ b/backend/src/main/java/com/dbaagent/service/DatabaseConfigurationService.java @@ -434,13 +434,20 @@ public List getPendingRecommendations(String connec ); } + /** + * Load recommendation by id or throw. + */ + public ConfigurationRecommendation requireById(String id) { + return recommendationRepository.findById(id) + .orElseThrow(() -> new IllegalArgumentException("Recommendation not found: " + id)); + } + /** * Mark recommendation as applied */ @Transactional public ConfigurationRecommendation markAsApplied(String id) { - ConfigurationRecommendation recommendation = recommendationRepository.findById(id) - .orElseThrow(() -> new IllegalArgumentException("Recommendation not found: " + id)); + ConfigurationRecommendation recommendation = requireById(id); recommendation.markAsApplied(); return recommendationRepository.save(recommendation); diff --git a/backend/src/main/java/com/dbaagent/service/IndexRecommendationService.java b/backend/src/main/java/com/dbaagent/service/IndexRecommendationService.java index 32dcf4c..f6e35e1 100644 --- a/backend/src/main/java/com/dbaagent/service/IndexRecommendationService.java +++ b/backend/src/main/java/com/dbaagent/service/IndexRecommendationService.java @@ -1006,13 +1006,20 @@ public List getPendingRecommendations(String connecti ); } + /** + * Load recommendation by id or throw. + */ + public IndexRecommendationEntity requireById(String id) { + return recommendationRepository.findById(id) + .orElseThrow(() -> new IllegalArgumentException("Recommendation not found: " + id)); + } + /** * Mark recommendation as applied */ @Transactional public IndexRecommendationEntity markAsApplied(String id) { - IndexRecommendationEntity recommendation = recommendationRepository.findById(id) - .orElseThrow(() -> new IllegalArgumentException("Recommendation not found: " + id)); + IndexRecommendationEntity recommendation = requireById(id); recommendation.markAsApplied(); return recommendationRepository.save(recommendation); diff --git a/backend/src/main/java/com/dbaagent/service/LockContentionService.java b/backend/src/main/java/com/dbaagent/service/LockContentionService.java index 09db8fb..380c65f 100644 --- a/backend/src/main/java/com/dbaagent/service/LockContentionService.java +++ b/backend/src/main/java/com/dbaagent/service/LockContentionService.java @@ -2,7 +2,9 @@ import com.dbaagent.model.ConnectionRequest; import com.dbaagent.model.LockContention; +import com.dbaagent.provider.DatabaseProviderRegistry; import com.dbaagent.repository.LockContentionRepository; +import com.dbaagent.util.SessionKillSupport; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.stereotype.Service; @@ -22,6 +24,7 @@ public class LockContentionService { private final LockContentionRepository lockContentionRepository; private final CredentialService credentialService; private final ConnectionService connectionService; + private final DatabaseProviderRegistry providerRegistry; /** * Detect current lock contentions in the database @@ -33,12 +36,12 @@ public List detectLockContentions(String connectionId) { try { ConnectionRequest connRequest = credentialService.getDecryptedConnection(connectionId); try (Connection conn = connectionService.getConnection(connectionId, connRequest)) { - String dbType = connRequest.getDbType().toUpperCase(); + String dbType = providerRegistry.getCanonicalName(connRequest.getDbType()); List currentContentions; - if ("POSTGRESQL".equals(dbType)) { + if ("postgres".equals(dbType)) { currentContentions = detectPostgreSQLLocks(connectionId, conn); - } else if ("MYSQL".equals(dbType)) { + } else if ("mysql".equals(dbType)) { currentContentions = detectMySQLLocks(connectionId, conn); } else { log.warn("Lock detection not supported for database type: {}", dbType); @@ -289,37 +292,42 @@ private List detectMySQLBlockedFromProcesslist(String connection */ @Transactional public void killBlockingSession(String connectionId, String pid) { - log.info("Killing blocking session {} on connection {}", pid, connectionId); + long backendPid = SessionKillSupport.requireNumericPid(pid); + log.info("Killing blocking session {} on connection {}", backendPid, connectionId); try { ConnectionRequest connRequest = credentialService.getDecryptedConnection(connectionId); try (Connection conn = connectionService.getConnection(connectionId, connRequest)) { - String dbType = connRequest.getDbType().toUpperCase(); - String killQuery; - - if ("POSTGRESQL".equals(dbType)) { - killQuery = "SELECT pg_terminate_backend(" + pid + ")"; - } else if ("MYSQL".equals(dbType)) { - killQuery = "KILL " + pid; + String dbType = providerRegistry.getCanonicalName(connRequest.getDbType()); + + if ("postgres".equals(dbType)) { + try (PreparedStatement ps = conn.prepareStatement("SELECT pg_terminate_backend(?)")) { + ps.setLong(1, backendPid); + ps.execute(); + } + } else if ("mysql".equals(dbType)) { + // MySQL KILL is not reliably parameterizable across drivers; pid is digit-only. + try (Statement stmt = conn.createStatement()) { + stmt.execute("KILL " + backendPid); + } } else { throw new IllegalArgumentException("Kill session not supported for database type: " + dbType); } - try (Statement stmt = conn.createStatement()) { - stmt.execute(killQuery); - log.info("Successfully killed session {}", pid); - - // Mark related contentions as resolved - lockContentionRepository.markResolvedByPid( - connectionId, - pid, - LocalDateTime.now(), - "MANUAL_KILL" - ); - } + log.info("Successfully killed session {}", backendPid); + + // Mark related contentions as resolved (store the original digit string) + lockContentionRepository.markResolvedByPid( + connectionId, + Long.toString(backendPid), + LocalDateTime.now(), + "MANUAL_KILL" + ); } + } catch (IllegalArgumentException e) { + throw e; } catch (Exception e) { - log.error("Error killing session {}", pid, e); + log.error("Error killing session {}", backendPid, e); throw new RuntimeException("Failed to kill session: " + e.getMessage(), e); } } diff --git a/backend/src/main/java/com/dbaagent/service/PlaybookExecutionService.java b/backend/src/main/java/com/dbaagent/service/PlaybookExecutionService.java index c7f44f9..56b9d18 100644 --- a/backend/src/main/java/com/dbaagent/service/PlaybookExecutionService.java +++ b/backend/src/main/java/com/dbaagent/service/PlaybookExecutionService.java @@ -331,8 +331,7 @@ public Map getRunStatistics(String connectionId) { */ @Transactional public void cancelRun(String runId) { - PlaybookRun run = playbookRunRepository.findById(runId) - .orElseThrow(() -> new IllegalArgumentException("Run not found: " + runId)); + PlaybookRun run = requireRunById(runId); if (run.getStatus() == PlaybookRun.RunStatus.RUNNING) { run.setStatus(PlaybookRun.RunStatus.CANCELLED); @@ -340,4 +339,9 @@ public void cancelRun(String runId) { playbookRunRepository.save(run); } } + + public PlaybookRun requireRunById(String runId) { + return playbookRunRepository.findById(runId) + .orElseThrow(() -> new IllegalArgumentException("Run not found: " + runId)); + } } diff --git a/backend/src/main/java/com/dbaagent/service/PlaybookService.java b/backend/src/main/java/com/dbaagent/service/PlaybookService.java index 9adacda..682eabc 100644 --- a/backend/src/main/java/com/dbaagent/service/PlaybookService.java +++ b/backend/src/main/java/com/dbaagent/service/PlaybookService.java @@ -149,13 +149,17 @@ public List getRecentAlerts(String connectionId, int hours) { */ @Transactional public PlaybookAlert acknowledgeAlert(String alertId, String acknowledgedBy) { - PlaybookAlert alert = playbookAlertRepository.findById(alertId) - .orElseThrow(() -> new IllegalArgumentException("Alert not found: " + alertId)); + PlaybookAlert alert = requireAlertById(alertId); alert.acknowledge(acknowledgedBy); return playbookAlertRepository.save(alert); } + public PlaybookAlert requireAlertById(String alertId) { + return playbookAlertRepository.findById(alertId) + .orElseThrow(() -> new IllegalArgumentException("Alert not found: " + alertId)); + } + /** * Get alert statistics for a connection */ diff --git a/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java b/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java new file mode 100644 index 0000000..88ef752 --- /dev/null +++ b/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java @@ -0,0 +1,24 @@ +package com.dbaagent.util; + +/** + * Shared helpers for terminating DB backends/sessions without SQL injection. + * PID/session ids must be numeric; never concatenate unvalidated strings into SQL. + */ +public final class SessionKillSupport { + + private SessionKillSupport() {} + + /** + * Parse a session/backend id. Rejects null, empty, signed, hex, or any non-digit input. + */ + public static long requireNumericPid(String pid) { + if (pid == null || pid.isBlank() || !pid.chars().allMatch(Character::isDigit)) { + throw new IllegalArgumentException("Invalid session id: must be a non-negative integer"); + } + try { + return Long.parseLong(pid); + } catch (NumberFormatException e) { + throw new IllegalArgumentException("Invalid session id: value out of range", e); + } + } +} diff --git a/backend/src/main/resources/application-prod.properties b/backend/src/main/resources/application-prod.properties index d4d056f..baf6572 100644 --- a/backend/src/main/resources/application-prod.properties +++ b/backend/src/main/resources/application-prod.properties @@ -143,10 +143,10 @@ management.opentelemetry.tracing.export.otlp.endpoint=http://localhost:4318/v1/t management.otlp.metrics.export.enabled=false management.metrics.export.otlp.enabled=false -# Actuator Endpoints - Expose metrics for monitoring -management.endpoints.web.exposure.include=health,info,metrics,caches,prometheus -management.endpoint.health.show-details=always -management.endpoint.prometheus.enabled=true +# Actuator — prod: expose health (+ optional authenticated metrics). Never show details anonymously. +management.endpoints.web.exposure.include=health +management.endpoint.health.show-details=when_authorized +management.endpoint.prometheus.enabled=false # Email Configuration for Growth Monitoring Alerts # Configure with environment variables or override in production diff --git a/backend/src/main/resources/application.properties b/backend/src/main/resources/application.properties index 3ef9e6d..635699d 100644 --- a/backend/src/main/resources/application.properties +++ b/backend/src/main/resources/application.properties @@ -279,9 +279,9 @@ spring.ai.retry.on-client-errors=false # with the provider (OpenAiCompatibleEmbeddingProvider / OpenAiCompatibleChatProvider); a knob # here would read as if it still controlled them. -# Actuator Endpoints - Expose metrics for monitoring +# Actuator — health is public for compose/k8s probes; everything else needs auth management.endpoints.web.exposure.include=health,info,metrics,caches,prometheus -management.endpoint.health.show-details=always +management.endpoint.health.show-details=when_authorized management.endpoint.prometheus.enabled=true # Email Configuration for Growth Monitoring Alerts diff --git a/backend/src/test/java/com/dbaagent/security/JwtUtilFailClosedTest.java b/backend/src/test/java/com/dbaagent/security/JwtUtilFailClosedTest.java new file mode 100644 index 0000000..58bfcb4 --- /dev/null +++ b/backend/src/test/java/com/dbaagent/security/JwtUtilFailClosedTest.java @@ -0,0 +1,46 @@ +package com.dbaagent.security; + +import org.junit.jupiter.api.Test; +import org.springframework.test.util.ReflectionTestUtils; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class JwtUtilFailClosedTest { + + @Test + void blankSecretFailsUnderProd() { + JwtUtil jwt = new JwtUtil(); + ReflectionTestUtils.setField(jwt, "jwtSecret", ""); + ReflectionTestUtils.setField(jwt, "authEnabled", false); + ReflectionTestUtils.setField(jwt, "activeProfiles", "prod"); + assertThrows(IllegalStateException.class, jwt::initialize); + } + + @Test + void blankSecretFailsWhenAuthEnabled() { + JwtUtil jwt = new JwtUtil(); + ReflectionTestUtils.setField(jwt, "jwtSecret", "short"); + ReflectionTestUtils.setField(jwt, "authEnabled", true); + ReflectionTestUtils.setField(jwt, "activeProfiles", "dev"); + assertThrows(IllegalStateException.class, jwt::initialize); + } + + @Test + void strongSecretAccepted() { + JwtUtil jwt = new JwtUtil(); + ReflectionTestUtils.setField(jwt, "jwtSecret", "x".repeat(32)); + ReflectionTestUtils.setField(jwt, "authEnabled", true); + ReflectionTestUtils.setField(jwt, "activeProfiles", "prod"); + assertDoesNotThrow(jwt::initialize); + } + + @Test + void ephemeralAllowedOnlyWhenAuthOffAndNotProd() { + JwtUtil jwt = new JwtUtil(); + ReflectionTestUtils.setField(jwt, "jwtSecret", ""); + ReflectionTestUtils.setField(jwt, "authEnabled", false); + ReflectionTestUtils.setField(jwt, "activeProfiles", "dev"); + assertDoesNotThrow(jwt::initialize); + } +} diff --git a/backend/src/test/java/com/dbaagent/util/SessionKillSupportTest.java b/backend/src/test/java/com/dbaagent/util/SessionKillSupportTest.java new file mode 100644 index 0000000..7e2401e --- /dev/null +++ b/backend/src/test/java/com/dbaagent/util/SessionKillSupportTest.java @@ -0,0 +1,25 @@ +package com.dbaagent.util; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class SessionKillSupportTest { + + @Test + void acceptsNumericPid() { + assertEquals(42L, SessionKillSupport.requireNumericPid("42")); + assertEquals(0L, SessionKillSupport.requireNumericPid("0")); + } + + @Test + void rejectsInjectionPayloads() { + assertThrows(IllegalArgumentException.class, () -> SessionKillSupport.requireNumericPid("1); DROP TABLE t;--")); + assertThrows(IllegalArgumentException.class, () -> SessionKillSupport.requireNumericPid("1 OR 1=1")); + assertThrows(IllegalArgumentException.class, () -> SessionKillSupport.requireNumericPid("-1")); + assertThrows(IllegalArgumentException.class, () -> SessionKillSupport.requireNumericPid("0x10")); + assertThrows(IllegalArgumentException.class, () -> SessionKillSupport.requireNumericPid("")); + assertThrows(IllegalArgumentException.class, () -> SessionKillSupport.requireNumericPid(null)); + } +} diff --git a/docker-compose.yml b/docker-compose.yml index b0d04d8..e64c3a4 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,13 +1,18 @@ # ── DeepSQL Self-Hosted Stack ───────────────────────────────────────────────── # Quick start: # 1. cp .env.example .env -# 2. fill in required values in .env -# 3. docker compose up -d --build (or ./scripts/self-host/install.sh) +# 2. fill in required values in .env (or run ./scripts/self-host/install.sh) +# 3. docker compose up -d --build # # There are no prebuilt DeepSQL images and no container registry: `backend`, # `frontend`, and `deepsql-agent` are built from this checkout. The first build # compiles the Java backend and takes several minutes. # +# Network posture (OSS security): only the frontend (:3000) is published on all +# interfaces by default. Postgres, Valkey, backend, and the Agent API bind to +# loopback on the host (127.0.0.1) so a cloud VM is not WAN-open. Compose-network +# DNS (postgres/valkey/backend/deepsql-agent) is unchanged for inter-service traffic. +# # Recommended self-host mode — pgvector locally for RAG storage: # VECTOR_STORE_TYPE=pgvector # AZURE_SEARCH_ENABLED=false @@ -22,14 +27,14 @@ services: environment: POSTGRES_DB: dba_agent POSTGRES_USER: postgres - POSTGRES_PASSWORD: ${DB_PASSWORD:-postgres} + POSTGRES_PASSWORD: ${DB_PASSWORD:?Set DB_PASSWORD in .env (install.sh generates one)} command: > postgres -c shared_preload_libraries=pg_stat_statements -c pg_stat_statements.track=all -c max_connections=200 ports: - - "${DEEPSQL_POSTGRES_PORT:-5432}:5432" + - "127.0.0.1:${DEEPSQL_POSTGRES_PORT:-5432}:5432" volumes: - dba-agent-postgres:/var/lib/postgresql - ./docker/postgres/init:/docker-entrypoint-initdb.d:ro @@ -43,12 +48,18 @@ services: valkey: image: valkey/valkey:9.1.1 restart: unless-stopped + environment: + DEEPSQL_VALKEY_PASSWORD: ${DEEPSQL_VALKEY_PASSWORD:?Set DEEPSQL_VALKEY_PASSWORD in .env} + command: + - valkey-server + - --requirepass + - ${DEEPSQL_VALKEY_PASSWORD:?Set DEEPSQL_VALKEY_PASSWORD in .env} ports: - - "${DEEPSQL_VALKEY_PORT:-6379}:6379" + - "127.0.0.1:${DEEPSQL_VALKEY_PORT:-6379}:6379" volumes: - dba-agent-valkey:/data healthcheck: - test: ["CMD", "valkey-cli", "ping"] + test: ["CMD-SHELL", "valkey-cli -a \"$$DEEPSQL_VALKEY_PASSWORD\" ping | grep -q PONG"] interval: 10s timeout: 3s retries: 5 @@ -69,12 +80,12 @@ services: # Vault DB — points to the postgres service above DB_URL: jdbc:postgresql://postgres:5432/dba_agent?sslmode=disable DB_USERNAME: postgres - DB_PASSWORD: ${DB_PASSWORD:-postgres} + DB_PASSWORD: ${DB_PASSWORD:?Set DB_PASSWORD in .env} # Cache — points to the valkey service above spring.data.redis.host: valkey spring.data.redis.port: "6379" - spring.data.redis.password: ${DEEPSQL_VALKEY_PASSWORD:-} + spring.data.redis.password: ${DEEPSQL_VALKEY_PASSWORD:?Set DEEPSQL_VALKEY_PASSWORD in .env} # Encryption key(s) for the credential vault — required, no baked-in default. # Set ENCRYPTION_KEYS (and ENCRYPTION_KEY_ID if it holds more than one key) in @@ -96,7 +107,8 @@ services: # Automatically set by install.sh for pgvector self-host mode SPRING_AUTOCONFIGURE_EXCLUDE: ${SPRING_AUTOCONFIGURE_EXCLUDE:-} ports: - - "${DEEPSQL_BACKEND_PORT:-8080}:8080" + # Loopback only — public traffic must go through frontend nginx (:3000) + - "127.0.0.1:${DEEPSQL_BACKEND_PORT:-8080}:8080" volumes: - dba-agent-logs:/app/logs healthcheck: @@ -109,6 +121,9 @@ services: # ── DeepSQL Agent (Agent tab, AI dashboards, Slack/CLI agent turns) ─────────── # Fifth container: persona + skills + MCP + profile provisioner. Built from # agent/Dockerfile. Required for the Agent tab and AI dashboard generation. + # In-container bind remains 0.0.0.0 so backend/frontend can reach it on the + # compose bridge; host publish is loopback-only (nginx /agent-api is the + # public path and is gated with auth_request). deepsql-agent: build: context: . @@ -139,8 +154,8 @@ services: HERMES_WEBUI_TRUSTED_PROXY_CIDRS: ${DEEPSQL_AGENT_TRUSTED_PROXY_CIDRS:-10.0.0.0/8,172.16.0.0/12,192.168.0.0/16} HERMES_WEBUI_TRUSTED_AUTH_HEADER: X-Remote-User ports: - - "${DEEPSQL_AGENT_PORT:-8787}:8787" - - "${DEEPSQL_AGENT_PROVISIONER_PORT:-8788}:8788" + - "127.0.0.1:${DEEPSQL_AGENT_PORT:-8787}:8787" + - "127.0.0.1:${DEEPSQL_AGENT_PROVISIONER_PORT:-8788}:8788" volumes: - dba-agent-agent:/var/lib/deepsql-agent healthcheck: diff --git a/docs/oss-ux/OSS_SECURITY_REVIEW.md b/docs/oss-ux/OSS_SECURITY_REVIEW.md index 1a72b0c..f9b682f 100644 --- a/docs/oss-ux/OSS_SECURITY_REVIEW.md +++ b/docs/oss-ux/OSS_SECURITY_REVIEW.md @@ -23,6 +23,8 @@ Auth-on-by-default, signup closed, bootstrap localhost+secret, encrypted vault, Usability agents can keep shipping product fixes; **security Criticals should land before Sunday** (or the launch messaging must be “single-admin, localhost / private network only”). +> **Status (2026-08-13):** S1–S5 implementation landed — kill-session pid validation + prepared SQL; ACL on apply/kill/slow-log/growth/saved-query/playbook/configuration; Hermes default `127.0.0.1`; compose loopback binds + Valkey `requirepass` + Actuator lockdown; JWT fail-closed under prod/auth. See [`SECURITY.md`](../../SECURITY.md). Remaining High/Medium (H4–H9, M*) are still open. + --- ## Severity legend diff --git a/scripts/self-host/install.sh b/scripts/self-host/install.sh index cf7d66a..e9028ef 100755 --- a/scripts/self-host/install.sh +++ b/scripts/self-host/install.sh @@ -16,7 +16,9 @@ require_command() { is_placeholder() { local value="${1:-}" - [[ -z "$value" || "$value" == change-me-* || "$value" == replace-with-* || "$value" == your-* ]] + # "postgres" is the historical compose default — treat as unset so install.sh + # replaces it with a generated secret (OSS security C4). + [[ -z "$value" || "$value" == change-me-* || "$value" == replace-with-* || "$value" == your-* || "$value" == "postgres" ]] } require_env_value() { @@ -340,6 +342,7 @@ set +a generate_secret SECURITY_JWT_SECRET "openssl rand -base64 64 | tr -d '\n'" generate_secret ENCRYPTION_KEY "openssl rand -base64 32 | tr -d '\n'" generate_secret DB_PASSWORD "openssl rand -base64 16 | tr -d '\n'" +generate_secret DEEPSQL_VALKEY_PASSWORD "openssl rand -base64 24 | tr -d '\n'" generate_secret ADMIN_BOOTSTRAP_SECRET "openssl rand -base64 32 | tr -d '\n'" generate_secret AGENT_PROVISION_SECRET "openssl rand -base64 32 | tr -d '\n'" @@ -386,6 +389,7 @@ require_env_value SECURITY_JWT_SECRET require_env_value ENCRYPTION_KEY require_env_value ENCRYPTION_KEY_ID require_env_value DB_PASSWORD +require_env_value DEEPSQL_VALKEY_PASSWORD # Chat is resolved by LlmConfigResolver from DEEPSQL_CHAT_*. AZURE_OPENAI_KEY / # _ENDPOINT / _CHAT_DEPLOYMENT used to be required here; they no longer configure chat. # _CHAT_DEPLOYMENT is read by nothing at all, and _KEY/_ENDPOINT now feed only the diff --git a/scripts/self-host/setup-agent.sh b/scripts/self-host/setup-agent.sh index d80ac20..3d84f82 100755 --- a/scripts/self-host/setup-agent.sh +++ b/scripts/self-host/setup-agent.sh @@ -9,7 +9,7 @@ # product home, wires DeepSQL MCP, and starts the Agent API on :8787 with: # - Python MCP SDK installed in the runtime interpreter # - DeepSQL MCP wired to localhost:8080 with a per-user MCP token -# - Binding 0.0.0.0 (so Docker nginx/backend can reach it, if used) +# - Binding 127.0.0.1 by default (set HERMES_WEBUI_HOST=0.0.0.0 only if needed) # - No agent-side password (DeepSQL's /agent-api proxy has its own session gate) # # Idempotent. Safe to re-run after `git pull` or credential rotation. @@ -29,7 +29,10 @@ WEBUI_DIR="${HERMES_WEBUI_DIR:-$HERMES_HOME/hermes-webui}" AGENT_REPO="${HERMES_AGENT_REPO:-https://github.com/NousResearch/hermes-agent.git}" WEBUI_REPO="${HERMES_WEBUI_REPO:-https://github.com/nesquena/hermes-webui.git}" WEBUI_PORT="${HERMES_WEBUI_PORT:-8787}" -WEBUI_HOST="${HERMES_WEBUI_HOST:-0.0.0.0}" +# Default to loopback so a bare self-host install does not expose the Agent API +# on the WAN (nginx /agent-api already gates via auth_request). Override to +# 0.0.0.0 only when something outside this host must reach Hermes directly. +WEBUI_HOST="${HERMES_WEBUI_HOST:-127.0.0.1}" PID_FILE="${HERMES_HOME}/webui.pid" LOG_FILE="${HERMES_HOME}/logs/webui.log" BACKEND_PORT="${DEEPSQL_BACKEND_PORT:-8080}" diff --git a/scripts/self-host/smoke-test.sh b/scripts/self-host/smoke-test.sh index 3381fe8..575961d 100755 --- a/scripts/self-host/smoke-test.sh +++ b/scripts/self-host/smoke-test.sh @@ -333,5 +333,30 @@ if [[ "$DEEPSQL_SMOKE_AGENT" == "1" ]]; then echo "Agent session: $session_id" fi +# ── Security posture (OSS Criticals C3/C4) ─────────────────────────────────── +# Actuator metrics must not be anonymous; health may stay 200. +actuator_prom="$(curl -sS -o /dev/null -w '%{http_code}' \ + "http://127.0.0.1:${DEEPSQL_BACKEND_PORT:-8080}/api/actuator/prometheus" || echo "000")" +if [[ "$actuator_prom" == "200" ]]; then + echo "Error: /api/actuator/prometheus returned 200 without auth (expected 401)." >&2 + exit 1 +fi +auth_me="$(curl -sS -o /dev/null -w '%{http_code}' \ + "http://127.0.0.1:${DEEPSQL_BACKEND_PORT:-8080}/api/auth/me" || echo "000")" +if [[ "$auth_me" == "200" ]]; then + echo "Error: /api/auth/me returned 200 without cookies (expected 401)." >&2 + exit 1 +fi +# Host publishes for DB/cache/backend/agent must be loopback-only (not 0.0.0.0). +if command -v ss >/dev/null 2>&1; then + open_binds="$(ss -ltn 2>/dev/null | awk '/0\.0\.0\.0:(5432|6379|8080|8787|8788)\s/ {print}' || true)" + if [[ -n "$open_binds" ]]; then + echo "Error: sensitive ports published on 0.0.0.0 (expected 127.0.0.1 only):" >&2 + echo "$open_binds" >&2 + exit 1 + fi +fi +echo "Security smoke checks passed (actuator locked, auth required, no WAN binds on 5432/6379/8080/8787)." + echo "Smoke test passed." echo "Connection ID: ${connection_id}" From 7833f311b2d9533ba626c86c3908b567e5b737a3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 13 Aug 2026 13:08:00 +0000 Subject: [PATCH 2/2] fix(security): restore disclosure policy and fail-closed slow-log ACL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Release review of S1–S5: keep vulnerability-reporting SLAs in SECURITY.md, require ACL before slow-log cancel/resume, document Valkey requirepass, and restrict session kill pids to ASCII digits. Co-authored-by: Venkat SF --- SECURITY.md | 85 +++++++++++++++++-- .../controller/SlowLogSourceController.java | 18 +++- .../com/dbaagent/util/SessionKillSupport.java | 3 +- docs/root/SELF_HOST_GUIDE.md | 6 +- 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 7e3d27f..214dcc4 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,13 +1,84 @@ -# DeepSQL Security +# Security Policy -## Threat model (be honest) +DeepSQL stores database credentials in an encrypted vault, holds an AES-GCM key +whose loss is unrecoverable, and enforces read-only SQL execution as a guardrail. +We treat reports against those paths as our highest priority. + +## Reporting a vulnerability + +Report privately via **Security → Report a vulnerability** on this repository. +Do not open a public issue, and do not describe the problem in a pull request. + +We acknowledge reports within **48 hours**, provide an assessment within +**5 business days**, and aim to ship a fix within **90 days**. We credit +reporters in the published advisory unless you prefer otherwise. + +If a report is time-critical and you have had no acknowledgement within 48 hours, +open a public issue containing no technical detail — just a request that a +maintainer check private reports — and we will pick it up. + +## Supported versions + +The latest tagged release receives security fixes. Older tags do not. + +## In scope + +- Credential-vault encryption and key handling +- Read-only SQL execution enforcement, and any bypass of it +- Authentication, JWT handling, and MCP token authorisation +- The admin bootstrap endpoint +- The dashboard sandbox iframe and its read-only query bridge, including the + public share path +- SSH tunnelling +- Connection access control (IDOR) on connection-scoped APIs +- Reachable dependency vulnerabilities + +## Out of scope + +These are by design, and reporting them will get a courteous decline: + +- Behaviour when `SECURITY_AUTH_ENABLED=false`. This is a development-only + shortcut and is documented as such. +- The hand-written SQL editor's ability to mutate data for a confirming admin. + A DBA tool that cannot run `UPDATE` is not a DBA tool; the guardrail governs + *generated* and *agent-issued* SQL, not a human who has explicitly confirmed. +- The localhost-only bootstrap endpoint when deliberately enabled. +- Anything requiring prior host compromise. +- Missing hardening headers with no demonstrated impact. + +## How fixes are handled + +Fixes are developed in a private fork through GitHub Security Advisories. The +advisory and the patched release are published simultaneously. A vulnerability is +never fixed in a normal public pull request: on a repository anyone can watch, +that commit is a roadmap to the bug for everyone still running the old version. + +## A note on review requirements + +`.github/CODEOWNERS` routes changes under the vault, authentication and +SQL-execution paths to the security owners, so the right people are *required* +reviewers. It cannot, however, require a larger *number* of approvals on those +paths specifically — GitHub carries a single repo-wide approval count. The +two-approval rule on security-critical paths is therefore a maintainer +convention, enforced by reviewers rather than by the platform. Treat a +security-path pull request carrying only one approval as not yet ready. + +## Please do not + +- Test against infrastructure you do not own. +- Include real credentials, API keys, or `ENCRYPTION_KEY` values in a report. + Redact them; we can reproduce from a description. + +--- + +## Threat model (operators) | Posture | Who is trusted | Network | |---------|----------------|---------| | **Private / single-admin** | One admin (or a fully trusted ops team) | Private network, Tailscale, or SSH tunnel. Prefer only `:3000` reachable from clients. | | **Internet multi-user** | Untrusted tenants sharing one install | Requires ACL on every connection-scoped API, hardened compose binds, Actuator lockdown, Hermes behind nginx only, JWT fail-closed — see [`docs/oss-ux/OSS_SECURITY_REVIEW.md`](docs/oss-ux/OSS_SECURITY_REVIEW.md). | -DeepSQL is **not** marketed as multi-tenant SaaS until the Criticals in that review are closed and High findings (SSRF, SET allowlist, share-password defaults) are addressed. +DeepSQL is **not** marketed as multi-tenant SaaS until remaining High findings (SSRF, SET allowlist, share-password defaults, residual controller ACL) from that review are addressed. Criticals S1–S5 (kill SQLi, connection ACL on dangerous APIs, Hermes/compose loopback, Valkey auth, Actuator lockdown, JWT fail-closed) are closed in current `main`. ## Required network layout @@ -18,7 +89,7 @@ Internet / LAN clients :3000 frontend (nginx) ├── /api/* → backend:8080 (auth cookies / JWT) └── /agent-api/* → deepsql-agent:8787 (auth_request → /api/auth/me) - + Host loopback only (not WAN): 127.0.0.1:5432 postgres 127.0.0.1:6379 valkey (--requirepass) @@ -36,7 +107,7 @@ Do **not** publish Postgres, Valkey, backend, or Hermes on `0.0.0.0` on a cloud | `SECURITY_JWT_SECRET` | Session token signing (≥32 bytes). **Required** under `prod` / auth-on — boot fails closed if missing. | | `ENCRYPTION_KEY` / `ENCRYPTION_KEYS` | Vault credential encryption | | `DB_PASSWORD` | Vault Postgres (never leave as `postgres` on a networked host) | -| `DEEPSQL_VALKEY_PASSWORD` | Valkey `--requirepass` | +| `DEEPSQL_VALKEY_PASSWORD` | Valkey `--requirepass` (required by Compose) | | `DEEPSQL_CHAT_*` / embedding keys | LLM | | `AGENT_PROVISION_SECRET` | Backend ↔ agent provisioner | @@ -56,10 +127,6 @@ MCP tokens are **full-account PATs** (same authority as the minting user). Revok A public share token authorizes **read-only SQL** against the dashboard’s connection while `is_public` is true. Treat share URLs like credentials; revoke by deleting the share / flipping `is_public`. Prefer password-protected shares once that control ships. -## Reporting vulnerabilities - -Please report security issues privately to the maintainers (GitHub Security Advisory on [DeepSQLAI/deepsql](https://github.com/DeepSQLAI/deepsql) preferred). Do not open public issues that include exploit details until a fix is available. - ## Explicit non-goals (until complete) - Guaranteeing every legacy controller has connection ACL (track remaining High/Medium in the security review) diff --git a/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java b/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java index c5858df..a155794 100644 --- a/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java +++ b/backend/src/main/java/com/dbaagent/controller/SlowLogSourceController.java @@ -200,9 +200,14 @@ public ResponseEntity> cancelJob(@PathVariable String jobId) try { Long executionId = Long.parseLong(jobId); Optional status = batchIngestionService.getJobStatus(executionId); - if (status.isPresent()) { - accessControlService.assertCanManageConnectionContent(status.get().connectionId()); + // Fail closed: unknown job → 404 (do not call stop without ACL). + if (status.isEmpty()) { + return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of( + "success", false, + "message", "Job not found or already completed" + )); } + accessControlService.assertCanManageConnectionContent(status.get().connectionId()); boolean stopped = batchIngestionService.stopJob(executionId); if (!stopped) { return ResponseEntity.ok(Map.of( @@ -232,9 +237,14 @@ public ResponseEntity> resumeJob(@PathVariable String jobId) try { Long executionId = Long.parseLong(jobId); Optional status = batchIngestionService.getJobStatus(executionId); - if (status.isPresent()) { - accessControlService.assertCanManageConnectionContent(status.get().connectionId()); + // Fail closed: unknown job → 404 (do not restart without ACL). + if (status.isEmpty()) { + return ResponseEntity.status(HttpStatus.NOT_FOUND).body(Map.of( + "success", false, + "message", "Job not found" + )); } + accessControlService.assertCanManageConnectionContent(status.get().connectionId()); BatchIngestionStatus job = batchIngestionService.restartJob(executionId); return ResponseEntity.ok(Map.of( "success", true, diff --git a/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java b/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java index 88ef752..2792565 100644 --- a/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java +++ b/backend/src/main/java/com/dbaagent/util/SessionKillSupport.java @@ -12,7 +12,8 @@ private SessionKillSupport() {} * Parse a session/backend id. Rejects null, empty, signed, hex, or any non-digit input. */ public static long requireNumericPid(String pid) { - if (pid == null || pid.isBlank() || !pid.chars().allMatch(Character::isDigit)) { + // ASCII digits only — avoid Unicode numeric characters that Long.parseLong accepts. + if (pid == null || pid.isBlank() || !pid.matches("[0-9]+")) { throw new IllegalArgumentException("Invalid session id: must be a non-negative integer"); } try { diff --git a/docs/root/SELF_HOST_GUIDE.md b/docs/root/SELF_HOST_GUIDE.md index cfc243f..bd93b9a 100644 --- a/docs/root/SELF_HOST_GUIDE.md +++ b/docs/root/SELF_HOST_GUIDE.md @@ -381,7 +381,7 @@ connections have to be re-initialised. |---|---|---| | `DB_PASSWORD` | [`docker-compose.yml:25,72`](../../docker-compose.yml) | Used for both the postgres container's `POSTGRES_PASSWORD` and the backend's datasource, so the two cannot drift. Changing it after first start does **not** change the password already stored in the volume. | | `DB_URL`, `DB_USERNAME` | `application.properties:52,54` | Compose pins these to its own `postgres` service ([`docker-compose.yml:70-71`](../../docker-compose.yml)) and its values win over `.env`. Only relevant when running the backend outside Compose. | -| `DEEPSQL_VALKEY_PASSWORD` | [`docker-compose.yml:77`](../../docker-compose.yml) → `spring.data.redis.password` | Empty by default. The bundled `valkey` container has no password configured, so setting this alone will break the connection. | +| `DEEPSQL_VALKEY_PASSWORD` | [`docker-compose.yml`](../../docker-compose.yml) → Valkey `--requirepass` and `spring.data.redis.password` | **Required.** Compose fails to start without it. `install.sh` generates a strong value when the placeholder is blank. Backend and Valkey must share the same password. | ### Public URLs — the one that breaks `deepsql login` @@ -424,8 +424,8 @@ serve from. |---|---|---| | `DEEPSQL_FRONTEND_PORT` | 3000 | [`docker-compose.yml:116`](../../docker-compose.yml) | | `DEEPSQL_BACKEND_PORT` | 8080 | [`docker-compose.yml:94`](../../docker-compose.yml) | -| `DEEPSQL_POSTGRES_PORT` | 5432 | [`docker-compose.yml:32`](../../docker-compose.yml) | -| `DEEPSQL_VALKEY_PORT` | 6379 | [`docker-compose.yml:47`](../../docker-compose.yml) | +| `DEEPSQL_POSTGRES_PORT` | 5432 (published as `127.0.0.1:…` only) | [`docker-compose.yml`](../../docker-compose.yml) | +| `DEEPSQL_VALKEY_PORT` | 6379 (published as `127.0.0.1:…` only) | [`docker-compose.yml`](../../docker-compose.yml) | | `CORS_ALLOWED_ORIGINS` | `http://localhost:3000` | `application.properties:87` → [`SecurityConfig.java`](../../backend/src/main/java/com/dbaagent/config/SecurityConfig.java) | ```env