fix(security): authorize the operated-on row, not a caller-supplied id

CLAUDE.md

@@ -1088,6 +1088,25 @@ it against a real database — not a theoretical hardening pass.
   while an allowed origin gets `200` + ACAO — so the annotation never had effect. It
   still reads like an intentional hole to the next person.
 
+- **A guard can be present and still authorize the wrong id.** Twelve endpoints across
+  `CodeScanController` (8), `CompanyKnowledgeController` (2) and `DashboardAlertController` (2)
+  each called `assertCanManageConnectionContent` — so the scanner passed them — but on an id
+  unrelated to what they operated on. The scan endpoints checked a `@RequestParam connectionId`
+  the caller owns while deleting/reading a `@PathVariable sourceId`/`jobId`/`suggestionId`
+  belonging to another tenant; `CompanyKnowledgeController.update` wrapped its guard in
+  `if (entry.getConnectionId() != null)`, so omitting the field skipped it entirely;
+  `DashboardAlertController` authorized `dashboardId` but acted on an `alertId` never bound to
+  it. Verified live: `analyst` (grant on one connection) deleted another tenant's scan source,
+  deleted and overwrote a knowledge entry, and deleted an alert — **200 unpatched, 404 patched**
+  in every case. **Resolve the row's own connection and assert on that** — `CodeScanService`
+  `findConnectionIdForSource/Job/Suggestion`, `CompanyKnowledgeService.findConnectionIdForEntry`,
+  `DashboardAlertService.findDashboardIdForAlert` — never a caller-supplied id. **404 for both
+  "unknown" and "not yours"** so the endpoint is not an existence oracle; **`bulk-decide` checks
+  every id and fails on one that resolves to nothing**. The `ConnectionScopedAuthorizationSafetyTest`
+  `AUTHORIZED` regex is presence-only (does *an* assert appear), not dataflow (does it assert on
+  the *right* id), so it cannot catch this class — the live cross-tenant test is the real guard.
+  See `docs/security/2026-09-16-wrong-id-authorization.md`.
+
 ### MCP & CLI Release Rules
 
 **Whenever you add, rename, or remove an MCP tool or a CLI subcommand, you MUST update all of these in the same commit — they are agent-facing surfaces and drift silently breaks discoverability:**

backend/src/main/java/com/dbaagent/controller/CodeScanController.java

@@ -57,7 +57,7 @@ public ResponseEntity<CodeScanSource> updateFocus(
         @RequestParam("connectionId") String connectionId,
         @RequestBody UpdateFocusRequest body
     ) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageSource(sourceId);
         return ResponseEntity.ok(
             codeScanService.updateFocus(sourceId, body == null ? null : body.focus())
         );
@@ -72,7 +72,7 @@ public ResponseEntity<List<CodeScanSource>> listSources(@RequestParam("connectio
     @DeleteMapping("/sources/{sourceId}")
     public ResponseEntity<Void> deleteSource(@PathVariable String sourceId,
                                              @RequestParam("connectionId") String connectionId) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageSource(sourceId);
         codeScanService.deleteSource(sourceId);
         return ResponseEntity.ok().build();
     }
@@ -86,7 +86,7 @@ public ResponseEntity<CodeScanJob> startScan(
         @RequestParam(value = "focus", required = false) String focus,
         @RequestParam("file") MultipartFile file
     ) throws IOException {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageSource(sourceId);
         return ResponseEntity.ok(
             codeScanService.startScan(sourceId, file, focus, accessControlService.getCurrentUsername())
         );
@@ -95,7 +95,7 @@ public ResponseEntity<CodeScanJob> startScan(
     @GetMapping("/jobs/{jobId}")
     public ResponseEntity<CodeScanJob> getJob(@PathVariable String jobId,
                                               @RequestParam("connectionId") String connectionId) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageJob(jobId);
         return codeScanService.getJob(jobId)
             .map(ResponseEntity::ok)
             .orElseGet(() -> ResponseEntity.notFound().build());
@@ -104,14 +104,14 @@ public ResponseEntity<CodeScanJob> getJob(@PathVariable String jobId,
     @GetMapping("/sources/{sourceId}/jobs")
     public ResponseEntity<List<CodeScanJob>> listJobs(@PathVariable String sourceId,
                                                       @RequestParam("connectionId") String connectionId) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageSource(sourceId);
         return ResponseEntity.ok(codeScanService.recentJobs(sourceId));
     }
 
     @GetMapping(value = "/jobs/{jobId}/stream", produces = MediaType.TEXT_EVENT_STREAM_VALUE)
     public SseEmitter streamJob(@PathVariable String jobId,
                                 @RequestParam("connectionId") String connectionId) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageJob(jobId);
         return codeScanService.subscribeJob(jobId);
     }
 
@@ -137,7 +137,7 @@ public ResponseEntity<CodeKnowledgeSuggestion> decide(
         @RequestParam("connectionId") String connectionId,
         @RequestBody DecideRequest body
     ) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+        assertCanManageSuggestion(suggestionId);
         return ResponseEntity.ok(
             codeScanService.decide(
                 suggestionId,
@@ -153,10 +153,15 @@ public ResponseEntity<Map<String, Object>> bulkDecide(
         @RequestParam("connectionId") String connectionId,
         @RequestBody BulkDecideRequest body
     ) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
         if (body == null || body.ids() == null || body.ids().isEmpty()) {
             return ResponseEntity.badRequest().body(Map.of("error", "ids required"));
         }
+        // Every id must resolve to a connection the caller can manage, and an id that
+        // resolves to nothing fails too — otherwise an unknown id rides into an otherwise
+        // valid batch. connectionId is accepted for wire compatibility but not trusted.
+        for (String suggestionId : body.ids()) {
+            assertCanManageSuggestion(suggestionId);
+        }
         var result = codeScanService.bulkDecide(
             body.ids(),
             body.decision(),
@@ -171,6 +176,25 @@ public ResponseEntity<Map<String, Object>> bulkDecide(
         return ResponseEntity.ok(payload);
     }
 
+    // Resolve the row's own connection and authorise against that — never the caller-supplied
+    // connectionId, which the caller may legitimately own while the id targets another tenant.
+    // 404 for both "no such id" and "not yours", so the endpoint is not an existence oracle,
+    // matching the rule DashboardWorkspaceService.assertCanReadDashboard already follows.
+    private void assertCanManageSource(String sourceId) {
+        accessControlService.assertCanManageConnectionContentOrNotFound(
+            codeScanService.findConnectionIdForSource(sourceId).orElse(null), "Scan source");
+    }
+
+    private void assertCanManageJob(String jobId) {
+        accessControlService.assertCanManageConnectionContentOrNotFound(
+            codeScanService.findConnectionIdForJob(jobId).orElse(null), "Scan job");
+    }
+
+    private void assertCanManageSuggestion(String suggestionId) {
+        accessControlService.assertCanManageConnectionContentOrNotFound(
+            codeScanService.findConnectionIdForSuggestion(suggestionId).orElse(null), "Suggestion");
+    }
+
     private static CodeKnowledgeSuggestion.Status parseStatus(String s) {
         try {
             return CodeKnowledgeSuggestion.Status.valueOf(s.toUpperCase(Locale.ROOT));

backend/src/main/java/com/dbaagent/controller/CompanyKnowledgeController.java

@@ -36,9 +36,11 @@ public ResponseEntity<CompanyKnowledgeEntry> create(@RequestBody CompanyKnowledg
     public ResponseEntity<CompanyKnowledgeEntry> update(
             @PathVariable String entryId,
             @RequestBody CompanyKnowledgeEntry entry) {
-        if (entry.getConnectionId() != null && !entry.getConnectionId().isBlank()) {
-            accessControlService.assertCanManageConnectionContent(entry.getConnectionId());
-        }
+        // Authorise against the stored entry's connection, unconditionally. The old check ran
+        // only when the body carried a connectionId, so omitting that field skipped it and let
+        // any authenticated user edit any tenant's entry. The body's connectionId is never
+        // trusted here; updateEntry already refuses to change it.
+        assertCanManageEntry(entryId);
         if (entry.getCreatedBy() == null || entry.getCreatedBy().isBlank()) {
             entry.setCreatedBy(accessControlService.getCurrentUsername());
         }
@@ -48,9 +50,15 @@ public ResponseEntity<CompanyKnowledgeEntry> update(
     @DeleteMapping("/{entryId}")
     public ResponseEntity<Void> delete(
             @PathVariable String entryId,
-            @RequestParam String connectionId) {
-        accessControlService.assertCanManageConnectionContent(connectionId);
+            @RequestParam(required = false) String connectionId) {
+        // The connectionId param was never compared to the entry being deleted; authorise on
+        // the entry's own connection instead. Accepted for wire compatibility, not trusted.
+        assertCanManageEntry(entryId);
         companyKnowledgeService.deleteEntry(entryId);
         return ResponseEntity.ok().build();
     }
+    private void assertCanManageEntry(String entryId) {
+        accessControlService.assertCanManageConnectionContentOrNotFound(
+            companyKnowledgeService.findConnectionIdForEntry(entryId).orElse(null), "Knowledge entry");
+    }
 }

backend/src/main/java/com/dbaagent/controller/DashboardAlertController.java

@@ -9,6 +9,7 @@
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.springframework.http.HttpStatus;
+import org.springframework.web.server.ResponseStatusException;
 import org.springframework.http.ResponseEntity;
 import org.springframework.web.bind.annotation.*;
 
@@ -68,6 +69,7 @@ public ResponseEntity<Map<String, Object>> update(@PathVariable UUID dashboardId
         try {
             SavedDashboard dashboard = requireDashboard(dashboardId);
             accessControlService.assertCanManageConnectionContent(dashboard.getConnectionId());
+            requireAlertOnDashboard(alertId, dashboardId);
             DashboardAlert updated = alertService.updateAlert(alertId, updates);
             return ResponseEntity.ok(Map.of("success", true, "alert", updated));
         } catch (IllegalArgumentException e) {
@@ -85,6 +87,7 @@ public ResponseEntity<Map<String, Object>> delete(@PathVariable UUID dashboardId
         try {
             SavedDashboard dashboard = requireDashboard(dashboardId);
             accessControlService.assertCanManageConnectionContent(dashboard.getConnectionId());
+            requireAlertOnDashboard(alertId, dashboardId);
             alertService.deleteAlert(alertId);
             return ResponseEntity.ok(Map.of("success", true));
         } catch (org.springframework.web.server.ResponseStatusException e) {
@@ -100,6 +103,16 @@ public ResponseEntity<Map<String, Object>> delete(@PathVariable UUID dashboardId
      * membership gate applies to all of them at once. The connection check stays with
      * each caller because read and write paths need different assertions.
      */
+    // Bind the alertId to the dashboard we just authorised. Authorising the dashboard is only
+    // half the check when a second id rides alongside it: 404 (not 403) for an unknown alert or
+    // one under a different dashboard, so this cannot confirm another dashboard's alert exists.
+    private void requireAlertOnDashboard(UUID alertId, UUID dashboardId) {
+        UUID owner = alertService.findDashboardIdForAlert(alertId).orElse(null);
+        if (owner == null || !owner.equals(dashboardId)) {
+            throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Alert not found");
+        }
+    }
+
     private SavedDashboard requireDashboard(UUID dashboardId) {
         SavedDashboard dashboard = savedDashboardService.getDashboardById(dashboardId)
             .orElseThrow(() -> new IllegalArgumentException("Dashboard not found"));

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

@@ -404,6 +404,17 @@ public CompanyKnowledgeEntry createEntry(CompanyKnowledgeEntry entry) {
         return annotateEntry(saved, schema);
     }
 
+    /**
+     * The connection an entry belongs to, or empty if there is no such entry. The controller
+     * authorises against this — not a caller-supplied connectionId that it never compares to
+     * the entry, and that on the update path was only consulted when the caller chose to send
+     * it, so omitting it skipped the check entirely.
+     */
+    public java.util.Optional<String> findConnectionIdForEntry(String entryId) {
+        return companyKnowledgeEntryRepository.findById(entryId)
+            .map(CompanyKnowledgeEntry::getConnectionId);
+    }
+
     @Transactional
     public CompanyKnowledgeEntry updateEntry(String entryId, CompanyKnowledgeEntry request) {
         validate(request, false);

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

@@ -85,6 +85,17 @@ public void deleteAlert(UUID alertId) {
         alertRepository.deleteById(alertId);
     }
 
+    /**
+     * The dashboard an alert belongs to, or empty if there is no such alert. The controller
+     * authorises the dashboard, then binds the alert to it with this — update/delete took an
+     * alertId beside the dashboardId and acted on the alert without checking it belonged to the
+     * authorised dashboard, so a dashboard you own paired with another tenant's alertId let you
+     * repoint or delete their alert.
+     */
+    public java.util.Optional<UUID> findDashboardIdForAlert(UUID alertId) {
+        return alertRepository.findById(alertId).map(DashboardAlert::getDashboardId);
+    }
+
     private DashboardAlert requireAlert(UUID id) {
         return alertRepository.findById(id)
             .orElseThrow(() -> new IllegalArgumentException("Alert not found with id: " + id));

backend/src/main/java/com/dbaagent/service/codescan/CodeScanService.java

@@ -174,6 +174,30 @@ public List<CodeScanSource> listSources(String connectionId) {
         return sourceRepository.findByConnectionIdAndActiveTrueOrderByCreatedAtDesc(connectionId);
     }
 
+    /**
+     * The connection a scan source belongs to, or empty if there is no such source.
+     *
+     * <p>The controller authorises against this, not against a caller-supplied {@code
+     * connectionId}. Every scan endpoint took a {@code @RequestParam connectionId} beside a
+     * {@code @PathVariable sourceId}/{@code jobId} and asserted on the param — so a caller
+     * passed a connection they own next to another tenant's source id, the assert passed, and
+     * the operation hit a row they had no access to.
+     */
+    public java.util.Optional<String> findConnectionIdForSource(String sourceId) {
+        return sourceRepository.findById(sourceId).map(CodeScanSource::getConnectionId);
+    }
+
+    /** The connection a scan job belongs to, or empty if there is no such job. */
+    public java.util.Optional<String> findConnectionIdForJob(String jobId) {
+        return jobRepository.findById(jobId).map(CodeScanJob::getConnectionId);
+    }
+
+    /** The connection a suggestion belongs to, or empty if there is no such suggestion. */
+    public java.util.Optional<String> findConnectionIdForSuggestion(String suggestionId) {
+        return suggestionRepository.findById(suggestionId)
+            .map(CodeKnowledgeSuggestion::getConnectionId);
+    }
+
     @Transactional
     public void deleteSource(String sourceId) {
         sourceRepository.findById(sourceId).ifPresent(s -> {