fix(feishu): repair interactive card content extraction#72397
fix(feishu): repair interactive card content extraction#72397vincentkoc merged 1 commit intomainfrom
Conversation
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Sensitive data exposure via interactive card template variable expansion
Description
If card template variables contain secrets/PII (tokens, internal IDs, user data), the expansion increases the chance of leaking sensitive values because the extracted text is subsequently used outside Feishu:
Vulnerable code (template expansion): return text.replace(/\$\{([A-Za-z0-9_.-]+)\}|\{\{\s*([A-Za-z0-9_.-]+)\s*\}\}/g, (match, a, b) => {
const variableName = typeof a === "string" ? a : b;
return variables.get(variableName) ?? match;
});RecommendationAvoid expanding template variables when producing agent/log-facing plaintext, or strictly limit what can be expanded. Options:
function applyCardTemplateVariables(text: string, _variables: Map<string, string>): string {
return text; // keep placeholders
}
const SAFE_VARS = new Set(["user_name", "title"]);
return text.replace(/\$\{([A-Za-z0-9_.-]+)\}|\{\{\s*([A-Za-z0-9_.-]+)\s*\}\}/g, (match, a, b) => {
const name = (typeof a === "string" ? a : b) ?? "";
if (!SAFE_VARS.has(name)) return match;
return variables.get(name) ?? match;
});
Additionally, consider adding a config flag (default: off) controlling variable expansion for interactive-card text extraction. Analyzed PR: #72397 at commit Last updated on: 2026-04-26T22:28:51Z |
Greptile SummaryThis PR refactors Confidence Score: 4/5Safe to merge; no critical defects found in the changed paths. Only P2-level findings: a minor JSON round-trip inefficiency in the post-fallback and a non-deterministic locale-selection order for extensions/feishu/src/send.ts — Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/feishu/src/send.ts
Line: 285
Comment:
**Unnecessary JSON round-trip in post fallback**
`parsed` was already deserialized by `parseFeishuMessageContent` before reaching here; `JSON.stringify(parsed)` re-serializes it only so `parsePostContent` can re-parse it. This is wasteful and introduces a subtle risk if `parsed` contains non-JSON-serializable values (e.g. `bigint` from `normalizeCardTemplateVariable`, though only as template variable values, not the card structure itself). Consider exposing an overload or a shared helper that accepts the already-parsed object instead.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: extensions/feishu/src/send.ts
Line: 257-281
Comment:
**`i18n_elements` locale iteration order is insertion-order-dependent**
`Object.values(candidate)` iterates `i18n_elements` locales in JSON insertion order (V8 preserves string-key insertion order, so this is reliable within Node.js). However, the first locale with non-empty elements wins — there is no preference for a user/bot locale or any explicit priority (e.g. `en_us` before `zh_cn`). If a card carries multiple locales with different content, the result depends on whichever locale was serialized first by the sender. This may produce surprising output when a preferred locale is available but not first in the object.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(feishu): repair interactive card con..." | Re-trigger Greptile |
| } | ||
|
|
||
| function parseInteractivePostFallback(parsed: unknown): string | undefined { | ||
| const textContent = parsePostContent(JSON.stringify(parsed)).textContent.trim(); |
There was a problem hiding this comment.
parsed was already deserialized by parseFeishuMessageContent before reaching here; JSON.stringify(parsed) re-serializes it only so parsePostContent can re-parse it. This is wasteful and introduces a subtle risk if parsed contains non-JSON-serializable values (e.g. bigint from normalizeCardTemplateVariable, though only as template variable values, not the card structure itself). Consider exposing an overload or a shared helper that accepts the already-parsed object instead.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/send.ts
Line: 285
Comment:
**Unnecessary JSON round-trip in post fallback**
`parsed` was already deserialized by `parseFeishuMessageContent` before reaching here; `JSON.stringify(parsed)` re-serializes it only so `parsePostContent` can re-parse it. This is wasteful and introduces a subtle risk if `parsed` contains non-JSON-serializable values (e.g. `bigint` from `normalizeCardTemplateVariable`, though only as template variable values, not the card structure itself). Consider exposing an overload or a shared helper that accepts the already-parsed object instead.
How can I resolve this? If you propose a fix, please make it concise.| return texts.join("\n").trim(); | ||
| } | ||
|
|
||
| function readInteractiveElementArrays(parsed: Record<string, unknown>): unknown[][] { | ||
| const body = isRecord(parsed.body) ? parsed.body : undefined; | ||
| const elementArrays: unknown[][] = []; | ||
|
|
||
| for (const candidate of [parsed.elements, body?.elements]) { | ||
| if (Array.isArray(candidate)) { | ||
| elementArrays.push(candidate); | ||
| } | ||
| } | ||
|
|
||
| for (const candidate of [parsed.i18n_elements, body?.i18n_elements]) { | ||
| if (!isRecord(candidate)) { | ||
| continue; | ||
| } | ||
| if (item.tag === "markdown" && typeof item.content === "string") { | ||
| texts.push(item.content); | ||
| for (const localeElements of Object.values(candidate)) { | ||
| if (Array.isArray(localeElements)) { | ||
| elementArrays.push(localeElements); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return elementArrays; |
There was a problem hiding this comment.
i18n_elements locale iteration order is insertion-order-dependent
Object.values(candidate) iterates i18n_elements locales in JSON insertion order (V8 preserves string-key insertion order, so this is reliable within Node.js). However, the first locale with non-empty elements wins — there is no preference for a user/bot locale or any explicit priority (e.g. en_us before zh_cn). If a card carries multiple locales with different content, the result depends on whichever locale was serialized first by the sender. This may produce surprising output when a preferred locale is available but not first in the object.
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/feishu/src/send.ts
Line: 257-281
Comment:
**`i18n_elements` locale iteration order is insertion-order-dependent**
`Object.values(candidate)` iterates `i18n_elements` locales in JSON insertion order (V8 preserves string-key insertion order, so this is reliable within Node.js). However, the first locale with non-empty elements wins — there is no preference for a user/bot locale or any explicit priority (e.g. `en_us` before `zh_cn`). If a card carries multiple locales with different content, the result depends on whichever locale was serialized first by the sender. This may produce surprising output when a preferred locale is available but not first in the object.
How can I resolve this? If you propose a fix, please make it concise.
Summary
Repair Feishu interactive-card content extraction for quoted/replied cards while keeping the patch limited to the existing parser and tests.
This carries forward the narrow work from #38776 and adds the review-bot-requested fallbacks for empty
elements/body.elements/i18n_elements, locale fallback when the preferred locale has no extractable text, non-string template variables, and focused post-format fallback coverage from #60383.Credit
Validation
pnpm check:changedNotes
This does not close the broader inbound interactive-card parsing work tracked by #41609 and #56795.
ProjectClownfish replacement details: