#16898: Fix Zalo webhook secret comparison vulnerable to timing attacks
channel: zalo
stale
size: S
trusted-contributor
Cluster:
Webhook Security Enhancements
The Zalo monitor webhook handler compares the incoming `x-bot-api-secret-token` header against the configured secret using plain `===` equality. This is susceptible to timing side-channel attacks that can incrementally recover the secret.
Other extensions already use constant-time comparison for webhook secrets:
- BlueBubbles → `timingSafeEqual`
- LINE → `crypto.timingSafeEqual`
- Browser HTTP auth → `safeEqualSecret`
This PR adds a local `safeEqualSecret` helper using `crypto.timingSafeEqual` and wires it into the webhook target matching.
**Tests:** 5 new test cases covering matching, mismatched, different-length, empty, and unicode secrets. Existing webhook tests continue to pass.
<!-- greptile_comment -->
<h3>Greptile Summary</h3>
Replaces the plain `===` webhook secret comparison in the Zalo extension with a constant-time `safeEqualSecret` helper using `crypto.timingSafeEqual`, mitigating timing side-channel attacks. The implementation follows the same pattern used by BlueBubbles and the shared `src/security/secret-equal.ts` utility. Since that utility isn't exported via the plugin SDK, the local duplication is justified.
- Security fix in `monitor.ts`: webhook secret comparison now uses `timingSafeEqual` instead of `===`
- New test file tests a duplicated copy of the helper rather than the production code; existing integration tests in `monitor.webhook.test.ts` already exercise the real path
<h3>Confidence Score: 4/5</h3>
- This PR is safe to merge — it's a straightforward, well-scoped security hardening change.
- The core change (replacing `===` with `timingSafeEqual`) is correct, follows established codebase patterns, and is minimal in scope. The only concern is the test file testing a copy rather than the production code, which is a minor quality issue but doesn't affect correctness.
- The test file `extensions/zalo/src/monitor.webhook-auth.test.ts` tests a duplicated copy of the function rather than the actual production code.
<sub>Last reviewed commit: f20bbf4</sub>
<!-- greptile_other_comments_section -->
<!-- /greptile_comment -->
Most Similar PRs
#8067: fix(telegram): use timing-safe comparison for webhook secret
by yubrew · 2026-02-03
79.6%
#8779: fix(security): use constant-time comparison for token validation
by hleliofficiel · 2026-02-04
74.3%
#17593: security: fail closed when LINE webhook secret is missing
by davidahmann · 2026-02-15
71.1%
#13521: telegram: require webhook secret in runtime webhook mode
by davidahmann · 2026-02-10
69.2%
#17182: security(line): fail closed when webhook token/secret are missing
by davidahmann · 2026-02-15
69.2%
#14197: fix(security): harden browser API auth, token comparisons, and hook...
by leecarollyn-gif · 2026-02-11
68.7%
#20775: fix(security): OC-10 add webhook payload schema validation to preve...
by aether-ai-agent · 2026-02-19
68.2%
#18825: fix(zalo): replace console.log/error with runtime logging
by Clawborn · 2026-02-17
67.2%
#16928: fix(security): OC-07 redact session history credentials and enforce...
by aether-ai-agent · 2026-02-15
66.6%
#11804: fix(webhook): return 503 from health endpoints when last processing...
by coygeek · 2026-02-08
65.5%