Feature/workspace multi user identity#380
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ad45699 to
f57c15e
Compare
zomux
left a comment
There was a problem hiding this comment.
Security Review — Changes Requested
The core security model needs rework before this can merge. Sender identity is entirely client-asserted with zero server-side validation, enabling trivial impersonation.
Blocking Issues
1. No server-side identity validation
The backend blindly trusts client-supplied sender_id, sender_name, and sender_type. Any user can impersonate any other user by sending arbitrary metadata in the POST body. The backend must validate sender identity against the auth token/session and strip or ignore client-supplied sender fields that conflict.
2. Client-generated user ID stored in localStorage
For unauthenticated users, crypto.randomUUID() creates the identity. An attacker can trivially change this to another user's ID. Server-side enforcement is needed.
3. Unauthenticated presence events
Anyone can emit workspace.user.joined or workspace.user.left with arbitrary user IDs, enabling ghost-user injection or forcibly removing others from the online list.
4. Email used as user ID
Using user.email as sender_id leaks PII in event payloads stored in the database and makes IDs predictable for targeted impersonation.
5. No input validation on sender_name
No length limit, no character validation on the backend side. Should be validated/sanitized before storage.
Other Issues (non-blocking but should be addressed)
sendMessagesource field changes fromhuman:usertohuman:<actual-name>— may break downstream consumers parsingsource- Hardcoded Chinese prompt string (
'请输入你的名称') — should use i18n or English window.prompt()loop with no recovery path if user cancels — chat input permanently disabled
Recommendation
Before merging:
- Backend must validate sender identity against the auth token or session
- Strip/ignore client-supplied sender fields when they conflict with authenticated identity
- Add input validation (length, characters) for
sender_nameon the backend - Don't use raw email as user ID in event payloads — use a hashed or opaque identifier
Summary
Testing
npx tsc --noEmitnode --checkfor changed launcher/agent-connector filesnode --test test/daemon.test.js