Skip to content

Commit bf7bdcf

Browse files
eauesqueclaude
andcommitted
feat(wd): accept a variant subdirectory for the model cache
Profiles may declare `hf_subdir` for repos that ship several builds under one name. The sidecar resolved the model directory from the model id alone, so yu-server had to forward those profiles to Python — the last reason a supported ONNX profile could not run here. `model_subdir` is optional and validated before it is joined: up to four segments of `[A-Za-z0-9._-]`, `.` and `..` refused, empty treated as absent. That mirrors `validate_hf_subdir` on the Python side. The value arrives over the wire and becomes a filesystem path, so it is refused rather than sanitised. `is_profile_model_downloaded` becomes `is_profile_model_ready` and takes the resolved directory, and the engine cache key follows the resolved directory too — two variants of one repo are two engines. Verified: 56 core tests (2 new, covering both the accepted and the refused subdirectory shapes), workspace 65 tests serially, clippy clean, and the camie cross-implementation check still matches Python exactly (16 / 23 / 20 tags, worst confidence delta 0.00005). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 8e52d2e commit bf7bdcf

2 files changed

Lines changed: 99 additions & 20 deletions

File tree

‎yu-hailo-infer-core/src/lib.rs‎

Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,51 @@ pub fn is_model_downloaded(cache_dir: &std::path::Path, model_id: &str) -> bool
1818
base.join("model.onnx").exists() && base.join("selected_tags.csv").exists()
1919
}
2020

21-
/// Whether the files a profile actually names are present.
21+
/// Whether the files a profile actually names are present in `model_dir`.
2222
///
2323
/// The WD-shaped check above would reject camie, whose weights are
2424
/// `camie-tagger-v2.onnx` and whose vocabulary is a JSON — and would accept a
2525
/// directory holding WD's files while the request asks for something else.
26-
pub fn is_profile_model_downloaded(
27-
cache_dir: &std::path::Path,
28-
model_id: &str,
29-
spec: &WdProfileSpec,
30-
) -> bool {
31-
let base = cache_dir.join(model_id);
26+
pub fn is_profile_model_ready(model_dir: &std::path::Path, spec: &WdProfileSpec) -> bool {
3227
let tag_file = match &spec.tag_source {
3328
profile::TagSourceSpec::Csv { file, .. }
3429
| profile::TagSourceSpec::JsonDict { file, .. } => file,
3530
};
36-
base.join(&spec.model_file).exists() && base.join(tag_file).exists()
31+
model_dir.join(&spec.model_file).exists() && model_dir.join(tag_file).exists()
32+
}
33+
34+
/// Resolve a model directory, honouring an optional variant subdirectory.
35+
///
36+
/// Mirrors `validate_hf_subdir` on the Python side: up to four segments of
37+
/// `[A-Za-z0-9._-]`, with `.` and `..` rejected. Returns `None` for anything
38+
/// else rather than joining it, because the result is a filesystem path built
39+
/// from a value that arrived over the wire.
40+
pub fn resolve_model_dir(
41+
cache_dir: &std::path::Path,
42+
model_id: &str,
43+
subdir: Option<&str>,
44+
) -> Option<std::path::PathBuf> {
45+
let mut dir = cache_dir.join(model_id);
46+
let Some(subdir) = subdir.filter(|s| !s.is_empty()) else {
47+
return Some(dir);
48+
};
49+
let segments: Vec<&str> = subdir.split('/').collect();
50+
if segments.len() > 4 {
51+
return None;
52+
}
53+
for segment in segments {
54+
if segment.is_empty()
55+
|| segment == "."
56+
|| segment == ".."
57+
|| !segment
58+
.chars()
59+
.all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '_' || c == '-')
60+
{
61+
return None;
62+
}
63+
dir = dir.join(segment);
64+
}
65+
Some(dir)
3766
}
3867

3968
#[cfg(test)]
@@ -83,26 +112,61 @@ mod tests {
83112
fs::create_dir(&model_dir).unwrap();
84113
let spec = camie_spec();
85114

86-
assert!(!is_profile_model_downloaded(dir.path(), "camie", &spec));
115+
assert!(!is_profile_model_ready(&model_dir, &spec));
87116
fs::write(model_dir.join("camie-tagger-v2.onnx"), b"dummy").unwrap();
88-
assert!(!is_profile_model_downloaded(dir.path(), "camie", &spec));
117+
assert!(!is_profile_model_ready(&model_dir, &spec));
89118
fs::write(model_dir.join("camie-tagger-v2-metadata.json"), b"{}").unwrap();
90-
assert!(is_profile_model_downloaded(dir.path(), "camie", &spec));
119+
assert!(is_profile_model_ready(&model_dir, &spec));
91120
}
92121

