fix(message): persist bulk-sent messages, sanitize SSRF errors, honor cross-instance cancel#340
Closed
rmyndharis wants to merge 1 commit into
Closed
fix(message): persist bulk-sent messages, sanitize SSRF errors, honor cross-instance cancel#340rmyndharis wants to merge 1 commit into
rmyndharis wants to merge 1 commit into
Conversation
… cross-instance cancel Bulk batches called the engine send methods directly, so each message bypassed the single-send persistence path: the rows never reached the messages table (invisible to chat history and statistics), a blocked-destination (SSRF) error was stored verbatim in the batch result — exposing the internal address it refused, readable via GET batch status — and a cancellation was only honored by the process that created the batch. Bulk sends now reuse the shared outgoing-message persistence, a blocked-destination error is reported as a generic code (and no longer logged verbatim), and processBatch re-reads the batch status from the database at its save cadence and before the final write, so a cancel issued by another instance (or after a restart) stops the batch and is not clobbered back to a completed status.
Merged
rmyndharis
added a commit
that referenced
this pull request
Jun 19, 2026
* fix(webhook): deliver session lifecycle events and key webhook hardening (#335) * fix(security): pin outbound webhook and media fetches to validated IP (#338) * fix(plugins): persist plugin enable/config and restore (#339) * fix(message): persist bulk-sent messages, sanitize SSRF (#340) * fix(engine): return the real id for forwarded messages (#341) * fix(security): harden outbound requests, IPv6 SSRF (#344) * fix(security): secret-file perms, key pepper, allowedIps (#345) * fix(storage): bound tar imports, contain storage keys (#346) * fix(session): reconcile late acks, serialize reactions (#348) * fix(contract): webhook timeout, bounded shutdown, 501 (#350) * feat(session): force-kill a stuck session (#352) * merge #343 * merge #351 * chore(release): v0.4.3 — CHANGELOG, version bump (package.json/dashboard/swagger), README + docs
Owner
Author
|
Shipped in v0.4.3 (integrated via the release PR #354 and tagged |
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
1 participant
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.
Problem
Bulk batches called the engine's send methods directly, bypassing the single-send persistence path. Three consequences:
messagestable, so they were invisible to chat history and statistics (the engine echo fires the webhook/WS but does not write the DB).String(error)— which names the refused internal host/IP — was stored verbatim in the batch result and returned by the batch-status endpoint.Fix
saveOutgoingMessage, now public) after each successful send — best-effort, so a persistence failure never flips a sent message to failed. Confirmed no double-write (the engine echo does not persist).sanitizeBatchErrormaps a blocked-destination error to a genericSEND_BLOCKEDcode; the internal address is no longer stored, returned, or logged verbatim.processBatchre-readsbatch.statusfrom the database at its save cadence — and once more before the final write — so a cancel from another instance/restart stops the batch and is never clobbered back to a completed status. The status is read before each save so a concurrentCANCELLEDis never overwritten.Scope notes
Implemented additively (bulk-only) rather than re-routing through
message.service's full send pipeline, to avoid changing bulk's plugin-hook/typing-humaniser behaviour. Multi-replica cancel is honoured at the save cadence (the existing checkpoint granularity), matching the batch's periodic-save design.Compatibility
No API or response-shape change; non-breaking. SemVer: patch.
Tests
sanitizeBatchError: SSRF → generic message (no internal address); ordinary error passes through underSEND_FAILED.processBatch: persists each sent message; stops when the batch is cancelled in the DB by another instance; does not clobber aCANCELLEDthat lands after the last cadence read.