Skip to content

fix(group,template,status): bound participant arrays and template renders, and reconcile status media orphans - #964

Merged
rmyndharis merged 1 commit into
mainfrom
fix/bounds-leftovers
Jul 27, 2026
Merged

fix(group,template,status): bound participant arrays and template renders, and reconcile status media orphans#964
rmyndharis merged 1 commit into
mainfrom
fix/bounds-leftovers

Conversation

@rmyndharis

Copy link
Copy Markdown
Owner

Summary

Three bounds/consistency hardenings across the group, template, and status-store modules.

1. Group: bound participant arrays, report partially-applied settings patches

  • CreateGroupDto.participants and the shared ParticipantsDto (add / remove / promote / demote) now enforce @ArrayMaxSize(256). The engine works a participant list sequentially — one WhatsApp round-trip per participant with an honest per-participant result — so an unbounded array turned a single HTTP request into unbounded serial engine work. 256 stays well inside WhatsApp's own group-size limit; larger imports batch across requests. Over-limit bodies now fail validation with a 400 before any engine call. The OpenAPI schema gains maxItems: 256 on both DTOs (snapshot regenerated).
  • GroupService.updateGroupSettings applied fields sequentially and, on a mid-patch engine failure, propagated the bare engine error — leaving the group silently half-patched (e.g. ephemeralSeconds applied, announce failed). A failure on a later field now throws an error that names the failed field and the ones already applied, preserving the underlying HTTP status (a 403 refusal stays 403, a 501 stays 501). A failure on the first applied field propagates unchanged — nothing was applied, so there is no partial state to report. The existing ordering rule (ephemeral first, so wwjs's deterministic 501 never half-applies a patch) is preserved.

2. Template: cap the post-render length

MessageService.sendTemplate substituted caller-supplied variables into the stored template with no bound on the final text, so a small template plus a huge variable inflated the payload toward the engine and the messages table unboundedly. The joined rendered text is now capped at 64 KiB (TEMPLATE_RENDER_MAX_CHARS, also WhatsApp's own text-message ceiling). An over-cap render is rejected with a 400 naming the limit — never silently truncated; at-or-under renders are byte-identical to before.

3. Status store: safe media write ordering, purge retry, orphan reconciliation

  • Row-first ingest. ingest previously wrote the media file before saving the row, so a crash in between leaked a permanent orphan file (and the unique-race loser needed bespoke reaping). The row is now saved first — media-less outcomes settled up front, a keepable blob saved as omitted-for-now — and the file write plus row update happen after. A crash mid-ingest now leaves a consistent media-omitted row (worst case: a missing attachment); the only remaining orphan source is the narrow window between the file write and the row update. The race loser no longer writes a file at all, so the reaping-on-save-failure logic is gone.
  • Purge keeps rows whose file delete fails. purgeExpired swallowed a deleteFile failure and deleted the row anyway, permanently orphaning the file. A row whose media delete fails is now kept and retried on the next sweep; file-less rows in the same batch are still purged. (An already-missing file counts as a successful delete, so a gone file never wedges its row.)
  • Orphan reconciliation sweep. A new lightweight sweep (startup + hourly, STATUS_ORPHAN_SWEEP_INTERVAL_MS) lists statuses/ media keys via StorageService.listFiles() and deletes ones no row references — scoped to the statuses/ prefix so shared chat media is never touched. A file is deleted only after being seen unreferenced for a grace window (STATUS_ORPHAN_GRACE_MS, default 1h; first-seen tracked in memory, so a restart fails safe by restarting the clock), which keeps mid-ingest files safe.

Tests

  • Group: participant arrays at/over 256 accepted/rejected (both DTOs); a partially-applied settings patch names the failed field and the applied ones, keeps the underlying status, and stops at the failure; first-field failures propagate unchanged.
  • Template: over-cap render → 400 naming the limit (engine never called); exactly-at-cap render passes through unchanged; TEMPLATE_RENDER_MAX_CHARS override honored.
  • Status store: purge with a failing deleteFile keeps the media row (text row still purged) and a later healthy sweep finishes; orphan sweep reaps an unreferenced file only after the grace window, never touches row-referenced or non-statuses/ files, honors the grace override; a file that becomes referenced between sightings is never reaped; race tests re-pinned to row-first semantics (no file is written before the row lands).
  • npx jest src/modules/group src/modules/template src/modules/status-store src/modules/message — 15 suites / 284 tests pass; npx eslint clean on the touched modules; npm run build clean; npm run openapi:check passes (snapshot regenerated for the DTO maxItems).

Risks

  • Behavioral 400s: participant batches over 256 and template renders over 64 KiB now fail client-visible 400s where they previously ran. Both limits are documented in .env.example; the template cap is env-tunable, the participant cap is a DTO constant.
  • Two saves per media status ingest (row, then row+media reference) instead of one — negligible at status volume, and it is what makes the crash window consistent.
  • In-memory grace tracking for the orphan sweep: a restart defers orphan deletion by one grace window (conservative direction; orphans are never referenced, so they can never go live).
  • A status read in the milliseconds between the two ingest saves briefly shows its media as omitted; the follow-up update flips it before the status.received webhook fires.

…ders, and reconcile status media orphans

Group: cap participant arrays at 256 per create/batch request (@ArrayMaxSize;
the engine works the list sequentially, so an unbounded array turned one HTTP
call into unbounded serial engine work). A settings patch that fails midway now
throws an error naming the failed field and the ones already applied, keeping
the underlying HTTP status, instead of leaving a silently half-applied patch.

Template: cap the final rendered text of a send-template request at 64 KiB
(TEMPLATE_RENDER_MAX_CHARS). Caller-supplied variables could inflate a small
template unboundedly toward the engine/DB; an over-cap render is now rejected
with a 400 naming the limit, never silently truncated.

Status store: ingest now saves the row BEFORE writing its media file, so a
crash mid-ingest leaves a consistent media-omitted row instead of a permanent
orphan file. The TTL purge keeps a row whose media delete fails (retried on
the next sweep) instead of orphaning its file. A new hourly reconciliation
sweep (STATUS_ORPHAN_SWEEP_INTERVAL_MS) deletes statuses/ files no row
references after a grace window (STATUS_ORPHAN_GRACE_MS, default 1h each).
@rmyndharis
rmyndharis force-pushed the fix/bounds-leftovers branch from e094985 to 3cb2e3c Compare July 27, 2026 08:07
@rmyndharis
rmyndharis merged commit d7b904b into main Jul 27, 2026
14 checks passed
@rmyndharis
rmyndharis deleted the fix/bounds-leftovers branch July 27, 2026 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant