From 2ad8d490b7ec85bcd646d3df555c1d72dc577cc4 Mon Sep 17 00:00:00 2001 From: "Fabian @ Blax Software" Date: Fri, 17 Apr 2026 11:03:02 +0200 Subject: [PATCH] fix: harden IPC callbacks and decouple auth lookup via configurable resolver --- config/websockets.php | 16 ++++++ src/Server/Loggers/Logger.php | 2 +- src/Server/WebSocketHandler.php | 11 ++-- src/Websocket/Controller.php | 49 ++++++++++++++++-- src/Websocket/ControllerResolver.php | 12 ++--- src/Websocket/Handler.php | 77 +++++++++++++++++++++++----- 6 files changed, 138 insertions(+), 29 deletions(-) diff --git a/config/websockets.php b/config/websockets.php index e69aafc..cff88fd 100644 --- a/config/websockets.php +++ b/config/websockets.php @@ -39,6 +39,22 @@ return [ */ 'introspection' => env('WEBSOCKET_INTROSPECTION', false), + /* + |-------------------------------------------------------------------------- + | Auth Resolver + |-------------------------------------------------------------------------- + | + | Callable that receives the `authtoken` string from an incoming message and + | returns either a user model (Authenticatable) or null. Used by Controller + | self-heal when `need_auth = true` and the connection has no user yet. + | + | Defaults to Laravel Sanctum lookup when Sanctum is installed. Applications + | can supply their own by binding `websockets.auth_resolver` in the container + | or by setting this to a `[Class::class, 'method']` / Closure reference. + | + */ + 'auth_resolver' => null, + /* |-------------------------------------------------------------------------- | Max Concurrent Children (Fork Limit) diff --git a/src/Server/Loggers/Logger.php b/src/Server/Loggers/Logger.php index 48955c5..6e2bd13 100644 --- a/src/Server/Loggers/Logger.php +++ b/src/Server/Loggers/Logger.php @@ -146,7 +146,7 @@ class Logger try { $channel = config('logging.channels.websocket') ? 'websocket' : null; - Log::channel($channel)->log($logLevel, '[WebSocket] '.$message); + Log::channel($channel)->log($logLevel, '[WebSocket] ' . $message); } catch (\Throwable) { // Logging must never crash the WS server } diff --git a/src/Server/WebSocketHandler.php b/src/Server/WebSocketHandler.php index 740c25a..fa9412f 100644 --- a/src/Server/WebSocketHandler.php +++ b/src/Server/WebSocketHandler.php @@ -88,7 +88,7 @@ class WebSocketHandler implements MessageComponentInterface public function onMessage(ConnectionInterface $connection, MessageInterface $message) { if (! isset($connection->app)) { - $this->wsLog('warning', 'Message dropped: connection has no app (likely failed auth). Payload: '.Str::limit($message->getPayload(), 200)); + $this->wsLog('warning', 'Message dropped: connection has no app (likely failed auth). Payload: ' . Str::limit($message->getPayload(), 200)); return; } @@ -129,7 +129,10 @@ class WebSocketHandler implements MessageComponentInterface $ch = $this->channelManager->find($connection->app->id, $channel); if ($ch) { $ch->broadcastToEveryoneExcept( - $payload, $connection->socketId, $connection->app->id, false + $payload, + $connection->socketId, + $connection->app->id, + false ); } } @@ -210,7 +213,7 @@ class WebSocketHandler implements MessageComponentInterface App::findByKey($appKey) ->then(function ($app) use ($appKey, $connection, $deferred) { if (! $app) { - $this->wsLog('error', "Unknown app key: '{$appKey}'. Check that PUSHER_APP_KEY in .env matches the key used by the frontend. Configured apps: ".implode(', ', array_map(fn ($a) => $a['key'] ?? 'null', config('websockets.apps', [])))); + $this->wsLog('error', "Unknown app key: '{$appKey}'. Check that PUSHER_APP_KEY in .env matches the key used by the frontend. Configured apps: " . implode(', ', array_map(fn($a) => $a['key'] ?? 'null', config('websockets.apps', [])))); $deferred->reject(new Exceptions\UnknownAppKey($appKey)); } @@ -325,7 +328,7 @@ class WebSocketHandler implements MessageComponentInterface { try { $channel = config('logging.channels.websocket') ? 'websocket' : config('logging.default'); - Log::channel($channel)->log($level, '[WebSocket] '.$message); + Log::channel($channel)->log($level, '[WebSocket] ' . $message); } catch (\Throwable) { // Logging must never break the server } diff --git a/src/Websocket/Controller.php b/src/Websocket/Controller.php index 7822f48..745b338 100644 --- a/src/Websocket/Controller.php +++ b/src/Websocket/Controller.php @@ -10,7 +10,6 @@ use BlaxSoftware\LaravelWebSockets\Channels\PresenceChannel; use BlaxSoftware\LaravelWebSockets\Channels\PrivateChannel; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Log; -use Laravel\Sanctum\PersonalAccessToken; use Ratchet\ConnectionInterface; class Controller @@ -98,9 +97,9 @@ class Controller $authtoken = @$message['data']['authtoken'] ?? null; if ($authtoken) { try { - $tokenRecord = PersonalAccessToken::findToken($authtoken); - if ($tokenRecord?->tokenable) { - $connection->user = $tokenRecord->tokenable; + $resolved = self::resolveUserFromToken($authtoken); + if ($resolved) { + $connection->user = $resolved; Auth::login($connection->user); // Clear parent's stale auth cache so it re-authenticates if ($connection instanceof MockConnectionSocketPair) { @@ -166,6 +165,48 @@ class Controller } } + /** + * Resolve a user from an authtoken string. First tries the configured + * `websockets.auth_resolver` callable; falls back to Laravel Sanctum's + * `PersonalAccessToken::findToken()` if the class exists. + * + * Returns an Authenticatable user or null. + */ + protected static function resolveUserFromToken(string $authtoken) + { + // 1. Configured resolver (closure or [Class, method]) + $resolver = config('websockets.auth_resolver'); + if ($resolver && is_callable($resolver)) { + $user = $resolver($authtoken); + if ($user) { + return $user; + } + } + + // 2. Container binding (useful for class-based resolvers) + if (app()->bound('websockets.auth_resolver')) { + $bound = app('websockets.auth_resolver'); + if (is_callable($bound)) { + $user = $bound($authtoken); + if ($user) { + return $user; + } + } + } + + // 3. Fallback to Sanctum if available (string class name to avoid + // autoload errors when the package isn't installed) + $sanctumClass = 'Laravel\\Sanctum\\PersonalAccessToken'; + if (class_exists($sanctumClass)) { + $tokenRecord = $sanctumClass::findToken($authtoken); + if ($tokenRecord?->tokenable) { + return $tokenRecord->tokenable; + } + } + + return null; + } + final public function progress( mixed $payload = null, ?string $event = null, diff --git a/src/Websocket/ControllerResolver.php b/src/Websocket/ControllerResolver.php index b42f592..da0d0e1 100644 --- a/src/Websocket/ControllerResolver.php +++ b/src/Websocket/ControllerResolver.php @@ -105,11 +105,11 @@ class ControllerResolver private static function resolveWithHotReload(string $eventPrefix): ?string { $directName = self::kebabToPascal($eventPrefix) . 'Controller'; - + // Try app namespace first $appClass = self::APP_NAMESPACE . $directName; $appFile = self::getControllerFilePath($appClass); - + if ($appFile && file_exists($appFile)) { self::invalidateAndReload($appFile); if (class_exists($appClass, true)) { @@ -120,7 +120,7 @@ class ControllerResolver // Try vendor namespace $vendorClass = self::VENDOR_NAMESPACE . $directName; $vendorFile = self::getControllerFilePath($vendorClass); - + if ($vendorFile && file_exists($vendorFile)) { self::invalidateAndReload($vendorFile); if (class_exists($vendorClass, true)) { @@ -141,7 +141,7 @@ class ControllerResolver // Try app namespace with folder $appClass = self::APP_NAMESPACE . str_replace('/', '\\', $folder) . '\\' . $name; $appFile = self::getControllerFilePath($appClass); - + if ($appFile && file_exists($appFile)) { self::invalidateAndReload($appFile); if (class_exists($appClass, true)) { @@ -178,7 +178,7 @@ class ControllerResolver return $appPath . '/' . $relativePath . '.php'; } } - + // For vendor namespace if (str_starts_with($className, self::VENDOR_NAMESPACE)) { $relativePath = str_replace(self::VENDOR_NAMESPACE, '', $className); @@ -199,7 +199,7 @@ class ControllerResolver // e.g., 'app' → '\App\Websocket\Controllers\AppController' $directName = self::kebabToPascal($eventPrefix) . 'Controller'; $appClass = self::APP_NAMESPACE . $directName; - + // class_exists with autoload=true is fast for already-loaded classes if (class_exists($appClass, true)) { return $appClass; diff --git a/src/Websocket/Handler.php b/src/Websocket/Handler.php index 4aa9487..03213db 100644 --- a/src/Websocket/Handler.php +++ b/src/Websocket/Handler.php @@ -686,9 +686,40 @@ class Handler implements MessageComponentInterface $startTime = microtime(true); $ipc->setupParent( - // onData callback - called INSTANTLY when child sends + // onData callback - called INSTANTLY when child sends. + // CRITICAL: this callback runs inside the React event loop. Any + // uncaught throwable here would propagate up through ExtEvLoop and + // crash the entire WebSocket server (supervisor would then restart + // it, dropping every connected client). We must catch and log. function ($data) use ($connection, $message, $startTime) { - $this->handleChildData($connection, $message, $data); + try { + $this->handleChildData($connection, $message, $data); + } catch (\Throwable $e) { + Log::channel('websocket')->error('handleChildData failed: ' . $e->getMessage(), [ + 'event' => $message['event'] ?? 'unknown', + 'file' => $e->getFile() . ':' . $e->getLine(), + 'data_preview' => is_string($data) ? substr($data, 0, 200) : gettype($data), + ]); + + if (app()->bound('sentry')) { + try { + app('sentry')->captureException($e); + } catch (\Throwable $_) { + // Sentry capture failed — never let logging crash the loop. + } + } + + // Best-effort: notify the client so it doesn't hang forever. + try { + $connection->send(json_encode([ + 'event' => ($message['event'] ?? 'unknown') . ':error', + 'data' => ['message' => 'Internal server error'], + 'channel' => $message['channel'] ?? null, + ])); + } catch (\Throwable $_) { + // Connection may already be gone — swallow. + } + } // Log latency for debugging $elapsed = (microtime(true) - $startTime) * 1000; @@ -696,14 +727,21 @@ class Handler implements MessageComponentInterface Log::channel('websocket')->debug('IPC latency: ' . round($elapsed, 2) . 'ms'); } }, - // onClose callback - child process ended + // onClose callback - child process ended. + // Same isolation rules apply: must not throw out of the loop. function () { - // Cleanup zombie process - pcntl_waitpid(-1, $status, WNOHANG); + try { + // Cleanup zombie process + pcntl_waitpid(-1, $status, WNOHANG); - // Free up a child slot and process any queued messages - $this->activeChildCount = max(0, $this->activeChildCount - 1); - $this->processDeferredMessages(); + // Free up a child slot and process any queued messages + $this->activeChildCount = max(0, $this->activeChildCount - 1); + $this->processDeferredMessages(); + } catch (\Throwable $e) { + Log::channel('websocket')->error('IPC onClose failed: ' . $e->getMessage(), [ + 'file' => $e->getFile() . ':' . $e->getLine(), + ]); + } } ); } @@ -841,8 +879,14 @@ class Handler implements MessageComponentInterface unset($connection->authLoaded); $connection->user = null; - // Clear any custom connection data that was stored via C:SET - foreach (($connection->_connectionDataKeys ?? []) as $key => $_) { + // Clear any custom connection data that was stored via C:SET. + // Read-modify-write the tracker via a local copy because the + // connection may be wrapped in a decorator (e.g. ConnectionLogger) + // whose __get returns by value — direct array mutation on the + // overloaded property would raise "Indirect modification has no + // effect" and Laravel's error handler turns that into a fatal. + $keys = $connection->_connectionDataKeys ?? []; + foreach ($keys as $key => $_) { unset($connection->$key); } $connection->_connectionDataKeys = []; @@ -854,15 +898,20 @@ class Handler implements MessageComponentInterface $key = substr($rest, 0, $pos); $value = json_decode(substr($rest, $pos + 1)); $connection->$key = $value; - $connection->_connectionDataKeys ??= []; - $connection->_connectionDataKeys[$key] = true; + + // Read-modify-write via local copy (see note above). + $keys = $connection->_connectionDataKeys ?? []; + $keys[$key] = true; + $connection->_connectionDataKeys = $keys; } } elseif (str_starts_with($op, 'DEL:')) { // C:DEL:key $key = substr($op, 4); unset($connection->$key); - if (isset($connection->_connectionDataKeys[$key])) { - unset($connection->_connectionDataKeys[$key]); + $keys = $connection->_connectionDataKeys ?? []; + if (isset($keys[$key])) { + unset($keys[$key]); + $connection->_connectionDataKeys = $keys; } }