Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"dependencies": {
"@internxt/css-config": "^1.1.0",
"@internxt/lib": "^1.4.1",
"@internxt/sdk": "^1.17.11",
"@internxt/sdk": "^1.17.16",
"@internxt/ui": "^0.1.25",
"@phosphor-icons/react": "^2.1.10",
"@reduxjs/toolkit": "^2.11.2",
Expand Down
30 changes: 25 additions & 5 deletions src/components/compose-message/hooks/useComposeSend.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ const recipient = (email: string): Recipient => ({ id: email, email });
const show = vi.mocked(notificationsService.show);

const mockEncryptionBlock = {
version: 'v1',
version: 'v2',
encryptedText: 'ct',
encryptedPreview: 'cp',
wrappedKeys: [],
attachmentWrappedKeys: [],
encryptedPreview: 'cp',
previewWrappedKeys: [],
};

const renderSend = (overrides: Partial<Parameters<typeof useComposeSend>[0]> = {}) => {
Expand Down Expand Up @@ -175,11 +175,31 @@ describe('useComposeSend', () => {
expect.any(Uint8Array),
);
expect(mocks.sendEmail).toHaveBeenCalledWith(
expect.objectContaining({ encryption: expect.objectContaining({ version: 'v1' }), deliveryMode: 'INTERNXT' }),
expect.objectContaining({ encryption: expect.objectContaining({ version: 'v2' }), deliveryMode: 'INTERNXT' }),
);
expect(onSent).toHaveBeenCalled();
});

test('When all recipients are Internxt and one is Bcc, then the send is blocked', async () => {
const buildSpy = vi.spyOn(MailEncryptionService.instance, 'buildEncryptionBlock');

const { result, onSent } = renderSend({
toRecipients: [recipient('bob@inxt.me')],
bccRecipients: [recipient('carol@inxt.me')],
});

expect(result.current.encryptionState).toBe('internxt');

await act(async () => {
await result.current.send();
});

expect(show).toHaveBeenCalledWith(expect.objectContaining({ text: 'errors.mail.bccNotSupportedEncrypted' }));
expect(buildSpy).not.toHaveBeenCalled();
expect(mocks.sendEmail).not.toHaveBeenCalled();
expect(onSent).not.toHaveBeenCalled();
});

describe('external recipients', () => {
let getVariable: ReturnType<typeof vi.spyOn>;

Expand Down Expand Up @@ -226,7 +246,7 @@ describe('useComposeSend', () => {
expect.any(Uint8Array),
);
expect(mocks.sendEmail).toHaveBeenCalledWith(
expect.objectContaining({ deliveryMode: 'EXTERNAL', encryption: expect.objectContaining({ version: 'v1' }) }),
expect.objectContaining({ deliveryMode: 'EXTERNAL', encryption: expect.objectContaining({ version: 'v2' }) }),
);
expect(onSent).toHaveBeenCalled();
});
Expand Down
10 changes: 9 additions & 1 deletion src/components/compose-message/hooks/useComposeSend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@

if (pendingInherited.length > 0) {
const senderKeysForAttachments = MailKeysService.instance.getCurrentKeys();
if (!senderKeysForAttachments) {
const senderAddress = MailKeysService.instance.getCurrentAddress();
if (!senderKeysForAttachments || !senderAddress) {
Comment on lines 96 to +100

Copy link
Copy Markdown

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 handleInheritAttachments encounters missing keys, missing addresses, or decryption errors, it displays an error toast and returns undefined. However, the send function executes const attachmentsToSend = await handleInheritAttachments(); and immediately proceeds without verifying if the result is undefined.

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 send function and abort the operation if the attachment recovery failed.

    // In `send` (around line 183):
    const attachmentsToSend = await handleInheritAttachments();
    if (!attachmentsToSend) return;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/compose-message/hooks/useComposeSend.ts` around lines 96 -
100, Update the send function to check the result of handleInheritAttachments
immediately after awaiting it, and return early when it is undefined before
dispatching the email; preserve normal sending when attachment recovery
succeeds.

notificationsService.show({
text: translate('errors.mail.forwardAttachmentFailed'),
type: ToastType.Error,
Expand All @@ -110,6 +111,7 @@
const originalSessionKey = await MailEncryptionService.instance.decryptAttachmentsSessionKey(
item.originalEnvelope,
senderKeysForAttachments,
senderAddress,
);

const { blob } = await NetworkService.instance.download({
Expand Down Expand Up @@ -167,6 +169,12 @@
return;
}

// TODO: remove this once per-recipient delivery is implemented

Check warning on line 172 in src/components/compose-message/hooks/useComposeSend.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Complete the task associated to this "TODO" comment.

See more on https://sonarcloud.io/project/issues?id=internxt_mail-web&issues=AZ9jC4RTG8KTb5wKI8pw&open=AZ9jC4RTG8KTb5wKI8pw&pullRequest=71
if (encryptionState === 'internxt' && bccRecipients.length > 0) {
notificationsService.show({ text: translate('errors.mail.bccNotSupportedEncrypted'), type: ToastType.Warning });
return;
}

if (!senderKeys?.address || !senderKeys.publicKey) {
notificationsService.show({ text: translate('errors.mail.keyLookupFailed'), type: ToastType.Error });
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,11 @@ const editor = { getHTML: () => '<p>hi</p>', getText: () => 'hi', on: vi.fn(), o
const recipient = (email: string): Recipient => ({ id: email, email });

const mockEncryptionBlock = {
version: 'v1',
version: 'v2',
encryptedText: 'ct',
encryptedPreview: 'cp',
wrappedKeys: [],
attachmentWrappedKeys: [],
encryptedPreview: 'cp',
previewWrappedKeys: [],
};

const renderDraft = (overrides: Partial<Parameters<typeof useDraftMessage>[0]> = {}) => {
Expand Down Expand Up @@ -84,7 +84,7 @@ describe('Draft Message', () => {
expect.objectContaining({
subject: 'A subject',
to: [{ email: 'bob@inxt.me' }],
encryption: expect.objectContaining({ version: 'v1' }),
encryption: expect.objectContaining({ version: 'v2' }),
}),
);
});
Expand Down
5 changes: 3 additions & 2 deletions src/components/compose-message/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,10 @@ export const ComposeMessageDialog = () => {
const envelope = composeDialogData?.mode === 'draft' ? composeDialogData.draft.encryption : undefined;
if (!envelope) return;
const keys = MailKeysService.instance.getCurrentKeys();
if (!keys) return;
const address = MailKeysService.instance.getCurrentAddress();
if (!keys || !address) return;
MailEncryptionService.instance
.decryptAttachmentsSessionKey(envelope as EncryptionBlock, keys)
.decryptAttachmentsSessionKey(envelope as EncryptionBlock, keys, address)
.then((key) => {
setAttachmentsSessionKey(key);
sessionKeyHydratedRef.current = true;
Expand Down
7 changes: 4 additions & 3 deletions src/hooks/mail/useAttachmentsSessionKey.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,17 @@ import { MailKeysService } from '@/services/mail-keys';
const KEY = new Uint8Array([1, 2, 3, 4]);
const KEYPAIR = { secretKey: new Uint8Array(32), publicKey: new Uint8Array(32) } as unknown as HybridKeyPair;
const ENVELOPE = {
version: 'v1',
version: 'v2',
encryptedText: 'ct',
encryptedPreview: 'cp',
wrappedKeys: [],
attachmentWrappedKeys: [],
encryptedPreview: 'cp',
previewWrappedKeys: [],
} as unknown as EncryptionBlock;

const setKeys = (keys: HybridKeyPair | null) => {
const spy = vi.spyOn(MailKeysService.instance, 'getCurrentKeys');
spy.mockReturnValue(keys);
vi.spyOn(MailKeysService.instance, 'getCurrentAddress').mockReturnValue(keys ? 'me@inxt.me' : null);
return spy;
};

Expand Down
7 changes: 4 additions & 3 deletions src/hooks/mail/useAttachmentsSessionKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 mailId.

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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/mail/useAttachmentsSessionKey.ts` around lines 20 - 24, Update the
cache guard and cache entries in the effect within useAttachmentsSessionKey so
cached results are keyed by the normalized address in addition to mailId,
preventing reuse across account switches; include envelope and keypair identity
as cache inputs where available. Preserve existing success and failure handling,
and add a regression test covering an address change that must not reuse the
previous cached entry.


let cancelled = false;
MailEncryptionService.instance
.decryptAttachmentsSessionKey(envelope, keypair)
.decryptAttachmentsSessionKey(envelope, keypair, address)
.then((key) => {
if (!cancelled) setCache((prev) => ({ ...prev, [mailId]: { ok: true, key } }));
})
Expand All @@ -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];
Expand Down
1 change: 1 addition & 0 deletions src/hooks/mail/useDecryptedPreviews.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const plainSummary = (id: string): Summary => ({ id }) as unknown as Summary;

const setKeys = (keys: HybridKeyPair | null) => {
vi.spyOn(MailKeysService.instance, 'getCurrentKeys').mockReturnValue(keys);
vi.spyOn(MailKeysService.instance, 'getCurrentAddress').mockReturnValue(keys ? 'me@inxt.me' : null);
};

const spyOnPreviewDecrypt = () => vi.spyOn(MailEncryptionService.instance, 'decryptSummaryPreview');
Expand Down
23 changes: 16 additions & 7 deletions src/hooks/mail/useDecryptedPreviews.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The 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 Promise.all to decrypt them concurrently will significantly improve performance when rendering a view with multiple encrypted previews.

♻️ 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

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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,
);
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;
};
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hooks/mail/useDecryptedPreviews.ts` around lines 9 - 21, Update
decryptPendingPreviews to start all decryptSummaryPreview operations
concurrently and await them with Promise.all, then populate the resolved record
while preserving each summary.id mapping and existing error-handling behavior.

} catch (error) {
console.error('Failed to decrypt mail preview', { mailId: summary.id, error });
}
Expand All @@ -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 }));
}
Expand All @@ -50,7 +59,7 @@ export const useDecryptedPreviews = (summaries: Summary[] | undefined): Record<s
return () => {
cancelled = true;
};
}, [summaries, senderKeys]);
}, [summaries, senderKeys, senderAddress]);

return previews;
};
1 change: 1 addition & 0 deletions src/i18n/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@
"forwardAttachmentFailed": "Could not prepare an attachment for forwarding. Please try again.",
"internxtKeyMissing": "Could not find encryption keys for an Internxt recipient. Please try again later.",
"encryptionUnavailable": "Encryption isn't ready yet. Please try again in a moment.",
"bccNotSupportedEncrypted": "Bcc isn't available for encrypted emails yet. Move the recipient to To or Cc.",
"draftSaveFailed": "Could not save the draft. Your changes are still here.",
"draftDiscardFailed": "Could not delete the draft. Please try again.",
"draftOpenFailed": "Could not open the draft. Please try again."
Expand Down
1 change: 1 addition & 0 deletions src/i18n/locales/es.json
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@
"forwardAttachmentFailed": "No se pudo preparar un adjunto para reenviar. Inténtalo de nuevo.",
"internxtKeyMissing": "No se encontraron claves de cifrado para un destinatario de Internxt. Inténtalo de nuevo más tarde.",
"encryptionUnavailable": "El cifrado aún no está listo. Inténtalo de nuevo en un momento.",
"bccNotSupportedEncrypted": "Cco aún no está disponible para correos cifrados. Mueve el destinatario a Para o Cc.",
"draftSaveFailed": "No se pudo guardar el borrador. Tus cambios siguen aquí.",
"draftDiscardFailed": "No se pudo eliminar el borrador. Inténtalo de nuevo.",
"draftOpenFailed": "No se pudo abrir el borrador. Inténtalo de nuevo."
Expand Down
1 change: 1 addition & 0 deletions src/i18n/locales/fr.json
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@
"forwardAttachmentFailed": "Impossible de préparer une pièce jointe pour le transfert. Veuillez réessayer.",
"internxtKeyMissing": "Impossible de trouver les clés de chiffrement pour un destinataire Internxt. Veuillez réessayer plus tard.",
"encryptionUnavailable": "Le chiffrement n'est pas encore prêt. Veuillez réessayer dans un instant.",
"bccNotSupportedEncrypted": "Le Cci n'est pas encore disponible pour les e-mails chiffrés. Déplacez le destinataire vers À ou Cc.",
"draftSaveFailed": "Impossible d'enregistrer le brouillon. Vos modifications sont toujours là.",
"draftDiscardFailed": "Impossible de supprimer le brouillon. Veuillez réessayer.",
"draftOpenFailed": "Impossible d'ouvrir le brouillon. Veuillez réessayer."
Expand Down
1 change: 1 addition & 0 deletions src/i18n/locales/it.json
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@
"forwardAttachmentFailed": "Impossibile preparare un allegato per l'inoltro. Riprova.",
"internxtKeyMissing": "Impossibile trovare le chiavi di cifratura per un destinatario Internxt. Riprova più tardi.",
"encryptionUnavailable": "La crittografia non è ancora pronta. Riprova tra un momento.",
"bccNotSupportedEncrypted": "Ccn non è ancora disponibile per le email crittografate. Sposta il destinatario in A o Cc.",
"draftSaveFailed": "Impossibile salvare la bozza. Le tue modifiche sono ancora qui.",
"draftDiscardFailed": "Impossibile eliminare la bozza. Riprova.",
"draftOpenFailed": "Impossibile aprire la bozza. Riprova."
Expand Down
Loading
Loading