feat(unlocker): support Biscuit OTA rescue and expose TLS failures - #25
feat(unlocker): support Biscuit OTA rescue and expose TLS failures#25FrostyMead wants to merge 2 commits into
Conversation
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe patch adds Biscuit-specific fixed-length OTA manifest handling, HTTPS firmware URL selection, TLS handshake logging, synchronous listener binding, supervised server shutdown, ESP32-C3 chip mapping corrections, tests, and maintainer validation documentation. ChangesBiscuit OTA rescue
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Biscuit
participant github_releases_latest
participant biscuit_manifest_response
participant HTTPS_firmware_URL
Biscuit->>github_releases_latest: Request latest manifest
github_releases_latest->>biscuit_manifest_response: Build Biscuit manifest response
biscuit_manifest_response-->>Biscuit: Return fixed-length JSON with firmware.bin
Biscuit->>HTTPS_firmware_URL: Download firmware over HTTPS
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@unlocker-tool/crates/unlocker-core/src/http.rs`:
- Around line 774-796: Update the server startup flow in start so both HTTP and
HTTPS axum_server Server objects are constructed before spawning either
supervise_server task. Only spawn the HTTP and HTTPS tasks after both from_tcp
calls succeed, ensuring an HTTPS construction error returns without leaving the
HTTP task or listener orphaned; preserve the existing ServerHandles wiring and
shutdown behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ad1f91bd-2216-45ba-8448-f9f1d4a0c71e
⛔ Files ignored due to path filters (1)
unlocker-tool/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
FINDINGS.mdunlocker-tool/crates/unlocker-core/Cargo.tomlunlocker-tool/crates/unlocker-core/src/http.rs
📜 Review details
🧰 Additional context used
🪛 LanguageTool
FINDINGS.md
[uncategorized] ~21-~21: The official name of this software platform is spelled with a capital “H”.
Context: ...manifest connection:** Biscuit resolves api.github.com, opens TLS to port 443, and req...
(GITHUB)
[uncategorized] ~69-~69: The official name of this software platform is spelled with a capital “H”.
Context: ...before this patch - DNS already spoofs api.github.com and `unlocker.crosspointreader.c...
(GITHUB)
[uncategorized] ~231-~231: The official name of this software platform is spelled with a capital “H”.
Context: ...he hotspot. 5. Confirm Biscuit resolves api.github.com to 192.168.137.1. 6. Run **Check...
(GITHUB)
🔇 Additional comments (7)
unlocker-tool/crates/unlocker-core/Cargo.toml (1)
34-35: LGTM!FINDINGS.md (1)
1-373: LGTM!unlocker-tool/crates/unlocker-core/src/http.rs (5)
4-13: LGTM!Also applies to: 23-42
89-92: 🎯 Functional CorrectnessChip ID mapping fix looks correct.
Confirmed against Espressif's
esp_image_header_t.chip_idassignments: 5 = ESP32-C3, 9 = ESP32-S3. The old code's 5→S3/6→C3 mapping was indeed wrong; this correction and the accompanyingesp32c3_image_chip_id_is_fivetest are accurate.Also applies to: 130-141
143-222: 🩺 Stability & AvailabilityCustom TLS acceptor implementation looks solid.
The
LoggingTlsAcceptorwrappingRustlsAcceptorfollows axum-server's documented custom-Acceptpattern, and theAccept<TcpStream, S>impl signature matches the library trait. Logging TCP/TLS diagnostics here (rather than relying on axum-server's internal accept loop, which silently discards acceptor errors) is a sound fix for the "invisible pre-manifest failure" problem described in FINDINGS.md.
337-337: LGTM!Also applies to: 554-589, 649-657, 694-694
939-1130: LGTM!Tests cover the Biscuit fixed-length manifest, stock/vCodex routes, per-repo scheme selection (including the
biscuit-nextsubstring-collision edge case), and the corrected chip ID mapping.
|
Hey thanks for this PR! I will test and get this pushed up prior to the 7/30 deadline |
2 participants
Summary
Adds explicit OTA support for Biscuit and exposes TCP/TLS failures that
currently happen before the Unlocker's normal HTTP request logging.
Background
An XTEINK X4 running Biscuit successfully joins the Unlocker hotspot and
resolves
api.github.comto192.168.137.1, but Biscuit reportsUpdate failedand the existing helper logs no GitHub manifest request.The previous Unlocker could report its servers as ready before ports 80 and
443 had successfully bound. TLS handshake failures occurring before Axum were
also not logged.
Separately, Biscuit requires an HTTPS OTA download URL. The generic Unlocker
response previously supplied Biscuit with an HTTP firmware URL.
Changes
tag_nameexactly99.9.9firmware.binhttps://unlocker.crosspointreader.com/firmware/firmware.binContent-LengthCPR-vCodex behaviour
Validation
cargo fmt -p unlocker-core -- --checkcargo test --locked -p unlocker-core— 4 passedcargo check --locked -p unlocker-corecargo clippy --locked -p unlocker-core --all-targetsgit diff --checkThe public repository omits the production
privkey.pem, so no helper orinstaller build was attempted. No replacement key, self-signed certificate
or TLS bypass was introduced.
Certificate action required
The certificate currently committed in
fullchain.pemexpires on2026-07-30. Any Windows test installer must use a currently valid
unlocker.crosspointreader.comcertificate, complete chain and themaintainers' legitimate matching private key.
Maintainer test request
Please produce a Windows test installer using the legitimate renewed
certificate infrastructure.
For the first device test:
Check for Updatesonce.size, ESP32-C3 chip ID and returned manifest have been verified.