Skip to content

Commit 7a13116

Browse files
fuleinistfuleinist
committed
fix(relay): prevent unbounded recursion in sendTelegram on sustained 429
The 429 handler in sendTelegram() called itself unconditionally with no retry counter. Under sustained Telegram rate limiting, this could recurse indefinitely and crash the Railway relay process. Changes: - Add _retryCount parameter (default 0) to sendTelegram() - Add guard: if (_retryCount ?? 0) >= 1, log and return false immediately - Pass (_retryCount ?? 0) + 1 on the recursive 429 call (single retry only) - Non-429 paths (200, 400, 403, 401) are unaffected Testing: - Add regression test (static analysis of source, matching repo patterns) - All 4 new assertions pass: param signature, guard + return false, incremented recursive call, single call site - All 27 existing relay tests continue to pass Fixes #3060
1 parent 3741676 commit 7a13116

2 files changed

Lines changed: 129 additions & 2 deletions

File tree

‎scripts/notification-relay.cjs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ async function processFlushQuietHeld(event) {
272272

273273
// ── Delivery: Telegram ────────────────────────────────────────────────────────
274274

275-
async function sendTelegram(userId, chatId, text) {
275+
async function sendTelegram(userId, chatId, text, _retryCount = 0) {
276276
if (!TELEGRAM_BOT_TOKEN) {
277277
console.warn('[relay] Telegram: TELEGRAM_BOT_TOKEN not set — skipping');
278278
return false;
@@ -293,10 +293,14 @@ async function sendTelegram(userId, chatId, text) {
293293
return false;
294294
}
295295
if (res.status === 429) {
296+
if ((_retryCount ?? 0) >= 1) {
297+
console.warn(`[relay] Telegram 429 retry limit reached for ${userId} — bailing`);
298+
return false;
299+
}
296300
const body = await res.json().catch(() => ({}));
297301
const wait = ((body.parameters?.retry_after ?? 5) + 1) * 1000;
298302
await new Promise(r => setTimeout(r, wait));
299-
return sendTelegram(userId, chatId, text); // single retry
303+
return sendTelegram(userId, chatId, text, (_retryCount ?? 0) + 1); // single retry
300304
}
301305
if (res.status === 401) {
302306
console.error('[relay] Telegram 401 Unauthorized — TELEGRAM_BOT_TOKEN is invalid or belongs to a different bot; correct the Railway env var to restore Telegram delivery');

‎tests/notification-relay-telegram-retry.test.mjs‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/**
2+
* Regression test: scripts/notification-relay.cjs sendTelegram() must NOT
3+
* recurse infinitely on sustained 429 responses.
4+
*
5+
* Before the fix, the 429 handler called sendTelegram() unconditionally with
6+
* no retry counter, creating unbounded recursion during sustained rate limiting.
7+
* This could stack-overflow the Railway relay process.
8+
*
9+
* The fix adds a `_retryCount` parameter (default 0) and bails after one retry.
10+
*
11+
* Run: node --test tests/notification-relay-telegram-retry.test.mjs
12+
*/
13+
14+
import { describe, it } from 'node:test';
15+
import assert from 'node:assert/strict';
16+
import { readFileSync } from 'node:fs';
17+
import { resolve, dirname } from 'node:path';
18+
import { fileURLToPath } from 'node:url';
19+
20+
const __dirname = dirname(fileURLToPath(import.meta.url));
21+
const relaySrc = readFileSync(
22+
resolve(__dirname, '..', 'scripts', 'notification-relay.cjs'),
23+
'utf-8',
24+
);
25+
26+
function extractSendTelegram(src) {
27+
const idx = src.indexOf('async function sendTelegram(');
28+
assert.ok(idx !== -1, 'sendTelegram not found in notification-relay.cjs');
29+
const openIdx = src.indexOf('{', idx);
30+
let depth = 1;
31+
let i = openIdx + 1;
32+
while (i < src.length && depth > 0) {
33+
if (src[i] === '{') depth++;
34+
else if (src[i] === '}') depth--;
35+
i++;
36+
}
37+
return src.slice(idx, i);
38+
}
39+
40+
describe('notification-relay sendTelegram retry discipline', () => {
41+
const fn = extractSendTelegram(relaySrc);
42+
43+
it('has a _retryCount parameter defaulting to 0', () => {
44+
assert.match(
45+
fn,
46+
/async function sendTelegram\s*\(\s*\w+,\s*\w+,\s*\w+,\s*_retryCount\s*=\s*0\s*\)/,
47+
'sendTelegram must accept _retryCount = 0 parameter',
48+
);
49+
});
50+
51+
it('bails and returns false when _retryCount >= 1 (no infinite recursion)', () => {
52+
// There must be a guard that returns false when retry count is exceeded.
53+
// The guard code is: if ((_retryCount ?? 0) >= 1) { console.warn(...); return false; }
54+
assert.match(
55+
fn,
56+
/_retryCount.*?\)\s*>=\s*1/,
57+
'sendTelegram must guard against _retryCount >= 1 to prevent infinite recursion',
58+
);
59+
60+
// The guard must return false (not recurse again)
61+
assert.ok(
62+
/if\s*\([^)]*_retryCount[^)]*\)\s*>=\s*1/.test(fn),
63+
'guard if-statement with _retryCount >= 1 check not found',
64+
);
65+
assert.ok(
66+
fn.includes('return false'),
67+
'guard must return false when retry limit is exceeded',
68+
);
69+
70+
// The return false must be INSIDE the guard if-block, not somewhere else
71+
const guardBlockIdx = fn.indexOf('if ((_retryCount ?? 0) >= 1)');
72+
assert.ok(guardBlockIdx !== -1, 'guard block not found');
73+
const afterGuard = fn.slice(guardBlockIdx);
74+
const returnIdx = afterGuard.indexOf('return false');
75+
76+
// Find the matching closing brace of the guard block (not the first '}')
77+
const openBraceIdx = afterGuard.indexOf('{');
78+
let braceDepth = 0;
79+
let closingBraceIdx = -1;
80+
for (let j = openBraceIdx; j < afterGuard.length; j++) {
81+
if (afterGuard[j] === '{') braceDepth++;
82+
else if (afterGuard[j] === '}') {
83+
braceDepth--;
84+
if (braceDepth === 0) { closingBraceIdx = j; break; }
85+
}
86+
}
87+
88+
assert.ok(
89+
returnIdx !== -1 && returnIdx < closingBraceIdx,
90+
'return false must appear before the closing brace of the guard block',
91+
);
92+
});
93+
94+
it('passes incremented retry count on the recursive 429 call', () => {
95+
// The recursive call inside the 429 block must pass an incremented retry count.
96+
// Since regex can't easily match nested parens, we verify:
97+
// 1. sendTelegram is called recursively (the only call site in the 429 block)
98+
// 2. _retryCount is passed as an argument
99+
// 3. + 1 is present in the function (the increment)
100+
const recursiveCallIdx = fn.indexOf('return sendTelegram(');
101+
assert.ok(recursiveCallIdx !== -1, 'recursive sendTelegram call not found');
102+
const afterCall = fn.slice(recursiveCallIdx);
103+
assert.ok(
104+
/_retryCount/.test(afterCall),
105+
'recursive call must pass _retryCount parameter',
106+
);
107+
assert.ok(
108+
/\+\s*1/.test(afterCall),
109+
'recursive call must increment retry count with + 1',
110+
);
111+
});
112+
113+
it('does not recurse for any status code other than 429', () => {
114+
// Count all sendTelegram( calls inside the function body.
115+
// There should be exactly 1: the recursive one in the 429 handler.
116+
const bodyOnly = fn.slice(fn.indexOf('{') + 1, fn.lastIndexOf('}'));
117+
const allCalls = bodyOnly.match(/sendTelegram\s*\(/g) || [];
118+
assert.equal(
119+
allCalls.length, 1,
120+
`sendTelegram body should contain exactly 1 self-call (the 429 retry); found ${allCalls.length}`,
121+
);
122+
});
123+
});

0 commit comments

Comments
 (0)