feat: prevent reservation self-block in booking handlers

Pass excludeUserID to CheckTimeBlockerOverlap in AdminReserveSlotHandler, CreateBookingHandler, EditBookingHandler, AdminRescheduleBookingHandler, AdminCreateBookingForUserHandler, AdminApproveEditRequestHandler, and ReserveSlotHandler. Also clean up stale reservations before overlap check in AdminReserveSlotHandler and ReserveSlotHandler via db.Conn.Exec for cross-connection visibility. Improve error handling in RequestEditHandler's service update block.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
2026-07-05 11:45:02 +01:00
co-authored by Sisyphus
parent 920eb761fd
commit c1a9f35ea5
4 changed files with 83 additions and 12 deletions
+15 -1
View File
@@ -136,7 +136,21 @@ func AdminReserveSlotHandler(w http.ResponseWriter, r *http.Request) {
}
}
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime)
// Clean up existing admin reservation BEFORE the overlap check,
// using db.Conn.Exec so the delete is visible to the separate connection
// used by CheckTimeBlockerOverlap. The in-transaction DELETE is kept
// as a safety net for the insert-phase.
if _, delErr := db.Conn.Exec(r.Context(), `
DELETE FROM time_blockers
WHERE description LIKE 'RESERVATION:admin:%'
AND created_by = $1
`, adminID); delErr != nil {
log.Printf("Failed to delete existing admin reservation: %v", delErr)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime, &adminID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
} else if blockerOverlap {
+4 -3
View File
@@ -2248,7 +2248,8 @@ func CreateBookingHandler(w http.ResponseWriter, r *http.Request) {
}
// Check for time blocker overlap (read-only, uses its own connection, safe to call in-tx)
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime)
// Pass excludeUserID so the user's own RESERVATION doesn't self-block
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime, &userID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
} else if blockerOverlap {
@@ -2464,7 +2465,7 @@ func EditBookingHandler(w http.ResponseWriter, r *http.Request) {
return
}
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEndTime)
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEndTime, &userID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
} else if blockerOverlap {
@@ -4171,7 +4172,7 @@ func AdminRescheduleBookingHandler(w http.ResponseWriter, r *http.Request) {
newEndTime := req.StartTime.Add(time.Duration(durationMinutes) * time.Minute)
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEndTime)
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEndTime, &adminID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
} else if blockerOverlap {
+36 -7
View File
@@ -686,7 +686,8 @@ func AdminCreateBookingForUserHandler(w http.ResponseWriter, r *http.Request) {
newEnd := req.StartTime.Add(time.Duration(dur) * time.Minute)
// Check for time blocker overlap - admin can proceed with warning
blockerOverlap, blockerDesc, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEnd)
// Pass adminID as excludeUserID so the admin's own reservation doesn't self-block
blockerOverlap, blockerDesc, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, newEnd, &adminID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
}
@@ -1529,19 +1530,41 @@ func RequestEditHandler(w http.ResponseWriter, r *http.Request) {
// Update services if requested
if len(req.NewServices) > 0 {
_, _ = tx.Exec(r.Context(), "DELETE FROM booking_services WHERE booking_id = $1", bookingID)
_, _ = tx.Exec(r.Context(), "DELETE FROM booking_custom_services WHERE booking_id = $1", bookingID)
if _, err := tx.Exec(r.Context(), "DELETE FROM booking_services WHERE booking_id = $1", bookingID); err != nil {
log.Printf("Failed to delete services for booking %s: %v", bookingID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if _, err := tx.Exec(r.Context(), "DELETE FROM booking_custom_services WHERE booking_id = $1", bookingID); err != nil {
log.Printf("Failed to delete custom services for booking %s: %v", bookingID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
for _, sid := range req.NewServices {
if _, err := tx.Exec(r.Context(), "INSERT INTO booking_services (booking_id, service_id) VALUES ($1, $2)", bookingID, sid); err != nil {
log.Printf("Failed to insert auto-approve service %s: %v", sid, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
}
}
// Clean up the edit request and reservations
_, _ = tx.Exec(r.Context(), "DELETE FROM booking_edit_requests WHERE id = $1", editReq.ID)
_, _ = tx.Exec(r.Context(), `DELETE FROM time_blockers WHERE description = $1`, fmt.Sprintf("RESERVATION:edit_request:%s", bookingID))
_, _ = tx.Exec(r.Context(), `UPDATE admin_notifications SET acknowledged_at = NOW() WHERE booking_id = $1 AND reason = 'edit_requested' AND acknowledged_at IS NULL`, bookingID)
if _, err := tx.Exec(r.Context(), "DELETE FROM booking_edit_requests WHERE id = $1", editReq.ID); err != nil {
log.Printf("Failed to delete edit request %s: %v", editReq.ID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if _, err := tx.Exec(r.Context(), `DELETE FROM time_blockers WHERE description = $1`, fmt.Sprintf("RESERVATION:edit_request:%s", bookingID)); err != nil {
log.Printf("Failed to delete time blocker for edit request %s: %v", bookingID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if _, err := tx.Exec(r.Context(), `UPDATE admin_notifications SET acknowledged_at = NOW() WHERE booking_id = $1 AND reason = 'edit_requested' AND acknowledged_at IS NULL`, bookingID); err != nil {
log.Printf("Failed to acknowledge notification for booking %s: %v", bookingID, err)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
if err := tx.Commit(r.Context()); err != nil {
log.Printf("Failed to commit auto-approve edit request: %v", err)
@@ -1732,6 +1755,12 @@ func AdminListEditRequestsHandler(w http.ResponseWriter, r *http.Request) {
// AdminApproveEditRequestHandler approves an edit request and updates the booking
func AdminApproveEditRequestHandler(w http.ResponseWriter, r *http.Request) {
adminID, ok := r.Context().Value(mw.UserIDKey).(string)
if !ok || adminID == "" {
http.Error(w, "Authentication required", http.StatusUnauthorized)
return
}
requestID := chi.URLParam(r, "request_id")
if requestID == "" || !validators.IsValidID(requestID) {
http.Error(w, "Edit request not found", http.StatusNotFound)
@@ -1856,7 +1885,7 @@ func AdminApproveEditRequestHandler(w http.ResponseWriter, r *http.Request) {
log.Printf("ALERT: failed to delete time_blocker: %v", delErr)
}
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), *newStartTime, newEndTime)
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), *newStartTime, newEndTime, &adminID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
}
+28 -1
View File
@@ -129,8 +129,35 @@ func ReserveSlotHandler(w http.ResponseWriter, r *http.Request) {
// g. Check existing booking overlap (same query as CreateBookingHandler)
endTime := req.StartTime.Add(time.Duration(svcDuration) * time.Minute)
// Clean up existing reservation BEFORE the overlap check,
// using db.Conn.Exec (not tx.Exec) so the delete is visible to the separate
// connection used by CheckTimeBlockerOverlap. The in-transaction DELETE
// is kept as a safety net for the insert-phase.
if hasAuth {
// Also compute IP hash to clean up anonymous reservations
// that may have been created before the user logged in.
ipHash := fmt.Sprintf("%x", md5.Sum([]byte(ip)))[:8]
if _, delErr := db.Conn.Exec(r.Context(), `
DELETE FROM time_blockers
WHERE (created_by = $1 AND description LIKE 'RESERVATION:user:%')
OR (description LIKE 'RESERVATION:anon:' || $2 || ':%')
`, userID, ipHash); delErr != nil {
log.Printf("Failed to delete existing reservation: %v", delErr)
http.Error(w, "Internal server error", http.StatusInternalServerError)
return
}
}
// h. Check time blocker overlap using scheduling.CheckTimeBlockerOverlap
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime)
// Pass excludeUserID so this user's own RESERVATION doesn't self-block.
// The pre-overlap DELETE (above) and excludeUserID are complementary:
// the DELETE cleans stale reservations from prior attempts; excludeUserID
// prevents any remaining own-RESERVATION entries from blocking.
var excludeUserID *string
if hasAuth {
excludeUserID = &userID
}
blockerOverlap, _, err := scheduling.CheckTimeBlockerOverlap(r.Context(), req.StartTime, endTime, excludeUserID)
if err != nil {
log.Printf("Failed to check time blocker overlap: %v", err)
} else if blockerOverlap {