[SebTardif]

What Problem This Solves

A Signal request with an IPv6 zone ID in its Host header, such as [::1%lo0], could throw from the HTTP listener and terminate the server.

User Impact

Malformed zone-ID Hosts now receive HTTP 400, and subsequent health checks still succeed. Ordinary IPv4, IPv6, and DNS Hosts retain their existing behavior.

Why This Change Was Made

Node's IP parser accepts scoped IPv6 addresses that WHATWG URL rejects. Host normalization now rejects zone IDs before canonicalization and contains URL parsing failures. This completes @SebTardif's fix with raw, encoded, and IPv4-mapped zone-ID coverage, setup documentation, and an Unreleased entry.

Evidence

The original raw-TCP regression failed against the base with an uncaught ERR_INVALID_URL. The candidate passed all 44 Signal tests and pnpm verify on Linux/Node 24: 2,000 tests passed, 69 skipped, and coverage thresholds passed. Independent Codex review covers P0–P3. GitHub CI also checks Node 22 and Node 24.

Exact-head CI: https://github.com/openclaw/crabline/actions/runs/37581244203 (attempt 2). The first Node 24 attempt timed out in the unchanged Telegram readiness rollback test; the single failed-job retry passed. Node 22 and CodeQL passed as well.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed October 7, 2026, 2:27 AM ET / 06:27 UTC (Revision 3).

ClawSweeper review

What this changes

The branch rejects Signal IPv6 zone-ID Host headers with HTTP 400, adds three continued-service regression cases and documentation, and incorporates the already-merged shared autoreview tooling update.

Merge readiness

✅ Ready for maintainer review

This remains a useful, focused repair: current main and v0.1.29 retain the uncaught Host conversion. No blocking patch defect was found, and the previously accepted real TCP proof remains relevant.

Priority: P2
Reviewed head: c1502ed9b00019f4ed28a9203dfcdb9ce819da5a

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with relevant accepted runtime proof, expanded transport coverage, and no blocking correctness findings.
Proof confidence	🐚 platinum hermit (4/6)	Sufficient (terminal): Previously accepted macOS TCP output exercises the patched Signal listener, observes zone-ID rejection with HTTP 400, and confirms subsequent service. The current Linux/Node 24 report and real-TCP regression cases extend coverage to three Host forms and a successful health check. No stored-data contract changes.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): Previously accepted macOS TCP output exercises the patched Signal listener, observes zone-ID rejection with HTTP 400, and confirms subsequent service. The current Linux/Node 24 report and real-TCP regression cases extend coverage to three Host forms and a successful health check. No stored-data contract changes.
Evidence reviewed	9 items	Pinned change and inherited tooling: The pinned merge-base-to-head delta was inspected. Raw head parents are fetched main followed by the earlier contributor head. Comparing the actual head with fetched main leaves only four Signal code, test, and documentation files; the tooling paths are identical to main. GitHub verifies #335 merged as 45eb015. The stale test-merge record was not used to attribute removals. Current main still needs the repair: Fetched main calls the WHATWG URL constructor on IPv6 values without catching conversion errors. Host validation executes synchronously in the HTTP listener before the asynchronous route error handler. Latest release also lacks the repair: v0.1.29 contains the same unguarded IPv6 URL conversion. This is not an already-shipped fix.
Findings	None	None.
Security	None	None.

How this fits together

Crabline’s Signal server emulates the signal-cli HTTP daemon for compatible clients. Host validation runs before requests reach health checks, event streams, messaging, or authenticated test-control routes.

flowchart TD
 A[HTTP client request] --> B[Normalize Host]
 B --> C{Valid allowed host?}
 C -->|No| D[HTTP 400]
 C -->|Yes| E[Signal HTTP routes]
 E --> F[Provider response]
 D --> G[Server continues serving]
 F --> G

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Signal production and test delta	production +10/-1, tests +42/-0	The small crash-containment guard has a clear purpose and three real-transport regression cases.

Technical review

Best possible solution:

Keep malformed Host input within the existing HTTP 400 path while preserving valid-host handling and continued server availability.

Do we have a high-confidence way to reproduce the issue?

Yes: a raw HTTP request with Host [::1%lo0] reaches main’s unguarded IPv6 URL conversion before the route error handler. This review established the path from source and did not execute the server.

Is this the best way to solve the issue?

Yes: rejecting zone IDs and containing URL conversion failures repairs the existing validation boundary without introducing a competing route or configuration mode.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 45eb01541f6e.

Labels

Label changes:

No label changes.

Label justifications:

• P2: This repairs a malformed-request crash in the local Signal provider server with limited product scope.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): Previously accepted macOS TCP output exercises the patched Signal listener, observes zone-ID rejection with HTTP 400, and confirms subsequent service. The current Linux/Node 24 report and real-TCP regression cases extend coverage to three Host forms and a successful health check. No stored-data contract changes.
• proof: sufficient: Contributor real behavior proof is sufficient. Previously accepted macOS TCP output exercises the patched Signal listener, observes zone-ID rejection with HTTP 400, and confirms subsequent service. The current Linux/Node 24 report and real-TCP regression cases extend coverage to three Host forms and a successful health check. No stored-data contract changes.

Evidence

What I checked:

• Pinned change and inherited tooling: The pinned merge-base-to-head delta was inspected. Raw head parents are fetched main followed by the earlier contributor head. Comparing the actual head with fetched main leaves only four Signal code, test, and documentation files; the tooling paths are identical to main. GitHub verifies chore(autoreview): use the shared canonical skill #335 merged as 45eb015. The stale test-merge record was not used to attribute removals. (src/servers/signal.ts:918, c1502ed9b000)
• Current main still needs the repair: Fetched main calls the WHATWG URL constructor on IPv6 values without catching conversion errors. Host validation executes synchronously in the HTTP listener before the asynchronous route error handler. (src/servers/signal.ts:921, 45eb01541f6e)
• Latest release also lacks the repair: v0.1.29 contains the same unguarded IPv6 URL conversion. This is not an already-shipped fix. (src/servers/signal.ts:921, 3a970b84f8c1)
• Rejection happens before provider effects: Zone IDs and failed canonicalization produce an empty normalized hostname, which fails hostname validation before allowlist lookup. The existing listener then returns HTTP 400 before dispatch. Valid-host canonicalization, admin authentication, and stored recorder formats are unchanged. (src/servers/signal.ts:919, c1502ed9b000)
• Production transport regression coverage: Three cases use node:net TCP connections against the actual Signal listener: raw, encoded, and IPv4-mapped zone-ID Hosts. Each asserts the HTTP 400 status and error body, then a successful health request. The captured PR body reports all 44 Signal tests passing on Linux/Node 24; these tests exercise a real transport rather than a mocked client. (test/signal-server.test.ts:212, c1502ed9b000)
• Preserved accepted runtime proof: The completed prior review at fix(signal): return 400 for an IPv6 zone-id Host #336 (comment) records captured macOS terminal output from the patched production listener: a raw TCP zone-ID Host receives HTTP 400, followed by a still-serving 404 response. This was accepted as sufficient real behavior proof. The original terminal transcript is not reproduced in the current body; this assessment preserves the recorded evidence rather than claiming a new reviewer execution. (a250b1dae7a2)

Likely related people:

• SebTardif: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• vincentkoc: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)

• reviewed 2026-10-03T13:10:48.209Z sha a250b1d :: needs maintainer review before merge. :: none
• reviewed 2026-10-06T19:46:00.134Z sha a250b1d :: needs maintainer review before merge. :: none