[Rust Frontend][gRPC] Enforce LoRA path validation across transports (#53756)
Signed-off-by: Connor Carpenter <connorc@nvidia.com> Co-authored-by: Bugen Zhao <i@bugenzhao.com>
This commit is contained in:
co-authored by
Bugen Zhao
parent
e5377e6727
commit
3cc015acd2
@@ -13,7 +13,7 @@ use vllm_engine_core_client::protocol::lora::LoraRequest;
|
||||
use vllm_engine_core_client::protocol::utility::PauseMode as EnginePauseMode;
|
||||
|
||||
use super::{ControlServer, pb};
|
||||
use crate::lora::{LoadLoraError, LoraDisabledError, UnloadLoraError};
|
||||
use crate::lora::{LoadLoraError, LoraDisabledError, LoraPathAccessError, UnloadLoraError};
|
||||
use crate::state::AppState;
|
||||
|
||||
pub(crate) type ControlGrpcService = ControlServer<ControlServiceImpl>;
|
||||
@@ -148,6 +148,10 @@ fn load_lora_status(error: LoadLoraError) -> Status {
|
||||
let code = match &error {
|
||||
LoadLoraError::Disabled(_) => Code::FailedPrecondition,
|
||||
LoadLoraError::InvalidRequest(_) => Code::InvalidArgument,
|
||||
LoadLoraError::PathAccess(LoraPathAccessError::InvalidPath { .. }) => Code::InvalidArgument,
|
||||
LoadLoraError::PathAccess(LoraPathAccessError::InvalidConfiguration { .. }) => {
|
||||
Code::Internal
|
||||
}
|
||||
LoadLoraError::AlreadyLoaded { .. } | LoadLoraError::BaseModelName { .. } => {
|
||||
Code::AlreadyExists
|
||||
}
|
||||
|
||||
@@ -1632,6 +1632,18 @@ async fn control_lora_lifecycle_selects_adapter_for_generation() {
|
||||
.await;
|
||||
let mut control_client = ControlClient::new(channel.clone());
|
||||
let mut inference_client = InferenceClient::new(channel);
|
||||
|
||||
let denied_source_path =
|
||||
std::env::temp_dir().join(format!("vllm-grpc-lora-denied-{}", std::process::id()));
|
||||
let denied = control_client
|
||||
.load_lora(pb::LoadLoraRequest {
|
||||
lora_name: "denied-adapter".to_string(),
|
||||
source_path: denied_source_path.to_string_lossy().into_owned(),
|
||||
})
|
||||
.await
|
||||
.expect_err("local LoRA path should be validated before engine load");
|
||||
assert_eq!(denied.code(), tonic::Code::InvalidArgument);
|
||||
|
||||
let loaded = control_client
|
||||
.load_lora(pb::LoadLoraRequest {
|
||||
lora_name: "adapter".to_string(),
|
||||
|
||||
@@ -1,14 +1,86 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
// SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicU64, Ordering};
|
||||
|
||||
use indexmap::IndexMap;
|
||||
use thiserror::Error;
|
||||
use thiserror_ext::Macro;
|
||||
use tokio::sync::{Mutex, RwLock};
|
||||
use vllm_engine_core_client::EngineCoreClient;
|
||||
use vllm_engine_core_client::protocol::lora::{LoraRequest, LoraRequestError};
|
||||
|
||||
const RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV: &str = "VLLM_RUNTIME_LORA_ALLOWED_PATH_PREFIXES";
|
||||
|
||||
#[derive(Debug, Error, Macro)]
|
||||
pub(crate) enum LoraPathAccessError {
|
||||
#[error("{message}")]
|
||||
InvalidPath { message: String },
|
||||
#[error("{message}")]
|
||||
InvalidConfiguration { message: String },
|
||||
}
|
||||
|
||||
fn runtime_lora_allowed_path_prefixes() -> Option<Vec<PathBuf>> {
|
||||
let prefixes = std::env::var_os(RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV)?;
|
||||
let prefixes: Vec<_> = std::env::split_paths(&prefixes)
|
||||
.filter(|path| !path.as_os_str().is_empty())
|
||||
.collect();
|
||||
(!prefixes.is_empty()).then_some(prefixes)
|
||||
}
|
||||
|
||||
fn looks_like_local_lora_path(lora_path: &str) -> bool {
|
||||
let path = Path::new(lora_path);
|
||||
path.is_absolute()
|
||||
|| lora_path.starts_with('~')
|
||||
|| lora_path.starts_with('.')
|
||||
|| path.components().any(|component| matches!(component, Component::ParentDir))
|
||||
}
|
||||
|
||||
fn validate_lora_path_access(
|
||||
lora_path: &str,
|
||||
allowed_prefixes: Option<&[PathBuf]>,
|
||||
) -> Result<Option<String>, LoraPathAccessError> {
|
||||
let path = Path::new(lora_path);
|
||||
if !looks_like_local_lora_path(lora_path) && !path.exists() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let Some(allowed_prefixes) = allowed_prefixes else {
|
||||
return Err(invalid_path!(
|
||||
"Local LoRA adapter paths require {RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV} to be configured."
|
||||
));
|
||||
};
|
||||
|
||||
if !path.is_absolute() {
|
||||
return Err(invalid_path!(
|
||||
"Local LoRA adapter paths must be absolute and under one of the prefixes configured by {RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV}."
|
||||
));
|
||||
}
|
||||
|
||||
let canonical_path = path
|
||||
.canonicalize()
|
||||
.map_err(|_| invalid_path!("Local LoRA adapter path must exist and be accessible."))?;
|
||||
let canonical_prefixes = allowed_prefixes
|
||||
.iter()
|
||||
.map(|prefix| {
|
||||
prefix.canonicalize().map_err(|_| {
|
||||
invalid_configuration!(
|
||||
"configured {RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV} path prefix must exist and be accessible"
|
||||
)
|
||||
})
|
||||
})
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
|
||||
if !canonical_prefixes.iter().any(|prefix| canonical_path.starts_with(prefix)) {
|
||||
return Err(invalid_path!(
|
||||
"Local LoRA adapter path is outside the configured allowed prefixes."
|
||||
));
|
||||
}
|
||||
|
||||
Ok(Some(canonical_path.to_string_lossy().into_owned()))
|
||||
}
|
||||
|
||||
/// Snapshot of the currently served model names plus the requested LoRA, if
|
||||
/// the model name resolves to a dynamic adapter.
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -37,6 +109,8 @@ pub(crate) enum LoadLoraError {
|
||||
Disabled(#[from] LoraDisabledError),
|
||||
#[error(transparent)]
|
||||
InvalidRequest(#[from] LoraRequestError),
|
||||
#[error(transparent)]
|
||||
PathAccess(#[from] LoraPathAccessError),
|
||||
#[error("LoRA adapter `{lora_name}` is already loaded")]
|
||||
AlreadyLoaded { lora_name: String },
|
||||
#[error("LoRA adapter `{lora_name}` conflicts with a served base model")]
|
||||
@@ -117,6 +191,9 @@ impl LoraManager {
|
||||
load_inplace: bool,
|
||||
is_3d_lora_weight: bool,
|
||||
) -> Result<LoraRequest, LoadLoraError> {
|
||||
let allowed_prefixes = runtime_lora_allowed_path_prefixes();
|
||||
let lora_path = validate_lora_path_access(&lora_path, allowed_prefixes.as_deref())?
|
||||
.unwrap_or(lora_path);
|
||||
let _guard = self.update_lock.lock().await;
|
||||
if base_model_names.iter().any(|name| name == &lora_name) {
|
||||
return Err(LoadLoraError::BaseModelName { lora_name });
|
||||
@@ -195,3 +272,98 @@ impl LoraManager {
|
||||
Ok(self.requests.write().await.shift_remove(lora_name).unwrap_or(lora_request))
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
use super::validate_lora_path_access;
|
||||
|
||||
fn temp_lora_dir(test_name: &str) -> PathBuf {
|
||||
let suffix = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.expect("clock should be after unix epoch")
|
||||
.as_nanos();
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"vllm-lora-{test_name}-{}-{suffix}",
|
||||
std::process::id()
|
||||
));
|
||||
fs::create_dir_all(&path).expect("create temp lora dir");
|
||||
path
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_allows_hf_repo_ids_without_prefixes() {
|
||||
assert_eq!(
|
||||
validate_lora_path_access("org/adapter-a", None).expect("hf repo id should be allowed"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_rejects_local_paths_without_prefixes() {
|
||||
assert!(validate_lora_path_access("/tmp/adapter-a", None).is_err());
|
||||
assert!(validate_lora_path_access("./adapter-a", None).is_err());
|
||||
assert!(validate_lora_path_access("~/adapter-a", None).is_err());
|
||||
assert!(validate_lora_path_access("subdir/../../../etc/sensitive", None).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_rejects_existing_bare_relative_paths_without_prefixes() {
|
||||
let root =
|
||||
PathBuf::from("target").join(format!("vllm-lora-relative-{}", std::process::id()));
|
||||
let adapter = root.join("adapter-a");
|
||||
fs::create_dir_all(&adapter).expect("create relative adapter dir");
|
||||
|
||||
assert!(
|
||||
validate_lora_path_access(adapter.to_str().expect("utf-8 temp path"), None).is_err()
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_allows_absolute_paths_under_configured_prefixes() {
|
||||
let root = temp_lora_dir("allowed-prefix");
|
||||
let allowed = root.join("allowed");
|
||||
let adapter = allowed.join("adapter-a");
|
||||
fs::create_dir_all(&adapter).expect("create adapter dir");
|
||||
|
||||
let prefixes = [allowed];
|
||||
let resolved =
|
||||
validate_lora_path_access(adapter.to_str().expect("utf-8 temp path"), Some(&prefixes))
|
||||
.expect("path under configured prefix should be allowed");
|
||||
assert_eq!(
|
||||
resolved.as_deref(),
|
||||
Some(
|
||||
adapter
|
||||
.canonicalize()
|
||||
.expect("canonical adapter")
|
||||
.to_str()
|
||||
.expect("utf-8 temp path")
|
||||
)
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_rejects_parent_escape_from_configured_prefixes() {
|
||||
let root = temp_lora_dir("parent-escape");
|
||||
let allowed = root.join("allowed");
|
||||
let private_adapter = root.join("private").join("adapter-a");
|
||||
fs::create_dir_all(&allowed).expect("create allowed dir");
|
||||
fs::create_dir_all(&private_adapter).expect("create private adapter dir");
|
||||
|
||||
let escaped = allowed.join("../private/adapter-a");
|
||||
let prefixes = [allowed];
|
||||
assert!(
|
||||
validate_lora_path_access(escaped.to_str().expect("utf-8 temp path"), Some(&prefixes))
|
||||
.is_err()
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).ok();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
// SPDX-FileCopyrightText: Copyright contributors to the vLLM project
|
||||
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
use axum::extract::State;
|
||||
@@ -10,13 +9,11 @@ use thiserror_ext::AsReport;
|
||||
use validator::Validate;
|
||||
|
||||
use crate::error::ApiError;
|
||||
use crate::lora::{LoadLoraError, UnloadLoraError};
|
||||
use crate::lora::{LoadLoraError, LoraPathAccessError, UnloadLoraError};
|
||||
use crate::routes::openai::utils::types::Normalizable;
|
||||
use crate::routes::openai::utils::validated_json::ValidatedJson;
|
||||
use crate::state::AppState;
|
||||
|
||||
const RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV: &str = "VLLM_RUNTIME_LORA_ALLOWED_PATH_PREFIXES";
|
||||
|
||||
#[derive(Debug, Deserialize, Validate)]
|
||||
pub(crate) struct LoadLoraAdapterRequest {
|
||||
lora_name: String,
|
||||
@@ -38,81 +35,17 @@ pub(crate) struct UnloadLoraAdapterRequest {
|
||||
|
||||
impl Normalizable for UnloadLoraAdapterRequest {}
|
||||
|
||||
fn runtime_lora_allowed_path_prefixes() -> Option<Vec<PathBuf>> {
|
||||
let prefixes = std::env::var_os(RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV)?;
|
||||
let prefixes: Vec<_> = std::env::split_paths(&prefixes)
|
||||
.filter(|path| !path.as_os_str().is_empty())
|
||||
.collect();
|
||||
(!prefixes.is_empty()).then_some(prefixes)
|
||||
}
|
||||
|
||||
fn looks_like_local_lora_path(lora_path: &str) -> bool {
|
||||
let path = Path::new(lora_path);
|
||||
path.is_absolute()
|
||||
|| lora_path.starts_with('~')
|
||||
|| lora_path.starts_with('.')
|
||||
|| path.components().any(|component| matches!(component, Component::ParentDir))
|
||||
}
|
||||
|
||||
fn validate_lora_path_access(
|
||||
lora_path: &str,
|
||||
allowed_prefixes: Option<&[PathBuf]>,
|
||||
) -> Result<Option<String>, ApiError> {
|
||||
let path = Path::new(lora_path);
|
||||
if !looks_like_local_lora_path(lora_path) && !path.exists() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
let Some(allowed_prefixes) = allowed_prefixes else {
|
||||
return Err(ApiError::invalid_request(
|
||||
format!(
|
||||
"Local LoRA adapter paths require {RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV} to be configured."
|
||||
),
|
||||
Some("lora_path"),
|
||||
));
|
||||
};
|
||||
|
||||
if !path.is_absolute() {
|
||||
return Err(ApiError::invalid_request(
|
||||
format!(
|
||||
"Local LoRA adapter paths must be absolute and under one of the prefixes configured by {RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV}."
|
||||
),
|
||||
Some("lora_path"),
|
||||
));
|
||||
}
|
||||
|
||||
let canonical_path = path.canonicalize().map_err(|_| {
|
||||
ApiError::invalid_request(
|
||||
"Local LoRA adapter path must exist and be accessible.".to_string(),
|
||||
Some("lora_path"),
|
||||
)
|
||||
})?;
|
||||
let canonical_prefixes = allowed_prefixes
|
||||
.iter()
|
||||
.map(|prefix| {
|
||||
prefix.canonicalize().map_err(|_| {
|
||||
ApiError::server_error(format!(
|
||||
"configured {RUNTIME_LORA_ALLOWED_PATH_PREFIXES_ENV} path prefix must exist and be accessible"
|
||||
))
|
||||
})
|
||||
})
|
||||
.collect::<Result<Vec<_>, _>>()?;
|
||||
|
||||
if !canonical_prefixes.iter().any(|prefix| canonical_path.starts_with(prefix)) {
|
||||
return Err(ApiError::invalid_request(
|
||||
"Local LoRA adapter path is outside the configured allowed prefixes.".to_string(),
|
||||
Some("lora_path"),
|
||||
));
|
||||
}
|
||||
|
||||
Ok(Some(canonical_path.to_string_lossy().into_owned()))
|
||||
}
|
||||
|
||||
fn load_lora_api_error(error: LoadLoraError) -> ApiError {
|
||||
let message = error.to_report_string();
|
||||
match error {
|
||||
LoadLoraError::Disabled(_) => ApiError::invalid_request(message, None),
|
||||
LoadLoraError::InvalidRequest(_) => ApiError::invalid_request(message, None),
|
||||
LoadLoraError::PathAccess(LoraPathAccessError::InvalidPath { .. }) => {
|
||||
ApiError::invalid_request(message, Some("lora_path"))
|
||||
}
|
||||
LoadLoraError::PathAccess(LoraPathAccessError::InvalidConfiguration { .. }) => {
|
||||
ApiError::server_error(message)
|
||||
}
|
||||
LoadLoraError::AlreadyLoaded { .. } => ApiError::invalid_request(
|
||||
format!(
|
||||
"{message}. If you want to load the adapter in place, set 'load_inplace' to true."
|
||||
@@ -147,15 +80,11 @@ pub async fn load_lora_adapter(
|
||||
State(state): State<Arc<AppState>>,
|
||||
ValidatedJson(request): ValidatedJson<LoadLoraAdapterRequest>,
|
||||
) -> Result<String, ApiError> {
|
||||
let allowed_prefixes = runtime_lora_allowed_path_prefixes();
|
||||
let lora_path = validate_lora_path_access(&request.lora_path, allowed_prefixes.as_deref())?
|
||||
.unwrap_or(request.lora_path);
|
||||
|
||||
let lora_name = request.lora_name;
|
||||
state
|
||||
.load_lora(
|
||||
lora_name.clone(),
|
||||
lora_path,
|
||||
request.lora_path,
|
||||
request.load_inplace,
|
||||
request.is_3d_lora_weight,
|
||||
)
|
||||
@@ -189,98 +118,3 @@ pub async fn unload_lora_adapter(
|
||||
lora_request.lora_name
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
|
||||
use super::validate_lora_path_access;
|
||||
|
||||
fn temp_lora_dir(test_name: &str) -> PathBuf {
|
||||
let suffix = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.expect("clock should be after unix epoch")
|
||||
.as_nanos();
|
||||
let path = std::env::temp_dir().join(format!(
|
||||
"vllm-lora-{test_name}-{}-{suffix}",
|
||||
std::process::id()
|
||||
));
|
||||
fs::create_dir_all(&path).expect("create temp lora dir");
|
||||
path
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_allows_hf_repo_ids_without_prefixes() {
|
||||
assert_eq!(
|
||||
validate_lora_path_access("org/adapter-a", None).expect("hf repo id should be allowed"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_rejects_local_paths_without_prefixes() {
|
||||
assert!(validate_lora_path_access("/tmp/adapter-a", None).is_err());
|
||||
assert!(validate_lora_path_access("./adapter-a", None).is_err());
|
||||
assert!(validate_lora_path_access("~/adapter-a", None).is_err());
|
||||
assert!(validate_lora_path_access("subdir/../../../etc/sensitive", None).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_rejects_existing_bare_relative_paths_without_prefixes() {
|
||||
let root =
|
||||
PathBuf::from("target").join(format!("vllm-lora-relative-{}", std::process::id()));
|
||||
let adapter = root.join("adapter-a");
|
||||
fs::create_dir_all(&adapter).expect("create relative adapter dir");
|
||||
|
||||
assert!(
|
||||
validate_lora_path_access(adapter.to_str().expect("utf-8 temp path"), None).is_err()
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_allows_absolute_paths_under_configured_prefixes() {
|
||||
let root = temp_lora_dir("allowed-prefix");
|
||||
let allowed = root.join("allowed");
|
||||
let adapter = allowed.join("adapter-a");
|
||||
fs::create_dir_all(&adapter).expect("create adapter dir");
|
||||
|
||||
let prefixes = [allowed];
|
||||
let resolved =
|
||||
validate_lora_path_access(adapter.to_str().expect("utf-8 temp path"), Some(&prefixes))
|
||||
.expect("path under configured prefix should be allowed");
|
||||
assert_eq!(
|
||||
resolved.as_deref(),
|
||||
Some(
|
||||
adapter
|
||||
.canonicalize()
|
||||
.expect("canonical adapter")
|
||||
.to_str()
|
||||
.expect("utf-8 temp path")
|
||||
)
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).ok();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lora_path_rejects_parent_escape_from_configured_prefixes() {
|
||||
let root = temp_lora_dir("parent-escape");
|
||||
let allowed = root.join("allowed");
|
||||
let private_adapter = root.join("private").join("adapter-a");
|
||||
fs::create_dir_all(&allowed).expect("create allowed dir");
|
||||
fs::create_dir_all(&private_adapter).expect("create private adapter dir");
|
||||
|
||||
let escaped = allowed.join("../private/adapter-a");
|
||||
let prefixes = [allowed];
|
||||
assert!(
|
||||
validate_lora_path_access(escaped.to_str().expect("utf-8 temp path"), Some(&prefixes))
|
||||
.is_err()
|
||||
);
|
||||
|
||||
fs::remove_dir_all(root).ok();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user