# WorkFlow Engine — Deep Entity Analysis

**Date:** 2026-03-02
**Path:** `GB5Shared/WorkFlow/` + `GB5Framework/FrameworkBLL/SchedulerTaskGenerator/`
**Entities:** WorkflowEngine, WorkflowRunTime, WorkflowDefinition, WorkflowTask, WorkflowHistory, AutoTransition, RuleEngine, Scheduler
**Risk Level:** 🔴 VERY HIGH — Concurrent approval race, infinite loop crash, SQL injection

---

## CRITICAL ISSUES

### WF-01: Concurrent Approval Race Condition — CRITICAL

**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:427–553`

**Problem:** Two approvers clicking "Approve" simultaneously pass the status check and both complete the task:
```csharp
// Status check is NOT atomic with the update
if ((WorkflowTaskStatus)task.WorkflowTaskStatus != WorkflowTaskStatus.Pending)
    return; // Line 436 — read is NOT locked

// RACE WINDOW HERE — another thread passes the same check

task.WorkflowTaskStatus = (int)WorkflowTaskStatus.Completed;
task.ActionTaken = (int)ctx.Action;
await _IWorkFlowRunTime.UpdateTaskAsync(task, login, dbTransaction); // Line 457
```

**Scenario:**
```
T=0ms:  Approver A reads task → Status = Pending ✓
T=1ms:  Approver B reads task → Status = Pending ✓ (same row, not yet locked)
T=5ms:  Approver A updates → Status = Completed, Action = Approve
T=6ms:  Approver B updates → Status = Completed, Action = REJECT (overwrites!)
Result: Task "approved" then "rejected" — workflow in invalid state, audit trail corrupted
```

**Impact:** For financial approval workflows, this allows a transaction to be both approved and rejected. For PO/payment approval, either: duplicate payments or blocked payments.

**Fix — Pessimistic locking (SQL Server):**
```csharp
// In GetWorkflowTask, add lock hint
const string SelectWithLock = @"
    SELECT TOP 1 * FROM TWORKFLOWTASK WITH (ROWLOCK, UPDLOCK)
    WHERE WORKFLOWTASKID = @TaskId
    AND WORKFLOWTASKSTATUS = 0"; // Only lock if Pending

var task = await _queryExecutor.QuerySingleAsync<WorkflowTask>(
    login, SelectWithLock, new { TaskId = ctx.TaskId }, dbTransaction);

if (task is null) return; // Another process already actioned it
```

---

### WF-02: Infinite Loop via Circular Workflow Definition — CRITICAL

**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:601–635`

**Problem:** `EnterStepAsync` → `AutoTransitionAsync` → `EnterStepAsync` is recursive with no depth limit:
```csharp
private async Task AutoTransitionAsync(WorkflowInstance instance, WorkflowDefinition def,
    WorkflowStep current, ...)
{
    // ... resolves next step ...
    await EnterStepAsync(instance, def, next, ...); // RECURSIVE — no depth check
}

private async Task EnterStepAsync(WorkflowInstance instance, WorkflowDefinition def,
    WorkflowStep step, ...)
{
    if (step.StepType == 2 || step.StepType == 3)
        await AutoTransitionAsync(instance, def, step, ...); // CALLS BACK
}
```

**Scenario:** Workflow: Step A (Auto) → Step B (Auto) → Step A (misconfigured cycle)
→ `StackOverflowException` → application crash → all requests on that thread are terminated.

**Fix:**
```csharp
private async Task AutoTransitionAsync(..., int depth = 0)
{
    if (depth > 50)
        throw new InvalidOperationException(
            $"Circular workflow detected on instance {instance.WorkflowInstanceId} at step {current.StepKey}");

    // ... logic ...
    await EnterStepAsync(..., depth + 1);
}
```

---

### WF-03: Transaction Boundary Violation — Multiple Workflows Updated Atomically

