openvibers/openvibe.chat Commits

Answer 400 for non-positive-integer ids on emotes and other param routes (#4)

OP OpenVibers committed 2 Oct 2026 · e5b291cea0 · 8 files, +164 −12

checks run by GitHub CI (test) · CI ✓ 3/3 · review SHIP by DeepSeek harness

https://github.com/OpenVibers/OpenVibe.Chat/pull/4

Changes

ddiff --git a/server/chat/channel-mod-routes.js b/server/chat/channel-mod-routes.js
iindex fc0fdde..9d9050a 100644
---- a/server/chat/channel-mod-routes.js
++++ b/server/chat/channel-mod-routes.js
@@@ -24,6 +24,7 @@ const db = require('../db/database');
const ctx = require('../live-context');
const { requireAuth } = require('../auth/auth');
const permissions = require('../auth/permissions');
++const { positiveInt } = require('./route-params');
const router = express.Router();
@@@ -128,7 +129,8 @@ router.post('/:channelId/mods', requireAuth, async (req, res) => {
router.delete('/:channelId/mods/:userId', requireAuth, async (req, res) => {
try {
const channelId = parseInt(req.params.channelId);
-- const userId = parseInt(req.params.userId);
++ const userId = positiveInt(req.params.userId);
++ if (userId === null) return res.status(400).json({ error: 'invalid user id' });
const channel = await ctx.getChannelById(channelId);
if (!channel) return res.status(404).json({ error: 'Channel not found' });
if (!canAssignMods(req.user, channel)) {
@@@ -266,7 +268,8 @@ router.get('/:channelId/moderation/chat-search', requireAuth, requireChannelAcce
// ── Delete a channel chat message ────────────────────────────
router.post('/:channelId/moderation/messages/:messageId/delete', requireAuth, requireChannelAccess, async (req, res) => {
try {
-- const messageId = parseInt(req.params.messageId, 10);
++ const messageId = positiveInt(req.params.messageId);
++ if (messageId === null) return res.status(400).json({ error: 'invalid message id' });
const message = await db.getChatMessageById(messageId);
if (!message) return res.status(404).json({ error: 'Message not found' });
ddiff --git a/server/chat/dm-routes.js b/server/chat/dm-routes.js
iindex 46abafa..b5375b7 100644
---- a/server/chat/dm-routes.js
++++ b/server/chat/dm-routes.js
@@@ -20,6 +20,7 @@ const { limits } = require('../net/actor-limits');
const dm = require('./dm');
const ctx = require('../live-context');
const prefStores = require('../prefs/stores');
++const { idParam } = require('./route-params');
/**
* The target's chat.dm_settings (Network user module): does this person accept a new direct
@@@ -39,6 +40,11 @@ function publicMessage(m) {
// All DM routes require authentication
router.use(requireAuth);
++// Ids are validated before the handler (and its queries) see them, one rule per param name.
++router.param('id', idParam('invalid conversation id'));
++router.param('userId', idParam('invalid user id'));
++router.param('msgId', idParam('invalid message id'));
++
// Per-actor limits (net/actor-limits.js), after requireAuth: reads take the defaults at app.js; each
// write below has its own number. The per-person DM limits further down (10 messages a minute, 5 new
// conversations an hour) keep deciding what is sent; these cap the requests, refused ones included.
ddiff --git a/server/chat/emotes-routes.js b/server/chat/emotes-routes.js
iindex ac7341f..49cb0c9 100644
---- a/server/chat/emotes-routes.js
++++ b/server/chat/emotes-routes.js
@@@ -33,6 +33,7 @@ const config = require('../config');
const media = require('../media/client');
const { requireAuth, optionalAuth } = require('../auth/auth');
const permissions = require('../auth/permissions');
++const { positiveInt } = require('./route-params');
const router = express.Router();
@@@ -210,7 +211,8 @@ router.get('/global', async (req, res) => {
router.get('/channel/:userId', async (req, res) => {
try {
-- const userId = parseInt(req.params.userId);
++ const userId = positiveInt(req.params.userId);
++ if (userId === null) return res.status(400).json({ error: 'invalid user id' });
const emotes = (await db.getChannelEmotes(userId)).map((e) => ({
id: `custom-${e.id}`,
emote_id: e.id,
@@@ -361,7 +363,9 @@ router.use((err, req, res, next) => {
router.patch('/:id', requireAuth, async (req, res) => {
try {
-- const emote = await db.getEmoteById(req.params.id);
++ const id = positiveInt(req.params.id);
++ if (id === null) return res.status(400).json({ error: 'invalid emote id' });
++ const emote = await db.getEmoteById(id);
if (!emote) return res.status(404).json({ error: 'Emote not found' });
let allowed = emote.user_id === req.user.id || permissions.can(req.user, 'staff.assets.manage');
if (!allowed) {
@@@ -413,7 +417,9 @@ router.patch('/:id', requireAuth, async (req, res) => {
router.delete('/:id', requireAuth, async (req, res) => {
try {
-- const emote = await db.getEmoteById(req.params.id);
++ const id = positiveInt(req.params.id);
++ if (id === null) return res.status(400).json({ error: 'invalid emote id' });
++ const emote = await db.getEmoteById(id);
if (!emote) return res.status(404).json({ error: 'Emote not found' });
let allowed = emote.user_id === req.user.id || permissions.can(req.user, 'staff.assets.manage');
if (!allowed) {
ddiff --git a/server/chat/route-params.js b/server/chat/route-params.js
nnew file mode 100644
iindex 0000000..4e95888
---- /dev/null
++++ b/server/chat/route-params.js
@@@ -0,0 +1,25 @@
++'use strict';
++/**
++ * Route ids. PostgreSQL rejects a NaN / zero / negative integer parameter where SQLite matched no
++ * rows, so after the engine move an unvalidated parseInt turned a mistyped URL (e.g.
++ * /api/emotes/channel/abc) into a 500. Every :userId/:id route validates its param first and
++ * answers 400 "invalid … id" instead.
++ */
++
++/** A route param that must be a positive integer; null when it is not one. */
++function positiveInt(value) {
++ const s = typeof value === 'string' ? value : String(value == null ? '' : value);
++ if (!/^\d+$/.test(s)) return null;
++ const n = Number(s);
++ return Number.isSafeInteger(n) && n > 0 ? n : null;
++}
++
++/** Express router.param handler: 400 for a non-positive-integer param, else next(). */
++function idParam(message) {
++ return (req, res, next, value) => {
++ if (positiveInt(value) === null) return res.status(400).json({ error: message });
++ next();
++ };
++}
++
++module.exports = { positiveInt, idParam };
ddiff --git a/server/chat/routes.js b/server/chat/routes.js
iindex ee6004f..041fb18 100644
---- a/server/chat/routes.js
++++ b/server/chat/routes.js
@@@ -20,6 +20,7 @@ const permissions = require('../auth/permissions');
const historyStore = require('./history-store');
const networkBlocks = require('./network-blocks');
const { limits } = require('../net/actor-limits');
++const { positiveInt } = require('./route-params');
/**
* A signed-in reader does not get the lines of people they blocked on the network (one-way; only
@@@ -411,7 +412,8 @@ router.get('/search', requireAuth, searchLimit, async (req, res) => {
// ── User Chat History ────────────────────────────────────────
router.get('/user/:userId/history', requireAuth, async (req, res) => {
try {
-- const userId = parseInt(req.params.userId);
++ const userId = positiveInt(req.params.userId);
++ if (userId === null) return res.status(400).json({ error: 'invalid user id' });
const limit = Math.min(parseInt(req.query.limit || '50'), 200);
const offset = parseInt(req.query.offset || '0');
@@@ -550,12 +552,14 @@ router.get('/:streamId/replay', optionalAuth, async (req, res) => {
// Pass ?scope=stream to restrict to the single session (legacy behavior).
router.get('/:streamId/history', optionalAuth, async (req, res) => {
try {
++ const streamId = positiveInt(req.params.streamId);
++ if (streamId === null) return res.status(400).json({ error: 'Invalid stream ID' });
const limit = Math.min(parseInt(req.query.limit || '500'), 500);
const before = req.query.before; // ISO timestamp for pagination
const scope = req.query.scope || 'streamer';
// Resolve which broadcaster this stream belongs to.
-- const stream = await ctx.ensureStream(req.params.streamId);
++ const stream = await ctx.ensureStream(streamId);
const broadcasterId = stream ? stream.user_id : null;
const spanStreamer = scope !== 'stream' && broadcasterId;
@@@ -593,7 +597,7 @@ router.get('/:streamId/history', optionalAuth, async (req, res) => {
LEFT JOIN ctx_users cu ON cm.channel_user_id = cu.id
WHERE cm.stream_id = ? AND cm.is_deleted = 0
AND (cm.auto_delete_at IS NULL OR datetime(cm.auto_delete_at) > ov_now())`;
-- params.push(req.params.streamId);
++ params.push(streamId);
}
// after_id: cursor read (oldest→newest, primary-key range scan) for reopen/reconnect.
@@@ -638,10 +642,10 @@ router.get('/:streamId/history', optionalAuth, async (req, res) => {
} catch { /* non-critical */ }
}
// A cursor read also names the rows at or under the cursor deleted since (history-store.js).
-- const deleted = afterId != null ? { deleted_ids: await historyStore.deletedIds(spanStreamer ? `channel:${broadcasterId}` : `stream:${parseInt(req.params.streamId, 10) || 0}`, afterId) } : {};
++ const deleted = afterId != null ? { deleted_ids: await historyStore.deletedIds(spanStreamer ? `channel:${broadcasterId}` : `stream:${streamId}`, afterId) } : {};
res.json({
messages, latest_id, complete, ...deleted, liveSlots, channel,
-- activeStreamId: parseInt(req.params.streamId) || null,
++ activeStreamId: streamId,
activeManagedId: stream ? (stream.managed_stream_id || null) : null,
});
} catch (err) {
ddiff --git a/server/chat/sounds-routes.js b/server/chat/sounds-routes.js
iindex 3d9b313..11768be 100644
---- a/server/chat/sounds-routes.js
++++ b/server/chat/sounds-routes.js
@@@ -24,6 +24,7 @@ const ctx = require('../live-context');
const { requireAuth } = require('../auth/auth');
const { limits } = require('../net/actor-limits');
const permissions = require('../auth/permissions');
++const { positiveInt } = require('./route-params');
// Per-actor limits (net/actor-limits.js), after requireAuth and before the upload is read: reads take
// the defaults at app.js (sound files are left to the per-address limit). An upload is up to
@@@ -314,7 +315,9 @@ router.use((err, req, res, next) => {
// ── Delete a sound ───────────────────────────────────────────
router.delete('/:id', requireAuth, editLimit, async (req, res) => {
try {
-- const sound = await db.getChannelSoundById(parseInt(req.params.id));
++ const soundId = positiveInt(req.params.id);
++ if (soundId === null) return res.status(400).json({ error: 'invalid sound id' });
++ const sound = await db.getChannelSoundById(soundId);
if (!sound) return res.status(404).json({ error: 'Sound not found' });
let allowed = sound.created_by === req.user.id || permissions.can(req.user, 'staff.assets.manage');
if (!allowed) {
ddiff --git a/server/chat/tts-routes.js b/server/chat/tts-routes.js
iindex 565460c..b562276 100644
---- a/server/chat/tts-routes.js
++++ b/server/chat/tts-routes.js
@@@ -33,6 +33,7 @@ const permissions = require('../auth/permissions');
const { isOwner } = permissions;
const ttsEngine = require('./tts-engine');
const audioQueue = require('./audio-queue');
++const { positiveInt } = require('./route-params');
const db = require('../db/database');
const ctx = require('../live-context');
const config = require('../config');
@@@ -253,11 +254,13 @@ router.post('/queue/clear', requireAuth, queueModerate, async (req, res) => {
router.post('/queue/:id/report', requireAuth, queueReport, async (req, res) => {
try {
++ const requestId = positiveInt(req.params.id);
++ if (requestId === null) return res.status(400).json({ error: 'invalid request id' });
const r = await queueRoom(req, res);
if (!r) return;
const state = String((req.body && req.body.state) || '');
if (state !== 'played' && state !== 'failed') return res.status(400).json({ error: "state must be 'played' or 'failed'" });
-- const ok = await audioQueue.report(r.room, parseInt(req.params.id, 10), state, { error: req.body && req.body.error, actor: `user:${req.user.username}` });
++ const ok = await audioQueue.report(r.room, requestId, state, { error: req.body && req.body.error, actor: `user:${req.user.username}` });
if (!ok) return res.status(409).json({ error: 'That request is not playing' });
res.json({ ok: true, queue: await audioQueue.list(r.room) });
} catch (e) { res.status(500).json({ error: e.message }); }
ddiff --git a/test/route-id-validation.test.js b/test/route-id-validation.test.js
nnew file mode 100644
iindex 0000000..909241b
---- /dev/null
++++ b/test/route-id-validation.test.js
@@@ -0,0 +1,102 @@
++'use strict';
++/**
++ * Route ids are validated before they reach a query. PostgreSQL rejects a NaN / zero / negative
++ * integer parameter where SQLite matched no rows, so after the engine move a mistyped
++ * /api/emotes/channel/abc answered 500 ("Failed to load channel emotes") instead of the old empty
++ * list. Every :userId/:id route now answers 400 for anything that is not a positive integer, and
++ * the valid-id paths are unchanged.
++ */
++const assert = require('assert');
++const { boot, suite } = require('./helpers');
++
++const t = suite('route-id-validation');
++let h, admin, cid;
++
++const BAD = ['abc', '0', '-1', '1.5', '1e3'];
++
++t('boot', async () => {
++ h = await boot({});
++ admin = h.addUser('idval-admin', { role: 'admin' });
++ cid = h.addChannel(admin.id, { moderators: [] });
++ await h.ctx.sync();
++});
++
++t('emotes: /channel/:userId is 400 for a non-positive-integer', async () => {
++ for (const bad of BAD) {
++ const r = await h.http('GET', `/api/emotes/channel/${bad}`);
++ assert.strictEqual(r.status, 400, `${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid user id' });
++ }
++ const ok = await h.http('GET', '/api/emotes/channel/999999');
++ assert.strictEqual(ok.status, 200, ok.text);
++ assert.deepStrictEqual(ok.body.emotes, [], 'an id with no emotes is still an empty list');
++});
++
++t('emotes: PATCH/DELETE /:id are 400 for a non-positive-integer, 404 for a missing emote', async () => {
++ for (const bad of BAD) {
++ let r = await h.http('PATCH', `/api/emotes/${bad}`, { token: admin.token, body: { code: 'nope' } });
++ assert.strictEqual(r.status, 400, `PATCH ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid emote id' });
++ r = await h.http('DELETE', `/api/emotes/${bad}`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `DELETE ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid emote id' });
++ }
++ const gone = await h.http('PATCH', '/api/emotes/999999', { token: admin.token, body: { code: 'nope' } });
++ assert.strictEqual(gone.status, 404, gone.text);
++});
++
++t('sounds: DELETE /:id is 400 for a non-positive-integer', async () => {
++ for (const bad of BAD) {
++ const r = await h.http('DELETE', `/api/sounds/${bad}`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid sound id' });
++ }
++});
++
++t('chat history: bad :userId / :streamId are 400', async () => {
++ for (const bad of BAD) {
++ let r = await h.http('GET', `/api/chat/user/${bad}/history`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `user ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid user id' });
++
++ r = await h.http('GET', `/api/chat/${bad}/history`);
++ assert.strictEqual(r.status, 400, `stream ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'Invalid stream ID' });
++ }
++});
++
++t('dm: bad conversation :id and block :userId are 400', async () => {
++ for (const bad of BAD) {
++ let r = await h.http('GET', `/api/dm/conversations/${bad}`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `conversation ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid conversation id' });
++
++ r = await h.http('DELETE', `/api/dm/blocks/${bad}`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `block ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid user id' });
++ }
++ const gone = await h.http('GET', '/api/dm/conversations/999999', { token: admin.token });
++ assert.strictEqual(gone.status, 403, gone.text, 'a valid id with no membership stays 403');
++});
++
++t('channel moderation: bad :userId / :messageId are 400', async () => {
++ for (const bad of BAD) {
++ let r = await h.http('DELETE', `/api/chat/channels/${cid}/mods/${bad}`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `mod ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid user id' });
++
++ r = await h.http('POST', `/api/chat/channels/${cid}/moderation/messages/${bad}/delete`, { token: admin.token });
++ assert.strictEqual(r.status, 400, `message ${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid message id' });
++ }
++});
++
++t('tts queue report: bad :id is 400', async () => {
++ for (const bad of BAD) {
++ const r = await h.http('POST', `/api/tts/queue/${bad}/report`, { token: admin.token, body: { state: 'played', channel_user_id: admin.id } });
++ assert.strictEqual(r.status, 400, `${bad} -> ${r.status} ${r.text}`);
++ assert.deepStrictEqual(r.body, { error: 'invalid request id' });
++ }
++});
++
++t.run(async () => { if (h) await h.close(); });