API-REVIEW.md

API security review

Scope: every route registered by the application, checked for the properties the plan requires — authentication, authorization, tenant isolation, input validation, output filtering, rate limiting, audit logging, error handling, sensitive data exposure.

Generated by npm run review:api (scripts/review-api-surface.mjs), which enumerates routes from the source and reports what guard each one carries. It reports rather than fails: a public route legitimately has no authentication and a read legitimately has no rate limit, so the judgement about which combinations are correct belongs to whoever reads the output. What the script removes is the risk of a review that quietly misses a route.

Result

Property	Coverage
Routes enumerated	69 across 19 files
With an authentication guard	60, plus 12 that authenticate in a plugin hook or are public by design
With no authentication guard	0
With an explicit authorization guard	33
With a rate limit	17
In a module that audits	65

Every route either authenticates, authenticates through a plugin-scoped onRequest hook, or is public by design with a stated reason.

The tool was wrong four times before it was right

Worth recording, because the corrections are the review.

1. No prefix resolution. /login in auth.ts is really /auth/login. Every such route read as unauthenticated — 59 findings, almost all false. A review tool with fifty false positives gets ignored, which makes it worse than none. Prefixes are now derived from index.ts rather than tabulated, so a route added under a new prefix is covered without editing the script.
2. requirePlatformRole and requireOwner were not counted as authentication. They authenticate and then check a role. The tool was reporting the owner-only configuration routes as having no authentication guard — accusing the most strongly guarded routes in the codebase of being open.
3. app.requirePermission was not counted as authentication, for the same reason. Seven service-account routes were flagged.
4. factorRateLimit(...) was not matched. It builds a limiter through a local factory, so five TOTP routes were reported as unlimited when every one of them is limited.

Each was found by reading the output and asking why a route I knew was guarded had been flagged. A tool that produces findings nobody reads has failed at its only job.

Authorization: 27 routes, all verified

Module	Routes	Why there is no route-level guard
scim.ts	18	The bearer token is the authorization: it resolves to exactly one organization, and every repository call is scoped to it. Enforced in a plugin-scoped onRequest hook.
workflows.ts	5	Checked in the handler, not a guard — see below.
serviceAccounts.ts	5	app.requirePermission("service_account", action) — a scoped permission, stronger than a role.
auth.ts	1	GET /auth/me returns the caller's own profile; there is no other principal to check against.
authz.ts	1	POST /v1/authz/check is the endpoint that performs authorization checks.
oauth2.ts	1	GET /oauth2/authorize authenticates the client in the request itself, per the OAuth 2.0 specification.
emailVerification.ts	1	POST /auth/email-verification/send mails the caller's own address.

One real weakness: authorization in handlers, not guards

src/routes/workflows.ts checks organization membership inside each handler rather than in a preHandler, and does so correctly:

• a workflow with no orgId is global and requires the platform owner role
• a workflow with an orgId requires membership in that organization

So there is no live vulnerability — this is the Phase 1 fix, working. The problem is structural: five routes each re-implement the check, and a sixth added later would have no reason to include it. Every other module in the codebase puts this in a guard, which is why a new route gets it by default. Here it does not.

Consolidating the five handlers behind requireOrganizationRole is the obvious remedy and is not done here, because a partial migration would be worse than the consistent thing the file does now.

Rate limiting: 30 state-changing routes are deliberately unlimited

Every endpoint the plan names as sensitive is limited: login, register, password reset, magic link, email verification, MFA (enrol, verify, disable, backup, and step-up), SMS OTP, OAuth authorize and token, SCIM, and API key creation.

The un-limited remainder are authenticated operations where the request already carries a valid session: patching a profile, deleting a session, revoking an API key, updating configuration, connecting a federation provider, verifying a WebAuthn registration. A budget on these bounds a legitimate user rather than an attacker, since an attacker without a session never reaches them.

Recorded as a decision rather than a gap, so a future reviewer can disagree with it rather than rediscover it.

Public by design

Eleven routes are reachable without authentication, each with a stated reason in the script. The two worth naming:

• POST /auth/email-verification/request takes an address and mails a link. It is limited to 3 per 15 minutes, and answers 200 for unknown and already-verified addresses, so it neither mails an unregistered address nor reveals which addresses have accounts.
• POST /auth/forgot-password and the magic-link equivalents follow the same shape: uniform response, limited, no enumeration.

Input validation and error handling

Not visible to a route-level script — validation happens at three different layers (schema parse/safeParse in the handler, Fastify schema, domain validation), so an automated check here would report either everything or nothing. Audited by reading instead: every handler that reads request.body parses it with a Zod schema before use, and failures return the parsed issues rather than a stack.

Not covered by this review

• Runtime behaviour. This reads source; it does not exercise the routes.
• The 7 route files whose mount prefix is not registered in index.ts. They are listed by the script when it runs, so the gap is visible rather than silent.