fix(launcher): support local workspace endpoints#404
Conversation
|
@Edison-A-N is attempting to deploy a commit to the Raphael's projects Team on Vercel. A member of the Team first needs to authorize it. |
zomux
left a comment
There was a problem hiding this comment.
Review — Changes Requested
Well-structured PR that cleanly adds local workspace endpoint support. The settings-driven hot-reload and URL normalization approach are good. A few security items to address:
Blocking
1. normalizeWorkspaceEndpoint accepts malformed URLs
The catch block silently returns the raw input minus trailing slash. A non-URL string (e.g., javascript:alert(1)) would pass through. Should reject invalid URLs entirely or at minimum validate the scheme.
2. No URL scheme restriction
Custom workspace URLs accept any scheme. Should restrict to http:/https: to prevent protocol injection — these values flow into openExternal() which is especially dangerous in Electron.
Notes (non-blocking)
this._connector!non-null assertions — several places assume_connectorexists. If core fails to load, these throw unhelpful errors. Consider a guard.- Token in URL query params — pre-existing pattern but this PR extends it to arbitrary endpoints, slightly increasing exposure surface.
- Overlap with PR #358 — significant overlap in intent but no file-level conflicts. #358 targets old JS codebase while #404 targets the TypeScript rewrite. #404 is the cleaner, more focused implementation.
Positive
- Good accessibility fixes (
htmlFor/id,<button>instead of<a href="#">) workspaceDisplayHost()utility replaces scattered hardcoded strings- Error re-throw in
connectToWorkspace()is a good catch - No migration changes
Reject non-http(s) URLs in normalizeWorkspaceEndpoint and parseCustomWorkspaceUrl to prevent protocol injection — these values flow into Electron's openExternal() which can execute arbitrary protocols. Also reject malformed URLs entirely instead of passing them through.
zomux
left a comment
There was a problem hiding this comment.
Pushed a security fix: URL scheme validation in normalizeWorkspaceEndpoint and parseCustomWorkspaceUrl — rejects non-http(s) URLs and malformed input. Original review concerns are now addressed. LGTM.
Summary
Test