fix(time-log): an update cannot move a manual time log to an employee of another organization

Merged
EV
11d ago
+979-132

Follow-ups queue Q14, found while fixing #10754's review thread T4 (manual time judged on the wrong organization).

The hole

A caller with CHANGE_SELECTED_EMPLOYEE reaches every employee of the tenant. Through PUT /timesheet/time-log/:id or GraphQL updateManualTime, such a caller could name an employee of another organization:

  • The log moves across organizations. TimeLogUpdateHandler keeps the stored row's organizationId, but employeeId is not a scope field. The log was re-pointed at the other organization's employee while staying in its own organization, leaving an inconsistent row.
  • The cleanup deletes the other organization's slots. The update's overlap cleanup ran in the new employee's organization (organizationId = employee.organizationId). It deleted that organization's overlapping time slots under an allowModifyTime policy that was judged on the log's organization, never on theirs.

Fix

The fix is in TimeLogService.updateManualTime, the method both routes call (there is no bulk update path):

  • Refuse cross-organization moves. Naming an employee whose organization is not the log's own is refused for every caller, CHANGE_SELECTED_EMPLOYEE included. The answer is 403 "A time log cannot be moved to an employee of another organization", the same convention as the existing refusal right above it for callers without that permission. It runs after the stored log is read and before any overlap lookup, slot deletion or write.
  • Scope the cleanup to the log. The overlap lookup now always uses the log's own organization.
  • Unchanged: moving a log to another employee of the same organization still works for a CHANGE_SELECTED_EMPLOYEE caller, and ALLOW_MODIFY_TIME is still judged on the log's organization by the guard.
  • Edge case: a log whose organizationId is null (none are expected; time logs are organization-scoped) can no longer be re-pointed at an employee who has one.

Red first

time-log.service.spec.ts on develop: 2 failed / 46 passed. With the fix, 48/48.

  • "an update naming an employee of another organization is refused and writes nothing": on develop the update went through. Now there is no write, no overlap lookup and no slot deletion.
  • "the conflict cleanup of an update never deletes another organization's slots": the other organization's overlapping slot survives, and a same-organization edit looks in the log's organization even when the body names another.
  • Control, passing before and after: "an update moving the log to another employee of the same organization still works for a CHANGE_SELECTED_EMPLOYEE caller".
  • A pin in time-log-update.handler.spec.ts, passing before and after: the handler's own slot cleanup always uses the stored log's organization and employee.
  • The existing update fixtures gained an organizationId, which they lacked; without it the new check would refuse them.

Verification (on develop 7ab5a707fb3)

CheckResult
time-log + time-slot specs, every command-handler spec, the two guard suites18 suites, 292 tests passed
tsc -p packages/core/tsconfig.lib.json --noEmit0 errors
static gates30 run, failed: 0

🤖 Generated with Claude Code


Summary by cubic

Fixes updating a manual time log via PUT /timesheet/time-log/:id or GraphQL updateManualTime so a log can't be moved to an employee of another organization, and so edits, moves, and deletes only release time the log itself holds.

  • Cross-organization moves are refused with 403 for every caller, and the overlap cleanup always runs in the log's own organization.
  • Same-organization moves re-home the log's timesheet, time slots, and worked hours with the new employee; a log with tracked screenshots, activity, or keyboard/mouse records can't be moved.
  • A shortened range releases only its own dropped slots, and the previous employee keeps a slot another of their logs still holds, whether on a move or a range edit.
  • A moved log links to the new employee's existing shared-boundary slot.
  • A failed save is now reported instead of answered as success, and a move saves the log once before releasing the previous employee's slots, so their time is untouched until the move is saved.
  • Fixed TimeSlotBulkDeleteHandler deleting only the first qualifying slot and crashing on slots without logs; TimesheetRecalculateCommand is no longer sent without a timesheet id.