fix(workflow): let a webhook body fill a built-in trigger field it names - #5922
Open
Chessing234 wants to merge 3 commits into
Open
fix(workflow): let a webhook body fill a built-in trigger field it names#5922Chessing234 wants to merge 3 commits into
Chessing234 wants to merge 3 commits into
Conversation
`TriggerContext::get_field` matched the built-in names before consulting
`webhook_fields`, so a webhook body key called `text` (or `author`,
`timestamp`, `emoji`, `message_id`) resolved to the built-in's empty
default and the posted value was unreachable from a template. A webhook
trigger populates none of those fields, so `{{trigger.text}}` rendered
empty with no error and the line silently vanished from the message.
Fall back to `webhook_fields` only when the built-in is empty, so a body
still cannot spoof a field the trigger actually set.
Signed-off-by: Taksh <takshkothari09@gmail.com>
`evaluate_condition` registered the webhook fields as `trigger_<key>` and then overwrote them with the built-ins, including the empty ones. So a condition on `trigger_text` for a webhook trigger compared against "" even though the body carried a `text` key, while a template on the same field now reads it. Resolve both through `get_field` so the two agree; a populated built-in still wins, which is what the ordering was there to guarantee. Signed-off-by: Taksh <takshkothari09@gmail.com>
Templates and conditions both: a body key named `text` resolves for a webhook trigger, a body key cannot shadow a built-in the trigger set, and an absent field still resolves empty rather than being left literal. Signed-off-by: Taksh <takshkothari09@gmail.com>
themiguelamador
approved these changes
Aug 16, 2026
themiguelamador
left a comment
There was a problem hiding this comment.
No findings. I traced every TriggerContext producer and verified the precedence change across event, manual, scheduled, and webhook runs. The linked issue's text reproduction is fixed for both templates and condition evaluation, while populated server-derived fields such as author and channel_id still take precedence over same-named payload fields.
Verified with all 160 runnable buzz-workflow library tests (2 Postgres-only tests ignored by the crate), strict Clippy, rustfmt, and git diff --check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5769.
TriggerContext::get_fieldmatches the built-in names before it consultswebhook_fields, so a webhook body key calledtext— orauthor,timestamp,emoji,message_id— resolves to the built-in's empty default. A webhook trigger populates none of those, so{{trigger.text}}renders empty, the run still succeeds, and the posted value simply vanishes from the message. Renaming the key to anything non-reserved works, which is what makes it look like a template bug rather than a shadowing one.The fall-back to
webhook_fieldshappens only when the built-in is empty, so a body still cannot spoof a field the trigger actually set — amessage_postedtrigger'sauthorstays authoritative even if the payload names one. There is a test for exactly that.evaluate_conditionhad the same split: it registered the webhook fields astrigger_<key>and then overwrote them with the built-ins, including the empty ones, so a condition ontrigger_textcompared against""while a template on the same field would now read the body. Both go throughget_fieldnow, so the two agree.Related: #3503 fixes the manual-trigger half of this in
command_executor.rsby mapping known--inputskeys onto the built-in fields. This is the same root cause a layer down — withget_fieldfixed, the manual path resolves too, without the caller-side mapping.Verified locally:
cargo test -p buzz-workflow --lib(160 passed, 5 of them new),cargo clippy -p buzz-workflow --all-targetsclean,cargo fmt --all --checkclean. I did not run a webhook end to end against a relay — the two new relay-facing claims (the body lands inwebhook_fields,channel_idis the only built-in a webhook populates) are read frombridge.rs:1879-1894.