Skip to content

fix(websocket): enforce auth, prevent provider hijack, and add consumer limits#345

Open
Akash504-ai wants to merge 1 commit into
AOSSIE-Org:mainfrom
Akash504-ai:fix/websocket-security
Open

fix(websocket): enforce auth, prevent provider hijack, and add consumer limits#345
Akash504-ai wants to merge 1 commit into
AOSSIE-Org:mainfrom
Akash504-ai:fix/websocket-security

Conversation

@Akash504-ai
Copy link
Copy Markdown

@Akash504-ai Akash504-ai commented May 2, 2026

Addressed Issues:

Fixes # (security / stability issue in websocket server)


Description

This PR fixes multiple critical security and stability issues in the WebSocket server implementation.

Key Fixes:

  • Enforced authorization for privileged actions (config update, input injection, provider control)
  • Restricted sensitive operations to localhost only
  • Prevented multiple providers from being registered simultaneously
  • Fixed provider lifecycle bug (stale provider lock after disconnect)
  • Ensured only the active provider can stream binary data
  • Added consumer limit to prevent broadcast amplification (DoS scenario)
  • Added safe JSON parsing to avoid crash / log spam

Impact:

  • Prevents remote clients from gaining system-level control
  • Eliminates provider hijacking and stale lock issues
  • Protects against resource exhaustion via uncontrolled consumers
  • Improves overall robustness of WebSocket handling

Screenshots/Recordings:

N/A (backend/security fix)


Functional Verification

Screen Mirror

  • Screen Mirror works.

Authentication

  • Connection doesn't work without a valid token.

Basic Gestures

  • One-finger tap: Verified as Left Click.
  • Two-finger tap: Verified as Right Click.
  • Click and drag: Verified selection behavior.
  • Pinch to zoom: Verified zoom functionality (if applicable).

Modes & Settings

  • Cursor mode: Cursor moves smoothly and accurately.
  • Scroll mode: Page scrolls as expected.
  • Sensitivity: Verified changes in cursor speed/sensitivity settings.
  • Copy and Paste: Verified both Copy and Paste functionality.
  • Invert Scrolling: Verified scroll direction toggles correctly.

Advanced Input

  • Key combinations: Verified modifier behavior (e.g., Ctrl+C).
  • Keyboard input: Verified Space, Backspace, and Enter keys.
  • Glide typing: Verified path drawing and text output.
  • Voice input: Not tested
  • Backspace doesn't send previous input.

Any other gesture or input behavior introduced:

  • No new gestures introduced

Additional Notes:

  • Changes are limited to src/server/websocket.ts
  • No API changes introduced
  • Existing localhost workflow remains unchanged
  • Security hardening applied without affecting normal usage

Checklist

  • My PR addresses a single issue, fixes a single bug or makes a single improvement.
  • My code follows the project's code style and conventions
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • If applicable, I have made corresponding changes or additions to the documentation
  • If applicable, I have made corresponding changes or additions to tests
  • My changes generate no new warnings or errors
  • I have joined the community and shared this PR (optional)
  • I have read the contributing guidelines
  • I will address CodeRabbit's comments

Summary by CodeRabbit

  • Security Improvements

    • Strengthened connection privilege verification for WebSocket operations
    • Implemented maximum consumer connection limits to prevent resource exhaustion
  • Stability Improvements

    • Enhanced message parsing to gracefully handle invalid JSON instead of throwing errors

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 2, 2026

Walkthrough

The WebSocket server adds privilege-based access control via an isPrivileged helper and introduces singleton provider tracking with a consumer limit (MAX_CONSUMERS = 5). Frame relay now gates on the active provider identity, and protected operations (start-provider, start-mirror, update-config, input injection) enforce authorization checks. JSON parsing errors are handled gracefully, and provider state is cleared on disconnect.

Changes

WebSocket Authorization & Role Management

Layer / File(s) Summary
State Initialization
src/server/websocket.ts (lines 51–53)
Introduces currentProvider variable (initially null) and MAX_CONSUMERS = 5 constant for tracking the active provider and enforcing consumer limits.
Authorization Helper
src/server/websocket.ts (lines 115–117)
New isPrivileged(ws, isLocal) helper centralizes privilege validation, returning isLocal as the authorization criterion.
Frame Relay Gating
src/server/websocket.ts (line 153)
Changes provider-to-consumer frame relay condition from isProvider flag to ws === currentProvider, ensuring only the tracked active provider's frames are forwarded.
Provider Registration
src/server/websocket.ts (lines 254–268)
start-provider handler enforces isPrivileged check, assigns ws to currentProvider, and rejects if a provider already exists.
Consumer Registration & Limits
src/server/websocket.ts (lines 230–243)
start-mirror handler enforces isPrivileged check, counts existing consumers from wss.clients, and rejects when count reaches MAX_CONSUMERS.
Configuration & Input Protection
src/server/websocket.ts (lines 270–280, 401–404)
update-config and input injection handlers gate execution behind isPrivileged; unauthorized attempts are logged and rejected.
Error Handling & Cleanup
src/server/websocket.ts (lines 174–180, 417–421)
JSON parsing wraps in try/catch with warning on invalid input; close handler clears currentProvider if the closing socket was the active provider and logs the slot release.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

Typescript Lang

Poem

🐰 The provider hops in, takes the crown,
Consumers queue up all around town,
Five frames relay, no more, no less,
With privilege checked—we're blessed!
JSON parse errors? We caught them all,
And cleanup's tidy when sockets fall. 🔌

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title directly and accurately summarizes the main security and stability fixes in the WebSocket server implementation.
Description check ✅ Passed The description follows the repository template with addressed issues, detailed explanation of key fixes, functional verification checklist completed, and comprehensive notes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
Review rate limit: 0/1 reviews remaining, refill in 60 minutes.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/server/websocket.ts`:
- Around line 115-116: isPrivileged currently only allows localhost (returns
isLocal) which blocks token-authenticated remote viewers because startMirror()
marks sockets as isConsumer and frame relay targets isConsumer sockets; update
isPrivileged to allow privileged access for sockets that are marked as consumers
in addition to local sockets (e.g., return isLocal || (ws as any).isConsumer or
consult the socket's consumer/auth flag). Apply the same change to the other
privilege checks referenced (the blocks around lines 139-142, 153-160, 229-244)
so that all checks allow isConsumer sockets the same way they allow isLocal.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 99012c59-82e0-4828-9f37-87392019e9dd

📥 Commits

Reviewing files that changed from the base of the PR and between 782183e and a81cffb.

📒 Files selected for processing (1)
  • src/server/websocket.ts

Comment thread src/server/websocket.ts
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