Workflow Designer allows duplicate transition rows that silently override each other non-deterministically #62

Open
opened 2026-08-31 23:57:36 -04:00 by jared · 0 comments
Owner

Severity: Medium

api/manage_workflows.php:82-127 (POST) and :129-176 (PUT) — nothing prevents creating a second status_transitions row for the same (from_status, to_status) pair; only from_status !== to_status is validated (server- and client-side, WorkflowDesignerView.php:228).

WorkflowModel::getAllTransitions() (models/WorkflowModel.php:34-60) queries WHERE is_active = TRUE with no ORDER BY and collapses rows into a PHP array keyed by [from_status][to_status], so when two active rows exist for the same pair, one silently overwrites the other with no deterministic winner (MySQL doesn't guarantee row return order without ORDER BY).

Impact: WorkflowDesignerView.php lists all rows from a separate, un-deduped query, so the admin sees two distinct-looking transition rows for the same pair — possibly with different requires_comment/requires_admin settings — while only one, non-deterministically chosen, actually governs isTransitionAllowed()/transitionRequiresComment() at runtime. An admin editing what looks like "the" transition rule for a pair may be editing the row that isn't actually enforced.

Fix: Add a uniqueness constraint (DB unique index on (from_status, to_status) where is_active = TRUE, or an application-level check in manage_workflows.php before insert) and add ORDER BY to getAllTransitions() for deterministic behavior regardless.

**Severity:** Medium `api/manage_workflows.php:82-127` (POST) and `:129-176` (PUT) — nothing prevents creating a second `status_transitions` row for the same `(from_status, to_status)` pair; only `from_status !== to_status` is validated (server- and client-side, `WorkflowDesignerView.php:228`). `WorkflowModel::getAllTransitions()` (models/WorkflowModel.php:34-60) queries `WHERE is_active = TRUE` with **no `ORDER BY`** and collapses rows into a PHP array keyed by `[from_status][to_status]`, so when two active rows exist for the same pair, one silently overwrites the other with no deterministic winner (MySQL doesn't guarantee row return order without `ORDER BY`). **Impact:** `WorkflowDesignerView.php` lists all rows from a separate, un-deduped query, so the admin sees two distinct-looking transition rows for the same pair — possibly with different `requires_comment`/`requires_admin` settings — while only one, non-deterministically chosen, actually governs `isTransitionAllowed()`/`transitionRequiresComment()` at runtime. An admin editing what looks like "the" transition rule for a pair may be editing the row that isn't actually enforced. **Fix:** Add a uniqueness constraint (DB unique index on `(from_status, to_status)` where `is_active = TRUE`, or an application-level check in `manage_workflows.php` before insert) and add `ORDER BY` to `getAllTransitions()` for deterministic behavior regardless.
jared added the data-integritypriority/mediumworkflow labels 2026-09-08 10:15:46 -04:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: LotusGuild/tinker_tickets#62