From 42eb6ea410569b7a003f240adcd55d519406445d Mon Sep 17 00:00:00 2001
From: Hoshina <hoshina@evaz.org>
Date: Tue, 3 Mar 2026 01:27:39 +0800
Subject: [PATCH] fix(feishu): address review findings

- Remove stale "falls back to plain text" comment on Send
- Add empty ChatID validation in SendMedia to match Send
- Use messageID+fileKey as local filename to avoid write collisions
- Check allowlist before downloading inbound media to avoid wasted I/O
- Return errUnsupported consistently from all 32-bit stub methods
---
 pkg/channels/feishu/feishu_32.go | 16 +++++++++-------
 pkg/channels/feishu/feishu_64.go | 27 ++++++++++++++++++---------
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/pkg/channels/feishu/feishu_32.go b/pkg/channels/feishu/feishu_32.go
index 62d6d95cb..f5e3aa224 100644
--- a/pkg/channels/feishu/feishu_32.go
+++ b/pkg/channels/feishu/feishu_32.go
@@ -16,6 +16,8 @@ type FeishuChannel struct {
 	*channels.BaseChannel
 }
 
+var errUnsupported = errors.New("feishu channel is not supported on 32-bit architectures")
+
 // NewFeishuChannel returns an error on 32-bit architectures where the Feishu SDK is not supported
 func NewFeishuChannel(cfg config.FeishuConfig, bus *bus.MessageBus) (*FeishuChannel, error) {
 	return nil, errors.New(
@@ -25,35 +27,35 @@ func NewFeishuChannel(cfg config.FeishuConfig, bus *bus.MessageBus) (*FeishuChan
 
 // Start is a stub method to satisfy the Channel interface
 func (c *FeishuChannel) Start(ctx context.Context) error {
-	return nil
+	return errUnsupported
 }
 
 // Stop is a stub method to satisfy the Channel interface
 func (c *FeishuChannel) Stop(ctx context.Context) error {
-	return nil
+	return errUnsupported
 }
 
 // Send is a stub method to satisfy the Channel interface
 func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) error {
-	return errors.New("feishu channel is not supported on 32-bit architectures")
+	return errUnsupported
 }
 
 // EditMessage is a stub method to satisfy MessageEditor
 func (c *FeishuChannel) EditMessage(ctx context.Context, chatID, messageID, content string) error {
-	return nil
+	return errUnsupported
 }
 
 // SendPlaceholder is a stub method to satisfy PlaceholderCapable
 func (c *FeishuChannel) SendPlaceholder(ctx context.Context, chatID string) (string, error) {
-	return "", nil
+	return "", errUnsupported
 }
 
 // ReactToMessage is a stub method to satisfy ReactionCapable
 func (c *FeishuChannel) ReactToMessage(ctx context.Context, chatID, messageID string) (func(), error) {
-	return func() {}, nil
+	return func() {}, errUnsupported
 }
 
 // SendMedia is a stub method to satisfy MediaSender
 func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error {
-	return nil
+	return errUnsupported
 }
diff --git a/pkg/channels/feishu/feishu_64.go b/pkg/channels/feishu/feishu_64.go
index 24ab4fa85..7aa24588a 100644
--- a/pkg/channels/feishu/feishu_64.go
+++ b/pkg/channels/feishu/feishu_64.go
@@ -105,7 +105,6 @@ func (c *FeishuChannel) Stop(ctx context.Context) error {
 }
 
 // Send sends a message using Interactive Card format for markdown rendering.
-// Falls back to plain text if card building fails.
 func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) error {
 	if !c.IsRunning() {
 		return channels.ErrNotRunning
@@ -245,6 +244,10 @@ func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMess
 		return channels.ErrNotRunning
 	}
 
+	if msg.ChatID == "" {
+		return fmt.Errorf("chat ID is empty: %w", channels.ErrSendFailed)
+	}
+
 	store := c.GetMediaStore()
 	if store == nil {
 		return fmt.Errorf("no media store available: %w", channels.ErrSendFailed)
@@ -330,6 +333,17 @@ func (c *FeishuChannel) handleMessageReceive(ctx context.Context, event *larkim.
 	messageID := stringValue(message.MessageId)
 	rawContent := stringValue(message.Content)
 
+	// Check allowlist early to avoid downloading media for rejected senders.
+	// BaseChannel.HandleMessage will check again, but this avoids wasted network I/O.
+	senderInfo := bus.SenderInfo{
+		Platform:    "feishu",
+		PlatformID:  senderID,
+		CanonicalID: identity.BuildCanonicalID("feishu", senderID),
+	}
+	if !c.IsAllowedSender(senderInfo) {
+		return nil
+	}
+
 	// Extract content based on message type
 	content := extractContent(messageType, rawContent)
 
@@ -390,12 +404,6 @@ func (c *FeishuChannel) handleMessageReceive(ctx context.Context, event *larkim.
 		"preview":    utils.Truncate(content, 80),
 	})
 
-	senderInfo := bus.SenderInfo{
-		Platform:    "feishu",
-		PlatformID:  senderID,
-		CanonicalID: identity.BuildCanonicalID("feishu", senderID),
-	}
-
 	c.HandleMessage(ctx, peer, messageID, senderID, chatID, content, mediaRefs, metadata, senderInfo)
 	return nil
 }
@@ -569,7 +577,7 @@ func (c *FeishuChannel) downloadResource(
 		filename += fallbackExt
 	}
 
-	// Write to the shared picoclaw_media directory using the original filename.
+	// Write to the shared picoclaw_media directory using a unique name to avoid collisions.
 	mediaDir := filepath.Join(os.TempDir(), "picoclaw_media")
 	if mkdirErr := os.MkdirAll(mediaDir, 0o700); mkdirErr != nil {
 		logger.ErrorCF("feishu", "Failed to create media directory", map[string]any{
@@ -577,7 +585,8 @@ func (c *FeishuChannel) downloadResource(
 		})
 		return ""
 	}
-	localPath := filepath.Join(mediaDir, utils.SanitizeFilename(filename))
+	ext := filepath.Ext(filename)
+	localPath := filepath.Join(mediaDir, utils.SanitizeFilename(messageID+"-"+fileKey+ext))
 
 	out, err := os.Create(localPath)
 	if err != nil {