fix: panic guards, dedup, and unsafe documentation

- diff.rs: graceful error instead of expect() on item ID in spawned thread
- common.rs: lazy_static regex, avoid unwrap on regex captures
- db.rs: ok_or_else guard on item.id in delete_item
- list/get/info/export/client/list: use settings.meta_filter() helper
- item_service.rs: expect() on meta lock instead of silent swallow
- filter_plugin/mod.rs: extract parse_encoding_option() helper
- main.rs: document unsafe libc::umask block with safety rationale
This commit is contained in:
2026-03-20 17:17:58 -03:00
parent 52e9787edb
commit 2cfee5075e
11 changed files with 56 additions and 86 deletions

View File

@@ -533,7 +533,10 @@ pub fn update_item(conn: &Connection, item: Item) -> Result<()> {
/// ``` /// ```
pub fn delete_item(conn: &Connection, item: Item) -> Result<()> { pub fn delete_item(conn: &Connection, item: Item) -> Result<()> {
debug!("DB: Deleting item: {item:?}"); debug!("DB: Deleting item: {item:?}");
conn.execute("DELETE FROM items WHERE id=?1", params![item.id])?; let id = item
.id
.ok_or_else(|| anyhow::anyhow!("Cannot delete item: ID is None"))?;
conn.execute("DELETE FROM items WHERE id=?1", params![id])?;
Ok(()) Ok(())
} }

View File

@@ -702,13 +702,9 @@ fn create_specific_filter(
"head_tokens filter requires 'count' parameter", "head_tokens filter requires 'count' parameter",
) )
})?; })?;
let encoding = options let (encoding, tokenizer) = parse_encoding_option(options);
.get("encoding")
.and_then(|v| v.as_str())
.and_then(|s| s.parse::<crate::tokenizer::TokenEncoding>().ok())
.unwrap_or_default();
let mut f = tokens::HeadTokensFilter::new(count); let mut f = tokens::HeadTokensFilter::new(count);
f.tokenizer = crate::tokenizer::get_tokenizer(encoding).clone(); f.tokenizer = tokenizer;
f.encoding = encoding; f.encoding = encoding;
Ok(Box::new(f)) Ok(Box::new(f))
} }
@@ -724,13 +720,9 @@ fn create_specific_filter(
"skip_tokens filter requires 'count' parameter", "skip_tokens filter requires 'count' parameter",
) )
})?; })?;
let encoding = options let (encoding, tokenizer) = parse_encoding_option(options);
.get("encoding")
.and_then(|v| v.as_str())
.and_then(|s| s.parse::<crate::tokenizer::TokenEncoding>().ok())
.unwrap_or_default();
let mut f = tokens::SkipTokensFilter::new(count); let mut f = tokens::SkipTokensFilter::new(count);
f.tokenizer = crate::tokenizer::get_tokenizer(encoding).clone(); f.tokenizer = tokenizer;
f.encoding = encoding; f.encoding = encoding;
Ok(Box::new(f)) Ok(Box::new(f))
} }
@@ -746,19 +738,28 @@ fn create_specific_filter(
"tail_tokens filter requires 'count' parameter", "tail_tokens filter requires 'count' parameter",
) )
})?; })?;
let encoding = options let (encoding, tokenizer) = parse_encoding_option(options);
.get("encoding")
.and_then(|v| v.as_str())
.and_then(|s| s.parse::<crate::tokenizer::TokenEncoding>().ok())
.unwrap_or_default();
let mut f = tokens::TailTokensFilter::new(count); let mut f = tokens::TailTokensFilter::new(count);
f.tokenizer = crate::tokenizer::get_tokenizer(encoding).clone(); f.tokenizer = tokenizer;
f.encoding = encoding; f.encoding = encoding;
Ok(Box::new(f)) Ok(Box::new(f))
} }
} }
} }
#[cfg(feature = "tokens")]
fn parse_encoding_option(
options: &std::collections::HashMap<String, serde_json::Value>,
) -> (crate::tokenizer::TokenEncoding, crate::tokenizer::Tokenizer) {
let encoding = options
.get("encoding")
.and_then(|v| v.as_str())
.and_then(|s| s.parse::<crate::tokenizer::TokenEncoding>().ok())
.unwrap_or_default();
let tokenizer = crate::tokenizer::get_tokenizer(encoding).clone();
(encoding, tokenizer)
}
/// Parses an option value from a string into a JSON value. /// Parses an option value from a string into a JSON value.
/// ///
/// # Arguments /// # Arguments

View File

@@ -291,6 +291,9 @@ fn main() -> Result<(), Error> {
} }
} }
// SAFETY: umask is thread-safe by POSIX spec, and we invoke it exactly once
// before any file operations to set a secure default mask. No other threads
// exist yet at this point in main(), so there is no data race.
unsafe { unsafe {
libc::umask(0o077); libc::umask(0o077);
} }

View File