**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:537–552`

**Problem:** `ProcessTenantWorkflows` is called inside the main approval transaction but iterates multiple workflows, each with early-return paths:
```csharp
if (isLastStep)
    await ProcessTenantWorkflows(ctx.TaskId, login, dbTransaction); // Inside outer tx

// ProcessTenantWorkflows → loops workflows → each may UPDATE independently
// If loop fails at item 3/10: items 1-2 committed, items 3-10 skipped
// Result: mixed state across related workflows
```

**Fix:** Use separate transaction for tenant workflow cascade — failures don't block the primary workflow:
```csharp
await _queryExecutor.CommitAsync(dbTransaction); // Commit primary first

var tx2 = await _queryExecutor.BeginTransactionAsync(login);
try { await ProcessTenantWorkflows(ctx.TaskId, login, tx2); await tx2.CommitAsync(); }
catch (Exception ex) { await tx2.RollbackAsync(); _logger.LogError(ex, "Tenant cascade failed"); }
```

---

### WF-04: Unsafe Rule Evaluation — Unhandled Type Coercion Exceptions

**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:201–227`

**Problem:**
```csharp
private object ParseValue(string value, string type) => type switch
{
    "Decimal" => decimal.Parse(value),       // Throws if "N/A" in rule config
    "Int"     => int.Parse(value),            // Throws if decimal stored in DB
    "Date"    => DateTime.Parse(value),       // Regional format mismatch!
    "Bool"    => bool.Parse(value),           // Only "True"/"False" exact
    _ => value
};

private int Compare(object left, object right)
{
    var r = Convert.ChangeType(right, left.GetType()); // Throws on type mismatch
    return c.CompareTo(r);
}
```

**Impact:** A misconfigured rule (bad value type in MRULECONFIG) crashes the entire workflow for all instances, blocking ALL approvals system-wide.

**Fix:**
```csharp
private bool TryParseValue(string? value, string type, out object? result)
{
    result = null;
    if (string.IsNullOrWhiteSpace(value)) return false;
    try
    {
        result = type switch
        {
            "Decimal" => decimal.Parse(value, CultureInfo.InvariantCulture),
            "Int"     => int.Parse(value, CultureInfo.InvariantCulture),
            "Date"    => DateTime.ParseExact(value, "yyyy-MM-dd", CultureInfo.InvariantCulture),
            "Bool"    => value.Equals("True", StringComparison.OrdinalIgnoreCase),
            _         => value
        };
        return true;
    }
    catch { return false; }
}
```

---

### WF-05: SQL Injection via Dynamic Table Name

**Severity:** 🔴 CRITICAL (inherited from systemic issue — see 04_Security_Analysis.md WF detail)
**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:668–688`

```csharp
string tableName = wf.DbObjectName.Trim();
// Basic char validation only — doesn't block schema prefix [db].[table]
string checkSql = $"SELECT COUNT(1) FROM {tableName} WHERE {primaryKeyColumn} = @objectid";
string updateSql = $"UPDATE {tableName} SET STATUS = 1 WHERE {primaryKeyColumn} = @objectid";
```

**Fix:** Use `QUOTENAME()` wrapping + whitelist:
```csharp
// SQL Server
string checkSql = $"SELECT COUNT(1) FROM {SqlQuoteName(tableName)} WHERE {SqlQuoteName(primaryKeyColumn)} = @objectid";

private static string SqlQuoteName(string name)
{
    // Only allow alphanumeric + underscore; wrap in brackets
    if (!Regex.IsMatch(name, @"^[A-Za-z_][A-Za-z0-9_]*$"))
        throw new ArgumentException($"Invalid identifier: {name}");
    return $"[{name}]";
}
```

---

## HIGH SEVERITY ISSUES

### WF-06: N+1 Rule Group Loading — O(n²) In-Memory Join

**File:** `GB5Shared/WorkFlow/WorkFlowRunTime/WorkFlowRunTime.cs:388–403`

```csharp
// Query 1: load all groups
var groups = (await _IQueryExecutor.QueryAsync<WorkflowRuleGroup>(...)).ToList();
// Query 2: load all rules
var rules  = (await _IQueryExecutor.QueryAsync<WorkflowRule>(...)).ToList();

