fix: Migrate missing fixes from dev-1.2.7#619
Merged
PeterJhongLinksys merged 2 commits intodev-2.0.0from Feb 6, 2026
Merged
Conversation
NOW-713: Update OpenDNS Family Shield IPs - Changed from 208.67.222.123/208.67.220.123 to 208.67.222.222/208.67.220.220 - Updated service, test data, and test assertions QUALITY-439: Allow /31 subnet masks for WAN Static IP - Added max: 31 parameter to SubnetMaskValidator in: - internet_settings_form_validator.dart - pnp_static_ip_view.dart Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Review Summary by QodoMigrate missing fixes from dev-1.2.7 branch
WalkthroughsDescription• Migrate two critical fixes from dev-1.2.7 branch • Update OpenDNS Family Shield DNS server IPs • Allow /31 subnet masks for WAN Static IP configuration • Update corresponding test data and assertions Diagramflowchart LR
A["NOW-713: OpenDNS IP Update"] --> B["Update DNS Constants"]
B --> C["instant_safety_service.dart"]
B --> D["instant_safety_test_data.dart"]
B --> E["instant_safety_service_test.dart"]
F["QUALITY-439: /31 Subnet Support"] --> G["SubnetMaskValidator max: 31"]
G --> H["internet_settings_form_validator.dart"]
G --> I["pnp_static_ip_view.dart"]
File Changes1. lib/page/instant_safety/services/instant_safety_service.dart
|
Code Review by Qodo
1. No tests for /31 masks
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+46
to
48
| // QUALITY-439: Override default max (30) to allow /31 subnet masks for WAN Static IP | ||
| final subnetMaskValidator = SubnetMaskValidator(max: 31); | ||
| if (subnetMaskValidator.validate(value)) { |
There was a problem hiding this comment.
1. No tests for /31 masks 📘 Rule violation ⛯ Reliability
• The PR changes WAN Static IP subnet mask validation to allow /31 by constructing SubnetMaskValidator(max: 31) in multiple places. • There is no automated test asserting that a /31 mask (e.g., 255.255.255.254) is accepted in these flows, and existing tests still only cover /24 in the PNP static IP view. • Without coverage for this boundary change, regressions may go undetected and the intended fix may not be reliably validated.
Agent Prompt
## Issue description
The PR enables `/31` subnet masks for WAN Static IP by using `SubnetMaskValidator(max: 31)`, but there are no automated tests asserting that a `/31` mask (e.g., `255.255.255.254`) is accepted in these updated flows.
## Issue Context
Existing tests for the PNP static IP view only use `255.255.255.0` as the valid subnet mask, and current `NetworkUtils.isValidSubnetMask` tests treat `255.255.255.254` as invalid under default parameters. This leaves the new boundary behavior unverified.
## Fix Focus Areas
- lib/page/advanced_settings/internet_settings/utils/internet_settings_form_validator.dart[42-53]
- lib/page/instant_setup/troubleshooter/views/isp_settings/pnp_static_ip_view.dart[34-40]
- test/page/instant_setup/troubleshooter/views/isp_settings/localizations/pnp_static_ip_view_test.dart[110-175]
- test/utils_test.dart[773-808]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+52
to
+54
| // NOW-713: Updated OpenDNS Family Shield IPs | ||
| static const _openDnsDns1 = '208.67.222.222'; | ||
| static const _openDnsDns2 = '208.67.220.220'; |
There was a problem hiding this comment.
2. Opendns config regression 🐞 Bug ✓ Correctness
• InstantSafetyService determines the current provider via an exact match on dnsServer1; changing the OpenDNS constant means routers still configured with the previously documented OpenDNS IPs will now be classified as off. • This can surface as an incorrect UI state (shows “off” even though OpenDNS is configured) and can lead to unintended changes when the user saves settings. • Repo specs/contracts still state OpenDNS uses the old IPs, so implementation and documented behavior are inconsistent, increasing the chance of future regressions.
Agent Prompt
### Issue description
InstantSafetyService identifies OpenDNS by exact matching `dnsServer1` against `_openDnsDns1`. Updating `_openDnsDns1/_openDnsDns2` changes the detection key, so devices configured with the previously documented OpenDNS values will now be treated as `off`.
### Issue Context
Repo documentation/specs still describe OpenDNS as `208.67.222.123`/`208.67.220.123` and show detection logic matching against `208.67.222.123`. The implementation now uses `208.67.222.222`/`208.67.220.220`.
### Fix Focus Areas
- lib/page/instant_safety/services/instant_safety_service.dart[50-156]
- specs/009-instant-safety-service/spec.md[115-120]
- specs/009-instant-safety-service/contracts/instant_safety_service_contract.md[124-231]
### Suggested approach
1. Introduce legacy OpenDNS constants (or a set/list of acceptable DNS1 values).
2. Update `_determineSafeBrowsingType` to return `openDNS` when `dnsServer1` equals either legacy or new DNS1 (optionally also validate DNS2 for additional safety).
3. Decide the desired DNS values to *write* in `saveSettings` (new vs legacy) and update specs/contracts accordingly.
4. Add/extend tests to ensure fetchSettings returns `InstantSafetyType.openDNS` when LAN settings contain the legacy DNS1 value.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
- Add /31 subnet mask tests for QUALITY-439 - Test validates /31 mask (255.255.255.254) accepted with max: 31 - Test validates /31 mask rejected with default max (30) - Update spec documents with new OpenDNS IPs (NOW-713) - spec.md: Update assumptions section - contracts/instant_safety_service_contract.md: Update DNS constants and examples Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PeterJhongLinksys
approved these changes
Feb 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Migrates two missing fixes from
dev-1.2.7that were not included in thedev-2.0.0branch:Changes
NOW-713: OpenDNS IP Update
208.67.222.123/208.67.220.123to208.67.222.222/208.67.220.220lib/page/instant_safety/services/instant_safety_service.darttest/mocks/test_data/instant_safety_test_data.darttest/page/instant_safety/services/instant_safety_service_test.dartQUALITY-439: SubnetMask Validator Fix
max: 31parameter toSubnetMaskValidatorto allow /31 subnet masks for WAN Static IPlib/page/advanced_settings/internet_settings/utils/internet_settings_form_validator.dartlib/page/instant_setup/troubleshooter/views/isp_settings/pnp_static_ip_view.dartTest plan
flutter analyzepasses with no errorsinstant_safety_service_test.dart- 11/11 tests pass🤖 Generated with Claude Code