fix(tenancy): AspNetUserRoles carries a tenant column, so a role write naming another farm's user is refused (#670)

AGENTS.md

@@ -75,7 +75,7 @@ dotnet test  Cluckwork.sln                 # 688 tests as of 2026-07; integratio
 ### Data and correctness
 
 - **Every aggregate mutation must bump `Version`.** `Version` is an EF concurrency token (`IsConcurrencyToken`): EF puts the *original* value in the UPDATE's `WHERE` but never auto-increments it, so a mutation without `Version++` silently loses concurrent races — both writers match `WHERE Version = N` — instead of 409ing. This shipped three times; each fix carries a parallel-race integration test, and so must any new mutation.
-- **Multi-tenancy:** every tenant-owned entity has `AccountId`, enforced by an EF **global query filter** plus a **`TenantStampInterceptor`** (stamps on insert, and rejects a mismatched write on update/delete). `TenantContext` resolves per-request from the JWT `account_id` claim and is **single-assignment** — a differing re-resolve throws; at startup it is unresolved, so seeders use `IgnoreQueryFilters()`. Several farms now coexist on one deployment: sign-in takes a farm code, and one email address can belong to a user in more than one farm. **`AccountId` is also an EF concurrency token on every entity that carries one (#562):** the `UPDATE`/`DELETE` the database runs carries `AccountId = <original>`, so a detached stub aimed at another farm's row matches nothing — the interceptor alone cannot see a detached write, and an owned-only edit on an attached stub was writing through until the token landed. The token comes from a model walk in `AppDbContext.OnModelCreating`, so a new entity is covered automatically; never remove it, and a database refusal under a resolved tenant is logged as `Tenant.WriteRefusedByDatabase`. → [`530-multi-farm-tenancy.md`](docs/decisions/530-multi-farm-tenancy.md)
+- **Multi-tenancy:** every tenant-owned entity has `AccountId`, enforced by an EF **global query filter** plus a **`TenantStampInterceptor`** (stamps on insert, and rejects a mismatched write on update/delete). `TenantContext` resolves per-request from the JWT `account_id` claim and is **single-assignment** — a differing re-resolve throws; at startup it is unresolved, so seeders use `IgnoreQueryFilters()`. Several farms now coexist on one deployment: sign-in takes a farm code, and one email address can belong to a user in more than one farm. **`AccountId` is also an EF concurrency token on every entity that carries one (#562):** the `UPDATE`/`DELETE` the database runs carries `AccountId = <original>`, so a detached stub aimed at another farm's row matches nothing — the interceptor alone cannot see a detached write, and an owned-only edit on an attached stub was writing through until the token landed. The token comes from a model walk in `AppDbContext.OnModelCreating`, so a new entity is covered automatically; never remove it, and a database refusal under a resolved tenant is logged as `Tenant.WriteRefusedByDatabase`. **Identity's `AspNetUserRoles` carries a shadow `AccountId` plus a composite foreign key to `AspNetUsers(Id, AccountId)` (#670):** both layers select by property NAME, so the shadow column is stamped, verified and tokened like any other, and the FK makes a role grant to another farm's user — or under no resolved tenant — a database refusal; the other four user-keyed Identity tables have no writer in `src/` and are recorded as accepted risk. → [`530-multi-farm-tenancy.md`](docs/decisions/530-multi-farm-tenancy.md)
 - **Flock-scoped EF reads are discovered from the model (#613).** Every mapped `Flock` or entity with a scalar `FlockId` must keep one structural `AccountId AND flock-scope` query filter; `UserRoleAssignment` is the sole deliberate exclusion because those rows resolve the scope itself. Two no-`FlockId` children rely on named parent/policy gates: daily-entry list/detail loads `DailyEntryGrade` only through a filtered `DailyEntry` aggregate and Worker-readable production totals correlate it through that same filtered parent; the lot movement ledger gates `EggInventoryMovement` through filtered `EggLot`; both children's direct exports remain `AdminOnly`. Any new exclusion or parent-derived child needs an explicit rationale and a causal mutation test; do not replace the model walk with a recalled entity list or expression `ToString()` checks.
 - **Transient-DB retry stops at unreplayable work (#269).** `EnableRetryOnFailure` covers self-contained EF units only; an automatic replay above a stateful detector (a counter, a CAS stamp, a single-use claim) cannot tell "this request racing itself" from the signal the detector exists to catch. Two cures, and picking wrong ships the bug: `SingleAttemptExecution` when the replay is itself observable, a durability probe on a self-minted token when the replay writes nothing. → [`269-transient-db-retry-boundary.md`](docs/decisions/269-transient-db-retry-boundary.md)
 - **`AuditEvents` is not time-partitioned, on purpose (#505).** The dominant read filters on `AccountId`+`EntityType`+`EntityId` with no date predicate, so monthly partitions would turn one index lookup into one per partition for no pruning benefit. If it is ever needed, partition by `AccountId`. → [`505-audit-events-no-time-partition.md`](docs/decisions/505-audit-events-no-time-partition.md)

docs/decisions/530-multi-farm-tenancy.md

@@ -169,9 +169,44 @@ exists to keep the snapshot equal to the model). The refusal is indistinguishabl
 `Version` race inside the process and is logged under a resolved tenant as
 `Tenant.WriteRefusedByDatabase` (owner decision: a run of them is the signal, a lone one is a
 race). Pinned by `DetachedTenantWriteTests`, `AccountIdConcurrencyTokenModelTests` and
-`TenantWriteRefusalLoggingTests`. **Still outside both layers:** entity types with no
-`AccountId` property — Identity's own six tables, of which `AspNetUserRoles` is live RBAC state
-(#670) — and every `ExecuteUpdate`/`ExecuteDelete`/raw-SQL path, which #536's guard governs.
+`TenantWriteRefusalLoggingTests`. **Still outside both layers:** every
+`ExecuteUpdate`/`ExecuteDelete`/raw-SQL path, which #536's guard governs, and the four
+user-keyed Identity tables below.
+
+**The Identity table that is live RBAC state, and what closed it (#670, 2026-09-03).** Both
+layers select by a property NAMED `AccountId`, and `AspNetUserRoles` had none: serving farm A, a
+hand-built `IdentityUserRole` row naming farm B's user was inserted, and B's Owner grant deleted,
+with no refusal (reproduced on the unmodified tree). Closed by the smallest way INTO the two
+existing layers rather than a third one: a **shadow** `Guid` `AccountId` on `IdentityUserRole<Guid>`,
+which the interceptor stamps and verifies and the #562 walk tokens with no change to either, plus a
+**composite foreign key** `(UserId, AccountId) → AspNetUsers(Id, AccountId)` so the stamped value is
+provably the user's own farm — a grant to another farm's user is a `23503`, an unforged detached
+`Remove` is refused by the interceptor, a forged one by the token, and a role write under no
+resolved tenant is refused by the FK rather than inserted unowned. One migration, with a
+hand-inserted backfill from `AspNetUsers` and a `DROP DEFAULT` (`UserRoleAccountIdMigrationTests`
+migrates to the point before it, inserts a role row, and migrates forward). No query filter,
+deliberately — `FirstRunStatusService` reads the table anonymously — and the #536 scanner keeps
+`UserRoles` on its stricter non-tenant track because that split is on CLR shape. Pinned by
+`UserRoleTenantWriteTests`, `UserRoleAccountIdModelTests` and `UserRoleAccountIdMigrationTests`
+(the last four of those tests cover the tracked shape too: loading another farm's row is a
+one-line query on a filter-free table, and a relabel is refused by the interceptor and then by the
+FK, a tracked `Remove` by the interceptor alone). **What no layer covers on this table, stated
+exactly:** under an *unresolved* tenant neither write layer inspects a `DELETE` — the same as every
+entity — but on every filtered entity such a scope reads zero rows unless it writes
+`IgnoreQueryFilters()`, a marker a reviewer sees, while here there is no filter and so no marker: an
+unresolved-tenant `db.UserRoles.Where(…)` + `RemoveRange` would delete every farm's matching
+grants with no refusal. Nothing in `src/` does that, and what holds the arm shut is #536's scanner
+(every `db.UserRoles` site is a classified candidate in `filter-free-set-sites.tsv`) — a guarded
+convention, not a mechanism; narrowing that entry is what reopens it.
+**Accepted risk, deliberately:** `AspNetUserClaims`, `AspNetUserLogins`, `AspNetUserTokens` and
+`AspNetRoleClaims` keep no tenant column. Nothing in `src/` writes or reads them, any direct
+`db.<Set>` access is already a #536 candidate requiring an allow-list entry, and `AspNetRoles` is
+global reference data. Two residuals no source walk can see: a future `UserManager`
+claim/login/token call — the same treatment as `AspNetUserRoles` is the fix the day one appears —
+and `RoleManager.DeleteAsync`, which deletes a *global* role and, through
+`FK_AspNetUserRoles_AspNetRoles_RoleId … ON DELETE CASCADE`, every farm's grants of it with the
+change tracker never holding an `IdentityUserRole` row; no caller exists, and one would be a
+farm-wide operation by construction, never a per-farm one.
 
 ---
 
@@ -368,6 +403,13 @@ Most of it is enforced by the guards named per section. The parts that are **not
 
 - the deploy-owned "replicas > 1 ⇒ Redis configured" invariant (§8) — **nothing in this
   repo enforces it**; it relies on the deployment repo;
+- the accepted risk on the four claim/login/token/role-claim Identity tables (§5, #670) —
+  **nothing enforces that no writer appears**; a direct `db.<Set>` access is caught by #536's
+  scanner, a `UserManager` claim/login/token call is not, and relies on review;
+- the unresolved-tenant `DELETE` arm on `AspNetUserRoles` (§5, #670) — **held shut by #536's
+  scanner's classification of every `db.UserRoles` site, not by a write layer**; and
+  `RoleManager.DeleteAsync`'s cascade across every farm's grants — **nothing enforces that no
+  caller appears**; both rely on review;
 - the "no slug as a metric label" and "no `AccessFailedAsync` on the unknown-farm branch"
   constraints (§4) — **nothing enforces these; they rely on review** and on the comments at
   the call site;