Include decrypted image in push notifications#419
Merged
benthecarman merged 2 commits intosledtools:masterfrom Mar 4, 2026
Merged
Include decrypted image in push notifications#419benthecarman merged 2 commits intosledtools:masterfrom
benthecarman merged 2 commits intosledtools:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds end-to-end image support for rich push notifications: adds Changes
Sequence DiagramsequenceDiagram
participant iOS as NotificationService
participant NSE as NSE (Rust)
participant ImgSrc as ImageServer
participant FS as FileSystem
iOS->>NSE: request decrypt_push_notification(encrypted_payload)
NSE->>NSE: parse tags -> notif_media(tags) -> NotifMedia{kind, tag}
alt media is Image
NSE->>ImgSrc: download image (ureq, <= 10 MB)
ImgSrc-->>NSE: image bytes
NSE->>NSE: download_and_decrypt_image(bytes)
NSE-->>iOS: PushNotificationContent(image_data)
else no valid image
NSE-->>iOS: PushNotificationContent(no image_data)
end
iOS->>iOS: if imageData -> createImageAttachment(data)
iOS->>FS: write temp file
FS-->>iOS: file URL
iOS->>iOS: create UNNotificationAttachment & assign to content.attachments
iOS->>iOS: finalize and deliver notification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
When a push notification contains an image message, the NSE now downloads and decrypts the image via MDK's media manager and attaches it as a UNNotificationAttachment so iOS displays the image thumbnail inline in the notification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/pika-nse/src/lib.rs (1)
256-257:⚠️ Potential issue | 🟠 MajorConfigure bounded request timeouts and HTTPS-only for NSE downloads.
The
ureq::get().call()uses unbounded defaults which can stall NSE processing indefinitely, and permits HTTP requests which could leak encrypted content over unencrypted channels.🔧 Proposed fix using ureq 3.x config API
+use std::time::Duration; + fn download_and_decrypt_image( mdk: &mdk_support::PikaMdk, mls_group_id: &mdk_storage_traits::GroupId, imeta_tag: &nostr::Tag, ) -> Option<Vec<u8>> { let manager = mdk.media_manager(mls_group_id.clone()); let reference = manager.parse_imeta_tag(imeta_tag).ok()?; - let response = ureq::get(&reference.url).call().ok()?; + let response = ureq::get(&reference.url) + .config() + .https_only(true) + .timeout_global(Some(Duration::from_secs(8))) + .build() + .call() + .ok()?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/pika-nse/src/lib.rs` around lines 256 - 257, The code currently calls ureq::get(&reference.url).call().ok()? which uses unbounded timeouts and allows HTTP; change to require HTTPS by validating reference.url.scheme() == "https" (return None or error otherwise), create a configured ureq Agent with ureq::AgentBuilder::new().timeout_connect(Duration::from_secs(...)).timeout_read(Duration::from_secs(...)).timeout_write(Duration::from_secs(...)).build(), then replace ureq::get(&reference.url).call() with agent.get(&reference.url).call().ok()? so downloads use bounded timeouts and only HTTPS.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/pika-nse/src/lib.rs`:
- Around line 256-257: The code currently calls
ureq::get(&reference.url).call().ok()? which uses unbounded timeouts and allows
HTTP; change to require HTTPS by validating reference.url.scheme() == "https"
(return None or error otherwise), create a configured ureq Agent with
ureq::AgentBuilder::new().timeout_connect(Duration::from_secs(...)).timeout_read(Duration::from_secs(...)).timeout_write(Duration::from_secs(...)).build(),
then replace ureq::get(&reference.url).call() with
agent.get(&reference.url).call().ok()? so downloads use bounded timeouts and
only HTTPS.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f1366ad-d192-4ba9-9ea0-e00bdd446f51
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
crates/pika-nse/Cargo.tomlcrates/pika-nse/src/lib.rsios/NotificationService/NotificationService.swift
🚧 Files skipped from review as they are similar to previous changes (1)
- ios/NotificationService/NotificationService.swift
- Configure ureq with https_only and 8s global timeout so a slow download cannot consume the NSE's time budget - Detect actual image format via CGImageSource and pass the correct UTI to UNNotificationAttachment instead of hardcoding .jpg Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ios/NotificationService/NotificationService.swift (1)
156-173: Consider pruning stale temp files to avoid gradual storage buildup.
Line 171creates a new unique file each time, but old files innotif-imagesare never removed. A small pre-write cleanup pass (e.g., remove files older than N hours) would keep NSE container storage bounded.Suggested lightweight cleanup pattern
private static func createImageAttachment(data: Data) -> UNNotificationAttachment? { let tmpDir = FileManager.default.temporaryDirectory .appendingPathComponent("notif-images", isDirectory: true) try? FileManager.default.createDirectory(at: tmpDir, withIntermediateDirectories: true) + // Best-effort cleanup of stale temp files. + if let files = try? FileManager.default.contentsOfDirectory( + at: tmpDir, + includingPropertiesForKeys: [.contentModificationDateKey], + options: [.skipsHiddenFiles] + ) { + let cutoff = Date().addingTimeInterval(-24 * 60 * 60) + for file in files { + if let values = try? file.resourceValues(forKeys: [.contentModificationDateKey]), + let modified = values.contentModificationDate, + modified < cutoff { + try? FileManager.default.removeItem(at: file) + } + } + } + // Detect the actual image type so iOS can decode it correctly. var uti: String = UTType.jpeg.identifier var ext: String = "jpg"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/NotificationService/NotificationService.swift` around lines 156 - 173, Before writing the new image file in NotificationService.swift (around the tmpDir / fileURL / data.write block), add a lightweight cleanup that lists contents of tmpDir via FileManager, checks each file's creation/modification date, and removes files older than a configurable threshold (e.g., N hours) using removeItem(at:). Perform this pass with try? or do/catch to avoid crashing the service, ignore failures per-file, and run it immediately before creating/appending the new UUID file so storage is bounded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/pika-nse/src/lib.rs`:
- Around line 262-263: The NSE currently performs network I/O directly from
untrusted message metadata by calling agent.get(&reference.url).call(); before
doing that, validate reference.url's host against a trusted media-host allowlist
(or rewrite the URL to your trusted media proxy endpoint) and reject or return
None for disallowed hosts; specifically, add a guard near where response is
created that parses reference.url, checks the domain against the allowlist (or
constructs a proxy URL mapping), and only then calls agent.get(...) (otherwise
early-return None or an error) so that agent.get is never invoked on arbitrary
third-party hosts.
---
Nitpick comments:
In `@ios/NotificationService/NotificationService.swift`:
- Around line 156-173: Before writing the new image file in
NotificationService.swift (around the tmpDir / fileURL / data.write block), add
a lightweight cleanup that lists contents of tmpDir via FileManager, checks each
file's creation/modification date, and removes files older than a configurable
threshold (e.g., N hours) using removeItem(at:). Perform this pass with try? or
do/catch to avoid crashing the service, ignore failures per-file, and run it
immediately before creating/appending the new UUID file so storage is bounded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3771a13e-b5c4-4300-9542-32de0181526b
📒 Files selected for processing (2)
crates/pika-nse/src/lib.rsios/NotificationService/NotificationService.swift
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.
Summary
UNNotificationAttachmentso iOS displays the image thumbnail inlineureq(lightweight blocking HTTP client) to pika-nse for the downloadTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes