Skip to content

Comments

feat(slack): P0+P1 fixes - path security, ACK retry, gap analysis#23

Merged
hrygo merged 10 commits intomainfrom
feat/slack-media-support
Feb 24, 2026
Merged

feat(slack): P0+P1 fixes - path security, ACK retry, gap analysis#23
hrygo merged 10 commits intomainfrom
feat/slack-media-support

Conversation

@hrygo
Copy link
Owner

@hrygo hrygo commented Feb 24, 2026

Summary

Addresses critical feature gaps identified in Issue #21 (Slack vs OpenClaw comparison report):

P0 - Security & Reliability:

  • ✅ Path traversal attack protection (blocks access to /etc, /var, /root, etc.)
  • ✅ Socket Mode ACK retry with exponential backoff (fixes duplicate message processing)
  • ✅ Sensitive path detection and blocking

P1 - Code Quality:

  • ✅ Comprehensive unit tests for expandPath (93.8% coverage)
  • ✅ Fix duplicate event handler call in Socket Mode
  • ✅ Proper Slack API acknowledgment (3-second requirement)

Changes

Security Fixes (chatapps/setup.go)

  • Add expandPath() with ~ expansion and path cleaning
  • Add isSensitivePath() to block system directories
  • Block path traversal attempts (e.g., ../etc/passwd)

Reliability Fixes (chatapps/slack/socket_mode.go)

  • Remove duplicate handleEventsAPI() call
  • Implement sendACKWithRetry() with exponential backoff
  • Slack-compliant ACK response (envelope_id within 3 seconds)

Documentation (docs/chatapps/)

Configuration (chatapps/configs/slack.yaml)

  • Document path security features
  • Document ACK retry mechanism
  • Add system prompt injection explanation
  • Add troubleshooting examples

Testing

# Run all tests
go test ./chatapps/... -v

# Test expandPath
go test ./chatapps/... -run TestExpandPath

# Build verification
go build ./...

# Linter check
golangci-lint run ./chatapps/...

Test Coverage:

  • expandPath: 93.8%
  • isSensitivePath: 100%
  • All existing tests: ✅ PASS

Verification

  • go test ./... - All tests pass
  • go build ./... - Build succeeds
  • golangci-lint run - No issues
  • Path security blocks sensitive directories
  • ACK retry handles connection failures

Related Issues

Remaining Gaps (Future Work)

Per Issue #21 analysis, the following features are still pending:

P0 (Critical):

  • Slash Commands support
  • Slack Actions API (sendMessage, editMessage, etc.)
  • Block Kit support
  • Multi-account support

P1 (Enhancement):

  • Streaming/typing indicators
  • Advanced Markdown conversion (tables, lists)
  • DM/Channel policies
  • Interactive messages

See docs/chatapps/slack-gap-analysis.md for complete roadmap.

黄飞虹 and others added 10 commits February 25, 2026 01:12
…al communication

- Fix Socket Mode payload parsing: use "payload" field instead of "body"
- Add proper Slack API response validation (check "ok" field)
- Fix subtype filtering: allow file_share/bot_message, only skip message_changed/deleted
- Fix nil pointer panic in EngineMessageHandler by checking workDirFn and taskInstrFn
- Preserve original message metadata (channel_id, thread_ts) in StreamCallback for replies

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When bot is mentioned in a channel with @botName, Slack sends app_mention
event instead of regular message event. Register both handlers to support
both DMs and channel mentions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Slack bot now uses current working directory (HotPlex source) as workspace
- Other platforms continue using /tmp/hotplex-chatapps for isolation
- Add app_mention handler for @mentions in channels

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add SendAttachment method for sending files/images to Slack
- Add sendFileFromURL for external URL uploads
- Integrate with defaultSender to handle RichContent attachments
- Support thread replies for attachments
- Add engine.work_dir config option in slack.yaml
- Use "." (current directory) as default for HotPlex development
- Falls back to /tmp/hotplex-chatapps if not configured

This allows the Slack bot to operate on any directory specified in config,
making it easy to use for HotPlex development or other projects.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add comprehensive comparison analysis between HotPlex and OpenClaw Slack implementations
- Document 13 feature gaps across 6 categories (P0/P1/P2 priority)
- Include 3-phase implementation roadmap (14-20 weeks estimated)
- Identify technical debt risks in socket_mode.go and retry.go
- Reference: Issue #21
- Add expandPath helper function to expand ~ to absolute paths
- Update slack.yaml with engine.work_dir config option- Falls back to /tmp/hotplex-chatapps

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove duplicate s.handleEventsAPI call that caused double message processing
- Implement ACK response for Socket Mode events (required by Slack API)
- Slack expects envelope_id acknowledgment within 3 seconds

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add sensitive path detection to block access to /etc, /var, /root, etc.
- Add path traversal attack detection in expandPath
- Implement ACK retry with exponential backoff for Socket Mode
- Add comprehensive unit tests for expandPath function

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@hrygo hrygo changed the title feat(slack): add media/attachment sending support feat(slack): P0+P1 fixes - path security, ACK retry, gap analysis Feb 24, 2026
@hrygo hrygo merged commit 08c13a8 into main Feb 24, 2026
8 checks passed
@hrygo hrygo deleted the feat/slack-media-support branch February 24, 2026 18:13
hrygo pushed a commit that referenced this pull request Feb 24, 2026
- Add comprehensive v0.11.0 changelog with all P0+P1 fixes
- Document path traversal protection and ACK retry mechanism
- Include test coverage statistics (93.8%+)
- Link to PR #23, Issue #21, and release notes
- Follow Keep a Changelog format
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Slack 实现与 OpenClaw 差距分析报告 - 功能缺失与改进路线

1 participant