// O(n²): for each group, scan entire rules list
foreach (var g in groups)
    g.Rules = rules.Where(r => r.RuleGroupId == g.RuleGroupId).ToList(); // LINQ Where per group
```

**100 groups × 10,000 rules = 1,000,000 iterations per rule evaluation.**

**Fix:**
```csharp
// Single JOIN query + GroupBy
var sql = @"SELECT g.*, r.* FROM WorkflowRuleGroup g LEFT JOIN WorkflowRule r ON r.RuleGroupId = g.RuleGroupId WHERE g.RuleGroupId IN @ids";
var lookup = await _queryExecutor.QueryAsync<RuleGroupRuleRow>(login, sql, new { ids });
return lookup.GroupBy(r => r.RuleGroupId)
             .ToDictionary(g => g.Key, g => MapGroup(g));
```

---

### WF-07: Workflow Definition Graph — Not Validated for Unreachable Steps

**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:579–599`

**Existing validation only checks:**
- At least 1 step exists
- Exactly 1 initial step
- Non-final steps have at least 1 outgoing transition

**Missing:**
- **Reachability check** — orphaned steps that can never be reached from the initial step
- **Transition target validation** — transition targets a step ID that doesn't exist
- **Auto-step without outgoing** — auto step with IsFinal=false but no transitions → freeze

**Fix — BFS reachability check:**
```csharp
var visited = new HashSet<long>();
var queue = new Queue<long>();
queue.Enqueue(def.Steps.Single(s => s.IsInitial).WorkflowDetailId);

while (queue.TryDequeue(out var id))
{
    if (!visited.Add(id)) continue; // cycle
    foreach (var t in def.Transitions.Where(t => t.FromStepId == id))
    {
        if (!stepIds.Contains(t.ToStepId))
            throw new InvalidOperationException($"Transition target {t.ToStepId} not found");
        queue.Enqueue(t.ToStepId);
    }
}
var unreachable = stepIds.Except(visited).ToList();
if (unreachable.Any())
    throw new InvalidOperationException($"Unreachable steps: {string.Join(", ", unreachable)}");
```

---

### WF-08: Scheduler Duplicate Job Execution Under Multiple Instances

**File:** `GB5Framework/FrameworkBLL/SchedulerTaskGenerator/SchedulerTaskManager.cs:72–110`

**Problem:** `LoadReadyTasksAsync` → `MarkExecutionStartedAsync` is two separate steps. Between them, another scheduler instance (different pod) can pick up the same job:

```
Instance-A: SELECT ready jobs → picks JobId=5
Instance-B: SELECT ready jobs → also picks JobId=5 (Instance-A hasn't marked InProgress yet)
Both: publish JobId=5 to message queue → job executes TWICE
```

**Fix — Atomic claim with UPDATE OUTPUT:**
```sql
-- SQL Server: atomic claim-and-return
UPDATE TOP(1) TJOBEXECUTION
SET Status = 2, ClaimedBy = @InstanceId, ClaimedAt = GETUTCDATE()
OUTPUT INSERTED.*
WHERE Status = 1 AND NextRunOn <= @Now AND ClaimedBy IS NULL;
```

---

### WF-09: Delegation Chain Not Resolved Transitively

**File:** `GB5Shared/WorkFlow/WorkFlowEngine/WorkFlowEngine.cs:166–181`

```csharp
var dels = await _delegationResolver.GetAsync(userId, loginDTO, DbTransaction);
var hit = dels.FirstOrDefault(d => d.StartsOn <= at && d.EndsOn >= at);
var effectiveUserId = hit?.DelegateUserId ?? userId;
// Returns delegate — but if delegate also delegated, chain not followed
```

