feat(backend): background GC jobs, schema audit, device capability negotiation, ciphertext-only migration - #482
Merged
Merged
Conversation
…s, files, devices Adds scheduled, idempotent cleanup so storage and DB stay bounded over time: - deviceGc.ts prunes consumed/expired one-time prekeys + MLS key packages, and flags (never deletes) devices revoked past the stale window. - envelopeGc.ts deletes message envelopes that are fully delivered past retention or past a hard max-age ceiling. - fileCleanup.ts gains a configurable hard-delete grace period and pending- upload TTL (previously hardcoded), and a latent bug where the JS `&&` operator silently dropped the isNotNull(deletedAt) filter (only isNull(hardDeletedAt) was actually applied) is fixed to use `and(...)`. New mls_key_packages table + devices.stale_flagged_at column, migrated via 0001_gc_background_jobs.sql. All retention windows are env-configurable (PREKEY_CONSUMED_RETENTION_DAYS, PREKEY_UNCONSUMED_MAX_AGE_DAYS, DEVICE_STALE_AFTER_DAYS, ENVELOPE_DELIVERED_RETENTION_DAYS, ENVELOPE_MAX_AGE_DAYS, FILE_HARD_DELETE_GRACE_MS, PENDING_UPLOAD_TTL_MS) with safe defaults, and every pass is a plain idempotent DELETE/UPDATE WHERE so a crash mid-run is safe to retry.
Audited every db.query.* / relation `with:` usage across routes/, services/, lib/, middleware/, and socket/ against the current db/schema.ts: - Cross-checked every relation name used in a `with:` clause (members, user, wallets, messages, sender, senderDevice, envelopes, editsMessage/edits, conversation, device, prekeys, mlsKeyPackages, pushSubscriptions) against the `relations()` declarations in schema.ts. - Grepped for likely-stale field names from a pre-ciphertext model (.content, .publicKey, .isEncrypted, .nonce/.iv, .mediaUrl/.thumbnailUrl, .sequenceNumber) — no references found; the only `content` occurrences are the client-facing socket payload field (mapped to `ciphertext` before persisting) and unrelated `contentType` usages. - Result: no references to dropped columns/relations remain, and every relation query's result shape matches its consuming type (ConversationPayload/ConversationMemberPayload/MessageLike). Documented, in routes/conversations.ts, why the `as never` casts around the dynamic `getConversationRelations` config are a drizzle generic-inference limitation rather than an unverified shape. - One dangling reference found (not a dropped column): routes/conversations.ts and a test link to docs/message-encryption-migration.md, which doesn't exist yet — authored in the next commit (Task 4). Full backend build + test suite should be run in CI to confirm green, per instruction not to run test/build scripts locally in this session.
Adds devices.capabilities (jsonb) advertising supported protocols
(sealed_box/signal/mls), ciphersuites, and file-transfer versions, so
senders can pick an encryption path both sides support and new protocols
can roll out without breaking older clients.
- lib/capabilities.ts: schema, normalization, and selectProtocol() — a
priority-ordered (mls > signal > sealed_box) negotiation function that
always falls back to sealed_box (every device implicitly supports it,
including rows from before this column existed), and silently ignores
protocol/ciphersuite strings it doesn't recognize instead of erroring.
- POST /auth/verify and POST /devices accept an optional capabilities field
at registration, and apply it on re-verify/re-registration ("upgrade").
- New PATCH /devices/:id/capabilities lets a device advertise updated
capabilities standalone, without re-registering its identity key.
- GET /devices, GET /users/:userId/devices/:deviceId/key-bundle,
GET /user-devices/:id/public-key, and GET /conversations/:id/devices now
return each device's capabilities; the conversation devices endpoint also
returns a computed negotiatedProtocol against the caller's own device.
Migrated via 0002_device_capabilities.sql (defaults existing rows to the
sealed_box-only baseline). Updated devices.test.ts's exact-response-shape
assertion for the new field.
Authors drizzle/0003_ciphertext_only_messages.sql, the migration that resets messages to the ciphertext-only model, plus its rollback and the documented policy decision. - Policy: archive-then-purge (chosen over an in-place tombstone) — any existing plaintext is copied into a new message_content_archive table (no FK to messages, no route/query path through the app's API) before the content column and its GIN index are dropped. Documented in full, including why the tombstone alternative was rejected, at docs/message-encryption-migration.md — this also resolves the dangling link found during the Task 2 audit (routes/conversations.ts's 410 search response, and a matching test, already pointed at this doc path). - Forward migration is idempotent/guarded: the archive-insert only runs if information_schema shows `content` still exists (a plain reference to a dropped column would fail to parse otherwise), every DROP uses IF EXISTS, and the new-columns/tables it also ensures (messages.ciphertext, senderDeviceId, fileId, editsMessageId, deletedAt, message_envelopes) use IF NOT EXISTS — safe to run on this repo's current DB (already ciphertext-only, so every guard is a no-op) or on a populated legacy DB that still has plaintext. - Rollback lives at drizzle/rollback/0003_ciphertext_only_messages.down.sql (drizzle-kit has no down-migration runner, so it's invoked manually) and restores archived plaintext + a GIN index, with its data-loss/scope limitations documented at the top of the script and in the doc. - No silent E2EE claim: once `content` is dropped, serializeMessage()'s existing fallback chain means a pre-cutover row with no ciphertext/ envelope resolves to `unavailable: true` rather than looking like normal ciphertext.
|
@G-ELM Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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
2 participants
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.
Summary
services/deviceGc.ts(prunes consumed/expired one-time prekeys + MLS key packages, flags long-revoked stale devices) andservices/envelopeGc.ts(deletes fully-delivered/expired message envelopes), plus configurable retention windows added to the existingservices/fileCleanup.ts(hard-delete grace period, pending-upload TTL). All passes are plain idempotent DELETE/UPDATE ... WHERE, safe to retry after a crash. Newmls_key_packagestable +devices.stale_flagged_atcolumn (migration0001_gc_background_jobs.sql). Fixed a latent bug infileCleanup.tswhere&&between two drizzle SQL conditions silently dropped theisNotNull(deletedAt)filter.db.query.*/relationwith:usage acrossroutes/,services/,lib/,middleware/, andsocket/againstdb/schema.ts. No references to dropped columns/relations found; documented why theas nevercasts inroutes/conversations.tsare a drizzle generic-inference limitation, not an unverified shape. Surfaced one dangling doc link (resolved by the migration doc below).devices.capabilities(jsonb) advertises supported protocols (sealed_box/signal/mls), ciphersuites, and file-transfer versions (migration0002_device_capabilities.sql). Newlib/capabilities.tsprovides normalization +selectProtocol()(priority-ordered, always falls back to the universalsealed_boxbaseline, ignores unrecognized values instead of erroring). Wired into device registration (POST /auth/verify,POST /devices), a newPATCH /devices/:id/capabilitiesupgrade path, and exposed onGET /devices, the key-bundle endpoint,GET /user-devices/:id/public-key, andGET /conversations/:id/devices(including a computednegotiatedProtocolper device).0003_ciphertext_only_messages.sqlarchives any existingmessages.contentplaintext into a newmessage_content_archivetable (no FK, no API route — archive-then-purge, chosen over an in-place tombstone) before dropping the column and its GIN index. Guarded/idempotent throughout (safe on this repo's already-ciphertext DB or a populated legacy DB). Rollback script + full policy rationale documented atdocs/message-encryption-migration.md, including why old plaintext is never re-served as if it were ciphertext.Test plan
0001–0003against a populated staging DB and confirm they complete cleanlyPATCH /devices/:id/capabilitiesand confirmGET /conversations/:id/devicesreturns the expectednegotiatedProtocoldrizzle/rollback/0003_ciphertext_only_messages.down.sqlagainst a migrated staging copy and confirm archived plaintext is restoredstartDeviceGcJob,startEnvelopeGcJob,startFileCleanupJob) start cleanly on boot and respect their env-configured retention windowsCloses #389
Closes #390
Closes #391
Closes #392