Multimedia in PersonalID Messenger

[ctsims]

Technical Summary

Adds Rich (multimedia enabled) Messaging to the existing PersonalID messenger service along with the new models and storage. Safe, in-place addition that doesn't require changes to other infra (exiting callers, or existing Android apps).

Likely best reviewed commit by commit.

Detailed design notes available

Initial mobile implementation located here.

Adds POST /messaging/create_message/ (multipart: text plus encrypted attachments), attachment storage and download, and rich fields in the device sync. Sending stays switched off behind RICH_MESSAGING_ENABLED (default False). Design: rich-messaging/02-ground-design.md, 05-personalid-spec.md.

Migration (messaging/0008)

• Additive only: four nullable columns on Message and a new MessageAttachment table.
• ⚠️ A partial index on messaging_message(expires_at): building it scans the table once and blocks writes to it while it runs. Plain messages are never deleted today, so in theory the row count should be checked today in case this reindex is extensive.

Changes outside of new Rich Messages
The design attempts to isolate new functionality strictly to the new callpaths. The push, retrieve_messages, reply forwarding and send_fcm are unchanged. Literal pinning tests in messaging/test_message_projections.py cover them.

However, there are a couple of areas that are changed for existing code.

• retrieve_notifications: every messaging entry gains "version": 2. This is the only change to an existing output. CommCare apps with older messaging versions ignore keys they don't know.
• Storage: profile photos move from Django's default storage to a named user_photos storage (as discussed in Configuration mode for Local development  #307). Same bucket, same AWS_S3_PHOTO_BUCKET_NAME. There is no longer a default storage, so any code using default_storage fails loudly instead of writing to the photo bucket.

Known limits or risks

• This creates the surface area for expiration, but it doesn't actually set up storage cleanup.
• Potentially amplifies other risks (IE: Auto channel enrollment) as a path to denial of service
• Media messages are still encrypted, but the presence and size of attachments is known to the PersonalID server to support other controls.

Logging and monitoring

No initial surface area after rollout until adopted.

Safety Assurance

Safety story

Safety

The behavior of this PR was validated against the real android application with a test sender app using both existing Android apps (to ensure compatibility and fallbacks were correctly implemented) and the new implementation (to test edge cases and ensure compatibility with previous messaging).

Core storage refactors were already applied, and existing messaging continues to work even if the new attachment bucket fails.

All migrations are additive only: Redeploy of previous version can happen in-place if critical.

Other key safety notes:

• There are limits on individual and overall attachment volume: A request declaring more than 16 MiB is refused before the body is parsed. Limits: 10 attachments, 2.5 MiB each, 15 MiB per message.

• Attachments are auth scoped to their message. Downloads check that the message belongs to the requesting user first, then look the attachment up within it.

• PersonalID still never has access to message contents, but the metadata and number/type of attachments are visible to ensure processing is functional.

• I am confident that this change will not break current and/or previous versions of CommCare apps

Automated test coverage

Automated tests cover both the new functionality and the compatibility guarantees important to rollout.

QA Plan

Existing Messaging tests should continue to function identically for QA assurance.

Deployment

Rollout will require:

• Creating the new bucket (personalid-message-attachments) with identical ACL and semantics to the profile photo bucket. This be done pre-emptively and idempotently)

  • This is done, and a check was completed on the worker to confirm permissions for creating, reading, deleting from the new bucket.

• The migration and new index on deploy

  • Index size check performed to confirm safety. (~300mb or so of existing data)

• This PR can be deployed: any required configuration, commands, or other manual steps required before this PR can be deployed are done.

Labels & Review

• The set of people pinged as reviewers is appropriate for the level of risk of the change

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1a6158c0-9c71-40fe-9ae8-19bc7df5a482

📥 Commits

Reviewing files that changed from the base of the PR and between bb6756c and 364acf6.

📒 Files selected for processing (26)

• .env_template
• CLAUDE.md
• conftest.py
• connectid/settings.py
• messaging/apps.py
• messaging/const.py
• messaging/factories.py
• messaging/management/commands/_local_message_server/README.md
• messaging/management/commands/_local_message_server/server.py
• messaging/management/commands/_local_message_server/tests/test_local_message_server.py
• messaging/migrations/0008_rich_messages.py
• messaging/models.py
• messaging/rich_messages.py
• messaging/serializers.py
• messaging/signals.py
• messaging/test_attachment_download.py
• messaging/test_message_projections.py
• messaging/test_models.py
• messaging/test_rich_messages.py
• messaging/urls.py
• messaging/views.py
• users/services.py
• users/tests/test_services.py
• users/tests/test_views.py
• utils/storage.py
• utils/tests/test_storage.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The change adds configurable storage buckets for user photos and message attachments. It adds rich-message fields and attachment records, a multipart message-creation endpoint, message serialization updates, and attachment downloads. The local message server can send rich messages, create future-version messages, and clear unacknowledged messages. Tests cover storage configuration, message validation, serialization, attachment access, and local-server operations.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 364ac

No actionable issue remains from the reviewed changes; the PR is mergeable after normal checks and the documented deployment prerequisites.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 364ac

Message creation and downloads have explicit authentication and ownership controls, and new sending is disabled by default. Before enabling it, expiry and attachment cleanup need an end-to-end lifecycle guarantee: expired messages remain eligible for delivery, and interrupted or failed cleanup can leave stored files behind.

Retained concerns

• Medium · security · observed: When sending is enabled, expires_at blocks attachment downloads but does not exclude expired messages from notification sync, legacy retrieval, or notification resending. Retention still deletes only messages received more than seven days ago, so never-received rich messages and their attachment records have no application-level expiry cleanup. This extends the pre-existing retention behavior to newly persisted encrypted files; the default-off staging flag limits current exposure.
• Medium · security · inferred: Attachment files are written before durable ownership rows exist. Ordinary failures are compensated, but hard interruption can leave unowned files, and failed deletion after row removal loses the application's ownership reference. The deletion code names a bucket lifecycle rule as its backstop, but that deployed rule was not established. Eventual retirement of encrypted data therefore remains conditional on recovery outside the inspected application path.

Security review details

Security Blast Radius

• inferred — A compromised messaging-server credential can submit new rich resources to consenting channels belonging to that server when intake is enabled. A compromised user identity can address attachments across that user's channels. Retained or orphaned encrypted files affect the shared attachment store; plaintext compromise and public bucket exposure were not established.

Security Findings and Attack Paths

• observed — After activation, an authenticated sender can create a message with a future expiry that later remains eligible for sync or resend. An unreceived message is not selected by the existing received-based deletion task. This is an incomplete lifecycle for new rich resources, not an anonymous access path or demonstrated cross-user download.

Trust Boundaries and Controls

• observed — Uploaded filenames do not choose storage keys: keys use channel, message, and attachment IDs. Validation limits attachments to ten, 2.5 MiB each and 15 MiB total, and checks actual file sizes against declarations. A separate pre-parse guard checks the declared request length; transport-independent enforcement was not verified.

Resilience and Maintainability Implications

• inferred — Database rollback and successful compensation contain ordinary partial failures. They cannot guarantee file retirement after process interruption or a failed post-commit delete. The code's stated lifecycle backstop needs deployment evidence before it can be treated as a dependable retention control.

Hardening Proposals

• proposed — Before enabling intake, define expiry consistently across delivery and retention consumers, and establish flag-independent file recovery through durable cleanup work or verified bucket lifecycle behavior. Separately verify ingress and framework body limits rather than treating the declared-length guard as proof of complete resource containment.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.97% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 144 functions across 23 files. (3 skipped…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title identifies multimedia messaging in PersonalID Messenger, which summarizes the main change.
Description check	✅ Passed	The description explains the rich-messaging endpoint, attachments, storage, compatibility, limits, tests, and deployment considerations.

Full details: Docstring Coverage

Explanation

Docstring coverage is 15.97% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 144 functions across 23 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}