SetRackSlotPosition accepts out-of-bounds slots (no dimension read, no rack lock, no membership check)

[flesher]

Summary

DeviceCollectionService.SetRackSlotPosition (and its DeviceSetService facade) writes a slot position without ever reading the rack's dimensions, so it accepts positions outside the grid. It also takes no rack row lock and does not verify membership.

Found while reviewing #855, which fixed the equivalent gaps in the batch path (AssignDevicesToRack). This is the pre-existing single-slot path and is not touched by that PR.

Example

Rack R1 is 2×2, so the only valid cells are (0,0), (0,1), (1,0), (1,1). Miner M is a member of R1.

Call	Result
AssignDevicesToRack{target_rack_id: R1, slot_assignments: [{M, row: 5, col: 5}]}	rejected — slot row 5 is out of bounds (rack has 2 rows)
SetRackSlotPosition{collection_id: R1, device_identifier: M, position: {row: 5, col: 5}}	200 OK, row persisted

M now has a rack_slot row at a cell the 2×2 grid can never render. It reads as placed in the data but is invisible in the grid UI, so an operator cannot drag it off — recovering needs a ClearRackSlotPosition API call.

No race is required for this; the bounds check is simply absent. buf.validate on RackSlotPosition only constrains row >= 0 / column >= 0 (proto/collection/v1/collection.proto), with no upper bound.

Two related gaps in the same method

No rack lock. A bounds check alone would still be racy. Resize goes through resolveAndApplyRackPlacement, which takes LockRackPlacementForWrite; the slot RPC takes none, so:

1. B places M at (11,11) in a 12×12 rack — reads dims, 11 < 12, passes
2. A resizes R1 to 2×2 — commits
3. B writes (11,11) — same orphaned slot

Silent success for a non-member. The store query is INSERT INTO rack_slot ... SELECT ... FROM device_set_membership WHERE .... If the device is not a member, zero rows insert, no error is raised, and the RPC returns 200 for a placement that never happened. This is the same bug Codex flagged in applyRackSlotDelta on #855, which was fixed there but not here.

Proposed fix

Mirror what AssignDevicesToRack already does, inside the existing transaction in Service.SetRackSlotPosition (server/internal/domain/collection/service.go). Both the collection and deviceset handlers delegate to this one service method, so there is a single place to change:

1. LockRackPlacementForWrite(ctx, req.CollectionId, orgID) before the reads, so the checks hold for the transaction (this also gives the canonical rack-first lock order).
2. GetRackInfo(...); treat nil as a broken invariant and fail — a RACK always has a device_set_rack row.
3. Reject row >= rackInfo.Rows or column >= rackInfo.Columns with InvalidArgument, matching the batch path's wording.
4. Confirm the device is actually a member before reporting success, rather than relying on the query's silent no-op.

Roughly 15 lines plus tests: out-of-bounds rejected, non-member rejected, in-bounds still succeeds.

Worth considering alongside: ClearRackSlotPosition has the same no-lock, no-membership-check shape, though with no bounds concern since it only deletes.

Severity

Low. Triggering it needs PermRackManage plus a direct API or CLI call — the web UI only offers rendered cells, so it will not happen by accident. Nothing crashes; the symptom is a miner stuck in an unrenderable slot.

Notes

An occupied-cell collision on this RPC used to surface as a 500. #855 maps uk_rack_slot_position to InvalidArgument in the store, which covers this RPC too, so that part is already handled once #855 lands.