diff --git a/CHANGELOG.md b/CHANGELOG.md index e1c1e52..6063021 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,90 @@ ## parse-stack-next Changelog +### 5.2.1 + +#### Webhook trigger handlers now receive the full Parse object + +Webhook trigger payloads (`beforeSave`/`afterSave`/`beforeDelete`/`afterDelete`) +are delivered by Parse Server and authenticated by the webhook key, so they are +trusted, server-authoritative state. Previously the payload was filtered through +the wide mass-assignment denylist, which stripped server-issued +`createdAt`/`updatedAt` (and other non-credential fields) before the handler +could see them. That broke `Parse::Object#existed?` and `#new?` inside +`afterSave` handlers — `existed?` always returned `false` and `new?` always +returned `true`, regardless of whether the object was created or updated — and +hid the object's timestamps and ACL from handler code. + +- **FIXED**: `afterSave`/`beforeSave` handlers now receive the full object as + Parse Server sends it (`createdAt`, `updatedAt`, `ACL`, internal fields). + `Parse::Object#existed?` and `#new?` are now reliable inside `afterSave` + handlers. (Genuine credentials — session tokens and password hashes — are + still stripped, and `Parse::User` continues to protect `authData` on + `payload.user`.) +- **NEW**: `afterSave` handlers on an updated object now carry dirty tracking + relative to the prior state, so `title_changed?`, `changed`, and `changes` + work inside `afterSave` the same way they already did inside `beforeSave`. +- **CHANGED**: Inbound webhook trigger payloads are now scrubbed of genuine + credential material only (`sessionToken`, `_hashed_password`, + `_password_history`) rather than the full mass-assignment denylist. Protection + against persisting forged privileged fields remains on the write path: a save + emits only declared, dirty-tracked properties, and an after-trigger response + is `true`/`false`, so forged `_rperm`/`_wperm`/`authData` cannot be persisted + through a handler. This applies only to the inbound webhook trigger payload; + client login/signup responses are unaffected and still return session tokens. +- **CHANGED**: In an `afterSave` handler, `new?` now correctly returns `false` + (the object is already persisted) where the previous timestamp-stripping bug + made it return `true`. Use `existed?` to distinguish create from update inside + `afterSave` (`existed? == false` for a create, `true` for an update); `new?` + is intended for `beforeSave`. +- **CHANGED**: Dirty-gated `after_save` side effects now fire on client/REST- + initiated saves where they previously silently no-op'd. With timestamps and + dirty tracking restored, a callback such as `after_save { notify if + title_changed? }` will now activate for objects created or updated via REST / + JS cloud code, not only for Ruby-model saves. + +```ruby +Parse::Webhooks.route :after_save, "Post" do + post = parse_object + + if post.existed? # now reliable: false on create, true on update + NotificationService.changed(post) if post.title_changed? + else + post.create_default_associations! + end + true +end +``` + +#### Lifecycle callbacks run in ActiveModel order for client-initiated saves + +Parse Server exposes no separate `beforeCreate`/`afterCreate` triggers — only +`beforeSave` and `afterSave`. The webhook layer now runs the model lifecycle +callbacks for a client-initiated create in the canonical ActiveModel order: +`before_save` → `before_create` (in the `beforeSave` webhook) then +`after_create` → `after_save` (in the `afterSave` webhook). + +- **FIXED**: `before_create` callbacks now run for client/REST/JS/Auth0-created + objects. The `beforeSave` webhook runs `before_create` after `before_save` for + new objects (an object with no `original`); previously `before_create` never + fired for non-Ruby creates, so create-time setup written as `before_create` + was silently skipped. +- **FIXED**: `after_save` no longer double-fires on client-initiated saves. The + `beforeSave` webhook entry point previously ran the full save callback chain, + firing `after_save` during `beforeSave` in addition to the `afterSave` + webhook. It now runs the before phase only. +- **NEW**: `Parse::Object#run_before_save_callbacks` and + `#run_before_create_callbacks` — the before-phase counterparts to the existing + `run_after_save_callbacks` / `run_after_create_callbacks`. +- **CHANGED**: `Parse::Object#prepare_save!` is retained as a back-compat alias + for `run_before_save_callbacks` and now runs the before phase only (it no + longer also fires `after_save`). The before-phase runners honor `:if`/`:unless` + callback conditions and the callback terminator. +- **NOTE**: the webhook layer runs `before_save`/`before_create` and + `after_create`/`after_save`, but not `before_update`/`after_update` — those + `:update`-specific callbacks fire only on Ruby-model saves, not for + client-initiated (REST/JS/Auth0) saves. Use `before_save`/`after_save` (which + run for every save) and branch on `existed?` if you need update-only logic. + ### 5.2.0 #### Retrieval layer — `Parse::Retrieval` (`Parse::RAG`) diff --git a/Gemfile.lock b/Gemfile.lock index 1f5b71f..24f0fd1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - parse-stack-next (5.2.0) + parse-stack-next (5.2.1) activemodel (>= 6.1, < 9) activesupport (>= 6.1, < 9) connection_pool (>= 2.2, < 4) diff --git a/README.md b/README.md index eac3f50..a6bf3a1 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,7 @@ A full-featured Ruby client SDK for [Parse Server](http://parseplatform.org/). [ ### What's new in 5.2 +- **5.2.1 — Webhook triggers receive the full Parse object** — trigger handlers (`beforeSave`/`afterSave`/…) now get the complete server object (`createdAt`/`updatedAt`, `ACL`, internal fields); only live credentials (session tokens, password hashes) are stripped. `Parse::Object#existed?` / `#new?` are reliable in `afterSave`, `afterSave` updates carry dirty tracking, and the model lifecycle runs in ActiveModel order — `before_save → before_create` then `after_create → after_save` — so `before_create` now fires for REST/JS/Auth0 creates (and `after_save` no longer double-fires). See [Cloud Code Triggers](#cloud-code-triggers) - **Retrieval layer — `Parse::Retrieval` (`Parse::RAG`)** — `Parse::Retrieval.retrieve(query:, klass:, k:, filter:, tenant_scope:, …)` embeds a natural-language query, runs Atlas `$vectorSearch` through the existing ACL-enforcing `find_similar`, and splits each retrieved document's text field into scored `Parse::Retrieval::Chunk`s. Chunking is presentation-only (embedding stays one-vector-per-record), via `Parse::Retrieval::Chunker::FixedSizeOverlap(size:, overlap:, by:, max_chunks_per_document:)` (subclass `Chunker::Base` for custom strategies). ACL is mongo-direct (no REST two-stage); tenant scope folds into the Atlas pre-filter - **`semantic_search` agent tool + `agent_searchable`** — declare `agent_searchable field:, filter_fields:` on a model to expose it to the readonly, client-safe `semantic_search` tool. The handler enforces the full agent envelope: searchable-class allowlist, recursive underscore-key refusal + filter-field allowlist on input, `field_allowlist` projection plus tenant-scope re-assertion on output, and score quantization in non-admin contexts - **MCP elicitation — human-in-the-loop approval** — opt in with `Parse::Agent.require_approval_for = [:write, :admin]` to require spec-native `elicitation/create` approval before destructive tool calls. A pluggable `agent.approval_gate` (reachable on the non-MCP path too) shows the dry-run diff and blocks on the client's reply; `call_method` resolves the *effective* tier from the target `agent_method`. Fails closed (no capability / no listening stream / non-streaming transport / timeout → refuse); replies are session-bound @@ -397,6 +398,7 @@ The 1.x line is the original [`modernistik/parse-stack`](https://github.com/mode - [Cloud Code Webhooks](#cloud-code-webhooks) - [Cloud Code Functions](#cloud-code-functions) - [Cloud Code Triggers](#cloud-code-triggers) + - [Trigger object state](#trigger-object-state) - [Mounting Webhooks Application](#mounting-webhooks-application) - [Register Webhooks](#register-webhooks) - [Parse REST API Client](#parse-rest-api-client) @@ -4825,7 +4827,9 @@ end ``` ### Cloud Code Triggers -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. In `:before_save` triggers, this object already contains dirty tracking information of what has been changed. +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. ```ruby # recommended way @@ -4862,6 +4866,60 @@ For any `after_*` hook, return values are not needed since Parse does not utiliz > for saves from other clients (JS / iOS / REST), the webhook runs them, since > the SDK never had the chance. +#### Trigger object state + +Because the trigger payload is server-authoritative, the `parse_object` your +handler receives is the complete object, and the usual `Parse::Object` +introspection works inside the trigger: + +| What you want to know | In `:before_save` | In `:after_save` | +|---|---|---| +| Is this a create or an update? | `parse_object.new?` (`true` = create) | `parse_object.existed?` (`false` = create) or `payload.original.nil?` | +| What changed? | `name_changed?`, `changes`, `changed` | `name_changed?`, `changes`, `changed` (relative to the prior state) | +| Server timestamps | not yet assigned (`new?` create) | `created_at` / `updated_at` populated | +| The prior stored values | `payload.original_parse_object` | `payload.original_parse_object` | + +Use `new?` in `:before_save` and `existed?` in `:after_save`. In `:after_save` +the object is already persisted, so `new?` is `false` for both creates and +updates — `existed?` (`created_at != updated_at`) is the create/update signal, +equivalently `payload.original.nil?`. + +```ruby +Parse::Webhooks.route :after_save, :Post do + post = parse_object + if post.existed? + Search.reindex(post) if post.title_changed? # update + else + post.create_default_associations! # first save + end + true +end +``` + +**Lifecycle callback order.** Parse Server has no separate `beforeCreate` / +`afterCreate` triggers — only `beforeSave` and `afterSave`. The SDK runs your +model's ActiveModel callbacks in canonical order across the two webhooks: + +``` +beforeSave webhook : before_save → before_create (before_create only for new objects) + [Parse Server persists] +afterSave webhook : after_create → after_save (after_create only for new objects) +``` + +So a model `before_create` / `after_create` callback runs for objects created by +**any** client (REST / JS cloud code / Auth0 / iOS), not just Ruby-model saves — +provided the corresponding trigger is registered with Parse Server (see +[Register Webhooks](#register-webhooks)). These callbacks fire **once** per save; +Ruby-SDK-initiated saves run them locally and the webhook skips them to avoid +double-firing. `:if`/`:unless` conditions on these callbacks are honored. + +> **`before_update` / `after_update` do not run from webhooks.** The webhook +> layer runs `before_save` / `before_create` / `after_create` / `after_save` +> only. The `:update`-specific callbacks fire on Ruby-model saves but **not** +> for client-initiated (REST / JS / Auth0) saves, because Parse Server has no +> `beforeUpdate` / `afterUpdate` trigger. For update-time logic that must run +> for all clients, use `before_save` / `after_save` and branch on `existed?`. + > **Keep `after_save` handlers fast.** Parse Server **waits** for the `after_save` > webhook response before returning to the saving client (only LiveQuery events > are truly fire-and-forget), so a slow handler adds latency to that client's diff --git a/docs/mcp_guide.md b/docs/mcp_guide.md index c2d3853..b2aa806 100644 --- a/docs/mcp_guide.md +++ b/docs/mcp_guide.md @@ -515,10 +515,14 @@ Parse Server version and its `masterKeyIps` configuration.) - **Subscriptions do not survive a listening-stream reconnect.** Closing the `GET` stream tears down the session's LiveQuery subscriptions; a client that reconnects must re-issue its `resources/subscribe` calls. -- **Session id is a bearer capability.** The listening stream authenticates via - the agent factory and keys delivery off the server-issued `Mcp-Session-Id`, - which the client must keep secret — possession of a valid session id (plus a - valid agent) is sufficient to attach. This matches the cancellation model. +- **Listening streams are owner-bound (not a bare bearer capability).** The + stream authenticates via the agent factory *and* the server-issued + `Mcp-Session-Id` is bound to the principal that established it, so another + authenticated caller who knows or guesses the id is refused with `403`. The + `Mcp-Session-Id` is still secret-bearing and should be kept confidential, but + possession alone is no longer sufficient — see **Listening-stream ownership** + below for the binding model, its limits, and the `principal_resolver:` knob + master-key deployments need to make it effective. - **Per-session and global caps.** A client that subscribes but never opens (or later drops) its listening stream leaves LiveQuery subscriptions running until the session is torn down. A per-session ceiling (default 100, @@ -538,6 +542,73 @@ Parse Server version and its `masterKeyIps` configuration.) one-time warning at construction when a streaming or subscription/notification surface is enabled without a cap. +### Listening-stream ownership + +The GET listening stream is the single server→client bus shared by resource +subscriptions, [server-initiated notifications](#server-initiated-notifications-general-purpose), +and [approval elicitation](#approval-workflows-mcp-elicitation). Whoever holds +that stream receives everything pushed to its `Mcp-Session-Id` — another +session's `notifications/resources/updated`, `elicitation/create` approval +prompts, and arbitrary `notify` payloads. So the stream is **owner-bound**: a +session is tied to the principal that established it, and only the same +principal may later open (or re-open) its stream. + +How the binding is established and checked: + +- **Initialize-bound.** A session created through an `initialize` POST is bound + authoritatively to that caller's principal. A later `GET` carrying the same + `Mcp-Session-Id` from a *different* principal is refused with HTTP `403` + (`-32600`, "Mcp-Session-Id is owned by another principal"). A re-`initialize` + by the same caller refreshes the binding. +- **Trust-on-first-use (TOFU) for the decoupled bus.** A session id that + `initialize` never saw — the `notifications: true` bus, where application code + pushes to ids it chose itself — is claimed by the first principal to attach a + listener; a different principal attaching afterward is refused. TOFU closes + the prior model's eviction-after-claim hole (a second caller could overwrite + or shadow an existing listener), but a first-mover attacker can still claim an + *unused* id, so **notification-bus session ids must be high-entropy**. +- **Stream close keeps the claim.** The binding is dropped only on an explicit + `DELETE` termination, not on mere stream close — a reconnecting owner keeps + its claim, and an attacker cannot grab the id during a brief disconnect. + +The principal fingerprint is derived, in order, from: an operator-supplied +`principal_resolver:`, then the agent's `session_token` (hashed), then +`acl_user`, then `acl_role`. With none of these the agent falls back to a shared +`"mk"` (master-key) principal: + +- **A master-key-everywhere factory makes owner-binding a no-op.** If every + request builds a bare master-key agent (no `session_token:` / `acl_user:` / + `acl_role:`), all agents share the `"mk"` fingerprint and are + indistinguishable, so the `403` never fires among them. Deployments that + authenticate users upstream and run master-key agents should supply a + `principal_resolver:` to restore a real per-user identity: + + ```ruby + app = Parse::Agent::MCPRackApp.new( + streaming: true, + notifications: true, # or resource_subscriptions: true + principal_resolver: ->(agent, env) { + # Return a stable per-user id (String). nil/empty falls through to the + # agent's own scope, then to the shared "mk" principal. + env["myapp.authenticated_user_id"] + }, + agent_factory: ->(env) { ... }, + ) + ``` + + The resolver must respond to `#call`; an invalid one raises `ArgumentError` at + construction. Per-user impersonation (binding a real `session_token` per + request) achieves the same effect without a resolver. + +**Limits (same scope as the cancellation registry):** the owner registry is +per-`MCPRackApp` instance and **single-process** — it does not span Puma workers +or survive a restart. In a clustered deployment the `initialize` POST and the +`GET` stream may land on different workers, so the initialize-binding degrades +to TOFU there. The registry is LRU-bounded (default 10,000 sessions) so a stream +of `initialize`-without-`DELETE` sessions cannot grow it without limit; evicting +an active owner just downgrades that id to TOFU on its next attach. Blank +session ids or blank fingerprints fail closed. + --- ## Approval Workflows (MCP elicitation) diff --git a/lib/parse/model/core/actions.rb b/lib/parse/model/core/actions.rb index f063d1a..062c429 100644 --- a/lib/parse/model/core/actions.rb +++ b/lib/parse/model/core/actions.rb @@ -1196,16 +1196,14 @@ def destroy(session: nil) success end - # Runs all the registered `before_save` related callbacks. + # Back-compat alias for {Parse::Object#run_before_save_callbacks}. The + # canonical name spells out exactly what runs (the before_save callbacks, + # before phase only) and is symmetric with run_after_save_callbacks / + # run_before_create_callbacks / run_after_create_callbacks. Retained so + # existing callers of `prepare_save!` keep working. + # @return [Boolean] false if a before_save callback halted the chain, else true. def prepare_save! - # With terminator configured, run_callbacks will return false if any callback returns false - # We track if the block executes to know if callbacks were halted - callback_success = false - run_callbacks(:save) do - callback_success = true - true - end - callback_success + run_before_save_callbacks end # @return [Hash] a hash of the list of changes made to this instance. diff --git a/lib/parse/model/object.rb b/lib/parse/model/object.rb index 861881a..0fc23e3 100644 --- a/lib/parse/model/object.rb +++ b/lib/parse/model/object.rb @@ -1645,6 +1645,25 @@ def run_after_delete_callbacks run_callbacks_from_list(self.class._destroy_callbacks, :after) end + # Run before_save callbacks for this object (BEFORE phase only). Used by the + # beforeSave webhook. Honors :if/:unless conditions and the callback + # terminator: returns false if a callback halts the chain. The after_* + # callbacks are NOT run here -- they belong to the afterSave webhook. + # @return [Boolean] false if a before_save callback halted the chain, else true. + def run_before_save_callbacks + run_before_phase_callbacks(:save) + end + + # Run before_create callbacks for this object (BEFORE phase only). Parse + # Server exposes no separate beforeCreate trigger, so the beforeSave webhook + # runs these for new objects right after before_save -- matching ActiveModel + # order, where before_save wraps before_create. Honors :if/:unless and the + # terminator. + # @return [Boolean] false if a before_create callback halted the chain, else true. + def run_before_create_callbacks + run_before_phase_callbacks(:create) + end + # Returns a hash of all the changes that have been made to the object. By default # changes to the Parse::Properties::BASE_KEYS are ignored unless you pass true as # an argument. @@ -2054,6 +2073,28 @@ def run_callbacks_from_list(callbacks, kind) end true end + + # Runs ONLY the before-phase callbacks of an ActiveModel callback chain + # (`:save` or `:create`), fully honoring `:if`/`:unless` conditions and the + # model's terminator, WITHOUT running the after-phase callbacks. ActiveModel + # `run_callbacks(kind) { block }` runs before -> block -> after; we throw out + # of the block so the after callbacks never run, while the before callbacks + # get complete condition/terminator handling. If a before callback halts the + # chain (returns false), the block never executes, so `completed` stays false + # and we report the halt. Used by the webhook before-phase so it does not + # double-fire after_* (those belong to the afterSave webhook). + # @return [Boolean] false if the chain was halted by a before callback. + def run_before_phase_callbacks(kind) + tag = :"__parse_before_phase_#{kind}" + completed = false + catch(tag) do + run_callbacks(kind) do + completed = true + throw tag + end + end + completed + end end end diff --git a/lib/parse/stack/version.rb b/lib/parse/stack/version.rb index 820e123..df33858 100644 --- a/lib/parse/stack/version.rb +++ b/lib/parse/stack/version.rb @@ -6,6 +6,6 @@ module Parse # The Parse Server SDK for Ruby module Stack # The current version. - VERSION = "5.2.0" + VERSION = "5.2.1" end end diff --git a/lib/parse/webhooks.rb b/lib/parse/webhooks.rb index 0081bf1..0284450 100644 --- a/lib/parse/webhooks.rb +++ b/lib/parse/webhooks.rb @@ -233,11 +233,23 @@ def call_route(type, className, payload = nil) # ran ActiveModel before_save callbacks locally. A client-spoofed # `_RB_` without master falls through and runs them here. unless trusted_ruby_initiated - prepare_result = result.prepare_save! - # If prepare_save! returns false (callback chain was halted), throw an error - if prepare_result == false + before_save_result = result.run_before_save_callbacks + # If a before_save callback halted the chain (returned false), reject the save. + if before_save_result == false raise Parse::Webhooks::ResponseError, "Save halted by before_save callback" end + # Parse Server exposes no separate beforeCreate trigger, so the + # beforeSave hook is the single point at which before_create must + # run for a client-initiated create. Run it AFTER before_save, for + # new objects only -- matching ActiveModel order (before_save wraps + # before_create) and mirroring the afterSave hook, which runs + # after_create then after_save. `original.nil?` marks a create. + if payload && payload.original.nil? + create_result = result.run_before_create_callbacks + if create_result == false + raise Parse::Webhooks::ResponseError, "Save halted by before_create callback" + end + end end # For before_save, return the changes payload (what Parse Server expects) result = result.changes_payload diff --git a/lib/parse/webhooks/payload.rb b/lib/parse/webhooks/payload.rb index e3c8b0f..4cdbfe9 100644 --- a/lib/parse/webhooks/payload.rb +++ b/lib/parse/webhooks/payload.rb @@ -78,24 +78,35 @@ def initialize(hash = {}) hash = Hash[hash.map { |k, v| [k.to_s.underscore.to_sym, v] }] @raw = hash @master = hash[:master] - # Strip protected mass-assignment keys (sessionToken, _rperm, _wperm, - # _hashed_password, authData, roles, etc.) BEFORE constructing the - # user object. Without this, an attacker reaching the webhook - # endpoint with a valid key (or with the optional unauthenticated - # mode enabled) can forge any of these fields on +payload.user+ - # via the +objectId+-present hydration branch that bypasses the - # +Parse::Object#apply_attributes!+ protected-key filter. + # Webhook trigger payloads (beforeSave/afterSave/etc.) are delivered by + # Parse Server and, when a webhook key is configured (the default; see + # Parse::Webhooks.allow_unauthenticated for the opt-out used in tests / + # local dev), authenticated by it -- so they are treated as trusted, + # server-authoritative state. A handler is meant to receive the full + # object -- createdAt/updatedAt, ACL, internal fields and all. The only + # thing stripped here is genuine credential material a handler never + # legitimately needs to read (live session tokens, offline-crackable + # password hashes); see WEBHOOK_TRIGGER_CREDENTIAL_KEYS. Protection + # against *persisting* forged privileged fields lives on the write path + # (changes_payload emits only declared, dirty-tracked properties), not on + # this read path. if hash[:user].present? - @user = Parse::User.new(self.class.scrub_protected_keys(hash[:user])) + # Trusted hydration via .build (not .new) so server-sent timestamps and + # data fields remain readable; credentials are removed first. Note + # Parse::User applies its own protections, so `payload.user.auth_data` + # is not exposed here. 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 @installation_id = hash[:installation_id] @params = hash[:params] @params = @params.with_indifferent_access if @params.is_a?(Hash) @function_name = hash[:function_name] - @object = self.class.scrub_protected_keys(hash[:object]) + @object = self.class.scrub_credentials(hash[:object]) @trigger_name = hash[:trigger_name] - @original = self.class.scrub_protected_keys(hash[:original]) - @update = self.class.scrub_protected_keys(hash[:update]) || {} + @original = self.class.scrub_credentials(hash[:original]) + @update = self.class.scrub_credentials(hash[:update]) || {} # Added for beforeFind and afterFind triggers @query = hash[:query] @objects = hash[:objects] || [] @@ -103,25 +114,32 @@ def initialize(hash = {}) end # @!visibility private - # Routing metadata that must be preserved on payload hashes even - # though the general mass-assignment denylist forbids it. Stripping - # +className+ here breaks +parse_class+/+parse_object+ resolution and - # silently disables +payload_class_mismatch?+. The denylist still - # protects +Parse::Object#apply_attributes!+ at hydration time. - PAYLOAD_PRESERVED_KEYS = %w[className __type].freeze + # Genuine credential material that is stripped from every webhook trigger + # payload before a handler can see it, even though the rest of the + # (trusted, server-authoritative) payload passes through untouched. A + # session token is a live bearer credential; a password hash is + # offline-crackable. A handler has no legitimate reason to read either, + # and removing them keeps them out of logs and out of any object a handler + # might persist. Everything else Parse Server sends -- createdAt/updatedAt, + # ACL, authData, roles, _rperm/_wperm, internal fields -- is preserved so + # the handler observes the full object. Write-side protection + # (changes_payload emits only declared, dirty-tracked properties) is what + # prevents persisting forged privileged fields. + WEBHOOK_TRIGGER_CREDENTIAL_KEYS = %w[ + sessionToken session_token + _hashed_password _password_history + ].freeze # @!visibility private - # Returns a copy of +obj+ with the +PROTECTED_MASS_ASSIGNMENT_KEYS+ - # removed, except for routing metadata in +PAYLOAD_PRESERVED_KEYS+. - # Operates on string and symbol keys (Parse Server uses camelCase + # Returns a copy of +obj+ with only +WEBHOOK_TRIGGER_CREDENTIAL_KEYS+ + # removed. Operates on string and symbol keys (Parse Server uses camelCase # strings on the wire; downstream code may have already symbolized). # Pass-through for non-Hash input. - def self.scrub_protected_keys(obj) + def self.scrub_credentials(obj) return obj unless obj.is_a?(Hash) - denied = Parse::Properties::PROTECTED_MASS_ASSIGNMENT_KEYS + denied = WEBHOOK_TRIGGER_CREDENTIAL_KEYS obj.reject do |k, _| name = k.to_s - next false if PAYLOAD_PRESERVED_KEYS.include?(name) denied.include?(name) || denied.include?(name.underscore) end end @@ -278,24 +296,34 @@ def build_parse_object if @original.present? && @original.is_a?(Hash) o = Parse::Object.build @original, parse_class o.apply_attributes! @object, dirty_track: true - - if o.is_a?(Parse::User) && @update.present? && @update["authData"].present? - o.auth_data = @update["authData"] - end return o else #else the object must be new klass = Parse::Object.find_class parse_class # if we have a class, return that with updated changes, otherwise # default to regular object - if klass.present? - o = klass.new(@object || {}) - if o.is_a?(Parse::User) && @update.present? && @update["authData"].present? - o.auth_data = @update["authData"] - end - return o - end # if klass.present? + return klass.new(@object || {}) if klass.present? end # if we have original end # if before_trigger? + + # 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 + end + + # afterSave on a CREATE (and every other trigger): the full object as the + # server sent it. createdAt/updatedAt survive (only credentials are + # scrubbed), so `new?` / `existed?` read correctly. Parse::Object.build(@object, parse_class) end diff --git a/test/lib/parse/agent_integration_test.rb b/test/lib/parse/agent_integration_test.rb index 6a9320d..40afb64 100644 --- a/test/lib/parse/agent_integration_test.rb +++ b/test/lib/parse/agent_integration_test.rb @@ -126,7 +126,7 @@ def test_get_schema_for_nonexistent_class result = agent.execute(:get_schema, class_name: "NonExistentClass") refute result[:success], "Should fail for nonexistent class" - assert_match(/failed/i, result[:error]) + assert_match(/could not fetch schema/i, result[:error]) end end end diff --git a/test/lib/parse/security_hardening_test.rb b/test/lib/parse/security_hardening_test.rb index eca4bcb..e626c6c 100644 --- a/test/lib/parse/security_hardening_test.rb +++ b/test/lib/parse/security_hardening_test.rb @@ -831,7 +831,13 @@ def test_pipeline_allows_safe_expr Parse::PipelineSecurity.validate_filter!(pipeline) end - # ─── Webhook payload PROTECTED_MASS_ASSIGNMENT_KEYS scrub ────────────── + # ─── Webhook payload credential scrub (trusted callbacks get full object) ── + # + # Webhook trigger payloads are server-authoritative and authenticated by the + # webhook key, so a handler receives the FULL object. Only genuine credential + # material (live session tokens, password hashes) is stripped. The + # forged-privileged-field defense moved to the WRITE path: the + # *_do_not_persist_* test below proves a save never transmits them. def test_webhook_payload_strips_session_token_from_user payload = Parse::Webhooks::Payload.new( @@ -845,40 +851,45 @@ def test_webhook_payload_strips_session_token_from_user refute_equal "r:forged", payload.user.session_token end - def test_webhook_payload_strips_authData_from_user - payload = Parse::Webhooks::Payload.new( - "user" => { - "objectId" => "userAttacker", - "authData" => { "facebook" => { "id" => "victim_fb", "access_token" => "x" } } - }, - "triggerName" => "beforeSave" - ) - # Confirm the scrubbed hash that fed the constructor does not contain - # authData. Inspecting payload.user.auth_data would trigger autofetch - # on the unsaved record; the scrub happens at construction time so we - # verify the raw hash post-scrub directly. - scrubbed = Parse::Webhooks::Payload.scrub_protected_keys( - "objectId" => "userAttacker", - "authData" => { "facebook" => { "id" => "victim_fb" } } + def test_webhook_credentials_scrub_preserves_authdata_strips_credentials + # authData / timestamps / ACL are server-authoritative and must reach the + # handler unchanged; only live credentials are removed. + scrubbed = Parse::Webhooks::Payload.scrub_credentials( + "objectId" => "u1", + "authData" => { "facebook" => { "id" => "fb1" } }, + "createdAt" => "2026-06-04T12:00:00.000Z", + "ACL" => { "u1" => { "read" => true, "write" => true } }, + "sessionToken" => "r:live", + "_hashed_password" => "$2b$x" ) - refute scrubbed.key?("authData") - refute scrubbed.key?("auth_data") + assert scrubbed.key?("authData"), "authData is preserved for trusted callbacks" + assert scrubbed.key?("createdAt"), "server timestamps are preserved" + assert scrubbed.key?("ACL"), "ACL is preserved" + refute scrubbed.key?("sessionToken"), "live session token is stripped" + refute scrubbed.key?("_hashed_password"), "password hash is stripped" end - def test_webhook_payload_strips_acl_perms_from_object + def test_webhook_object_preserves_full_server_object_for_trusted_callback payload = Parse::Webhooks::Payload.new( "object" => { "className" => "Account", "objectId" => "abc", "_rperm" => ["*"], "_wperm" => ["userAttacker"], + "createdAt" => "2026-06-04T12:00:00.000Z", + "updatedAt" => "2026-06-04T12:00:00.000Z", "balance" => 100, }, - "triggerName" => "beforeSave" + "triggerName" => "afterSave" ) obj_hash = payload.instance_variable_get(:@object) - refute obj_hash.key?("_rperm") - refute obj_hash.key?("_wperm") + # Trusted, server-authoritative payload: the full object survives so the + # handler can read it (createdAt/updatedAt protection is write-side, not + # read-side). _rperm/_wperm are Mongo-internal with no property accessor and + # cannot be persisted via a handler save (see the write-side test below). + assert obj_hash.key?("_rperm"), "server _rperm survives for the handler to read" + assert obj_hash.key?("_wperm"), "server _wperm survives for the handler to read" + assert obj_hash.key?("createdAt"), "createdAt survives (read protection only)" assert obj_hash.key?("balance") end @@ -897,6 +908,29 @@ def test_webhook_payload_strips_hashed_password_from_user refute attrs.key?("_hashed_password") end + def test_webhook_user_forged_privileged_fields_never_reach_save_body + # Write-side guarantee for the trigger _User object: even though the inbound + # payload is no longer broadly scrubbed, a beforeSave handler that returns + # the built user must never transmit forged authData / roles / row-perms. + payload = Parse::Webhooks::Payload.new( + "triggerName" => "beforeSave", + "object" => { + "className" => "_User", + "objectId" => "userAttacker", + "username" => "x", + "authData" => { "facebook" => { "id" => "victim", "access_token" => "t" } }, + "roles" => ["Admin"], + "_rperm" => ["*"], + "_wperm" => ["userAttacker"], + } + ) + body = payload.parse_object.changes_payload + %w[authData auth_data roles _rperm _wperm].each do |forbidden| + refute body.key?(forbidden), + "forged #{forbidden} on a trigger _User must never reach the save body" + end + end + # ─── Role hierarchy direction documentation ──────────────────────────── def test_role_grant_capabilities_to_methods_exist diff --git a/test/lib/parse/webhook_aftersave_payload_fidelity_test.rb b/test/lib/parse/webhook_aftersave_payload_fidelity_test.rb new file mode 100644 index 0000000..33149ca --- /dev/null +++ b/test/lib/parse/webhook_aftersave_payload_fidelity_test.rb @@ -0,0 +1,550 @@ +# encoding: UTF-8 +# frozen_string_literal: true + +require_relative "../../test_helper" +require "minitest/autorun" + +# Unit-level regression suite that LOCKS IN the webhook full-object contract: +# +# "An afterSave / beforeSave webhook handler receives the FULL Parse object +# exactly as Parse Server sent it (timestamps, ACL, internal fields) and can +# correctly distinguish NEW vs CHANGED objects and detect field changes." +# +# These tests construct Parse::Webhooks::Payload.new(hash) directly (the same +# pattern as test/lib/parse/webhook_callbacks_test.rb) with FULL afterSave +# payload fixtures that mirror what Parse Server 8.4.0 actually sends. The +# fixture shapes were captured from a live container via the diagnostic in +# test/lib/parse/webhook_aftersave_state_integration_test.rb and a one-off +# raw-payload probe: +# +# NEW create raw object keys: +# ["className","createdAt","objectId","status","title","updatedAt"] +# (createdAt == updatedAt; ACL only present when the record has one) +# UPDATE raw object & original keys (record HAS an ACL): +# ["ACL","className","createdAt","objectId","status","title","updatedAt"] +# (createdAt < updatedAt; "original" present) +# +# Parse Server 8.4.0 does NOT send the Mongo-internal _rperm / _wperm in the +# REST webhook payload, so we do not assert on them here (there is also no +# Parse::Object property accessor for them). ACL DOES come through and survives, +# so it is asserted. +# +# ============================ WHAT THIS LOCKS IN ============================ +# Parse::Webhooks::Payload#initialize scrubs trigger payloads with +# `scrub_credentials` (lib/parse/webhooks/payload.rb), which removes ONLY +# WEBHOOK_TRIGGER_CREDENTIAL_KEYS (session tokens, password hashes). Every +# other server-authoritative field -- createdAt/updatedAt, ACL, authData, +# roles, internal fields -- passes through to the handler, so `existed?` / +# `new?` read correctly and the full-object-fidelity contract holds. +# +# These tests are the guard that catches anyone re-introducing over-broad +# scrubbing (e.g. switching back to PROTECTED_MASS_ASSIGNMENT_KEYS, which +# includes createdAt/updatedAt): a dropped server field makes the fidelity +# assertions fail. +# ============================================================================ + +# Neutral domain class per CLAUDE.md domain-term hygiene (Post, not Capture). +class FidelityPost < Parse::Object + parse_class "FidelityPost" + property :title, :string + property :status, :string + property :body, :string +end + +# Records `self` inside each model lifecycle callback so the tests can assert +# that a webhook-built object handed to before_save / before_create / +# after_create / after_save is the full object the server sent. +class LifecyclePost < Parse::Object + parse_class "LifecyclePost" + property :title, :string + property :status, :string + property :body, :string + + class << self + attr_accessor :seen + end + self.seen = [] + + before_save { LifecyclePost.snapshot(self, :before_save) } + before_create { LifecyclePost.snapshot(self, :before_create) } + after_create { LifecyclePost.snapshot(self, :after_create) } + after_save { LifecyclePost.snapshot(self, :after_save) } + + def self.snapshot(obj, hook) + seen << { + hook: hook, + id: obj.id, + title: obj.title, + status: obj.status, + has_created_at: !obj.created_at.nil?, + has_updated_at: !obj.updated_at.nil?, + new?: obj.new?, + existed?: obj.existed?, + has_acl: !obj.acl.nil?, + title_changed?: obj.title_changed?, + } + true + end +end + +# A model whose before_create halts the save (returns false). Used to verify +# the webhook before-phase propagates a before_create halt as a rejection. +class HaltCreatePost < Parse::Object + parse_class "HaltCreatePost" + property :title, :string + before_create { false } +end + +# A model with conditional (`:if`) before callbacks. Used to verify the +# before-phase runner honors `:if`/`:unless` (the conditional callbacks must +# be skipped, not run unconditionally). +class CondCbPost < Parse::Object + parse_class "CondCbPost" + property :title, :string + class << self; attr_accessor :ran; end + self.ran = [] + before_save :always_bs + before_save :skip_bs, if: -> { false } + before_create :always_bc + before_create :skip_bc, if: -> { false } + def always_bs; CondCbPost.ran << :always_bs; end + def skip_bs; CondCbPost.ran << :skip_bs; end + def always_bc; CondCbPost.ran << :always_bc; end + def skip_bc; CondCbPost.ran << :skip_bc; end +end + +class WebhookAfterSavePayloadFidelityTest < Minitest::Test + def setup + Parse::Webhooks.instance_variable_set(:@routes, nil) + end + + def teardown + Parse::Webhooks.instance_variable_set(:@routes, nil) + end + + # ------------------------------------------------------------------------ + # Fixtures: realistic Parse Server 8.4.0 afterSave payloads. + # ------------------------------------------------------------------------ + + CREATED_AT = "2026-06-04T12:00:00.000Z" + UPDATED_AT = "2026-06-04T12:05:30.250Z" + + # A brand-new object: createdAt == updatedAt, NO "original". + # ACL included to mirror a record that carries one (owner-only). + def new_aftersave_payload + { + "triggerName" => "afterSave", + "object" => { + "className" => "FidelityPost", + "objectId" => "NEWobj0001", + "title" => "hello world", + "status" => "draft", + "createdAt" => CREATED_AT, + "updatedAt" => CREATED_AT, # equal => freshly created + "ACL" => { "u_owner" => { "read" => true, "write" => true } }, + }, + "headers" => { "x-parse-request-id" => "client_create_01" }, + } + end + + # A changed object: createdAt < updatedAt, WITH "original". + # Title changed hello->goodbye; status unchanged. Both object and original + # carry createdAt/updatedAt/ACL exactly as Parse Server sends them. + def changed_aftersave_payload + { + "triggerName" => "afterSave", + "object" => { + "className" => "FidelityPost", + "objectId" => "UPDobj0001", + "title" => "goodbye world", + "status" => "draft", + "body" => "v2", + "createdAt" => CREATED_AT, + "updatedAt" => UPDATED_AT, # differs => this is an update + "ACL" => { "u_owner" => { "read" => true, "write" => true } }, + }, + "original" => { + "className" => "FidelityPost", + "objectId" => "UPDobj0001", + "title" => "hello world", + "status" => "draft", + "body" => "v1", + "createdAt" => CREATED_AT, + "updatedAt" => CREATED_AT, + "ACL" => { "u_owner" => { "read" => true, "write" => true } }, + }, + "headers" => { "x-parse-request-id" => "client_update_01" }, + } + end + + # ======================================================================== + # A1. NEW object semantics + # ======================================================================== + + def test_new_payload_object_identity_and_acl + obj = Parse::Webhooks::Payload.new(new_aftersave_payload).parse_object + refute_nil obj, "parse_object must build a typed object for an afterSave create" + assert_instance_of FidelityPost, obj + assert_equal "NEWobj0001", obj.id, "objectId must be present on the built object" + assert_equal "hello world", obj.title + assert_equal "draft", obj.status + + # ACL is NOT in the scrub denylist, so it survives today and must keep + # surviving. This guards against ACL being added to the scrub set. + refute_nil obj.acl, "ACL must survive into the built afterSave object" + assert obj.acl.present?, "ACL must be populated" + assert_includes obj.acl.permissions.keys, "u_owner", + "ACL row-permission for the owner must be readable in the handler" + end + + # A freshly-created object must expose the server-issued createdAt/updatedAt + # (equal), which `existed?`/`new?` depend on. Regression guard against + # re-introducing timestamp scrubbing on the webhook path. + def test_new_payload_timestamps_populated + obj = Parse::Webhooks::Payload.new(new_aftersave_payload).parse_object + refute_nil obj.created_at, + "created_at must survive into the afterSave object (server-authoritative)" + refute_nil obj.updated_at, + "updated_at must survive into the afterSave object (server-authoritative)" + assert_equal obj.created_at, obj.updated_at, + "a freshly-created object has createdAt == updatedAt" + end + + # existed? must be false for a fresh create. This passes on HEAD ONLY by + # accident (timestamps are nil -> existed? short-circuits to false). It is + # kept so that AFTER the fix it still holds via the real createdAt==updatedAt + # path. It does NOT by itself discriminate fixed-vs-broken; the timestamp + # test above does. + def test_new_payload_existed_is_false + obj = Parse::Webhooks::Payload.new(new_aftersave_payload).parse_object + # createdAt == updatedAt on a fresh create => existed? false (via the real + # timestamp path now, not the nil short-circuit). + refute obj.existed?, "a brand-new object must report existed? == false in afterSave" + refute obj.new?, "a persisted object (createdAt present) reports new? == false in afterSave" + assert_nil Parse::Webhooks::Payload.new(new_aftersave_payload).original, + "a create payload has no original" + end + + # ======================================================================== + # A2. CHANGED object semantics + # ======================================================================== + + def test_changed_payload_object_identity_and_acl + payload = Parse::Webhooks::Payload.new(changed_aftersave_payload) + obj = payload.parse_object + refute_nil obj + assert_equal "UPDobj0001", obj.id + assert_equal "goodbye world", obj.title, "final object reflects the new value" + refute_nil obj.acl, "ACL must survive into the built afterSave update object" + refute_nil payload.original, "an update payload carries original" + end + + # An updated object must expose createdAt < updatedAt so `existed?` returns + # true. This is the assertion that most clearly discriminates the fix from + # the old timestamp-stripping behavior. + def test_changed_payload_existed_true_and_timestamps_differ + obj = Parse::Webhooks::Payload.new(changed_aftersave_payload).parse_object + refute_nil obj.created_at, "created_at must populate on an update object" + refute_nil obj.updated_at, "updated_at must populate on an update object" + refute_equal obj.created_at, obj.updated_at, + "an updated object has createdAt < updatedAt" + assert obj.existed?, + "existed? must be true for an object that was updated (not first save)" + refute obj.new?, "new? must be false for an already-persisted, updated object" + end + + # ----- Change detection: TODAY variant (holds on HEAD) ------------------- + # afterSave does NOT dirty-track (build_parse_object only merges original on + # before_trigger?), so `*_changed?` is false on HEAD. The supported way to + # diff today is to compare original_parse_object vs parse_object explicitly. + def test_changed_payload_diff_via_original_vs_object + payload = Parse::Webhooks::Payload.new(changed_aftersave_payload) + obj = payload.parse_object + orig = payload.original_parse_object + + refute_nil orig, "original_parse_object must build from the original hash" + assert_equal "hello world", orig.title, "original retains previous value" + assert_equal "goodbye world", obj.title, "object holds new value" + refute_equal orig.title, obj.title, "a field-level change is detectable by comparison" + assert_equal orig.status, obj.status, "unchanged field is equal across original/object" + end + + # ----- Change detection: dirty tracking on afterSave ---------------------- + # parse_object on an afterSave update is dirty-tracked relative to original, + # so a webhook author can gate a side effect on `title_changed?` / inspect + # `changes` -- symmetric with the beforeSave path. + def test_changed_payload_dirty_tracking + obj = Parse::Webhooks::Payload.new(changed_aftersave_payload).parse_object + assert obj.title_changed?, + "title_changed? must be true on an afterSave update where title changed" + refute obj.status_changed?, + "status_changed? must be false where status did not change" + assert_includes obj.changed, "title", + "changed must include the mutated attribute" + # changes maps attr => [old, new] + assert_equal ["hello world", "goodbye world"], obj.changes["title"], + "changes must reflect original -> object for the mutated field" + end + + # ======================================================================== + # A3. FULL-OBJECT FIDELITY GUARD + # ------------------------------------------------------------------------ + # The core regression guard. It compares the set of fields Parse Server + # actually SENT (read from the UNSCRUBBED raw payload, NOT payload.object + # which is already scrubbed) against the fields the built object exposes, + # after mapping remote<->local aliases (objectId<->id, createdAt<->created_at, + # updatedAt<->updated_at, ACL<->acl). If anyone re-adds timestamp/ACL/ internal + # scrubbing to the webhook path, a server field silently goes missing and + # this FAILS. + # + # ======================================================================== + + # Remote (wire) key -> local accessor used to read it back off the object. + REMOTE_TO_LOCAL = { + "objectId" => :id, + "createdAt" => :created_at, + "updatedAt" => :updated_at, + "ACL" => :acl, + }.freeze + + # Wire keys that are routing metadata, not data fields, and are not expected + # to be readable as object attributes. + ROUTING_ONLY = %w[className __type].freeze + + def assert_full_object_fidelity(raw_object_hash, built) + expected_fields = raw_object_hash.keys - ROUTING_ONLY + missing = expected_fields.reject do |wire_key| + reader = REMOTE_TO_LOCAL[wire_key] || wire_key.to_sym + built.respond_to?(reader) && !built.public_send(reader).nil? + end + 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_credentials)." + end + + def test_full_object_fidelity_new_create + payload = Parse::Webhooks::Payload.new(new_aftersave_payload) + # Reference set = what the SERVER sent = the UNSCRUBBED raw payload. + raw_object = payload.raw[:object] + assert raw_object.is_a?(Hash), "raw payload object must be retained for the guard" + assert_full_object_fidelity(raw_object, payload.parse_object) + end + + def test_full_object_fidelity_changed_update + payload = Parse::Webhooks::Payload.new(changed_aftersave_payload) + raw_object = payload.raw[:object] + assert_full_object_fidelity(raw_object, payload.parse_object) + + # And the original side must be equally faithful. + raw_original = payload.raw[:original] + assert raw_original.is_a?(Hash), "raw original must be retained for the guard" + assert_full_object_fidelity(raw_original, payload.original_parse_object) + end + + # Belt-and-suspenders: prove the guard's reference set is the UNSCRUBBED raw, + # not the already-scrubbed payload.object. Use a payload that DOES carry a + # credential so the two key sets genuinely differ (server-authoritative + # fields like createdAt are kept by both; only the credential is dropped). + def test_guard_reference_set_uses_unscrubbed_raw + p = new_aftersave_payload + p["object"] = p["object"].merge("sessionToken" => "r:should-be-scrubbed") + payload = Parse::Webhooks::Payload.new(p) + + raw_keys = payload.raw[:object].keys.sort + scrubbed_keys = payload.object.keys.sort + + assert_includes raw_keys, "createdAt", + "raw payload must retain createdAt for the fidelity reference set" + assert_includes raw_keys, "sessionToken", + "raw payload retains the credential (reference set is pre-scrub)" + refute_includes scrubbed_keys, "sessionToken", + "the scrubbed object must drop the credential" + assert_includes scrubbed_keys, "createdAt", + "the scrubbed object must KEEP server-authoritative createdAt" + refute_equal raw_keys, scrubbed_keys, + "raw and scrubbed key sets differ only by the credential" + end + + # ======================================================================== + # A4. WRITE-SIDE GUARANTEE (the defense that replaces read-side scrubbing) + # ------------------------------------------------------------------------ + # The handler can READ the full object, but a save of a webhook-built object + # must never transmit forged privileged fields. _rperm/_wperm/_hashed_password + # are not declared properties, so changes_payload/attribute_updates exclude + # them; createdAt/updatedAt are BASE_KEYS and are likewise excluded from a + # save body. + # ======================================================================== + def test_forged_privileged_fields_never_reach_save_body + payload = Parse::Webhooks::Payload.new( + "triggerName" => "beforeSave", + "object" => { + "className" => "FidelityPost", + "objectId" => "UPDobj0001", + "title" => "goodbye world", + "_rperm" => ["*"], + "_wperm" => ["userAttacker"], + "_hashed_password" => "$2b$forged", + "createdAt" => CREATED_AT, + "updatedAt" => UPDATED_AT, + }, + "original" => { + "className" => "FidelityPost", + "objectId" => "UPDobj0001", + "title" => "hello world", + "createdAt" => CREATED_AT, + "updatedAt" => CREATED_AT, + }, + ) + body = payload.parse_object.changes_payload + %w[_rperm _wperm _hashed_password createdAt updatedAt].each do |forbidden| + refute body.key?(forbidden), + "#{forbidden} must never reach the save body returned to Parse Server" + end + assert_equal "goodbye world", body["title"], + "a legitimate field change is still transmitted" + end + + # ======================================================================== + # A5. LIFECYCLE-CALLBACK FIDELITY + ORDER (before_save / before_create / + # after_create / after_save) + # ------------------------------------------------------------------------ + # Driven through the REAL webhook dispatcher (call_route), since Parse Server + # exposes no separate beforeCreate/afterCreate triggers -- the beforeSave hook + # runs before_save then before_create (for new objects), and the afterSave + # hook runs after_create then after_save. This is ActiveModel order + # (before_save wraps before_create; after_create precedes after_save). Each + # callback must also see the FULL object the server sent. + # ======================================================================== + + # beforeSave on a CREATE: Parse Server sends the submitted object (no server + # timestamps yet, since it is not persisted), no "original". + def beforesave_create_payload + { + "triggerName" => "beforeSave", + "object" => { + "className" => "LifecyclePost", + "objectId" => "BScreate01", + "title" => "new title", + "status" => "draft", + "ACL" => { "u_owner" => { "read" => true, "write" => true } }, + }, + "headers" => { "x-parse-request-id" => "client_bs_create" }, + } + end + + # beforeSave on an UPDATE: object carries the new values + server timestamps; + # original carries the prior state. + def beforesave_update_payload + { + "triggerName" => "beforeSave", + "object" => { + "className" => "LifecyclePost", "objectId" => "BSupd01", + "title" => "new title", "status" => "draft", + "createdAt" => CREATED_AT, "updatedAt" => CREATED_AT, + }, + "original" => { + "className" => "LifecyclePost", "objectId" => "BSupd01", + "title" => "old title", "status" => "draft", + "createdAt" => CREATED_AT, "updatedAt" => CREATED_AT, + }, + "headers" => { "x-parse-request-id" => "client_bs_update" }, + } + end + + def lifecycle_snapshot(hook) + LifecyclePost.seen.find { |s| s[:hook] == hook } + end + + # Dispatch a payload through the real webhook router with a route block. + def dispatch_lifecycle(trigger, payload_hash, &route) + LifecyclePost.seen = [] + Parse::Webhooks.instance_variable_set(:@routes, nil) + Parse::Webhooks.route(trigger, "LifecyclePost", &route) + Parse::Webhooks.call_route(trigger, "LifecyclePost", + Parse::Webhooks::Payload.new(payload_hash)) + LifecyclePost.seen.map { |s| s[:hook] } + end + + def test_before_save_then_before_create_fire_in_order_on_create + # Parse Server has no beforeCreate webhook; the beforeSave hook runs + # before_save THEN before_create for a new object (ActiveModel order). + order = dispatch_lifecycle(:before_save, beforesave_create_payload) { parse_object } + assert_equal [:before_save, :before_create], order, + "beforeSave webhook runs before_save then before_create on a create" + LifecyclePost.seen.each do |s| + assert_equal "BScreate01", s[:id], "#{s[:hook]} sees the objectId" + assert_equal "new title", s[:title], "#{s[:hook]} sees the submitted title" + assert_equal "draft", s[:status], "#{s[:hook]} sees the submitted status" + assert s[:has_acl], "#{s[:hook]} sees the submitted ACL" + assert s[:new?], "#{s[:hook]} on a create reports new? == true (not yet persisted)" + end + end + + def test_before_create_does_not_fire_on_update + order = dispatch_lifecycle(:before_save, beforesave_update_payload) { parse_object } + assert_includes order, :before_save, "before_save runs on an update" + refute_includes order, :before_create, "before_create must NOT run on an update" + + bs = lifecycle_snapshot(:before_save) + assert_equal "new title", bs[:title], "before_save sees the new value on update" + assert bs[:has_created_at], "before_save sees server createdAt on update" + assert bs[:title_changed?], "before_save sees dirty tracking (title changed) on update" + end + + def test_after_create_then_after_save_fire_in_order_on_create + raw = new_aftersave_payload + raw["object"] = raw["object"].merge("className" => "LifecyclePost") + order = dispatch_lifecycle(:after_save, raw) { true } + assert_equal [:after_create, :after_save], order, + "afterSave webhook runs after_create then after_save on a create" + LifecyclePost.seen.each do |s| + assert_equal "NEWobj0001", s[:id], "#{s[:hook]} sees the persisted objectId" + assert_equal "hello world", s[:title], "#{s[:hook]} sees the persisted title" + assert s[:has_created_at], "#{s[:hook]} sees server createdAt" + assert s[:has_updated_at], "#{s[:hook]} sees server updatedAt" + assert s[:has_acl], "#{s[:hook]} sees the ACL" + refute s[:existed?], "#{s[:hook]} on a fresh create reports existed? == false" + refute s[:new?], "#{s[:hook]} reports new? == false (object is persisted)" + end + end + + # A before_create returning false must halt the save the same way before_save + # does — the dispatcher raises a ResponseError that becomes a Parse Server + # rejection. + def test_before_create_halt_rejects_the_save + Parse::Webhooks.instance_variable_set(:@routes, nil) + Parse::Webhooks.route(:before_save, "HaltCreatePost") { parse_object } + payload = Parse::Webhooks::Payload.new( + "triggerName" => "beforeSave", + "object" => { "className" => "HaltCreatePost", "objectId" => "h1", "title" => "x" }, + "headers" => { "x-parse-request-id" => "client_halt" }, + ) + err = assert_raises(Parse::Webhooks::ResponseError) do + Parse::Webhooks.call_route(:before_save, "HaltCreatePost", payload) + end + assert_match(/before_create/, err.message, + "a before_create returning false halts the save with a before_create error") + end + + # The before-phase runner must honor `:if`/`:unless` (and the terminator), + # not run every callback unconditionally. + def test_conditional_before_callbacks_are_honored + CondCbPost.ran = [] + Parse::Webhooks.instance_variable_set(:@routes, nil) + Parse::Webhooks.route(:before_save, "CondCbPost") { parse_object } + payload = Parse::Webhooks::Payload.new( + "triggerName" => "beforeSave", + "object" => { "className" => "CondCbPost", "objectId" => "c1", "title" => "x" }, + "headers" => { "x-parse-request-id" => "client_cond" }, + ) + Parse::Webhooks.call_route(:before_save, "CondCbPost", payload) + + assert_includes CondCbPost.ran, :always_bs, "unconditional before_save runs" + assert_includes CondCbPost.ran, :always_bc, "unconditional before_create runs (create)" + refute_includes CondCbPost.ran, :skip_bs, "`if: false` before_save must be skipped" + refute_includes CondCbPost.ran, :skip_bc, "`if: false` before_create must be skipped" + end +end diff --git a/test/lib/parse/webhook_aftersave_state_integration_test.rb b/test/lib/parse/webhook_aftersave_state_integration_test.rb new file mode 100644 index 0000000..d28d0d0 --- /dev/null +++ b/test/lib/parse/webhook_aftersave_state_integration_test.rb @@ -0,0 +1,303 @@ +require_relative "../../test_helper_integration" +require_relative "../../support/webhook_test_server" + +# End-to-end integration test for the state of the Parse::Object handed to an +# afterSave webhook block, exercising the full pipeline: +# +# Parse Server (Docker) -> HTTP POST afterSave webhook -> in-process WEBrick -> +# Parse::Webhooks Rack app -> webhook block -> capture object state +# +# This focuses on the two questions webhook authors actually rely on inside an +# afterSave handler for objects created by a non-Ruby client (REST / JS cloud +# code / Auth0): +# +# 1. New-object detection -- "is this the first save of this object?" The +# idioms in the wild are `payload.original.nil?`, `new?`, and `existed?`. +# 2. Change detection -- "which fields changed on this update?" via +# dirty tracking (`changed`, `*_changed?`, `changes`). +# +# Requires Docker (PARSE_TEST_USE_DOCKER=true) and a Parse Server container +# whose `host.docker.internal` resolves back to the test host. Skipped +# automatically if the test server cannot be reached from the container. + +# Neutral domain class (see CLAUDE.md domain-term hygiene). acl_policy :public +# so a non-master client create/update is not blocked at the ACL gate before +# the webhook runs -- the non-master path is the whole point (mimics Auth0/JS). +class WebhookStatePost < Parse::Object + parse_class "WebhookStatePost" + acl_policy :public + property :title, :string + property :status, :string + property :body, :string + + # Records, per phase, whether the model-level after_* callbacks fired and + # what the dirty state looked like at callback time. Lets us prove point 2: + # a side effect gated on `*_changed?` silently no-ops on an afterSave object. + class << self + attr_accessor :model_callbacks + end + self.model_callbacks = [] + + before_save :__record_before_save + before_create :__record_before_create + after_create :__record_after_create + after_save :__record_after_save + + def __record(hook) + self.class.model_callbacks << { + hook: hook, + new?: new?, + existed?: existed?, + changed: changed.dup, + title_changed?: title_changed?, + } + true + end + + def __record_before_save = __record(:before_save) + def __record_before_create = __record(:before_create) + def __record_after_create = __record(:after_create) + def __record_after_save = __record(:after_save) +end + +module WebhookAfterSaveStateSetup + def setup + super + Parse::Webhooks.instance_variable_set(:@routes, nil) + Parse::Webhooks.allow_unauthenticated = true + @prior_allow_private_webhook_urls = Parse::Webhooks.instance_variable_get(:@allow_private_webhook_urls) + Parse::Webhooks.allow_private_webhook_urls = true + + WebhookStatePost.model_callbacks = [] + + @server = Parse::Test::WebhookTestServer.new.start! + + unless docker_can_reach_host? + @server.stop! + skip "Parse Server container cannot reach the test host at " \ + "#{@server.url}; ensure docker-compose has " \ + "extra_hosts: [\"host.docker.internal:host-gateway\"]." + end + end + + def teardown + begin + Parse::Webhooks.remove_all_triggers! if @server + rescue StandardError + # parent resets DB anyway + end + @server&.stop! + Parse::Webhooks.allow_unauthenticated = false + Parse::Webhooks.instance_variable_set(:@allow_private_webhook_urls, @prior_allow_private_webhook_urls) + super + end +end + +class WebhookAfterSaveStateIntegrationTest < Minitest::Test + include ParseStackIntegrationTest + prepend WebhookAfterSaveStateSetup + + # ==================================================================== + # HARD-ASSERTION regression tests (modeled on + # field_guards_end_to_end_integration_test.rb). These exercise the full + # Docker pipeline (real Parse Server -> HTTP webhook -> Rack app) and assert + # the full-object contract end to end: the webhook path preserves + # server-authoritative createdAt/updatedAt/ACL (only credentials are scrubbed + # via Parse::Webhooks::Payload#scrub_credentials), so existed?/new? are + # reliable and afterSave updates carry dirty tracking. The unit suite in + # webhook_aftersave_payload_fidelity_test.rb pins the same contract off Docker. + # ==================================================================== + + # Captures the built-object state seen inside the afterSave handler so the + # test body can assert on it after the webhook fires. + def capture_after_save(class_name) + captured = {} + Parse::Webhooks.route(:after_save, class_name) do + obj = parse_object + orig = original_parse_object + 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 + captured[:raw_updatedAt] = raw_obj.is_a?(Hash) ? raw_obj["updatedAt"] : nil + captured[:raw_has_acl] = raw_obj.is_a?(Hash) ? raw_obj.key?("ACL") : false + if obj + captured[:id] = obj.id + captured[:new?] = obj.new? + captured[:existed?] = obj.existed? + captured[:created_at] = obj.created_at + captured[:updated_at] = obj.updated_at + captured[:acl_present] = !obj.acl.nil? + captured[:title] = obj.title + captured[:title_changed?] = obj.title_changed? + captured[:changed] = obj.changed.dup + captured[:changes] = obj.changes + end + captured[:orig_title] = orig&.title + true + end + Parse::Webhooks.register_triggers!(@server.url) + captured + end + + def test_b_new_object_full_state_on_nonmaster_create + captured = capture_after_save("WebhookStatePost") + + id = non_master_create("WebhookStatePost", { "title" => "hello", "status" => "draft" }) + wait_until("afterSave fires for non-master create") { captured[:fired] } + + assert id, "create returned an id" + assert_equal id, captured[:id], "handler saw the persisted objectId" + assert captured[:payload_original_nil], "a create has no original" + + # Parse Server sends createdAt == updatedAt on a fresh create. + assert_equal captured[:raw_createdAt], captured[:raw_updatedAt], + "createdAt == updatedAt on a fresh create (raw payload)" + refute_nil captured[:raw_createdAt], "Parse Server sends createdAt for a create" + + # ---- SPEC (FAILS ON HEAD: scrub strips timestamps) ---- + refute_nil captured[:created_at], + "[SPEC] built object must expose server createdAt in afterSave" + refute_nil captured[:updated_at], + "[SPEC] built object must expose server updatedAt in afterSave" + refute captured[:existed?], + "a brand-new object must report existed? == false" + end + + def test_b_new_object_full_state_on_ruby_model_save + # Ruby-initiated create via the model API. The afterSave webhook still + # fires server-side; the handler must see the same full state. + captured = capture_after_save("WebhookStatePost") + + obj = WebhookStatePost.new(title: "ruby-create", status: "draft") + obj.save + wait_until("afterSave fires for ruby model save") { captured[:fired] } + + assert captured[:fired], "afterSave must fire for a Ruby model save" + refute_nil captured[:id], "handler saw an objectId for the Ruby-created object" + assert_equal captured[:raw_createdAt], captured[:raw_updatedAt], + "createdAt == updatedAt on a fresh Ruby-initiated create" + + # ---- SPEC (FAILS ON HEAD) ---- + refute_nil captured[:created_at], + "[SPEC] built object must expose createdAt for a Ruby model save" + refute captured[:existed?], "Ruby-created object must report existed? == false" + end + + def test_b_changed_object_full_state_and_change_detection_on_update + # Seed as master WITH an explicit ACL so Parse Server includes ACL, + # createdAt and updatedAt in the afterSave payload (confirmed behavior: + # a record that carries an ACL emits ACL/createdAt/updatedAt on update). + id = master_create("WebhookStatePost", { + "title" => "original-title", "status" => "draft", "body" => "v1", + "ACL" => { "u_owner" => { "read" => true, "write" => true }, + "*" => { "read" => true, "write" => true } }, + }) + + captured = capture_after_save("WebhookStatePost") + + non_master_update("WebhookStatePost", id, { "title" => "changed-title" }) + wait_until("afterSave fires for non-master update") { captured[:fired] } + + refute captured[:payload_original_nil], "an update carries original" + assert_equal "original-title", captured[:orig_title], + "original_parse_object retains the previous title" + assert_equal "changed-title", captured[:title], + "built object reflects the new title" + + # Raw payload carries server-authoritative fields on an ACL-bearing record. + refute_nil captured[:raw_createdAt], "raw createdAt present on update" + refute_nil captured[:raw_updatedAt], "raw updatedAt present on update" + refute_equal captured[:raw_createdAt], captured[:raw_updatedAt], + "createdAt < updatedAt on an update (raw payload)" + assert captured[:raw_has_acl], "raw payload includes ACL for an ACL-bearing record" + + # ACL survives scrubbing today and must keep surviving. + assert captured[:acl_present], "built object must expose the ACL in afterSave" + + # ---- SPEC (FAILS ON HEAD: timestamps stripped => existed? false) ---- + refute_nil captured[:created_at], "[SPEC] built object must expose createdAt on update" + refute_nil captured[:updated_at], "[SPEC] built object must expose updatedAt on update" + assert captured[:existed?], + "[SPEC] existed? must be true for an updated (already-persisted) object" + + # ---- Change detection: TODAY variant (holds on HEAD) ---- + refute_equal captured[:orig_title], captured[:title], + "field change is detectable by comparing original vs object" + + # ---- Change detection: FIX variant (SPEC, FAILS ON HEAD) ---- + assert captured[:title_changed?], + "[SPEC] title_changed? must be true on an afterSave update" + assert_includes captured[:changed], "title", + "[SPEC] changed must include the mutated field" + end + + def test_b_full_lifecycle_order_end_to_end_on_nonmaster_create + # End-to-end proof that a client-initiated create runs the model lifecycle + # callbacks in ActiveModel order through a real Parse Server: the beforeSave + # webhook runs before_save then before_create (Parse Server has no separate + # beforeCreate trigger), and the afterSave webhook runs after_create then + # after_save. This is the gap the fix closes -- before_create previously + # never ran for REST/JS/Auth0-created objects, and after_save double-fired. + WebhookStatePost.model_callbacks = [] + Parse::Webhooks.route(:before_save, "WebhookStatePost") { parse_object } + Parse::Webhooks.route(:after_save, "WebhookStatePost") { true } + Parse::Webhooks.register_triggers!(@server.url) + + non_master_create("WebhookStatePost", { "title" => "lifecycle", "status" => "draft" }) + wait_until("after_save model callback fires") do + WebhookStatePost.model_callbacks.any? { |c| c[:hook] == :after_save } + end + + order = WebhookStatePost.model_callbacks.map { |c| c[:hook] } + assert_equal [:before_save, :before_create, :after_create, :after_save], order, + "client create must run the lifecycle in ActiveModel order exactly once each; got #{order.inspect}" + + # after_* callbacks see the persisted object (timestamps present). + after_create = WebhookStatePost.model_callbacks.find { |c| c[:hook] == :after_create } + refute after_create[:existed?], "after_create on a fresh create reports existed? == false" + end + + private + + def wait_until(description, timeout: 8) + deadline = Time.now + timeout + loop do + return if yield + flunk "timed out (#{timeout}s) waiting for: #{description}" if Time.now >= deadline + sleep 0.05 + end + end + + def docker_can_reach_host? + result = `docker exec #{ENV["PSNEXT_PREFIX"] || "psnext-it"}-server sh -c 'getent hosts host.docker.internal' 2>&1` + !result.empty? && $?.success? + end + + def non_master_create(class_name, body, extra_headers: {}) + response = Parse.client.request( + :post, + "classes/#{class_name}", + body: body.to_json, + headers: { "X-Parse-Master-Key" => "" }.merge(extra_headers), + opts: { use_master_key: false }, + ) + response.result["objectId"] + end + + def master_create(class_name, body) + response = Parse.client.request(:post, "classes/#{class_name}", body: body.to_json) + response.result["objectId"] + end + + def non_master_update(class_name, object_id, body, extra_headers: {}) + Parse.client.request( + :put, + "classes/#{class_name}/#{object_id}", + body: body.to_json, + headers: { "X-Parse-Master-Key" => "" }.merge(extra_headers), + opts: { use_master_key: false }, + ) + end +end diff --git a/test/lib/parse/webhook_callbacks_test.rb b/test/lib/parse/webhook_callbacks_test.rb index 5e39fdb..9840ad5 100644 --- a/test/lib/parse/webhook_callbacks_test.rb +++ b/test/lib/parse/webhook_callbacks_test.rb @@ -78,11 +78,15 @@ def test_before_save_callback_handling puts "\n=== Testing Before Save Callback Handling ===" # Track callback invocations - prepare_save_called = false + before_save_called = false + before_create_called = false - # Mock Parse::Object with prepare_save! method + # Mock Parse::Object with the before-phase callback runners the dispatcher + # invokes (run_before_save_callbacks always; run_before_create_callbacks for + # new objects -- original.nil?). test_object = Object.new - test_object.define_singleton_method(:prepare_save!) { prepare_save_called = true } + test_object.define_singleton_method(:run_before_save_callbacks) { before_save_called = true } + test_object.define_singleton_method(:run_before_create_callbacks) { before_create_called = true } test_object.define_singleton_method(:changes_payload) { { "name" => "test" } } test_object.define_singleton_method(:is_a?) { |klass| klass == Parse::Object } @@ -91,7 +95,7 @@ def test_before_save_callback_handling test_object end - # Test Ruby-initiated before_save (should skip prepare_save!). + # Test Ruby-initiated before_save (should skip the before callbacks). # A "trusted" Ruby-initiated request requires both the _RB_ header AND # master:true. The header alone is client-controllable; honoring it # without master would let a non-master client spoof the bypass. @@ -105,14 +109,18 @@ def test_before_save_callback_handling ruby_payload = Parse::Webhooks::Payload.new(ruby_payload_data) result = Parse::Webhooks.call_route(:before_save, "TestObject", ruby_payload) - refute prepare_save_called, "prepare_save! should not be called for Ruby-initiated requests" + refute before_save_called, "before_save callbacks should not run for Ruby-initiated requests" + refute before_create_called, "before_create callbacks should not run for Ruby-initiated requests" assert_equal({ "name" => "test" }, result, "Should return changes payload") - puts "✅ Ruby-initiated before_save skips prepare_save!" + puts "✅ Ruby-initiated before_save skips before callbacks" # Reset tracking - prepare_save_called = false + before_save_called = false + before_create_called = false - # Test client-initiated before_save (should call prepare_save!) + # Test client-initiated before_save on a NEW object (no original): both + # before_save AND before_create run, since Parse Server has no separate + # beforeCreate trigger. client_payload_data = { "triggerName" => "beforeSave", "object" => { "className" => "TestObject", "objectId" => "def456" }, @@ -122,9 +130,29 @@ def test_before_save_callback_handling client_payload = Parse::Webhooks::Payload.new(client_payload_data) result = Parse::Webhooks.call_route(:before_save, "TestObject", client_payload) - assert prepare_save_called, "prepare_save! should be called for client-initiated requests" + assert before_save_called, "before_save callbacks should run for client-initiated requests" + assert before_create_called, "before_create callbacks should run for a client-initiated create" assert_equal({ "name" => "test" }, result, "Should return changes payload") - puts "✅ Client-initiated before_save calls prepare_save!" + puts "✅ Client-initiated before_save runs before_save + before_create" + + # Reset tracking + before_save_called = false + before_create_called = false + + # Client-initiated before_save on an UPDATE (original present): before_save + # runs, before_create does NOT (not a create). + update_payload_data = { + "triggerName" => "beforeSave", + "object" => { "className" => "TestObject", "objectId" => "def456", "name" => "new" }, + "original" => { "className" => "TestObject", "objectId" => "def456", "name" => "old" }, + "headers" => { "x-parse-request-id" => "client_update_id" }, + } + update_payload = Parse::Webhooks::Payload.new(update_payload_data) + Parse::Webhooks.call_route(:before_save, "TestObject", update_payload) + + assert before_save_called, "before_save callbacks should run on a client update" + refute before_create_called, "before_create callbacks must NOT run on an update" + puts "✅ Client-initiated before_save on update skips before_create" end def test_after_save_callback_handling