tidy-guard: why DialControl strips its own address, with a test - #286
Merged
Conversation
netguard.go holds two implementations of "get the bare host out of this": canonicalHost, and the inline strip in DialControl. On a sweep for duplicated logic they look like one function written twice. They are not, and the difference runs the wrong way to merge. canonicalHost trims surrounding whitespace, so it parses inputs DialControl refuses: " 8.8.8.8:53 " is unparsable in DialControl and a public address through canonicalHost, which turns a rejection into a dial. Every other input tried — bare and bracketed v6, v4-mapped, portless, empty, ":8080", malformed — reaches the same verdict through both, so this is the whole of the difference. No such address can arrive: net.Dialer hands Control the resolved host:port it built itself. But a fail-closed predicate at the SSRF boundary is the wrong place to widen what is accepted on the strength of an argument about what cannot happen, and "the two look identical" is exactly how that widening would arrive — the reasoning gateway.fsSafe already writes down for itself against proclog's rule. So: a comment saying why they stay apart, and " 8.8.8.8:53 " joins TestDialControl's block list to enforce it. Mutation-checked — rewriting DialControl to call canonicalHost fails that case and nothing else. No behaviour change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Nightly tidy pass over
internal/guard/netguard(2026-08-10 05:17 round).The slice produced no refactor, and that is the result.
netguard.goholds two implementations of "get the bare host out of this":canonicalHost, and the inline strip insideDialControl. On a sweep forduplicated logic — the shape that has landed three commits already tonight —
they look like one function written twice.
I differential-tested them before merging. They are not equivalent, and the
difference runs the wrong way:
DialControltodaycanonicalHost" 8.8.8.8:53 "canonicalHosttrims surrounding whitespace, so it parses inputsDialControlrefuses. Every other input tried reaches the same verdict through both — bare
and bracketed v6, v4-mapped, portless, empty,
":8080", malformed — so thatis the whole of the difference.
No such address can arrive:
net.DialerhandsControlthe resolvedhost:portit built itself. But a fail-closed predicate at the SSRF boundaryis the wrong place to widen what is accepted on the strength of an argument
about what cannot happen — and "the two look identical" is precisely how that
widening would arrive. It is the reasoning
gateway.fsSafealready writesdown for itself against proclog's rule, which is what stopped me merging that
pair two rounds ago.
DialControlsaying why the two stay apart." 8.8.8.8:53 "joinsTestDialControl's block list, so the comment isenforced rather than asserted. Mutation-checked: rewriting
DialControlto callcanonicalHostfails that case and nothing else.No behaviour change. Verification:
make cigreen.