93122
#[test]
94123
fn the_wd_shaped_check_would_have_rejected_a_ready_camie_directory() {
95-
// Guards the reason is_profile_model_downloaded exists at all.
124+
// Guards the reason is_profile_model_ready exists at all.
96125
let dir = TempDir::new().unwrap();
97126
let model_dir = dir.path().join("camie");
98127
fs::create_dir(&model_dir).unwrap();
99128
fs::write(model_dir.join("camie-tagger-v2.onnx"), b"dummy").unwrap();
100129
fs::write(model_dir.join("camie-tagger-v2-metadata.json"), b"{}").unwrap();
101-
assert!(is_profile_model_downloaded(
102-
dir.path(),
103-
"camie",
104-
&camie_spec()
105-
));
130+
assert!(is_profile_model_ready(&model_dir, &camie_spec()));
106131
assert!(!is_model_downloaded(dir.path(), "camie"));
107132
}
133+
134+
#[test]
135+
fn a_variant_subdirectory_is_joined_when_it_is_a_plain_relative_path() {
136+
let root = std::path::Path::new("/cache");
137+
assert_eq!(
138+
resolve_model_dir(root, "repo", None),
139+
Some(root.join("repo"))
140+
);
141+
assert_eq!(
142+
resolve_model_dir(root, "repo", Some("")),
143+
Some(root.join("repo")),
144+
"an empty subdir means no subdir, as on the Python side"
145+
);
146+
assert_eq!(
147+
resolve_model_dir(root, "repo", Some("v1.1/fp16")),
148+
Some(root.join("repo").join("v1.1").join("fp16"))
149+
);
150+
}
151+
152+
#[test]
153+
fn a_subdirectory_that_escapes_the_cache_is_refused() {
154+
let root = std::path::Path::new("/cache");
155+
for bad in [
156+
"..",
157+
"../other",
158+
"v1/../../etc",
159+
"/abs",
160+
"a/b/c/d/e",
161+
"a b",
162+
"a\\b",
163+
".",
164+
] {
165+
assert_eq!(
166+
resolve_model_dir(root, "repo", Some(bad)),
167+
None,
168+
"subdir {bad:?} must be refused, not joined"
169+
);
170+
}
171+
}
108172
}

‎yu-hailo-infer/src/router.rs‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,11 @@ pub struct InferWdRequest {
193193
/// adapter family and belongs with the registry, not here.
194194
#[serde(default)]
195195
pub profile: Option<infer_core::WdProfileSpec>,
196+
/// Variant subdirectory under the model's cache directory, for repos that
197+
/// ship several builds (the Python side calls this `hf_subdir`). Validated
198+
/// as a plain relative path before being joined.
199+
#[serde(default)]
200+
pub model_subdir: Option<String>,
196201
}
197202

198203
#[derive(Debug, Deserialize)]
@@ -1672,8 +1677,18 @@ async fn infer_wd(
16721677
.into_response();
16731678
}
16741679

1675-
let model_dir = state.wd_cache_dir.join(&body.model_id);
1676-
if !infer_core::is_profile_model_downloaded(&state.wd_cache_dir, &body.model_id, &spec) {
1680+
let Some(model_dir) = infer_core::resolve_model_dir(
1681+
&state.wd_cache_dir,
1682+
&body.model_id,
1683+
body.model_subdir.as_deref(),
1684+
) else {
1685+
return (
1686+
StatusCode::BAD_REQUEST,
1687+
Json(json!({"error": "invalid model_subdir"})),
1688+
)
1689+
.into_response();
1690+
};
1691+
if !infer_core::is_profile_model_ready(&model_dir, &spec) {
16771692
return (
16781693
StatusCode::SERVICE_UNAVAILABLE,
16791694
Json(json!({"error": "model not downloaded"})),
@@ -1685,7 +1700,7 @@ async fn infer_wd(
16851700
// recipe, so the model id alone does not identify it: two requests for the
16861701
// same directory with different profiles must not share one engine.
16871702
let cache_key = match serde_json::to_string(&spec) {
1688-
Ok(fingerprint) => format!("{}\x00{}", body.model_id, fingerprint),
1703+
Ok(fingerprint) => format!("{}\x00{}", model_dir.display(), fingerprint),
16891704
Err(error) => {
16901705
tracing::error!("yu-infer wd profile fingerprint failed: {error}");
16911706
return (

0 commit comments

Comments
 (0)