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

Twelve endpoints each called assertCanManageConnectionContent — so the safety
scanner passed them — but on an id unrelated to what they operated on.

CodeScanController's 8 scan endpoints checked a @RequestParam connectionId the
caller owns while acting on a @PathVariable sourceId/jobId/suggestionId
belonging to another tenant. CompanyKnowledgeController.update wrapped its guard
in `if (entry.getConnectionId() != null)`, so omitting the field skipped the
check entirely and let any authenticated user overwrite any entry. Its delete
checked a @RequestParam never compared to the entry. DashboardAlertController
authorized dashboardId but then updated/deleted an alertId never bound to that
dashboard.

Reproduced live against the running stack, not inferred. analyst (a grant on
one connection only) attacked rows on a connection they have no access to, by
passing their own connection/dashboard beside the victim's row id:

  delete another tenant's scan source     200 unpatched -> 404 patched
  delete another tenant's knowledge entry 200 unpatched -> 404 patched
  overwrite an entry via PUT (no connId)  200 unpatched -> 404 patched
  delete an alert via own dashboard + id  200 unpatched -> 404 patched

Legitimate access is unchanged: analyst deletes a source on their own granted
connection (200), admin reads their own connection's rows (200).

The fix resolves each row's own connection and authorizes against 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 connectionId params are still accepted for wire compatibility but ignored.

The ConnectionScopedAuthorizationSafetyTest AUTHORIZED check 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, and the
suite still passes because the new helpers contain assertCan.

32 tests green, compile clean. Both users' password hashes and every planted
row were restored to the prior state.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: notSumit25

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 -> {