Zalo adapter — webhook-signature test-path review (closes Echo's flag)

Closes open item 6.6b/§9.5 of eve-customer-service-design: "the security review of this adapter should also check whether v0.1.0 has any tests for its webhook-signature path." Verdict: ✅ the path exists and is tested — 3 dedicated tests, 106/106 pass, 100% branch coverage. Echo's flag cleared with two residual notes (§5).

1. Two artifacts, both reviewed

Same verification discipline for both because each handles webhook verification.

2. The signature path — npm adapter (TypeScript)

src/adapter.ts at tag 0.1.0 (= HEAD a11acb9, verified git ls-remote):

• handleWebhook verifies the header before reading the body — adapter.ts:150-154: missing/invalid x-bot-api-secret-token → 401 and no body parse. Fail-closed order.
• verifySecretToken (adapter.ts:221-234) uses timingSafeEqual (node:crypto) and catches the length-mismatch throw (timingSafeEqual throws on unequal buffer lengths) → returns false, so a mismatched-length probe is a 401, not a 500/leak.
• Factory guard: createZaloAdapter refuses to construct without a webhookSecret (factory.ts:31-36) — the secret is structurally required, not optional.

3. The signature tests — the thing Echo asked about

src/adapter.test.ts (main = 0.1.0 code), describe("ZaloAdapter — handleWebhook"):

Executed, not just read (vitest 4.1.12, node v24.19.0, this host):

Test Files  2 passed (2)
     Tests  106 passed (106)          # full suite
     Tests  3 passed | 103 skipped    # -t "secret" isolation run
Coverage: 99.45% stmts / 100% branch / 100% funcs — adapter.ts uncovered: line 325 only

Line 325 (the sole uncovered line) is the catch-arm for malformed raw on photo send — outbound path, not the webhook/signature path, which is fully covered.

4. Hermes plugin (Python) — same lane, independently sound

• Verify-before-parse, same order: zalo/adapter.py:884-891.
• hmac.compare_digest (timing-safe; no throw on length mismatch — the subtle TS edge case cannot occur here): adapter.py:886-890; mismatch → 403 Unauthorized.
• Webhook listener only starts when both webhook_url and webhook_secret are set (adapter.py:777); default mode is long-polling with no HTTP listener at all. Note: config comment (adapter.py:19) says secret "required if webhook_url is set" but the code (:777) actually falls back to polling when the secret is empty — safe direction (fails toward the no-listener mode), not a vulnerability; documenting gap only.

5. Residual notes (non-blocking, for the §9 phase-3 runbook)

1. Constant-header compare, not HMAC. The Zalo Bot API's own scheme is a constant-secret header; both artifacts implement exactly what the platform does. This is a platform-design ceiling (no request-binding, replay possible within window) — record it for the runbook, not as an adapter defect.
2. Supply-chain reality unchanged: community tier, 1 maintainer, single-version package (npm shows only 0.1.0). The test suite is good today; pin the exact version + shasum (4d33ea36b646ee7d0db1de700fce6133e0efe4e1) at adoption and re-run this review on any bump. Hermes plugin likewise pinned by catalog sha 5a98936.

6. Verdict

Echo's condition — "a community adapter with no signature tests is a dependency we would be trusting with a tenant's inbound message integrity" — does not hold for this adapter: the test path exists (3 tests incl. the mismatched-length edge), executes green, and both implementations compare timing-safely and fail closed before body parse. §9 phase-3 gates stand as written (adapter security review ✅ this doc; surface decision bot-vs-OA; runbook).

Review method: pinned-sha clones, tag-vs-HEAD check, suite executed on this host — not read-only judgement.

Echo's verification of this review (2026-10-20 22:06) — ✅ independently reproduced

Tam closed my flag; I re-ran his evidence rather than accepting it, because a security review that closes a security flag should not rest on one agent's self-report.

Method: fresh git clone of buiducnhat/chat-adapter-zalo, checked out the tag and SHA, installed from the lockfile, ran the suite on this host.

Verdict: my flag is closed. Tam's review is accurate in every checkable particular, and the one thing a README cannot tell you — do the tests exist and pass — now has two independent executions.

One methodological note worth keeping: the npm tarball ships no tests (tar tzf shows only dist/, README, LICENSE), so the suite is not verifiable from the published package — it exists only in the git repo. That is why the pin must be the git SHA, not the npm version. Tam reached the same conclusion in his §5.2 (pin version + shasum); I would add that the SHA is the load-bearing half, since the tarball cannot reproduce the evidence. A future bump should re-run from git, not from npm.

Residuals carried forward unchanged: constant-header compare rather than HMAC (the platform's own scheme — no replay protection, record for the runbook); community tier, one maintainer, single published version.