**Missing:** If A → B and B → C, task should go to C. No loop detection for A → B → A.

---

### WF-10: Silent Exception Swallowing — No Logging Context

**Files:** `WorkFlowEngine.cs:177–180, 223–226, 577, 631–634` and `SchedulerTaskServiceBLL.cs:154–157`

```csharp
catch (Exception) { throw; }       // In WorkflowEngine — no log
catch { return null; }             // In SchedulerTaskServiceBLL — silently drops all errors
```

**Fix:**
```csharp
catch (Exception ex)
{
    _logger.LogError(ex, "Workflow step {StepKey} failed for instance {InstanceId}", step.StepKey, instance.Id);
    throw new WorkflowException($"Step '{step.StepKey}' failed", ex);
}
```

---

## MEDIUM SEVERITY ISSUES

### WF-11: WorkflowHistory Uses -1 as NULL Sentinel for ObjectId

**File:** `GB5Shared/WorkFlow/WorkFlowRunTime/WorkFlowRunTime.cs:297–326`

```sql
INSERT INTO TWORKFLOWHISTORY (...) VALUES (..., ISNULL(@objectid, -1), ...)
```

Queries filtering `WHERE ObjectId = @ObjectId` will match `-1` records when looking for real object ID `-1`. Replace with proper `NULL` and use `IS NULL` predicates.

---

### WF-12: 3–5 Roundtrips to Load Workflow Definition

**File:** `GB5Shared/WorkFlow/WorkFlowRunTime/WorkFlowRunTime.cs:354–403`

```csharp
var workflow    = await _db.QuerySingleAsync<WorkflowDTO>(sql1, ...);  // Query 1
var steps       = await _db.QueryAsync<WorkflowStepDTO>(sql2, ...);   // Query 2
var transitions = await _db.QueryAsync<WorkflowTransitionDTO>(sql3); // Query 3
var groups      = await _db.QueryAsync<RuleGroupDTO>(sql4, ...);      // Query 4
var rules       = await _db.QueryAsync<RuleDTO>(sql5, ...);           // Query 5
```

**Fix:** Single `QueryMultipleAsync` call with 5 result sets.

---

## Summary Table

| # | Issue | Severity | File | Lines | Priority |
|---|-------|----------|------|-------|----------|
| WF-01 | Concurrent approval race condition | 🔴 CRITICAL | WorkFlowEngine.cs | 427–553 | P0 |
| WF-02 | Infinite loop / circular workflow | 🔴 CRITICAL | WorkFlowEngine.cs | 601–635 | P0 |
| WF-03 | Transaction boundary violation | 🔴 CRITICAL | WorkFlowEngine.cs | 537–552 | P0 |
| WF-04 | Rule evaluation type parse crash | 🔴 CRITICAL | WorkFlowEngine.cs | 201–227 | P0 |
| WF-05 | SQL injection via table name | 🔴 CRITICAL | WorkFlowEngine.cs | 682, 687 | P0 |
| WF-06 | N+1 + O(n²) rule group loading | 🟠 HIGH | WorkFlowRunTime.cs | 388–403 | P1 |
| WF-07 | Missing reachability validation | 🟠 HIGH | WorkFlowEngine.cs | 579–599 | P1 |
| WF-08 | Duplicate job execution (multi-instance) | 🟠 HIGH | SchedulerTaskManager.cs | 72–110 | P1 |
| WF-09 | Delegation chain not transitive | 🟠 HIGH | WorkFlowEngine.cs | 166–181 | P1 |
| WF-10 | Silent exception swallowing | 🟠 HIGH | Multiple | Various | P1 |
| WF-11 | Magic -1 for NULL in history | 🟡 MEDIUM | WorkFlowRunTime.cs | 297–326 | P2 |
| WF-12 | 5-roundtrip definition load | 🟡 MEDIUM | WorkFlowRunTime.cs | 354–403 | P2 |