RFC: Tenant-Isolation Enforcement

Replace ~90 near-identical cross-tenant security point-fixes with three base-layer enforcements that scope tenancy by default — and keep the hole from reopening tomorrow.

Owner: Ross RasmussenCreated: 2026-06-16

💬 Feedback wanted — draft. A GitHub Discussion (Ideas) will be linked here before we commit. Open questions are in Risks & open questions.

TL;DR

🅰 The flood

119 auto-generated PRs, ~90 the same bug

The rampcullis scanner found cross-tenant IDOR everywhere — each PR scopes one query/write to the caller's vendor. Merging 90+ near-identical fixes is slow and every one risks the vendor_id = 0 self-tenant regression.

🅱 The insight

Tenancy is a cross-cutting concern, enforced per-endpoint

Today isolation is opt-in: each route must remember to scope. A missing scope is a silent hole. Point-fixes patch today's holes; they don't stop the 81st.

🅲 The fix

Triage by ROI — systematize the big/recurring, merge the rest

Close out the ~90 cross-tenant IDOR with 2–3 default-on base-layer controls; merge the small one-off buckets as-is. ~2–4 changesets + a handful of merges, not 120 — and each superseded PR's test is banked as proof the layer closes it.

Default-on means no per-endpoint code: ~90 forgettable hand-scopes become 3 listeners; only the rare cross-tenant exception opts out.

Current state — what rampcullis produced

119 open PRs (label rampcullis), AI-generated, no human in the plan loop. By primary vuln class:

Class	PRs	What it is
cross-tenant IDOR	91	a query/write not scoped to the caller's vendor
mass-assignment	7	client body sets vendor_id / immutable columns
missing-authn	5	endpoint reachable unauthenticated
cron-auth	5	EasyCron endpoint missing shared-secret
broken-access-control	5	authenticated but no role gate
secret-exposure / weak-crypto / other	6	irreducible point-fixes

