mirror of
https://github.com/koala73/worldmonitor.git
synced 2026-04-25 17:14:57 +02:00
fix(security): strip importanceScore from /api/notify payload + scope fan-out by userId (#3143)
* fix(security): strip importanceScore from /api/notify payload + scope fan-out by userId Closes todo #196 (activation blocker for IMPORTANCE_SCORE_LIVE=1). Before this fix, any authenticated Pro user could POST to /api/notify with `payload.importanceScore: 100` and `severity: 'critical'`, bypassing the relay's IMPORTANCE_SCORE_MIN gate and fan-out would hit every Pro user with matching rules (no userId filter). This was a pre-existing vulnerability surfaced during the scoring pipeline work in PR #3069. Two changes: 1. api/notify.ts — strip `importanceScore` and `corroborationCount` from the user-submitted payload before publishing to wm:events:queue. These fields are relay-internal (computed by ais-relay's scoring pipeline). Also validates `severity` against the known allowlist (critical, high, medium, low, info) instead of accepting any string. 2. scripts/notification-relay.cjs — scope rule matching: if the event carries `event.userId` (browser-submitted via /api/notify), only match rules where `rule.userId === event.userId`. Relay-emitted events (from ais-relay, regional-snapshot) have no userId and continue to fan out to all matching Pro users. This prevents a single user from broadcasting crafted events to every other Pro subscriber's notification channels. Net effect: browser-submitted events can only reach the submitting user's own Telegram/Slack/Email/webhook channels, and cannot carry an injected importanceScore. 🤖 Generated with Claude Opus 4.6 via Claude Code Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(security): reject internal relay control events from /api/notify Review found that `flush_quiet_held` and `channel_welcome` are internal relay control events (dispatched by Railway cron scripts) that the public /api/notify endpoint accepted because only eventType length was checked. A Pro user could POST `{"eventType":"flush_quiet_held","payload":{}, "variant":"full"}` to force-drain their held quiet-hours queue on demand, bypassing batch_on_wake behavior. Now returns 403 for reserved event types. The denylist approach (vs allowlist) is deliberate: new user-facing event types shouldn't require an API change to work, while new internal events must explicitly be added to the deny set if they carry privileged semantics. * fix(security): exempt browser events from score gate + hoist Sets to module scope Two review findings from Greptile on PR #3143: P1: Once IMPORTANCE_SCORE_LIVE=1 activates, browser-submitted rss_alert events (which had importanceScore stripped by the first commit) would evaluate to score=0 at the relay's top-level gate and be silently dropped before rule matching. Fix: add `&& !event.userId` to the gate condition — browser events carry userId and have no server-computed score, so the gate must not apply to them. Relay-emitted events (no userId, server-computed score) are still gated as before. P2: VALID_SEVERITIES and INTERNAL_EVENT_TYPES Sets were allocated inside the handler on every request. Hoisted to module scope. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,9 @@ import { jsonResponse } from './_json-response.js';
|
|||||||
import { validateBearerToken } from '../server/auth-session';
|
import { validateBearerToken } from '../server/auth-session';
|
||||||
import { getEntitlements } from '../server/_shared/entitlement-check';
|
import { getEntitlements } from '../server/_shared/entitlement-check';
|
||||||
|
|
||||||
|
const VALID_SEVERITIES = new Set(['critical', 'high', 'medium', 'low', 'info']);
|
||||||
|
const INTERNAL_EVENT_TYPES = new Set(['flush_quiet_held', 'channel_welcome']);
|
||||||
|
|
||||||
export default async function handler(req: Request): Promise<Response> {
|
export default async function handler(req: Request): Promise<Response> {
|
||||||
if (isDisallowedOrigin(req)) {
|
if (isDisallowedOrigin(req)) {
|
||||||
return jsonResponse({ error: 'Origin not allowed' }, 403);
|
return jsonResponse({ error: 'Origin not allowed' }, 403);
|
||||||
@@ -58,6 +61,14 @@ export default async function handler(req: Request): Promise<Response> {
|
|||||||
return jsonResponse({ error: 'eventType required (string, max 64 chars)' }, 400, cors);
|
return jsonResponse({ error: 'eventType required (string, max 64 chars)' }, 400, cors);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Reject internal relay control events. These are dispatched by Railway
|
||||||
|
// cron scripts (seed-digest-notifications, quiet-hours) and must never be
|
||||||
|
// user-submittable. flush_quiet_held would let a Pro user force-drain their
|
||||||
|
// held queue on demand, bypassing batch_on_wake behaviour.
|
||||||
|
if (INTERNAL_EVENT_TYPES.has(body.eventType)) {
|
||||||
|
return jsonResponse({ error: 'Reserved event type' }, 403, cors);
|
||||||
|
}
|
||||||
|
|
||||||
if (typeof body.payload !== 'object' || body.payload === null || Array.isArray(body.payload)) {
|
if (typeof body.payload !== 'object' || body.payload === null || Array.isArray(body.payload)) {
|
||||||
return jsonResponse({ error: 'payload must be an object' }, 400, cors);
|
return jsonResponse({ error: 'payload must be an object' }, 400, cors);
|
||||||
}
|
}
|
||||||
@@ -69,8 +80,18 @@ export default async function handler(req: Request): Promise<Response> {
|
|||||||
return jsonResponse({ error: 'Service unavailable' }, 503, cors);
|
return jsonResponse({ error: 'Service unavailable' }, 503, cors);
|
||||||
}
|
}
|
||||||
|
|
||||||
const { eventType, payload } = body;
|
const { eventType } = body;
|
||||||
const severity = typeof body.severity === 'string' ? body.severity : 'high';
|
|
||||||
|
// Strip relay-internal scoring fields from user-supplied payload. These are
|
||||||
|
// computed server-side by the relay's importanceScore pipeline; allowing
|
||||||
|
// user-supplied values would let a Pro user bypass the IMPORTANCE_SCORE_MIN
|
||||||
|
// gate and fan out arbitrary alerts to every subscriber.
|
||||||
|
const payload = { ...(body.payload as Record<string, unknown>) };
|
||||||
|
delete payload.importanceScore;
|
||||||
|
delete payload.corroborationCount;
|
||||||
|
|
||||||
|
const rawSeverity = typeof body.severity === 'string' ? body.severity : 'high';
|
||||||
|
const severity = VALID_SEVERITIES.has(rawSeverity) ? rawSeverity : 'high';
|
||||||
const variant = typeof body.variant === 'string' ? body.variant : undefined;
|
const variant = typeof body.variant === 'string' ? body.variant : undefined;
|
||||||
|
|
||||||
const msg = JSON.stringify({
|
const msg = JSON.stringify({
|
||||||
|
|||||||
@@ -687,9 +687,12 @@ async function processEvent(event) {
|
|||||||
// short-circuit, but we still pay the promise/microtask cost unless gated here.
|
// short-circuit, but we still pay the promise/microtask cost unless gated here.
|
||||||
if (event.eventType === 'rss_alert') shadowLogScore(event).catch(() => {});
|
if (event.eventType === 'rss_alert') shadowLogScore(event).catch(() => {});
|
||||||
|
|
||||||
// Score gate — only for rss_alert; other event types (oref_siren, conflict_escalation,
|
// Score gate — only for relay-emitted rss_alert (no userId). Browser-submitted
|
||||||
// notam_closure, etc.) never attach importanceScore so they must never be gated here.
|
// events (with userId) have importanceScore stripped at ingestion and no server-
|
||||||
if (IMPORTANCE_SCORE_LIVE && event.eventType === 'rss_alert') {
|
// computed score; gating them would drop every browser notification once
|
||||||
|
// IMPORTANCE_SCORE_LIVE=1 is activated. Other event types (oref_siren,
|
||||||
|
// conflict_escalation, notam_closure) never attach importanceScore.
|
||||||
|
if (IMPORTANCE_SCORE_LIVE && event.eventType === 'rss_alert' && !event.userId) {
|
||||||
const score = event.payload?.importanceScore ?? 0;
|
const score = event.payload?.importanceScore ?? 0;
|
||||||
if (score < IMPORTANCE_SCORE_MIN) {
|
if (score < IMPORTANCE_SCORE_MIN) {
|
||||||
console.log(`[relay] Score gate: dropped ${event.eventType} score=${score} < ${IMPORTANCE_SCORE_MIN}`);
|
console.log(`[relay] Score gate: dropped ${event.eventType} score=${score} < ${IMPORTANCE_SCORE_MIN}`);
|
||||||
@@ -705,11 +708,17 @@ async function processEvent(event) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// If the event carries a userId (browser-submitted via /api/notify), scope
|
||||||
|
// delivery to ONLY that user's own rules. Relay-emitted events (ais-relay,
|
||||||
|
// regional-snapshot) have no userId and fan out to all matching Pro users.
|
||||||
|
// Without this guard, a Pro user can POST arbitrary rss_alert events that
|
||||||
|
// fan out to every other Pro subscriber — see todo #196.
|
||||||
const matching = enabledRules.filter(r =>
|
const matching = enabledRules.filter(r =>
|
||||||
(!r.digestMode || r.digestMode === 'realtime') && // skip digest-mode rules — handled by seed-digest-notifications cron
|
(!r.digestMode || r.digestMode === 'realtime') &&
|
||||||
(r.eventTypes.length === 0 || r.eventTypes.includes(event.eventType)) &&
|
(r.eventTypes.length === 0 || r.eventTypes.includes(event.eventType)) &&
|
||||||
shouldNotify(r, event) &&
|
shouldNotify(r, event) &&
|
||||||
(!event.variant || !r.variant || r.variant === event.variant)
|
(!event.variant || !r.variant || r.variant === event.variant) &&
|
||||||
|
(!event.userId || r.userId === event.userId)
|
||||||
);
|
);
|
||||||
|
|
||||||
if (matching.length === 0) return;
|
if (matching.length === 0) return;
|
||||||
|
|||||||
Reference in New Issue
Block a user