Skip to content

Worker flock assignment is enforced on writes but not on reads — a restricted worker can enumerate and read unassigned flocks #388

Description

@mforce

Found while building the #385 Playwright E2E suite, by probing the live #243 simulation fixture rather than by reading code alone.

What was measured

SimulationDataSeeder.RestrictOneWorkerAsync assigns sim-worker-1 to exactly one of the fixture's two flocks (Sim House A), deliberately leaving Sim House B unassigned so the load test has a genuinely narrowed worker.

Against that fixture, as sim-worker-1:

Request Flock Result
POST /api/v1/daily-entries assigned (Sim House A) 201 Created
POST /api/v1/daily-entries unassigned (Sim House B) 422 FlockScope.NotAssigned"You are not assigned to this flock — ask an owner or manager."
GET /api/v1/flocks 200, returns BOTH flocks — byte-identical to what an unrestricted worker (sim-worker-2, sim-worker-3) receives
GET /api/v1/flocks/{id} unassigned 200 — name, breed, placement date, initialCount, currentBirds, status
GET /api/v1/daily-entries?flockId= unassigned 200 — that flock's production history

So the write path enforces the assignment correctly. The read paths do not consult it at all.

Why this is worth a decision rather than a shrug

IUserRoleAssignmentRepository has exactly four consumers in the codebase: the DI registration, AssignFlockHandler, UserEndpoints (managing the assignments), and SimulationDataSeeder. No query-side feature reads it. There is no read-side notion of flock scope anywhere.

Re-measured 2026-08-25, after epic #530 closed its tenancy slices. The asymmetry has widened rather than closed: IFlockScopeGuard now gates four write handlers — SubmitDailyEntry, RecordDailyEntry, RecordFeedUsage and RecordWaterUsage — while no list, get or query handler references it (grep -rln FlockScopeGuard src/Cluckwork.Application/Features/ returns only those four). The original text said "a single check on the production-write path"; it is now four, all writes. IUserRoleAssignmentRepository still has exactly the four consumers named above.

This issue was sequenced into epic #530 to land after the FlockScopeGuard / ICurrentUser surface settled. That surface has now settled — #530's tenancy slices are shipped and its decision record is 530-multi-farm-tenancy.md — so the sequencing reason no longer blocks it. What blocks it is the unanswered question below, which is an owner decision and costs about a minute: answering "out of scope" makes this a GLOSSARY + #277 acceptance-text correction; answering "intended" makes it a horizontal-authorization slice across the flock list, flock detail and daily-entry reads.

Note the axis, because it is easy to misread: this is about which flock within one farm, not which farm. #530's tenant isolation neither fixes nor worsens it.

That leaves an "assign this worker to a flock" affordance whose meaning is narrower than it reads. An owner assigning a worker to one house may reasonably believe they have scoped that worker to that house — while the worker can still enumerate every flock on the farm, open any of them, and read 90 days of another house's production history.

DECIDED (owner, 2026-08-25): read scoping is INTENDED

The question this issue asked — is read-side flock scoping intended and missing, or deliberately out of
scope — is settled. Intended. Assigning a Worker to a flock scopes what they may see, not only what
they may write.

So this is a horizontal-authorization slice, and #277's Worker acceptance text ("the flock-restricted
worker sees only assigned flocks") becomes correct as written rather than needing correction.

The original question and both candidate answers are preserved below for the reasoning; the answer above
is the one to build to.

The read surface, enumerated 2026-08-25 (not recalled — grepped)

Every endpoint that can expose another flock's data to a scoped Worker. Verify this list again before
building
; it is the enumeration this slice lives or dies by, and a missed site is a silent hole.

Endpoint File
GET /flocks (list) src/Cluckwork.Api/Endpoints/Flocks/FlockEndpoints.cs:31
GET /flocks/{id} FlockEndpoints.cs:35
GET /flocks/{id}/movements FlockEndpoints.cs:66
GET /daily-entries (flockId filter) src/Cluckwork.Api/Endpoints/DailyEntries/DailyEntryEndpoints.cs:44
GET /daily-entries/{id} DailyEntryEndpoints.cs:40
GET /water (flockId filter) src/Cluckwork.Api/Endpoints/Water/WaterUsageEndpoints.cs:30
GET /inventory/usage (flockId filter) src/Cluckwork.Api/Endpoints/Inventory/InventoryEndpoints.cs:76,246-252
GET /expenses, GET /expenses/{id} src/Cluckwork.Api/Endpoints/Expenses/ExpenseEndpoints.cs:39,43 — expenses carry a nullable FlockId (:229,240)

Mortality is not a separate endpoint — it lives on the daily entry
(DailyEntryEndpoints.cs), so it is covered by scoping those two reads.

Two shapes to decide during design, because they are not the same rule:

  1. Nullable FlockId (expenses, and inventory usage where flockId is optional): a row with no
    flock is farm-wide. Does a scoped Worker see it? "Hide everything not mine" also hides general farm
    expenses, which is probably wrong. This needs an explicit rule, not an implicit one.
  2. List filtering vs. detail refusal. A hidden row in a list and a 404 on its id must agree, or the
    list becomes an enumeration oracle for exactly the ids it is hiding.

Why a chokepoint, not eight call sites

IFlockScopeGuard currently gates four write handlers and zero reads. Adding a read check to eight
endpoints by hand recreates the same asymmetry one endpoint at a time, and the ninth endpoint someone
adds later will not have it. Prefer a structural chokepoint — the shape the #546 cross-reference comment
below argues for — so a new flock-keyed read is scoped by construction rather than by remembering.

Not the tenancy axis

This is which flock within one farm, not which farm. #530's tenant isolation neither fixes nor
worsens it, and the two must not be conflated in review.

What #385 does in the meantime

The E2E Worker persona asserts the guarantee the app actually provides — the 422 FlockScope.NotAssigned refusal surfacing in the UI on the unassigned flock, and a successful entry on the assigned one — rather than asserting the premise #277 stated. Per the standing rule that a failure links back to the owning implementation issue instead of weakening the assertion, that spec carries a pointer to this issue. If read scoping lands, the spec gains the read assertions here.

Related: #127 (ReadOnly could read customer PII and sales financials via ungated reads) was the same shape — a role whose write restrictions were enforced while the matching reads were not.


Code audit (2026-08-05)

Confirmed against source (not just grep), to size the fix before deciding read-scoping is/isn't intended.

The guard: IFlockScopeGuard.CheckAsync(flockId, ct) — defined at src/Cluckwork.Application/Common/IFlockScopeGuard.cs:10-13, implemented by FlockScopeGuard in src/Cluckwork.Infrastructure/Repositories/UserRoleAssignmentRepository.cs:34-36, registered at src/Cluckwork.Api/Hosting/CluckworkFeatureServiceCollectionExtensions.cs:89.

Called only from 4 write handlers, each invoking it manually: RecordDailyEntryHandler, SubmitDailyEntryHandler, RecordFeedUsageHandler, RecordWaterUsageHandler. No MediatR pipeline, no EF global query filter, no middleware consults it — it's a plain scoped DI service called ad hoc per handler.

18 read-path methods across 8 endpoint classes never call it:

  • FlockEndpoints: ListFlocks, GetFlock, ListMovements
  • DailyEntryEndpoints: GetDailyEntry, ListDailyEntries
  • InventoryEndpoints.ListFeedUsage (FeedUsage.FlockId)
  • WaterUsageEndpoints.ListWaterUsage (WaterUsage.FlockId)
  • ExpenseEndpoints: ListExpenses, GetExpense (Expense.FlockId nullable)
  • StockEndpoints: GetStock, ListLots, ListLotMovements (EggLot.FlockId)
  • ReportEndpoints: Production, Sales, Expenses, Profit
  • ExportEndpoints: ExportAll, ExportDataset

Reports/Exports weren't named in the original write-up above and widen the question further: those are aggregate queries, not flockId-keyed lookups, so scoping them isn't a guard-clause 403 — it's a filtering-logic decision (does a restricted worker's production report show farm-wide totals, or just their assigned flock's?).

Effort, if read scoping is confirmed intended: ~3–5 focused days. The guard has zero query-side consumers today, so this is linear (18 individual call sites, no shared hook to wire once), each needing the same parallel-race + RoleMatrixTests treatment the write path already has, plus report/export filtering design, plus the GLOSSARY.md/Help-page sync AGENTS.md requires for any user-visible change.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:frontendReact/Vite web clientepic-1.6Phase 1.6 — Multi-farm tenancy

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions