0120 — Every request resolves an org envelope before it resolves property scope

• Status: Accepted
• Date: 2026-07-30
• Deciders: Fede
• Related: ADR-0019 (organization model — the envelope this ADR finishes installing), ADR-0024 (work-order identity / displayId), ADR-0027

Context

ADR-0019 decided the shape of multi-tenancy in one sentence (§Decision):

Org-scoping is the outer envelope; existing property-scoping stays alongside as the inner privacy boundary within an org.

The inner boundary was built. The outer envelope was not — not as a thing every request passes through. It exists as a set of helpers (src/lib/platform/auth/org-scope.ts) that a route may call, and most routes don't.

The sweep in #5059 (54 query-channel routes, 31 body-channel, 12 path-segment, each traced into its call stack rather than judged by its imports) fixed ten surfaces that had no visibility check at all, and then stopped at a wall it could not fix in a security patch. This ADR is that wall.

The shape of the gap

Three helpers carry almost all authorization in this codebase, and all three apply property scope only:

// src/lib/platform/auth/scope.ts:25
export function getUserPropertyScope(user: AuthenticatedUser): Set<string> | null {
  if (user.role === 'platform_admin' || user.role === 'org_admin') {
    return null; // null = all properties
  }
  return new Set(user.assignedPropertyIds || []);
}

null means "no filter" to every consumer: scopeByProperty returns the input unchanged, isInScope returns true for any id, and isAccessDenied — the fail-closed gate that replaced ~21 hand-rolled route checks — returns false for every property in the deployment.

For platform_admin that is correct and intended. For org_admin it is a cross-org hole. The helper's own docblock asserts the bound is restored elsewhere ("org_admin's 'all' is bounded to their own org via org-scope.ts"), and ORG_BYPASS_ROLES in org-scope.ts correctly contains only platform_admin. But nothing composes them. The bound is restored only where a route separately calls getUserOrgScope — which most don't. The docblock describes an intention; the type system enforces nothing.

Confirmed cross-org by reading the code, not inferred:

loadWorkOrderDetail is the sharpest illustration, because the comment above it names the exact threat the code then fails to cover:

// SECURITY (ADR-0024 §URL): scope the lookup by the user's assigned properties
// so a cross-org displayId collision can't surface the wrong WO. scope===null
// is admin → pass undefined to skip the filter.
const scope = user ? getUserPropertyScope(user) : null;
const workOrder = await resolveWorkOrderRef(displayId, scope ? [...scope] : undefined);

The author knew cross-org displayId collision was the risk. scope === null for an org admin turns the mitigation off.

Two adjacent gaps the same sweep found

Routes with no auth call at all. POST /api/turnovers contains no requireUser, no getCurrentUserUnchecked, no requireAdminAuth, no scope helper — nothing. Middleware's session gate is the only thing in front of it, and it short-circuits below production. Any authenticated user can mint a turnover against any property in any org. It resolves the tenant against the property's org, with a comment saying so explicitly ("NOT the session org"). The same file's GET is dual-mode session-or-admin-bearer with a long SECURITY comment. The read side was hardened; the write side on the next screen was not, and nothing detected the difference.

Reads with no filter at all. Omitting ?propertyId= on /api/leasing/renewals, /api/maintenance-manuals and /api/dashboard/live returned deployment-wide data before #5059 guarded the filtered path. That is strictly worse than a missing org envelope and deserves naming separately so it isn't read as covered by it.

Why a regex cannot finish this

#5059 shipped property-scope-surface-registry.drift.test.ts, and it is deliberately not the mechanical rule "a route reading propertyId must import filterVisibleProperties" — /api/costs disproves that rule, since its scoping correctly lives in its loader, so the rule would flag a fixed route and be silenced by an unused import in a broken one. What the registry asserts instead is the one thing a regex can honestly check: every route reading a caller-supplied ?propertyId= appears in a registry with a human verdict.

