From cc4533d554cf221e8d519450cd83fcc4121aaaeb Mon Sep 17 00:00:00 2001 From: Marco Beretta <81851188+berry-13@users.noreply.github.com> Date: Sun, 5 Jul 2026 03:11:42 +0200 Subject: [PATCH] fix: apply forced retention before creating a share MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The share route enforced retention after createSharedLink and the permission grant. If the cascade threw at that point, the client got a 500 with a live share already created, and a retry never reached the cascade again because createSharedLink rejects when an active share exists — leaving a valid share whose source conversation stays non-temporary and non-expiring under ephemeral mode. Convert the source conversation first: a failed share attempt (or an existing-share retry) still converts the touched chat, and the share expiration resolution now reads the converted conversation's deadline. --- api/server/routes/__tests__/share.spec.js | 16 ++++++++++++++-- api/server/routes/share.js | 17 ++++++++++++----- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/api/server/routes/__tests__/share.spec.js b/api/server/routes/__tests__/share.spec.js index 10d5ddaeba..3c8b976d36 100644 --- a/api/server/routes/__tests__/share.spec.js +++ b/api/server/routes/__tests__/share.spec.js @@ -374,7 +374,7 @@ describe('share routes', () => { ); }); - it('does not convert the source conversation when the share is not created', async () => { + it('converts the source conversation before creating the share so retries stay covered', async () => { mockGetSharedLinkExpiration.mockResolvedValue(activeExpiration); createSharedLink.mockResolvedValue(null); @@ -383,7 +383,19 @@ describe('share routes', () => { .send({ targetMessageId: 'msg-123' }); expect(response.status).toBe(404); - expect(applyForcedRetention).not.toHaveBeenCalled(); + /** + * Retention runs before createSharedLink: a share attempt that fails (or hits an existing + * active share on retry) must still convert the touched conversation, otherwise a live + * share could outlast a source chat that never converts. + */ + expect(applyForcedRetention).toHaveBeenCalledWith( + { userId: 'user-123', interfaceConfig: { retentionMode: RetentionMode.EPHEMERAL } }, + { conversationId: 'convo-123' }, + expect.objectContaining({ context: expect.any(String) }), + ); + expect(applyForcedRetention.mock.invocationCallOrder[0]).toBeLessThan( + createSharedLink.mock.invocationCallOrder[0], + ); }); it('converts the source conversation under forced retention when updating a share', async () => { diff --git a/api/server/routes/share.js b/api/server/routes/share.js index c023988189..5177d1cad2 100644 --- a/api/server/routes/share.js +++ b/api/server/routes/share.js @@ -467,6 +467,18 @@ router.post( async (req, res) => { try { const { targetMessageId } = req.body; + /** + * Convert the source conversation before creating the link. createSharedLink rejects + * when an active share already exists, so a retention failure after creation would + * leave a live share whose source chat never converts — no retry could reach the + * cascade again. Converting first also lets the share expiration below read the + * converted conversation's deadline. + */ + await enforceForcedRetention( + req, + req.params.conversationId, + 'POST /api/share/:conversationId', + ); const expiredAt = await resolveSharedLinkExpiration(req, req.params.conversationId); if (expiredAt != null && !isActiveExpirationDate(expiredAt)) { return res.status(404).end(); @@ -488,11 +500,6 @@ router.post( ); if (created) { await grantCreationPermissions(created._id, req.user.id, grantPublic, expiredAt); - await enforceForcedRetention( - req, - req.params.conversationId, - 'POST /api/share/:conversationId', - ); res.status(200).json(created); } else { res.status(404).end();