Skip to content

Second pass: close two unauthenticated webhook holes, ship a musl TUI build - #22

Merged
felirami merged 2 commits into
mainfrom
review/pass-2
Jul 25, 2026
Merged

Second pass: close two unauthenticated webhook holes, ship a musl TUI build#22
felirami merged 2 commits into
mainfrom
review/pass-2

Conversation

@felirami

Copy link
Copy Markdown
Owner

Follow-up to #21. Reviews the surfaces the first pass never reached — payments, webhooks, db,
results — and closes the Alpine limitation that pass documented. Everything below was verified
against a running server or a real Alpine machine, not just unit tests.

Two unauthenticated webhook endpoints

/webhooks/resend was an open email relay. It checked only that the svix-id,
svix-timestamp and svix-signature headers were present — never that the signature was
valid — and skipped even that when no secret was configured. The route sends mail from
arca@arcabot.ai to a real inbox, so anyone who knew the URL could forge an email.received
event and choose the from, subject and body.

The Lemon Squeezy webhook could not verify a real signature. It hashed
JSON.stringify(req.body) — the parsed object re-serialized, not the bytes Lemon Squeezy
signed — so a genuine signature never matched, compared with !== rather than constant-time,
and accepted any POST as a payment notification when no secret was set.

Both now verify over the raw bytes (captured in the json parser for /webhook routes) and fail
closed on an unset secret, a missing raw body, or a bad signature. Svix verification follows the
spec: HMAC-SHA256 over id.timestamp.rawBody, constant-time compare, timestamp tolerance to
bound replay.

Live against the running server:

forged Resend delivery,   secret unset      -> HTTP 401   Rejected: not_configured
forged Lemon Squeezy,     secret unset      -> HTTP 401   Rejected: not_configured
correctly signed Resend,  secret configured -> HTTP 200   {"received":true}
forged signature,         secret configured -> HTTP 401

Two more in the same path: email_id came from the webhook body and was interpolated into a
Resend API path called with the full-access key (now constrained to an opaque id and
encoded), and inbound from/to/subject went unescaped into the forwarded HTML.

The TUI now runs on Alpine

@opentui/core resolves its native library by a hardcoded package name with no musl detection,
so on Alpine it asked for @opentui/core-linux-x64 and failed — gcompat did not help, and
neither did installing the musl package, because the loader asks for the glibc one by name.

The new linux-x64-musl target compiles with bun-linux-x64-musl and stages the musl library
under the asset key the runtime actually asks for. Driven through a PTY on a real Alpine 3.21
machine:

{ "ok": true, "mode": "pty", "rendered": true, "acceptsInput": true, "exitCode": "0" }

Added to the release matrix, so Alpine-based OpenClaw containers get a working binary. Its
runtime smoke is skipped on the glibc runner where it cannot execute; the build is still verified
there.

Site

Binaries are described as covering glibc and musl hosts, the stale "PR #20 is merged on main"
line is gone now that it has shipped, and the copy states that every release binary is driven
through a real terminal before publishing. Backed by assertions in tooling-regressions.

Gates

392 node tests, 87 TUI tests, tsc clean, npm audit clean, darwin binary still renders/accepts
input/exits, musl binary verified on Alpine.

Flagged, not fixed

src/routes/payment.js handles order_created with // TODO: Mark fix as paid in database.
With Lemon Squeezy configured a user can be charged $2 and nothing records it or unlocks
anything. That needs a product decision — record and gate on payment, or remove the paid path
until it is real — so it is called out rather than guessed at. Details in REVIEW.md.

Cad from Arca added 2 commits July 25, 2026 06:19
Both webhook endpoints accepted forged requests.

/webhooks/resend checked only that the svix-id, svix-timestamp and
svix-signature headers were *present*, never that the signature was valid — and
skipped even that when no secret was configured. The route sends mail from
arca@arcabot.ai to a real inbox, so anyone who knew the URL had an
unauthenticated relay: forge an email.received event, choose the from, subject
and body, and the server mails it. Now verified as Svix specifies — HMAC-SHA256
over `id.timestamp.rawBody` with the base64 secret, constant-time compare, and a
timestamp tolerance so a captured delivery cannot be replayed.

/api/webhook/lemonsqueezy hashed `JSON.stringify(req.body)` — the parsed object
re-serialized, not the bytes Lemon Squeezy signed — so a genuine signature could
not match, and it compared with `!==` rather than a constant-time compare. It
also skipped verification entirely without a secret, accepting any POST as a
payment notification.

Both now fail closed on an unset secret, a missing raw body, or a bad signature.
The raw body is captured in the json parser for /webhook routes, since a
signature can only be checked against the bytes that were sent.

Also in the Resend path: `email_id` came from the webhook body and was
interpolated into a Resend API path called with the full-access key, so it is
now constrained to an opaque id and encoded; and inbound from/to/subject are
escaped before being interpolated into the forwarded HTML.

Verified live against the running server: forged deliveries get 401 with both
secrets unset, a correctly signed Resend delivery gets 200, and a forged
signature against a configured secret gets 401.
@opentui/core resolves its native library by a hardcoded package name per
platform with no musl detection, so on Alpine it asked for
@opentui/core-linux-x64 and failed to load — all 23 render tests failed there,
and neither gcompat nor installing the musl package helped, because the loader
asks for the glibc package by name.

The new linux-x64-musl target compiles with bun-linux-x64-musl and stages the
musl library under the asset key the runtime actually asks for. Verified on a
real Alpine 3.21 machine by driving the compiled binary through a PTY: it
renders, accepts typed input, and exits cleanly.

Added to the release matrix. Its runtime smoke is skipped on the glibc runner,
where the binary cannot execute; the build is still verified there.

Site copy updated to match what ships: binaries for glibc and musl hosts, and
the fact that every release binary is driven through a real terminal first. Also
drops the stale "PR #20 is merged on main" line now that it has shipped.
@felirami
felirami requested a review from arcabotai as a code owner July 25, 2026 10:26
@felirami
felirami merged commit 84fa75b into main Jul 25, 2026
3 checks passed
@felirami
felirami deleted the review/pass-2 branch July 25, 2026 10:26
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