Skip to content

Commit db24111

Browse files
author
Kiro
committed
security(gateway): add path validation and security comments to media store
- Add validate_path() — Core must call before reading any Attachment.path - Canonicalize resolves symlinks and '..' to prevent path traversal - Sanitize ext in store_media() to strip path separators - Document security considerations learned from OpenClaw CVE-2026-25475 - Explain why our design is safe: UUID filenames, Gateway-only path generation, agent never controls paths, TTL bounds exposure window
1 parent 3306615 commit db24111

1 file changed

Lines changed: 49 additions & 2 deletions

File tree

gateway/src/media_store.rs

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,28 @@
1-
use std::path::{Path, PathBuf};
2-
use std::time::{Duration, Instant};
1+
//! Media Store — co-locate filesystem mode
2+
//!
3+
//! Gateway downloads media from platform APIs (LINE, Telegram, Feishu) and writes
4+
//! to `~/.openab/media/inbound/<uuid>.<ext>`. Core reads from the same path.
5+
//!
6+
//! # Security Considerations (learned from OpenClaw CVE-2026-25475)
7+
//!
8+
//! 1. **Path is Gateway-generated only** — filenames are UUID v4, never derived
9+
//! from user input or agent output. This prevents path traversal attacks.
10+
//!
11+
//! 2. **Core must validate paths before reading** — use `validate_path()` to
12+
//! ensure the path is within the allowed media directory. This guards against
13+
//! a compromised Gateway or malformed event injecting `../../etc/passwd`.
14+
//!
15+
//! 3. **Agent cannot control paths** — the `Attachment.path` field flows from
16+
//! Gateway → Core only. Agent subprocess never sees or influences it.
17+
//!
18+
//! 4. **TTL eviction** — files are deleted after 2 minutes. Even if a path leaks,
19+
//! the window of exposure is bounded.
20+
//!
21+
//! 5. **No symlink following** — `validate_path()` uses `canonicalize()` to resolve
22+
//! symlinks before checking the prefix, preventing symlink-based escapes.
23+
24+
use std::path::PathBuf;
25+
use std::time::Duration;
326
use tokio::fs;
427
use tracing::{info, warn};
528

@@ -16,12 +39,17 @@ pub fn media_dir() -> PathBuf {
1639
}
1740

1841
/// Write media bytes to disk, return the absolute path.
42+
///
43+
/// Filenames are always `<uuid>.<ext>` — never user-supplied names.
44+
/// This eliminates path traversal via crafted filenames.
1945
pub async fn store_media(data: &[u8], ext: &str) -> Option<String> {
2046
let dir = media_dir();
2147
if let Err(e) = fs::create_dir_all(&dir).await {
2248
warn!(err = %e, "failed to create media dir");
2349
return None;
2450
}
51+
// Sanitize ext: strip any path separators or dots that could escape
52+
let ext = ext.trim_start_matches('.').replace(['/', '\\', '.'], "");
2553
let filename = format!("{}.{}", uuid::Uuid::new_v4(), ext);
2654
let path = dir.join(&filename);
2755
if let Err(e) = fs::write(&path, data).await {
@@ -32,6 +60,25 @@ pub async fn store_media(data: &[u8], ext: &str) -> Option<String> {
3260
Some(path.to_string_lossy().into_owned())
3361
}
3462

63+
/// Validate that a path is within the allowed media directory.
64+
///
65+
/// Core MUST call this before reading any file from `Attachment.path`.
66+
/// Guards against path traversal (e.g. `../../etc/passwd`) and symlink escapes.
67+
///
68+
/// Returns the canonicalized path if valid, None otherwise.
69+
pub fn validate_path(path: &str) -> Option<PathBuf> {
70+
let path = PathBuf::from(path);
71+
// Canonicalize resolves symlinks and `..` components
72+
let canonical = path.canonicalize().ok()?;
73+
let allowed_dir = media_dir().canonicalize().ok()?;
74+
if canonical.starts_with(&allowed_dir) {
75+
Some(canonical)
76+
} else {
77+
warn!(path = %canonical.display(), "media path outside allowed directory — rejecting");
78+
None
79+
}
80+
}
81+
3582
/// Spawn a background task that removes files older than TTL.
3683
pub fn spawn_cleanup_task() {
3784
tokio::spawn(async {

0 commit comments

Comments
 (0)