Skip to content

fix(security): harden /v1/sse endpoint β€” auth, tenant scoping, connection limiting#4395

Merged
aegis-gh-agent[bot] merged 1 commit into
developfrom
fix/4393-sse-auth-tenant
May 28, 2026
Merged

fix(security): harden /v1/sse endpoint β€” auth, tenant scoping, connection limiting#4395
aegis-gh-agent[bot] merged 1 commit into
developfrom
fix/4393-sse-auth-tenant

Conversation

@OneStepAt4time
Copy link
Copy Markdown
Owner

Summary

Fixes #4393 (P1 Security β€” Themis audit finding)

The /sse SSE bridge endpoint was unauthenticated and unscoped:

  • No auth middleware coverage (route didn't match SSE auth regex)
  • No tenant filtering (all events broadcast to all clients)
  • No connection limiting (resource exhaustion risk)
  • Wired to isolated LocalEventBus (inert, but would leak on wiring)

Changes

  1. Route β†’ /v1/sse β€” now matches auth middleware SSE regex
  2. Auth middleware updated β€” regex expanded to /v1/(events|sse)
  3. Tenant filtering β€” uses isGlobalEventVisibleToRequest() from routes/events.ts
  4. Connection limiting β€” reuses ctx.sseLimiter (same as /v1/events)
  5. Real event bus β€” wired to SessionEventBus.subscribeGlobal() instead of isolated LocalEventBus
  6. Removed dead code β€” LocalEventBus import removed from server.ts

Testing

  • 8 new/updated tests covering: route registration, limiter rejection (per-IP and global), event forwarding, tenant filtering, subscription failure, SSE headers
  • All passing

Security Impact

Before: On localhost (zero-config), any local process could connect to /sse and read all events across all tenants. On production, the endpoint was dead code (401 from generic catch-all).
After: Full auth coverage, tenant isolation, and connection limits β€” same posture as /v1/events.

…tion limiting (#4393)

Addresses P1 security finding from Themis audit:
1. Move /sse β†’ /v1/sse, add to auth middleware SSE regex so Bearer/SSE
   token, dashboard cookie, and query-string ?token= all apply
2. Add tenant filtering via isGlobalEventVisibleToRequest() β€” clients
   only see events for their tenant
3. Add SSEConnectionLimiter β€” reuse server's existing limiter instance
4. Wire to real SessionEventBus (not isolated LocalEventBus) with
   subscribeGlobal() instead of wildcard pattern matching
5. Rewrite sse-bridge.ts to accept RouteContext, matching /v1/events
   patterns for auth/tenant/limiter
6. Remove dead LocalEventBus import from server.ts
7. Update tests to match new API (8 tests, all passing)
Copy link
Copy Markdown
Contributor

@aegis-gh-agent aegis-gh-agent Bot left a comment

Choose a reason for hiding this comment

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

βœ… Approved β€” P1 security fix, clean implementation.

Gates: All 9 pass β€” CI green (18/18), no conflicts, proper tests, targets develop.

What's good:

  • Route moved to /v1/sse, auth regex expanded β€” endpoint now fully auth-covered
  • Tenant filtering reuses proven isGlobalEventVisibleToRequest()
  • Connection limiting reuses SSEConnectionLimiter (same posture as /v1/events)
  • Wired to real SessionEventBus.subscribeGlobal() instead of inert LocalEventBus
  • Clean cleanup on disconnect (heartbeat + unsubscribe + limiter release)
  • 8 tests covering auth rejection, tenant filtering, subscription failure, SSE headers

Nit (non-blocking): SYSTEM_TENANT imported but unused in sse-bridge.ts β€” remove it in a follow-up.

Merge authorized.

@aegis-gh-agent aegis-gh-agent Bot merged commit a7e0c4e into develop May 28, 2026
18 checks passed
@aegis-gh-agent aegis-gh-agent Bot deleted the fix/4393-sse-auth-tenant branch May 28, 2026 03:49
aegis-gh-agent Bot pushed a commit that referenced this pull request May 28, 2026
Documents the SSE bridge endpoint in:
- api-reference.md: full endpoint spec with auth, tenant scoping,
  connection limits, and error responses
- api-quick-ref.md: quick reference entry
- real-time-events.md: SSE bridge section alongside global/per-session

Endpoint was introduced in #4373 and hardened in #4395 but never documented.

Co-authored-by: Hephaestus <hep@aegis.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant