Skip to content

Commit f4fd7d3

Browse files
authored
fix(buck): normalize session/start path and handle 404 (#1992)
* fix(buck): normalize session/start path and handle 404 - Add MonoServiceLogic::normalize_repo_path: trim, strip trailing slash, ensure leading slash; collapse middle repeated slashes and "." segments; reject "..", backslash, Windows drive letters, and leading ":" - create_buck_session: use normalized path for get_main_ref and create_session; return MegaError::NotFound when path not in mega_refs - Add unit test test_normalize_repo_path Signed-off-by: Wan Yidong <1360947433yd@gmail.com> * fix(buck): reject dot-only paths in normalize_repo_path Paths that consist only of '.' and slashes (e.g. ".", "./") are now returned as ValidationError instead of silently resolving to root "/", avoiding unintended root-scope session creation. Signed-off-by: Wan Yidong <1360947433yd@gmail.com> --------- Signed-off-by: Wan Yidong <1360947433yd@gmail.com>
1 parent 26e3aee commit f4fd7d3

File tree

1 file changed

+154
-4
lines changed

1 file changed

+154
-4
lines changed

ceres/src/api_service/mono_api_service.rs

Lines changed: 154 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,67 @@ impl MonoServiceLogic {
181181
}
182182
}
183183

184+
/// Normalize and validate repository path.
185+
///
186+
/// Rules: trim; reject empty or whitespace-only (validation error). Reject `..`, backslash,
187+
/// Windows drive letters (e.g. `C:`), and paths starting with `:`. Strip trailing `/`;
188+
/// input consisting only of slashes becomes `"/"`. Collapse middle repeated slashes and
189+
/// remove `.` segments (e.g. `//project//foo` → `/project/foo`, `project/./foo` → `/project/foo`).
190+
/// Paths that consist only of `.` and slashes (e.g. `"."`, `"./"`) are rejected so they do not
191+
/// silently resolve to root. Non-empty result gets a leading `"/"` if missing. Result matches
192+
/// mega_refs.path format.
193+
pub fn normalize_repo_path(path: &str) -> Result<String, MegaError> {
194+
let s = path.trim();
195+
if s.is_empty() {
196+
return Err(MegaError::Buck(BuckError::ValidationError(
197+
"Path cannot be empty".to_string(),
198+
)));
199+
}
200+
if s.contains("..") {
201+
return Err(MegaError::Buck(BuckError::ValidationError(format!(
202+
"Path traversal not allowed: {}",
203+
s
204+
))));
205+
}
206+
if s.contains('\\') {
207+
return Err(MegaError::Buck(BuckError::ValidationError(format!(
208+
"Path must use '/' separator: {}",
209+
s
210+
))));
211+
}
212+
if s.len() >= 2 {
213+
let mut chars = s.chars();
214+
if let (Some(c1), Some(':')) = (chars.next(), chars.next())
215+
&& c1.is_ascii_alphabetic()
216+
{
217+
return Err(MegaError::Buck(BuckError::ValidationError(format!(
218+
"Absolute path not allowed (Windows drive letter detected): {}",
219+
s
220+
))));
221+
}
222+
}
223+
if s.starts_with(':') {
224+
return Err(MegaError::Buck(BuckError::ValidationError(
225+
"Path must not start with ':'".to_string(),
226+
)));
227+
}
228+
let s = s.trim_end_matches('/');
229+
if s.is_empty() {
230+
return Ok("/".to_string());
231+
}
232+
let parts: Vec<&str> = s
233+
.split('/')
234+
.filter(|p| !p.is_empty() && *p != ".")
235+
.collect();
236+
let s = parts.join("/");
237+
if s.is_empty() {
238+
return Err(MegaError::Buck(BuckError::ValidationError(
239+
"Path cannot be empty or consist only of '.' segments".to_string(),
240+
)));
241+
}
242+
Ok(format!("/{}", s))
243+
}
244+
184245
pub fn update_tree_hash(
185246
tree: Arc<Tree>,
186247
name: &str,
@@ -2681,7 +2742,7 @@ impl MonoApiService {
26812742
///
26822743
/// # Arguments
26832744
/// * `username` - User creating the session
2684-
/// * `path` - Repository path
2745+
/// * `path` - Repository path (may be with or without leading `/`; normalized to match mega_refs format)
26852746
///
26862747
/// # Returns
26872748
/// Returns `SessionResponse` on success
@@ -2690,16 +2751,17 @@ impl MonoApiService {
26902751
username: &str,
26912752
path: &str,
26922753
) -> Result<jupiter::service::buck_service::SessionResponse, MegaError> {
2754+
let normalized_path = MonoServiceLogic::normalize_repo_path(path)?;
26932755
let refs = self
26942756
.storage
26952757
.mono_storage()
2696-
.get_main_ref(path)
2758+
.get_main_ref(&normalized_path)
26972759
.await?
2698-
.ok_or_else(|| MegaError::Other(format!("Path not found: {}", path)))?;
2760+
.ok_or_else(|| MegaError::NotFound(format!("Path not found: {}", normalized_path)))?;
26992761
let response = self
27002762
.storage
27012763
.buck_service
2702-
.create_session(username, path, refs.ref_commit_hash)
2764+
.create_session(username, &normalized_path, refs.ref_commit_hash)
27032765
.await?;
27042766

27052767
Ok(response)
@@ -3240,6 +3302,94 @@ mod test {
32403302
assert_eq!(MonoServiceLogic::clean_path_str("abc///"), "abc");
32413303
}
32423304

3305+
#[test]
3306+
fn test_normalize_repo_path() {
3307+
use common::errors::{BuckError, MegaError};
3308+
3309+
// Normalization: add leading slash, strip trailing
3310+
assert_eq!(
3311+
MonoServiceLogic::normalize_repo_path("project").unwrap(),
3312+
"/project"
3313+
);
3314+
assert_eq!(
3315+
MonoServiceLogic::normalize_repo_path("/project").unwrap(),
3316+
"/project"
3317+
);
3318+
assert_eq!(
3319+
MonoServiceLogic::normalize_repo_path("project/").unwrap(),
3320+
"/project"
3321+
);
3322+
assert_eq!(
3323+
MonoServiceLogic::normalize_repo_path("/project/").unwrap(),
3324+
"/project"
3325+
);
3326+
assert_eq!(
3327+
MonoServiceLogic::normalize_repo_path(" /project ").unwrap(),
3328+
"/project"
3329+
);
3330+
assert_eq!(MonoServiceLogic::normalize_repo_path("/").unwrap(), "/");
3331+
3332+
// Empty / whitespace-only -> ValidationError
3333+
assert!(MonoServiceLogic::normalize_repo_path("").is_err());
3334+
assert!(MonoServiceLogic::normalize_repo_path(" ").is_err());
3335+
assert!(matches!(
3336+
MonoServiceLogic::normalize_repo_path(""),
3337+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3338+
));
3339+
3340+
// Path traversal and invalid chars -> ValidationError
3341+
assert!(MonoServiceLogic::normalize_repo_path("project/../foo").is_err());
3342+
assert!(MonoServiceLogic::normalize_repo_path("project\\foo").is_err());
3343+
3344+
// Middle slashes and "." segments are collapsed
3345+
assert_eq!(
3346+
MonoServiceLogic::normalize_repo_path("//project//foo//").unwrap(),
3347+
"/project/foo"
3348+
);
3349+
assert_eq!(
3350+
MonoServiceLogic::normalize_repo_path("project/./foo").unwrap(),
3351+
"/project/foo"
3352+
);
3353+
assert_eq!(
3354+
MonoServiceLogic::normalize_repo_path("/project/./foo").unwrap(),
3355+
"/project/foo"
3356+
);
3357+
3358+
// Dot-only paths are rejected (do not silently resolve to root)
3359+
assert!(matches!(
3360+
MonoServiceLogic::normalize_repo_path("."),
3361+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3362+
));
3363+
assert!(matches!(
3364+
MonoServiceLogic::normalize_repo_path("./"),
3365+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3366+
));
3367+
assert!(matches!(
3368+
MonoServiceLogic::normalize_repo_path("./."),
3369+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3370+
));
3371+
3372+
// Leading colon is rejected
3373+
assert!(matches!(
3374+
MonoServiceLogic::normalize_repo_path(":/test"),
3375+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3376+
));
3377+
assert!(matches!(
3378+
MonoServiceLogic::normalize_repo_path(":"),
3379+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3380+
));
3381+
3382+
// Windows drive letters are rejected
3383+
assert!(matches!(
3384+
MonoServiceLogic::normalize_repo_path("C:"),
3385+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3386+
));
3387+
assert!(matches!(
3388+
MonoServiceLogic::normalize_repo_path("D:/project"),
3389+
Err(MegaError::Buck(BuckError::ValidationError(_)))
3390+
));
3391+
}
3392+
32433393
#[test]
32443394
fn test_update_tree_hash() {
32453395
let item = TreeItem::new(

0 commit comments

Comments
 (0)