fix(security): close OSS Criticals S1–S5

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 <venkatesh.sakamuri@stayflexi.com>

Author: cursoragent

.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 ───────────────────────────────────────────────────────

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

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",

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<List<ActiveQuery>> captureActiveQueries(@PathVariable String connectionId) {
         log.info("API: Capturing active queries for connection {}", connectionId);
         try {
+            accessControlService.assertCanManageConnectionContent(connectionId);
             List<ActiveQuery> queries = activeQueryService.captureActiveQueries(connectionId);
             return ResponseEntity.ok(queries);
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -45,6 +48,7 @@ public ResponseEntity<List<ActiveQuery>> captureActiveQueries(@PathVariable Stri
     public ResponseEntity<List<ActiveQuery>> getLatestQueries(@PathVariable String connectionId) {
         log.info("API: Getting latest queries for connection {}", connectionId);
         try {
+            accessControlService.assertCanReadConnectionContent(connectionId);
             List<ActiveQuery> queries = activeQueryService.getLatestQueries(connectionId);
             return ResponseEntity.ok(queries);
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -66,6 +70,7 @@ public ResponseEntity<List<ActiveQuery>> getQueriesByFilter(
             @RequestParam String value) {
         log.info("API: Getting queries for connection {} filtered by {}={}", connectionId, type, value);
         try {
+            accessControlService.assertCanReadConnectionContent(connectionId);
             List<ActiveQuery> queries = activeQueryService.getQueriesByFilter(connectionId, type, value);
             return ResponseEntity.ok(queries);
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -84,6 +89,7 @@ public ResponseEntity<List<ActiveQuery>> getQueriesByFilter(
     public ResponseEntity<Map<String, Object>> getStatistics(@PathVariable String connectionId) {
         log.info("API: Getting query statistics for connection {}", connectionId);
         try {
+            accessControlService.assertCanReadConnectionContent(connectionId);
             Map<String, Object> stats = activeQueryService.getStatistics(connectionId);
             return ResponseEntity.ok(stats);
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -102,6 +108,7 @@ public ResponseEntity<Map<String, Object>> getStatistics(@PathVariable String co
     public ResponseEntity<Map<String, List<String>>> getFilterOptions(@PathVariable String connectionId) {
         log.info("API: Getting filter options for connection {}", connectionId);
         try {
+            accessControlService.assertCanReadConnectionContent(connectionId);
             Map<String, List<String>> options = activeQueryService.getFilterOptions(connectionId);
             return ResponseEntity.ok(options);
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -122,11 +129,17 @@ public ResponseEntity<Map<String, String>> 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<Map<String, String>> killQuery(
     public ResponseEntity<Map<String, Object>> 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",

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,13 +22,15 @@
 public class ConfigurationController {
 
     private final DatabaseConfigurationService configurationService;
+    private final AccessControlService accessControlService;
 
     /**
      * Analyze database configuration and generate tuning recommendations
      */
     @PostMapping("/analyze/{connectionId}")
     public ResponseEntity<AnalyzeResponse> analyzeConfiguration(@PathVariable String connectionId) {
         log.info("Analyzing configuration for connection: {}", connectionId);
+        accessControlService.assertCanManageConnectionContent(connectionId);
 
         try {
             List<ConfigurationRecommendation> recommendations = configurationService.analyzeConfiguration(connectionId);
@@ -59,6 +62,7 @@ public ResponseEntity<AnalyzeResponse> analyzeConfiguration(@PathVariable String
     @GetMapping("/{connectionId}")
     public ResponseEntity<List<ConfigurationRecommendation>> getRecommendations(@PathVariable String connectionId) {
         log.info("Fetching configuration recommendations for connection: {}", connectionId);
+        accessControlService.assertCanReadConnectionContent(connectionId);
 
         List<ConfigurationRecommendation> recommendations = configurationService.getRecommendations(connectionId);
         return ResponseEntity.ok(recommendations);
@@ -70,6 +74,7 @@ public ResponseEntity<List<ConfigurationRecommendation>> getRecommendations(@Pat
     @GetMapping("/pending/{connectionId}")
     public ResponseEntity<List<ConfigurationRecommendation>> getPendingRecommendations(@PathVariable String connectionId) {
         log.info("Fetching pending configuration recommendations for connection: {}", connectionId);
+        accessControlService.assertCanReadConnectionContent(connectionId);
 
         List<ConfigurationRecommendation> recommendations = configurationService.getPendingRecommendations(connectionId);
         return ResponseEntity.ok(recommendations);
@@ -83,6 +88,8 @@ public ResponseEntity<ConfigurationRecommendation> 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<ConfigurationRecommendation> 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<Map<String, String>> 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) {