Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 41 additions & 3 deletions crates/tracevault-server/src/api/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,9 +424,21 @@ pub async fn get_blame(
.peel_to_commit()
.map_err(|e| AppError::BadRequest(format!("Ref is not a commit: {e}")))?;

// Prevent path traversal attacks by rejecting paths containing '..'.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate inline path traversal validation added in handler; extract into a shared helper to avoid repeated logic and reduce file bloat.

Details

✨ AI Reasoning
​The change adds repeated path traversal validation blocks into multiple request-handling functions. This introduces duplicated logic (same Path::new + components check + identical error handling) in several places, increasing code duplication and the file's size. Duplicated validation scattered across handlers makes future updates/error message changes error-prone and contributes to maintainability issues in an already large source file. A single shared helper would reduce duplication and keep the file more focused.

🔧 How do I fix it?
Split large files into smaller, focused modules. Each file should have a single responsibility.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

let path = std::path::Path::new(&query.path);
if path
.components()
.any(|c| c == std::path::Component::ParentDir)
{
return Err(AppError::BadRequest(format!(
"Invalid input: {}",
path.display()
)));
}

let blame = repo
.blame_file(
std::path::Path::new(&query.path),
path,
Some(git2::BlameOptions::new().newest_commit(commit.id())),
)
.map_err(|e| AppError::Internal(format!("Blame failed: {e}")))?;
Expand Down Expand Up @@ -598,8 +610,21 @@ pub async fn generate_story(
.peel_to_commit()
.map_err(|e| AppError::BadRequest(format!("Ref is not a commit: {e}")))?;
let tree = commit.tree()?;

// Prevent path traversal attacks by rejecting paths containing '..'.
let path = std::path::Path::new(&req.path);
if path
.components()
.any(|c| c == std::path::Component::ParentDir)
{
return Err(AppError::BadRequest(format!(
"Invalid input: {}",
path.display()
)));
}

let entry = tree
.get_path(std::path::Path::new(&req.path))
.get_path(path)
.map_err(|e| AppError::NotFound(format!("File not found: {e}")))?;
let blob_obj = entry.to_object(&repo)?;
let blob = blob_obj
Expand Down Expand Up @@ -841,8 +866,21 @@ pub async fn get_function_sessions(
.peel_to_commit()
.map_err(|e| AppError::BadRequest(format!("Ref is not a commit: {e}")))?;
let tree = commit.tree()?;

// Prevent path traversal attacks by rejecting paths containing '..'.
let path = std::path::Path::new(&query.path);
if path
.components()
.any(|c| c == std::path::Component::ParentDir)
{
return Err(AppError::BadRequest(format!(
"Invalid input: {}",
path.display()
)));
}

let entry = tree
.get_path(std::path::Path::new(&query.path))
.get_path(path)
.map_err(|e| AppError::NotFound(format!("File not found: {e}")))?;
let blob_obj = entry.to_object(&repo)?;
let blob = blob_obj
Expand Down
Loading