-
Notifications
You must be signed in to change notification settings - Fork 1
[_]: refactor: update encryption handling and address management in email components #71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,15 +16,16 @@ export const useAttachmentsSessionKey = ( | |
| envelope: EncryptionBlock | null, | ||
| ): Uint8Array | null => { | ||
| const keypair = MailKeysService.instance.getCurrentKeys(); | ||
| const address = MailKeysService.instance.getCurrentAddress(); | ||
| const [cache, setCache] = useState<Record<string, CachedKey>>({}); | ||
|
|
||
| useEffect(() => { | ||
| if (!mailId || !envelope || !keypair) return; | ||
| if (!mailId || !envelope || !keypair || !address) return; | ||
| if (cache[mailId]) return; | ||
|
Comment on lines
20
to
24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Key the cache by all decryption inputs, not only When the address changes, the effect reruns but immediately accepts the previous address’s cached entry. This can return a stale cross-account session key—or retain a prior failure. Include at least the normalized address, and ideally envelope/keypair identity, in the cache entry and add an address-switch regression test. Also applies to: 40-40 🤖 Prompt for AI Agents |
||
|
|
||
| let cancelled = false; | ||
| MailEncryptionService.instance | ||
| .decryptAttachmentsSessionKey(envelope, keypair) | ||
| .decryptAttachmentsSessionKey(envelope, keypair, address) | ||
| .then((key) => { | ||
| if (!cancelled) setCache((prev) => ({ ...prev, [mailId]: { ok: true, key } })); | ||
| }) | ||
|
|
@@ -36,7 +37,7 @@ export const useAttachmentsSessionKey = ( | |
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, [mailId, envelope, keypair, cache]); | ||
| }, [mailId, envelope, keypair, address, cache]); | ||
|
|
||
| if (!mailId) return null; | ||
| const entry = cache[mailId]; | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,11 +6,19 @@ import { MailKeysService } from '@/services/mail-keys'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type Summary = EmailListResponse['emails'][number]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const decryptPendingPreviews = async (pending: Summary[], keypair: HybridKeyPair): Promise<Record<string, string>> => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const decryptPendingPreviews = async ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pending: Summary[], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| keypair: HybridKeyPair, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address: string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ): Promise<Record<string, string>> => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const resolved: Record<string, string> = {}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (const summary of pending) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolved[summary.id] = await MailEncryptionService.instance.decryptSummaryPreview(summary.encryption!, keypair); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolved[summary.id] = await MailEncryptionService.instance.decryptSummaryPreview( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| summary.encryption!, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| keypair, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| address, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+9
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win Process preview decryptions concurrently. Currently, the loop processes preview decryptions sequentially. Since WebCrypto operations are asynchronous and offloaded to the browser's crypto thread, using ♻️ Proposed refactor-const decryptPendingPreviews = async (
- pending: Summary[],
- keypair: HybridKeyPair,
- address: string,
-): Promise<Record<string, string>> => {
- const resolved: Record<string, string> = {};
- for (const summary of pending) {
- try {
- resolved[summary.id] = await MailEncryptionService.instance.decryptSummaryPreview(
- summary.encryption!,
- keypair,
- address,
- );
- } catch (error) {
- console.error('Failed to decrypt mail preview', { mailId: summary.id, error });
- }
- }
- return resolved;
-};
+const decryptPendingPreviews = async (
+ pending: Summary[],
+ keypair: HybridKeyPair,
+ address: string,
+): Promise<Record<string, string>> => {
+ const resolved: Record<string, string> = {};
+ await Promise.all(
+ pending.map(async (summary) => {
+ try {
+ resolved[summary.id] = await MailEncryptionService.instance.decryptSummaryPreview(
+ summary.encryption!,
+ keypair,
+ address,
+ );
+ } catch (error) {
+ console.error('Failed to decrypt mail preview', { mailId: summary.id, error });
+ }
+ })
+ );
+ return resolved;
+};📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error('Failed to decrypt mail preview', { mailId: summary.id, error }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -21,27 +29,28 @@ const decryptPendingPreviews = async (pending: Summary[], keypair: HybridKeyPair | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Decrypts the preview snippet for the encrypted rows on a list page. The | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * backend projects an `encryption` block ({ encryptedPreview, wrappedKeys }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * onto each encrypted summary the caller can read; we trial-decrypt it with the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * caller's keypair, exactly as the full body is decrypted. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * onto each encrypted summary the caller can read; the caller's wrapped key is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * found by its address label, exactly as the full body is decrypted. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Returns a map of `emailId -> decrypted preview`. Rows are decrypted at most | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * once (tracked in `attempted`), so re-renders and pagination don't re-run the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * crypto, and a row that fails simply stays absent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const useDecryptedPreviews = (summaries: Summary[] | undefined): Record<string, string> => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const senderKeys = MailKeysService.instance.getCurrentKeys(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const senderAddress = MailKeysService.instance.getCurrentAddress(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [previews, setPreviews] = useState<Record<string, string>>({}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const attempted = useRef<Set<string>>(new Set()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useEffect(() => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!senderKeys || !summaries?.length) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!senderKeys || !senderAddress || !summaries?.length) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const pending = summaries.filter((s) => s.encryption && !attempted.current.has(s.id)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pending.length === 0) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pending.forEach((s) => attempted.current.add(s.id)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let cancelled = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| decryptPendingPreviews(pending, senderKeys).then((resolved) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| decryptPendingPreviews(pending, senderKeys, senderAddress).then((resolved) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!cancelled && Object.keys(resolved).length) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| setPreviews((prev) => ({ ...prev, ...resolved })); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -50,7 +59,7 @@ export const useDecryptedPreviews = (summaries: Summary[] | undefined): Record<s | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cancelled = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [summaries, senderKeys]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, [summaries, senderKeys, senderAddress]); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return previews; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Abort email dispatch if inherited attachments fail to process.
When
handleInheritAttachmentsencounters missing keys, missing addresses, or decryption errors, it displays an error toast and returnsundefined. However, thesendfunction executesconst attachmentsToSend = await handleInheritAttachments();and immediately proceeds without verifying if the result isundefined.This causes the email to be silently sent without its attachments if processing fails. To prevent this data loss, verify the return value in the
sendfunction and abort the operation if the attachment recovery failed.🤖 Prompt for AI Agents