Skip to content

Commit fd56419

Browse files
committed
Validate app-server read file paths
1 parent 7954d02 commit fd56419

1 file changed

Lines changed: 58 additions & 5 deletions

File tree

src/cortex-app-server/src/api/files.rs

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use axum::{
1010
use crate::error::{AppError, AppResult};
1111
use crate::state::AppState;
1212

13-
use super::path_security::{validate_path_for_delete, validate_path_for_write};
13+
use super::path_security::{validate_path_for_delete, validate_path_for_write, validate_path_safe};
1414
use super::types::{
1515
CreateDirRequest, DeleteFileRequest, DeleteFileResponse, FileEntry, FileTreeNode,
1616
FileTreeQuery, ListFilesRequest, ListFilesResponse, ReadFileRequest, ReadFileResponse,
@@ -194,19 +194,20 @@ pub async fn read_file(Json(req): Json<ReadFileRequest>) -> AppResult<Json<ReadF
194194
use std::fs;
195195

196196
let path = std::path::Path::new(&req.path);
197+
let validated_path = validate_path_safe(path).map_err(AppError::BadRequest)?;
197198

198-
if !path.exists() {
199+
if !validated_path.exists() {
199200
return Err(AppError::NotFound(format!("File not found: {}", req.path)));
200201
}
201202

202-
if !path.is_file() {
203+
if !validated_path.is_file() {
203204
return Err(AppError::BadRequest("Path is not a file".to_string()));
204205
}
205206

206-
let content = fs::read_to_string(path)
207+
let content = fs::read_to_string(&validated_path)
207208
.map_err(|e| AppError::Internal(format!("Failed to read file: {e}")))?;
208209

209-
let size = path.metadata().map(|m| m.len()).unwrap_or(0);
210+
let size = validated_path.metadata().map(|m| m.len()).unwrap_or(0);
210211

211212
Ok(Json(ReadFileResponse {
212213
path: req.path,
@@ -344,3 +345,55 @@ pub async fn watch_files(
344345

345346
axum::response::Sse::new(stream).keep_alive(KeepAlive::default())
346347
}
348+
349+
#[cfg(test)]
350+
mod tests {
351+
use std::path::PathBuf;
352+
353+
use axum::Json;
354+
355+
use super::*;
356+
357+
#[tokio::test]
358+
async fn test_read_file_reads_valid_allowed_path() {
359+
let dir = unique_temp_dir();
360+
std::fs::create_dir_all(&dir).unwrap();
361+
let file_path = dir.join("allowed.txt");
362+
std::fs::write(&file_path, "allowed content").unwrap();
363+
364+
let response = read_file(Json(ReadFileRequest {
365+
path: file_path.to_string_lossy().to_string(),
366+
}))
367+
.await
368+
.unwrap()
369+
.0;
370+
371+
assert_eq!(response.content, "allowed content");
372+
assert_eq!(response.size, "allowed content".len() as u64);
373+
374+
std::fs::remove_dir_all(dir).unwrap();
375+
}
376+
377+
#[cfg(unix)]
378+
#[tokio::test]
379+
async fn test_read_file_rejects_system_path_outside_allowed_roots() {
380+
let result = read_file(Json(ReadFileRequest {
381+
path: "/etc/passwd".to_string(),
382+
}))
383+
.await;
384+
let err = result.err().expect("system path should be rejected");
385+
386+
assert!(matches!(err, AppError::BadRequest(_)));
387+
assert!(
388+
err.to_string().contains("outside allowed directories"),
389+
"unexpected error: {err}"
390+
);
391+
}
392+
393+
fn unique_temp_dir() -> PathBuf {
394+
std::env::temp_dir().join(format!(
395+
"cortex-app-server-read-file-test-{}",
396+
uuid::Uuid::new_v4()
397+
))
398+
}
399+
}

0 commit comments

Comments
 (0)