Skip to content

DTAB-591: Stop edit-participant prefill leaking sibling participations#92

Open
hitesh-jain wants to merge 3 commits into
7.x-5.8-patchesfrom
DTAB-591-fix-edit-participant-leak
Open

DTAB-591: Stop edit-participant prefill leaking sibling participations#92
hitesh-jain wants to merge 3 commits into
7.x-5.8-patchesfrom
DTAB-591-fix-edit-participant-leak

Conversation

@hitesh-jain
Copy link
Copy Markdown

@hitesh-jain hitesh-jain commented May 22, 2026

Overview

A webform set up to let an admin open and edit someone's existing event registration could quietly pre-fill the form with data from a different event the same person was registered for, not the one the admin actually opened. Saving the form would then overwrite the correct record with the wrong values — silently, with no warning that anything was off.

Bug flow

Before — wide query lets a sibling participation overwrite the prefill:

sequenceDiagram
  autonumber
  participant Admin
  participant Webform as webform_civicrm
  participant CiviCRM
  Admin->>Webform: GET ?cid1=A&event1=E1
  Note over Webform: $this->events is the entire<br/>event catalogue of the<br/>configured event_type
  Webform->>CiviCRM: Participant.get contact_id=A,<br/>event_id IN (E1, E2, ... every type-match)
  CiviCRM-->>Webform: P1(E1, Registered)<br/>P2(E2, Attended, custom data)
  Note over Webform: Status filter keeps P2<br/>(Attended is in the dropdown)
  Webform-->>Admin: Form pre-filled from P2 ❌<br/>(wrong event's data)
Loading

After — query narrows to the URL-pinned event:

sequenceDiagram
  autonumber
  participant Admin
  participant Webform as webform_civicrm
  participant CiviCRM
  Admin->>Webform: GET ?cid1=A&event1=E1
  Note over Webform: getUrlPinnedEventIds() → [E1]<br/>intersect with $this->events → [E1]
  Webform->>CiviCRM: Participant.get contact_id=A,<br/>event_id IN (E1)
  CiviCRM-->>Webform: P1(E1, Registered)
  Webform-->>Admin: Form pre-filled from P1 ✓
Loading

Before

For a contact with participations in multiple events of the same event_type_id:

  • The form prefilled the participant status (the participant_status_id dropdown — typically a restricted set like Attended / Cancelled / Trainer Paid) with a sibling participation's status (e.g. "Attended") instead of the URL-pinned participant's actual status.
  • Participant-level custom fields (CiviCRM custom group columns attached to the participant row, e.g. a multi-select for H&S training selections) were populated from the sibling's record.
  • Submitting the form would write those values onto the URL-pinned participant — silent data corruption: the wrong row gets updated with values from a different event.

After

  • The form only prefills from the participant matched by the URL event pin (event{n} / c{c}event{n} — explained below under Triggering URLs).
  • Sibling participations of the same contact in other events no longer leak status, role, or custom data into the form.
  • Public registration forms with no URL event pin behave exactly as before.

Technical Details

Affected webform_civicrm configuration (click to expand)

The bug only triggers when all of the following are true on a given webform:

  • reg_options.allow_url_load = TRUE — webform_civicrm setting on the Settings tab labelled "Allow URL Loading". It opts the form into reading cid / event{n} / c{c}event{n} from the URL at render time to prefill existing CiviCRM data. Forms without this setting never read URL event pins, so the wide query never runs in a way that matters.
  • Form has at least one participant_event_id component, rendered as a hidden field with no static items. This is the webform component that holds the event id for a given participant slot. When it's type=hidden with no extra.items, webform_civicrm resolves its option list live at render time via wf_crm_get_events($reg_options) — which returns every active event whose event_type_id matches the form's reg_options.event_type filter.
  • participant_status_id exposed with a restricted set of options (e.g. only Attended / Cancelled / Trainer Paid). participant_status_id is the CiviCRM enum for participant statuses (Registered, Attended, Cancelled, No-show, etc). The status filter inside loadParticipants() keys off the form's exposed options — that's what allows a sibling participation with one of those statuses to overwrite the prefill. Forms exposing every status, or no status field at all, behave differently and weren't observed leaking.
  • At least one contact configured with number_of_participant >= 1 (webform_civicrm data setting that says "contact N has at least one participant slot on the form").

Triggering URLs

The URL convention webform_civicrm reads depends on the form's participant_reg_type setting, which decides how multiple contacts on the same form relate to events:

  • participant_reg_type = 'all' — every contact on the form registers for the same set of events. The URL pin for the event applies to everyone, so it's a single param per participant slot: event{n}.

    ?cid1=<contact_id>&event1=<event_id>
    
  • participant_reg_type = 'separate' — each contact has their own independent event(s). The URL pin needs to address a specific contact's participant slot, so the param includes the contact index: c{c}event{n}.

    ?cid1=<contact_id>&c1event1=<event_id>
    

Legacy aliases also supported:

  • ?cid=<contact_id> — treated as cid1 for backward compatibility.
  • ?event1=<event_id> — now also honored for contact 1 on participant_reg_type = separate forms with this patch (admin URLs that set event1 regardless of reg type narrow the prefill correctly).

Why this matters

Forms that expose a hidden participant_event_id field resolve its options via wf_crm_get_events($reg_options). That function returns every active event of the configured type, so $this->events (the internal map of events the form knows about) ends up holding the whole event catalogue, not just the events the form actually cares about for prefill.

Root cause

wf_crm_webform_preprocess::loadParticipants() (the routine that prefills existing participant data when a form opens for a known contact) queries Participant.get with event_id IN array_keys($this->events). With the wide events list, that returns every participation the contact has across all those events, not just the one pinned in the URL. The per-row status filter inside the function then accepts any record whose status_id is in the form's exposed dropdown options, so the last-iterated Attended / Cancelled / Trainer-Paid sibling overwrites the prefill. End result: the form shows that sibling's data, not the participant the URL pinned.

Worked example

Contact A has:

  • Participant P1 in Event E1, status Registered, no custom data.
  • Participant P2 in Event E2 (same event type as E1), status Attended, with participant custom values populated (e.g. H&S training selections).

Admin opens an edit-participant webform with URL ?cid1=A&event1=E1.

Expected Form prefills from P1 (empty status, empty custom fields).
Actual (before this patch) Form prefills from P2 (Attended + the H&S checkboxes ticked).
Save action P2's values get written onto P1's record — silent corruption.
Actual (after this patch) Form prefills from P1. ✓

The fix

  • Adds private helper getUrlPinnedEventIds($c) that scans $_GET for both URL conventions used by loadURLEvents(): event{n} (used by participant_reg_type = 'all') and c{c}event{n} (used by 'separate'). For contact 1 it also accepts the 'all' convention even on separate forms, so admin URLs that set event1 regardless of reg type still narrow the prefill.
  • In loadParticipants($c), when reg_options.allow_url_load is on and getUrlPinnedEventIds() returns ids, narrow the participant query to the intersection of $this->events and the URL-pinned ids. Falls back to the original wide query when the intersection is empty so a stray URL pin cannot silently zero out the prefill.
  • No behaviour change when no URL event pin is present.
  • Performance side-effect: when the URL pins a single event, the Participant.get query narrows from "every type-matching event in CiviCRM" to a single id. Materially cheaper on sites with many events of the configured type, but unchanged when no URL pin is present.

Verification

Test data and scenarios

Setup: Contact A registered for two events E1 and E2 of the same event type. Form's status dropdown exposes a restricted set (e.g. Attended / Cancelled / Trainer Paid). Webform configured with allow_url_load = TRUE, participant_reg_type = separate, one contact + one participant.

# Scenario Setup URL Before patch After patch
1 Leak case: Registered + Attended sibling A has P1 in E1 (Registered) and P2 in E2 (Attended + participant custom values) ?cid1=A&event1=E1 Form shows P2's "Attended" status and P2's custom values Status and custom fields empty; P2's data no longer leaks ✓
2 Pinned participant's own data still loads A has P3 in E1 (Attended + custom values), no siblings ?cid1=A&event1=E1 Form correctly shows P3's data Same — no regression ✓
3 Multiple Attended siblings A has P_curr (E1 Registered), P_x (E2 Attended), P_y (E3 Attended) with different custom values ?cid1=A&event1=E1 Form prefilled with whichever sibling API returned last (non-deterministic) Query narrowed to E1; only P_curr considered; no leak ✓
4 No URL event pin (public registration form) Form rendered without event{n} / c{c}event{n} in URL (no event param) Wide query (existing behaviour) Same — getUrlPinnedEventIds() returns empty, short-circuit ✓
5 event1 on participant_reg_type = separate form Admin URL uses event1 instead of c1event1 on a separate-reg form ?cid1=A&event1=E1 loadURLEvents() ignores event1; prefill stays wide; leak still occurs getUrlPinnedEventIds() accepts event1 for contact 1 regardless of reg type; narrows correctly ✓

Comments

Tip

The fix is conservative: gated behind allow_url_load AND the presence of event{n} / c{c}event{n} URL params. Forms without URL pins behave exactly as before.

  • The accept-both-conventions logic in getUrlPinnedEventIds() also resolves a latent inconsistency where admin URLs using event1 on a participant_reg_type = separate form were not honored by loadURLEvents() either.
  • The per-row status filter inside loadParticipants() is unchanged — participants whose status_id isn't in the form's exposed dropdown still get skipped. This patch only narrows which participants are considered, not how each is then filtered.
  • Backward compatibility verified for the early-return paths in prepareForm() (resumed drafts, multi-page submissions, edit-op with an existing #submission): all three short-circuit before loadParticipants() is called, so the patch has no effect on them.
  • A parallel fix against the D8 branch (8.x-5.x) will be raised as a follow-up. The bug shape is likely the same but requires reproduction + a regression test in the D8 test suite before opening upstream / fork PRs.

When allow_url_load is on and the URL pins specific events (event{n} or
c{c}event{n}), narrow loadParticipants() to those events instead of
querying across every type-matching event in CiviCRM.

Without this, a sibling participation of the same contact in another
event whose status is in the form's exposed status dropdown overwrites
the form defaults, so edit-participant flows show the wrong participant's
status and custom data and can silently corrupt records on save.

Falls back to the original wide query when the intersection is empty.
No behaviour change when no URL event pin is present.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the participant loading process to narrow prefill lookups based on event IDs provided in the URL when the 'allow_url_load' setting is active. This prevents unrelated event data from overwriting form defaults. A new helper method, getUrlPinnedEventIds, was added to parse these IDs. Feedback from the reviewer suggests refactoring this new method to consolidate duplicated logic used for parsing different URL parameter formats, which would improve code maintainability.

Comment thread includes/wf_crm_webform_preprocess.inc
Address review feedback (PR #92): the two regex branches in
getUrlPinnedEventIds were each running the same explode/foreach to
collect ids. Merge them into a single branch gated by either match.
@hitesh-jain hitesh-jain marked this pull request as ready for review May 22, 2026 12:03
Address review feedback (PR #92): $_GET parameters can come in as
arrays (e.g. ?event1[]=123), and casting an array to string in PHP 8+
emits a deprecation warning. Check for array first and skip the
explode in that case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants