DockerLocalhostRewrite.rewrite_to is interpolated into the netloc unvalidated; a bare IPv6 value breaks webhook signing

[KonstantinMirin]

Summary

DockerLocalhostRewrite.rewrite_to is interpolated straight into a netloc with no validation. An IPv6 value produces an unbracketed authority, which is the one shape RFC 3986 makes ambiguous with a port. This is the only place a non-signing layer synthesizes the netloc that later becomes the signed @authority.

Cause

src/adcp/webhook_transport_hooks.py:97-100:

netloc = self.rewrite_to
if parsed.port is not None:
    netloc = f"{self.rewrite_to}:{parsed.port}"
return urlunsplit((parsed.scheme, netloc, parsed.path, parsed.query, parsed.fragment))

rewrite_to is a plain str dataclass field (:82) with no constraint beyond its default. The docstring suggests 172.17.0.1 for Linux hosts, so operators are already expected to put an IP literal here; nothing tells them a v6 literal needs brackets, and nothing rejects it if they omit them.

Reproduce

from adcp.webhook_transport_hooks import DockerLocalhostRewrite
from adcp.signing.canonical import canonicalize_authority

h = DockerLocalhostRewrite(rewrite_to="::1")
h.rewrite_url("https://localhost:9000/hook")
# 'https://::1:9000/hook'

canonicalize_authority("https://::1:9000/hook")

rewrite_to	rewritten URL	@authority
host.docker.internal	https://host.docker.internal:9000/hook	host.docker.internal:9000
172.17.0.1	https://172.17.0.1:9000/hook	172.17.0.1:9000
::1	https://::1:9000/hook	TargetUriMalformedError
2001:db8::1	https://2001:db8::1:9000/hook	TargetUriMalformedError
[::1]	https://[::1]:9000/hook	[::1]:9000

Bracketing it by hand works. Nothing tells the operator to.

Why it matters

The hook runs before signing — webhook_sender.py:974 calls apply_hooks, and the signer sees the rewritten URL — so the ordering is correct and the signature does cover the URL actually dialled. The problem is the value the hook hands forward.

Before #985, _canon_authority split the unbracketed netloc on its last colon and reassembled it, so ::1:9000 round-tripped and looked fine while being an ambiguous authority. After #985 the same input is a hard TargetUriMalformedError at sign time. Failing loudly is the better outcome, but it converts a silent misconfiguration into a delivery failure at first send, with the error raised in canonical.py rather than anywhere near the hook that caused it.

Scope is genuinely small: this is an opt-in e2e-testing hook that already refuses to construct unless the sender was built with allow_private_destinations=True (:102-118), and the failure is at the adopter's own configured destination. It is a config-validation gap, not a protocol defect.

Suggested direction

validate_for_sender (:102) is already the construction-time validation seam, and it already raises ValueError for the other misconfiguration this hook has. Validate rewrite_to there: reject anything that is not a valid hostname or a bracketed/bare IP literal, and bracket a bare IPv6 literal rather than rejecting it — the operator's intent is unambiguous, and the docstring actively encourages IP-literal values.

Doing it at construction keeps the error at wiring time, which is the stated design of this hook's existing check, rather than at the first delivery attempt.

Related

• fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985 — notes this in its own PR body as needing a follow-up ticket; this is that ticket. fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985 changes the symptom (silent mis-signing → hard rejection) but does not validate the hook input.

[bokelley]

Triage

Classification: Bug
Bucket(s): signing, validation (neither has a matching repo label — noted as a label gap)
Status: ready-to-implement
Milestone: omitted (no milestone named in issue text)

What the experts said:

