feat: outbound file attachments (addresses #355, opt-in, rate-limited)

[DrVictorChen]

Description

Closes #298. Addresses #355.

Agents running through OpenAB can receive files from users, but have no native path to send files back to the chat. This PR adds an opt-in, config-driven, rate-limited pathway: agents include ![alt](/path/to/file) markdown in their reply, OpenAB validates the path, uploads the file as a native attachment, and strips the marker from the visible text.

Supersedes #300. That PR proposed the same feature but was deferred pending the five security items listed in #355. This PR addresses all five plus one self-imposed hardening.

Changes

New platform-agnostic modules

• src/media.rs::extract_outbound_attachments(text, &OutboundConfig) — regex extraction + canonicalization + allowlist + size/count validation.
• src/outbound_rate.rs::OutboundRateLimiter — HashMap<channel_key, VecDeque<Instant>> 60-second sliding window, auto-prunes on each call.

ChatAdapter trait

• New send_file_attachments(channel, paths) method with a default no-op, so adapters that don't support native file upload silently drop the list instead of erroring. DiscordAdapter overrides to upload via serenity CreateAttachment::path + CreateMessage::add_file. Slack / future platforms just override the method.

Wiring

• AdapterRouter::stream_prompt — after the final text edit: extract markers → rate-admit → send_file_attachments with whatever the limiter granted.

Config (opt-in, top-level [outbound])

[outbound]
enabled = false                            # opt-in, disabled by default
allowed_dirs = ["/tmp/", "/var/folders/"]  # canonicalized at send time
max_file_size_mb = 25                      # matches Discord limit
max_per_message = 10                       # per agent response
max_per_minute_per_channel = 30            # sliding-window flood guard

Security checklist from #355

#	Requirement	Implementation
1	Symlink resolution (canonicalize)	std::fs::canonicalize() before allowlist check
2	Path traversal (..)	covered by the same canonicalize
3	Configurable allowed_dirs	[outbound].allowed_dirs in config.toml
4	Opt-in default	OutboundConfig::default().enabled == false
5	Rate limiting	per-message cap + per-channel per-minute sliding window
6 (extra)	Component-wise allowlist check	Path::starts_with on canonical paths, not str::starts_with (blocks /tmp-fake/x)

Markers that fail validation stay in the displayed text so the user sees what the agent tried — follows the Hermes Agent fallback-chain pattern rather than silent drop.

Verified

• cargo build passes
• cargo test — 54 passed (13 new: 8 in media::outbound_tests, 5 in outbound_rate::tests)
• cargo clippy --all-targets -- -D warnings -A clippy::manual_contains clean
• Four test scenarios covered end-to-end:
  • happy path (/tmp/ file, native upload)
  • symlink escape (ln -s /etc/hosts /tmp/x.png → blocked)
  • path traversal (![x](/tmp/../etc/hosts) → blocked)
  • rate-limit flood (5 requested vs 3 remaining slots → 3 granted, 2 dropped with warn)

Not in this PR

• The Makefile macOS AMFI codesign fix bundled into feat: outbound image/file attachments from agent → Discord #300 — will be a separate PR per masami-agent's suggestion on that review.

---

Discord Discussion URL: https://discord.com/channels/1491295327620169908/1493297320622821718

🤖 Generated with Claude Code

[masami-agent]

PR Review: #401

Summary

• Problem: Agents can receive files from users but have no native path to send files back to chat. Closes #298, addresses all security requirements from #355.
• Approach: ![alt](/path) markdown markers in agent responses → regex extraction → canonicalize + allowlist + size validation → native upload via ChatAdapter::send_file_attachments. Opt-in via [outbound] config. Per-channel sliding-window rate limiter in a separate module.
• Risk level: Medium (opens a filesystem → chat channel pathway, but the security model is solid)

Core Assessment

1. Problem clearly stated: ✅ (linked #298, #355, supersedes #300)
2. Approach appropriate: ✅ (platform-agnostic extraction, trait method with default no-op, standalone rate limiter)
3. Alternatives considered: ✅ (PR description references #300 and explains why it was deferred)
4. Best approach for now: ✅ (clean separation of concerns, opt-in by default)

Findings

Architecture — well-structured:

• media.rs — platform-agnostic extraction + validation (no Discord dependency)
• outbound_rate.rs — standalone rate limiter with HashMap<String, VecDeque<Instant>> sliding window
• ChatAdapter trait — send_file_attachments with default no-op (Slack silently drops, no breakage)
• AdapterRouter — wiring only, no business logic leakage

Security checklist from #355 — all 5 items addressed:

#	Requirement	Status
1	Symlink resolution (canonicalize)	✅
2	Path traversal (..)	✅
3	Configurable allowed_dirs	✅
4	Opt-in default	✅
5	Rate limiting	✅
6 (extra)	Component-wise Path::starts_with	✅

Review Summary

🔴 Blockers

• Branch is 29 commits behind main — must rebase before merge. The Cargo.lock diff will likely conflict. This is a hard blocker per repo merge rules.

💬 Questions

• canonicalize_allowlist() is called on every extract_outbound_attachments() invocation — this does filesystem I/O (std::fs::canonicalize) on each agent response. Was caching considered? For the default 2 dirs the cost is negligible, but worth documenting the trade-off or adding a note on why runtime canonicalization was preferred over startup-time caching (e.g. dirs could be mounted/unmounted at runtime).

🔧 Suggested Changes

• Discord upload is sequential per file — the for path in paths loop in discord.rs uploads one file at a time. serenity supports a single CreateMessage with multiple add_file() calls, which would reduce API calls and be more atomic. Not blocking, but a nice improvement.
• Config field naming — config uses max_file_size_mb but the method is max_size_bytes(). Consider max_file_size_bytes() to mirror the config field more closely, or add a doc comment on the method noting the MB → bytes conversion.

ℹ️ Info

• The Cargo.lock change (0.7.6 → 0.7.7) is not a version bump by this PR — it fixes a pre-existing inconsistency on main where Cargo.toml is already 0.7.7 but Cargo.lock still says 0.7.6.
• Slack adapter correctly inherits the default no-op send_file_attachments — no changes needed.
• Tests are solid: 8 in media::outbound_tests + 5 in outbound_rate::tests, covering happy path, symlink escape, path traversal, custom allowlist, size limit, per-message cap, multi-file stripping, and rate limiter edge cases.

⚪ Nits

• config.toml.example comment says 25 MB is Discord's limit — Discord's actual limit varies by boost level (8/25/50/100 MB). The default of 25 MB is a reasonable safe choice; the comment could note this nuance.

Verdict

COMMENT — code quality and security model are strong. The branch being 29 commits behind main is a hard blocker. After rebase (watch for Cargo.lock conflicts), this should be a clean approve. Nice work addressing all the #355 security items and splitting the Makefile fix into a separate PR as suggested.