fix: reject non-string tag and conversationId in forced-retention helpers

The bookmark-tag and conversation ids passed to the forced-retention
helpers come from untyped request bodies, so a crafted PUT /api/tags
body like {"tag": {"$gt": ""}} reached Conversation.find({ tags }) as a
query operator and matched every tagged conversation instead of one,
bulk-converting them under ephemeral retention (NoSQL operator
injection). The same applied to req.body.conversationId on POST.

Guard applyForcedRetention and applyForcedRetentionToTag to ignore any
non-string conversationId/messageId/tag, and pass a guaranteed string
from the tag rename route.
This commit is contained in:
Marco Beretta 2026-06-24 15:43:46 +02:00
parent e764c983cf
commit aae066dca6
No known key found for this signature in database
GPG key ID: D918033D8E74CC11
3 changed files with 38 additions and 2 deletions

View file

@ -98,7 +98,8 @@ router.put('/:tag', configMiddleware, async (req, res) => {
const decodedTag = decodeURIComponent(req.params.tag);
const tag = await updateConversationTag(req.user.id, decodedTag, req.body);
if (tag) {
await enforceForcedRetentionForTag(req, req.body?.tag || decodedTag, 'PUT /api/tags/:tag');
const renamedTag = typeof req.body?.tag === 'string' ? req.body.tag : decodedTag;
await enforceForcedRetentionForTag(req, renamedTag, 'PUT /api/tags/:tag');
res.status(200).json(tag);
} else {
res.status(404).json({ error: 'Tag not found' });

View file

@ -1244,6 +1244,29 @@ describe('Message Operations', () => {
const convo = await Conversation().findOne({ conversationId }).lean();
expect(convo?.expiredAt ?? null).toBeNull();
});
it('ignores a non-string tag instead of matching every conversation (NoSQL injection)', async () => {
const conversationId = uuidv4();
await Conversation().create({
conversationId,
user: 'user123',
endpoint: 'openAI',
tags: ['work'],
});
await applyForcedRetentionToTag(
{
userId: 'user123',
interfaceConfig: { temporaryChatRetention: 24, retentionMode: RetentionMode.EPHEMERAL },
},
{ tag: { $gt: '' } as unknown as string },
{ context: 'PUT /api/tags/:tag' },
);
const convo = await Conversation().findOne({ conversationId }).lean();
expect(convo?.isTemporary ?? null).not.toBe(true);
expect(convo?.expiredAt ?? null).toBeNull();
});
});
describe('Message cursor pagination', () => {

View file

@ -384,6 +384,12 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
if (!isForcedTemporaryRetention(interfaceConfig?.retentionMode)) {
return;
}
if (typeof conversationId !== 'string' || conversationId.length === 0) {
logger.warn(
`[applyForcedRetention] Ignoring non-string conversationId (context: ${metadata?.context ?? 'n/a'})`,
);
return;
}
let forcedExpiredAt: Date;
try {
@ -409,7 +415,7 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
);
}
if (messageId) {
if (typeof messageId === 'string' && messageId.length > 0) {
await Message.updateOne(
{ messageId, user: userId },
{ $set: { isTemporary: true, expiredAt: forcedExpiredAt } },
@ -440,6 +446,12 @@ export function createMessageMethods(mongoose: typeof import('mongoose')): Messa
if (!isForcedTemporaryRetention(interfaceConfig?.retentionMode)) {
return;
}
if (typeof tag !== 'string' || tag.length === 0) {
logger.warn(
`[applyForcedRetentionToTag] Ignoring non-string tag (context: ${metadata?.context ?? 'n/a'})`,
);
return;
}
let forcedExpiredAt: Date;
try {