@@ -15,12 +15,7 @@ pub fn mode(
) -> Result<(), anyhow::Error> { ) -> Result<(), anyhow::Error> {
debug!("CLIENT_LIST: Listing items via remote server"); debug!("CLIENT_LIST: Listing items via remote server");
let meta_filter: std::collections::HashMap<String, Option<String>> = settings let items = client.list_items(ids, tags, "newest", 0, 100, &settings.meta_filter())?;
.meta
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
let items = client.list_items(ids, tags, "newest", 0, 100, &meta_filter)?;
if settings.ids_only { if settings.ids_only {
for item in &items { for item in &items {

View File

@@ -21,6 +21,7 @@ use chrono::{DateTime, Utc};
use clap::Command; use clap::Command;
use clap::error::ErrorKind; use clap::error::ErrorKind;
use comfy_table::{Attribute, Cell, ContentArrangement, Table}; use comfy_table::{Attribute, Cell, ContentArrangement, Table};
use lazy_static::lazy_static;
use log::debug; use log::debug;
use regex::Regex; use regex::Regex;
use serde::{Deserialize, Serialize}; use serde::{Deserialize, Serialize};
@@ -56,38 +57,21 @@ pub enum OutputFormat {
Yaml, Yaml,
} }
/// Extracts metadata from KEEP_META_* environment variables. lazy_static! {
/// static ref KEEP_META_RE: Regex = Regex::new(r"^KEEP_META_(.+)$").unwrap();
/// Scans environment for variables prefixed with KEEP_META_ and extracts }
/// key-value pairs for initial item metadata. Ignores KEEP_META_PLUGINS.
///
/// # Returns
///
/// `HashMap<String, String>` - Metadata from environment variables, with keys in uppercase without prefix.
///
/// # Errors
///
/// None; silently ignores non-matching vars and PLUGINS.
///
/// # Examples
///
/// ```ignore
/// use std::env;
/// env::set_var("KEEP_META_COMMAND", "ls -la");
/// let meta = keep::modes::common::get_meta_from_env();
/// assert_eq!(meta.get("COMMAND"), Some(&"ls -la".to_string()));
/// ```
pub fn get_meta_from_env() -> HashMap<String, String> { pub fn get_meta_from_env() -> HashMap<String, String> {
debug!("COMMON: Getting meta from KEEP_META_*"); debug!("COMMON: Getting meta from KEEP_META_*");
let re = Regex::new(r"^KEEP_META_(.+)$").unwrap();
let mut meta_env: HashMap<String, String> = HashMap::new(); let mut meta_env: HashMap<String, String> = HashMap::new();
for (key, value) in env::vars() { for (key, value) in env::vars() {
if let Some(meta_name_caps) = re.captures(key.as_str()) { if let Some(meta_name_caps) = KEEP_META_RE.captures(key.as_str()) {
let name = String::from(meta_name_caps.get(1).unwrap().as_str()); let name = meta_name_caps.get(1).map(|m| m.as_str().to_string());
// Ignore KEEP_META_PLUGINS if let Some(name) = name {
if name != "PLUGINS" { if name != "PLUGINS" {
debug!("COMMON: Found meta: {}={}", name.clone(), value.clone()); debug!("COMMON: Found meta: {}={}", name, value);
meta_env.insert(name, value.clone()); meta_env.insert(name, value);
}
} }
} }
} }

View File

@@ -6,7 +6,7 @@
use crate::config; use crate::config;
use crate::services::compression_service::CompressionService; use crate::services::compression_service::CompressionService;
use crate::services::item_service::ItemService; use crate::services::item_service::ItemService;
use anyhow::{Context, Result}; use anyhow::{Context, Result, anyhow};
use clap::Command; use clap::Command;
use command_fds::{CommandFdExt, FdMapping}; use command_fds::{CommandFdExt, FdMapping};
use log::debug; use log::debug;
@@ -104,16 +104,19 @@ fn spawn_writer_thread(
write_fd: OwnedFd, write_fd: OwnedFd,
) -> std::thread::JoinHandle<Result<()>> { ) -> std::thread::JoinHandle<Result<()>> {
let data_path = item_service.get_data_path().clone(); let data_path = item_service.get_data_path().clone();
let item_id = item.item.id.expect("item must have ID"); let id = match item.item.id {
Some(id) => id,
None => return std::thread::spawn(|| Err(anyhow!("item missing ID"))),
};
let compression = item.item.compression.clone(); let compression = item.item.compression.clone();
let mut item_path = data_path; let mut item_path = data_path;
item_path.push(item_id.to_string()); item_path.push(id.to_string());
std::thread::spawn(move || -> Result<()> { std::thread::spawn(move || -> Result<()> {
let compression_service = CompressionService::new(); let compression_service = CompressionService::new();
let mut reader = compression_service let mut reader = compression_service
.stream_item_content(item_path, &compression) .stream_item_content(item_path, &compression)
.map_err(|e| anyhow::anyhow!("Failed to stream item {item_id}: {e}"))?; .map_err(|e| anyhow::anyhow!("Failed to stream item {id}: {e}"))?;
// Convert OwnedFd to File — safe, takes ownership, closes on drop // Convert OwnedFd to File — safe, takes ownership, closes on drop
let mut writer = std::fs::File::from(write_fd); let mut writer = std::fs::File::from(write_fd);
@@ -121,7 +124,7 @@ fn spawn_writer_thread(
use std::io::Write; use std::io::Write;
writer.write_all(chunk) writer.write_all(chunk)
}) })
.map_err(|e| anyhow::anyhow!("Error reading item {item_id}: {e}"))?; .map_err(|e| anyhow::anyhow!("Error reading item {id}: {e}"))?;
// writer dropped here, closing write_fd → diff sees EOF // writer dropped here, closing write_fd → diff sees EOF
Ok(()) Ok(())
}) })

View File

@@ -44,11 +44,7 @@ pub fn mode_export(
} }
let item_service = ItemService::new(data_path.clone()); let item_service = ItemService::new(data_path.clone());
let meta_filter: HashMap<String, Option<String>> = settings let meta_filter = settings.meta_filter();
.meta
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
// Resolve items // Resolve items
let items: Vec<ItemWithMeta> = if !ids.is_empty() { let items: Vec<ItemWithMeta> = if !ids.is_empty() {

View File

@@ -51,13 +51,8 @@ pub fn mode_get(
// If both are empty, find_item will find the last item // If both are empty, find_item will find the last item
let item_service = ItemService::new(data_path.clone()); let item_service = ItemService::new(data_path.clone());
let meta_filter: std::collections::HashMap<String, Option<String>> = settings
.meta
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
let item_with_meta = item_service let item_with_meta = item_service
.find_item(conn, ids, tags, &meta_filter) .find_item(conn, ids, tags, &settings.meta_filter())
.map_err(|e| anyhow!("Unable to find matching item in database: {}", e))?; .map_err(|e| anyhow!("Unable to find matching item in database: {}", e))?;
let item_id = item_with_meta.item.id.context("Item missing ID")?; let item_id = item_with_meta.item.id.context("Item missing ID")?;

View File

@@ -64,13 +64,8 @@ pub fn mode_info(
// If both are empty, find_item will find the last item // If both are empty, find_item will find the last item
let item_service = ItemService::new(data_path.clone()); let item_service = ItemService::new(data_path.clone());
let meta_filter: std::collections::HashMap<String, Option<String>> = settings
.meta
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
let item_with_meta = item_service let item_with_meta = item_service
.find_item(conn, ids, tags, &meta_filter) .find_item(conn, ids, tags, &settings.meta_filter())
.map_err(|e| anyhow!("Unable to find matching item in database: {}", e))?; .map_err(|e| anyhow!("Unable to find matching item in database: {}", e))?;
show_item(item_with_meta, settings, data_path) show_item(item_with_meta, settings, data_path)

View File

@@ -89,12 +89,7 @@ pub fn mode_list(
data_path: std::path::PathBuf, data_path: std::path::PathBuf,
) -> Result<()> { ) -> Result<()> {
let item_service = ItemService::new(data_path.clone()); let item_service = ItemService::new(data_path.clone());
let meta_filter: std::collections::HashMap<String, Option<String>> = settings let items_with_meta = item_service.get_items(conn, ids, tags, &settings.meta_filter())?;
.meta
.iter()
.map(|(k, v)| (k.clone(), v.clone()))
.collect();
let items_with_meta = item_service.get_items(conn, ids, tags, &meta_filter)?;
if settings.ids_only { if settings.ids_only {
for item_with_meta in &items_with_meta { for item_with_meta in &items_with_meta {

View File

@@ -688,10 +688,9 @@ impl ItemService {
meta_service.finalize_plugins(&mut plugins); meta_service.finalize_plugins(&mut plugins);
// Write collected plugin metadata to DB // Write collected plugin metadata to DB
if let Ok(entries) = collected_meta.lock() { let entries = collected_meta.lock().expect("meta lock poisoned");
for (name, value) in entries.iter() { for (name, value) in entries.iter() {
db::add_meta(conn, item_id, name, value)?; db::add_meta(conn, item_id, name, value)?;
}
} }
item.uncompressed_size = Some(total_bytes); item.uncompressed_size = Some(total_bytes);
@@ -885,7 +884,8 @@ impl ItemService {
meta_service.finalize_plugins(&mut plugins); meta_service.finalize_plugins(&mut plugins);
} }
if run_meta && let Ok(entries) = collected_meta.lock() { if run_meta {
let entries = collected_meta.lock().expect("meta lock poisoned");
for (name, value) in entries.iter() { for (name, value) in entries.iter() {
db::add_meta(conn, item_id, name, value)?; db::add_meta(conn, item_id, name, value)?;
} }