Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,42 @@ it against a real database — not a theoretical hardening pass.
`POST /users/admin/reset` on every install that had run `setup-agent.sh`, since that
mints an admin MCP token on each run.

### Endpoint Authorization Rules

- **Authentication is not authorization.** `SecurityConfig` only asserts
`.anyRequest().authenticated()` and `JwtAuthenticationFilter` only resolves a
principal — neither looks at a `connectionId`. Connections are **private per user**
(`ConnectionAccessService.resolveAccess` keys on `ownerUsername` plus an explicit
grant table), so any endpoint taking a caller-supplied `connectionId` **must** call
`accessControlService.assertCanReadConnectionContent` (reads) or
`assertCanManageConnectionContent` (writes) itself. There is no filter, interceptor
or aspect that does this for you.
- **`BrainController` shipped with 93 of its 116 endpoints unguarded.** Only the first
~15 (`/understanding`, `/notes/*`, `/tasks/*`, `/key-columns/*`,
`/inferred-relationships/*`) had the check; every later "Phase" block did not — so an
authenticated user could pass someone else's connection id to
`/brain/health-scores/{id}`, `/brain/data-sensitivity/{id}` (which names the PII
columns), `/brain/cost-attribution/{id}`, `/brain/ml-overview/{id}` and ~90 more and
read that user's database intelligence. All 116 are now guarded, and
`BrainControllerAuthorizationSafetyTest` fails the build if a new one is not. The
misses clustered by **when a section was written**, not by read/write semantics —
when adding a controller section, guard it as you write it.
- **When the path carries some other id** (`simulationId`, `experimentId`, `patternId`,
`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.
- **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
connection's grants. `@EnableMethodSecurity(prePostEnabled = true)` is on in
`SecurityConfig`, so `@PreAuthorize` is live.
- **Assert inside the `try`, and rethrow `ResponseStatusException` before the
catch-all.** Every handler in `BrainController` ends with a
`catch (Exception) -> 500`; without the earlier
`catch (ResponseStatusException e) { throw e; }` a 403 is swallowed and reported as a
server error, so a client cannot tell "not yours" from "broken". The safety test
asserts this too.

### 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:**
Expand Down
Loading
Loading