Preserve full Parse object in webhooks (v5.2.1)#13
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the webhook trigger payload pipeline so webhook handlers can observe a more server-faithful Parse::Object (timestamps/ACL/etc.) and adds regression coverage to lock in afterSave semantics (new vs. update + dirty tracking). It also bumps the gem version to 5.2.1 and updates documentation/changelog to describe the new behavior.
Changes:
- Replace broad webhook payload scrubbing with a credential-only scrub and adjust hydration to preserve more server-sent fields.
- Add afterSave update “overlay” logic intended to enable dirty tracking (
*_changed?,changed,changes) inside afterSave handlers. - Add unit + Docker integration test suites covering full-object fidelity and afterSave state semantics; update docs/changelog/version accordingly.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
lib/parse/webhooks/payload.rb |
Narrows webhook payload scrubbing and changes hydration/afterSave overlay behavior to preserve server fields and enable dirty tracking. |
test/lib/parse/webhook_aftersave_state_integration_test.rb |
New Docker end-to-end integration coverage capturing afterSave object state across create/update flows. |
test/lib/parse/webhook_aftersave_payload_fidelity_test.rb |
New unit-level regression suite asserting full-object fidelity + afterSave dirty tracking behavior. |
test/lib/parse/security_hardening_test.rb |
Updates security expectations around webhook payload scrubbing and “write-side” defenses. |
test/lib/parse/agent_integration_test.rb |
Adjusts schema error assertion text expectation. |
docs/mcp_guide.md |
Documents listening-stream ownership model and principal_resolver: behavior. |
CHANGELOG.md |
Adds 5.2.1 entry documenting webhook payload fidelity + afterSave dirty tracking. |
lib/parse/stack/version.rb |
Bumps version to 5.2.1. |
Gemfile.lock |
Updates locked gem version to 5.2.1. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
91
to
97
| if hash[:user].present? | ||
| @user = Parse::User.new(self.class.scrub_protected_keys(hash[:user])) | ||
| # Trusted hydration via .build (not .new) so server-sent authData / | ||
| # roles / timestamps remain readable; credentials are removed first. | ||
| # The built object is pristine, so a handler that saves payload.user | ||
| # transmits nothing (no dirty changes) and cannot persist forgeries. | ||
| @user = Parse::User.build(self.class.scrub_credentials(hash[:user])) | ||
| end |
Comment on lines
+81
to
+86
| # Webhook trigger payloads (beforeSave/afterSave/etc.) are delivered by | ||
| # Parse Server itself and authenticated by the webhook key, so they are | ||
| # trusted, server-authoritative state. A handler is meant to receive the | ||
| # FULL object -- createdAt/updatedAt, ACL, authData, roles, _rperm/_wperm, | ||
| # internal fields and all. The only thing stripped here is genuine | ||
| # credential material a handler never legitimately needs to read (live |
Comment on lines
+304
to
+317
| # afterSave on an UPDATE: build the prior state, then overlay the final | ||
| # state with dirty tracking so `*_changed?` / `changes` work inside | ||
| # afterSave handlers (symmetric with the beforeSave path above). The | ||
| # filter uses the timestamp-preserving INITIALIZE key set rather than the | ||
| # wide mass-assignment set: the wide set would strip the incoming | ||
| # `updatedAt` from the overlay, leaving the prior `updatedAt` and breaking | ||
| # `existed?`. The diff still excludes credentials / _rperm / _wperm / | ||
| # authData / roles, and an after-trigger response is only true/false, so | ||
| # there is no path for a forged privileged field to be persisted. | ||
| if after_save? && @original.present? && @original.is_a?(Hash) | ||
| o = Parse::Object.build @original, parse_class | ||
| o.apply_attributes! @object, dirty_track: true, | ||
| protected_set: Parse::Properties::PROTECTED_INITIALIZE_KEYS | ||
| return o |
Comment on lines
+294
to
+300
| assert_empty missing, | ||
| "Webhook object dropped server-sent field(s) #{missing.inspect}. " \ | ||
| "Parse Server sent #{raw_object_hash.keys.sort.inspect}; the built " \ | ||
| "object must expose every data field. A non-empty diff means " \ | ||
| "over-broad scrubbing was (re)introduced on the webhook path " \ | ||
| "(see Parse::Webhooks::Payload#scrub_protected_keys)." | ||
| end |
Comment on lines
+191
to
+195
| # NOTE: several assertions below are SPEC assertions for the proposed fix | ||
| # (the webhook path must stop stripping server-authoritative | ||
| # createdAt/updatedAt via Parse::Webhooks::Payload#scrub_protected_keys). | ||
| # They FAIL on current HEAD and pass once the scrub is narrowed. The unit | ||
| # suite in webhook_aftersave_payload_fidelity_test.rb documents the same |
Comment on lines
+100
to
+107
| # ---- DIAGNOSTIC PASS: capture, print, do not assert behavior we are unsure | ||
| # of. Convert to hard assertions once the actual values are known. ---- | ||
|
|
||
| def test_new_object_detection_signals_on_nonmaster_create | ||
| captured = {} | ||
|
|
||
| Parse::Webhooks.route(:after_save, "WebhookStatePost") do | ||
| obj = parse_object |
Comment on lines
+146
to
+154
| def test_change_detection_signals_on_nonmaster_update | ||
| # Seed an object as master so it already exists, then mutate one field | ||
| # as a non-master client and inspect dirty state in afterSave. | ||
| id = master_create("WebhookStatePost", { "title" => "original-title", "status" => "draft", "body" => "v1" }) | ||
|
|
||
| captured = {} | ||
| Parse::Webhooks.route(:after_save, "WebhookStatePost") do | ||
| obj = parse_object | ||
| orig = original_parse_object |
Comment on lines
+315
to
+316
| o.apply_attributes! @object, dirty_track: true, | ||
| protected_set: Parse::Properties::PROTECTED_INITIALIZE_KEYS |
Comment on lines
+2076
to
+2092
| # Helper to run the :before callbacks from a compiled list, in registration | ||
| # order, stopping and returning false if any returns false (matching the | ||
| # model's `result == false` terminator). Unlike `run_callbacks(:save) { ... }` | ||
| # this runs ONLY the before phase, never the after-phase callbacks that a | ||
| # block-wrapped run_callbacks would also fire. | ||
| def run_before_callbacks_from_list(callbacks) | ||
| callbacks.each do |callback| | ||
| next unless callback.kind == :before | ||
| result = case callback.filter | ||
| when Symbol then send(callback.filter) | ||
| when Proc then instance_exec(&callback.filter) | ||
| when String then instance_eval(callback.filter) | ||
| else true | ||
| end | ||
| return false if result == false | ||
| end | ||
| true |
| "Parse Server sent #{raw_object_hash.keys.sort.inspect}; the built " \ | ||
| "object must expose every data field. A non-empty diff means " \ | ||
| "over-broad scrubbing was (re)introduced on the webhook path " \ | ||
| "(see Parse::Webhooks::Payload#scrub_protected_keys)." |
Comment on lines
+191
to
+194
| # NOTE: several assertions below are SPEC assertions for the proposed fix | ||
| # (the webhook path must stop stripping server-authoritative | ||
| # createdAt/updatedAt via Parse::Webhooks::Payload#scrub_protected_keys). | ||
| # They FAIL on current HEAD and pass once the scrub is narrowed. The unit |
AdrianCurtin
force-pushed
the
dev
branch
5 times, most recently
from
0c9a35e to
8399fcb
Compare
Treat webhook trigger payloads as server-authoritative: preserve timestamps, ACL, authData, roles and internal fields while stripping only genuine credentials (sessionToken, _hashed_password, _password_history). Replace broad mass-assignment scrubbing with a targeted scrub_credentials path, hydrate trusted objects with .build so handlers can read createdAt/updatedAt and auth fields, and enable dirty-tracking overlay for afterSave updates so *_changed?/changes work inside afterSave. Updates include: changelog and version bump to 5.2.1, docs describing listening-stream ownership, payload parsing changes in lib/parse/webhooks/payload.rb, security test adjustments, and a new suite of unit and integration tests that lock in full-object webhook fidelity and write-side guarantees.
Comment on lines
+317
to
+321
| if after_save? && @original.present? && @original.is_a?(Hash) | ||
| o = Parse::Object.build @original, parse_class | ||
| o.apply_attributes! @object, dirty_track: true, | ||
| protected_set: Parse::Properties::PROTECTED_INITIALIZE_KEYS | ||
| return o |
Comment on lines
+4830
to
+4832
| You can register webhooks to handle the different object triggers: `:before_save`, `:after_save`, `:before_delete` and `:after_delete`. The `payload` object, which is an instance of `Parse::Webhooks::Payload`, contains several properties that represent the payload. One of the most important ones is `parse_object`, which will provide you with the instance of your specific Parse object. | ||
|
|
||
| The `parse_object` handed to your handler is the **full object as Parse Server sent it** — `createdAt`/`updatedAt`, `ACL`, and internal fields all survive (only live credentials — session tokens and password hashes — are stripped; `Parse::User` additionally protects `authData` on `payload.user`). Both `:before_save` and `:after_save` objects carry **dirty tracking** of what changed (`name_changed?`, `changes`), and `Parse::Object#existed?` / `#new?` are reliable inside `:after_save`. See [Trigger object state](#trigger-object-state) below. |
Comment on lines
+118
to
+122
| captured[:fired] = true | ||
| captured[:payload_original_nil] = original.nil? | ||
| raw_obj = @raw.is_a?(Hash) ? @raw[:object] : nil | ||
| captured[:raw_object_keys] = raw_obj.is_a?(Hash) ? raw_obj.keys.sort : nil | ||
| captured[:raw_createdAt] = raw_obj.is_a?(Hash) ? raw_obj["createdAt"] : nil |
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.
Treat webhook trigger payloads as server-authoritative: preserve timestamps, ACL, authData, roles and internal fields while stripping only genuine credentials (sessionToken, _hashed_password, _password_history). Replace broad mass-assignment scrubbing with a targeted scrub_credentials path, hydrate trusted objects with .build so handlers can read createdAt/updatedAt and auth fields, and enable dirty-tracking overlay for afterSave updates so *_changed?/changes work inside afterSave. Updates include: changelog and version bump to 5.2.1, docs describing listening-stream ownership, payload parsing changes in lib/parse/webhooks/payload.rb, security test adjustments, and a new suite of unit and integration tests that lock in full-object webhook fidelity and write-side guarantees.