Its stated boundary is the query channel. propertyId also arrives in request bodies and path segments, and those are not machine-detectable. POST /api/simulate/sms is the proof: requireUser only, propertyId read from the body, and it drives the live Clara inbound router — real model calls, real conversation writes, against a property the caller may have no relationship to. The registry provably cannot see it.

So the fix is an envelope every request passes through, plus a tripwire on the thing that is mechanically checkable: whether a route authenticates at all.

Decision

1. Every request resolves an org envelope before it resolves property scope.

Authorization is a composition of two boundaries in a fixed order — org first, property second — and never property alone. The canonical expression already exists in filterVisibleProperties: test-property gate → org scope (ADR-0019) → property scope. That order becomes the rule for every surface, list or single-id, read or write.

2. org_admin means every property IN THAT ORG. Never null-scope.

getUserPropertyScope returning null for org_admin is the defect. null may continue to mean "unbounded" for platform_admin only. For an org admin the effective scope is the intersection:

intersectScopes(propertyScope, orgPropertyIds)   // org-scope.ts
// propertyScope === null → orgPropertyIds   ← this is the line that re-bounds an org admin

The two shapes this takes, both already in the tree and both to be generalized rather than invented:

• List reads — loadScopedTurnovers: getUserPropertyScope ∩ getOrgScopedPropertyIds(properties, getUserOrgScope(user)), computed from a Property[] the route already loaded. Four lines, zero extra reads.
• Single-id reads and all writes — canViewProperty(propertyId, user) (the single-id form of filterVisibleProperties), or the explicit resolvePropertyOrg intersection used by /api/automations/cadences, which additionally resolves the null-caller-org ambiguity with a positive isActivePlatformAdmin check rather than falling through.

Failure modes are fixed and identical to #5059's: a read fails closed as empty (the id is a filter and nothing passes it); a write or detail read fails closed as 404, never 403, with a body byte-identical to a genuinely-missing id — so no endpoint becomes an oracle for which property ids are real in another organization.

A property with no organizationId is out of scope for every org-bounded caller. Property.organizationId is still optional, a leftover of the ADR-0019 backfill window. Unstamped ⇒ invisible to org-bounded callers, reachable only by platform staff. This matches what isPropertyInOrgScope and getOrgScopedPropertyIds already do, and makes an unstamped property a visible operational problem rather than a silent hole.

3. A route that makes no auth call is a build error.

The POST /api/turnovers class is mechanically detectable in a way property scoping is not: the question is not "is this the right check?" (unanswerable by regex) but "is there any check?" (answerable by reading the module). The property-scope-surface-registry.drift.test.ts registry is extended with a second, independent assertion:

Every route.ts under src/app/api that exports a mutating handler (POST/PATCH/PUT/DELETE) must reference at least one recognized authentication entry point — requireUser, getCurrentUserUnchecked, requireAdminAuth, or the cron-secret gate — or appear in an explicit, reasoned exemption list.

Same discipline as the existing registry: a new route fails CI until a human classifies it, and the classification is a reviewable line in the diff that cannot be satisfied by copying an import. SCOPE in that file grows a second paragraph stating this assertion's boundary too: it proves a check exists, never that it is the right check. The existing verdict registry keeps answering that second question.

4. The 21 unverified-legacy routes migrate on a staged plan, not a big-bang sweep.

unverified-legacy is the registry's honest placeholder: "applies at least property scope in the route or its loader, org envelope unconfirmed". Twenty-one routes hold it. They are not all equally risky, and a single 21-route PR would be unreviewable, so they migrate in four stages, each its own PR with fails-on-old-code tests in both directions (a cross-org id fails closed and the partition is never read; the caller's own property still works unchanged):

