fix(feishu): route /btw through out-of-band lanes#64324
Conversation
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟠 Feishu /btw and /stop bypass per-chat FIFO and debounce, enabling parallel expensive work (DoS/cost amplification)
DescriptionThe Feishu message dispatcher now routes This introduces a cost/DoS amplification vector:
Vulnerable behavior (new routing): // monitor.account.ts
const enqueue = createSequentialQueue();
...
const sequentialKey = getFeishuSequentialKey({ accountId, event, ... });
...
await enqueue(sequentialKey, task);// sequential-key.ts
if (isAbortRequestText(text)) return `${baseKey}:control`;
if (isBtwRequestText(text)) return `${baseKey}:btw`;
return baseKey;If RecommendationAdd explicit throttling/backpressure so Options (pick one or combine):
// Pseudocode
const perChatLimiter = new Map<string, Semaphore>(/* max=1 */);
await perChatLimiter.get(chatId).runExclusive(task);
if (isBtwRequestText(text) && !btwLimiter.allow(chatId)) {
return; // or reply with "Too many /btw requests"
}
Analyzed PR: #64324 at commit Last updated on: 2026-04-10T14:51:46Z |
openclaw-barnacle
bot
added
channel: feishu
Greptile SummaryThis PR fixes a Feishu-specific scheduling bug where Confidence Score: 5/5Safe to merge — change is well-scoped, logically equivalent for normal traffic, and directly tested. The createSequentialQueue extraction is byte-for-byte identical in semantics to the removed createChatQueue. The key classifier correctly covers all three lanes (normal, control, btw) with a per-message-id btw key that prevents any same-key collision between concurrent side questions. Both new modules have focused unit tests that prove the FIFO invariant and the concurrency bypass. No P0 or P1 findings. No files require special attention. Reviews (1): Last reviewed commit: "fix(feishu): route /btw through out-of-b..." | Re-trigger Greptile |
|
Thanks — good catch. I agreed with the core concern: the original I pushed a follow-up fix in 7886b99 that bounds BTW concurrency per chat instead of per message:
That preserves the original bug fix for #60843 (BTW no longer waits behind the busy normal chat lane) while restoring bounded lane cardinality and backpressure. Also added coverage to lock in that multiple BTW messages in the same chat resolve to the same stable lane. Verified again with:
@aisle-research-bot please re-review the latest commit. |
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Queue key collision/DoS via constant fallback sequential key when chat_id is missing
Description
This creates a reliability/security risk:
Vulnerable code: const chatId = event.message.chat_id?.trim() || "unknown";
const baseKey = `feishu:${accountId}:${chatId}`;RecommendationAvoid a constant shared fallback key for missing/blank Preferred fixes (choose one):
const chatId = event.message.chat_id?.trim();
if (!chatId) {
throw new Error("Missing chat_id in Feishu message event");
}
const baseKey = `feishu:${accountId}:${chatId}`;
const chatId = event.message.chat_id?.trim();
const fallback = event.message.message_id?.trim();
const keyId = chatId ?? (fallback ? `nochat:${fallback}` : `nochat:${crypto.randomUUID()}`);
const baseKey = `feishu:${accountId}:${keyId}`;Also consider logging/metrics for missing Analyzed PR: #64324 at commit Last updated on: 2026-04-10T14:14:33Z |
ngutman
force-pushed
the
fix/feishu-btw-out-of-band
branch
from
7886b99 to
4fbde69
Compare
|
Landed via temp rebase onto main.
Thanks @ngutman! |
* fix(feishu): route /btw through out-of-band lanes * fix(feishu): bound btw out-of-band lanes * fix: route feishu btw out-of-band (openclaw#64324) (thanks @ngutman)
1 participant
Summary
/btwwaits behind an active normal reply instead of behaving like an out-of-band side question./btwplus abort-style requests onto dedicated out-of-band keys.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
extensions/feishu/src/monitor.account.ts, so/btwnever reached the existing fast inline-command path until the prior message finished./btwor/stop, unlike channels that already special-case those commands.Regression Test Plan (if applicable)
extensions/feishu/src/sequential-key.test.ts,extensions/feishu/src/sequential-queue.test.ts/btwand/stopresolve to separate keys that can run concurrently with the busy main chat lane.src/gateway/server.chat.gateway-server-chat.test.tsalready covers core/btwside-result behavior outside Feishu.User-visible / Behavior Changes
/btw ...no longer waits behind a busy normal message in the same chat./stopalso bypasses the normal same-chat lane instead of waiting behind ordinary traffic.Diagram (if applicable)
Security Impact (required)
No)No)No)No)No)Yes, explain risk + mitigation: N/ARepro + Verification
Environment
Steps
/btw <question>in the same Feishu chat while the first reply is still running.Expected
/btwruns immediately without waiting behind the normal chat lane.Actual
/btwbehind the active normal chat task./btwto a dedicated sequential key and no longer blocks it behind ordinary traffic.Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
pnpm test extensions/feishu/src/sequential-key.test.ts extensions/feishu/src/sequential-queue.test.ts;pnpm build/btwmissing-message-id fallback key;/stopstays out-of-band; ordinary slash commands like/statusremain on the normal chat lane.Review Conversations
If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.
Compatibility / Migration
Yes)No)No)Risks and Mitigations
/btwand abort-style requests get dedicated out-of-band keys; normal messages and ordinary control commands keep the base chat key.