Skip to content

Commit 354051d

Browse files
committed
guest: address review feedback on compose version policy
- Allow requirements on any manifest_version >= 3 instead of == 3, so a future v4 guest does not reject v4 manifests carrying requirements - Reject non-canonical manifest_version strings like "03" and "+3" - Rewrite unquote_os_release_value with single-pass escape handling (fixes backslash-before-quote mishandling; single quotes take no escapes, matching shell semantics) - Add tests for the above
1 parent fdf3309 commit 354051d

2 files changed

Lines changed: 60 additions & 12 deletions

File tree

dstack-types/src/lib.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,11 @@ fn parse_manifest_version_string(value: &str) -> Result<String, String> {
197197
if parsed == 0 {
198198
return Err("manifest_version must be greater than 0".to_string());
199199
}
200+
if parsed.to_string() != value {
201+
return Err(format!(
202+
"manifest_version must be a canonical integer string, got {value:?}"
203+
));
204+
}
200205
Ok(parsed.to_string())
201206
}
202207

@@ -346,6 +351,28 @@ mod app_compose_tests {
346351
assert!(err.to_string().contains("legacy versions 1 and 2"));
347352
}
348353

354+
#[test]
355+
fn manifest_version_rejects_invalid_numeric_values() {
356+
let err = parse_compose(serde_json::json!(0)).unwrap_err();
357+
assert!(err.to_string().contains("legacy versions 1 and 2"));
358+
let err = parse_compose(serde_json::json!(-1)).unwrap_err();
359+
assert!(err.to_string().contains("positive integer"));
360+
assert!(parse_compose(serde_json::json!(2.5)).is_err());
361+
}
362+
363+
#[test]
364+
fn manifest_version_rejects_non_canonical_strings() {
365+
let err = parse_compose(serde_json::json!("0")).unwrap_err();
366+
assert!(err.to_string().contains("greater than 0"));
367+
let err = parse_compose(serde_json::json!("03")).unwrap_err();
368+
assert!(err.to_string().contains("canonical integer string"));
369+
let err = parse_compose(serde_json::json!("+3")).unwrap_err();
370+
assert!(err.to_string().contains("canonical integer string"));
371+
let err = parse_compose(serde_json::json!("")).unwrap_err();
372+
assert!(err.to_string().contains("must not be empty"));
373+
assert!(parse_compose(serde_json::json!("3.0")).is_err());
374+
}
375+
349376
#[test]
350377
fn requirements_support_os_version_and_platforms() {
351378
let compose: AppCompose = serde_json::from_value(serde_json::json!({

dstack-util/src/system_setup.rs

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -797,9 +797,9 @@ fn verify_app_compose_policy(app_compose: &AppCompose) -> Result<()> {
797797

798798
fn verify_manifest_feature_requirements(app_compose: &AppCompose) -> Result<()> {
799799
let manifest_version = verify_manifest_version(app_compose)?;
800-
if app_compose.requirements.is_some() && manifest_version != MANIFEST_VERSION_3 {
800+
if app_compose.requirements.is_some() && manifest_version < MANIFEST_VERSION_3 {
801801
bail!(
802-
"requirements requires manifest_version == {MANIFEST_VERSION_3}; use string manifest_version \"{MANIFEST_VERSION_3}\" so older guests fail closed"
802+
"requirements requires manifest_version >= {MANIFEST_VERSION_3}; use string manifest_version \"{MANIFEST_VERSION_3}\" so older guests fail closed"
803803
);
804804
}
805805
Ok(())
@@ -900,16 +900,21 @@ fn os_release_value(content: &str, key: &str) -> Option<String> {
900900

901901
fn unquote_os_release_value(value: &str) -> String {
902902
let value = value.trim();
903-
let bytes = value.as_bytes();
904-
if bytes.len() >= 2
905-
&& ((bytes[0] == b'"' && bytes[bytes.len() - 1] == b'"')
906-
|| (bytes[0] == b'\'' && bytes[bytes.len() - 1] == b'\''))
907-
{
908-
let inner = &value[1..value.len() - 1];
909-
return inner
910-
.replace("\\\"", "\"")
911-
.replace("\\'", "'")
912-
.replace("\\\\", "\\");
903+
if let Some(inner) = value.strip_prefix('"').and_then(|v| v.strip_suffix('"')) {
904+
// Double-quoted: a backslash escapes the next character.
905+
let mut unescaped = String::with_capacity(inner.len());
906+
let mut chars = inner.chars();
907+
while let Some(c) = chars.next() {
908+
match c {
909+
'\\' => unescaped.push(chars.next().unwrap_or('\\')),
910+
_ => unescaped.push(c),
911+
}
912+
}
913+
return unescaped;
914+
}
915+
if let Some(inner) = value.strip_prefix('\'').and_then(|v| v.strip_suffix('\'')) {
916+
// Single-quoted: shell single quotes have no escape sequences.
917+
return inner.to_string();
913918
}
914919
value.to_string()
915920
}
@@ -2204,3 +2209,19 @@ VERSION_ID="0.6.1"
22042209
Some("0.6.1")
22052210
);
22062211
}
2212+
2213+
#[test]
2214+
fn test_unquote_os_release_value_handles_quoting_styles() {
2215+
assert_eq!(unquote_os_release_value("0.6.1"), "0.6.1");
2216+
assert_eq!(unquote_os_release_value("\"0.6.1\""), "0.6.1");
2217+
assert_eq!(unquote_os_release_value("'0.6.1'"), "0.6.1");
2218+
// Double-quoted: backslash escapes the next character.
2219+
assert_eq!(unquote_os_release_value(r#""a\"b""#), "a\"b");
2220+
assert_eq!(unquote_os_release_value(r#""a\\b""#), r"a\b");
2221+
assert_eq!(unquote_os_release_value(r#""a\\\"b""#), r#"a\"b"#);
2222+
// Single-quoted: no escape sequences.
2223+
assert_eq!(unquote_os_release_value(r"'a\\b'"), r"a\\b");
2224+
// Unbalanced/degenerate quotes are returned verbatim.
2225+
assert_eq!(unquote_os_release_value("\""), "\"");
2226+
assert_eq!(unquote_os_release_value("\"a"), "\"a");
2227+
}

0 commit comments

Comments
 (0)