Skip to content

Commit 2c137cb

Browse files
committed
address comments and rebase
1 parent 23a6992 commit 2c137cb

File tree

6 files changed

+75
-50
lines changed

6 files changed

+75
-50
lines changed

crates/api/src/routes/vector_stores.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ use crate::{
1212
models::{
1313
CreateVectorStoreFileBatchRequest, CreateVectorStoreFileRequest, CreateVectorStoreRequest,
1414
ErrorResponse, ModifyVectorStoreRequest, UpdateVectorStoreFileAttributesRequest,
15-
VectorStoreDeleteResponse, VectorStoreFileDeleteResponse, VectorStoreFileListResponse,
16-
VectorStoreFileObject, VectorStoreFileBatchObject, VectorStoreListResponse,
15+
VectorStoreDeleteResponse, VectorStoreFileBatchObject, VectorStoreFileDeleteResponse,
16+
VectorStoreFileListResponse, VectorStoreFileObject, VectorStoreListResponse,
1717
VectorStoreObject, VectorStoreSearchRequest, VectorStoreSearchResponse,
1818
},
1919
routes::api::AppState,
@@ -594,13 +594,24 @@ pub async fn create_vector_store_file_batch(
594594
) -> Result<Json<Value>, (StatusCode, Json<ErrorResponse>)> {
595595
let vs_uuid = parse_uuid_with_prefix(&vector_store_id, PREFIX_VS)?;
596596

597-
// Try file_ids first, then files as fallback
597+
// Enforce mutual exclusivity: only one of file_ids or files is allowed
598+
let has_file_ids = body.get("file_ids").and_then(|v| v.as_array()).is_some();
599+
let has_files = body.get("files").and_then(|v| v.as_array()).is_some();
600+
601+
if has_file_ids && has_files {
602+
return Err((
603+
StatusCode::BAD_REQUEST,
604+
Json(ErrorResponse::new(
605+
"file_ids and files are mutually exclusive; provide only one".to_string(),
606+
"invalid_request_error".to_string(),
607+
)),
608+
));
609+
}
610+
598611
let file_id_strs: Vec<&Value> =
599612
if let Some(file_ids) = body.get("file_ids").and_then(|v| v.as_array()) {
600-
// Use file_ids array directly
601613
file_ids.iter().collect()
602614
} else if let Some(files) = body.get("files").and_then(|v| v.as_array()) {
603-
// Extract file_id from each file spec in files array
604615
files.iter().collect()
605616
} else {
606617
return Err((

crates/database/src/migrations/sql/V0044__add_vector_stores.sql renamed to crates/database/src/migrations/sql/V0045__add_vector_stores.sql

File renamed without changes.

crates/database/src/repositories/file.rs

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -477,34 +477,4 @@ impl FileRepositoryTrait for FileRepository {
477477
) -> Result<Vec<File>, RepositoryError> {
478478
self.get_by_ids_and_workspace(ids, workspace_id).await
479479
}
480-
481-
async fn verify_workspace_ownership(
482-
&self,
483-
file_ids: &[Uuid],
484-
workspace_id: Uuid,
485-
) -> Result<bool, RepositoryError> {
486-
if file_ids.is_empty() {
487-
return Ok(true);
488-
}
489-
490-
let count: i64 = retry_db!("verify_file_workspace_ownership", {
491-
let client = self
492-
.pool
493-
.get()
494-
.await
495-
.context("Failed to get database connection")
496-
.map_err(RepositoryError::PoolError)?;
497-
498-
client
499-
.query_one(
500-
"SELECT COUNT(*) AS cnt FROM files WHERE id = ANY($1) AND workspace_id = $2",
501-
&[&file_ids, &workspace_id],
502-
)
503-
.await
504-
.map_err(map_db_error)
505-
})?
506-
.get("cnt");
507-
508-
Ok(count == file_ids.len() as i64)
509-
}
510480
}

crates/services/src/files/ports.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,12 +81,4 @@ pub trait FileRepositoryTrait: Send + Sync {
8181
ids: &[Uuid],
8282
workspace_id: Uuid,
8383
) -> Result<Vec<File>, RepositoryError>;
84-
85-
/// Verify that ALL given file IDs belong to the specified workspace.
86-
/// Returns true if all files exist and belong to the workspace, false otherwise.
87-
async fn verify_workspace_ownership(
88-
&self,
89-
file_ids: &[Uuid],
90-
workspace_id: Uuid,
91-
) -> Result<bool, RepositoryError>;
9284
}

crates/services/src/id_prefixes.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ pub const PREFIX_MCPR: &str = "mcpr_";
2727
/// Prefix for vector store IDs
2828
pub const PREFIX_VS: &str = "vs_";
2929

30-
/// Prefix for vector store file IDs
31-
pub const PREFIX_VSF: &str = "vsf_";
32-
3330
/// Prefix for vector store file batch IDs
3431
pub const PREFIX_VSFB: &str = "vsfb_";
3532

@@ -43,6 +40,5 @@ pub const ALL_PREFIXES: &[&str] = &[
4340
PREFIX_SK,
4441
PREFIX_MCPR,
4542
PREFIX_VS,
46-
PREFIX_VSF,
4743
PREFIX_VSFB,
4844
];

crates/services/src/vector_stores/mod.rs

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,41 @@ pub struct RagFileMetadataEntry {
176176
pub filename: String,
177177
}
178178

179+
// ---------------------------------------------------------------------------
180+
// Typed filter structs for RAG passthrough (prevents mass assignment)
181+
// ---------------------------------------------------------------------------
182+
183+
/// Allowed fields for POST /v1/vector_stores/{id} (modify).
184+
#[derive(Debug, Serialize, Deserialize)]
185+
struct ModifyVectorStoreFilter {
186+
#[serde(skip_serializing_if = "Option::is_none")]
187+
name: Option<String>,
188+
#[serde(skip_serializing_if = "Option::is_none")]
189+
expires_after: Option<Value>,
190+
#[serde(skip_serializing_if = "Option::is_none")]
191+
metadata: Option<HashMap<String, Value>>,
192+
}
193+
194+
/// Allowed fields for POST /v1/vector_stores/{id}/search.
195+
#[derive(Debug, Serialize, Deserialize)]
196+
struct SearchVectorStoreFilter {
197+
query: Value,
198+
#[serde(skip_serializing_if = "Option::is_none")]
199+
max_num_results: Option<u32>,
200+
#[serde(skip_serializing_if = "Option::is_none")]
201+
filters: Option<Value>,
202+
#[serde(skip_serializing_if = "Option::is_none")]
203+
ranking_options: Option<Value>,
204+
#[serde(skip_serializing_if = "Option::is_none")]
205+
rewrite_query: Option<bool>,
206+
}
207+
208+
/// Allowed fields for POST /v1/vector_stores/{id}/files/{file_id} (update attributes).
209+
#[derive(Debug, Serialize, Deserialize)]
210+
struct UpdateFileAttributesFilter {
211+
attributes: HashMap<String, Value>,
212+
}
213+
179214
// ---------------------------------------------------------------------------
180215
// Service Error
181216
// ---------------------------------------------------------------------------
@@ -530,9 +565,15 @@ impl VectorStoreServiceTrait for VectorStoreServiceImpl {
530565
) -> Result<Value, VectorStoreServiceError> {
531566
self.verify_vs(vs_uuid, workspace_id).await?;
532567

568+
// Filter to allowed fields only (prevents mass assignment)
569+
let typed: ModifyVectorStoreFilter = serde_json::from_value(body)
570+
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
571+
let filtered_body = serde_json::to_value(&typed)
572+
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
573+
533574
let mut response = self
534575
.rag
535-
.update_vector_store(&vs_uuid.to_string(), body)
576+
.update_vector_store(&vs_uuid.to_string(), filtered_body)
536577
.await?;
537578
add_id_prefixes(&mut response);
538579
Ok(response)
@@ -563,9 +604,15 @@ impl VectorStoreServiceTrait for VectorStoreServiceImpl {
563604
) -> Result<Value, VectorStoreServiceError> {
564605
self.verify_vs(vs_uuid, workspace_id).await?;
565606

607+
// Filter to allowed fields only (prevents mass assignment)
608+
let typed: SearchVectorStoreFilter = serde_json::from_value(body)
609+
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
610+
let filtered_body = serde_json::to_value(&typed)
611+
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
612+
566613
let mut response = self
567614
.rag
568-
.search_vector_store(&vs_uuid.to_string(), body)
615+
.search_vector_store(&vs_uuid.to_string(), filtered_body)
569616
.await?;
570617
add_id_prefixes(&mut response);
571618
Ok(response)
@@ -642,9 +689,15 @@ impl VectorStoreServiceTrait for VectorStoreServiceImpl {
642689
self.verify_vs(vs_uuid, workspace_id).await?;
643690
self.verify_file(file_uuid, workspace_id).await?;
644691

692+
// Filter to allowed fields only (prevents mass assignment)
693+
let typed: UpdateFileAttributesFilter = serde_json::from_value(body)
694+
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
695+
let filtered_body = serde_json::to_value(&typed)
696+
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
697+
645698
let mut response = self
646699
.rag
647-
.update_vs_file(&vs_uuid.to_string(), &file_uuid.to_string(), body)
700+
.update_vs_file(&vs_uuid.to_string(), &file_uuid.to_string(), filtered_body)
648701
.await?;
649702
add_id_prefixes(&mut response);
650703
Ok(response)
@@ -684,6 +737,9 @@ impl VectorStoreServiceTrait for VectorStoreServiceImpl {
684737
.map_err(|e| VectorStoreServiceError::InvalidParams(e.to_string()))?;
685738

686739
if let Some(obj) = rag_body.as_object_mut() {
740+
// Remove unverified files field — only verified file_ids should reach RAG
741+
obj.remove("files");
742+
687743
// Replace file_ids with raw UUIDs
688744
let uuid_strings: Vec<Value> = file_uuids
689745
.iter()

0 commit comments

Comments
 (0)