fix(brief): address 2 remaining P2 review comments on PR #3157

Greptile review (2026-04-18T05:04Z) flagged three P2 items. The
first (shouldExitNonZero never wired into cron) was already fixed in
commit 35a46aa34. This commit addresses the other two.

1. composeBriefForRule: issuedAt used Date.now() instead of the
   caller-supplied nowMs. Under the digest cron the delta is
   milliseconds and harmless, but it broke the function's
   determinism contract — same input must produce same output for
   tests + retries. Now uses the passed nowMs.

2. buildChannelBodies: magazineUrl embedded raw inside Telegram HTML
   <a href="..."> and Slack <URL|text> syntax. The URL is HMAC-
   signed and shape-validated upstream (userId regex + YYYY-MM-DD
   date), so injection is practically impossible — but the email
   CTA (injectBriefCta) escapes per-target and channel footers
   should match that discipline. Added:
     - Telegram: escape &, <, >, " to HTML entities
     - Slack: strip <, >, | (mrkdwn metacharacters)
   Discord and plain-text paths unchanged — Discord links tolerate
   raw URLs, plain text has no metacharacters to escape.

Tests: 98/98 still pass (deterministic issuedAt change was
transparent to existing assertions because tests already pass nowMs
explicitly via the issuedAt fixture field).
This commit is contained in:
Elie Habib
2026-04-18 12:21:36 +04:00
parent f1d209ea59
commit 7f474b2700
2 changed files with 20 additions and 3 deletions

View File

@@ -172,7 +172,10 @@ export function composeBriefForRule(rule, insights, { nowMs = Date.now() } = {})
dateLong: dateLongFromIso(issueDate), dateLong: dateLongFromIso(issueDate),
issue: issueCodeFromIso(issueDate), issue: issueCodeFromIso(issueDate),
insightsNumbers: insights.numbers, insightsNumbers: insights.numbers,
issuedAt: Date.now(), // Same nowMs as the rest of the envelope so the function stays
// deterministic for a given input — tests + retries see identical
// output.
issuedAt: nowMs,
localHour: localHourInTz(nowMs, tz), localHour: localHourInTz(nowMs, tz),
}); });
} }

View File

@@ -820,14 +820,28 @@ const DIVIDER = '─'.repeat(40);
* complexity stays within the lint budget. * complexity stays within the lint budget.
*/ */
function buildChannelBodies(storyListPlain, aiSummary, magazineUrl) { function buildChannelBodies(storyListPlain, aiSummary, magazineUrl) {
// The URL is already HMAC-signed and shape-validated at sign time
// (userId regex + YYYY-MM-DD), but we still escape it per-target
// as defence-in-depth — same discipline injectBriefCta uses for
// the email button. Each target has different metacharacter rules.
const telegramSafeUrl = magazineUrl
? String(magazineUrl)
.replace(/&/g, '&amp;')
.replace(/</g, '&lt;')
.replace(/>/g, '&gt;')
.replace(/"/g, '&quot;')
: '';
const slackSafeUrl = magazineUrl
? String(magazineUrl).replace(/[<>|]/g, '')
: '';
const briefFooterPlain = magazineUrl const briefFooterPlain = magazineUrl
? `\n\n${DIVIDER}\n\n📖 Open your WorldMonitor Brief magazine:\n${magazineUrl}` ? `\n\n${DIVIDER}\n\n📖 Open your WorldMonitor Brief magazine:\n${magazineUrl}`
: ''; : '';
const briefFooterTelegram = magazineUrl const briefFooterTelegram = magazineUrl
? `\n\n${DIVIDER}\n\n📖 <a href="${magazineUrl}">Open your WorldMonitor Brief magazine</a>` ? `\n\n${DIVIDER}\n\n📖 <a href="${telegramSafeUrl}">Open your WorldMonitor Brief magazine</a>`
: ''; : '';
const briefFooterSlack = magazineUrl const briefFooterSlack = magazineUrl
? `\n\n${DIVIDER}\n\n📖 <${magazineUrl}|Open your WorldMonitor Brief magazine>` ? `\n\n${DIVIDER}\n\n📖 <${slackSafeUrl}|Open your WorldMonitor Brief magazine>`
: ''; : '';
const briefFooterDiscord = magazineUrl const briefFooterDiscord = magazineUrl
? `\n\n${DIVIDER}\n\n📖 [Open your WorldMonitor Brief magazine](${magazineUrl})` ? `\n\n${DIVIDER}\n\n📖 [Open your WorldMonitor Brief magazine](${magazineUrl})`