Major Feature: Web Interface by pedramamini · Pull Request #4 · RunMaestro/Maestro

pedramamini · 2025-11-27T18:50:36Z

MAESTRO: Add /web/ route namespace for web interface*
MAESTRO: Add WebSocket upgrade handler for web clients at /ws/web
MAESTRO: Add optional PIN/token authentication for web interface
MAESTRO: Broadcast session state changes to connected web clients
MAESTRO: Send current theme on initial WebSocket connection
MAESTRO: Broadcast theme changes to connected web clients
MAESTRO: Add rate limiting for web interface endpoints
MAESTRO: Implement /api/sessions endpoint to return actual session data
MAESTRO: Implement /api/session/:id endpoint for detailed session data
MAESTRO: Implement /api/session/:id/send POST endpoint for sending commands
MAESTRO: Implement /api/session/:id/interrupt POST endpoint for session interruption
MAESTRO: Implement /api/theme GET endpoint for current theme configuration
MAESTRO: Extract theme types to shared location for web interface
MAESTRO: Create ThemeProvider component for web interface
MAESTRO: Create CSS custom properties generator for web interface theme system
MAESTRO: Create reusable Button component for web interface
MAESTRO: Create reusable Input and TextArea components for web interface
MAESTRO: Create reusable Badge component for web interface status indicators
MAESTRO: Create reusable Card component for web interface
MAESTRO: Create useWebSocket hook for web interface connection management
MAESTRO: Create useSessions hook for real-time session state management
MAESTRO: Create separate Vite config for web interface build
MAESTRO: Add npm run build:web script for web interface bundling
MAESTRO: Integrate web build into main build process
MAESTRO: Serve built web assets from Fastify static handler
MAESTRO: Configure code splitting for mobile vs desktop web bundles
MAESTRO: Create mobile entry point with utilities and configuration
MAESTRO: Add mobile app shell with dynamic connection status header
MAESTRO: Add comprehensive responsive viewport meta tags for mobile browsers
MAESTRO: Add PWA manifest for Add to Home Screen support
MAESTRO: Add service worker for PWA offline capability
MAESTRO: Add pull-to-refresh functionality for mobile session list
MAESTRO: Add horizontal scrollable session pill bar for mobile web
MAESTRO: Add long-press session info popover for mobile session pills
MAESTRO: Add collapsible group headers to mobile session pill bar
MAESTRO: Add All Sessions view with larger cards for mobile web
MAESTRO: Add sticky bottom CommandInputBar for mobile web interface
MAESTRO: Add large touch-friendly textarea input for mobile command bar
MAESTRO: Add mode toggle button (AI/Terminal) to mobile command input bar
MAESTRO: Add interrupt button (red X) to mobile command input bar
MAESTRO: Add auto-expanding textarea for mobile command input (up to 4 lines)
MAESTRO: Add command history drawer with swipe-up gesture for mobile web
MAESTRO: Add recent command chips for quick-tap reuse in mobile web
MAESTRO: Add slash command autocomplete popup for mobile web
MAESTRO: Add haptic feedback for mobile web command input
MAESTRO: Add session status banner for mobile web interface
MAESTRO: Add cost tracker to mobile web session status banner
MAESTRO: Add context window usage bar to mobile web session status banner
MAESTRO: Add collapsible last response preview to mobile web session status banner
MAESTRO: Add tap-to-expand full-screen response viewer for mobile web
MAESTRO: Add share button to copy last response to clipboard in mobile web
MAESTRO: Add syntax highlighting for code blocks in mobile web response viewer
MAESTRO: Add copy button for each code block in mobile web response viewer
MAESTRO: Add swipe left/right navigation between responses in mobile web response viewer
MAESTRO: Add pinch-to-zoom for code readability in mobile web response viewer
MAESTRO: Add notification permission request on first visit for mobile web
MAESTRO: Add push notifications for AI response completion in mobile web
MAESTRO: Add unread response badge count for mobile web PWA
MAESTRO: Add voice input button for speech-to-text in mobile web
MAESTRO: Add quick actions menu for mobile web send button
MAESTRO: Add swipe gestures for common actions in mobile web
MAESTRO: Add offline queue for commands typed while disconnected
MAESTRO: Add connection status indicator with retry button for mobile web
MAESTRO: Add device color scheme preference support for mobile web

Created dedicated web interface route namespace in WebServer class: - /web - Root endpoint returning available interfaces info - /web/desktop - Desktop web interface entry point (placeholder) - /web/desktop/* - Wildcard for client-side routing - /web/mobile - Mobile web interface entry point (placeholder) - /web/mobile/* - Wildcard for client-side routing - /web/api - Web API namespace root with endpoint discovery This establishes the foundation for the new web interface that will provide both desktop (collaborative) and mobile (remote control) access to Maestro sessions.

- Added new WebSocket endpoint at /ws/web for web interface clients - Implemented client connection tracking with unique client IDs - Added connection/disconnection event handling with logging - Added message handling for ping/pong, subscribe, and echo - Added broadcastToWebClients() method for real-time updates - Added getWebClientCount() method for monitoring connected clients - Imported WebSocket from 'ws' for readyState checks - Added WebClient and WebClientMessage type definitions

- Add WebAuthConfig type and auth state management in WebServer - Generate 6-character alphanumeric PINs (excludes confusing chars) - Support authentication via: - WebSocket query string (?token=XXX) - WebSocket auth message { type: 'auth', token: '<token>' } - REST API headers (Authorization: Bearer or X-Auth-Token) - Add IPC handlers for auth management: - webserver:getAuthConfig - webserver:setAuthEnabled (auto-generates token if needed) - webserver:generateNewToken - webserver:setAuthToken - webserver:getConnectedClients - Persist auth settings in electron-store - Add /web/api/auth/status and /web/api/auth/verify endpoints

Added real-time session state broadcasting to the web interface: - Added broadcastSessionStateChange() method to broadcast when session state, name, or input mode changes - Added broadcastSessionAdded() and broadcastSessionRemoved() methods for tracking session lifecycle - Added broadcastSessionsList() method for bulk session sync - Modified sessions:setAll IPC handler to detect session changes and broadcast them to all authenticated web clients - Added setGetSessionsCallback() to allow web server to fetch current sessions - Send initial sessions_list to newly connected/authenticated web clients - Only broadcast to authenticated clients for security WebSocket message types added: - session_state_change: { type, sessionId, state, name?, toolType?, inputMode?, cwd?, timestamp } - session_added: { type, session, timestamp } - session_removed: { type, sessionId, timestamp } - sessions_list: { type, sessions, timestamp }

- Add WebTheme type and GetThemeCallback to web-server.ts - Add setGetThemeCallback method to WebServer class - Send theme after sessions list on initial connection - Send theme after auth success for authenticated clients - Create src/main/themes.ts with all theme definitions for main process - Wire up theme callback in index.ts using getThemeById helper

Added broadcastThemeChange method to WebServer class and integrated it with the settings:set IPC handler to automatically notify all connected web clients when the user switches themes in the desktop app.

- Install @fastify/rate-limit package - Configure rate limiting with sensible defaults: - 100 requests/minute for GET endpoints - 30 requests/minute for POST endpoints (more restrictive) - Add RateLimitConfig interface for configuration - Apply rate limiting to all /web/* routes - Add /web/api/rate-limit endpoint to check current limits - Skip rate limiting for /health endpoint - Custom error response with retry-after information - Support for X-Forwarded-For header for proxied requests

Updated the /api/sessions endpoint to use the getSessionsCallback to return real session data instead of an empty array placeholder. Added authentication and rate limiting to the endpoint.

- Add SessionDetail interface with extended session fields (aiLogs, shellLogs, usageStats, claudeSessionId, isGitRepo) - Add GetSessionDetailCallback type and setGetSessionDetailCallback method - Implement /api/session/:id endpoint with authentication and rate limiting - Returns 404 for non-existent sessions, 503 if callback not configured - Wire up callback in index.ts to fetch session data from sessions store - Update header comments to reflect current implementation status

…mmands Add POST endpoint to send commands to active sessions via the web API: - Add WriteToSessionCallback type for session input - Implement /api/session/:id/send endpoint with rate limiting (POST limits) - Validate command presence and type before processing - Check session exists before attempting to write - Wire up callback in index.ts using processManager.write()

…on interruption Add REST API endpoint to send SIGINT/Ctrl+C signal to sessions via the web interface. This allows mobile and desktop web clients to gracefully interrupt running AI agents or terminal processes. - Add InterruptSessionCallback type for session interrupt operations - Add setInterruptSessionCallback method to WebServer class - Create /api/session/:id/interrupt POST endpoint with authentication and rate limiting - Wire up callback in main process to use ProcessManager.interrupt()

…ation Added a new REST API endpoint at /api/theme that returns the currently configured theme. The endpoint: - Is protected by token-based authentication (if enabled) - Is rate limited using GET rate limit config - Returns the theme object with all color values - Returns 503 if theme service not configured - Returns 404 if no theme is currently set

- Create src/shared/theme-types.ts with Theme, ThemeId, ThemeMode, ThemeColors types - Add isValidThemeId type guard utility function - Update renderer types to re-export from shared location - Update main process themes.ts to use shared types - Update web-server.ts to import Theme from shared instead of defining WebTheme - This enables the web interface build to access theme types without duplicating code

- Add src/web/components/ThemeProvider.tsx with React context for theming - Provide useTheme and useThemeColors hooks for child components - Include default theme (Dracula) for initial render before WebSocket connection - Update tsconfig.json to include src/web and src/shared directories

…me system Add utility module that converts theme colors to CSS custom properties: - generateCSSProperties(): Creates property map from theme - generateCSSString(): Outputs CSS with :root selector - injectCSSProperties(): Adds/updates style element in document head - removeCSSProperties(): Cleans up injected styles - setElementCSSProperties(): Applies to specific elements - cssVar(): Helper for inline style usage CSS variables use --maestro-* prefix with kebab-case naming (e.g., theme.colors.bgMain -> --maestro-bg-main). ThemeProvider now automatically injects CSS properties when theme changes, enabling CSS-based theming alongside React context.

Add Button and IconButton components with theme support: - Multiple variants: primary, secondary, ghost, danger, success - Three sizes: sm, md, lg - Loading state with spinner animation - Support for left/right icons - Full width option - IconButton variant for icon-only buttons - Uses theme colors via useTheme hook

Add Input, TextArea, and InputGroup components following the same patterns as the Button component with theme support via useTheme() hook. Features: - Three variants: default, filled, ghost - Three sizes: sm, md, lg - Error state support with visual feedback - Icon support (left/right) for Input component - Auto-resize capability for TextArea - InputGroup wrapper for label, helper text, and error display

…icators Add Badge component with multiple variants for session states: - success (green): Ready/idle sessions - warning (yellow): Agent thinking/busy - error (red): No connection/error - connecting (orange with pulse): Connecting state - info (accent color): Informational badges - default: Neutral styling Includes three badge styles (solid, outline, subtle, dot) and convenience components StatusDot and ModeBadge for common use cases.

Add Card component with support for multiple variants (default, elevated, outlined, filled, ghost), padding options, and interactive states. Includes: - CardHeader subcomponent for consistent headers with title/subtitle/actions - CardBody subcomponent for main content with padding control - CardFooter subcomponent for footer actions with optional border - SessionCard convenience component for session list items Supports interactive states (clickable, selected, disabled) with keyboard navigation and accessibility attributes.

…ment Add a comprehensive useWebSocket hook that handles: - WebSocket connection lifecycle (connect, disconnect, reconnect) - Authentication flow (token-based auth via query param or message) - Real-time message handling for sessions, themes, and state changes - Automatic reconnection with configurable attempts and delays - Heartbeat/ping functionality for connection health - Full TypeScript support with typed message interfaces

Implements a useSessions hook that wraps useWebSocket to provide: - Real-time session list management with add/remove/update handlers - Active session tracking and selection - Session interaction methods (sendCommand, interrupt, switchMode) - Sessions grouped by tool type - Client-side state tracking (isSending, lastError) preserved across updates - Auto-connect option and refresh capability

- Add vite.config.web.mts with dedicated web interface configuration - Configure build output to dist/web/ directory - Set up code splitting with React in separate chunk - Enable source maps for development debugging - Add dev server proxy for API and WebSocket connections - Create index.html entry point with mobile-optimized meta tags - Create main.tsx with device detection for mobile/desktop routing - Add index.css with Tailwind, CSS custom properties, and animations - Create placeholder mobile/desktop App components for Phase 1/2 - Update tailwind.config.mjs to include src/web files

- Add build:web step to main build script chain - Add dev:web script for standalone web development server - Web assets now built automatically during npm run build - Packaged app will include dist/web/* via existing files glob

- Install @fastify/static for serving static files - Add resolveWebAssetsPath method to find web assets in dist/web - Register @fastify/static plugin to serve assets at /web/assets/ - Add serveIndexHtml helper that transforms asset paths for SPA - Update /web/desktop and /web/mobile routes to serve index.html - Transform relative ./assets/ paths to /web/assets/ for proper routing

Updated vite.config.web.mts to use dynamic chunk naming and manual chunking function for better code splitting: - Mobile and desktop apps now build into separate named chunks - React dependencies are isolated in their own chunk for caching - Dynamic chunkFileNames function preserves mobile/desktop naming - Added webpackChunkName magic comments in main.tsx for clarity Build output now shows distinct bundles: mobile-*.js, desktop-*.js, main-*.js, and react-*.js for optimal lazy loading.

Add src/web/mobile/index.tsx as the proper module entry point for the mobile web interface. This includes: - Re-exports of MobileApp component for cleaner imports - Mobile configuration options (haptics, voice input, offline queue) - Mobile viewport breakpoint constants - Safe area padding defaults for notched devices - Haptic feedback utilities with pattern presets - Gesture threshold constants for swipe/pull-to-refresh detection - Voice input support detection Updated main.tsx to import from './mobile' instead of './mobile/App' to leverage the new entry point for better code organization.

- Integrate useWebSocket hook for real-time connection state - Add MobileHeader component with Badge-based status indicator - Display different UI states: disconnected, connecting, connected - Enable retry button when disconnected - Use proper safe-area insets for notched devices (env()) - Enable/disable input controls based on connection state

…rowsers - Enhanced viewport meta tag with minimum-scale, maximum-scale for zoom control - Added theme-color meta tags with media queries for light/dark preference - Added format-detection to disable auto-linking of phone/dates/emails - Added msapplication-tap-highlight for Edge browser support - Added mobile-web-app-capable and application-name for Android PWA - Added dynamic viewport height (dvh) with -webkit-fill-available fallback - Added iOS-specific fixes for rubber-banding and touch zoom - Added touch-action: manipulation to prevent double-tap zoom - Added input font-size fix to prevent iOS Safari zoom on focus - Added touch callout and user-select controls for interactive elements - Added .scrollable utility class for momentum scrolling

- Create manifest.json with app metadata, icons, and shortcuts - Add PWA icon generation script (scripts/generate-pwa-icons.mjs) - Configure Vite public directory for static PWA assets - Add apple-touch-icon links for iOS home screen - Add favicon links for browser tabs - Include npm script for regenerating PWA icons

- Move side effects (toast notifications, queued message processing) out of state updater functions to avoid double-invocation issues in StrictMode - Add support for built-in slash commands (like /synopsis, /clear) in the remote command handler, not just custom AI commands - Add refs for slash command functions so remote handler can access latest function versions Claude ID: 3dd3573f-bed1-4a0e-984c-10081124092a Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682

- Add live:disableAll IPC handler to properly disconnect all live sessions when turning off remote access mode - Mobile: Input bar now expands drawer-style on small screens when focused, shrinking message history area for better keyboard space - Mobile: Replace "All Sessions" pill with hamburger menu icon at the start of session pill bar for consistent navigation - Mobile: Remove pull-to-refresh feature for simpler interaction model - Desktop: Simplify TerminalOutput auto-scroll logic using three separate useEffects that only check hasExpandedLogs Claude ID: 3dd3573f-bed1-4a0e-984c-10081124092a Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682

claude · 2025-11-28T10:40:12Z

PR #4 Review: Web Interface Implementation

Executive Summary

This PR adds a comprehensive web interface (~19,600 lines across 75 files) enabling mobile and desktop browser access to Maestro. While the implementation shows excellent UX design and clean architecture, there are critical security vulnerabilities that must be addressed before merging.

Overall Risk Assessment: HIGH

CRITICAL SECURITY ISSUES (Must Fix Before Merge)

CRIT-1: Insufficient Authentication Model

File: src/main/web-server.ts:196, 214, 359-384

Issue: The current token-based auth has serious weaknesses:

Token is just a UUID in the URL path (visible in browser history, proxy logs, referer headers)
No session-based authentication or time-limited tokens
Anyone with the URL has full system access

Recommendation:

Implement proper PIN/password authentication
Add session-based auth with time-limited tokens
Move authentication out of URL path - use HTTP headers
Consider HTTPS with self-signed certificates
Add IP whitelisting option

CRIT-2: No CSRF Protection

File: src/main/web-server.ts:667-713, 748-793

Issue: POST endpoints lack CSRF protection. A malicious website could execute commands in user sessions.

Recommendation:

Implement CSRF token mechanism
Validate Origin/Referer headers
Require custom header for API requests

CRIT-3: Command Injection Risk

File: src/main/index.ts:1876-1916

Issue: The notification:speak handler spawns child processes with user-supplied commands without proper validation.

Recommendation:

Whitelist allowed TTS commands
Use absolute paths only
Properly escape/sanitize all arguments

HIGH-1: WebSocket Input Validation

File: src/main/web-server.ts:899-1081

Issue: WebSocket messages lack validation. Malformed messages could cause crashes.

Recommendation:

Use schema validation (Zod, Joi) for all WebSocket messages
Validate string lengths
Type-check all message fields
Add per-connection rate limiting

HIGH-2: Inadequate Rate Limiting

File: src/main/web-server.ts:171-176

Current limits:

100 GET requests/minute
30 POST requests/minute
No WebSocket message rate limiting

Recommendation:

Add WebSocket message rate limiting
Reduce POST rate limit to 10/minute
Add max concurrent connections limit

MEDIUM PRIORITY ISSUES

MED-1: Missing Security Headers

Add CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy

MED-2: Memory Leak Risks

WebSocket clients map has no size limits or cleanup for stale connections.

MED-3: No HTTPS Support

Web server runs on HTTP only, vulnerable to MITM on local network.

MED-4: localStorage Data Not Validated

JSON.parse operations lack try-catch and validation.

POTENTIAL BUGS

BUG-1: Race Condition in Session State Updates

src/main/index.ts:536-578 - Session state changes broadcasted during iteration.

BUG-2: WebSocket Reconnection Issues

src/web/hooks/useWebSocket.ts:594-610 - Stale state in closure.

BUG-3: Missing try-catch for JSON.parse

Corrupted localStorage data will crash the app.

MISSING ELEMENTS

No Tests

Zero test files found in src/web/ directory.

Required:

Unit tests for hooks
Integration tests for WebSocket
Security tests for injection attacks

No Error Boundaries

React components lack error boundaries.

No Documentation

Missing security model docs, API docs, setup guide.

POSITIVE HIGHLIGHTS

Excellent mobile UX - comprehensive touch interactions, PWA support
Clean architecture - well-organized component structure
Good conventions - follows CLAUDE.md guidelines
Feature completeness - full mobile experience
Type safety - consistent TypeScript usage

ACTION ITEMS

Must Fix Before Merge:

Implement proper authentication system
Add CSRF protection
Fix command injection vulnerability
Add input validation for WebSocket messages
Add security headers
Add basic test coverage

Should Fix Soon:

Improve rate limiting
Add React error boundaries
Fix memory leak risks
Fix race conditions

Nice to Have:

Add HTTPS support
Validate localStorage data
Optimize WebSocket broadcasting
Add comprehensive documentation

Estimated effort to fix critical issues: 3-5 days

FINAL VERDICT

Do not merge until critical security issues are resolved.

This PR adds valuable functionality but introduces significant security risks:

Unauthorized command execution via CSRF
Potential RCE via command injection
Session hijacking via token theft
DoS via uncontrolled WebSocket connections

The authentication model needs a complete overhaul. Once security issues are addressed, this will be an excellent addition to Maestro.

Size: Very Large (~19,600 additions, 75 files)
Risk Level: HIGH
Reviewed: 2025-11-28

- Web server now starts only when user enables the web interface (lazy initialization) - Added live:startServer and live:stopServer IPC handlers for explicit control - live:disableAll now also stops the server and cleans up resources - Enlarged QR code from 100px to 240px for easier mobile scanning - Pinned hamburger menu button outside scroll container in session pill bar - Disabled WebSocket auto-reconnect (users retry manually via button) - Shortened AI input placeholder on small screens Claude ID: 3dd3573f-bed1-4a0e-984c-10081124092a Maestro ID: 5a166b38-b7e9-47f0-a8ff-0113c65f2682

claude · 2025-11-28T11:00:57Z

Code Review: PR #4 - Web Interface for Maestro

Executive Summary

Comprehensive security review of PR #4 adding web interface with WebSocket support, PWA capabilities, and mobile interface. This is a substantial addition (~20,000 lines across 75 files) introducing remote access to Maestro.

Overall Assessment: Excellent architectural thinking and solid patterns, but CRITICAL security vulnerabilities must be addressed before merging.

CRITICAL ISSUES (Must Fix Before Merge)

1. CORS Configuration Too Permissive - SEVERITY: CRITICAL

Location: src/main/web-server.ts
Issue: CORS with origin: true accepts requests from ANY origin, enabling CSRF attacks if the security token is leaked.
Recommendation: Restrict CORS to only allow same-origin or localhost requests.

2. Server Binds to 0.0.0.0 - SEVERITY: HIGH

Location: src/main/web-server.ts
Issue: Exposes server to all network interfaces including public WiFi.
Recommendation: Default to 127.0.0.1 for local-only access, add opt-in config for LAN binding with UI warning.

3. Authentication Relies Solely on URL Token - SEVERITY: HIGH

Location: src/main/web-server.ts
Issue: UUID in URL exposed in browser history, proxy logs, screenshots, and shoulder surfing.
Recommendation: Add PIN/password authentication layer, token rotation, session timeout, and IP whitelisting.

4. Command Injection Risk - SEVERITY: HIGH

Location: src/main/web-server.ts, src/main/index.ts
Issue: No input validation on forwarded commands (length, special characters, ANSI escape codes).
Recommendation: Validate command length (max 50KB) and filter control characters.

5. Rate Limiting Insufficient - SEVERITY: MEDIUM-HIGH

Location: src/main/web-server.ts
Issue: Per-IP only (easily bypassed), 100 GET/min too generous, no global limit.
Recommendation: Use 30 GET and 10 POST per minute with global limits.

IMPORTANT ISSUES (Should Fix)

No Content Security Policy: Missing CSP headers enables XSS attacks
WebSocket Validation Insufficient: No message size limits or schema validation
Session State Race Condition: TOCTOU between state check and command execution
No Request Size Limits: Fastify missing bodyLimit and requestTimeout
Memory Leak Risk: WebSocket clients may not be cleaned up on error

POSITIVE ASPECTS

Excellent separation of concerns via IPC callbacks
Strong TypeScript usage throughout
Security-aware: Uses execFileNoThrow avoiding shell injection
Comprehensive logging
Clean React patterns and hooks
Well-designed PWA implementation
Responsive mobile-first design
Proper theme integration across renderer and web

TEST COVERAGE GAPS

Critical areas need tests: authentication, rate limiting, command injection, WebSocket security, CORS, session state race conditions

DOCUMENTATION NEEDS

Security model and threat assumptions
Deployment guide with firewall rules
API documentation for WebSocket and REST
Mobile setup troubleshooting

FINAL VERDICT: REQUEST CHANGES

Must Fix Before Merge:

CORS configuration (Critical)
Bind to localhost by default (Critical)
Add authentication beyond URL token (High)
Input validation for commands (High)
CSP and security headers (High)
Rate limiting improvements (Medium)

Should Fix Soon After:

Comprehensive security tests
Audit logging
Error boundaries
Security documentation

Excellent work overall! Once security issues are addressed, this will be a fantastic addition to Maestro. The architecture is sound, code quality is high, and the feature set is comprehensive.

Review Scope: 75 files, ~20,000 lines
Focus: OWASP Top 10, WebSocket security, authentication
Reviewer: Claude Code (Automated Security Review)

claude · 2025-11-28T11:03:08Z

Pull Request Review: Web Interface Implementation

Overview

This is an extensive PR adding a comprehensive web interface for Maestro (~19,689 additions). The implementation includes both a mobile PWA and desktop web interface with real-time WebSocket communication, authentication, and session management.

Critical Security Concerns

🔴 HIGH SEVERITY: Security Token in URL Path

Location: src/main/web-server.ts, src/web/utils/config.ts

The security model relies on a UUID token embedded in the URL path:

URLs: http://IP:PORT/$TOKEN/, http://IP:PORT/$TOKEN/session/:id
Token is regenerated on app restart
Issue: Tokens in URLs are logged in browser history, proxy logs, referrer headers, and analytics

Recommendations:

Move authentication to HTTP headers or cookies
Add session timeout and rotation mechanisms
Implement rate limiting per IP (currently per-endpoint only)
Consider adding HTTPS support or at minimum warn users about unencrypted traffic on LANs

Reference: OWASP Top 10:2021 A01:2021-Broken Access Control

🔴 MEDIUM SEVERITY: No CSRF Protection

Location: POST endpoints in src/main/web-server.ts

POST endpoints (/api/session/:id/send, /api/session/:id/interrupt) lack CSRF tokens. While the security token provides some protection, cross-site attacks are still possible if an attacker knows the token.

Recommendations:

Add CSRF tokens for state-changing operations
Validate Origin/Referer headers
Use SameSite cookie attributes if implementing cookie-based auth

🟡 MEDIUM SEVERITY: Command Injection Surface Expanded

Location: src/main/web-server.ts:924-998, src/web/mobile/CommandInputBar.tsx

Commands from the web interface are routed through executeCommandCallback to the renderer process. While the main process uses execFileNoThrow properly, the web interface significantly expands the attack surface.

Current Controls:

Session busy check prevents command queueing (line 950)
Commands are logged with context (line 966)

Recommendations:

Add command validation/sanitization layer before execution
Implement command whitelist/blacklist for web interface
Add audit logging for all web-initiated commands
Consider requiring additional confirmation for destructive commands

🟡 MEDIUM SEVERITY: WebSocket Message Validation

Location: src/main/web-server.ts:899-1081

Message handling in handleWebClientMessage has basic validation but could be more robust:

Line 872: Generic try-catch silently handles invalid JSON
Missing schema validation for message payloads
Type assertions without runtime checks (e.g., message.sessionId as string)

Recommendations:

Add schema validation using zod/joi/yup
Return specific error codes for invalid messages
Implement message size limits to prevent DoS
Add structured logging for security events

Security Best Practices Observed ✅

Good: Uses execFileNoThrow for command execution (avoiding shell injection)
Good: Rate limiting implemented with @fastify/rate-limit
Good: CORS configured with @fastify/cors
Good: Validates token before serving routes
Good: Server binds to 0.0.0.0 but docs should warn about firewall setup
Good: Session detail checks prevent unauthorized access to non-existent sessions

Code Quality Issues

🟡 High Complexity Functions

Location: src/web/mobile/CommandInputBar.tsx (1,260 lines)

The CommandInputBar component is extremely large and handles multiple concerns:

Input management
Voice recognition
Slash commands
Quick actions
Haptic feedback
Keyboard handling

Recommendation: Split into smaller, focused components

🟡 Inconsistent Error Handling

Location: Multiple files

Error handling varies across the codebase:

Some errors return 503, others 500
Some log errors, others silently fail
WebSocket error handler at line 889 lacks detail

Recommendation: Standardize error responses and add error tracking

🟡 Missing Input Validation

Location: src/main/web-server.ts:676

The command parameter is validated for presence and type but not for:

Maximum length
Character restrictions
Known malicious patterns

Performance Considerations

🟡 Broadcast Efficiency

Location: src/main/web-server.ts:1086-1122

broadcastToWebClients serializes JSON for every client individually. For many clients, this could be optimized, but reviewing the code shows this is already implemented correctly (line 1087) with JSON serialization outside the loop. Could still benefit from batching for high-frequency updates.

🟡 Memory Management

Location: src/main/index.ts:184-200

Session logs are truncated for web preview (first 3 lines/500 chars) but full logs are still sent. Consider:

Paginating log history
Implementing log streaming instead of bulk transfer
Adding max log retention policies

Testing Gaps

🔴 No Tests Included

This PR adds a significant amount of code (~19,689 lines) without accompanying tests.

Critical areas needing tests:

Authentication flow and token validation
WebSocket message handling
Command routing and validation
Rate limiting behavior
Session state synchronization
PWA offline functionality

Documentation Issues

🟡 Missing Security Documentation

The PR lacks documentation on:

Security model and threat model
Network setup recommendations (firewall rules)
TLS/HTTPS setup guide
Token rotation procedures
Audit logging setup

Architecture Concerns

🟡 Tight Coupling Between Desktop and Web

The web interface relies heavily on desktop app state via callbacks. Consider:

Defining clear API boundaries
Implementing API versioning
Adding deprecation warnings for breaking changes

Mobile-Specific Issues

🟡 PWA Service Worker Caching

Location: src/web/public/sw.js

Service worker implementation needs review:

Cache invalidation strategy unclear
No version management for cache
Offline queue behavior not fully specified

Positive Aspects ⭐

Comprehensive implementation covering mobile and desktop use cases
Good UX attention (haptic feedback, voice input, pull-to-refresh)
Accessibility considered (ARIA labels, keyboard navigation)
Real-time synchronization with desktop app
Theme system properly shared between main and renderer
Type safety with TypeScript throughout
Responsive design with proper viewport handling

Recommendations Summary

Before Merge (Blockers):

⚠️ Add authentication documentation and security warnings
⚠️ Implement rate limiting per IP (not just per endpoint)
⚠️ Add command validation layer for web-initiated commands
⚠️ Add basic integration tests for critical paths

High Priority (Post-Merge):

Move token to header-based authentication
Add CSRF protection
Implement HTTPS support
Add comprehensive test coverage
Split large components (CommandInputBar)
Add security event logging

Medium Priority:

Add message schema validation
Improve error handling consistency
Add API versioning
Document security model
Add metrics/monitoring

Additional Files to Review

Given the size, I recommend focused review of:

src/main/session-web-server.ts (if different from web-server.ts)
src/web/hooks/useOfflineQueue.ts (offline command queueing)
src/web/public/sw.js (service worker security)
vite.config.web.mts (build configuration)

Final Assessment

This PR represents a substantial feature addition with strong UX design but needs security hardening before production use. The code quality is generally good, but the security model needs review and documentation.

Recommendation: Approve with conditions - address critical security concerns and add documentation before enabling by default.

Estimated Risk Level: Medium-High (due to expanded attack surface and authentication model)

Review completed using Claude Code PR review workflow. For questions about specific findings, please reference line numbers in the location sections.

Address PR review feedback: - Issue #1: Add early returns in openModal, closeModal, and closeAll to skip Map recreation when state hasn't changed. Prevents unnecessary selector re-evaluations across all subscribers. - Issue #2: Add JSDoc to useModalActions() explaining that the ~40 selector subscriptions are intentionally transitional — identical to old Context behavior. Components should migrate to direct useModalStore(selectModalOpen(...)) calls for granular subscriptions. - Issue #5: Add 14 new tests covering no-op guard behavior (7 tests including render count verification), getModalActions() API surface and cross-call store sharing (3 tests), and useModalActions() reactive state and compatibility (4 tests). Issues #3 (memory leak) and #4 (type overloads) are not actionable: 47 fixed modal IDs is bounded, and ModalDataFor<T> already resolves to undefined for unmapped modals.

Adds investigation and planning documents for: 1. PRICING-BUGFIXES-PHASE2.md - Summary of 5 issues found during testing: - Issue RunMaestro#1: Agent Settings Detection - WORKING CORRECTLY - Issue RunMaestro#2: Folder Toggle Shows Blank - Minor bug, fix ready - Issue RunMaestro#3: Folder Detection Shows API - Critical bug, fix ready - Issue RunMaestro#4: Analysis of RunMaestro#1 vs RunMaestro#3 - Issue RunMaestro#5: Dashboard costs - Requires separate plan 2. PRICING-DASHBOARD-COST-FIX.md - Comprehensive plan for Issue RunMaestro#5: - Root cause: Costs stored at API rates, billing mode never applied - Solution: Recalculate at storage time with proper billing mode - 6-phase implementation plan with database schema changes - Risk assessment and mitigation strategies - Timeline estimate: 12-18 hours Auto Run fix documents created in /app/__AUTORUN/: - PRICING-FIX-02-FOLDER-TOGGLE.md - PRICING-FIX-03-FOLDER-DETECTION.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

pedramamini added 30 commits November 27, 2025 02:33

MAESTRO: Broadcast theme changes to connected web clients

55761ee

Added broadcastThemeChange method to WebServer class and integrated it with the settings:set IPC handler to automatically notify all connected web clients when the user switches themes in the desktop app.

MAESTRO: Implement /api/sessions endpoint to return actual session data

e7e037f

Updated the /api/sessions endpoint to use the getSessionsCallback to return real session data instead of an empty array placeholder. Added authentication and rate limiting to the endpoint.

MAESTRO: Add npm run build:web script for web interface bundling

a22c2b2

MAESTRO: Integrate web build into main build process

1ad7daf

- Add build:web step to main build script chain - Add dev:web script for standalone web development server - Web assets now built automatically during npm run build - Packaged app will include dist/web/* via existing files glob

pedramamini added 2 commits November 28, 2025 04:07

we have a functional remote control

c27af28

pedramamini merged commit 30910dc into main Nov 28, 2025
1 check passed

pedramamini deleted the web-interface branch November 28, 2025 11:05

This was referenced Nov 29, 2025

Laundry List of Fixes #6

Merged

Remote Tunnel Support #10

Merged

Autorun Overhaul #13

Merged

This was referenced Dec 6, 2025

Maestro CLI for Playbooks #16

Merged

Onboarding Wizard #19

Merged

This was referenced Dec 14, 2025

Refactor #25

Merged

refactor ii #26

Merged

More Testing, More Features #28

Merged

Group Chat Support! #39

Merged

This was referenced Jan 3, 2026

Add inline /wizard command for Auto Run document creation #112

Merged

feat: Add Symphony token donation system for open source contributions #122

Merged

claude bot mentioned this pull request Feb 7, 2026

0.15.0 polish #324

Merged

This was referenced Feb 12, 2026

refactor: added groupChatStore, operationStore, batchStore #363

Merged

feat: CSV table rendering + fix: group chat write race & SSH spawn #364

Merged

This was referenced Feb 17, 2026

fix: complete theme token system with WCAG-compliant color tokens #370

Open

feat: add auto-scroll toggle for AI output #390

Merged

This was referenced Feb 17, 2026

feat: Encore Features system for optional feature gating #396

Merged

refactor: extract 4 domain hooks from App.tsx (−3,645 lines, 223 tests) #404

Merged

openasocket mentioned this pull request Feb 19, 2026

fix: auto-scroll follows thinking streams + Codex tool display #410

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Major Feature: Web Interface#4

Major Feature: Web Interface#4
pedramamini merged 74 commits intomainfrom
web-interface

pedramamini commented Nov 27, 2025

Uh oh!

claude bot commented Nov 28, 2025

Uh oh!

claude bot commented Nov 28, 2025

Uh oh!

claude bot commented Nov 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

Conversation

pedramamini commented Nov 27, 2025

Uh oh!

claude bot commented Nov 28, 2025

PR #4 Review: Web Interface Implementation

Executive Summary

CRITICAL SECURITY ISSUES (Must Fix Before Merge)

CRIT-1: Insufficient Authentication Model

CRIT-2: No CSRF Protection

CRIT-3: Command Injection Risk

HIGH-1: WebSocket Input Validation

HIGH-2: Inadequate Rate Limiting

MEDIUM PRIORITY ISSUES

MED-1: Missing Security Headers

MED-2: Memory Leak Risks

MED-3: No HTTPS Support

MED-4: localStorage Data Not Validated

POTENTIAL BUGS

BUG-1: Race Condition in Session State Updates

BUG-2: WebSocket Reconnection Issues

BUG-3: Missing try-catch for JSON.parse

MISSING ELEMENTS

No Tests

No Error Boundaries

No Documentation

POSITIVE HIGHLIGHTS

ACTION ITEMS

Must Fix Before Merge:

Should Fix Soon:

Nice to Have:

FINAL VERDICT

Uh oh!

claude bot commented Nov 28, 2025

Code Review: PR #4 - Web Interface for Maestro

Executive Summary

CRITICAL ISSUES (Must Fix Before Merge)

1. CORS Configuration Too Permissive - SEVERITY: CRITICAL

2. Server Binds to 0.0.0.0 - SEVERITY: HIGH

3. Authentication Relies Solely on URL Token - SEVERITY: HIGH

4. Command Injection Risk - SEVERITY: HIGH

5. Rate Limiting Insufficient - SEVERITY: MEDIUM-HIGH

IMPORTANT ISSUES (Should Fix)

POSITIVE ASPECTS

TEST COVERAGE GAPS

DOCUMENTATION NEEDS

FINAL VERDICT: REQUEST CHANGES

Uh oh!

claude bot commented Nov 28, 2025

Pull Request Review: Web Interface Implementation

Overview

Critical Security Concerns

🔴 HIGH SEVERITY: Security Token in URL Path

🔴 MEDIUM SEVERITY: No CSRF Protection

🟡 MEDIUM SEVERITY: Command Injection Surface Expanded

🟡 MEDIUM SEVERITY: WebSocket Message Validation

Security Best Practices Observed ✅

Code Quality Issues

🟡 High Complexity Functions

🟡 Inconsistent Error Handling

🟡 Missing Input Validation

Performance Considerations

🟡 Broadcast Efficiency

🟡 Memory Management

Testing Gaps

🔴 No Tests Included

Documentation Issues

🟡 Missing Security Documentation

Architecture Concerns

🟡 Tight Coupling Between Desktop and Web

Mobile-Specific Issues

🟡 PWA Service Worker Caching

Positive Aspects ⭐

Recommendations Summary

Before Merge (Blockers):

High Priority (Post-Merge):

Medium Priority:

Additional Files to Review

Final Assessment

Uh oh!

Uh oh!