DeepSQLAI
diff --git a/‎CLAUDE.md‎
Lines changed: 20 additions & 0 deletions b/‎CLAUDE.md‎
Lines changed: 20 additions & 0 deletions
diff --git a/‎backend/src/main/java/com/dbaagent/controller/SetupController.java‎
Lines changed: 24 additions & 3 deletions b/‎backend/src/main/java/com/dbaagent/controller/SetupController.java‎
Lines changed: 24 additions & 3 deletions
diff --git a/‎backend/src/test/java/com/dbaagent/controller/GlobalConfigAuthorizationSafetyTest.java‎
Lines changed: 183 additions & 0 deletions b/‎backend/src/test/java/com/dbaagent/controller/GlobalConfigAuthorizationSafetyTest.java‎
Lines changed: 183 additions & 0 deletions
@@ -839,6 +839,26 @@ it against a real database — not a theoretical hardening pass.
   `noteId`, `taskId`), resolve the owning connection first via that service's
   `getConnectionId(id)` and assert on the result. Do not skip the check because the
   path has no `connectionId` in it.
+- **A global-config controller is invisible to the connection-scoped scanner.**
+  `SetupController` shipped with **no authorization at all** — no `@PreAuthorize`, no
+  `assertCan*` — so any authenticated user (lowest role included) could `POST
+  /setup/llm-config` and repoint the org's LLM endpoint and API key. `LlmConfigResolver`
+  reads the **database tier before the environment tier**, so that write silently
+  overrode a correctly configured install with no restart, sending every chat turn,
+  schema and query result to a host of the caller's choosing; `/setup/llm-config/test`
+  passed the same unvalidated URL into `RestClient.baseUrl` (SSRF → cloud metadata).
+  Its javadoc said "All other endpoints require an authenticated user" — true, and
+  exactly the trap. `ConnectionScopedAuthorizationSafetyTest` could not see it: that
+  scanner keys on a `connectionId` or a `@PathVariable …Id`, and this controller has
+  **neither**, so the suite stayed green over a critical hole. Fixed with a class-level
+  `@PreAuthorize("hasRole('ADMIN')")` plus explicit `permitAll()` on the two genuinely
+  pre-login routes (`/status`, `/initialize`); the wizard itself is already behind
+  `<ProtectedRoute>` and reached only by the bootstrap-created admin, so no flow breaks.
+  `GlobalConfigAuthorizationSafetyTest` now covers this shape. **Its own detector had to
+  be mutation-tested**: the first version used `source.indexOf("@PreAuthorize")`, which
+  the `import` line and a javadoc mention both satisfy, so deleting the real gate left
+  the test green — match an annotation at the start of a line instead. See
+  `docs/security/2026-09-10-setup-controller-authorization.md`.
 - **An endpoint with no connection scope at all is admin-only.**
   `POST /brain/column-values/embed-all` spans every connection, so it carries
   `@PreAuthorize("hasRole('ADMIN')")` — it cannot be authorized against one
 
@@ -8,6 +8,7 @@
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.http.ResponseEntity;
+import org.springframework.security.access.prepost.PreAuthorize;
 import org.springframework.web.bind.annotation.*;
 import org.springframework.web.client.RestClient;
 
@@ -17,14 +18,32 @@
 /**
  * REST API for the first-run onboarding wizard.
  *
- * <p>The {@code GET /setup/status} endpoint is publicly accessible (no auth required)
- * so the frontend can detect first-run before login. All other endpoints require
- * an authenticated user.
+ * <p>{@code GET /setup/status} and {@code POST /setup/initialize} are publicly accessible
+ * (both sit in {@code SecurityConfig}'s permitAll set) because they run before any account
+ * exists: the frontend probes {@code /status} to detect a first run, and {@code /initialize}
+ * performs it. Both are annotated {@code @PreAuthorize("permitAll()")} so the class-level
+ * gate below does not close the onboarding flow.
+ *
+ * <p>Every other endpoint here writes <em>installation-wide</em> configuration and is
+ * admin-only. This class previously carried no authorization at all, and its javadoc said
+ * "All other endpoints require an authenticated user" — true, and exactly the trap:
+ * authentication is not authorization. Any authenticated user, the lowest role included,
+ * could POST {@code /setup/llm-config} and repoint the organization's LLM endpoint and API
+ * key. {@code LlmConfigResolver} reads the database tier before the environment tier, so
+ * that write silently overrode a correctly configured install with no restart, sending every
+ * chat turn, schema and query result to a host of the caller's choosing;
+ * {@code /setup/llm-config/test} issued a server-side request to the same unvalidated URL.
+ *
+ * <p>These settings belong to no single connection, so they cannot be authorized against
+ * one — which is why {@code ConnectionScopedAuthorizationSafetyTest} could not see the gap.
+ * {@code GlobalConfigAuthorizationSafetyTest} covers this shape and fails the build if a
+ * handler here loses its gate.
  */
 @RestController
 @RequestMapping("/setup")
 @RequiredArgsConstructor
 @Slf4j
+@PreAuthorize("hasRole('ADMIN')")
 public class SetupController {
 
     /** Provider id used when the caller does not name one. */
@@ -57,6 +76,7 @@ public class SetupController {
     // ── GET /setup/status ─────────────────────────────────────────────────────
 
     /** Returns setup completion state. Public endpoint — no auth required. */
+    @PreAuthorize("permitAll()")
     @GetMapping("/status")
     public SetupStatusResponse getStatus() {
         boolean hasOrgInfo     = systemConfigService.get("setup.org.name")
@@ -87,6 +107,7 @@ public SetupStatusResponse getStatus() {
      * and returns a JWT so the caller is immediately logged in.
      * Returns 409 if any user already exists (setup already done).
      */
+    @PreAuthorize("permitAll()")
     @PostMapping("/initialize")
     public ResponseEntity<Map<String, Object>> initialize(
             @RequestBody InitializeRequest request) {
 
@@ -0,0 +1,183 @@
+package com.dbaagent.controller;
+
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.ArrayList;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import java.util.stream.Stream;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * Closes the gap {@link ConnectionScopedAuthorizationSafetyTest} structurally cannot cover.
+ *
+ * <p>That scanner only inspects handlers carrying a {@code connectionId} or a
+ * {@code @PathVariable …Id}, because its job is per-connection tenancy. A controller that
+ * writes <em>global</em> configuration has neither, so it is invisible to it — and the suite
+ * still reports green.
+ *
+ * <p>{@code SetupController} lived in exactly that blind spot. It had no authorization of any
+ * kind, so any authenticated user — the lowest role included — could POST
+ * {@code /setup/llm-config} and repoint the organization's LLM endpoint and API key.
+ * {@code LlmConfigResolver} reads the database tier before the environment tier, so that write
+ * silently overrode a correctly configured install with no restart, sending every chat turn,
+ * schema and query result to an attacker-chosen host. {@code /setup/llm-config/test} took the
+ * same unvalidated URL and issued a server-side request to it.
+ *
+ * <p>Its class javadoc read "All other endpoints require an authenticated user" — true, and
+ * precisely the trap: authentication is not authorization. This test encodes the rule CLAUDE.md
+ * already states in prose, that an endpoint with no connection scope at all is admin-only.
+ *
+ * <p>Scanned as source text on purpose, matching {@link CorsAllowlistSafetyTest}: the guarantee
+ * wanted is that no <em>future</em> global-config controller ships unguarded, whatever it is
+ * named, and a text scan needs no database, Redis or LLM credentials to run.
+ */
+class GlobalConfigAuthorizationSafetyTest {
+
+    private static final Path CONTROLLERS = Path.of("src/main/java/com/dbaagent/controller");
+
+    /**
+     * Controllers that write installation-wide configuration — settings that belong to no single
+     * connection and therefore cannot be authorized against one. Every handler in these, apart
+     * from the explicitly public ones below, must be admin-gated.
+     */
+    private static final Set<String> GLOBAL_CONFIG_CONTROLLERS = Set.of("SetupController.java");
+
+    /**
+     * Endpoints that must stay reachable without authentication, with the reason each is safe.
+     *
+     * <p>Both are genuinely pre-login: the frontend calls {@code /setup/status} to detect a
+     * first run before any account exists, and {@code /setup/initialize} performs that first
+     * run. They are also listed in {@code SecurityConfig}'s permitAll set, so gating them here
+     * would break the onboarding flow rather than harden it.
+     */
+    private static final Set<String> PUBLIC_BY_DESIGN = Set.of(
+        "/status",      // pre-login first-run probe; returns no secret (api keys are masked)
+        "/initialize"   // performs the first run itself, before any user exists to authorize
+    );
+
+    /**
+     * Handler mappings only. {@code @RequestMapping} is deliberately excluded: on these
+     * controllers it is the class-level base path, not an endpoint, and counting it produced a
+     * phantom offender ({@code "/setup"}) on the first run of this test.
+     */
+    private static final Pattern MAPPING = Pattern.compile(
+        "@(?:Get|Post|Put|Delete|Patch)Mapping\\s*\\(\\s*(?:value\\s*=\\s*)?\"([^\"]*)\"");
+
+    private static final Pattern CLASS_DECLARATION = Pattern.compile("\\bpublic\\s+class\\b");
+
+    /**
+     * A {@code @PreAuthorize} annotation in annotation position: at the start of a line,
+     * indentation aside. Excludes the {@code import} line and any javadoc mention, both of
+     * which precede the class declaration and would otherwise read as a gate.
+     */
+    private static final Pattern CLASS_LEVEL_PREAUTHORIZE = Pattern.compile(
+        "(?m)^[ \\t]*@PreAuthorize\\s*\\(");
+
+    private static List<Path> globalConfigControllers() throws IOException {
+        try (Stream<Path> files = Files.walk(CONTROLLERS)) {
+            return files.filter(Files::isRegularFile)
+                        .filter(p -> GLOBAL_CONFIG_CONTROLLERS.contains(p.getFileName().toString()))
+                        .toList();
+        }
+    }
+
+    /**
+     * True when a real {@code @PreAuthorize} annotation guards the class itself.
+     *
+     * <p>Matched at the start of a line, and the {@code import} line is excluded explicitly.
+     * A plain {@code source.indexOf("@PreAuthorize")} is not good enough and was wrong here on
+     * the first attempt: the import and a javadoc mention of the annotation both sit above the
+     * class declaration, so deleting the actual gate still left the check returning true. That
+     * was caught by removing the annotation and watching this test stay green — the same
+     * false-negative the existing ConnectionScopedAuthorizationSafetyTest is noted to have.
+     */
+    private static boolean hasClassLevelPreAuthorize(String source) {
+        Matcher declaration = CLASS_DECLARATION.matcher(source);
+        if (!declaration.find()) {
+            return false;
+        }
+        String beforeClass = source.substring(0, declaration.start());
+        return CLASS_LEVEL_PREAUTHORIZE.matcher(beforeClass).find();
+    }
+
+    /** The body of one handler: from its mapping annotation to the start of the next one. */
+    private static String handlerBody(String source, int mappingStart) {
+        Matcher next = MAPPING.matcher(source);
+        int end = source.length();
+        if (next.find(mappingStart + 1)) {
+            end = next.start();
+        }
+        return source.substring(mappingStart, end);
+    }
+
+    @Test
+    void everyGlobalConfigEndpointIsAdminGatedUnlessPublicByDesign() throws IOException {
+        List<String> offenders = new ArrayList<>();
+
+        for (Path file : globalConfigControllers()) {
+            String source = Files.readString(file);
+            boolean classGated = hasClassLevelPreAuthorize(source);
+
+            Matcher mappings = MAPPING.matcher(source);
+            while (mappings.find()) {
+                String route = mappings.group(1);
+                if (PUBLIC_BY_DESIGN.contains(route)) {
+                    continue;
+                }
+                boolean methodGated = handlerBody(source, mappings.start()).contains("@PreAuthorize");
+                if (!classGated && !methodGated) {
+                    offenders.add(file.getFileName() + " \"" + route + "\"");
+                }
+            }
+        }
+
+        assertThat(offenders)
+            .as("""
+                Global-configuration endpoints reachable by any authenticated user.
+
+                These write installation-wide settings, so they cannot be authorized against a \
+                connection and ConnectionScopedAuthorizationSafetyTest cannot see them. Guard the \
+                controller with @PreAuthorize("hasRole('ADMIN')") and keep genuinely pre-login \
+                routes in PUBLIC_BY_DESIGN with a reason.
+
+                Unguarded: %s""".formatted(offenders))
+            .isEmpty();
+    }
+
+    /**
+     * The public exemptions are only safe while they stay pre-login. If {@code /status} or
+     * {@code /initialize} ever stops being listed in {@code SecurityConfig}'s permitAll set, the
+     * exemption above is masking a real gate rather than reflecting one, so fail and force a
+     * re-read of both files together.
+     */
+    @Test
+    void publicByDesignRoutesAreStillPermitAllInSecurityConfig() throws IOException {
+        String securityConfig = Files.readString(
+            Path.of("src/main/java/com/dbaagent/config/SecurityConfig.java"));
+
+        Set<String> missing = new LinkedHashSet<>();
+        for (String route : PUBLIC_BY_DESIGN) {
+            if (!securityConfig.contains("\"/setup" + route + "\"")) {
+                missing.add("/setup" + route);
+            }
+        }
+
+        assertThat(missing)
+            .as("""
+                PUBLIC_BY_DESIGN exempts these from the admin gate because SecurityConfig \
+                permits them without authentication. They are no longer in that permitAll list, \
+                so the exemption no longer describes reality — either restore them there or \
+                drop them from PUBLIC_BY_DESIGN so they get gated.
+
+                No longer permitAll: %s""".formatted(missing))
+            .isEmpty();
+    }
+}