• ad-tech-protocol-expert: RFC 3986-correct approach — auto-bracket bare IPv6 in netloc is the right remediation; __post_init__ must explicitly guard zone-ID inputs (reject % in rewrite_to) since [fe80::1%eth0] is not a valid IP-literal and fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985 will turn the unguarded case into a hard error; sequencing matters — this fix ideally lands before or with fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985
• code-reviewer: fix seam should be __post_init__ (not validate_for_sender — that method can't mutate a frozen dataclass); ipaddress.ip_address() + object.__setattr__ is the established codebase idiom (same pattern in key_origins.py:260-267); six concrete tests needed; "[::1]" in _LOCALHOST_HOSTS is dead code (urlsplit().hostname always strips brackets)
• security-reviewer: not a security vulnerability; rewrite_to is operator-set at construction, not wire input; auto-coercion does not weaken any SSRF or signing invariant; proceed to implement without human security review

My take: All three experts converge on a narrow, non-breaking __post_init__ fix. The urgency is real: PR #985 (same author, currently open) changes the downstream failure mode for this misconfiguration from silent mis-signing to a hard TargetUriMalformedError raised inside canonical.py with no pointer back to DockerLocalhostRewrite — exactly the failure-at-delivery-not-at-wiring mode the hook's own docstring says the design exists to prevent.

Implementation brief:

• Scope: src/adcp/webhook_transport_hooks.py + tests/conformance/signing/test_webhook_transport_hooks.py
• Changes required:
  1. Add __post_init__ to DockerLocalhostRewrite: detect bare IPv6 with ipaddress.ip_address() — same pattern as src/adcp/signing/key_origins.py:260-267 — and bracket via object.__setattr__(self, "rewrite_to", f"[{value}]"). Already-bracketed values ("[::1]") raise ValueError inside ip_address() and fall through unchanged; idempotency is automatic.
  2. In the same __post_init__: reject rewrite_to values that are empty or contain %, @, /, ?, or # with a clear ValueError describing the accepted formats (hostname, bare IPv4, bare or bracketed IPv6 without zone ID). This covers both zone IDs (fe80::1%eth0) and fat-fingered path inclusions.
  3. Remove the dead "[::1]" entry from _LOCALHOST_HOSTS at line 56 — urlsplit().hostname always strips IPv6 brackets before returning, so this set member is never matched; removing it prevents future reader confusion about what forms the match covers.
• Required tests (all in test_webhook_transport_hooks.py):
  • DockerLocalhostRewrite(rewrite_to="::1").rewrite_to == "[::1]" — normalization visible on stored field
  • DockerLocalhostRewrite(rewrite_to="[::1]").rewrite_to == "[::1]" — idempotent
  • DockerLocalhostRewrite(rewrite_to="::1").rewrite_url("https://localhost:9000/hook") == "https://[::1]:9000/hook" — port case
  • DockerLocalhostRewrite(rewrite_to="::1").rewrite_url("https://localhost/hook") == "https://[::1]/hook" — no-port case
  • DockerLocalhostRewrite(rewrite_to="2001:db8::1").rewrite_url("http://127.0.0.1:8080/x") == "http://[2001:db8::1]:8080/x" — global unicast
  • pytest.raises(ValueError, match="zone") for DockerLocalhostRewrite(rewrite_to="fe80::1%eth0") — zone-ID rejection
• Non-breaking rationale: Correct configs ("[::1]", "172.17.0.1", "host.docker.internal") are entirely unaffected. Broken configs ("::1") currently produce wrong @authority signing (pre-fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985) or a hard TargetUriMalformedError (post-fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985) — auto-coercion is strictly better in both states.
• Sequencing note: Coordinate with PR fix(signing): reject malformed authorities, convert IDN hosts to A-labels #985 — this fix should land before or with it to avoid shipping the new hard error without the config-validation improvement that gives the clear message.
• Checks: pytest tests/conformance/signing/test_webhook_transport_hooks.py, mypy src/adcp/webhook_transport_hooks.py, ruff check . && black --check .

Label gap: repo has no signing or validation label; bucket names recorded here instead.

---

Triaged by Claude Code. Session: https://claude.ai/code/cse_01XVR71FC5mjB4oocarL7hpJ

---

Generated by Claude Code