Skip to content

Fix/security and quality improvements#2555

Open
plato23 wants to merge 4 commits intokoala73:mainfrom
plato23:fix/security-and-quality-improvements
Open

Fix/security and quality improvements#2555
plato23 wants to merge 4 commits intokoala73:mainfrom
plato23:fix/security-and-quality-improvements

Conversation

@plato23
Copy link
Copy Markdown

@plato23 plato23 commented Mar 30, 2026

Summary

Type of change

  • Bug fix
  • New feature
  • New data source / feed
  • New map layer
  • Refactor / code cleanup
  • Documentation
  • CI / Build / Infrastructure

Affected areas

  • Map / Globe
  • News panels / RSS feeds
  • AI Insights / World Brief
  • Market Radar / Crypto
  • Desktop app (Tauri)
  • API endpoints (/api/*)
  • Config / Settings
  • Other:

Checklist

  • Tested on worldmonitor.app variant
  • Tested on tech.worldmonitor.app variant (if applicable)
  • New RSS feed domains added to api/rss-proxy.js allowlist (if adding feeds)
  • No API keys or secrets committed
  • TypeScript compiles without errors (npm run typecheck)

Screenshots

plato23 and others added 3 commits March 25, 2026 18:55
Syncs fork's main branch with upstream daily at 6 AM UTC using
GitHub's native merge-upstream API. Can also be triggered manually
from the Actions tab.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- notification-relay: extend isPrivateIP to block 0.0.0.0/8, 169.254.x.x
  (link-local), and fe80: (IPv6 link-local) in SSRF guard
- cross-module-integration: add explicit null check for sanctions before
  property access (TypeScript narrowing fix)
- market-watchlist: replace `as any` with `Record<string, unknown>` for
  stricter type checking in coerceEntry()
- ChatAnalystPanel: deduplicate scrollToBottom rAF calls during streaming
  to avoid excessive layout thrashing; clean up on destroy

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added the trust:safe Brin: contributor trust score safe label Mar 30, 2026
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2026

@plato23 is attempting to deploy a commit to the Elie Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR applies targeted security hardening and code-quality fixes across five files. The most substantive change is expanding the SSRF private-IP guard in scripts/notification-relay.cjs to cover 0.0.0.0/8, 169.254.0.0/16, and IPv6 link-local ranges, while also anchoring the ::1 loopback pattern to prevent false matches against addresses like ::10. The remaining changes are housekeeping: a requestAnimationFrame deduplication guard in ChatAnalystPanel.ts prevents redundant scroll updates during streaming; an explicit null-check is added around sanctions in cross-module-integration.ts for better type narrowing; as any is replaced with as Record<string, unknown> in market-watchlist.ts; and a scheduled GitHub Actions workflow is added to keep the fork in sync with upstream.

Key observations:

  • The isPrivateIP regex improvements are correct and complete for IPv4. The added IPv6 patterns (::1$, fe80:, fc, fd) are currently unreachable because the code only calls dns.resolve4() — they are inert but harmless, and hosts with IPv6-only addresses are already blocked safely by the DNS resolution failure path.
  • The sanctions && guard in cross-module-integration.ts is redundant at runtime (a truthy leadSanctions already implies sanctions is non-null via optional chaining), but it correctly narrows the TypeScript type for sanctions inside the block.
  • The RAF deduplication pattern in ChatAnalystPanel.ts follows the correct idiom and the cancelAnimationFrame cleanup in destroy() prevents potential access to a torn-down messagesEl.

Confidence Score: 5/5

Safe to merge — all changes are correct improvements with no functional regressions.

No P0 or P1 issues found. The single comment is P2 (dead IPv6 code paths in isPrivateIP) and does not affect correctness or security — the SSRF guard is still effective for the IPv4 path that actually runs. All other changes (RAF guard, null check, type cast, sync workflow) are clearly correct. The PR's stated goals are fully met.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/sync-upstream.yml New workflow to sync the fork with upstream daily via GitHub's merge-upstream API — clean and correct pattern.
scripts/notification-relay.cjs Expands the SSRF private-IP guard regex: adds 0.0.0.0/8, 169.254.0.0/16 link-local, anchors ::1 to avoid false-matches like ::10, and adds fe80: IPv6 link-local — all valid additions, though IPv6 patterns are unreachable via dns.resolve4.
src/components/ChatAnalystPanel.ts Adds RAF deduplication for scrollToBottom via a scrollRafId guard, and cancels the pending frame on destroy — correct and minimal fix.
src/services/cross-module-integration.ts Adds an explicit sanctions !== null guard before accessing sanctions.newEntryCount; technically redundant (leadSanctions being truthy already implies sanctions is non-null via optional chaining) but improves type narrowing.
src/services/market-watchlist.ts Replaces as any with as Record<string, unknown> for better type safety — straightforward improvement with no behavioural change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["sendSlack(userId, webhookEnvelope, text)"] --> B["decrypt(webhookEnvelope)"]
    B -->|fail| Z1["return - warn"]
    B -->|ok| C["Validate against SLACK_RE regex"]
    C -->|invalid| Z2["return - warn"]
    C -->|valid| D["dns.resolve4 - A records only"]
    D -->|throws| Z3["return - warn, catches IPv6-only hosts"]
    D -->|resolved IPs| E["addresses.some(isPrivateIP)"]
    E -->|private range matched| Z4["return - SSRF blocked"]
    E -->|public IP| F["fetch webhookUrl POST"]
    F -->|404 or 410| G["deactivateChannel slack"]
    F -->|ok| H["Message delivered"]
Loading

Reviews (1): Last reviewed commit: "Merge branch 'main' into fix/security-an..." | Re-trigger Greptile


function isPrivateIP(ip) {
return /^(10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.|127\.|::1|fc|fd)/.test(ip);
return /^(0\.|10\.|172\.(1[6-9]|2\d|3[01])\.|192\.168\.|169\.254\.|127\.|::1$|fe80:|fc|fd)/.test(ip);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 IPv6 patterns unreachable via dns.resolve4

The IPv6 patterns added (::1$, fe80:, fc, fd) can never match in the current code path because dns.resolve4() resolves only A records and always returns IPv4 address strings. If a webhook host resolves exclusively via AAAA (IPv6), dns.resolve4 will throw a ENODATA/ENOTFOUND error, which is caught and the request is safely blocked — so there is no vulnerability, but the IPv6 guards in this function are currently dead code.

If IPv6 SSRF protection is desirable in future, consider also calling dns.resolve6() and checking those results through isPrivateIP. For now the defensive patterns don't cause any harm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trust:safe Brin: contributor trust score safe

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant