agentscope-ai / agentscope-ai/QwenPaw

[Feature]: Unify media file path validation mechanisms across channels /统一各个 channel 的媒体文件路径验证机制

Offen
#1,220 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
enhancement
Vorherrschende Sprache
Python
Sterne
34.9k
Forks
3.1k
Ø Merge
1 T. 15 Std.
Gemergte PRs (30 T.)
225

Beschreibung

[Feature]: 统一各个 channel 的媒体文件路径验证机制

## Summary

Add a unified local media path validation mechanism for all channels that support media sending, so that file handling behavior is consistent across channels, security risks are reduced, and future maintenance is simpler.

希望为所有支持媒体发送的 channel 统一引入本地文件路径验证机制,避免各通道实现不一致带来的安全风险,并降低后续维护成本。

## Component(s) Affected

- [x] Core / Backend (app, agents, config, providers, utils, local_models)
- [ ] Console (frontend web UI)
- [x] Channels (DingTalk, Feishu, QQ, Discord, iMessage, etc.)
- [ ] Skills
- [ ] CLI
- [ ] Documentation (website)
- [x] Tests
- [ ] CI/CD
- [ ] Scripts / Deploy

## Problem / Motivation

Channels currently validate `file://` URLs and local media paths inconsistently. iMessage already appears to enforce stricter directory boundaries and path traversal protection, while Telegram, Mattermost, Matrix, and some other channels still lack a shared validation model or only apply partial checks.

This creates two practical problems:

1. Channel behavior is inconsistent, which increases implementation and maintenance complexity.
2. If a user-controlled or indirectly influenced path reaches `send_media`, the system may read and upload files outside the configured `media_dir`.

目前各个 channel 在处理 `file://` URL 或本地媒体路径时,校验逻辑不一致。iMessage 已实现较严格的目录限制和路径穿越防护,但 Telegram、Mattermost、Matrix 等 channel 仍存在未统一校验或校验不完整的问题。

这会带来两个问题:

1. 不同 channel 的行为不一致,增加使用和维护成本。
2. 如果用户输入或间接可控路径进入 `send_media` 流程,可能读取并上传 `media_dir` 之外的本地文件。

Examples of affected implementations include:

- Telegram: directly opens local files with `open(local_path)`
- Mattermost: processes local paths without a shared directory boundary rule
- Matrix: supports `file://` paths but does not apply a unified cross-directory restriction
- Discord / DingTalk: should be reviewed and aligned with the same validation model

受影响实现大致包括:

- Telegram:直接 `open(local_path)` 读取本地文件
- Mattermost:处理本地路径时无统一目录限制
- Matrix:支持 `file://`,但未统一阻止跨目录访问
- Discord / DingTalk:需要统一纳入同一套校验模型

## Proposed Solution

Introduce a shared media path validation utility and reuse it in every channel's `send_media` flow, or any equivalent local file sending path.

Recommended capabilities:

1. Normalize and `resolve()` the target path
2. Ensure the final path stays within the configured `media_dir`
3. Reject absolute paths, `../` traversal, and out-of-bound `file://` paths
4. Ensure the target exists and is a regular file
5. Add shared file size and extension whitelist checks
6. Log rejected access attempts without exposing sensitive path details

建议新增一套共享的媒体文件路径验证能力,并在所有 channel 的 `send_media` 或等价流程中统一复用。

建议能力包括:

1. 规范化并 `resolve()` 目标路径
2. 校验目标文件必须位于配置的 `media_dir` 内
3. 拒绝绝对路径、`../` 路径穿越和越界的 `file://` 路径
4. 校验文件存在且为普通文件
5. 统一补充文件大小和扩展名白名单检查
6. 统一记录拒绝日志,但避免暴露敏感路径细节

It could be abstracted as a shared component like:

```python
class MediaSecurityValidator:
@staticmethod
def validate_and_resolve_path(requested_path: str, media_dir: Path) -> Optional[Path]:
...
```

Then all channels would call the same validator instead of each channel maintaining different local file handling rules.

建议抽象为类似下面的公共组件:

```python
class MediaSecurityValidator:
@staticmethod
def validate_and_resolve_path(requested_path: str, media_dir: Path) -> Optional[Path]:
...
```

然后由各 channel 统一调用,而不是在每个 channel 中分别实现不一致的本地文件处理逻辑。

Expected acceptance outcomes:

- Telegram, Mattermost, and Matrix use the same shared validation logic for local media sending
- Discord and DingTalk are reviewed and integrated with the same validator where applicable
- `../`, absolute paths, and out-of-bound `file://` paths are rejected
- Tests cover both valid in-directory files and invalid traversal cases

建议验收结果:

- Telegram、Mattermost、Matrix 的本地媒体发送统一走共享校验逻辑
- Discord、DingTalk 完成审查,并在适用场景下接入同一验证器
- `../`、绝对路径、越界 `file://` 路径被拒绝
- 补充测试覆盖合法路径和非法路径场景

## Alternatives Considered

1. **Only patch the highest-risk channels**
This may land faster, but it preserves implementation divergence and makes future inconsistencies more likely.

2. **Let each channel keep its own validation logic**
This offers flexibility, but duplicates logic and makes it harder to guarantee a common safety baseline.

3. **Allow external files directly and rely on configuration only**
This may be useful as a later enhancement, but it should not replace a secure default baseline.
---
1. **仅修补高风险 channel**
可以更快落地,但会继续保留实现分叉,后续容易再次出现不一致问题。

2. **延续各 channel 自行校验**
灵活性更高,但重复代码较多,也更难确保所有 channel 都遵循相同安全标准。

3. **直接允许外部文件并通过配置控制**
适合作为后续增强,但不应替代默认的统一安全基线。

## Additional Context

Current code inspection suggests:

- iMessage can serve as the reference implementation for directory restriction and traversal blocking
- Telegram, Mattermost, and Matrix should be prioritized first
- Additional policy controls can be tracked separately later, such as:
- `allow_external_files_in_dm`
- `allow_external_files_in_group`
- `max_file_size_mb`
- `allowed_extensions`

Suggested test cases:

```python
rejected_cases = [
"../../../etc/passwd",
"../../config.json",
"/etc/passwd",
"file:///etc/shadow",
"~/.ssh/id_rsa",
"./../../sensitive.txt",
]

allowed_cases = [
"document.pdf",
"./images/photo.jpg",
"subdir/report.txt",
]
```

当前代码分析显示:

- iMessage 可作为目录限制和路径穿越防护的参考实现
- Telegram / Mattermost / Matrix 是优先级最高的统一对象
- 后续可再单独跟进配置增强,例如:
- `allow_external_files_in_dm`
- `allow_external_files_in_group`
- `max_file_size_mb`
- `allowed_extensions`

建议测试样例:

```python
rejected_cases = [
"../../../etc/passwd",
"../../config.json",
"/etc/passwd",
"file:///etc/shadow",
"~/.ssh/id_rsa",
"./../../sensitive.txt",
]

allowed_cases = [
"document.pdf",
"./images/photo.jpg",
"subdir/report.txt",
]
```

## Willing to Contribute

- [ ] I am willing to open a PR for this feature (after discussion).

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.