fix(talk): fail closed at the SDP HTTP boundary

[SebTardif]

Talk now keeps its single-use broker credential on the direct offerUrl by disabling automatic HTTP redirects. All non-2xx responses fail the exchange. The same HTTP boundary also ignored failed Content-Type, Authorization, extra-header and POST-body configuration; those failures now stop before transmission, clean up the client and deliver no SDP answer. The first-failure diagnostics are preserved and their documentation now describes the fatal setup errors correctly.

This reworks @SebTardif's PR, preserves contributor credit, and includes the related request-setup bug found while checking the send path. The Gateway or reverse proxy must provide the final offer endpoint, including when a same-origin redirect was previously used. No public API, dependency version or release changes.

Real transport proof

The new repeatable native harness builds actual production Talk C with ESP-IDF's HTTP client, TCP transport, FreeRTOS and cJSON, then runs the binary against two loopback HTTP servers. HTTP functions are not replaced. Only the Gateway session response/close and peer answer callback are synthetic. The run used macOS and ESP-IDF 5.5.5; it does not qualify TLS, Wi-Fi, media or physical boards.

# Activate ESP-IDF and configure the component-test app's cJSON dependency first.
python3 components/esp-openclaw-talk/tests/run_http_host_tests.py

On unchanged main source 7b8b32d, all ten 301/302/303/307/308 cases, across both same-origin and cross-port destinations, produced two requests and one credential-bearing redirected request. With the fixed source, every redirect fails with one request, zero redirected requests and zero answers. Direct authenticated POST succeeds both before and after those cases:

baseline /redirect/302/cross: result=0 answers=1 requests=2 redirected_requests=1 authorized_hops=1
fixed /redirect/302/cross: result=-6 answers=0 requests=1 redirected_requests=0 authorized_hops=0
fixed /redirect/308/same: result=-6 answers=0 requests=1 redirected_requests=0 authorized_hops=0
fixed /offer: result=0 answers=1 requests=1 redirected_requests=0 authorized_hops=0
12 real ESP-IDF HTTP exchanges passed

The runner accepts existing cJSON/WebRTC paths, an external persistent build directory, and a production-source override for repeating baseline failures. Its default build is temporary. It binds only loopback and uses synthetic data/credentials.

Regression and build validation

All 19 routing tests pass with ASan/UBSan, covering direct success, redirect statuses, every setup failure, cleanup, and an earlier offer-header failure followed by a successful later header. All 24 threaded ownership/diagnostics cases pass with real SDK headers under both ASan/UBSan and TSan. Existing diagnostics assertions that expected ignored setup errors now require failure, zero HTTP performs/answers and preserved cleanup/diagnostics.

Independent autoreview is clean through P2 for the final staged candidate. The native HTTP fixture also passed its fresh build and all 12 network exchanges. Optional Clang analysis produces one baseline-matching test-path lifetime warning, reproduced with unchanged main source and the same fixture; sanitizer execution passes, and no suppression was added.

Final head 8592d4dc79ede294d867a9e4b5cd62d2541e6e29 passed all five native firmware builds, the registered file-command suite, 19 routing cases, 24 threaded cases and 49 room lifecycle cases. Tab5 used the exact documented SDK base plus verified SDK/SDIO/camera patches, and its compiled-camera verification passed. The earlier #32 candidate passed its full five-target CI matrix; all five final-head CI builds passed at 8592d4dc79ede294d867a9e4b5cd62d2541e6e29.

[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: blocked before merge. Reviewed September 16, 2026, 12:13 AM ET / 04:13 UTC (Revision 5).

ClawSweeper review

What this changes

Talk now refuses redirects during voice-session negotiation and stops before transmission when HTTP request setup fails, with regression coverage and operator documentation.

Merge readiness

⛔ Blocked before merge - 1 item remains

Still needed on main. The earlier documentation and proof blockers are resolved, and no introduced correctness defect remains.

Priority: P2
Reviewed head: 8592d4dc79ede294d867a9e4b5cd62d2541e6e29

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with relevant real transport proof, resolved prior findings, and an explicit compatibility tradeoff.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (terminal): The captured PR body supplies macOS/ESP-IDF 5.5.5 traces through production Talk and the actual HTTP/TCP client: direct authenticated SDP succeeds, same-origin and cross-port redirects produce zero destination requests, and a subsequent direct exchange succeeds. The inspected harness matches that boundary; TLS, Wi-Fi and physical media qualification are outside this change.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (terminal): The captured PR body supplies macOS/ESP-IDF 5.5.5 traces through production Talk and the actual HTTP/TCP client: direct authenticated SDP succeeds, same-origin and cross-port redirects produce zero destination requests, and a subsequent direct exchange succeeds. The inspected harness matches that boundary; TLS, Wi-Fi and physical media qualification are outside this change.
Evidence reviewed	8 items	Current main still needs the repair: The pinned main implementation omits disable_auto_redirect and proceeds to HTTP perform despite header or body configuration errors. The complete introduced diff was inspected locally, including the portions truncated in the supplied context. Credential boundary and cleanup: The adapter validates Gateway-owned session material, rejects overrides of Authorization, Content-Type and Host, disables automatic redirects, and cleans up before transmission on request-configuration failure. Existing response validation rejects non-2xx answers. Real transport proof resolves the previous request: The supplied complete PR body, captured under sourceRevision 93269d5a6ef54916778e7f91687bfb3928c1087cea5ae1cf4974fe20bbac7bb3, reports macOS/ESP-IDF 5.5.5 execution using production Talk and the actual HTTP/TCP client. Baseline redirects sent a credential-bearing second request; fixed redirects produced one request, zero destination requests and zero answers. Direct authenticated exchanges succeeded before and after the redirect cases. The inspected harness links production Talk and real SDK HTTP; only Gateway responses and peer callbacks are synthetic.
Findings	None	None.
Security	None	None.

How this fits together

The Talk adapter connects ESP32 voice firmware to Gateway-owned voice sessions. It receives an offer endpoint and broker credential, sends the local session description over HTTP, and passes a successful answer to WebRTC.

flowchart TD
  A[Gateway session response] --> B[Validate endpoint and credential]
  C[Local session description] --> D[Configure HTTP request]
  B --> D
  D --> E{Setup succeeds?}
  E -->|No| F[Fail and clean up]
  E -->|Yes| G[Direct HTTP exchange]
  G -->|Redirect or error| F
  G -->|Valid successful answer| H[Deliver answer to WebRTC]

Loading

Before merge

• Resolve merge risk (P1) - Existing Gateway or reverse-proxy setups that redirect the offer endpoint, including same-origin redirects, will fail negotiation after upgrade until configured to serve the final endpoint directly; this intentional change is documented and direct-endpoint recovery is demonstrated.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production and test delta	Production +13/-5; tests and fixtures +397/-5	The small production change is justified by credential confinement and request-setup validation.
Real HTTP scenarios	10 redirect cases and 2 direct exchanges	The supplied run exercises all five redirect statuses at same-origin and cross-port destinations plus successful direct exchange.

Merge-risk options

Maintainer options:

1. Accept the documented direct-endpoint requirement (recommended)
   Retain redirect rejection and its operator migration guidance, supported by real-client proof that direct exchange succeeds after redirect failures.

Technical review

Best possible solution:

Keep credentials confined to the direct offer endpoint and retain the documented proxy migration backed by the supplied direct-exchange recovery proof.

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

Yes: current-main source and the SDK redirect contract establish the path, and the supplied native baseline trace demonstrates credential forwarding. This read-only review did not execute the harness.

Is this the best way to solve the issue?

Yes: the SDK's redirect switch and a pre-transmission error guard repair the existing boundary without a parallel transport implementation; the deliberate compatibility change has documentation and recovery proof.

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning medium; reviewed against 31b2cf08cabc.

Labels

Label changes:

• add proof: sufficient: Contributor real behavior proof is sufficient. The captured PR body supplies macOS/ESP-IDF 5.5.5 traces through production Talk and the actual HTTP/TCP client: direct authenticated SDP succeeds, same-origin and cross-port redirects produce zero destination requests, and a subsequent direct exchange succeeds. The inspected harness matches that boundary; TLS, Wi-Fi and physical media qualification are outside this change.
• add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured PR body supplies macOS/ESP-IDF 5.5.5 traces through production Talk and the actual HTTP/TCP client: direct authenticated SDP succeeds, same-origin and cross-port redirects produce zero destination requests, and a subsequent direct exchange succeeds. The inspected harness matches that boundary; TLS, Wi-Fi and physical media qualification are outside this change.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.
• remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.

Label justifications:

• P2: This is a focused Talk HTTP hardening repair with a bounded operator compatibility impact.
• merge-risk: 🚨 compatibility: Redirecting offer endpoints will stop working until operators configure the final endpoint directly.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The captured PR body supplies macOS/ESP-IDF 5.5.5 traces through production Talk and the actual HTTP/TCP client: direct authenticated SDP succeeds, same-origin and cross-port redirects produce zero destination requests, and a subsequent direct exchange succeeds. The inspected harness matches that boundary; TLS, Wi-Fi and physical media qualification are outside this change.
• proof: sufficient: Contributor real behavior proof is sufficient. The captured PR body supplies macOS/ESP-IDF 5.5.5 traces through production Talk and the actual HTTP/TCP client: direct authenticated SDP succeeds, same-origin and cross-port redirects produce zero destination requests, and a subsequent direct exchange succeeds. The inspected harness matches that boundary; TLS, Wi-Fi and physical media qualification are outside this change.

Evidence

What I checked:

• Current main still needs the repair: The pinned main implementation omits disable_auto_redirect and proceeds to HTTP perform despite header or body configuration errors. The complete introduced diff was inspected locally, including the portions truncated in the supplied context. (components/esp-openclaw-talk/src/esp_openclaw_talk.c:824, 31b2cf08cabc)
• Credential boundary and cleanup: The adapter validates Gateway-owned session material, rejects overrides of Authorization, Content-Type and Host, disables automatic redirects, and cleans up before transmission on request-configuration failure. Existing response validation rejects non-2xx answers. (components/esp-openclaw-talk/src/esp_openclaw_talk.c:836, 8592d4dc79ed)
• Real transport proof resolves the previous request: The supplied complete PR body, captured under sourceRevision 93269d5a6ef54916778e7f91687bfb3928c1087cea5ae1cf4974fe20bbac7bb3, reports macOS/ESP-IDF 5.5.5 execution using production Talk and the actual HTTP/TCP client. Baseline redirects sent a credential-bearing second request; fixed redirects produced one request, zero destination requests and zero answers. Direct authenticated exchanges succeeded before and after the redirect cases. The inspected harness links production Talk and real SDK HTTP; only Gateway responses and peer callbacks are synthetic. (components/esp-openclaw-talk/tests/http_host/main/CMakeLists.txt:5, 8592d4dc79ed)
• Authoritative HTTP dependency contract: The changed production source directly configures ESP-IDF's HTTP client, establishing dependency relevance. In v5.5.5, disable_auto_redirect causes redirect responses to emit an event instead of calling the redirection function; Talk's event callback does not follow that event. The tag resolves to the recorded dependency SHA. (components/esp_http_client/esp_http_client.c:1126, b774170ff46c)
• Prior findings resolved: The README now describes header/body errors as fatal and explicitly documents same-origin redirect rejection and final-endpoint configuration. The host HTTP stub includes the redirect field, and the Gateway test double preserves injected config/create submission errors. These address all concrete findings retained in the previous-review projection. (components/esp-openclaw-talk/README.md:43, 8592d4dc79ed)
• Regression coverage and measured scope: Added coverage checks direct authenticated success, five redirect statuses, each request-setup failure, cleanup, and retention of an earlier header failure. Production changes total +13/-5 lines; test code and fixtures total +397/-5 lines. No workflow, dependency-version, public API or persistent-schema change is introduced. (components/esp-openclaw-talk/tests/test_esp_openclaw_talk.c:687, 8592d4dc79ed)

Likely related people:

• steipete: 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 (4 earlier review cycles)

• reviewed 2026-08-30T03:34:16.720Z sha c84b574 :: needs real behavior proof before merge. :: none
• reviewed 2026-08-30T09:53:03.972Z sha c84b574 :: needs real behavior proof before merge. :: none
• reviewed 2026-09-07T19:16:55.539Z sha d4d3002 :: needs real behavior proof before merge. :: [P2] Add the redirect field to the host HTTP configuration stub | [P2] Preserve injected Gateway errors when starting signaling
• reviewed 2026-09-16T03:47:27.948Z sha 609e7cf :: needs real behavior proof before merge. :: [P3] Update the diagnostics description for fatal HTTP setup errors

[SebTardif]

@clawsweeper re-review

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/34155098292
• Updated: 2026-09-07T19:19:02.237Z