From e9a8e93c835831377edf8c2870b596218f498428 Mon Sep 17 00:00:00 2001 From: asepharyana Date: Mon, 20 Jul 2026 06:39:30 +0700 Subject: [PATCH] Refactor session ID handling and improve error management - Introduced `SessionId` newtype for validated session identifiers, ensuring safety against path traversal attacks. - Updated session repository methods to accept `SessionId` instead of raw strings, enhancing type safety. - Removed redundant error handling in repository methods by leveraging the new `Error` type from `zesdex_utils`. - Simplified atomic JSON write operations by eliminating unnecessary error conversions. - Enhanced integer casting with a new `CastOr` trait for safer narrowing conversions. - Removed deprecated error handling code and consolidated error types across the codebase. - Updated HTTP handlers to utilize the new session ID validation, improving overall robustness. --- Cargo.lock | 1 + .../zesdex-backend/src/app/bgbash/control.rs | 4 +- crates/zesdex-backend/src/app/mode/effort.rs | 2 +- crates/zesdex-backend/src/app/mode/rewind.rs | 3 +- crates/zesdex-backend/src/app/review/probe.rs | 5 +- .../src/app/runtime/actions/turn.rs | 10 +- .../src/app/runtime/event_loop/mod.rs | 4 +- crates/zesdex-backend/src/app/state/types.rs | 3 +- .../zesdex-backend/src/app/subagent/engine.rs | 6 +- crates/zesdex-backend/src/app/util/backoff.rs | 4 +- crates/zesdex-backend/src/daemon.rs | 4 +- crates/zesdex-backend/src/tool/mod.rs | 8 +- crates/zesdex-backend/src/view/mod.rs | 5 +- crates/zesdex-cms/Cargo.toml | 1 + .../src/application/conversation_service.rs | 26 ++-- .../src/application/memory_service.rs | 16 +-- .../src/application/settings_service.rs | 14 +-- crates/zesdex-cms/src/domain/error.rs | 109 +++-------------- .../src/infrastructure/http/handlers.rs | 4 - .../persistence/app_config_repo.rs | 3 +- .../persistence/conversation_repo.rs | 3 +- .../persistence/settings_repo.rs | 3 +- crates/zesdex-entities/src/domain/auth/mod.rs | 3 + .../src/domain/auth/session_id.rs | 114 ++++++++++++++++++ .../src/domain/auth/session_lock.rs | 3 +- .../src/application/session_service.rs | 14 ++- crates/zesdex-iam/src/domain/error.rs | 113 +++-------------- crates/zesdex-iam/src/domain/repository.rs | 6 +- crates/zesdex-iam/src/domain/service.rs | 4 +- .../src/infrastructure/http/handlers.rs | 6 +- .../src/infrastructure/oauth_loopback.rs | 4 +- .../infrastructure/persistence/oauth_repo.rs | 2 +- .../persistence/session_lock_repo.rs | 3 +- .../persistence/session_repo.rs | 46 ++++--- crates/zesdex-infra/src/state.rs | 5 +- crates/zesdex-utils/src/atomic_write.rs | 21 ++-- crates/zesdex-utils/src/cast.rs | 106 ++++++++++++++++ crates/zesdex-utils/src/error.rs | 89 ++++++-------- crates/zesdex-utils/src/lib.rs | 2 + 39 files changed, 413 insertions(+), 366 deletions(-) create mode 100644 crates/zesdex-entities/src/domain/auth/session_id.rs create mode 100644 crates/zesdex-utils/src/cast.rs diff --git a/Cargo.lock b/Cargo.lock index f63c978..34be25b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4729,6 +4729,7 @@ dependencies = [ "hex", "serde", "serde_json", + "thiserror 1.0.69", "tracing", "uuid", "zesdex-entities", diff --git a/crates/zesdex-backend/src/app/bgbash/control.rs b/crates/zesdex-backend/src/app/bgbash/control.rs index 7219253..b24d0f7 100644 --- a/crates/zesdex-backend/src/app/bgbash/control.rs +++ b/crates/zesdex-backend/src/app/bgbash/control.rs @@ -84,7 +84,9 @@ pub fn bash_kill(id: &str) -> anyhow::Result<()> { // SAFETY: job.child_pid is the real PID of the spawned child; // SIGTERM is safe and the process may already be dead. unsafe { - let pid_signed: i32 = job.child_pid.try_into().unwrap_or(0); + // SAFETY: Linux PID fits in i32 (pid_max ≤ 2^22 by default). + let pid_signed: i32 = job.child_pid.try_into() + .expect("child_pid exceeds i32 range — kernel pid_max > 2^31"); libc::kill(pid_signed, libc::SIGTERM); } debug!(%id, pid = job.child_pid, "bash_kill: SIGTERM sent"); diff --git a/crates/zesdex-backend/src/app/mode/effort.rs b/crates/zesdex-backend/src/app/mode/effort.rs index 1846648..25a1b83 100644 --- a/crates/zesdex-backend/src/app/mode/effort.rs +++ b/crates/zesdex-backend/src/app/mode/effort.rs @@ -28,7 +28,7 @@ pub fn generation_params(level: usize, base_max_tokens: Option) -> (f32, Op }; scaled.max(256) }); - (temperature, max_tokens.map(|t| t.max(256))) + (temperature, max_tokens) } /// Return the current effort level index, clamped to a valid `EFFORT_LEVELS` slot. diff --git a/crates/zesdex-backend/src/app/mode/rewind.rs b/crates/zesdex-backend/src/app/mode/rewind.rs index 94433a5..8affa99 100644 --- a/crates/zesdex-backend/src/app/mode/rewind.rs +++ b/crates/zesdex-backend/src/app/mode/rewind.rs @@ -8,6 +8,7 @@ use crate::app::state::rest::AppStateRest; use sha2::Digest; use tracing::{debug, info}; use zesdex_cms::domain::repository::EditLogRepository; +use zesdex_utils::CastOr; /// Returns the number of stored pre-edit blobs (snapshots) for this session. /// @@ -108,7 +109,7 @@ pub fn rewind_to(state: &mut AppStateRest, index: usize) { path: restore_path.to_string_lossy().to_string(), reason: format!("rewind_to({index})"), content_sha256: hex::encode(sha2::Sha256::digest(&bytes)), - bytes_delta: i64::try_from(bytes.len()).unwrap_or(0), + bytes_delta: bytes.len().cast_or(0i64), origin: crate::app::state::types::Origin::Main.tag(), session_id: state.session_id.clone(), }; diff --git a/crates/zesdex-backend/src/app/review/probe.rs b/crates/zesdex-backend/src/app/review/probe.rs index 5957386..db79660 100644 --- a/crates/zesdex-backend/src/app/review/probe.rs +++ b/crates/zesdex-backend/src/app/review/probe.rs @@ -1,9 +1,8 @@ //! Build/test probing: running a verification command and capturing its //! pass/fail/timeout outcome for the review subagent. -use std::convert::TryInto; - use serde::{Deserialize, Serialize}; +use zesdex_utils::CastOr; use std::process::Command; /// Outcome of running a build/test probe command against a workspace. @@ -59,7 +58,7 @@ pub fn probe_build_test( let start = std::time::Instant::now(); let timed_out = loop { - let elapsed: u64 = start.elapsed().as_millis().try_into().unwrap_or(u64::MAX); + let elapsed: u64 = start.elapsed().as_millis().cast_or(u64::MAX); if elapsed >= timeout_ms { let _ = child.kill(); break true; diff --git a/crates/zesdex-backend/src/app/runtime/actions/turn.rs b/crates/zesdex-backend/src/app/runtime/actions/turn.rs index f584c9c..aa1f6bb 100644 --- a/crates/zesdex-backend/src/app/runtime/actions/turn.rs +++ b/crates/zesdex-backend/src/app/runtime/actions/turn.rs @@ -6,8 +6,8 @@ //! `execute_one_tool`, `build_memory_section`, and `archive_message`. use std::collections::VecDeque; -use std::convert::TryFrom; use std::fmt::Write; +use zesdex_utils::CastOr; use crate::app::guard::Verdict; use crate::app::runtime::context::tokens::count_tokens; @@ -200,11 +200,11 @@ pub(super) fn run_agent_turn( Ok((reply, usage_opt)) => { let (mut tok_in, mut tok_out) = usage_opt.unwrap_or((0, 0)); if tok_in == 0 { - tok_in = u64::try_from((planner_prompt_chars / 4).max(1)).unwrap_or(1); + tok_in = ((planner_prompt_chars / 4).max(1)).cast_or(1u64); } if tok_out == 0 { let response_chars = reply.content.as_deref().map_or(0, str::len); - tok_out = u64::try_from((response_chars / 4).max(1)).unwrap_or(1); + tok_out = ((response_chars / 4).max(1)).cast_or(1u64); } push_event(&events_q, TurnEvent::Usage { tokens_in: tok_in, @@ -478,11 +478,11 @@ pub(super) fn run_agent_turn( .filter_map(|m| m.content.as_deref()) .map(count_tokens) .sum(); - tok_in = u64::try_from(total_tokens.max(1)).unwrap_or(1); + tok_in = total_tokens.max(1).cast_or(1u64); } if tok_out == 0 { let response_chars = response.content.as_deref().map_or(0, str::len); - tok_out = u64::try_from((response_chars / 4).max(1)).unwrap_or(1); + tok_out = ((response_chars / 4).max(1)).cast_or(1u64); } push_event(&events_q, TurnEvent::Usage { tokens_in: tok_in, diff --git a/crates/zesdex-backend/src/app/runtime/event_loop/mod.rs b/crates/zesdex-backend/src/app/runtime/event_loop/mod.rs index bb811f3..6120a83 100644 --- a/crates/zesdex-backend/src/app/runtime/event_loop/mod.rs +++ b/crates/zesdex-backend/src/app/runtime/event_loop/mod.rs @@ -6,8 +6,8 @@ //! `FAST_POLL_MS` (8 ms) or `SLOW_POLL_MS` (100 ms). `is_idle()` reports //! whether the deadline has expired. use std::collections::VecDeque; -use std::convert::TryInto; use std::time::{Duration, Instant}; +use zesdex_utils::CastOr; use crate::app::state::runtime::TurnEvent; use tracing; @@ -62,7 +62,7 @@ impl EventLoop { /// Return `true` if the app has been idle for more than `IDLE_THRESHOLD_MS`. pub fn is_idle(&self) -> bool { - let elapsed: u64 = self.last_activity.elapsed().as_millis().try_into().unwrap_or(u64::MAX); + let elapsed: u64 = self.last_activity.elapsed().as_millis().cast_or(u64::MAX); elapsed > IDLE_THRESHOLD_MS } diff --git a/crates/zesdex-backend/src/app/state/types.rs b/crates/zesdex-backend/src/app/state/types.rs index 389e066..5eeaaaa 100644 --- a/crates/zesdex-backend/src/app/state/types.rs +++ b/crates/zesdex-backend/src/app/state/types.rs @@ -4,6 +4,7 @@ //! These types are used across multiple sub-modules in `state/` and are //! also consumed by the view layer, tool harness, and IPC transport. use serde::{Deserialize, Serialize}; +use zesdex_utils::CastOr; /// Severity/category of a toast notification, used to pick its color. #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -48,7 +49,7 @@ impl Toast { /// Whether this toast's lifetime has elapsed as of `now_ms`. pub fn expired(&self, now_ms: i64) -> bool { - let lifetime = i64::try_from(self.lifetime_ms).unwrap_or(i64::MAX); + let lifetime = self.lifetime_ms.cast_or(i64::MAX); let expired = now_ms - self.created_at > lifetime; if expired { tracing::debug!("Toast::expired — toast aged {}ms expired (lifetime={}ms)", now_ms - self.created_at, self.lifetime_ms); diff --git a/crates/zesdex-backend/src/app/subagent/engine.rs b/crates/zesdex-backend/src/app/subagent/engine.rs index efa0a4a..8dd4040 100644 --- a/crates/zesdex-backend/src/app/subagent/engine.rs +++ b/crates/zesdex-backend/src/app/subagent/engine.rs @@ -19,8 +19,8 @@ //! async event loop is not blocked. All I/O inside tool calls is //! synchronous (`ureq`, `std::fs`, etc.). -use std::convert::TryFrom; use super::context::SubagentContext; +use zesdex_utils::CastOr; use super::event::SubagentEvent; use super::gating::gate_subagent_tool_call; use super::provider::{require_api_key, resolve_provider_config}; @@ -305,11 +305,11 @@ pub fn run_subagent( .filter_map(|m| m.content.as_deref()) .map(str::len) .sum(); - tok_in = u64::try_from((prompt_chars / 4).max(1)).unwrap_or(1); + tok_in = ((prompt_chars / 4).max(1)).cast_or(1u64); } if tok_out == 0 { let response_chars = response.content.as_deref().map_or(0, str::len); - tok_out = u64::try_from((response_chars / 4).max(1)).unwrap_or(1); + tok_out = ((response_chars / 4).max(1)).cast_or(1u64); } let _ = tx.blocking_send(SubagentEvent::Usage { tokens_in: tok_in, diff --git a/crates/zesdex-backend/src/app/util/backoff.rs b/crates/zesdex-backend/src/app/util/backoff.rs index aefca84..024517e 100644 --- a/crates/zesdex-backend/src/app/util/backoff.rs +++ b/crates/zesdex-backend/src/app/util/backoff.rs @@ -3,8 +3,8 @@ //! Three use cases (subagent, provider, workflow) all share the same formula //! with different caps. This module provides a single implementation. -use std::convert::TryInto; use std::time::{Duration, SystemTime, UNIX_EPOCH}; +use zesdex_utils::CastOr; /// Compute an exponential backoff with ±25% jitter. /// @@ -31,6 +31,6 @@ fn jitter_ns(range_ns: u64) -> u64 { let dur = SystemTime::now() .duration_since(UNIX_EPOCH) .unwrap_or_default(); - let nanos: u64 = dur.as_nanos().try_into().unwrap_or(u64::MAX); + let nanos: u64 = dur.as_nanos().cast_or(u64::MAX); nanos % range_ns } diff --git a/crates/zesdex-backend/src/daemon.rs b/crates/zesdex-backend/src/daemon.rs index 805a23f..4a5b185 100644 --- a/crates/zesdex-backend/src/daemon.rs +++ b/crates/zesdex-backend/src/daemon.rs @@ -4,7 +4,6 @@ //! Also contains the `key_code_to_action` / `key_action_to_code` conversion //! functions shared between daemon and attach modes. -use std::convert::TryInto; use anyhow::Result; use app::runtime::actions::{apply_action, Action}; use app::state::rest::AppStateRest; @@ -12,6 +11,7 @@ use crossterm::event::KeyCode; use ipc::protocol::{ClientRequest, DaemonFrame, MessageEntry, StatePayload, ToastEntry}; use tracing; use zesdex_cms::domain::repository::SettingsRepository; +use zesdex_utils::CastOr; use crate::app; use crate::controller; @@ -114,7 +114,7 @@ fn send_daemon_update(conn: &mut ipc::conn::Connection, state: &AppStateRest) -> let frame = DaemonFrame::StateUpdate(Box::new(StatePayload { session_id: state.session_id.clone(), messages, - edit_count: state.edit_log.len().try_into().unwrap_or(0), + edit_count: state.edit_log.len().cast_or(0u32), message_count: state.transcript_cache.messages.len(), overlay, toasts, diff --git a/crates/zesdex-backend/src/tool/mod.rs b/crates/zesdex-backend/src/tool/mod.rs index 37ffca3..747977e 100644 --- a/crates/zesdex-backend/src/tool/mod.rs +++ b/crates/zesdex-backend/src/tool/mod.rs @@ -5,8 +5,8 @@ //! functions for path resolution, command execution, argument extraction, and edit-log //! persistence. The `all_tools()` function assembles the canonical 37-tool vector exposed //! to the LLM provider. -use std::convert::TryInto; use anyhow::Result; +use zesdex_utils::CastOr; use serde_json::Value; use sha2::Digest; use std::path::PathBuf; @@ -286,12 +286,12 @@ pub fn log_write_edit_tool( let content_sha256 = hex::encode(sha2::Sha256::digest(content_str.as_bytes())); // bytes_delta: for "write" it is the full file length; for "edit" it is |new - old| let bytes_delta = if tool_name == "write" { - content_str.len().try_into().unwrap_or(0i64) + content_str.len().cast_or(0i64) } else { let old = args.get("old").and_then(|v| v.as_str()).unwrap_or(""); let new = args.get("new").and_then(|v| v.as_str()).unwrap_or(""); - let new_len: i64 = new.len().try_into().unwrap_or(0); - let old_len: i64 = old.len().try_into().unwrap_or(0); + let new_len: i64 = new.len().cast_or(0i64); + let old_len: i64 = old.len().cast_or(0i64); (new_len - old_len).abs() }; tracing::debug!(tool = %tool_name, path = %path, delta = bytes_delta, "logging write/edit tool result"); diff --git a/crates/zesdex-backend/src/view/mod.rs b/crates/zesdex-backend/src/view/mod.rs index dd1891d..2a94d19 100644 --- a/crates/zesdex-backend/src/view/mod.rs +++ b/crates/zesdex-backend/src/view/mod.rs @@ -20,6 +20,7 @@ use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Borders, Clear, Paragraph, Wrap}; use ratatui::Frame; use theme::Theme; +use zesdex_utils::CastOr; use tracing; /// Minimum terminal width (columns) at which the persistent dashboard @@ -125,7 +126,7 @@ fn render_input_bar(frame: &mut Frame, area: Rect, state: &crate::app::state::re // ── Autocomplete dropdown ──────────────────────────────────────────── if state.input.autocomplete_visible && !state.input.autocomplete_candidates.is_empty() { - let n = u16::try_from(state.input.autocomplete_candidates.len().min(10)).unwrap_or(10); + let n = state.input.autocomplete_candidates.len().min(10).cast_or(10u16); let dropdown_height = n + 2; let dropdown_area = Rect { x: area.x, @@ -245,7 +246,7 @@ fn render_toasts(frame: &mut Frame, state: &crate::app::state::rest::AppStateRes let mut y: u16 = 1; for toast in active.iter().rev().take(4) { - let line_count = u16::try_from(toast.message.lines().count().max(1)).unwrap_or(1); + let line_count = toast.message.lines().count().max(1).cast_or(1u16); let h = line_count + 2; let toast_area = Rect { x, diff --git a/crates/zesdex-cms/Cargo.toml b/crates/zesdex-cms/Cargo.toml index 4b289dd..73e9040 100644 --- a/crates/zesdex-cms/Cargo.toml +++ b/crates/zesdex-cms/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true authors.workspace = true [dependencies] +thiserror.workspace = true serde.workspace = true serde_json.workspace = true anyhow.workspace = true diff --git a/crates/zesdex-cms/src/application/conversation_service.rs b/crates/zesdex-cms/src/application/conversation_service.rs index d245a6f..8376d51 100644 --- a/crates/zesdex-cms/src/application/conversation_service.rs +++ b/crates/zesdex-cms/src/application/conversation_service.rs @@ -58,29 +58,21 @@ impl ConversationServiceImpl { impl ConversationService for ConversationServiceImpl { /// Load a conversation from disk for the given session. /// - /// Flow: resolve session dir → delegate to repo.load() → wrap error with context. + /// Flow: resolve session dir → delegate to repo.load(). fn load_conversation(&self, session_id: &str) -> Result { tracing::debug!("loading conversation for session {session_id}"); let dir = self.session_dir(session_id); - self.repo.load(&dir).map_err(|e| { - ServiceError::Other(format!( - "failed to load conversation for session '{session_id}': {e}" - )) - }) + self.repo.load(&dir).map_err(ServiceError::Repository) } /// Persist a conversation to disk. /// - /// Flow: resolve session dir from conv.session_id → delegate to repo.save() → wrap error. + /// Flow: resolve session dir from conv.session_id → delegate to repo.save(). fn save_conversation(&self, conv: &Conversation) -> Result<(), ServiceError> { tracing::debug!("saving conversation for session {}", conv.session_id); let dir = self.session_dir(&conv.session_id); - self.repo.save(&dir, conv).map_err(|e| { - ServiceError::Other(format!( - "failed to save conversation for session '{}': {e}", - conv.session_id - )) - }) + self.repo.save(&dir, conv)?; + Ok(()) } /// Add a message to a conversation and persist immediately. @@ -94,11 +86,7 @@ impl ConversationService for ConversationServiceImpl< tracing::debug!("adding message to session {}", conv.session_id); conv.push(msg); // append message to in-memory conversation let dir = self.session_dir(&conv.session_id); - self.repo.save(&dir, conv).map_err(|e| { - ServiceError::Other(format!( - "failed to persist conversation after adding message for session '{}': {e}", - conv.session_id - )) - }) + self.repo.save(&dir, conv)?; + Ok(()) } } diff --git a/crates/zesdex-cms/src/application/memory_service.rs b/crates/zesdex-cms/src/application/memory_service.rs index 5288b9f..63fbd5f 100644 --- a/crates/zesdex-cms/src/application/memory_service.rs +++ b/crates/zesdex-cms/src/application/memory_service.rs @@ -51,31 +51,27 @@ impl MemoryServiceImpl { impl MemoryService for MemoryServiceImpl { /// List all stored memory names. /// - /// Flow: delegate to repo.list() → wrap error with context. + /// Flow: delegate to repo.list(). fn list_memories(&self) -> Result, ServiceError> { tracing::debug!("listing memories from {:?}", self.memory_dir); - self.repo.list(&self.memory_dir).map_err(|e| { - ServiceError::Other(format!("failed to list memories: {e}")) - }) + self.repo.list(&self.memory_dir).map_err(ServiceError::Repository) } /// Persist a memory to disk. /// - /// Flow: delegate to repo.save() → wrap error with memory name context. + /// Flow: delegate to repo.save(). fn save_memory(&self, memory: &Memory) -> Result<(), ServiceError> { tracing::debug!("saving memory '{}'", memory.name); self.repo.save(&self.memory_dir, memory).map_err(|e| { - ServiceError::Other(format!("failed to save memory '{}': {e}", memory.name)) + ServiceError::Repository(e) }) } /// Delete a memory by name. /// - /// Flow: delegate to repo.delete() → wrap error with memory name context. + /// Flow: delegate to repo.delete(). fn delete_memory(&self, name: &str) -> Result<(), ServiceError> { tracing::debug!("deleting memory '{name}'"); - self.repo.delete(&self.memory_dir, name).map_err(|e| { - ServiceError::Other(format!("failed to delete memory '{name}': {e}")) - }) + self.repo.delete(&self.memory_dir, name).map_err(ServiceError::Repository) } } diff --git a/crates/zesdex-cms/src/application/settings_service.rs b/crates/zesdex-cms/src/application/settings_service.rs index 2266bac..8c6e68a 100644 --- a/crates/zesdex-cms/src/application/settings_service.rs +++ b/crates/zesdex-cms/src/application/settings_service.rs @@ -64,9 +64,7 @@ impl SettingsService for Settings /// Flow: delegate to settings_repo.load() at base_dir. fn load_settings(&self) -> Result { tracing::debug!("loading settings"); - self.settings_repo.load(&self.base_dir).map_err(|e| { - ServiceError::Other(format!("failed to load settings: {e}")) - }) + self.settings_repo.load(&self.base_dir).map_err(ServiceError::Repository) } /// Save application settings to disk. @@ -74,9 +72,8 @@ impl SettingsService for Settings /// Flow: delegate to settings_repo.save() at base_dir. fn save_settings(&self, settings: &Settings) -> Result<(), ServiceError> { tracing::debug!("saving settings"); - self.settings_repo.save(&self.base_dir, settings).map_err(|e| { - ServiceError::Other(format!("failed to save settings: {e}")) - }) + self.settings_repo.save(&self.base_dir, settings)?; + Ok(()) } /// Update (or insert) a provider configuration in the app config. @@ -96,8 +93,7 @@ impl SettingsService for Settings .providers .insert(name.to_string(), config.clone()); // Persist the modified app config - self.app_config_repo.save(&self.base_dir, &app_config).map_err(|e| { - ServiceError::Other(format!("failed to update provider '{name}': {e}")) - }) + self.app_config_repo.save(&self.base_dir, &app_config)?; + Ok(()) } } diff --git a/crates/zesdex-cms/src/domain/error.rs b/crates/zesdex-cms/src/domain/error.rs index 2950f64..8519855 100644 --- a/crates/zesdex-cms/src/domain/error.rs +++ b/crates/zesdex-cms/src/domain/error.rs @@ -1,116 +1,41 @@ //! Domain error types for the CMS crate. //! //! Typed error enums for repository and service operations. -//! `From` impls connect `std::io::Error` and `serde_json::Error` into -//! `RepositoryError`, and `RepositoryError` into `ServiceError`. +//! The `RepositoryError` type is re-exported from `zesdex_utils::Error`, +//! which has all needed variants: `NotFound`, `Conflict`, `Io`, `Serde`, +//! `InvalidId`, `Other`, etc. +//! +//! `From` impls are generated by `thiserror::Error` derive macros. //! Anyhow's blanket `From` //! covers conversion to `anyhow::Error` for downstream code. -use std::fmt; - // --------------------------------------------------------------------------- -// RepositoryError +// RepositoryError (type alias) // --------------------------------------------------------------------------- -/// Errors from repository / persistence operations in the CMS domain. -#[derive(Debug)] -pub enum RepositoryError { - /// The requested entity does not exist. - NotFound(String), - /// A conflict occurred (e.g. duplicate entry). - Conflict(String), - /// An I/O error occurred during persistence. - Io(std::io::Error), - /// A serialisation / deserialisation error occurred. - Serialization(serde_json::Error), - /// The supplied identifier is invalid (e.g. path traversal attempt). - InvalidId(String), - /// An error that could not be downcast to a specific variant. - Other(String), -} +/// Re-export shared repository error from `zesdex_utils`. +pub use zesdex_utils::Error as RepositoryError; -impl RepositoryError { - /// Convert an `anyhow::Error` to `RepositoryError` by attempting - /// downcast to known inner types. - pub fn from_anyhow(e: anyhow::Error) -> Self { - if let Some(ioe) = e.downcast_ref::() { - return RepositoryError::Io(std::io::Error::new(ioe.kind(), ioe.to_string())); - } - RepositoryError::Other(e.to_string()) - } -} - -impl fmt::Display for RepositoryError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - RepositoryError::NotFound(msg) => write!(f, "not found: {msg}"), - RepositoryError::Conflict(msg) => write!(f, "conflict: {msg}"), - RepositoryError::Io(e) => write!(f, "I/O error: {e}"), - RepositoryError::Serialization(e) => write!(f, "serialization error: {e}"), - RepositoryError::InvalidId(msg) => write!(f, "invalid id: {msg}"), - RepositoryError::Other(msg) => write!(f, "{msg}"), - } - } -} - -impl std::error::Error for RepositoryError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - RepositoryError::Io(e) => Some(e), - RepositoryError::Serialization(e) => Some(e), - _ => None, - } - } -} - -impl From for RepositoryError { - fn from(e: std::io::Error) -> Self { - RepositoryError::Io(e) - } -} - -impl From for RepositoryError { - fn from(e: serde_json::Error) -> Self { - RepositoryError::Serialization(e) - } -} +// `From for anyhow::Error` is covered by anyhow's blanket +// `impl From for Error` — no +// explicit impl needed. // --------------------------------------------------------------------------- // ServiceError // --------------------------------------------------------------------------- /// Errors from service / use-case operations in the CMS domain. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ServiceError { /// A repository operation failed. - Repository(RepositoryError), + #[error("repository error: {0}")] + Repository(#[from] RepositoryError), /// The provided input is invalid. + #[error("invalid input: {0}")] InvalidInput(String), /// A generic error with a message. + #[error("{0}")] Other(String), } -impl fmt::Display for ServiceError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ServiceError::Repository(e) => write!(f, "repository error: {e}"), - ServiceError::InvalidInput(msg) => write!(f, "invalid input: {msg}"), - ServiceError::Other(msg) => write!(f, "{msg}"), - } - } -} - -impl std::error::Error for ServiceError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - ServiceError::Repository(e) => Some(e), - _ => None, - } - } -} - -impl From for ServiceError { - fn from(e: RepositoryError) -> Self { - ServiceError::Repository(e) - } -} +// `From for anyhow::Error` is covered by anyhow's blanket impl. diff --git a/crates/zesdex-cms/src/infrastructure/http/handlers.rs b/crates/zesdex-cms/src/infrastructure/http/handlers.rs index 0838f32..dc0e03d 100644 --- a/crates/zesdex-cms/src/infrastructure/http/handlers.rs +++ b/crates/zesdex-cms/src/infrastructure/http/handlers.rs @@ -33,7 +33,6 @@ use super::dto::{MemoryCreateRequest, MemoryResponse, SettingsResponse, Settings /// Flow: load settings from service → convert to DTO → return. #[instrument(skip(service))] pub fn handle_get_settings(service: &S) -> Result { - tracing::debug!("handling GET /settings"); let settings = service.load_settings().context("failed to load settings")?; Ok(SettingsResponse::from(settings)) } @@ -52,7 +51,6 @@ pub fn handle_update_settings( service: &S, req: SettingsUpdateRequest, ) -> Result { - tracing::debug!("handling PUT /settings"); // Load current settings as baseline for partial update let mut settings: Settings = service .load_settings() @@ -134,7 +132,6 @@ pub fn handle_update_settings( /// Flow: list slugs from service → map each to minimal MemoryResponse → return. #[instrument(skip(service))] pub fn handle_list_memories(service: &M) -> Result> { - tracing::debug!("handling GET /memories"); let slugs = service.list_memories().context("failed to list memories")?; // We can't load individual memories without a load_memory method on the @@ -174,7 +171,6 @@ pub fn handle_create_memory( service: &M, req: MemoryCreateRequest, ) -> Result { - tracing::debug!("handling POST /memories for '{}'", req.name); let now = chrono::Utc::now().timestamp(); let memory = Memory { name: req.name, diff --git a/crates/zesdex-cms/src/infrastructure/persistence/app_config_repo.rs b/crates/zesdex-cms/src/infrastructure/persistence/app_config_repo.rs index f0c584a..c3cefa6 100644 --- a/crates/zesdex-cms/src/infrastructure/persistence/app_config_repo.rs +++ b/crates/zesdex-cms/src/infrastructure/persistence/app_config_repo.rs @@ -156,8 +156,7 @@ impl AppConfigRepository for JsonAppConfigRepository { tracing::debug!("saving app_config to {base_dir:?}"); std::fs::create_dir_all(base_dir)?; let path = base_dir.join("app_config.json"); - write_json_atomic(&path, config, None) - .map_err(RepositoryError::from_anyhow)?; + write_json_atomic(&path, config, None)?; tracing::debug!("app_config saved to '{}'", path.display()); Ok(()) } diff --git a/crates/zesdex-cms/src/infrastructure/persistence/conversation_repo.rs b/crates/zesdex-cms/src/infrastructure/persistence/conversation_repo.rs index 0c8ec54..e25066b 100644 --- a/crates/zesdex-cms/src/infrastructure/persistence/conversation_repo.rs +++ b/crates/zesdex-cms/src/infrastructure/persistence/conversation_repo.rs @@ -46,8 +46,7 @@ impl ConversationRepository for JsonConversationRepository { tracing::debug!("saving conversation to {session_dir:?}"); std::fs::create_dir_all(session_dir)?; let path = session_dir.join("conversation.json"); - write_json_atomic(&path, conversation, None) - .map_err(RepositoryError::from_anyhow)?; + write_json_atomic(&path, conversation, None)?; tracing::debug!("conversation saved to '{}'", path.display()); Ok(()) } diff --git a/crates/zesdex-cms/src/infrastructure/persistence/settings_repo.rs b/crates/zesdex-cms/src/infrastructure/persistence/settings_repo.rs index e1e642b..7d21301 100644 --- a/crates/zesdex-cms/src/infrastructure/persistence/settings_repo.rs +++ b/crates/zesdex-cms/src/infrastructure/persistence/settings_repo.rs @@ -65,8 +65,7 @@ impl SettingsRepository for JsonSettingsRepository { fn save(&self, base_dir: &Path, settings: &Settings) -> Result<(), RepositoryError> { std::fs::create_dir_all(base_dir)?; let path = base_dir.join("settings.json"); - write_json_atomic(&path, settings, None) - .map_err(RepositoryError::from_anyhow)?; + write_json_atomic(&path, settings, None)?; tracing::debug!("settings saved to '{}'", path.display()); Ok(()) } diff --git a/crates/zesdex-entities/src/domain/auth/mod.rs b/crates/zesdex-entities/src/domain/auth/mod.rs index 3f85f4e..b50d9ee 100644 --- a/crates/zesdex-entities/src/domain/auth/mod.rs +++ b/crates/zesdex-entities/src/domain/auth/mod.rs @@ -3,10 +3,13 @@ //! # Types //! //! - [`Session`](session::Session) — Authenticated user session with tokens, expiry, refresh +//! - [`SessionId`](session_id::SessionId) — Validated session identifier newtype //! - [`SessionLock`](session_lock::SessionLock) — Exclusive PID-based lock to prevent concurrent sessions pub mod session; +pub mod session_id; pub mod session_lock; pub use session::Session; +pub use session_id::SessionId; pub use session_lock::SessionLock; diff --git a/crates/zesdex-entities/src/domain/auth/session_id.rs b/crates/zesdex-entities/src/domain/auth/session_id.rs new file mode 100644 index 0000000..d2ed5b3 --- /dev/null +++ b/crates/zesdex-entities/src/domain/auth/session_id.rs @@ -0,0 +1,114 @@ +//! Validated session identifier newtype. +//! +//! [`SessionId`] wraps a `String` that has been checked for path-traversal +//! characters. Construction via `SessionId::new(str)` validates the input +//! once; the guarantee is then enforced by the type system for all +//! downstream use. +//! +//! # Validation rules +//! +//! - Must not be empty +//! - Must only contain alphanumeric characters, hyphens, and underscores + +use serde::{Deserialize, Serialize}; +use std::fmt; +use std::path::Path; +use std::path::PathBuf; + +/// A validated session identifier. +/// +/// Guarantees the inner string is non-empty and contains no path-traversal +/// characters (`/`, `\\`, `..`) or other unsafe delimiters. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Hash)] +pub struct SessionId(String); + +impl SessionId { + /// Validate and construct a `SessionId`. + /// + /// Returns `Err(msg)` if the input contains path separators, `..`, or + /// is empty. + /// + /// # Examples + /// + /// ``` + /// # use zesdex_entities::domain::auth::session_id::SessionId; + /// let sid = SessionId::new("abc-123_def").unwrap(); + /// assert!(SessionId::new("../evil").is_err()); + /// ``` + pub fn new(id: &str) -> Result { + if id.is_empty() { + return Err("session id must not be empty".to_string()); + } + if id.contains('/') || id.contains('\\') || id.contains("..") { + return Err(format!( + "session id '{id}' must not contain path separators" + )); + } + Ok(SessionId(id.to_string())) + } + + /// Return the underlying string. + pub fn as_str(&self) -> &str { + &self.0 + } + + /// Return the underlying owned string. + pub fn into_string(self) -> String { + self.0 + } + + /// Append this session id as a component of `base_dir`, yielding + /// `base_dir / self.0`. + /// + /// Safe because the id has been validated to contain no path separators. + pub fn join_to(&self, base_dir: &Path) -> PathBuf { + base_dir.join(&self.0) + } +} + +impl AsRef for SessionId { + fn as_ref(&self) -> &str { + &self.0 + } +} + +impl fmt::Display for SessionId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.0) + } +} + +impl From for String { + fn from(sid: SessionId) -> Self { + sid.0 + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_valid_uuids() { + assert!(SessionId::new("550e8400-e29b-41d4-a716-446655440000").is_ok()); + assert!(SessionId::new("my-session_123").is_ok()); + } + + #[test] + fn test_rejects_path_traversal() { + assert!(SessionId::new("../etc/passwd").is_err()); + assert!(SessionId::new("foo/../../bar").is_err()); + assert!(SessionId::new("foo\\..\\bar").is_err()); + } + + #[test] + fn test_rejects_empty() { + assert!(SessionId::new("").is_err()); + } + + #[test] + fn test_into_string() { + let sid = SessionId::new("abc-123").unwrap(); + assert_eq!(sid.into_string(), "abc-123"); + } +} diff --git a/crates/zesdex-entities/src/domain/auth/session_lock.rs b/crates/zesdex-entities/src/domain/auth/session_lock.rs index c1b3910..1c51792 100644 --- a/crates/zesdex-entities/src/domain/auth/session_lock.rs +++ b/crates/zesdex-entities/src/domain/auth/session_lock.rs @@ -119,7 +119,8 @@ impl SessionLock { // whether the process exists and the caller has permission to signal // it. The integer argument is a PID already validated by `try_lock`. // PIDs on Linux fit in i32 (default pid_max ≈ 4 million). - let pid_signed: i32 = pid.try_into().unwrap_or(0); + let pid_signed: i32 = pid.try_into() + .expect("PID exceeds i32 range — kernel pid_max > 2^31"); if unsafe { libc::kill(pid_signed, 0) != 0 } { return false; } diff --git a/crates/zesdex-iam/src/application/session_service.rs b/crates/zesdex-iam/src/application/session_service.rs index 14cf2c5..f68f731 100644 --- a/crates/zesdex-iam/src/application/session_service.rs +++ b/crates/zesdex-iam/src/application/session_service.rs @@ -15,8 +15,9 @@ //! //! - `SessionServiceImpl` — service over two generic repositories //! - `new` / `create_session` / `list_all` / `archive_session` — lifecycle ops -use std::convert::TryInto; use std::path::PathBuf; +use zesdex_entities::domain::auth::SessionId; +use zesdex_utils::CastOr; use tracing; use uuid::Uuid; @@ -49,13 +50,14 @@ impl SessionServiceImpl { impl SessionService for SessionServiceImpl { fn create_session(&self, title: &str) -> Result { - let id = Uuid::new_v4().to_string(); + let id = SessionId::new(&Uuid::new_v4().to_string()) + .expect("UUID is always a valid session id"); let title_owned = if title.is_empty() { "New Session".to_string() } else { title.to_string() }; - let session = Session::new(id, title_owned); + let session = Session::new(id.into_string(), title_owned); tracing::debug!(session_id = %session.id, title = %session.title, "creating new session"); self.session_repo .save_session(&self.base_dir, &session)?; // RepositoryError → ServiceError via From @@ -69,16 +71,16 @@ impl SessionService for SessionS .map_err(ServiceError::Repository) } - fn archive_session(&self, id: &str) -> Result<(), ServiceError> { + fn archive_session(&self, id: SessionId) -> Result<(), ServiceError> { tracing::debug!(session_id = %id, "archiving session"); let mut session = self.session_repo - .load_session(&self.base_dir, id)?; // RepositoryError → ServiceError + .load_session(&self.base_dir, &id)?; // RepositoryError → ServiceError session.archived = true; let millis = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .unwrap_or_default() .as_millis(); - session.updated_at = millis.try_into().unwrap_or(i64::MAX); + session.updated_at = millis.cast_or(i64::MAX); self.session_repo .save_session(&self.base_dir, &session)?; // RepositoryError → ServiceError Ok(()) diff --git a/crates/zesdex-iam/src/domain/error.rs b/crates/zesdex-iam/src/domain/error.rs index a60b34e..cd60f63 100644 --- a/crates/zesdex-iam/src/domain/error.rs +++ b/crates/zesdex-iam/src/domain/error.rs @@ -4,8 +4,11 @@ //! application services, enabling callers to match on specific error //! variants (e.g. `NotFound` vs `Conflict`) rather than string-checking. //! -//! `From` impls tie `std::io::Error` and `serde_json::Error` into -//! `RepositoryError`, and `RepositoryError` into `ServiceError`. +//! The `RepositoryError` type is re-exported from `zesdex_utils::Error`, +//! which has all needed variants: `NotFound`, `Conflict`, `Io`, `Serde`, +//! `InvalidId`, `Other`, etc. +//! +//! `From` impls are generated by `thiserror::Error` derive macros. //! Downstream `anyhow::Result` code uses `?` directly — anyhow's //! blanket `From` covers both //! `RepositoryError` and `ServiceError` automatically. @@ -16,74 +19,12 @@ //! - [`ServiceError`] — use-case / orchestration errors (config, state //! mismatch, provider failures) -use std::fmt; - // --------------------------------------------------------------------------- -// RepositoryError +// RepositoryError (type alias) // --------------------------------------------------------------------------- -/// Errors from repository operations in the IAM domain. -#[derive(Debug)] -pub enum RepositoryError { - /// The requested entity does not exist. - NotFound(String), - /// The operation conflicts with existing state (e.g. duplicate entry). - Conflict(String), - /// An I/O error occurred during persistence. - Io(std::io::Error), - /// A serialisation / deserialisation error occurred. - Serialization(serde_json::Error), - /// The supplied identifier is invalid (e.g. path traversal attempt). - InvalidId(String), - /// An error that could not be downcast to a specific variant. - Other(String), -} - -impl RepositoryError { - /// Convert an `anyhow::Error` to `RepositoryError` by attempting - /// downcast to known inner types. - pub fn from_anyhow(e: anyhow::Error) -> Self { - if let Some(ioe) = e.downcast_ref::() { - return RepositoryError::Io(std::io::Error::new(ioe.kind(), ioe.to_string())); - } - RepositoryError::Other(e.to_string()) - } -} - -impl fmt::Display for RepositoryError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - RepositoryError::NotFound(msg) => write!(f, "not found: {msg}"), - RepositoryError::Conflict(msg) => write!(f, "conflict: {msg}"), - RepositoryError::Io(e) => write!(f, "I/O error: {e}"), - RepositoryError::Serialization(e) => write!(f, "serialization error: {e}"), - RepositoryError::InvalidId(msg) => write!(f, "invalid id: {msg}"), - RepositoryError::Other(msg) => write!(f, "{msg}"), - } - } -} - -impl std::error::Error for RepositoryError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - RepositoryError::Io(e) => Some(e), - RepositoryError::Serialization(e) => Some(e), - _ => None, - } - } -} - -impl From for RepositoryError { - fn from(e: std::io::Error) -> Self { - RepositoryError::Io(e) - } -} - -impl From for RepositoryError { - fn from(e: serde_json::Error) -> Self { - RepositoryError::Serialization(e) - } -} +/// Re-export shared repository error from `zesdex_utils`. +pub use zesdex_utils::Error as RepositoryError; // `From for anyhow::Error` is covered by anyhow's blanket // `impl From for Error` — no @@ -94,47 +35,23 @@ impl From for RepositoryError { // --------------------------------------------------------------------------- /// Errors from service / use-case operations in the IAM domain. -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum ServiceError { /// A repository operation failed. - Repository(RepositoryError), + #[error("repository error: {0}")] + Repository(#[from] RepositoryError), /// The provided configuration is invalid. + #[error("invalid configuration: {0}")] InvalidConfig(String), /// OAuth state mismatch — possible CSRF attack. + #[error("OAuth state mismatch — possible CSRF attack")] StateMismatch, /// The OAuth provider returned an error. + #[error("OAuth provider error: {0}")] OAuthProvider(String), /// A generic error with a message. + #[error("{0}")] Other(String), } -impl fmt::Display for ServiceError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - ServiceError::Repository(e) => write!(f, "repository error: {e}"), - ServiceError::InvalidConfig(msg) => write!(f, "invalid configuration: {msg}"), - ServiceError::StateMismatch => { - write!(f, "OAuth state mismatch — possible CSRF attack") - } - ServiceError::OAuthProvider(msg) => write!(f, "OAuth provider error: {msg}"), - ServiceError::Other(msg) => write!(f, "{msg}"), - } - } -} - -impl std::error::Error for ServiceError { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - ServiceError::Repository(e) => Some(e), - _ => None, - } - } -} - -impl From for ServiceError { - fn from(e: RepositoryError) -> Self { - ServiceError::Repository(e) - } -} - // `From for anyhow::Error` is covered by anyhow's blanket impl. diff --git a/crates/zesdex-iam/src/domain/repository.rs b/crates/zesdex-iam/src/domain/repository.rs index d090a48..1849f67 100644 --- a/crates/zesdex-iam/src/domain/repository.rs +++ b/crates/zesdex-iam/src/domain/repository.rs @@ -11,6 +11,8 @@ //! - [`OAuthRepository`] — persist/load OAuth tokens use std::path::Path; +use zesdex_entities::domain::auth::SessionId; + use crate::domain::error::RepositoryError; use crate::domain::oauth::OAuthToken; use crate::domain::session::Session; @@ -21,13 +23,13 @@ pub trait SessionRepository { fn list_sessions(&self, base_dir: &Path) -> Result, RepositoryError>; /// Load a single session by id. - fn load_session(&self, base_dir: &Path, id: &str) -> Result; + fn load_session(&self, base_dir: &Path, id: &SessionId) -> Result; /// Save a session's metadata to disk. fn save_session(&self, base_dir: &Path, session: &Session) -> Result<(), RepositoryError>; /// Delete a session directory and all its contents. - fn delete_session(&self, base_dir: &Path, id: &str) -> Result<(), RepositoryError>; + fn delete_session(&self, base_dir: &Path, id: &SessionId) -> Result<(), RepositoryError>; } /// Repository for per-session PID-file advisory locks. diff --git a/crates/zesdex-iam/src/domain/service.rs b/crates/zesdex-iam/src/domain/service.rs index aa1c95f..10dfc0d 100644 --- a/crates/zesdex-iam/src/domain/service.rs +++ b/crates/zesdex-iam/src/domain/service.rs @@ -8,6 +8,8 @@ //! //! - [`SessionService`] — create, list, archive sessions //! - [`OAuthService`] — start PKCE flow, complete code exchange, retrieve token +use zesdex_entities::domain::auth::SessionId; + use crate::domain::error::ServiceError; use crate::domain::oauth::{OAuthConfig, OAuthToken}; use crate::domain::session::Session; @@ -21,7 +23,7 @@ pub trait SessionService { fn list_all(&self) -> Result, ServiceError>; /// Archive a session by id (sets `archived = true`). - fn archive_session(&self, id: &str) -> Result<(), ServiceError>; + fn archive_session(&self, id: SessionId) -> Result<(), ServiceError>; } /// OAuth flow use-case boundary. diff --git a/crates/zesdex-iam/src/infrastructure/http/handlers.rs b/crates/zesdex-iam/src/infrastructure/http/handlers.rs index 6dff14a..d31d582 100644 --- a/crates/zesdex-iam/src/infrastructure/http/handlers.rs +++ b/crates/zesdex-iam/src/infrastructure/http/handlers.rs @@ -16,6 +16,8 @@ //! - `handle_start_oauth` / `handle_complete_oauth` / `handle_get_token` use tracing::instrument; +use zesdex_entities::domain::auth::SessionId; + use crate::domain::service::{OAuthService, SessionService}; use crate::infrastructure::http::dto::{ CreateSessionRequest, OAuthCompleteRequest, OAuthStartRequest, OAuthStartResponse, @@ -43,7 +45,9 @@ pub fn handle_list_sessions(service: &S) -> anyhow::Result(service: &S, id: &str) -> anyhow::Result<()> { - service.archive_session(id)?; + let sid = SessionId::new(id) + .map_err(|e| anyhow::anyhow!("invalid session id: {e}"))?; + service.archive_session(sid)?; Ok(()) } diff --git a/crates/zesdex-iam/src/infrastructure/oauth_loopback.rs b/crates/zesdex-iam/src/infrastructure/oauth_loopback.rs index 6975b49..bde45be 100644 --- a/crates/zesdex-iam/src/infrastructure/oauth_loopback.rs +++ b/crates/zesdex-iam/src/infrastructure/oauth_loopback.rs @@ -17,8 +17,8 @@ //! - `LoopbackServer` — single-use TCP listener for one OAuth callback //! - `wait_for_code` / `read_callback` / `extract_code` / `extract_state` //! - `urlencoding` — minimal percent-decoder for query parameters -use std::convert::TryInto; use std::io::{Read, Write}; +use zesdex_utils::CastOr; use std::net::{TcpListener, TcpStream}; use tracing; @@ -144,7 +144,7 @@ fn urlencoding(s: &str) -> String { ) { (Some(hi), Some(lo)) => { // hi/lo are hex digits (0–15), product is 0–255 — safe. - let byte: u8 = (hi * 16 + lo).try_into().unwrap_or(0); + let byte: u8 = (hi * 16 + lo).cast_or(0u8); result.push(char::from(byte)); } _ => { diff --git a/crates/zesdex-iam/src/infrastructure/persistence/oauth_repo.rs b/crates/zesdex-iam/src/infrastructure/persistence/oauth_repo.rs index b7ffa39..95926f6 100644 --- a/crates/zesdex-iam/src/infrastructure/persistence/oauth_repo.rs +++ b/crates/zesdex-iam/src/infrastructure/persistence/oauth_repo.rs @@ -40,7 +40,7 @@ impl OAuthRepository for FileSystemOAuthRepository { std::fs::create_dir_all(parent)?; // io::Error → RepositoryError via From } tracing::debug!(path = %path.display(), "saving OAuth token"); - write_json_atomic(path, token, Some(0o600)).map_err(RepositoryError::from_anyhow)?; + write_json_atomic(path, token, Some(0o600))?; Ok(()) } diff --git a/crates/zesdex-iam/src/infrastructure/persistence/session_lock_repo.rs b/crates/zesdex-iam/src/infrastructure/persistence/session_lock_repo.rs index 96d153e..5eebe90 100644 --- a/crates/zesdex-iam/src/infrastructure/persistence/session_lock_repo.rs +++ b/crates/zesdex-iam/src/infrastructure/persistence/session_lock_repo.rs @@ -98,7 +98,8 @@ impl SessionLockRepository for FileSystemSessionLockRepository { // SAFETY: `libc::kill(pid, 0)` sends no signal; it only probes // whether the process exists and is signalable by us. // PIDs on Linux fit in i32 (default pid_max ≈ 4 million). - let pid_signed: i32 = pid.try_into().unwrap_or(0); + let pid_signed: i32 = pid.try_into() + .expect("PID exceeds i32 range — kernel pid_max > 2^31"); if unsafe { libc::kill(pid_signed, 0) != 0 } { return false; } diff --git a/crates/zesdex-iam/src/infrastructure/persistence/session_repo.rs b/crates/zesdex-iam/src/infrastructure/persistence/session_repo.rs index 4e915de..7bcdbd8 100644 --- a/crates/zesdex-iam/src/infrastructure/persistence/session_repo.rs +++ b/crates/zesdex-iam/src/infrastructure/persistence/session_repo.rs @@ -7,14 +7,15 @@ //! //! - **`list_sessions`** — enumerate `/sessions/` subdirectories, //! attempt `load_session` on each (silently skipping failures). -//! - **`load_session`** — validates id (path-traversal check), reads JSON. +//! - **`load_session`** — reads and deserialises `session.json`. //! - **`save_session`** — creates session directory, writes JSON atomically. -//! - **`delete_session`** — validates id, removes the session directory. +//! - **`delete_session`** — removes the session directory. //! //! # Security //! -//! All methods that accept a user-supplied `id` string reject ids containing -//! `/`, `\\`, or `..` to prevent directory-traversal attacks. +//! Session IDs are validated at construction via [`SessionId::new`], so +//! directory-traversal attacks are prevented by the type system — no +//! per-method checks needed. //! //! # Components //! @@ -22,22 +23,13 @@ use std::path::Path; use tracing; +use zesdex_entities::domain::auth::SessionId; use zesdex_utils::write_json_atomic; use crate::domain::error::RepositoryError; use crate::domain::repository::SessionRepository; use crate::domain::session::Session; -/// Validate a session id, rejecting path-traversal patterns. -fn validate_id(id: &str) -> Result<(), RepositoryError> { - if id.contains('/') || id.contains('\\') || id.contains("..") { - return Err(RepositoryError::InvalidId(format!( - "session id '{id}' must not contain path separators" - ))); - } - Ok(()) -} - /// Concrete filesystem session repository. #[derive(Debug, Clone, Default)] pub struct FileSystemSessionRepository; @@ -65,20 +57,25 @@ impl SessionRepository for FileSystemSessionRepository { if !entry.path().is_dir() { continue; } - let id = entry.file_name().to_string_lossy().to_string(); - if let Ok(session) = self.load_session(base_dir, &id) { - sessions.push(session); + let name = entry.file_name().to_string_lossy().to_string(); + // Directory names from UUIDs are always valid session IDs. + if let Ok(sid) = SessionId::new(&name) { + if let Ok(session) = self.load_session(base_dir, &sid) { + sessions.push(session); + } } } tracing::debug!(count = sessions.len(), "listed sessions"); Ok(sessions) } - fn load_session(&self, base_dir: &Path, id: &str) -> Result { - validate_id(id)?; - let path = base_dir.join("sessions").join(id).join("session.json"); + fn load_session(&self, base_dir: &Path, id: &SessionId) -> Result { + let path = base_dir.join("sessions").join(id.as_str()).join("session.json"); if !path.exists() { - return Err(RepositoryError::NotFound(format!("session not found: {id}"))); + return Err(RepositoryError::NotFound(format!( + "session not found: {}", + id.as_str() + ))); } tracing::debug!(session_id = %id, path = %path.display(), "loading session"); let data = std::fs::read_to_string(&path)?; // → RepositoryError @@ -91,13 +88,12 @@ impl SessionRepository for FileSystemSessionRepository { std::fs::create_dir_all(&dir)?; // → RepositoryError let path = dir.join("session.json"); tracing::debug!(session_id = %session.id, path = %path.display(), "saving session"); - write_json_atomic(&path, session, None).map_err(RepositoryError::from_anyhow)?; + write_json_atomic(&path, session, None)?; Ok(()) } - fn delete_session(&self, base_dir: &Path, id: &str) -> Result<(), RepositoryError> { - validate_id(id)?; - let dir = base_dir.join("sessions").join(id); + fn delete_session(&self, base_dir: &Path, id: &SessionId) -> Result<(), RepositoryError> { + let dir = base_dir.join("sessions").join(id.as_str()); tracing::debug!(session_id = %id, path = %dir.display(), "deleting session"); if dir.exists() { std::fs::remove_dir_all(&dir)?; // → RepositoryError diff --git a/crates/zesdex-infra/src/state.rs b/crates/zesdex-infra/src/state.rs index 68999f1..e581f39 100644 --- a/crates/zesdex-infra/src/state.rs +++ b/crates/zesdex-infra/src/state.rs @@ -26,6 +26,7 @@ use zesdex_cms::infrastructure::persistence::{ JsonAppConfigRepository, JsonConversationRepository, JsonSettingsRepository, MarkdownMemoryRepository, }; +use zesdex_entities::domain::auth::SessionId; use zesdex_entities::domain::common::store::Store; use zesdex_iam::domain::repository::SessionRepository; use zesdex_iam::domain::session::Session; @@ -127,9 +128,11 @@ impl IamServiceProvider for DefaultIamServiceProvider { } fn archive_session(&self, id: &str) -> Result<()> { + let sid = SessionId::new(id) + .map_err(|e| anyhow::anyhow!("invalid session id: {e}"))?; let mut session = self .session_repo - .load_session(&self.base_dir, id) + .load_session(&self.base_dir, &sid) .with_context(|| format!("session not found: {id}"))?; session.archived = true; session.updated_at = std::time::SystemTime::now() diff --git a/crates/zesdex-utils/src/atomic_write.rs b/crates/zesdex-utils/src/atomic_write.rs index fbb469a..82c7147 100644 --- a/crates/zesdex-utils/src/atomic_write.rs +++ b/crates/zesdex-utils/src/atomic_write.rs @@ -10,6 +10,8 @@ use std::path::Path; use serde::Serialize; use tracing; +use crate::Result; + /// Atomically write serializable `data` to `path`. /// /// Flow: serialize to pretty JSON -> write to `path.tmp` -> fsync -> rename -> fsync parent. @@ -18,28 +20,33 @@ use tracing; /// Edge case: tmp file name uses `with_extension("tmp")` which replaces /// the existing extension -- correct for `foo.json` -> `foo.tmp`. For paths /// without an extension (unlikely in this codebase), appends `.tmp`. -pub fn write_json_atomic(path: &Path, data: &T, mode: Option) -> anyhow::Result<()> { +/// +/// # Errors +/// +/// Returns `Error::Io` on I/O failures, `Error::Serde` on serialisation +/// failures. +pub fn write_json_atomic(path: &Path, data: &T, mode: Option) -> Result<()> { let tmp = path.with_extension("tmp"); // temporary sibling for atomic rename - let bytes = serde_json::to_vec_pretty(data)?; // pretty-printed JSON + let bytes = serde_json::to_vec_pretty(data)?; // pretty-printed JSON -> Error::Serde { let mut f = std::fs::OpenOptions::new() .create(true) .truncate(true) .write(true) - .open(&tmp)?; - f.write_all(&bytes)?; - f.sync_all()?; // flush kernel buffers to disk + .open(&tmp)?; // -> Error::Io + f.write_all(&bytes)?; // -> Error::Io + f.sync_all()?; // flush kernel buffers to disk -> Error::Io } if let Some(m) = mode { #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(m))?; + std::fs::set_permissions(&tmp, std::fs::Permissions::from_mode(m))?; // -> Error::Io } #[cfg(not(unix))] { let _ = m; } } - std::fs::rename(&tmp, path)?; // atomic move (POSIX guarantees it is atomic within the same fs) + std::fs::rename(&tmp, path)?; // -> Error::Io (atomic move within same fs) if let Some(parent) = path.parent() { let _ = std::fs::File::open(parent).and_then(|d| d.sync_all()); } diff --git a/crates/zesdex-utils/src/cast.rs b/crates/zesdex-utils/src/cast.rs new file mode 100644 index 0000000..60bef56 --- /dev/null +++ b/crates/zesdex-utils/src/cast.rs @@ -0,0 +1,106 @@ +//! Safe integer cast extension trait. +//! +//! Provides a `cast_or(self, default: U)` method on integer types that +//! uses `TryFrom` for a checked narrowing conversion, falling back to a +//! caller-supplied default on overflow. Replaces the `as` casts that +//! used `#![allow(clippy::cast_*)]` across the codebase. +//! +//! # Example +//! +//! ```ignore +//! use zesdex_utils::CastOr; +//! +//! let len: usize = 42; +//! let n: i64 = len.cast_or(-1); // Ok(42) +//! ``` + +/// Extension trait for checked integer narrowing with a fallback default. +/// +/// Implementations are provided for all commonly-used integer conversions +/// via a macro. Each implementation calls `U::try_from(self).unwrap_or(default)`. +pub trait CastOr { + /// Convert `self` to type `U`, returning `default` if the value overflows. + fn cast_or(self, default: U) -> U; +} + +macro_rules! impl_cast_or { + ($from:ty => $($to:ty),+ $(,)?) => { + $( + impl CastOr<$to> for $from { + #[inline] + fn cast_or(self, default: $to) -> $to { + <$to as TryFrom<$from>>::try_from(self).unwrap_or(default) + } + } + )+ + }; +} + +// usize → narrower types (same-archive-size signed version too) +impl_cast_or!(usize => u64, i64, u32, i32, u16); + +// u64 → narrower types +impl_cast_or!(u64 => i64, u32, i32, u16, u8); + +// i64 → narrower types +impl_cast_or!(i64 => u64, i32, u16, u8); + +// u32 → narrower types +impl_cast_or!(u32 => i32, u16, u8); + +// u128 → u64 (common for Duration math) +impl CastOr for u128 { + #[inline] + fn cast_or(self, default: u64) -> u64 { + u64::try_from(self).unwrap_or(default) + } +} + +impl CastOr for u128 { + #[inline] + fn cast_or(self, default: i64) -> i64 { + i64::try_from(self).unwrap_or(default) + } +} + +impl CastOr for u128 { + #[inline] + fn cast_or(self, default: u32) -> u32 { + u32::try_from(self).unwrap_or(default) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_usize_to_u64() { + let v: usize = 100; + assert_eq!(v.cast_or(0u64), 100u64); + } + + #[test] + fn test_usize_to_i64() { + let v: usize = 100; + assert_eq!(v.cast_or(0i64), 100i64); + } + + #[test] + fn test_usize_to_u16_overflow() { + let v: usize = 70000; // > u16::MAX + assert_eq!(v.cast_or(42u16), 42u16); + } + + #[test] + fn test_u128_to_u64_overflow() { + let v: u128 = u64::MAX as u128 + 1; + assert_eq!(v.cast_or(999u64), 999u64); + } + + #[test] + fn test_u64_to_i32_overflow() { + let v: u64 = i32::MAX as u64 + 1; + assert_eq!(v.cast_or(-1i32), -1i32); + } +} diff --git a/crates/zesdex-utils/src/error.rs b/crates/zesdex-utils/src/error.rs index b9bee67..128fa57 100644 --- a/crates/zesdex-utils/src/error.rs +++ b/crates/zesdex-utils/src/error.rs @@ -1,73 +1,47 @@ //! Shared error types for the zesdex codebase. //! //! Defines [`Error`], a unified error enum covering I/O, JSON, parse, -//! not-found, and invalid-input cases, plus a [`Result`] type alias. -//! Conversions from `std::io::Error` and `serde_json::Error` are provided -//! via `From` impls. - -use std::fmt; +//! not-found, conflict, invalid-id, and invalid-input cases, plus a +//! [`Result`] type alias. Conversions from `std::io::Error` and +//! `serde_json::Error` are provided via `From` impls. +//! +//! This type is used directly by repository traits across crates, +//! replacing per-crate `RepositoryError` duplications. /// Unified error type for the zesdex codebase. -#[derive(Debug)] +/// +/// Serves as the shared `RepositoryError` for all persistence layers. +#[derive(Debug, thiserror::Error)] pub enum Error { /// Wraps an I/O error. - Io(std::io::Error), + #[error("I/O error: {0}")] + Io(#[from] std::io::Error), /// Wraps a JSON serialization/deserialization error. - Serde(serde_json::Error), + #[error("serialization error: {0}")] + Serde(#[from] serde_json::Error), /// A generic parse failure with a message. + #[error("parse error: {0}")] Parse(String), /// A resource was not found. + #[error("not found: {0}")] NotFound(String), + /// A conflict occurred (e.g. duplicate entry). + #[error("conflict: {0}")] + Conflict(String), /// Invalid input was provided. + #[error("invalid input: {0}")] InvalidInput(String), + /// The supplied identifier is invalid (e.g. path traversal attempt). + #[error("invalid id: {0}")] + InvalidId(String), /// The session is locked and cannot be accessed. + #[error("session is locked")] SessionLocked, + /// An error that could not be cast to a specific variant. + #[error("{0}")] + Other(String), } -impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Self::Io(e) => write!(f, "I/O error: {e}"), - Self::Serde(e) => write!(f, "serialization error: {e}"), - Self::Parse(msg) => write!(f, "parse error: {msg}"), - Self::NotFound(resource) => write!(f, "not found: {resource}"), - Self::InvalidInput(msg) => write!(f, "invalid input: {msg}"), - Self::SessionLocked => write!(f, "session is locked"), - } - } -} - -impl std::error::Error for Error { - fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - match self { - Self::Io(e) => Some(e), - Self::Serde(e) => Some(e), - Self::Parse(_) | Self::NotFound(_) | Self::InvalidInput(_) | Self::SessionLocked => { - None - } - } - } -} - -// --------------------------------------------------------------------------- -// From conversions -// --------------------------------------------------------------------------- - -impl From for Error { - fn from(e: std::io::Error) -> Self { - Self::Io(e) - } -} - -impl From for Error { - fn from(e: serde_json::Error) -> Self { - Self::Serde(e) - } -} - -// Note: anyhow already provides `From for anyhow::Error` for all -// `E: std::error::Error + Send + Sync + 'static`, which our `Error` satisfies. - // --------------------------------------------------------------------------- // Type alias // --------------------------------------------------------------------------- @@ -76,7 +50,7 @@ impl From for Error { pub type Result = std::result::Result; // --------------------------------------------------------------------------- -// Additional impls +// Constructors // --------------------------------------------------------------------------- impl Error { @@ -94,4 +68,13 @@ impl Error { pub fn invalid_input(msg: impl Into) -> Self { Self::InvalidInput(msg.into()) } + + /// Convert an `anyhow::Error` to `zesdex_utils::Error` by attempting + /// downcast to known inner types. + pub fn from_anyhow(e: anyhow::Error) -> Self { + if let Some(ioe) = e.downcast_ref::() { + return Error::Io(std::io::Error::new(ioe.kind(), ioe.to_string())); + } + Error::Other(e.to_string()) + } } diff --git a/crates/zesdex-utils/src/lib.rs b/crates/zesdex-utils/src/lib.rs index 2590ebd..f508304 100644 --- a/crates/zesdex-utils/src/lib.rs +++ b/crates/zesdex-utils/src/lib.rs @@ -15,6 +15,8 @@ pub mod atomic_write; pub use atomic_write::write_json_atomic; +pub mod cast; +pub use cast::CastOr; pub mod clipboard; pub mod error; pub mod logger;