Skip to content

Write-side tenant guard + single-assignment TenantContext #546

Closed

Description

@mforce

Coding brief — read first. Start with epic #530 (the canonical decision record). This body is complete and current as of 2026-08-16 — no comments you must cross-reference. Repo conventions in AGENTS.md apply automatically. Definition of done = the Verify section at the end.

Slice T2 of epic #530 (Phase 1.6 — Multi-farm tenancy). Depends on T1 (#531). Lands before the login change (#532).

Surfaced by the codex review, 2026-08-16. Both defects exist today; both are harmless with one farm and load-bearing with two.

Problem 1 — the write path has no tenant guard

TenantStampInterceptor (src/Cluckwork.Infrastructure/Persistence/Interceptors/TenantStampInterceptor.cs:28) only fills AccountId when it is Guid.Empty, and only for entities in state Added. Two gaps follow directly:

  • An Added entity carrying an explicitly wrong non-empty AccountId is accepted and written. The interceptor's own comment says it exists so "writes can't be mis-tagged even if a handler forgets to pass it" — it delivers that for the forgetful case and not for the wrong case.
  • Modified entities are never inspected at all. A handler that loads an entity outside the filter (there are ~20 IgnoreQueryFilters() call sites) and mutates it writes across the tenant boundary with nothing objecting.

The read side is well defended by 27 fail-closed query filters. The write side is defended by convention.

Problem 2 — TenantContext can be re-resolved to a different account

TenantContext.Resolve (src/Cluckwork.Infrastructure/Persistence/TenantContext.cs:11) is a plain setter that may be called repeatedly with different values.

The concrete sequence, which only becomes reachable once /auth/login takes a farm code: /auth/login is AllowAnonymous but can still be called carrying a valid Farm A bearer. TenantResolutionMiddleware resolves Farm A from that token; the login handler then resolves Farm B from the farm code in the body. One request, two tenants, and everything downstream sees whichever won.

Scope

  • The interceptor rejects rather than silently accepts: any Added or Modified tenant-owned entity whose AccountId differs from the resolved tenant throws. An unresolved tenant keeps today's behaviour (the seeders and CLI verbs run that way deliberately).
  • TenantContext becomes single-assignment: re-resolving the same account is a no-op, re-resolving a different one throws.
  • Account discovery on the login path must therefore not mutate the request's tenant context — it resolves the account for its own lookup without claiming the ambient one.

Why it ships before #532

#532 introduces the exact sequence that makes problem 2 reachable. Landing the guard first means #532's tests run against a context that already refuses to switch, rather than #532 having to avoid a hazard by convention.

Tests

  • An added entity stamped with a foreign AccountId throws instead of being written.
  • A modified entity whose AccountId was changed throws.
  • An unresolved tenant still permits the seeder and CLI paths — with a test naming each, so this is not rediscovered as a regression.
  • Re-resolving the same account is a no-op; a different account throws.
  • /auth/login presented with a valid Farm A bearer and a Farm B farm code does not end up serving Farm A's context to Farm B's handler.

Mutation-checked: revert each guard individually and confirm the corresponding test goes red on its own named assertion, with a green baseline recorded first.

Verify

dotnet build Cluckwork.sln
dotnet test  Cluckwork.sln

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:apiAPI/endpoint layerepic-1.6Phase 1.6 — Multi-farm tenancypriority:criticalBlocks the vertical slicesliceThin vertical work item

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions