feat: watch for serve config that conflicts with WebGUI - #37

src/usr/local/emhttp/plugins/tailscale/include/Tailscale/System.php (2)

67-98: ⚠️ Potential issue

Add error handling for serve config API response.

The checkServeConfig method lacks proper error handling for potential API failures and type checking for the configuration values.

Improve the error handling when retrieving and processing the serve configuration:

 $localAPI    = new LocalAPI();
 $serveConfig = $localAPI->getServeConfig();

-$tcpConfig = $serveConfig->TCP ?? array();
+// Check if we got a valid response
+if (!isset($serveConfig) || !is_object($serveConfig)) {
+    Utils::logmsg("Failed to get valid serve config");
+    return;
+}
+
+$tcpConfig = $serveConfig->TCP ?? array();
+
+// Ensure tcpConfig is iterable
+if (!is_array($tcpConfig) && !is_object($tcpConfig)) {
+    Utils::logmsg("TCP config is not iterable");
+    return;
+}

🧰 Tools

🪛 PHPStan (2.0.3)

85-85: Argument of an invalid type mixed supplied for foreach, only iterables are supported.

(foreach.nonIterable)

---

86-86: Parameter #1 $value of function intval expects array|bool|float|int|resource|string|null, mixed given.

(argument.type)

---

85-97: 🛠️ Refactor suggestion

Improve type checking and add user notification.

The code currently doesn't verify the port key types before processing and lacks user notification when conflicts are resolved.

 foreach ($tcpConfig as $key => $val) {
-    $configPort = intval($key);
+    // Ensure the key is a valid port number
+    if (!is_numeric($key)) {
+        Utils::logmsg("Invalid TCP port key: {$key}");
+        continue;
+    }
+    
+    $configPort = intval($key);

     if ($configPort == $httpPort || $configPort == $httpsPort) {
         Utils::logmsg("Serve TCP Port {$configPort} conflicts with WebGUI, removing");
         $localAPI->resetServeConfig();
+        // Notify the user before restarting
+        $event = "Tailscale Serve Config Reset";
+        $command = self::NOTIFY_COMMAND . " -e " . escapeshellarg($event) . 
+                   " -s " . escapeshellarg("WebGUI port conflict resolved") . 
+                   " -d " . escapeshellarg("Removed Tailscale serve config using port {$configPort}") . 
+                   " -i \"warning\" -x 2>/dev/null";
+        exec($command);
         Utils::run_command(self::RESTART_COMMAND);

         return;

🧰 Tools

🪛 PHPStan (2.0.3)

85-85: Argument of an invalid type mixed supplied for foreach, only iterables are supported.

(foreach.nonIterable)

---

86-86: Parameter #1 $value of function intval expects array|bool|float|int|resource|string|null, mixed given.

(argument.type)

src/usr/local/emhttp/plugins/tailscale/include/Tailscale/System.php (2)

71-73: Improve config file error handling.

Consider adding more robust error handling for parsing the config file and handling potential missing values.

The current implementation correctly checks for scalar values before using intval(), but could be more defensive by checking if the config file was read successfully at all:

 $ident_config = parse_ini_file("/boot/config/ident.cfg") ?: array();

+// Log if we couldn't read the config file
+if (empty($ident_config)) {
+    Utils::logmsg("Could not read ident.cfg, using default WebGUI ports");
+}
+
 $httpPort = isset($ident_config['PORT']) && is_scalar($ident_config['PORT'])
             ? intval($ident_config['PORT']) : 80;

Also applies to: 76-77

---

95-95: Consider using the rate limit parameter in Utils::logmsg.

The logging check is being done in the code, but the rateLimit parameter of Utils::logmsg could be used instead for consistency.

-Utils::logmsg("Checked for WebGUI conflict with serve TCP Port {$configPort}", false, true);
+Utils::logmsg("Checked for WebGUI conflict with serve TCP Port {$configPort}", false, true);

While this doesn't change the functionality, it clarifies that this log message is intentionally rate-limited, matching the pattern used in the checkWebgui method.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)