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
This commit is contained in:
Hoshina 2026-03-03 01:27:39 +08:00
parent d21fd1492c
commit 9e584f306e
2 changed files with 27 additions and 16 deletions

View file

@ -16,6 +16,8 @@ type FeishuChannel struct {
*channels.BaseChannel *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 // 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) { func NewFeishuChannel(cfg config.FeishuConfig, bus *bus.MessageBus) (*FeishuChannel, error) {
return nil, errors.New( 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 // Start is a stub method to satisfy the Channel interface
func (c *FeishuChannel) Start(ctx context.Context) error { func (c *FeishuChannel) Start(ctx context.Context) error {
return nil return errUnsupported
} }
// Stop is a stub method to satisfy the Channel interface // Stop is a stub method to satisfy the Channel interface
func (c *FeishuChannel) Stop(ctx context.Context) error { func (c *FeishuChannel) Stop(ctx context.Context) error {
return nil return errUnsupported
} }
// Send is a stub method to satisfy the Channel interface // Send is a stub method to satisfy the Channel interface
func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) error { 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 // EditMessage is a stub method to satisfy MessageEditor
func (c *FeishuChannel) EditMessage(ctx context.Context, chatID, messageID, content string) error { func (c *FeishuChannel) EditMessage(ctx context.Context, chatID, messageID, content string) error {
return nil return errUnsupported
} }
// SendPlaceholder is a stub method to satisfy PlaceholderCapable // SendPlaceholder is a stub method to satisfy PlaceholderCapable
func (c *FeishuChannel) SendPlaceholder(ctx context.Context, chatID string) (string, error) { func (c *FeishuChannel) SendPlaceholder(ctx context.Context, chatID string) (string, error) {
return "", nil return "", errUnsupported
} }
// ReactToMessage is a stub method to satisfy ReactionCapable // ReactToMessage is a stub method to satisfy ReactionCapable
func (c *FeishuChannel) ReactToMessage(ctx context.Context, chatID, messageID string) (func(), error) { 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 // SendMedia is a stub method to satisfy MediaSender
func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error { func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error {
return nil return errUnsupported
} }

View file

@ -105,7 +105,6 @@ func (c *FeishuChannel) Stop(ctx context.Context) error {
} }
// Send sends a message using Interactive Card format for markdown rendering. // 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 { func (c *FeishuChannel) Send(ctx context.Context, msg bus.OutboundMessage) error {
if !c.IsRunning() { if !c.IsRunning() {
return channels.ErrNotRunning return channels.ErrNotRunning
@ -245,6 +244,10 @@ func (c *FeishuChannel) SendMedia(ctx context.Context, msg bus.OutboundMediaMess
return channels.ErrNotRunning return channels.ErrNotRunning
} }
if msg.ChatID == "" {
return fmt.Errorf("chat ID is empty: %w", channels.ErrSendFailed)
}
store := c.GetMediaStore() store := c.GetMediaStore()
if store == nil { if store == nil {
return fmt.Errorf("no media store available: %w", channels.ErrSendFailed) 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) messageID := stringValue(message.MessageId)
rawContent := stringValue(message.Content) 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 // Extract content based on message type
content := extractContent(messageType, rawContent) content := extractContent(messageType, rawContent)
@ -390,12 +404,6 @@ func (c *FeishuChannel) handleMessageReceive(ctx context.Context, event *larkim.
"preview": utils.Truncate(content, 80), "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) c.HandleMessage(ctx, peer, messageID, senderID, chatID, content, mediaRefs, metadata, senderInfo)
return nil return nil
} }
@ -569,7 +577,7 @@ func (c *FeishuChannel) downloadResource(
filename += fallbackExt 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") mediaDir := filepath.Join(os.TempDir(), "picoclaw_media")
if mkdirErr := os.MkdirAll(mediaDir, 0o700); mkdirErr != nil { if mkdirErr := os.MkdirAll(mediaDir, 0o700); mkdirErr != nil {
logger.ErrorCF("feishu", "Failed to create media directory", map[string]any{ logger.ErrorCF("feishu", "Failed to create media directory", map[string]any{
@ -577,7 +585,8 @@ func (c *FeishuChannel) downloadResource(
}) })
return "" 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) out, err := os.Create(localPath)
if err != nil { if err != nil {