Conversation
WalkthroughThis update introduces support for the Tailscale Funnel feature in the Unraid Tailscale plugin, including backend logic, UI controls, and localized warnings. It removes usage data collection, donation options, and the help system. License headers are added throughout the codebase. Locale files are updated to reflect the new Funnel feature and related warnings. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebGUI
participant SettingsPHP
participant ConfigPHP
participant UtilsPHP
participant LocalAPI
participant ServeConfig
User->>WebGUI: Change Funnel/Subnet/DNS setting
WebGUI->>SettingsPHP: onchange event
SettingsPHP->>WebGUI: showSettingWarning()
alt User confirms warning
WebGUI->>SettingsPHP: Submit change
SettingsPHP->>ConfigPHP: POST action (e.g., funnel-port)
ConfigPHP->>UtilsPHP: Check Nginx config, assigned ports
ConfigPHP->>LocalAPI: setServeConfig(ServeConfig)
LocalAPI->>ServeConfig: Build config object
LocalAPI->>Tailscale: POST serve config
LocalAPI->>ConfigPHP: Return result
ConfigPHP->>SettingsPHP: Update UI
else User cancels
WebGUI->>WebGUI: Revert selection
end
sequenceDiagram
participant System
participant Config
participant Info
participant Utils
participant LocalAPI
System->>Info: getAllowedFunnelPorts()
Info->>System: Return available ports
System->>Utils: get_assigned_ports()
Utils->>System: Return assigned ports
System->>Config: Check AllowFunnel property
alt AllowFunnel is false but serve config enables Funnel
System->>LocalAPI: setServeConfig(reset)
end
sequenceDiagram
participant User
participant WebGUI
participant TailscaleConfigJS
participant BackendPHP
User->>WebGUI: Select funnel port
WebGUI->>TailscaleConfigJS: setFunnelPort()
TailscaleConfigJS->>BackendPHP: POST funnel-port with port
BackendPHP->>BackendPHP: Update serve config
BackendPHP->>WebGUI: Refresh config display
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (31)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (21)
🧰 Additional context used🧬 Code Graph Analysis (1)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
🪛 PHPMD (2.15.0)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php437-437: Avoid unused local variables such as '$value'. (Unused Code Rules) (UnusedLocalVariable) 🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (22)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/DashboardInfo.php (1)
5-20: Consider relocating the license header above the namespace declarationPSR-12 recommends that file-level comments (including license blocks) appear immediately after the opening
<?phptag and before any code, such as anamespacestatement. This keeps the header visible in generated API docs and prevents tools that read the first declaration from skipping it.<?php -namespace Tailscale; - -/* +/* … */ + +namespace Tailscale;src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/PeerStatus.php (1)
5-20: GPL header looks good – consider adding an SPDX identifier for brevityThe full GPL-3.0 header is correctly inserted and matches other files. For consistency and to simplify future license scanning, you could prepend a single-line SPDX tag (e.g.
// SPDX-License-Identifier: GPL-3.0-or-later) and optionally trim the verbose block.This is entirely optional; feel free to keep the block as-is if project policy prefers the long form.
src/usr/local/emhttp/plugins/tailscale/include/Pages/Lock.php (1)
5-20: Add SPDX identifier for tooling friendlinessThe GPL-3 header is great, but most scanners & IDEs rely on an SPDX tag to recognise it automatically.
Consider inserting the identifier at the top of the block:/* + * SPDX-License-Identifier: GPL-3.0-or-later * * Copyright (C) 2025 Derek Kasersrc/usr/local/emhttp/plugins/tailscale/include/page.php (1)
5-20: License block sits after thenamespace– consider moving it to the very top.Many license-scanning and doc-generation tools expect the copyright / SPDX header to be the first non-shebang content in the file. Placing it after the
namespacecan cause those tools to miss it or mis-classify the file’s license.
If nothing else depends on its current position, shift the comment block above thenamespacedeclaration.src/usr/local/emhttp/plugins/tailscale/include/Pages/Main.php (1)
5-20: Consider moving the GPL header before thenamespacedeclaration and add an SPDX identifierPlacing the licence block at the very top of the file (immediately after
<?php) is the de-facto convention; it keeps licensing metadata clustered and prevents any executable code from appearing before it. While PHP happily ignores the block comment here, tools that scan only the first N lines may miss it. Adding a single-line SPDX tag (// SPDX-License-Identifier: GPL-3.0-or-later) also improves automated compliance tracking.Example patch:
-namespace Tailscale; - -/* +/* // SPDX-License-Identifier: GPL-3.0-or-later + namespace Tailscale;src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LockInfo.php (1)
5-20: Move the GPL header in front of the namespace for better tool-compatibilityMost PHP style guides (e.g. PSR-12) and many licence-scanning / doc-generation tools expect the file-level licence block to be the very first element after
<?php, beforenamespaceordeclare(strict_types=1);. Keeping it inside the namespace can cause those tools to miss the header.<?php -namespace Tailscale; - -/* +/* ... */ +namespace Tailscale;src/usr/local/emhttp/plugins/tailscale/include/Pages/Status.php (1)
5-20: Move license block above thenamespacefor standard header placementMost PHP style guides (PSR-12, Symfony, etc.) expect the file-level comment header to appear immediately after
<?php, before any declarations. Placing it after thenamespacecan break some license-scanning tools and makes the file slightly non-conformant to common conventions.<?php + +/* + Copyright (C) 2025 Derek Kaser + ... +*/ + namespace Tailscale; - -/* - Copyright (C) 2025 Derek Kaser - ... -*/This is a tiny nit, but aligning with conventions avoids tooling surprises later.
src/usr/local/emhttp/plugins/tailscale/include/Pages/Dashboard.php (1)
5-20: Move license header before the namespace to comply with PSR-12 file structurePSR-12 (§2.2) requires a file-level docblock—including license headers—to appear right after the opening
<?phptag and before thenamespacedeclaration.
Keeping it where it is may trip automated license scanners or code-style tools.<?php -namespace Tailscale; - -/* +/* Copyright (C) 2025 Derek Kaser @@ along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +namespace Tailscale;src/usr/local/emhttp/plugins/tailscale/include/data/Lock.php (1)
5-20: Add SPDX identifier for streamlined license detectionConsider adding a single-line SPDX tag before (or replacing) the verbose header so automated scanners can unambiguously detect the license:
<?php // SPDX-License-Identifier: GPL-3.0-or-laterThis is purely optional but increasingly common across open-source projects.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Warning.php (1)
5-20: Move the GPL header above thenamespacedeclaration for PSR-12 compliancePSR-12 requires that the namespace declaration be the first non-declare statement after the opening tag and any file-level docblocks/comments. Placing the license block after the
namespacetechnically breaks that ordering and may trigger linters.Suggested fix:
<?php - -namespace Tailscale; - -/* +/* Copyright (C) 2025 Derek Kaser … */ + +namespace Tailscale;src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ConnectionInfo.php (1)
5-20: Move license header to top of file (beforenamespace)PSR-12 dictates that the file-level docblock (license/header) must appear immediately after the
<?phpopening tag and before thenamespacedeclaration. Placing it inside the namespace may confuse tooling that looks for header metadata. Proposed re-ordering:<?php - -namespace Tailscale; - -/* +/* Copyright (C) 2025 Derek Kaser @@ along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +namespace Tailscale;src/usr/local/emhttp/plugins/tailscale/diagnostics.json (1)
74-75: Hard-coding an external upload endpoint can raise privacy & maintainability red flagsBefore merging, please confirm:
- Users are explicitly informed (UI + docs) that diagnostics will be POSTed to
https://plugin-diagnostics.edacerton.win/, and can opt-out.- The endpoint is owned/controlled by the project and served via valid TLS.
- A future change does not silently break older plugin versions if the domain is moved—consider loading the URL from a central config (e.g.
/boot/config/plugins/tailscale.cfg) instead of baking it into the JSON.If all three points are already covered elsewhere, feel free to disregard.
src/usr/local/emhttp/plugins/tailscale/pre-startup.php (1)
6-21: Add SPDX identifier for better tooling supportThe GPL block is perfect, but many compliance tools (Reuse, ScanCode, etc.) look for a one-liner SPDX tag. Adding it right above the block makes the license machine-readable without changing behavior.
+// SPDX-License-Identifier: GPL-3.0-or-later /* Copyright (C) 2025 Derek Kasersrc/usr/local/emhttp/plugins/tailscale/daily.php (1)
4-6: Place the file-level license block before thenamespacefor better tooling compatibility
Some static analysers and doc generators only recognise a file-level doc-block when it appears immediately after<?php. Consider moving the GPL header above thenamespacedeclaration to avoid it being treated as an in-namespace comment.@@ -<?php - -namespace Tailscale; +<?php +/* + (GPL v3 header here …) +*/ + +namespace Tailscale;src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Watcher.php (1)
5-20: Move license block above the namespace & add an SPDX identifierMost tooling that scans for license compliance expects the notice to appear immediately after the
<?phpdeclaration (before any code, including thenamespaceline).
While you’re touching this header, consider adding a single‐line SPDX tag to make automated license detection easier.Example fix:
<?php -namespace Tailscale; - +// SPDX-License-Identifier: GPL-3.0-or-later + /* Copyright (C) 2025 Derek Kaser @@ along with this program. If not, see <https://www.gnu.org/licenses/>. */ + +namespace Tailscale;This keeps the file compliant with common scanners and avoids accidental exclusion of the header from generated documentation.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ServeConfig.php (2)
24-25:$configshould be typed asarrayinstead ofstdClass
stdClassbuys no real advantage here (there is no behaviour) while complicating JSON serialisation, comparison, and testability. A plain associative array is cheaper, serialises trivially withjson_encode, and matches the TStailscale serveJSON examples.
46-49: Expose atoJson()helper for callersEvery caller ultimately needs JSON; avoiding duplicated
json_encode($serveConfig->getConfig())everywhere keeps things tidy.public function getConfig(): \stdClass { return $this->config; } + +public function toJson(int $flags = 0): string +{ + return json_encode($this->config, JSON_THROW_ON_ERROR | $flags); +}src/usr/local/emhttp/plugins/tailscale/locales/zh_CN.json (1)
98-103: Long warning strings will overflow the modal on 1080pThe three newly-added warnings (
funnel,subnet,dns) are ~300 chars each. In the current CSS the text will not wrap nicely and pushes the confirmation buttons off-screen. Consider shortening or inserting<br>breaks at ~120 chars, or convert the warnings into bullet points in the UI instead of the locale.src/usr/local/emhttp/plugins/tailscale/locales/en_US.json (1)
99-103: Minor typo – “conections”
settings.context.wireguardstill says “conections”. Fixing it avoids translator churn later.- "wireguard": "Change the port used for Wireguard conections. This usually does not need to be changed.", + "wireguard": "Change the port used for Wireguard connections. This usually does not need to be changed.",src/usr/local/emhttp/plugins/tailscale/locales/it_IT.json (1)
99-103: Escape not required inside Italian warningThe back-slash in
l\\'usowill be rendered literally in the UI (l\'uso). In JSON only the backslash itself must be escaped, not a single quote.- "funnel": "... durante l\\'uso di Exit Node", + "funnel": "... durante l'uso di Exit Node",src/usr/local/emhttp/plugins/tailscale/locales/de_DE.json (1)
99-103: Double-space between “empfohlene/” and “sichere”Tiny typography issue in the new funnel warning (
"empfohlene/sichere"currently has a zero-width space + normal space). Remove the extra to keep strings consistent across languages.src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
450-464: Improve funnel port parsing robustnessThe method logic is correct but could handle edge cases better.
Consider this more robust implementation:
public function getFunnelPort(): ?int { - if (isset($this->serve->AllowFunnel) && $this->serve->AllowFunnel) { + if (isset($this->serve->AllowFunnel) && is_object($this->serve->AllowFunnel)) { $funnelKeys = array_keys((array)$this->serve->AllowFunnel); if (count($funnelKeys) > 0) { $funnelKey = $funnelKeys[0]; $parts = explode(":", strval($funnelKey)); - if (count($parts) == 2 && is_numeric($parts[1])) { + if (count($parts) === 2 && is_numeric($parts[1]) && intval($parts[1]) > 0) { return intval($parts[1]); } } } return null; // Funnel not enabled }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (44)
src/usr/local/emhttp/plugins/tailscale/Tailscale-5-Help.page(0 hunks)src/usr/local/emhttp/plugins/tailscale/daily.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/diagnostics.json(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Dashboard.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Help.php(0 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Info.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Lock.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Main.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php(3 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Status.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php(3 hunks)src/usr/local/emhttp/plugins/tailscale/include/data/Config.php(3 hunks)src/usr/local/emhttp/plugins/tailscale/include/data/Lock.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/data/Status.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/page.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/locales/de_DE.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/en_US.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/es_ES.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/fr_FR.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/it_IT.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/ja_JP.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/ko_KR.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/nl_NL.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/no_NO.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/pl_PL.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/pt_BR.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/pt_PT.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/sv_SE.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/uk_UA.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/locales/zh_CN.json(5 hunks)src/usr/local/emhttp/plugins/tailscale/pre-startup.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Config.php(3 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ConnectionInfo.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/DashboardInfo.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php(4 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php(3 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LockInfo.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/PeerStatus.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ServeConfig.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/StatusInfo.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php(2 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php(2 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Warning.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Watcher.php(1 hunks)
💤 Files with no reviewable changes (2)
- src/usr/local/emhttp/plugins/tailscale/Tailscale-5-Help.page
- src/usr/local/emhttp/plugins/tailscale/include/Pages/Help.php
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php (3)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Config.php (1)
Config(22-60)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php (2)
Utils(22-189)logwrap(71-78)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
resetServeConfig(104-107)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ServeConfig.php (2)
ServeConfig(22-50)getConfig(46-49)
src/usr/local/emhttp/plugins/tailscale/include/data/Config.php (4)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php (3)
Utils(22-189)isFunnelAllowed(131-148)get_assigned_ports(156-188)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (4)
getAllowedFunnelPorts(431-448)tr(66-69)getFunnelPort(450-464)getDNSName(466-473)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (2)
resetServeConfig(104-107)setServeConfig(109-112)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/ServeConfig.php (2)
ServeConfig(22-50)getConfig(46-49)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
getServeConfig(99-102)
🪛 PHPMD (2.15.0)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php
437-437: Avoid unused local variables such as '$value'. (Unused Code Rules)
(UnusedLocalVariable)
🔇 Additional comments (46)
src/usr/local/emhttp/plugins/tailscale/daily.php (1)
6-21: License header addition looks good and aligns with GPL v3 requirements
No further action needed for the legal text itself.src/usr/local/emhttp/plugins/tailscale/include/Pages/Info.php (1)
5-20: License header addition looks good.The GPL v3 license header is properly formatted with correct copyright notice and license terms reference.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (3)
5-20: License header addition looks good.The GPL v3 license header is properly formatted with correct copyright notice and license terms reference.
48-48: Good improvement to JSON encoding.Adding
JSON_UNESCAPED_SLASHESprevents unnecessary escaping of slashes in JSON, which is particularly important for URLs and paths in serve configurations.
109-112: New setServeConfig method is well-implemented.The method follows the established pattern for API calls and properly integrates with the ServeConfig class. The implementation correctly passes the configuration object to the local API.
src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php (3)
5-20: License header addition looks good.The GPL v3 license header is properly formatted with correct copyright notice and license terms reference.
71-73: Good defensive programming for Select2 initialization.The conditional check ensures the Select2 widget is only initialized if the
funnelPortSelectelement exists, preventing potential JavaScript errors.
106-111: setFunnelPort function follows established patterns.The implementation is consistent with other similar async functions in the file:
- Shows spinner and disables controls during operation
- Sends POST request with appropriate action and data
- Refreshes configuration display after completion
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Config.php (3)
5-20: License header addition looks good.The GPL v3 license header is properly formatted with correct copyright notice and license terms reference.
31-31: AllowFunnel property declaration is correct.The property is properly declared as a public boolean, consistent with other configuration properties in the class.
54-54: AllowFunnel initialization follows established patterns.The initialization uses the same pattern as other configuration properties, with a secure default of false and proper type casting with
boolval().src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/System.php (2)
5-20: License header addition looks good.The GPL v3 license header is properly formatted with correct copyright notice and license terms reference.
107-112: Funnel validation logic enhances security and consistency.The added logic properly enforces consistency between the serve configuration and main configuration:
- Checks if serve config has AllowFunnel enabled but main config doesn't allow it
- Provides clear logging for the mismatch scenario
- Resets serve config to maintain security posture
- Uses proper object instantiation and property access
This prevents potential security issues where funnel might be enabled despite being disabled in the main configuration.
src/usr/local/emhttp/plugins/tailscale/include/Pages/Settings.php (2)
5-20: LGTM! License header addition is appropriate.The GPLv3 license header is properly formatted and provides necessary copyright and licensing information.
119-160: Good implementation of funnel setting with conditional display.The restructured form properly adds IDs and event handlers for the warning system. The conditional display of the funnel setting using
Utils::isFunnelAllowed()ensures the feature is only shown when safe to use.src/usr/local/emhttp/plugins/tailscale/locales/pt_BR.json (5)
19-20: LGTM! New UI strings properly added.The addition of "cancel" and "accept" strings supports the new warning dialog functionality.
30-30: LGTM! Warning string addition is appropriate.The generic "warning" label supports the new alert dialogs.
49-49: LGTM! Funnel setting translation is clear.The translation "Permitir Funnel do Tailscale" clearly conveys the funnel allowance setting.
86-87: LGTM! Funnel info strings are descriptive.The translations for "funnel_port" and "port_in_use" provide clear context for the UI.
99-103: LGTM! Comprehensive warning messages properly implemented.The warning messages provide detailed, appropriate cautions for funnel, subnet, DNS, and general usage. The translations are clear and maintain the security-focused tone of the original warnings.
src/usr/local/emhttp/plugins/tailscale/locales/ja_JP.json (4)
19-20: LGTM! UI strings properly localized.The Japanese translations for "cancel" (キャンセル) and "accept" (承認) are accurate and appropriate.
49-49: LGTM! Funnel setting translation is clear.The translation "Tailscale Funnel を許可" accurately conveys the funnel allowance setting.
86-87: LGTM! Funnel info translations are descriptive.The Japanese translations provide clear context for the funnel port and port usage information.
99-103: LGTM! Security warnings are comprehensive and well-translated.The warning messages are detailed and maintain the security-focused tone. The Japanese translations clearly convey the risks and recommendations for funnel, subnet, and DNS settings.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php (1)
5-20: LGTM! License header addition is appropriate.The GPLv3 license header is properly formatted and provides necessary legal information.
src/usr/local/emhttp/plugins/tailscale/locales/fr_FR.json (4)
19-20: LGTM! French UI strings properly implemented.The translations "Annuler" and "Accepter" are accurate and appropriate for the warning dialog buttons.
49-49: LGTM! Funnel setting translation is clear.The translation "Autoriser Tailscale Funnel" clearly conveys the funnel allowance setting.
86-87: LGTM! Funnel information strings are descriptive.The French translations for "Port Funnel pour le WebGUI" and "Port utilisé" provide clear context.
99-103: LGTM! Comprehensive security warnings properly translated.The warning messages maintain the appropriate security-focused tone and clearly communicate the risks associated with enabling funnel, subnet, and DNS settings. The French translations are accurate and comprehensive.
src/usr/local/emhttp/plugins/tailscale/locales/sv_SE.json (1)
19-20: Excellent localization support for the Funnel feature.The Swedish translations are comprehensive and well-structured, covering all necessary UI elements for the new Funnel functionality including proper warnings about security implications.
Also applies to: 30-30, 49-49, 61-62, 86-87, 99-103
src/usr/local/emhttp/plugins/tailscale/include/data/Config.php (3)
5-21: License header addition is appropriate.The GPLv3 license header properly identifies the copyright holder and license terms.
113-140: Well-implemented Funnel port selection UI with proper security checks.The implementation correctly:
- Gates functionality behind
Utils::isFunnelAllowed()security check- Filters out assigned ports to prevent conflicts
- Disables occupied ports in the dropdown
- Provides proper logging for debugging
- Includes a "none" option to disable Funnel
311-336: Secure and robust Funnel port configuration handler.The POST handler properly:
- Validates required parameters
- Handles both enabling (with port) and disabling (empty port) scenarios
- Reads system configuration safely
- Creates proper ServeConfig objects
- Includes comprehensive error handling and logging
- Uses LocalAPI for secure Tailscale configuration
src/usr/local/emhttp/plugins/tailscale/locales/es_ES.json (1)
19-20: High-quality Spanish localization for Funnel feature.The Spanish translations are comprehensive and accurately convey the security implications of the Funnel feature, maintaining consistency with other locale files.
Also applies to: 30-30, 49-49, 61-62, 86-87, 99-103
src/usr/local/emhttp/plugins/tailscale/locales/pt_PT.json (1)
19-20: Complete and accurate Portuguese localization.The Portuguese translations effectively communicate the Funnel feature functionality and security considerations, maintaining consistency with the other locale files.
Also applies to: 30-30, 49-49, 61-62, 86-87, 99-103
src/usr/local/emhttp/plugins/tailscale/locales/pl_PL.json (1)
19-20: Thorough Polish localization completing i18n support.The Polish translations provide comprehensive coverage of the Funnel feature with appropriate security warnings, maintaining consistency across all supported locales.
Also applies to: 30-30, 49-49, 61-62, 86-87, 99-103
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (3)
5-20: LGTM: License header additionThe GPLv3 license header is properly formatted and includes all required copyright and license information.
33-33: LGTM: Serve configuration property additionThe new
$serveproperty is correctly initialized from the LocalAPI'sgetServeConfig()method, providing access to Tailscale's serve configuration for Funnel functionality.Also applies to: 48-48
466-473: LGTM: DNS name method with proper error handlingThe method provides a clean interface for retrieving the DNS name with appropriate error handling using a RuntimeException when the required data is not available.
src/usr/local/emhttp/plugins/tailscale/locales/uk_UA.json (4)
19-20: LGTM: UI string additionsThe new general UI strings "cancel", "accept", and "warning" are properly translated and support the enhanced confirmation dialogs and warning displays.
Also applies to: 30-30
49-49: LGTM: Funnel setting localizationThe Funnel setting and its context description are well-translated, clearly explaining the feature's purpose and control mechanism.
Also applies to: 62-62
86-87: LGTM: Funnel information stringsThe informational strings for funnel port display and port usage status are appropriately translated.
99-103: LGTM: Comprehensive security warningsThe warning messages appropriately convey the security implications of enabling Funnel, subnet acceptance, and DNS settings. The serious tone and detailed explanations help users make informed decisions about these security-sensitive features.
src/usr/local/emhttp/plugins/tailscale/locales/nl_NL.json (1)
19-20: LGTM: Consistent Dutch localizationThe Dutch translations are well-executed and consistent with the overall localization pattern. The Funnel feature support, security warnings, and UI strings are appropriately translated with proper tone and clarity.
Also applies to: 30-30, 49-49, 62-62, 86-87, 99-103
src/usr/local/emhttp/plugins/tailscale/locales/ko_KR.json (1)
19-20: LGTM: Korean localization implementationThe Korean translations maintain consistency with other locale files and provide comprehensive support for the Funnel feature. The security warnings are appropriately detailed and the UI strings are well-translated.
Also applies to: 30-30, 49-49, 62-62, 86-87, 99-103
src/usr/local/emhttp/plugins/tailscale/locales/no_NO.json (1)
19-20: LGTM: Norwegian localization completionThe Norwegian translations complete the comprehensive international support for the Tailscale Funnel feature. All key elements are properly translated with appropriate security warnings and clear explanations.
Also applies to: 30-30, 49-49, 62-62, 86-87, 99-103
| public static function isFunnelAllowed(): bool | ||
| { | ||
| $directives = ["allow 127.0.0.1;", "allow ::1;"]; | ||
|
|
||
| $nginxConfig = file_get_contents('/etc/nginx/nginx.conf'); | ||
| if ($nginxConfig === false) { | ||
| return false; // Unable to read the nginx configuration file | ||
| } | ||
|
|
||
| // Search $nginxConfig for the allow directives. | ||
| foreach ($directives as $directive) { | ||
| if (strpos($nginxConfig, $directive) !== false) { | ||
| return false; // Directive found, funnel not safe to use | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify the logic and improve error handling.
The method logic might be confusing - it returns false when localhost allow directives are found in nginx config. Consider adding documentation to clarify why this indicates funnel is not allowed.
Improve the method with better documentation and error handling:
+ /**
+ * Check if Tailscale Funnel is allowed by verifying nginx doesn't have
+ * localhost-only restrictions that would conflict with funnel operation.
+ *
+ * @return bool True if funnel is safe to use, false otherwise
+ */
public static function isFunnelAllowed(): bool
{
$directives = ["allow 127.0.0.1;", "allow ::1;"];
$nginxConfig = file_get_contents('/etc/nginx/nginx.conf');
if ($nginxConfig === false) {
+ Utils::logwrap("Unable to read nginx configuration file", true);
return false; // Unable to read the nginx configuration file
}
- // Search $nginxConfig for the allow directives.
+ // Search for localhost-only allow directives that would conflict with funnel
foreach ($directives as $directive) {
if (strpos($nginxConfig, $directive) !== false) {
+ Utils::logwrap("Found localhost allow directive: {$directive}", true);
return false; // Directive found, funnel not safe to use
}
}
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static function isFunnelAllowed(): bool | |
| { | |
| $directives = ["allow 127.0.0.1;", "allow ::1;"]; | |
| $nginxConfig = file_get_contents('/etc/nginx/nginx.conf'); | |
| if ($nginxConfig === false) { | |
| return false; // Unable to read the nginx configuration file | |
| } | |
| // Search $nginxConfig for the allow directives. | |
| foreach ($directives as $directive) { | |
| if (strpos($nginxConfig, $directive) !== false) { | |
| return false; // Directive found, funnel not safe to use | |
| } | |
| } | |
| return true; | |
| } | |
| /** | |
| * Check if Tailscale Funnel is allowed by verifying nginx doesn't have | |
| * localhost-only restrictions that would conflict with funnel operation. | |
| * | |
| * @return bool True if funnel is safe to use, false otherwise | |
| */ | |
| public static function isFunnelAllowed(): bool | |
| { | |
| $directives = ["allow 127.0.0.1;", "allow ::1;"]; | |
| $nginxConfig = file_get_contents('/etc/nginx/nginx.conf'); | |
| if ($nginxConfig === false) { | |
| Utils::logwrap("Unable to read nginx configuration file", true); | |
| return false; // Unable to read the nginx configuration file | |
| } | |
| // Search for localhost-only allow directives that would conflict with funnel | |
| foreach ($directives as $directive) { | |
| if (strpos($nginxConfig, $directive) !== false) { | |
| Utils::logwrap("Found localhost allow directive: {$directive}", true); | |
| return false; // Directive found, funnel not safe to use | |
| } | |
| } | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php
around lines 131 to 148, the method isFunnelAllowed returns false if localhost
allow directives are found in the nginx config, which may be confusing. Add a
clear docblock comment explaining that presence of these directives means the
funnel is not safe to use. Also, improve error handling by logging or throwing
an exception if the nginx config file cannot be read instead of silently
returning false.
| public function get_assigned_ports(): array | ||
| { | ||
| $ports = array(); | ||
| $identCfg = parse_ini_file("/boot/config/ident.cfg", false, INI_SCANNER_RAW) ?: array(); | ||
| if (isset($identCfg['PORT'])) { | ||
| $ports[] = intval($identCfg['PORT']); | ||
| } | ||
| if (isset($identCfg['PORTSSL']) && isset($identCfg['USE_SSL']) && $identCfg['USE_SSL'] === 'yes') { | ||
| $ports[] = intval($identCfg['PORTSSL']); | ||
| } | ||
| if (isset($identCfg['PORTTELNET']) && isset($identCfg['USE_TELNET']) && $identCfg['USE_TELNET'] === 'yes') { | ||
| $ports[] = intval($identCfg['PORTTELNET']); | ||
| } | ||
| if (isset($identCfg['PORTSSH']) && isset($identCfg['USE_SSH']) && $identCfg['USE_SSH'] === 'yes') { | ||
| $ports[] = intval($identCfg['PORTSSH']); | ||
| } | ||
|
|
||
| // Get any open TCP ports from the system | ||
| $netstatOutput = shell_exec("netstat -tuln | grep LISTEN"); | ||
| if ($netstatOutput) { | ||
| $lines = explode("\n", trim($netstatOutput)); | ||
| foreach ($lines as $line) { | ||
| if (preg_match('/:(\d+)\s+/', $line, $matches)) { | ||
| $port = intval($matches[1]); | ||
| if ($port > 0 && $port < 65536) { | ||
| $ports[] = $port; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return array_unique($ports); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve security and error handling for shell command execution.
The method uses shell_exec with netstat which could have security implications. Also, the regex pattern should be more specific to avoid false matches.
Apply these improvements for better security and reliability:
public function get_assigned_ports(): array
{
$ports = array();
$identCfg = parse_ini_file("/boot/config/ident.cfg", false, INI_SCANNER_RAW) ?: array();
if (isset($identCfg['PORT'])) {
$ports[] = intval($identCfg['PORT']);
}
if (isset($identCfg['PORTSSL']) && isset($identCfg['USE_SSL']) && $identCfg['USE_SSL'] === 'yes') {
$ports[] = intval($identCfg['PORTSSL']);
}
if (isset($identCfg['PORTTELNET']) && isset($identCfg['USE_TELNET']) && $identCfg['USE_TELNET'] === 'yes') {
$ports[] = intval($identCfg['PORTTELNET']);
}
if (isset($identCfg['PORTSSH']) && isset($identCfg['USE_SSH']) && $identCfg['USE_SSH'] === 'yes') {
$ports[] = intval($identCfg['PORTSSH']);
}
// Get any open TCP ports from the system
- $netstatOutput = shell_exec("netstat -tuln | grep LISTEN");
+ $netstatOutput = shell_exec("netstat -tuln 2>/dev/null | grep LISTEN");
if ($netstatOutput) {
$lines = explode("\n", trim($netstatOutput));
foreach ($lines as $line) {
- if (preg_match('/:(\d+)\s+/', $line, $matches)) {
+ // Match TCP lines with port numbers more specifically
+ if (preg_match('/tcp.*?:(\d+)\s+.*?LISTEN/', $line, $matches)) {
$port = intval($matches[1]);
if ($port > 0 && $port < 65536) {
$ports[] = $port;
}
}
}
+ } else {
+ $this->logmsg("Failed to execute netstat command", true);
}
return array_unique($ports);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function get_assigned_ports(): array | |
| { | |
| $ports = array(); | |
| $identCfg = parse_ini_file("/boot/config/ident.cfg", false, INI_SCANNER_RAW) ?: array(); | |
| if (isset($identCfg['PORT'])) { | |
| $ports[] = intval($identCfg['PORT']); | |
| } | |
| if (isset($identCfg['PORTSSL']) && isset($identCfg['USE_SSL']) && $identCfg['USE_SSL'] === 'yes') { | |
| $ports[] = intval($identCfg['PORTSSL']); | |
| } | |
| if (isset($identCfg['PORTTELNET']) && isset($identCfg['USE_TELNET']) && $identCfg['USE_TELNET'] === 'yes') { | |
| $ports[] = intval($identCfg['PORTTELNET']); | |
| } | |
| if (isset($identCfg['PORTSSH']) && isset($identCfg['USE_SSH']) && $identCfg['USE_SSH'] === 'yes') { | |
| $ports[] = intval($identCfg['PORTSSH']); | |
| } | |
| // Get any open TCP ports from the system | |
| $netstatOutput = shell_exec("netstat -tuln | grep LISTEN"); | |
| if ($netstatOutput) { | |
| $lines = explode("\n", trim($netstatOutput)); | |
| foreach ($lines as $line) { | |
| if (preg_match('/:(\d+)\s+/', $line, $matches)) { | |
| $port = intval($matches[1]); | |
| if ($port > 0 && $port < 65536) { | |
| $ports[] = $port; | |
| } | |
| } | |
| } | |
| } | |
| return array_unique($ports); | |
| } | |
| public function get_assigned_ports(): array | |
| { | |
| $ports = array(); | |
| $identCfg = parse_ini_file("/boot/config/ident.cfg", false, INI_SCANNER_RAW) ?: array(); | |
| if (isset($identCfg['PORT'])) { | |
| $ports[] = intval($identCfg['PORT']); | |
| } | |
| if (isset($identCfg['PORTSSL']) && isset($identCfg['USE_SSL']) && $identCfg['USE_SSL'] === 'yes') { | |
| $ports[] = intval($identCfg['PORTSSL']); | |
| } | |
| if (isset($identCfg['PORTTELNET']) && isset($identCfg['USE_TELNET']) && $identCfg['USE_TELNET'] === 'yes') { | |
| $ports[] = intval($identCfg['PORTTELNET']); | |
| } | |
| if (isset($identCfg['PORTSSH']) && isset($identCfg['USE_SSH']) && $identCfg['USE_SSH'] === 'yes') { | |
| $ports[] = intval($identCfg['PORTSSH']); | |
| } | |
| // Get any open TCP ports from the system | |
| - $netstatOutput = shell_exec("netstat -tuln | grep LISTEN"); | |
| + $netstatOutput = shell_exec("netstat -tuln 2>/dev/null | grep LISTEN"); | |
| if ($netstatOutput) { | |
| $lines = explode("\n", trim($netstatOutput)); | |
| foreach ($lines as $line) { | |
| + // Match TCP lines with port numbers more specifically | |
| + if (preg_match('/tcp.*?:(\d+)\s+.*?LISTEN/', $line, $matches)) { | |
| $port = intval($matches[1]); | |
| if ($port > 0 && $port < 65536) { | |
| $ports[] = $port; | |
| } | |
| } | |
| } | |
| - } | |
| + } else { | |
| + $this->logmsg("Failed to execute netstat command", true); | |
| + } | |
| return array_unique($ports); | |
| } |
🤖 Prompt for AI Agents
In src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php
between lines 156 and 188, improve security and reliability by replacing the use
of shell_exec with a safer method such as proc_open or escapeshellcmd to
sanitize the command, and refine the regex pattern to more precisely match the
port number in the netstat output, ensuring it captures only valid listening TCP
ports without false positives. Also, add error handling to manage cases where
the shell command fails or returns unexpected output.
| public function getAllowedFunnelPorts(): array | ||
| { | ||
| $allowedPorts = array(); | ||
|
|
||
| if (isset($this->status->Self->CapMap)) { | ||
| $prefix = "https://tailscale.com/cap/funnel-ports?ports="; | ||
| foreach ($this->status->Self->CapMap as $cap => $value) { | ||
| if (strpos($cap, $prefix) === 0) { | ||
| $ports = explode(",", substr($cap, strlen($prefix))); | ||
| foreach ($ports as $port) { | ||
| $allowedPorts[] = intval($port); | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| return $allowedPorts; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address unused variable and improve robustness
The method correctly parses funnel port capabilities, but has an unused variable and could be more robust.
Apply this diff to fix the unused variable and improve error handling:
public function getAllowedFunnelPorts(): array
{
$allowedPorts = array();
if (isset($this->status->Self->CapMap)) {
$prefix = "https://tailscale.com/cap/funnel-ports?ports=";
- foreach ($this->status->Self->CapMap as $cap => $value) {
+ foreach ($this->status->Self->CapMap as $cap => $_) {
if (strpos($cap, $prefix) === 0) {
$ports = explode(",", substr($cap, strlen($prefix)));
foreach ($ports as $port) {
+ $port = trim($port);
+ if (is_numeric($port)) {
$allowedPorts[] = intval($port);
+ }
}
break;
}
}
}
return $allowedPorts;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public function getAllowedFunnelPorts(): array | |
| { | |
| $allowedPorts = array(); | |
| if (isset($this->status->Self->CapMap)) { | |
| $prefix = "https://tailscale.com/cap/funnel-ports?ports="; | |
| foreach ($this->status->Self->CapMap as $cap => $value) { | |
| if (strpos($cap, $prefix) === 0) { | |
| $ports = explode(",", substr($cap, strlen($prefix))); | |
| foreach ($ports as $port) { | |
| $allowedPorts[] = intval($port); | |
| } | |
| break; | |
| } | |
| } | |
| } | |
| return $allowedPorts; | |
| } | |
| public function getAllowedFunnelPorts(): array | |
| { | |
| $allowedPorts = array(); | |
| if (isset($this->status->Self->CapMap)) { | |
| $prefix = "https://tailscale.com/cap/funnel-ports?ports="; | |
| foreach ($this->status->Self->CapMap as $cap => $_) { | |
| if (strpos($cap, $prefix) === 0) { | |
| $ports = explode(",", substr($cap, strlen($prefix))); | |
| foreach ($ports as $port) { | |
| $port = trim($port); | |
| if (is_numeric($port)) { | |
| $allowedPorts[] = intval($port); | |
| } | |
| } | |
| break; | |
| } | |
| } | |
| } | |
| return $allowedPorts; | |
| } |
🧰 Tools
🪛 PHPMD (2.15.0)
437-437: Avoid unused local variables such as '$value'. (Unused Code Rules)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
In src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php
around lines 431 to 448, the method getAllowedFunnelPorts has an unused variable
and lacks robustness in parsing ports. Remove any unused variables and add
checks to ensure that port strings are valid integers before converting and
adding them to the allowedPorts array to prevent errors from malformed input.
1 participant
Summary by CodeRabbit
New Features
Localization
Bug Fixes
Chores