Totals to 119 (a handful of low-risk one-offs are already being merged individually — see Harvesting). The cross-tenant 91 split ~61 writes / ~30 reads (title-based estimate). That split is the whole story: a query-scoping mechanism fixes reads; it cannot stop a cross-tenant write (UPDATE/DELETE/INSERT against another tenant's row).

What's broken about merging all 90

🐌 90 near-identical merges

Each scopes one call site. Untenable to review/soak individually; Neon caps previews at 10.

💣 Every fix is a vendor-0 trap

The canonical bug is if vendor_id: truthiness — vendor_id = 0 is the self-tenant. Dev seed (vendor 1) hides it, so 90 chances to reintroduce it.

🔁 Zero future prevention

Patching today's holes does nothing about the next endpoint a dev writes without a scope. The scanner will just find more.

The decision: systematize or merge?

Not every bucket deserves a systemic control. One rule sorts the flood:

Build a systemic control only when the bucket is large, OR the class is structurally recurring AND the control is cheap. Otherwise merge the handful as-is — and revisit only if it keeps cropping up.

Bucket	~PRs	Control (cost)	Disposition
Cross-tenant read IDOR	~30	L1 read default (M / med)	Systematize — big bucket
Cross-tenant write IDOR	~61	L2 write guard (XL / high)	Systematize — far too many to hand-merge
Mass-assignment	~7	L3 bind allowlist (S / low)	Systematize — cheap + structurally recurring
Missing-authn / cron-auth	~10	L4 default-deny auth (M / high)	Team call — recurring, but M/high effort
Broken-access (RBAC)	~5	apply existing @requires_access_level (S / low)	Merge as-is — applying a decorator we already have
Secret / crypto / data-leak	~6	—	Merge as-is — genuine one-offs

Net: ~2–4 systemic changesets + ~11–28 individual merges — not 120. The ~90 cross-tenant IDOR get closed out systemically (volume + recurrence earn it); the small one-off buckets merge as-is.

"Revisit if it recurs" has one exception — fail-open-by-default classes. Missing-authn and mass-assignment fail open: every new route / write endpoint is a fresh silent hole, so they meet the "keeps cropping up" bar in advance, not in hindsight. Mass-assignment's guard is cheap → easy yes. Default-deny-auth is M/high → the one genuine judgment call to settle at sign-off.

How the systematized layers work

The "Systematize" rows above (plus the L4 team call) are default-on base-layer controls — one change each, no per-route code:

flowchart LR
    REQ["HTTP request"] --> AUTH["Layer 4: default-deny auth gate<br/>401 unless route opts out"]
    AUTH --> BIND["Layer 3: DTO-bind allowlist<br/>drops vendor_id / id / *_by"]
    BIND --> SVC[Service and controller]
    SVC --> SESS[SQLAlchemy session]
    SESS --> READ["Layer 1: with_loader_criteria<br/>scopes every SELECT to g.tenant_scope"]
    SESS --> WRITE["Layer 2: before_flush<br/>asserts row vendor in g.tenant_scope"]
    READ --> DB[(Postgres)]
    WRITE --> DB
    DB -.->|backstop| RLS["RLS USING + WITH CHECK<br/>catches raw SQL"]
    style AUTH fill:#e6f3ff,color:#000
    style BIND fill:#e6f3ff,color:#000
    style READ fill:#e6f3ff,color:#000
    style WRITE fill:#e6f3ff,color:#000

Control	How it works	Classes it covers
L1 — Read default	with_loader_criteria global do_orm_execute injects vendor_id IN g.tenant_scope into every tenant-model SELECT (no-op for an ALL_VENDORS principal)	cross-tenant read IDOR
L2 — Write guard	before_flush asserts every new/dirty/deleted row's vendor_id is in the caller's scope (membership, never truthiness) and blocks re-homing at every scope	cross-tenant write IDOR
L3 — Bind allowlist	base DTO-bind rejects client-supplied vendor_id / id / *_by	mass-assignment, forged-vendor writes
L4 — Default-deny auth	global before_request gate (401 unless the route is allowlisted public) + the existing requires_easy_cron_token decorator	missing-authn, cron-auth

(The IDOR ~30-read / ~61-write split is an estimate — the one number that sizes L1 vs L2; see open questions.)

What each layer looks like — before vs. after

Concrete, not hand-wavy. Each control is one listener/util, registered once — it replaces the fix the rampcullis PRs add by hand at every call site. Flip the toggle to read the scattered "before" against the single "after".

Why "after" wins — the same three properties for every layer:

• Less code. ~90 hand-written .where(vendor_id == …) / ownership checks collapse to 3 listeners, ~40 lines total. Nothing to copy-paste into the 81st endpoint.
• Safer. The default is fail-closed — an unscoped query returns nothing cross-tenant instead of leaking, and a cross-tenant write raises. Today a forgotten scope ships silently and leaks.
• Can't-forget. No per-endpoint step is left to forget. Isolation is inherited; crossing tenants requires a deliberate, reviewed wider scope (one line in resolve_scope), never a per-call-site bypass.

L1 — read default.

~30 read PRs each add the same guard, one call site at a time — and every future query has to remember it:

# app/api/lanes/services.py — and ~29 other call sites
stmt = select(ORLaneStage).where(ORLaneStage.uuid == uuid)
if vendor_id is not None:                 # easy to forget; forgetting = silent cross-tenant read
    stmt = stmt.where(ORLaneStage.vendor_id == vendor_id)

One do_orm_execute listener scopes every tenant-model SELECT — joins and relationship loads included — for all ~30 sites at once. The listener is always on; what varies per caller is the resolved scope, never whether the listener runs (see Exceptions are a wider scope, not a bypass):

# app/onramp/tenancy.py — registered once at startup
from sqlalchemy import event, with_loader_criteria
from sqlalchemy.orm import Session
from flask import g

@event.listens_for(Session, "do_orm_execute")
def _scope_reads_to_vendor(state):
    if not state.is_select:
        return
    scope = g.get("tenant_scope")         # resolved once per request from the principal
    if scope is None or scope is ALL_VENDORS:
        return                            # no principal (boot) / self-tenant: sees all, by policy
    state.statement = state.statement.options(
        with_loader_criteria(
            TenantMixin,                  # every vendor-scoped model in the query
            lambda cls: cls.vendor_id.in_(scope),   # {own} for a normal caller; a set when wider
            include_aliases=True,         # also scopes joins / relationship loads
        )
    )

scope is a frozenset of vendor ids (the common case: {g.vendor_id} — and {0} is the self-tenant, so membership, not truthiness, keeps vendor-0 safe), or the ALL_VENDORS sentinel for principals legitimately allowed to read every tenant. It is resolved once per request by resolve_scope(current_user) — not toggled per call site.

L2 — write guard. L1 is SELECT-only; the ~61 write IDORs need a flush-time assertion.

~61 write PRs each re-fetch the row and bolt an ownership check in front of the mutation — every write site, by hand:

# app/api/playbooks/services.py — and ~60 other write sites
module = session.get(ORModule, module_id)
if module.vendor_id != g.vendor_id:       # must remember on every UPDATE / DELETE
    abort(403)
module.name = new_name

One before_flush listener covers INSERT / UPDATE / DELETE for all ~61 sites at once. Same model as L1 — it resolves the caller's scope and checks membership; it is never switched off. A wider scope widens which vendors a write may touch; it never disables the re-homing guard:

@event.listens_for(Session, "before_flush")
def _guard_tenant_writes(session, flush_context, instances):
    scope = g.get("tenant_scope")
    for obj in session.new:               # INSERT: stamp it, never trust the client
        if isinstance(obj, TenantMixin):
            if obj.vendor_id is None:
                obj.vendor_id = default_write_vendor(scope)   # single-vendor scope only
            elif not in_scope(obj.vendor_id, scope):
                raise CrossTenantWrite(obj)
    for obj in session.dirty | session.deleted:        # UPDATE / DELETE
        if isinstance(obj, TenantMixin):
            if not in_scope(obj.vendor_id, scope):
                raise CrossTenantWrite(obj)             # the ~61 write IDORs, one place
            if get_history(obj, "vendor_id").has_changes():
                raise CrossTenantWrite(obj)             # re-homing stays blocked at EVERY scope

in_scope(v, scope) is True when scope is ALL_VENDORS or v in scope — a single membership test that treats 0 like any other id, so the self-tenant never trips it and never gets a truthiness special-case. The re-homing assertion sits outside the scope check on purpose: even an ALL_VENDORS principal may not silently move a row from one tenant to another.

L3 — bind allowlist. Mass-assignment plants a cross-tenant row by setting an ownership column from the request body.

Each controller binds the raw request body — the mass-assignment PRs each patch one of them by hand:

module = ORModule(**request.json)   # client can smuggle vendor_id -> cross-tenant plant

The base bind rejects ownership keys outright, for every controller at once:

PROTECTED = {"vendor_id", "id", "uuid", "created_by", "modified_by", "deleted_by"}

def bind(model_cls, payload: dict, *, allow: set[str]):
    leaked = payload.keys() & PROTECTED
    if leaked:
        raise MassAssignment(f"client may not set {sorted(leaked)}")
    return model_cls(**{k: v for k, v in payload.items() if k in allow})

Exceptions are a wider scope, not a bypass

The obvious way to handle a legitimate cross-tenant op is a skip_tenant_scope flag that short-circuits the listener. We reject that — a blanket bypass quietly re-opens the hole this RFC exists to close, in two ways:

📈 Bypass blast-radius grows silently

The session listener is where controls accrete — today L1 read-scoping, tomorrow field redaction, audit tagging, query-cost guards. A skip_tenant_scope written for one early reason silently disables every control added later at that same call site. The opt-out drifts wider than whoever wrote it ever reviewed.

🕳 A bypass is *absence of policy*

Inside a bypassed region the default flips back to fail-open — the exact property the RFC sells against. The canonical case proves the point: OnRamp self-tenant reading across vendors isn't "no tenancy," it's "allowed vendor set = all." That's still a rule. Encoding it as a bypass throws a real, recurring, modelable rule away and replaces it with a hole.

So the exception is modelled as a different scope, not the listener's absence. A pure resolver maps the principal to the vendors it may touch; the listeners always run and read that scope:

# app/onramp/tenancy.py — pure, unit-testable, the locked contract
ALL_VENDORS = object()        # sentinel: principal may cross every tenant

def resolve_scope(user) -> frozenset[int] | ALL_VENDORS:
    if user is None:                      # no request principal (boot, shell)
        return frozenset()                # fail-closed: sees nothing until widened
    if user.is_onramp_self_tenant:        # support / admin tooling
        return ALL_VENDORS
    return frozenset({user.vendor_id})    # the overwhelming common case — {0} included

g.tenant_scope = resolve_scope(current_user) is set once per request in before_request. Cron and a few service principals resolve to a specific frozenset({target_vendor}) — wider than one, still bounded. The code path is identical for every caller; the only thing that varies is the resolved data — the convention-over-configuration "derive from one discriminator" rule, applied to tenancy instead of environment.

The genuine residue — principal-less batch work. Offline migrations and backfills run with no current_user to resolve a scope from. They — and only they — get a real bypass: a narrow, audit-logged with cross_tenant(reason=...) context, scoped to one named control rather than the whole listener, so a future L5 can't be silently switched off by a migration written today. Everything reachable from a request goes through the resolver; nothing legitimately reachable from a request needs a flag.

What changes for the developer

Opt-in. Every route must remember to scope its query and validate ownership on writes. Forgetting is silent and ships — the failure mode is a data leak the scanner finds months later.

Scope-driven. A new endpoint inherits read-scoping, write-assertion, and ownership-key rejection for free. A caller that legitimately spans tenants gets that ability from its resolved scope (ALL_VENDORS or a wider set), decided once by resolve_scope from the principal — not a per-call-site opt-out the dev remembers to add. The only true bypass is the narrow, audit-logged cross_tenant(reason=…) context for principal-less batch work. Failure mode flips from silent hole to a single reviewed policy line.

Do I annotate every endpoint? No — only the exceptions

The most common question on this RFC: if isolation is enforced, does every one of the ~900 endpoints need a new marker? No. The default is applied once, at the session + base-DTO layer, and keys off the model carrying vendor_id (via TenantMixin) — never off the route. Endpoints inherit isolation with zero per-route code. The only things that carry a marker are the exceptions:

What carries a marker	Granularity	Roughly how many
New tenant model registered in the criteria registry (CI-enforced)	per new table, not per endpoint	rare
Wider-scope principal in resolve_scope	a principal class that legitimately spans tenants (self-tenant, a cron service account)	a handful, one place
cross_tenant(reason=…) bypass	principal-less batch work only (migrations, backfills) — per named control	a handful
Public-route allowlist (L4)	a route intentionally left unauthenticated	a handful

The polarity flips — that's the "less code / safer" win. Today the implicit annotation is a hand-written .where(vendor_id == …) on every read and an ownership check on every write — ~90 of them, each forgettable, each a vendor-0 trap. After this RFC the safe path is the default and only the deliberate exception is annotated. Fewer markers overall, and the ones that remain are loud and reviewable instead of silent and easy to miss.

Harvesting the PRs — we bank them, we don't close them

The ~113 supersedable PRs are not discarded. The systemic layers absorb their value three ways, so the tokens already spent keep paying off:

• Test corpus → the layer's spec. Every rampcullis PR ships a RED-before / GREEN-after regression test (rc#1961 carries three, including an explicit vendor_id = 0 case). Harvest all ~119 tests into the suite; the systemic layer must make every one pass. That is how we prove ~4 changesets actually close the ~90 vulns — we run the PRs' own tests against the layer. The tests migrate; only the per-call-site fix is superseded.
• Call-site inventory → the implementation map. Each PR's "Changes made" section names the exact vulnerable file/function — collectively, the map of every place the layers must reach and every model/route in scope.
• Fallback for the uncoverable. Paths a layer genuinely can't reach (raw SQL, odd ownership bridges, the irreducible handful) keep their individual PR and merge as-is.

PR lifecycle, not PR closure. Each supersedable PR moves to test-harvested → closed only once its test is green under the systemic layer. A PR closes when its vuln is provably covered — never as "won't fix."

Keeping it enforced (so this never recurs)

Default-on is the point — but back it with checks so the layers can't be quietly bypassed:

• Layer 3 — statically assertable. CI lint/AST rule: controllers may not Model(**request) or setattr from raw request data; bind goes through the allowlist util.
• Layer 1 — registry-asserted. A CI test fails if any model carrying vendor_id isn't covered by the criteria registry (or explicitly exempted with a reason). New tenant table → must be registered or the build breaks.
• Layer 2 — wiring-asserted. A test asserts the before_flush guard is registered on the session; the guard itself carries a pinned vendor_id = 0 fixture so the self-tenant boundary can't regress.
• The tricky residue — review-bot-affirmed. Most legitimate cross-tenant ability now lives in one reviewable place — the resolve_scope resolver — so a new wider-scope principal is a normal diff, not a scattered flag. What static checks still can't judge is the principal-less cross_tenant(reason=…) bypass. The rampcullis review bot updates its prompt to affirm, on every PR: each cross_tenant use is justified and confined to batch/migration paths, every new entry in resolve_scope earns its width, and each new write path either inherits the guard or documents why it doesn't.

Alternatives considered

• Opt-in helper (load_or_403) — rejected as the primary play: it still requires editing all ~80 call sites, supersedes 0 PRs as units, and prevents nothing (the next endpoint can still forget to call it). Useful only as the manual fallback for the handful of paths the base hooks can't cover.
• DB RLS as the primary control — rejected for now: as-built the app role has BYPASSRLS + a permissive USING(true) policy (inert on the read-write connection), and USING is SELECT-only (same write asymmetry). Keep RLS as a defense-in-depth backstop — it catches raw SQL the ORM hooks miss — on a separate durable track, not the lever.
• Keep merging point-fixes — rejected: 90+ merges, each a vendor-0 regression risk, and zero future prevention.

Risks & open questions

Highvendor_id = 0 self-tenant

Truthiness in any layer bricks the OnRamp self-tenant; dev seed (vendor 1) hides it.

Mitigation: is None / explicit cast everywhere; pin a vendor_id = 0 fixture in each layer's test.

HighWrite-guard blast radius

A wrong before_flush assertion blocks every write, app-wide and centrally.

Mitigation: ship in shadow / log-only mode for one full release, then flip to enforce.

MedLegitimate cross-tenant ops

Cron, migrations, self-tenant reads must cross tenants.

Mitigation: model them as a wider resolved scope (resolve_scope), not a listener bypass; reserve cross_tenant(reason=…) for principal-less batch work, review-affirmed.

MedRaw-SQL bypass

session.execute(text(...)) skips the ORM hooks.

Mitigation: the RLS backstop covers raw SQL; lint-flag raw execute in tenant paths.

Open questions

Exact read / write / mixed split of the 91? ~61/30 is a title-based estimate; the precise split sizes Layer 1 vs Layer 2 and which PRs each supersedes.

How is vendor reached per model? Direct column vs relationship bridge — can we reuse the agent SQL tool's __sql_tool_owner_bridges__ registry?

Scope resolver coverage? With the opt-out reframed as a resolved scope (not a bypass), the remaining question is enumerating the wider-scope principal classes for resolve_scope — and confirming cross_tenant(reason=…) is needed only for principal-less batch paths.

Per-layer CI assertability? Confirm which checks are static vs which fall to the review-bot prompt.