Skip to content

fix(api): blob endpoint returns 400/404 instead of 500 on bad paths #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kevincodex1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(api): blob endpoint returns 400/404 instead of 500 on bad paths #37

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 25 additions & 2 deletions crates/gitlawb-node/src/api/repos.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,18 @@ pub async fn get_blob(
use axum::http::header;
use axum::response::IntoResponse;

// Unnormalized paths ("../..", "./", "//") can't resolve in `git show`
// and crawlers combinatorially explode them from relative links — that's
// a client error, not a 500.
let file_path = file_path.trim_matches('/');
if file_path.is_empty()
|| file_path
.split('/')
.any(|seg| seg.is_empty() || seg == "." || seg == "..")
{
return Err(AppError::BadRequest("invalid file path".into()));
}

let record = state
.db
.get_repo(&owner, &name)
Expand All @@ -237,8 +249,19 @@ pub async fn get_blob(
.await
.map_err(|e| AppError::Git(e.to_string()))?;
let head_ref = store::resolve_head(&disk_path, &record.default_branch);
let content = store::read_file(&disk_path, &head_ref, &file_path)
.map_err(|e| AppError::Git(e.to_string()))?;
let content = store::read_file(&disk_path, &head_ref, file_path).map_err(|e| {
let msg = e.to_string();
// `git show ref:path` on a path absent from the tree is a 404,
// not a server error
if msg.contains("does not exist in")
|| msg.contains("invalid object name")
|| msg.contains("exists on disk, but not in")
{
AppError::NotFound(format!("file not found: {file_path}"))
} else {
AppError::Git(msg)
}
})?;
Comment on lines +252 to +264

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "fn read_file|read_file\(" -S crates/gitlawb-node/src || true
rg -n "struct store|mod store|pub mod store|store::read_file" -S crates/gitlawb-node/src || true
ls -R crates/gitlawb-node/src/api | head

Repository: Gitlawb/node

Length of output: 607

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "fn read_file|read_file\(" -S crates/gitlawb-node/src || true
rg -n "store::read_file" -S crates/gitlawb-node/src || true
rg -n "mod store|pub mod store|crate::store|use .*store" -S crates/gitlawb-node/src || true
ls -R crates/gitlawb-node/src/api | head

Repository: Gitlawb/node

Length of output: 1012

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '180,280p' crates/gitlawb-node/src/git/store.rs
sed -n '1,120p' crates/gitlawb-node/src/api/repos.rs

rg -n "enum AppError|struct AppError|AppError::NotFound|AppError::Git" crates/gitlawb-node/src -S
rg -n "NotFound\\(|Git\\(" crates/gitlawb-node/src/api/repos.rs -S

Repository: Gitlawb/node

Length of output: 14533

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '200,310p' crates/gitlawb-node/src/api/repos.rs
sed -n '1,120p' crates/gitlawb-node/src/error.rs
sed -n '120,220p' crates/gitlawb-node/src/error.rs
sed -n '200,280p' crates/gitlawb-node/src/git/store.rs
rg -n "pub fn read_file|pub fn read_object|cat-file|show failed" crates/gitlawb-node/src/git/store.rs
sed -n '280,380p' crates/gitlawb-node/src/git/store.rs

Repository: Gitlawb/node

Length of output: 12434

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "fn resolve_head" crates/gitlawb-node/src/git/store.rs crates/gitlawb-node/src/git -S
sed -n '1,220p' crates/gitlawb-node/src/git/store.rs

Repository: Gitlawb/node

Length of output: 7288

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "store::read_file\\(" crates/gitlawb-node/src -S
rg -n "does not exist in|invalid object name|exists on disk, but not in" crates/gitlawb-node/src -S
rg -n "empty repo|no commits|HEAD.*invalid" crates/gitlawb-node/src -S || true

Repository: Gitlawb/node

Length of output: 738

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n "store::read_file\\(" crates/gitlawb-node/src -S
rg -n "does not exist in|invalid object name|exists on disk, but not in" crates/gitlawb-node/src -S
rg -n "empty repo|no commits|HEAD.*invalid|rev-parse --verify\" crates/gitlawb-node/src -S || true

Repository: Gitlawb/node

Length of output: 647

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '220,290p' crates/gitlawb-node/src/api/repos.rs
rg -n "init_bare\\(|create.*bare|bare repo|refs/heads/main" crates/gitlawb-node/src/git -S
rg -n "resolve_head\\(" crates/gitlawb-node/src -S
sed -n '1,220p' crates/gitlawb-node/src/git/repo_store.rs

Repository: Gitlawb/node

Length of output: 14695

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '220,290p' crates/gitlawb-node/src/api/repos.rs
rg -n "init_bare\\(|create.*bare|bare repo|refs/heads/main" crates/gitlawb-node/src/git -S
rg -n "resolve_head\\(" crates/gitlawb-node/src -S
sed -n '1,260p' crates/gitlawb-node/src/git/repo_store.rs

Repository: Gitlawb/node

Length of output: 16517

Avoid mapping HTTP 404 from substring-matching raw git show stderr in get_blob.
In crates/gitlawb-node/src/api/repos.rs (~252-264), store::read_file embeds raw git show stderr via bail!("git show failed: {stderr}"), and get_blob then substring-matches (including "invalid object name") to return AppError::NotFound. Since store::resolve_head can fall back to "HEAD" even when it doesn’t resolve (e.g., empty repo), "invalid object name" can reflect a missing/invalid ref—not just a missing path in the tree—leading to 404s where a git/internal error (or a different error type) is more accurate. Move this to a structured store-level result (e.g., preflight ref/object existence using cat-file/tree lookup) and only map true tree-path misses to AppError::NotFound.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-node/src/api/repos.rs` around lines 252 - 264, The current
substring-match on read_file stderr in get_blob wrongly treats "invalid object
name" as a path-missing 404; update the flow so store::read_file returns a
structured error (e.g., enum like ReadFileError::PathNotFound and
ReadFileError::Other) or perform a preflight existence check (use git
cat-file/tree lookup via the existing store helpers) to distinguish "path not
present in tree" from "invalid ref/object" (store::resolve_head fallback
failure). Then change the mapping in get_blob to convert only the explicit
PathNotFound variant to AppError::NotFound and propagate other errors as
AppError::Git (preserving stderr) so invalid refs/internal git errors are not
mapped to HTTP 404.


// Guess content type
let mime = match file_path.rsplit('.').next() {
Expand Down
Loading