• Stage 1 — tenant/prospect-facing reads (5): conversations, leasing/availability, leasing/pipeline-members, leasing/tours, tenant-confirmations. Highest PII density, so first.
• Stage 2 — outbound and mass-send surfaces (5): mass-sends, mass-sends/[id], mass-sends/audience-options, vendor-outreach, unknown-caller-notes. These can send; a wrong envelope here reaches a real human, which is why they are not merely a read-scoping problem.
• Stage 3 — aggregates and rollups (4): cost-savings/summary, leasing/stats, vendors/adoption, plus the two org-envelope-gap-held routes (dashboard/stats, leasing/prospects). dashboard/stats keys its cache on scope, so its cache contract changes with it — called out here so that is a planned consequence, not a surprise.
• Stage 4 — OAuth authorize/status pairs (8, four integrations): integrations/{email,google-calendar,google-email,outlook}/*. Held for last and handled together because propertyId also arrives inside the OAuth state blob on their callback siblings — the same trust question through a different channel, and splitting the pair would fix one half of one question.

Each stage narrows its routes' registry verdicts from unverified-legacy to a real one. unverified-legacy is closed to new entries as of this ADR: a new route may not claim it, enforced by the registry test. The stage plan is complete when the verdict has no members and the value is deleted.

Consequences

Committed to:

• Org before property, everywhere. A route that applies property scope alone is a defect once its stage lands, not a style preference.
• getUserPropertyScope stops being sufficient on its own. Reviews should treat a bare call to it in a route as the smell that started this ADR.
• The registry grows from one assertion to two, and the auth-call assertion has an exemption list that will need maintaining. That is deliberate: an exemption someone has to write a reason for is the point.

Easier: a single answer to "may this caller touch this property?" that reviewers can check by eye. New routes get the envelope by default rather than by remembering.

Harder:

• There is no data-layer query for "all properties in org X" today. getProperties takes no org argument and there is no org GSI reader. Every org check therefore either filters a Property[] the caller already loaded (getOrgScopedPropertyIds) or resolves one property (isPropertyInOrgScope). Routes that hold neither will need one, and a genuinely org-indexed read is a follow-up this ADR deliberately does not decide.
• Platform staff paths must stay explicit. The null org scope is ambiguous — it means both "platform staff, intended cross-org reach" and "org-bounded caller whose org didn't resolve". Every site must disambiguate with a positive staff check, as /api/automations/cadences does. Falling through on null reintroduces the hole in a new place.
• PORTFOLIO rollups remain deployment-wide. The snapshot Lambda pre-rolls them and no caller-supplied id reaches them, so they are not this bug — but they are also not org-scoped, and re-scoping them would change every dashboard's numbers. Out of scope here, and named so it is not mistaken for covered.

Follow-up implied, not decided here: the state-blob propertyId in the four integrations/*/callback routes (Stage 4's sibling question); PATCH /api/vendors/memberships/[id], where the row is org-scoped but the submitted propertyIds array is not checked against the caller's scope; and whether /api/simulate/sms should be gated out of production entirely rather than scoped — a product call, since it drives the live router.

Alternatives considered

Make getUserPropertyScope return the org's property ids for org_admin. The obvious fix, and rejected: it needs an async property read inside a function that is synchronous and called on hot paths, and it would silently change the meaning of null for every existing caller — including the platform-staff paths that legitimately depend on it. Worse, it hides the composition. The two boundaries answer different questions and should stay visibly separate at every call site.

A lint rule requiring filterVisibleProperties in any route reading propertyId. Already tried and rejected in #5059, with /api/costs as the disproof: scoping legitimately lives in loaders, so the rule flags correct code and is silenced by an unused import in incorrect code. Kept out for the same reason here.

Middleware that resolves the envelope for every route. Attractive, and wrong at this layer: middleware sees the URL and session but not which of the request's many ids is the property, nor whether the handler is about to read one property or fifty. It would either be a no-op or a false sense of coverage. The envelope belongs where the id is interpreted.

Fix all 21 legacy routes in one PR. Rejected on reviewability. Each route needs two-directional tests and an individually-verified verdict; a 21-route diff gets rubber-stamped, which is how unverified-legacy accumulated in the first place.

Leave org_admin unbounded and rely on operational trust. Today every org admin is a PropFlow-side operator, so the hole has no known exploitation path. Rejected because it is a property of the current customer list, not of the system, and it inverts the moment of discovery: the first customer-side org admin makes it a live cross-tenant data exposure with no code change required.