Skip to content

fix(dns): enable was probing with a name that cannot be a URL - #479

Merged
ralyodio merged 1 commit into
mainfrom
fix/enable-probe-name
Aug 31, 2026
Merged

fix(dns): enable was probing with a name that cannot be a URL#479
ralyodio merged 1 commit into
mainfrom
fix/enable-probe-name

Conversation

@ralyodio

Copy link
Copy Markdown
Contributor

dns enable tests for the pinned-TLS proxy by asking it for a certificate over TLS, using a.${tlds[0]} as the name. Fine while the first ending the registry returned was a word. It is 00 now, so the probe asks about a.00 — and https://a.00/ is rejected outright by the WHATWG URL parser, because a final label of digits makes the host a candidate IPv4 literal and a.00 isn't a valid one.

The consequence wasn't a bad error message:

ok   proxy holds 127.0.0.1:443
--   no pinned-TLS proxy on this machine
--   https://a.00/ not verified yet — ERR_INVALID_URL

Detection concluded there was no proxy — on a machine that had just installed one, started it, and confirmed it holding 443 three lines earlier. The bridge was then started without proxy mode, so every Moshpit name answered its origin directly and a stock client got a certificate no CA had signed. On a machine whose setup had otherwise completed perfectly, including installing the local root into the system trust store.

The fix

The probe stays synthetic — the proxy mints a certificate for any name inside the namespace, so it needn't name something anyone registered. It only has to parse. So an all-numeric ending is skipped when choosing one.

Numeric endings stay sellable and wanted (.420, .2600); nothing here changes that. They're unusable as a probe, not as names.

--proxy-probe <name> overrides, matching what dns service has accepted since v0.83.0 — that flag was added for this exact failure, to one of the two commands that needed it.

Tests

Five, including the one the old code would have failed:

assert.doesNotThrow(() => new URL(`https://${probeName(["00","01","hacker"])}/`));
assert.throws(() => new URL("https://a.00/"), { code: "ERR_INVALID_URL" });

Suite: 2765 tests, 0 failures.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ThnQwoieWt8VR6N7gtgnhp

`dns enable` tests for the pinned-TLS proxy by asking it for a certificate over
TLS, using `a.${tlds[0]}` as the name. That was fine for as long as the first
ending the registry returns happened to be a word. It is `00` now, so the probe
asked about `a.00` — and `https://a.00/` is rejected outright by the WHATWG URL
parser, because a final label of digits makes the whole host a candidate IPv4
literal and `a.00` is not a valid one.

The consequence was not a bad error message. Detection concluded there was no
proxy, on a machine that had just installed one, started it, and confirmed it
holding 127.0.0.1:443 three lines earlier. The bridge was then started without
proxy mode, so every Moshpit name answered its origin directly and a stock
client got a certificate no CA had signed — on a machine whose setup had
otherwise completed perfectly, including installing the local root into the
system trust store.

    ok   proxy holds 127.0.0.1:443
    --   no pinned-TLS proxy on this machine
    --   https://a.00/ not verified yet — ERR_INVALID_URL

The probe stays synthetic: the proxy mints a certificate for any name inside the
namespace, so it does not have to name something anyone registered. It only has
to parse. So an all-numeric ending is skipped when choosing one.

Numeric endings remain sellable and wanted — `.420`, `.2600` — and nothing here
changes that. They are unusable as a probe, not as names, which is why this
skips them rather than the registry refusing them.

`--proxy-probe <name>` overrides, matching what `dns service` already accepted
since v0.83.0. That flag existed because of this same failure; it was added to
one of the two commands that needed it.

Five tests, including the one the old code would have failed: the chosen probe
must parse as a URL, and `https://a.00/` must throw ERR_INVALID_URL.

Suite: 2765 tests, 0 failures.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ThnQwoieWt8VR6N7gtgnhp
@github-actions

Copy link
Copy Markdown

ThreatCrush Security Scan

1 finding(s) in the 2 file(s) this pull request changes.

HIGH/CRITICAL: 1

Severity Rule Location
HIGH tls-verification-disabled src/dns.mjs:766
90 pre-existing finding(s) elsewhere in the repository — **HIGH/CRITICAL**: 7 | **MEDIUM**: 73 | **LOW**: 10

Not introduced by this pull request. The full set is in the Security tab.

Severity Rule Location
HIGH js-ssrf-outbound-request apps/pwa/public/sw.js:45
HIGH tls-verification-disabled apps/pwa/src/lib/moshpit-gateway.mjs:299
HIGH sh-remote-script-execution install.sh:86
HIGH sh-remote-script-execution install.sh:90
HIGH sh-remote-script-execution install.sh:202
HIGH sh-remote-script-execution install.sh:213
HIGH sh-remote-script-execution install.sh:219
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:139
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:153
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:179
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:373
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:377
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:422
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:671
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:867
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:869
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:928
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:974
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:1044
MEDIUM sql-template-interpolation apps/pwa/src/moshpit.mjs:1147

…and 70 more. Full results in the Security tab.

Snippets are redacted; ThreatCrush never prints matched credential material.

@ralyodio
ralyodio merged commit 8b81e18 into main Aug 31, 2026
6 checks passed
@ralyodio ralyodio mentioned this pull request Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant