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

[notSumit25]

The problem

Twelve endpoints each called assertCanManageConnectionContent — so ConnectionScopedAuthorizationSafetyTest passed them — but the check verified a different id than the one the endpoint operated on.

Shape 1 — check the connection, operate on an unrelated row. Every scan endpoint:

@DeleteMapping("/sources/{sourceId}")
public ResponseEntity<Void> deleteSource(@PathVariable String sourceId,
                                         @RequestParam("connectionId") String connectionId) {
    accessControlService.assertCanManageConnectionContent(connectionId);  // checks connectionId
    codeScanService.deleteSource(sourceId);                               // deletes sourceId
}

The caller controls both — pass a connection you own (check passes) + another tenant's sourceId (findById has no ownership filter).

Shape 2 — make the guard conditional on an attacker-controlled field. CompanyKnowledgeController.update guarded only if (entry.getConnectionId() != null), so omitting the field skips the check and overwrites any tenant's entry.

Shape 3 — two path variables. DashboardAlertController authorized dashboardId correctly but acted on an alertId never bound to that dashboard — pair a dashboard you own with another tenant's alertId.

Controller	Endpoints	Unchecked id
CodeScanController	8 (focus, delete, scan, getJob, listJobs, stream, decide, bulk-decide)	sourceId/jobId/suggestionId
CompanyKnowledgeController	PUT/DELETE /{entryId}	entryId
DashboardAlertController	PUT/DELETE /{alertId}	alertId

Reproduced live, not inferred

Two tenants: analyst has a grant on Demo Shop only; admin owns QA Vault Copy. Victim rows planted on QA Vault Copy, then analyst attacked each with their own connection/dashboard + the victim's row id:

Attack	Unpatched	Patched
Delete another tenant's scan source	200, active t→f	404, survives
Delete another tenant's knowledge entry	200, row 1→0	404, survives
Overwrite entry via PUT with no connectionId	200, title → "PWNED by analyst"	404, unchanged
Delete alert via own dashboard + their alertId	200, alert 1→0	404, survives

Legitimate access unchanged (also verified live): analyst deletes a source on their own granted connection (200); admin reads QA Vault Copy's rows (200).

The fix

Resolve the row's own connection and authorize against that:

• CodeScanService.findConnectionIdForSource/Job/Suggestion
• CompanyKnowledgeService.findConnectionIdForEntry (asserts unconditionally now)
• DashboardAlertService.findDashboardIdForAlert (binds the alert to the authorized dashboard)

404, not 403, for both "unknown" and "not yours" — not an existence oracle. bulk-decide checks every id and fails on one that resolves to nothing, so an unknown id can't ride into a valid batch. connectionId params still accepted for wire compat, ignored for the check.

Why the scanner missed it

ConnectionScopedAuthorizationSafetyTest's AUTHORIZED check is presence-only — does an assert appear in the handler, not which id it verifies. All twelve contained an assert. A general dataflow scanner is a much larger undertaking than this fix; the durable guard is the live cross-tenant test (the standard the audit itself used). The safety suite still passes because the new helpers contain assertCan....

Verification

Step	Result
Live cross-tenant attacks, 4 variants	200 unpatched → 404 patched, every one
Legitimate access (own connection; admin on own rows)	200, unchanged
Backend suites	32 tests, 0 failures
mvn compile	clean

DB restored: both password hashes byte-identical, all planted rows removed (tables back to empty).

Residual work (deliberately not here)

• DashboardAlertController returns 500, not 404, for a non-existent dashboardId — requireDashboard throws IllegalArgumentException, and the delete handler maps only ResponseStatusException + generic Exception. Surfaced by this QA but pre-existing and separate; deserves its own fix (map to 404, as the update handler already does).
• A dataflow-aware authorization scanner would catch this class structurally — worth doing, but research-shaped rather than a fix.

Write-up: docs/security/2026-09-16-wrong-id-authorization.md

🤖 Generated with Claude Code

[notSumit25]

Independent hands-on QA — 11/11 PASS, including coverage the implementation testing skipped

This round deliberately went beyond re-running the four deletes I proved during implementation. It targeted the endpoints and claims I had not individually verified live: the read/write scan endpoints, decide, and the bulk-decide atomicity claim.

Cross-tenant endpoints not previously exploited (analyst attacks QA Vault Copy via their own connection)

Endpoint	Unpatched	Patched
PUT /sources/{id}/focus	200	404
GET /jobs/{id}	200	404
GET /sources/{id}/jobs	200	404
POST /suggestions/{id}/decide	200, status→APPROVED, response named connectionId: 312e... (a connection analyst has no grant on)	404

S5 — bulk-decide atomicity (the claim I made in code but had never run)

Batch mixing analyst's own suggestion id + a victim id:

	HTTP	mine	victim
Unpatched	200	APPROVED	APPROVED
Patched	404	PENDING	PENDING

The victim id poisons the whole batch and it fails before applying any decision — analyst's own suggestion stays PENDING too. Atomic-fail, not fail-after-partial. This is the "an id that resolves to nothing/elsewhere fails too" property, proven live.

All 11 scenarios

ID	Scenario	Result
S1	focus write cross-tenant	PASS (200→404)
S2	getJob read cross-tenant	PASS (200→404)
S3	listJobs read cross-tenant	PASS (200→404)
S4	decide cross-tenant	PASS (200 APPROVED→404)
S5	bulk-decide mixed batch	PASS (atomic 404, no partial)
S6	non-existent ids	PASS (404, no 500/leak)
S7	happy path (own row)	PASS (200, deleted)
S8	frontend contract (connectionId param still accepted)	PASS (200)
S9	non-existent dashboardId → 500 on BOTH branches	PASS (pre-existing, not introduced)
S10	idempotent blocked delete	PASS (404 ×2, no change)
S11	state consistency (PUT→DB)	PASS

One honest test-harness correction

S4 first showed unpatched=400, which looked wrong. Investigating: my planted suggestion was missing target_object, so the applier rejected it before auth was even the deciding factor — a bad-test-data 400, not the vulnerability. After setting a valid target_object, unpatched returned 200 + APPROVED (real cross-tenant write) and patched 404. I fixed the fixture and re-ran rather than accepting the misleading status.

S9 confirms the residual bug is pre-existing

Non-existent dashboardId returns 500 on both my branch and main — identical behaviour, so the 500-not-404 issue flagged in the PR is not introduced here. Still worth its own fix.

Environment

• Health via logs + real requests; patched (:8096) vs unpatched (:8080), same live DB.
• DB restored: both password hashes byte-identical, all planted rows removed (5 tables back to 0).

Verdict: READY — zero blocking issues. Every one of the 12 endpoints blocks cross-tenant access (404) while preserving legitimate access (200), bulk-decide fails atomically, and no regression to the frontend contract.