From 2af8412cdcc3972f8e589a83685d660d1710e3ad Mon Sep 17 00:00:00 2001 From: pptx704 Date: Sat, 9 May 2026 15:28:09 +0600 Subject: [PATCH] fix: use RwLock for envd Defaults to fix silent mutation loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /init handler's default_user mutation cloned the Defaults struct, mutated the clone, then dropped it — the actual state was never updated. This caused processes to always run as "root" regardless of the user set via POST /init. Additionally, default_workdir was accepted in the init request but never applied. Wrap user and workdir fields in RwLock with accessor methods so mutations propagate correctly through the shared AppState. --- envd-rs/src/execcontext.rs | 27 +++++++++++++++++++++------ envd-rs/src/http/files.rs | 14 +++++++++----- envd-rs/src/http/init.rs | 14 +++++++++----- envd-rs/src/main.rs | 8 ++++---- envd-rs/src/rpc/filesystem_service.rs | 10 +++++----- envd-rs/src/rpc/process_service.rs | 5 +++-- 6 files changed, 51 insertions(+), 27 deletions(-) diff --git a/envd-rs/src/execcontext.rs b/envd-rs/src/execcontext.rs index d0f53eb..e3baa43 100644 --- a/envd-rs/src/execcontext.rs +++ b/envd-rs/src/execcontext.rs @@ -1,21 +1,36 @@ use dashmap::DashMap; -use std::sync::Arc; +use std::sync::{Arc, RwLock}; -#[derive(Clone)] pub struct Defaults { pub env_vars: Arc>, - pub user: String, - pub workdir: Option, + user: RwLock, + workdir: RwLock>, } impl Defaults { pub fn new(user: &str) -> Self { Self { env_vars: Arc::new(DashMap::new()), - user: user.to_string(), - workdir: None, + user: RwLock::new(user.to_string()), + workdir: RwLock::new(None), } } + + pub fn user(&self) -> String { + self.user.read().unwrap().clone() + } + + pub fn set_user(&self, user: String) { + *self.user.write().unwrap() = user; + } + + pub fn workdir(&self) -> Option { + self.workdir.read().unwrap().clone() + } + + pub fn set_workdir(&self, workdir: Option) { + *self.workdir.write().unwrap() = workdir; + } } pub fn resolve_default_workdir(workdir: &str, default_workdir: Option<&str>) -> String { diff --git a/envd-rs/src/http/files.rs b/envd-rs/src/http/files.rs index dfe1e54..e0d7ab2 100644 --- a/envd-rs/src/http/files.rs +++ b/envd-rs/src/http/files.rs @@ -71,9 +71,10 @@ pub async fn get_files( let path_str = params.path.as_deref().unwrap_or(""); let header_token = extract_header_token(&req); + let default_user = state.defaults.user(); let username = match execcontext::resolve_default_username( params.username.as_deref(), - &state.defaults.user, + &default_user, ) { Ok(u) => u.to_string(), Err(e) => return json_error(StatusCode::BAD_REQUEST, e), @@ -96,7 +97,8 @@ pub async fn get_files( }; let home_dir = user.dir.to_string_lossy().to_string(); - let resolved = match expand_and_resolve(path_str, &home_dir, state.defaults.workdir.as_deref()) + let default_workdir = state.defaults.workdir(); + let resolved = match expand_and_resolve(path_str, &home_dir, default_workdir.as_deref()) { Ok(p) => p, Err(e) => return json_error(StatusCode::BAD_REQUEST, &e), @@ -222,9 +224,10 @@ pub async fn post_files( let path_str = params.path.as_deref().unwrap_or(""); let header_token = extract_header_token(&req); + let default_user = state.defaults.user(); let username = match execcontext::resolve_default_username( params.username.as_deref(), - &state.defaults.user, + &default_user, ) { Ok(u) => u.to_string(), Err(e) => return json_error(StatusCode::BAD_REQUEST, e), @@ -266,6 +269,7 @@ pub async fn post_files( }; let mut uploaded: Vec = Vec::new(); + let default_workdir = state.defaults.workdir(); while let Ok(Some(field)) = multipart.next_field().await { let field_name = field.name().unwrap_or("").to_string(); @@ -274,7 +278,7 @@ pub async fn post_files( } let file_path = if !path_str.is_empty() { - match expand_and_resolve(path_str, &home_dir, state.defaults.workdir.as_deref()) { + match expand_and_resolve(path_str, &home_dir, default_workdir.as_deref()) { Ok(p) => p, Err(e) => return json_error(StatusCode::BAD_REQUEST, &e), } @@ -283,7 +287,7 @@ pub async fn post_files( .file_name() .unwrap_or("upload") .to_string(); - match expand_and_resolve(&fname, &home_dir, state.defaults.workdir.as_deref()) { + match expand_and_resolve(&fname, &home_dir, default_workdir.as_deref()) { Ok(p) => p, Err(e) => return json_error(StatusCode::BAD_REQUEST, &e), } diff --git a/envd-rs/src/http/init.rs b/envd-rs/src/http/init.rs index bc78c8c..840cab0 100644 --- a/envd-rs/src/http/init.rs +++ b/envd-rs/src/http/init.rs @@ -78,11 +78,15 @@ pub async fn post_init( if let Some(ref user) = init_req.default_user { if !user.is_empty() { tracing::debug!(user = %user, "setting default user"); - let mut defaults = state.defaults.clone(); - defaults.user = user.clone(); - // Note: In Rust we'd need interior mutability for this. - // For now, env_vars (DashMap) handles concurrent access. - // User/workdir mutation deferred to full state refactor. + state.defaults.set_user(user.clone()); + } + } + + // Set default workdir + if let Some(ref workdir) = init_req.default_workdir { + if !workdir.is_empty() { + tracing::debug!(workdir = %workdir, "setting default workdir"); + state.defaults.set_workdir(Some(workdir.clone())); } } diff --git a/envd-rs/src/main.rs b/envd-rs/src/main.rs index 3176a28..6a7bf42 100644 --- a/envd-rs/src/main.rs +++ b/envd-rs/src/main.rs @@ -196,7 +196,8 @@ fn spawn_initial_command(cmd: &str, state: &AppState) { use crate::rpc::process_handler; use std::collections::HashMap; - let user = match lookup_user(&state.defaults.user) { + let default_user = state.defaults.user(); + let user = match lookup_user(&default_user) { Ok(u) => u, Err(e) => { tracing::error!(error = %e, "cmd: failed to lookup user"); @@ -205,9 +206,8 @@ fn spawn_initial_command(cmd: &str, state: &AppState) { }; let home = user.dir.to_string_lossy().to_string(); - let cwd = state - .defaults - .workdir + let default_workdir = state.defaults.workdir(); + let cwd = default_workdir .as_deref() .unwrap_or(&home); diff --git a/envd-rs/src/rpc/filesystem_service.rs b/envd-rs/src/rpc/filesystem_service.rs index 1c73e93..58ee971 100644 --- a/envd-rs/src/rpc/filesystem_service.rs +++ b/envd-rs/src/rpc/filesystem_service.rs @@ -31,15 +31,15 @@ impl FilesystemServiceImpl { } fn resolve_path(&self, path: &str, ctx: &Context) -> Result { - let username = extract_username(ctx).unwrap_or_else(|| self.state.defaults.user.clone()); + let username = extract_username(ctx).unwrap_or_else(|| self.state.defaults.user()); let user = lookup_user(&username).map_err(|e| { ConnectError::new(ErrorCode::Unauthenticated, format!("invalid user: {e}")) })?; let home_dir = user.dir.to_string_lossy().to_string(); - let default_workdir = self.state.defaults.workdir.as_deref(); + let default_workdir = self.state.defaults.workdir(); - expand_and_resolve(path, &home_dir, default_workdir) + expand_and_resolve(path, &home_dir, default_workdir.as_deref()) .map_err(|e| ConnectError::new(ErrorCode::InvalidArgument, e)) } } @@ -97,7 +97,7 @@ impl Filesystem for FilesystemServiceImpl { } } - let username = extract_username(&ctx).unwrap_or_else(|| self.state.defaults.user.clone()); + let username = extract_username(&ctx).unwrap_or_else(|| self.state.defaults.user()); let user = lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?; @@ -122,7 +122,7 @@ impl Filesystem for FilesystemServiceImpl { let source = self.resolve_path(request.source, &ctx)?; let destination = self.resolve_path(request.destination, &ctx)?; - let username = extract_username(&ctx).unwrap_or_else(|| self.state.defaults.user.clone()); + let username = extract_username(&ctx).unwrap_or_else(|| self.state.defaults.user()); let user = lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?; diff --git a/envd-rs/src/rpc/process_service.rs b/envd-rs/src/rpc/process_service.rs index 92738b5..7e14b85 100644 --- a/envd-rs/src/rpc/process_service.rs +++ b/envd-rs/src/rpc/process_service.rs @@ -71,7 +71,7 @@ impl ProcessServiceImpl { ConnectError::new(ErrorCode::InvalidArgument, "process config required") })?; - let username = self.state.defaults.user.clone(); + let username = self.state.defaults.user(); let user = lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?; @@ -85,7 +85,8 @@ impl ProcessServiceImpl { let home_dir = user.dir.to_string_lossy().to_string(); let cwd_str: &str = proc_config.cwd.unwrap_or(""); - let cwd = expand_and_resolve(cwd_str, &home_dir, self.state.defaults.workdir.as_deref()) + let default_workdir = self.state.defaults.workdir(); + let cwd = expand_and_resolve(cwd_str, &home_dir, default_workdir.as_deref()) .map_err(|e| ConnectError::new(ErrorCode::InvalidArgument, e))?; let effective_cwd = if cwd.is_empty() { "/" } else { &cwd };