From 9d6ac5c13af650c2d540f2e54f24919f3442d65e Mon Sep 17 00:00:00 2001 From: voson Date: Sun, 5 Apr 2026 11:48:40 +0800 Subject: [PATCH] =?UTF-8?q?release(secrets-mcp):=200.5.4=20=E2=80=94=20Web?= =?UTF-8?q?=20=E5=88=86=E9=A1=B5=E4=BF=AE=E6=AD=A3=E4=B8=8E=20hex=20?= =?UTF-8?q?=E8=A7=A3=E7=A0=81=EF=BC=9B=E6=89=B9=E9=87=8F=E5=88=A0=E9=99=A4?= =?UTF-8?q?=E4=B8=8A=E9=99=90=EF=BC=9BMCP=20@=20=E8=B7=AF=E5=BE=84?= =?UTF-8?q?=E6=A3=80=E6=B5=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.lock | 3 +- crates/secrets-core/Cargo.toml | 1 + crates/secrets-core/src/crypto.rs | 13 +---- crates/secrets-core/src/service/add.rs | 5 ++ crates/secrets-core/src/service/audit_log.rs | 1 + crates/secrets-core/src/service/delete.rs | 14 +++++ crates/secrets-core/src/service/update.rs | 4 +- crates/secrets-mcp/Cargo.toml | 2 +- crates/secrets-mcp/src/tools.rs | 18 ++++-- crates/secrets-mcp/src/web.rs | 58 ++++++++++++++++---- crates/secrets-mcp/templates/entries.html | 5 ++ 11 files changed, 92 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d688929..ff75b1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2049,6 +2049,7 @@ dependencies = [ "aes-gcm", "anyhow", "chrono", + "hex", "rand 0.10.0", "serde", "serde_json", @@ -2065,7 +2066,7 @@ dependencies = [ [[package]] name = "secrets-mcp" -version = "0.5.3" +version = "0.5.4" dependencies = [ "anyhow", "askama", diff --git a/crates/secrets-core/Cargo.toml b/crates/secrets-core/Cargo.toml index da0b3bf..ef6a09d 100644 --- a/crates/secrets-core/Cargo.toml +++ b/crates/secrets-core/Cargo.toml @@ -12,6 +12,7 @@ aes-gcm.workspace = true anyhow.workspace = true thiserror.workspace = true chrono.workspace = true +hex = "0.4" rand.workspace = true serde.workspace = true serde_json.workspace = true diff --git a/crates/secrets-core/src/crypto.rs b/crates/secrets-core/src/crypto.rs index 91cb72d..93abbe8 100644 --- a/crates/secrets-core/src/crypto.rs +++ b/crates/secrets-core/src/crypto.rs @@ -61,7 +61,7 @@ pub fn decrypt_json(master_key: &[u8; 32], data: &[u8]) -> Result { /// Parse a 64-char hex string (from X-Encryption-Key header) into a 32-byte key. pub fn extract_key_from_hex(hex_str: &str) -> Result<[u8; 32]> { - let bytes = hex::decode_hex(hex_str.trim())?; + let bytes = ::hex::decode(hex_str.trim())?; if bytes.len() != 32 { bail!( "X-Encryption-Key must be 64 hex chars (32 bytes), got {} bytes", @@ -76,21 +76,14 @@ pub fn extract_key_from_hex(hex_str: &str) -> Result<[u8; 32]> { // ─── Public hex helpers ─────────────────────────────────────────────────────── pub mod hex { - use anyhow::{Result, bail}; + use anyhow::Result; pub fn encode_hex(bytes: &[u8]) -> String { bytes.iter().map(|b| format!("{:02x}", b)).collect() } pub fn decode_hex(s: &str) -> Result> { - let s = s.trim(); - if !s.len().is_multiple_of(2) { - bail!("hex string has odd length"); - } - (0..s.len()) - .step_by(2) - .map(|i| u8::from_str_radix(&s[i..i + 2], 16).map_err(|e| anyhow::anyhow!("{}", e))) - .collect() + Ok(::hex::decode(s.trim())?) } } diff --git a/crates/secrets-core/src/service/add.rs b/crates/secrets-core/src/service/add.rs index 3835ab3..a8e50e4 100644 --- a/crates/secrets-core/src/service/add.rs +++ b/crates/secrets-core/src/service/add.rs @@ -243,6 +243,11 @@ pub async fn run(pool: &PgPool, params: AddParams<'_>, master_key: &[u8; 32]) -> tracing::warn!(error = %e, "failed to snapshot entry history before upsert"); } + // Upsert the entry row. On conflict (existing entry with same user_id+folder+name), + // the entry columns are replaced wholesale. The old secret associations are torn down + // below within the same transaction, so the whole operation is atomic: if any step + // after this point fails, the transaction rolls back and the entry reverts to its + // pre-upsert state (including the version bump that happened in the DO UPDATE clause). let entry_id: Uuid = if let Some(uid) = params.user_id { sqlx::query_scalar( r#"INSERT INTO entries (user_id, folder, type, name, notes, tags, metadata, version, updated_at) diff --git a/crates/secrets-core/src/service/audit_log.rs b/crates/secrets-core/src/service/audit_log.rs index 6d644ba..5c72b42 100644 --- a/crates/secrets-core/src/service/audit_log.rs +++ b/crates/secrets-core/src/service/audit_log.rs @@ -11,6 +11,7 @@ pub async fn list_for_user( offset: i64, ) -> Result> { let limit = limit.clamp(1, 200); + let offset = offset.max(0); let rows = sqlx::query_as( "SELECT id, user_id, action, folder, type, name, detail, created_at \ diff --git a/crates/secrets-core/src/service/delete.rs b/crates/secrets-core/src/service/delete.rs index 86ef975..d0db2a6 100644 --- a/crates/secrets-core/src/service/delete.rs +++ b/crates/secrets-core/src/service/delete.rs @@ -31,6 +31,10 @@ pub struct DeleteParams<'a> { pub user_id: Option, } +/// Maximum number of entries that can be deleted in a single bulk operation. +/// Prevents accidental mass deletion when filters are too broad. +pub const MAX_BULK_DELETE: usize = 1000; + /// Delete a single entry by id (multi-tenant: `user_id` must match). pub async fn delete_by_id(pool: &PgPool, entry_id: Uuid, user_id: Uuid) -> Result { let mut tx = pool.begin().await?; @@ -374,6 +378,16 @@ async fn delete_bulk( } let rows = q.fetch_all(&mut *tx).await?; + if rows.len() > MAX_BULK_DELETE { + tx.rollback().await?; + anyhow::bail!( + "Bulk delete would affect {} entries (limit: {}). \ + Narrow your filters or delete entries individually.", + rows.len(), + MAX_BULK_DELETE, + ); + } + let mut deleted = Vec::with_capacity(rows.len()); for row in &rows { let entry_row: EntryRow = EntryRow { diff --git a/crates/secrets-core/src/service/update.rs b/crates/secrets-core/src/service/update.rs index 9c34a70..d53a461 100644 --- a/crates/secrets-core/src/service/update.rs +++ b/crates/secrets-core/src/service/update.rs @@ -402,8 +402,8 @@ pub async fn run( &mut tx, params.user_id, "update", - "", - "", + &row.folder, + &row.entry_type, params.name, serde_json::json!({ "add_tags": params.add_tags, diff --git a/crates/secrets-mcp/Cargo.toml b/crates/secrets-mcp/Cargo.toml index bdbe817..af7bc61 100644 --- a/crates/secrets-mcp/Cargo.toml +++ b/crates/secrets-mcp/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "secrets-mcp" -version = "0.5.3" +version = "0.5.4" edition.workspace = true [[bin]] diff --git a/crates/secrets-mcp/src/tools.rs b/crates/secrets-mcp/src/tools.rs index a3710ed..848533a 100644 --- a/crates/secrets-mcp/src/tools.rs +++ b/crates/secrets-mcp/src/tools.rs @@ -611,6 +611,10 @@ fn map_to_kv_strings(map: Map) -> Vec { /// contain `@` characters (e.g. `config:=@/etc/passwd`), the `:=` branch in /// `parse_kv` treats the right-hand side as raw JSON and never performs file /// reads. The `@` in such cases is just data, not a file reference. +/// +/// For entries without `=` that contain `@`, we only reject them if the `@` +/// appears to be file-path syntax (i.e., the part after `@` starts with `/`, +/// `~`, or `.`). This avoids false positives on values like `user@example.com`. fn contains_file_reference(entries: &[String]) -> Option { for entry in entries { // key:=json — safe, skip before checking for `=` @@ -625,12 +629,14 @@ fn contains_file_reference(entries: &[String]) -> Option { continue; } // key@path (no `=` present) - // parse_kv treats entries without `=` that contain `@` as file-read - // syntax (key@path). This includes strings like "user@example.com" - // if passed without a `=` separator — which is correct to reject here - // since the MCP server runs remotely and cannot read local files. - if entry.contains('@') { - return Some(entry.clone()); + // Only reject if the `@` looks like file-path syntax: the segment after + // `@` starts with `/`, `~`, or `.`, which are common path prefixes. + // Values like "user@example.com" pass through safely. + if let Some((_, path_part)) = entry.split_once('@') { + let trimmed = path_part.trim_start(); + if trimmed.starts_with('/') || trimmed.starts_with('~') || trimmed.starts_with('.') { + return Some(entry.clone()); + } } } None diff --git a/crates/secrets-mcp/src/web.rs b/crates/secrets-mcp/src/web.rs index f9813b5..6a710ba 100644 --- a/crates/secrets-mcp/src/web.rs +++ b/crates/secrets-mcp/src/web.rs @@ -199,6 +199,15 @@ fn request_user_agent(headers: &HeaderMap) -> Option { .map(ToOwned::to_owned) } +fn paginate(page: u32, total_count: i64, page_size: u32) -> (u32, u32, u32) { + let page_size = page_size.max(1); + let safe_total_count = u32::try_from(total_count.max(0)).unwrap_or(u32::MAX); + let total_pages = safe_total_count.div_ceil(page_size).max(1); + let current_page = page.max(1).min(total_pages); + let offset = (current_page - 1).saturating_mul(page_size); + (current_page, total_pages, offset) +} + // ── Routes ──────────────────────────────────────────────────────────────────── pub fn web_router() -> Router { @@ -605,8 +614,7 @@ async fn entries_page( .filter(|s| !s.is_empty()) .map(|s| s.to_string()); let page = q.page.unwrap_or(1).max(1); - let offset = (page - 1) * ENTRIES_PAGE_LIMIT; - let params = SearchParams { + let count_params = SearchParams { folder: folder_filter.as_deref(), entry_type: type_filter.as_deref(), name: None, @@ -615,18 +623,22 @@ async fn entries_page( query: None, sort: "updated", limit: ENTRIES_PAGE_LIMIT, - offset, + offset: 0, user_id: Some(user_id), }; - let total_count = count_entries(&state.pool, ¶ms) + let total_count = count_entries(&state.pool, &count_params) .await .inspect_err(|e| tracing::warn!(error = %e, "count_entries failed for web entries page")) .unwrap_or(0); - let total_pages = (total_count as u32).div_ceil(ENTRIES_PAGE_LIMIT).max(1); - let current_page = page.min(total_pages); + let (current_page, total_pages, offset) = paginate(page, total_count, ENTRIES_PAGE_LIMIT); - let rows = list_entries(&state.pool, params).await.map_err(|e| { + let list_params = SearchParams { + offset, + ..count_params + }; + + let rows = list_entries(&state.pool, list_params).await.map_err(|e| { tracing::error!(error = %e, "failed to load entries list for web"); StatusCode::INTERNAL_SERVER_ERROR })?; @@ -846,11 +858,8 @@ async fn audit_page( StatusCode::INTERNAL_SERVER_ERROR })?; - let total_pages = (total_count as u32) - .div_ceil(AUDIT_PAGE_LIMIT as u32) - .max(1); - let current_page = page.min(total_pages); - let actual_offset = ((current_page - 1) as i64) * AUDIT_PAGE_LIMIT; + let (current_page, total_pages, offset) = paginate(page, total_count, AUDIT_PAGE_LIMIT as u32); + let actual_offset = i64::from(offset); let rows = list_for_user(&state.pool, user_id, AUDIT_PAGE_LIMIT, actual_offset) .await @@ -1751,4 +1760,29 @@ mod tests { assert!(matches!(request_ui_lang(&headers), UiLang::ZhTw)); } + + #[test] + fn paginate_clamps_page_before_computing_offset() { + let (current_page, total_pages, offset) = paginate(100, 12, 10); + + assert_eq!(current_page, 2); + assert_eq!(total_pages, 2); + assert_eq!(offset, 10); + } + + #[test] + fn paginate_handles_large_page_without_overflow() { + let (current_page, total_pages, offset) = paginate(u32::MAX, 1, ENTRIES_PAGE_LIMIT); + + assert_eq!(current_page, 1); + assert_eq!(total_pages, 1); + assert_eq!(offset, 0); + } + + #[test] + fn paginate_saturates_large_total_count() { + let (_, total_pages, _) = paginate(1, i64::MAX, ENTRIES_PAGE_LIMIT); + + assert_eq!(total_pages, u32::MAX.div_ceil(ENTRIES_PAGE_LIMIT)); + } } diff --git a/crates/secrets-mcp/templates/entries.html b/crates/secrets-mcp/templates/entries.html index aca5321..c9d48b6 100644 --- a/crates/secrets-mcp/templates/entries.html +++ b/crates/secrets-mcp/templates/entries.html @@ -584,6 +584,11 @@ var SECRET_TYPE_OPTIONS = JSON.parse(document.getElementById('secret-type-option checkingSecretName: '检查中...', secretNameAvailable: '名称可用', secretNameTaken: '该名称已被使用', + secretNameInvalid: '名称不合法', + secretNameCheckError: '校验失败，请重试', + secretNameFixBeforeSave: '请先修复密文名称校验问题后再保存', + secretTypePlaceholder: '选择类型', + secretTypeInvalid: '类型不能为空', prevPage: '上一页', nextPage: '下一页', },