Add DNS-over-HTTPS (DoH) support - #4856
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1786067fcf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review latest changes |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
I realized this change makes a broader product decision by enabling DoH for all supported DNS configurations, and I did not confirm that behavior with Titus beforehand. I’m happy to adjust this to be optional (i.e., having Cloudflare and Cloudflare (DoH)), scope it differently, or move the design discussion into a feature request if that would be preferable. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a0adc802e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
📝 WalkthroughSummary by CodeRabbit
WalkthroughDNS provider configurations now include DNS-over-HTTPS templates. ChangesDNS-over-HTTPS configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SetWinUtilDNS
participant DNSProviderConfig
participant WindowsDoH
participant DoHRegistry
participant DNSClientCache
SetWinUtilDNS->>DNSProviderConfig: read DNS addresses and DohTemplate
SetWinUtilDNS->>WindowsDoH: add or update DoH server entries
SetWinUtilDNS->>DoHRegistry: write per-interface DohFlags
SetWinUtilDNS->>DNSClientCache: clear cache
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@functions/private/Set-WinUtilDNS.ps1`:
- Around line 61-65: Enable DoH auto-upgrade by changing the -AutoUpgrade
argument to $true in both Set-DnsClientDohServerAddress and
Add-DnsClientDohServerAddress within functions/private/Set-WinUtilDNS.ps1 lines
61-65. Update the corresponding Pester expectation in pester/dns.Tests.ps1 lines
103-126 to assert $true.
- Around line 62-80: Update the DoH registration flow around
Set-DnsClientDohServerAddress and Add-DnsClientDohServerAddress to use
terminating error behavior so failures reach the surrounding catch instead of
being suppressed. Also make the New-Item and New-ItemProperty registry writes
terminating, ensuring completion/logging and subsequent DNS cache handling occur
only after every DoH setup step succeeds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b9071f8d-0c61-4a38-8096-4b0921fb1014
📒 Files selected for processing (3)
config/dns.jsonfunctions/private/Set-WinUtilDNS.ps1pester/dns.Tests.ps1
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7af9c0988
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@ChrisTitusTech The latest review identified three behavior decisions around DHCP and DoH cleanup: global auto-upgrade entries may persist after switching to DHCP, cleanup currently removes the whole per-interface DoH tree, and a failed DoH setup can leave the adapter on plaintext DNS. Should WinUtil remove only DoH state it created, preserve existing settings, and roll back DNS changes if DoH setup fails? I’d prefer guidance before making these broader changes |
|
Yes, I'd say remove DoH state it created on failure and preserve existing settings. Resolved the messages for the 2 bad recommendations the one revert on dhcp is a good recommend. |
Thank you! Done. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
functions/private/Set-WinUtilDNS.ps1 (1)
94-96: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFlush the DNS cache after every transition. DHCP resets and static changes without DoH support skip
Clear-DnsClientCache, so stale answers can persist until TTL expiry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@functions/private/Set-WinUtilDNS.ps1` around lines 94 - 96, Update the DNS transition flow in Set-WinUtilDNS so Clear-DnsClientCache runs after every DHCP reset or static DNS change, regardless of DoH support or DohTemplate availability. Remove the current DoH-specific condition while preserving the existing transition behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@functions/private/Set-WinUtilDNS.ps1`:
- Around line 49-64: Track ownership of DoH registrations in Set-WinUtilDNS
before mutating the system-wide server-address list: at lines 49-64, remove only
entries created by WinUtil or persist and restore prior values instead of
deleting by address; at lines 75-89, avoid changing pre-existing registrations
unless their original templates and flags are captured and restored. Apply the
ownership tracking consistently across both sites.
---
Outside diff comments:
In `@functions/private/Set-WinUtilDNS.ps1`:
- Around line 94-96: Update the DNS transition flow in Set-WinUtilDNS so
Clear-DnsClientCache runs after every DHCP reset or static DNS change,
regardless of DoH support or DohTemplate availability. Remove the current
DoH-specific condition while preserving the existing transition behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1029d039-3f2e-4b83-89c3-9264cc5d185a
📒 Files selected for processing (2)
functions/private/Set-WinUtilDNS.ps1pester/dns.Tests.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- pester/dns.Tests.ps1
2 participants
Type of Change
Description
This PR introduces DNS-over-HTTPS (DoH) support as a transparent, encrypted-by-default security enhancement to the DNS configuration workflow for Windows 11 and forward.
Key Changes
dns.json):Set-WinUtilDNS.ps1):Add-DnsClientDohServerAddresscmdlet.-AllowFallbackToUdp $false).DohFlagsregistry value to1(strict) underHKLM:\System\CurrentControlSet\Services\Dnscache\InterfaceSpecificParameters\<InterfaceGuid>\DohInterfaceSettings.DohInterfaceSettingsregistry tree recursively when resetting adapter DNS configurations back to DHCP, preventing registry bloat.dns.Tests.ps1):Validation
Invoke-Pester)..\Compile.ps1to successfully compile the source modifications into the release artifact..\winutil.ps1and verified that choosing any of the DNS options successfully forces encrypted connections.git diff --check.(Encrypted)indicator:Issue related to PR