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

Closed
opened 2026-08-31 23:57:36 -04:00 by jared · 1 comment
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
Author
Owner

Fixed and verified (real MariaDB, real running server, real webhook capture, jsdom). Merged to main in commit b6c1709.

Fixed and verified (real MariaDB, real running server, real webhook capture, jsdom). Merged to main in commit b6c1709.
jared closed this issue 2026-09-11 14:49:56 -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