fix(telegram): normalize relay feed contracts#2753
fix(telegram): normalize relay feed contracts#2753shinzoxD wants to merge 2 commits intokoala73:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the World Monitor Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Normalizes Telegram relay payload variants so both the edge /api/telegram-feed endpoint (browser UI contract) and the server intelligence listTelegramFeed handler (public API contract) behave consistently when the relay returns either messages[] or items[], and ensures count is derived from normalized items.
Changes:
- Added typed normalization in
api/telegram-feed.js(field coercion, http(s) URL validation, non-null ISOts, and authoritativecount). - Updated
server/worldmonitor/intelligence/v1/list-telegram-feed.tsto normalize relay field variants and computecountfrom mapped messages. - Added regression tests covering both edge handler normalization and server handler normalization.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
api/telegram-feed.js |
Normalizes relay feed responses into the browser Telegram UI contract and adjusts cache TTL based on normalized count. |
server/worldmonitor/intelligence/v1/list-telegram-feed.ts |
Normalizes relay variants into the intelligence API contract and stops trusting relay-provided count. |
tests/telegram-feed-contract.test.mjs |
Adds focused tests to prevent contract drift across edge handler + server handler normalization. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
api/telegram-feed.js
Outdated
| return new Date(value > 1e12 ? value : value * 1000).toISOString(); | ||
| } | ||
| const raw = toString(value).trim(); | ||
| if (!raw) return EPOCH_ISO; | ||
| const numeric = Number(raw); | ||
| if (Number.isFinite(numeric) && numeric > 0) { | ||
| return new Date(numeric > 1e12 ? numeric : numeric * 1000).toISOString(); |
There was a problem hiding this comment.
toIsoTimestamp treats numeric values as milliseconds only when value > 1e12. This misclassifies an exact 1e12 ms timestamp (2001-09-09) as seconds and produces an incorrect far-future ISO string. Consider using >= 1e12 (and the same for the string-numeric branch) so boundary values are interpreted consistently as milliseconds.
| return new Date(value > 1e12 ? value : value * 1000).toISOString(); | |
| } | |
| const raw = toString(value).trim(); | |
| if (!raw) return EPOCH_ISO; | |
| const numeric = Number(raw); | |
| if (Number.isFinite(numeric) && numeric > 0) { | |
| return new Date(numeric > 1e12 ? numeric : numeric * 1000).toISOString(); | |
| return new Date(value >= 1e12 ? value : value * 1000).toISOString(); | |
| } | |
| const raw = toString(value).trim(); | |
| if (!raw) return EPOCH_ISO; | |
| const numeric = Number(raw); | |
| if (Number.isFinite(numeric) && numeric > 0) { | |
| return new Date(numeric >= 1e12 ? numeric : numeric * 1000).toISOString(); |
| channelName: toText(message.channelName || message.channelTitle || message.channel), | ||
| text: toText(message.text), | ||
| timestampMs: toTimestampMs(message.timestampMs ?? message.timestamp ?? message.ts), | ||
| mediaUrls: Array.isArray(message.mediaUrls) ? message.mediaUrls.map(String) : [], |
There was a problem hiding this comment.
toTimestampMs returns numeric values as-is, so if the relay provides timestamps in seconds (common for ts), timestampMs will be off by 1000x. Since this change now explicitly accepts timestampMs ?? timestamp ?? ts, it would be safer to detect second-based epochs (e.g., < 1e12) and multiply by 1000, similar to the edge handler’s normalization logic.
Greptile SummaryThis PR normalizes the
Confidence Score: 2/5Not safe to merge — the server handler has a silent timestamp data-corruption bug and an unsanitized mediaUrls XSS gap. The edge handler is solid and the test suite is a genuine improvement, but two P1 issues in server/worldmonitor/intelligence/v1/list-telegram-feed.ts requires fixes to Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant Edge as api/telegram-feed.js (Edge)
participant RPC as listTelegramFeed (Server RPC)
participant Relay as Relay Service
Browser->>Edge: GET /api/telegram-feed?limit=50
Edge->>Relay: GET /telegram/feed?limit=50
Relay-->>Edge: messages[]|items[], count (stale)
Edge->>Edge: normalizeTelegramFeed()<br/>toIsoTimestamp (s→ms ✓)<br/>toHttpUrl (strips javascript: ✓)
Edge-->>Browser: items[], count=items.length
Browser->>RPC: ListTelegramFeed RPC
RPC->>Relay: GET /telegram/feed?limit=N
Relay-->>RPC: messages[]|items[], count (stale)
RPC->>RPC: toTimestampMs (no s→ms ⚠️)<br/>.map(String) on mediaUrls (no URL filter ⚠️)
RPC-->>Browser: messages[], count=messages.length
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| channelName: toText(message.channelName || message.channelTitle || message.channel), | ||
| text: toText(message.text), | ||
| timestampMs: toTimestampMs(message.timestampMs ?? message.timestamp ?? message.ts), | ||
| mediaUrls: Array.isArray(message.mediaUrls) ? message.mediaUrls.map(String) : [], |
There was a problem hiding this comment.
mediaUrls not sanitized against non-http(s) schemes
The server handler converts mediaUrls with only .map(String), so a relay-supplied javascript:evil() URL passes through unchanged into the response. The edge function in api/telegram-feed.js guards against this explicitly by using toStringArray(message.mediaUrls, toHttpUrl), which strips any URL whose protocol is not http: or https:.
If downstream clients render these URLs in <img src>, <a href>, or similar attributes, the unfiltered values are a XSS vector. Apply the same scheme allowlist here:
| mediaUrls: Array.isArray(message.mediaUrls) ? message.mediaUrls.map(String) : [], | |
| mediaUrls: Array.isArray(message.mediaUrls) ? message.mediaUrls.map(String).filter(u => { try { const p = new URL(u); return p.protocol === 'http:' || p.protocol === 'https:'; } catch { return false; } }) : [], |
(Or, better, extract the toHttpUrl helper into a shared module and reuse it in both handlers.)
| timestamp?: string | number; | ||
| timestampMs?: string | number; | ||
| ts?: string | number; | ||
| mediaUrls?: string[]; |
There was a problem hiding this comment.
mediaUrls typed as string[] but relay sends mixed types
The TelegramRelayMessage interface declares mediaUrls?: string[], yet the PR's own test fixture sends mediaUrls: [91, 'https://cdn.example.com/chart.png'] — a mixed number | string array. The code handles this correctly at runtime via .map(String), but the interface type is misleading and will cause TypeScript to suppress the coercion as unnecessary.
The edge function's RawTelegramMessage correctly models this as mediaUrls?: unknown[]. Matching that here makes the intent explicit:
| mediaUrls?: string[]; | |
| mediaUrls?: unknown[]; |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aeef13cc30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const normalized = normalizeTelegramFeed(parsed); | ||
| if (normalized.count === 0) { | ||
| cacheControl = 'public, max-age=0, s-maxage=15, stale-while-revalidate=10'; | ||
| } | ||
| return buildRelayResponse(response, JSON.stringify(normalized), { |
There was a problem hiding this comment.
This branch normalizes and rewrites any JSON response body before checking response.ok, so a relay 4xx/5xx like {"error":"rate_limited"} is returned with the same HTTP status but with a feed-shaped body (count/items) and no error details. That regresses error semantics for clients that inspect response JSON for actionable diagnostics/backoff reasons, and it makes failures look like an empty feed payload instead of a real upstream error.
Useful? React with 👍 / 👎.
Summary
/api/telegram-feedinto the browser Telegram UI contract whether the relay returnsmessages[]oritems[]listTelegramFeedWhat changed
// @ts-checkand typed normalizers inapi/telegram-feed.jstimestampMs/timestamp/ts,sourceUrl/url,channelTitle/channelName/channel)tsnon-null with a safe ISO fallback so the browser panel never receivesnullhttp(s)only and coercetags/mediaUrlsto stringsserver/worldmonitor/intelligence/v1/list-telegram-feed.tsto normalize the same relay variants and stop trusting stale relaycountVerification
node --import tsx --test tests/telegram-feed-contract.test.mjsnpm run typecheck:apinode_modules/@biomejs/biome/bin/biome check api/telegram-feed.js server/worldmonitor/intelligence/v1/list-telegram-feed.ts tests/telegram-feed-contract.test.mjsCloses #2593.