Workflow and Webhook Pitfalls

Workflow & Webhook Pitfalls

Real bugs found in the Workflows and Webhooks code, what caused them, and how they were fixed. Most were found not by testing but by someone asking a question that forced us to enumerate something we had never enumerated. That pattern is the lesson of this page.

They're written up because every one of them has a general shape that will recur — in this codebase and in yours.

---

1. {{event}} silently rendered as an empty string

Symptom: none. That's the point.

The send_webhook action's default body template shipped as:

{
  "event": "{{event}}",
  "ticket_id": "{{ticket.id}}",
  "subject": "{{ticket.subject}}"
}

But WorkflowEngine::dispatch() never put an event key into the payload. So every custom webhook built from the shipped default had been POSTing "event": "" — forever. Nothing errored. The receiver got a 200-shaped, well-formed JSON document with one field quietly blank.

Root cause — the general shape: renderTemplate() resolves an unknown path to an empty string, not an error. That's a reasonable choice for template engines, but it means every mistake of this class is invisible. There is no failing test to write, because nothing fails.

Fix: inject the event name into the payload at the top of runInner(), before the trigger-payload snapshot is stored (so the execution audit shows exactly what the actions saw). A host module that supplies its own event key keeps it.

How it was found: a user asked "can we only offer merge codes the workflow can actually use?" Answering that meant enumerating, per trigger, what a workflow can resolve. The moment that list existed, the shipped default failed its own check.

Takeaway. If your template engine fails soft, you have a class of invisible bugs, not an instance. Build the thing that makes the class impossible — an allow-list, a picker, a linter — rather than hunting instances.

---

2. Merge codes offered variables the trigger could never fill

Symptom: a workflow on knowledge.published whose Slack message said {{ticket.subject}} posted a blank where the subject should be.

There is no ticket in a knowledge event. The editor's hint under every variable-friendly field hardcoded "Supports variables like {{ticket.id}}, {{ticket.subject}}" — regardless of the selected trigger. So the UI was actively teaching people to write codes that could never resolve, and (see #1) the failure was silent.

Fix: WorkflowEngine::availableVariables($trigger) — a first-class catalogue of the merge codes a given trigger can actually resolve. The editor's variable picker is driven from it, so a knowledge.published workflow is never offered a ticket code, and a hand-typed one is flagged under the field ("will come out blank — not available on this trigger").

Design note — why it isn't just availableFields(): that existing method answers "what can I write a condition on" (scalars only). Merge codes are a different question: {{event}} is a valid merge code and a useless condition field; {{ticket.full}} is a whole JSON object, meaningless in a comparison. Two questions, two catalogues. Conflating them would have made both wrong.

Design note — labels are derived, not mapped: ticket.requester_email → "Ticket · Requester email" is computed from the path. A hand-written label map across a 138-trigger catalogue would rot within a month.

Open-ended shapes: a form's answers are keyed by whatever the form author called their fields, so they can't be enumerated. variablePrefixes() declares submission.fields. as an open prefix, so the unknown-variable warning doesn't cry wolf over a legitimate {{submission.fields.Start date}}.

---

3. Encrypting a value into a column too narrow to hold it

Nearly shipped. Caught before any data was written.

Webhook URLs were about to be encrypted at rest into webhook_deliveries.url, a VARCHAR(1000).

AES-256-GCM + base64 inflates a string by roughly ⅓ plus 28 bytes. A maximum-length 1000-character URL encrypts to about 1377 characters. MySQL would have silently truncated it — and a truncated ciphertext can never be decrypted. The webhook would be permanently undeliverable, and no error would have appeared at write time.

Fix: widen to VARCHAR(2000) before anything is encrypted (freeitsm.sql + an explicit MODIFY in db_verify.php, since db_verify's $schema only adds missing columns — it does not widen existing ones).

Takeaway — the general rule. Encrypting a column changes its size. Before you encrypt any existing column, compute the worst-case ciphertext length and widen first. This applies to every at-rest encryption rollout, in any codebase.

---

4. Hiding a secret in the UI would have attached it to the wrong webhook

Caught during implementation; never shipped.

What we were trying to do

Everywhere else in FreeITSM, a stored secret (an AI API key, say) is masked in the UI. You never see it again after saving — the field just shows ****abcd. If you save the form without touching it, the app understands "leave it as it was" and keeps the stored value.

The plan was to do the same for the webhook signing secret. Sensible, and consistent.

Why it breaks on a canvas

Here's the bit that wasn't obvious.

In the workflow editor, actions aren't a fixed numbered list — they're boxes you drag around a canvas. When you save, the engine reads them top to bottom, using each box's vertical position on screen to decide the order. That's the whole reason you can reorder a workflow by dragging: position is the order.

Which means "action number 1" is not a stable thing. It's just "whichever box is highest up right now".

Now picture a workflow with two webhook actions:

   canvas                     saved as
   ┌──────────────────┐
   │ Post to Slack    │  ──►  action 1   (secret: slack-secret)
   └──────────────────┘
   ┌──────────────────┐
   │ Post to Discord  │  ──►  action 2   (secret: discord-secret)
   └──────────────────┘

You open it, drag the Discord box above the Slack box, and save. Now:

   canvas                     saved as
   ┌──────────────────┐
   │ Post to Discord  │  ──►  action 1
   └──────────────────┘
   ┌──────────────────┐
   │ Post to Slack    │  ──►  action 2
   └──────────────────┘

Both secrets were masked, so the browser sent back **** for each — meaning "leave mine as it was". But the code that restored them looked up the stored secret by position: "action 1 keeps the secret that action 1 had last time."

Action 1 is now Discord. The secret that action 1 had last time was Slack's.

So Discord gets signed with Slack's secret and Slack gets signed with Discord's. Both receivers reject every message as a forgery, and the delivery log shows two perfectly well-formed webhooks being refused for no visible reason. Nothing errors. Nothing logs. You just quietly broke both integrations by dragging a box.

What we did instead

Don't mask. The URL and secret are encrypted in the database, but sent back to the editor in the clear, so there's no "leave it as it was" placeholder to map back onto anything, and nothing to get wrong.

The reasoning:

• Masking only works if every action has a stable identity that survives a save. Canvas actions don't have one — they're identified by where they happen to be sitting.
• The thing encryption-at-rest actually protects against is a stolen database or a leaked backup. It is not trying to protect a secret from the admin who's looking at their own workflow in the editor — that person could change the secret anyway.
• A secret silently attached to the wrong endpoint is a far worse outcome than an admin being able to read a secret they already own.

This is a deliberate divergence from the API-key convention, and it's written down in api/workflow/save.php so the next person doesn't "fix" the inconsistency and reintroduce the bug. If masking is ever genuinely wanted, the prerequisite is giving each action a stable ID that survives being dragged — then position stops mattering.

Takeaway. Before you key anything by position — an index, a row number, an order — ask whether that position can change underneath you. If a user can reorder the thing by dragging it, the answer is yes.

And when a convention genuinely doesn't fit, say so loudly in the code. A silently broken safety feature is worse than a consciously absent one.

---

5. "Don't store payloads" would have left the worker nothing to send

Caught during implementation.

Payload retention gained a "don't store payloads at all" option. The obvious implementation — don't write request_body at enqueue time — is wrong, because delivery is asynchronous: the worker reads the body back out of the queue to send it. Not storing it means sending an empty payload.

Fix: "don't store" means scrub the moment the delivery settles, not "never write". The body exists exactly as long as it must, and no longer.

Related: Replay re-sends the stored body. Once retention has purged it, there is nothing to re-send. Re-queueing such a row would POST an empty payload to a live endpoint — worse than refusing. So webhookReplay() excludes purged rows and webhookReplayBlockedReason() explains why, rather than failing vaguely.

Takeaway. In an async pipeline, "don't retain X" and "don't write X" are different statements. Retention is about the resting state, not the in-flight one.

---

6. A scroll-spy that killed every link it didn't understand

Symptom: clicking the HTTPS certificates link in the Workflows help sidebar did nothing. No error, no navigation.

The page's scroll-spy did this:

const navLinks = document.querySelectorAll('.wfh-nav-link');   // ← everything
navLinks.forEach(link => {
    link.addEventListener('click', function (e) {
        e.preventDefault();                                     // ← always
        const el = document.getElementById(this.dataset.section);
        if (el) { /* smooth-scroll */ }                         // ← el is null; do nothing
    });
});

The sidebar contains two kinds of link: in-page anchors (which carry data-section) and real links to other pages (which don't). The handler preventDefault()'d both, then tried to scroll to getElementById(undefined). For a real page link that means: navigation cancelled, nothing else happens.

The Webhooks deep-dive link had been dead since the day it shipped, for the same reason. Nobody had clicked it.

Fix: scope the selector to [data-section] — which the sibling page help-webhooks.php already did correctly. Checked every other help page using the broad selector (Network Mapper, Tasks): all of their nav links are in-page anchors, so they were never affected.

Takeaway. A blanket preventDefault() on a selector that's broader than the behaviour you're implementing will silently break anything else matching it. Scope event handlers to the thing they actually handle.

---

7. Cryptic transport errors that blamed the wrong thing

Symptom: Transport error: SSL certificate problem: unable to get local issuer certificate

Accurate, and useless. It says what failed, not what to do — and it implies the webhook is broken when the actual cause is that PHP has no CA bundle configured, which on a stock Windows/WAMP install is the default state. Every outbound HTTPS call in the product fails identically.

Fix: webhookDiagnoseError() recognises the TLS-trust family (and DNS / refused / timeout), returning a plain-English explanation and a deep link to a full guide → HTTPS Certificates & CA Bundles. Rendered in both places a failure surfaces: the editor's Send-test panel and the System → Webhooks delivery log.

It diagnoses at render time from the stored error, so historic failed deliveries get the explanation retroactively — no schema column, nothing to backfill.

Platform gotcha: the error wording varies by cURL build. Windows/OpenSSL says SSL certificate problem: unable to get local issuer certificate; Linux/GnuTLS says server certificate verification failed. CAfile: none. The matcher covers both — the first draft only covered one.

Takeaway. An error message that names the failure but not the remedy is only half an error message. If you can recognise a cause, say what to do about it.

---

8. The Send-test preview showed variables that don't exist in production

Symptom: none, until production. The worst kind.

The Send test button builds a sample payload so {{variables}} render to realistic values. That sample invented fields the real dispatch payload never carries:

'status'    => 'Open',          // real payload has status_id only
'priority'  => 'High',          // real payload has priority_id only
'company'   => 'Example Ltd',   // not in the payload at all
'requester' => 'Jane Requester',
'assignee'  => 'Alex Analyst',
'url'       => '…/tickets/?ticket=1024',

So you'd write {{ticket.priority}}, press Send test, watch it render "High", conclude your webhook was correct, and switch it on. In production the identical code renders an empty string — because there is no ticket.priority in a real ticket event.

This is bug #1 reached from the opposite direction. #1 was a template referencing a variable the payload lacked. This is a preview offering a variable the payload lacks. Both end in a silent blank; both were invisible.

Fix: the sample now mirrors the real payload key for key — the canonical shape being WorkflowEngine::availableFields('ticket.created') — and runs through the same enrichWithLookupNames() the engine applies at run time. What Send test previews is now exactly what production sends. ({{ticket.full}} was always genuine and is unchanged.)

How it was found: a user asked "can we insert the priority's name, not just its id?" Building that meant looking at what the sample payload actually contained — and it contained fiction.

Takeaway. A preview that can show values production won't produce is worse than no preview: it manufactures false confidence and then hands you a silent failure. Generate previews from the same definition the real thing uses, never from a hand-written mock that can drift.

---

Meta-lesson

Six of these eight were invisible in production. Nothing crashed; nothing 500'd; no test would have failed. They were bugs of the form "the system quietly did less than you thought it was doing":

• a variable that resolved to nothing,
• a link that navigated nowhere,
• a credential that wasn't encrypted,
• a payload retained longer than anyone intended,
• an error that explained nothing.

These don't get caught by testing that the happy path works. They get caught by enumerating what the system claims to support and checking each claim — which is exactly what building a picker, a catalogue, or a diagnosis table forces you to do.

Build the thing that makes the class of bug impossible, and it will hand you the instances for free.