fix: subnet route CIDRs may not have host bits set by dkaser · Pull Request #89 · unraid/unraid-tailscale
Walkthrough
Frontend and backend CIDR validation were hardened: client adds ipaddr.js and new CIDRResult enum with precise host-bits checking; server replaces manual CIDR parsing with php-ip's IPBlock to ensure subnet route CIDRs are network addresses. A UI element was added for validation feedback.
Changes
| Cohort / File(s) | Change Summary |
|---|---|
Frontend CIDR validation logic src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php |
Added ipaddr.min.js; introduced CIDRResult enum (VALID, INVALID, EMPTY, HOSTBITS_SET); replaced isValidCIDR with ipaddr.js-based parser that derives network address and detects host bits; refactored validateTailscaleRoute to switch on CIDRResult and update UI/button state accordingly; showTailscaleConfig now invokes validation after load. |
Frontend UI feedback src/usr/local/emhttp/plugins/tailscale/include/data/Config.php |
Replaced an empty table cell with a <span id="tailscaleRouteValidation"> for route validation messages. |
Backend dependency (PHP) src/usr/local/php/unraid-tailscale-utils/composer.json |
Added rlanvin/php-ip: "^3.0" to require. |
Backend CIDR validation implementation src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Utils.php |
Replaced manual CIDR parsing/validation with PhpIP\IPBlock usage; validates by creating an IPBlock and comparing the network address against the input CIDR; added use PhpIP\IPBlock. |
CI configuration .github/workflows/lint.yml |
Enabled gmp PHP extension in composer step via php_extensions: gmp. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
- Review parity between client (ipaddr.js) and server (php-ip) validation semantics, especially host-bits detection for IPv4/IPv6.
- Inspect
validateTailscaleRouteUI state transitions and ensure the new span element ID is targeted correctly. - Verify composer update and CI
gmpaddition do not break dependency resolution or pipeline runs.
Pre-merge checks and finishing touches
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately summarizes the main change: validating that subnet route CIDRs do not have host bits set, which is the core objective of the pull request. |
| Linked Issues check | ✅ Passed | The PR implements the required validation logic in both frontend (Tailscale.php with CIDRResult enum and isValidCIDR refactor) and backend (Utils.php with IPBlock validation) to ensure subnet route CIDRs are network addresses without host bits set, matching issue #88 requirements. |
| Out of Scope Changes check | ✅ Passed | All changes are directly related to CIDR validation for subnet routes: frontend validation UI/logic, backend validation library, composer dependency addition, and GMP extension enablement for the IP library. No unrelated modifications detected. |
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
fix/cidr-validation
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/lint.yml(1 hunks)
🔇 Additional comments (1)
.github/workflows/lint.yml (1)
29-29: GMP extension is correctly added for PHPStan dependency resolution.The rlanvin/php-ip library requires the GMP extension, so enabling it during composer installation is necessary. The change at line 29 correctly configures this for the phpstan job.
Note: The php_extensions input includes additional PHP extensions in the action steps, which is the appropriate way to handle this dependency during CI analysis.
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.
Comment @coderabbitai help to get the list of available commands and usage tips.