forked from wrenn/wrenn
fix: use RwLock for envd Defaults to fix silent mutation loss
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.
This commit is contained in:
@ -1,21 +1,36 @@
|
|||||||
use dashmap::DashMap;
|
use dashmap::DashMap;
|
||||||
use std::sync::Arc;
|
use std::sync::{Arc, RwLock};
|
||||||
|
|
||||||
#[derive(Clone)]
|
|
||||||
pub struct Defaults {
|
pub struct Defaults {
|
||||||
pub env_vars: Arc<DashMap<String, String>>,
|
pub env_vars: Arc<DashMap<String, String>>,
|
||||||
pub user: String,
|
user: RwLock<String>,
|
||||||
pub workdir: Option<String>,
|
workdir: RwLock<Option<String>>,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Defaults {
|
impl Defaults {
|
||||||
pub fn new(user: &str) -> Self {
|
pub fn new(user: &str) -> Self {
|
||||||
Self {
|
Self {
|
||||||
env_vars: Arc::new(DashMap::new()),
|
env_vars: Arc::new(DashMap::new()),
|
||||||
user: user.to_string(),
|
user: RwLock::new(user.to_string()),
|
||||||
workdir: None,
|
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<String> {
|
||||||
|
self.workdir.read().unwrap().clone()
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn set_workdir(&self, workdir: Option<String>) {
|
||||||
|
*self.workdir.write().unwrap() = workdir;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn resolve_default_workdir(workdir: &str, default_workdir: Option<&str>) -> String {
|
pub fn resolve_default_workdir(workdir: &str, default_workdir: Option<&str>) -> String {
|
||||||
|
|||||||
@ -71,9 +71,10 @@ pub async fn get_files(
|
|||||||
let path_str = params.path.as_deref().unwrap_or("");
|
let path_str = params.path.as_deref().unwrap_or("");
|
||||||
let header_token = extract_header_token(&req);
|
let header_token = extract_header_token(&req);
|
||||||
|
|
||||||
|
let default_user = state.defaults.user();
|
||||||
let username = match execcontext::resolve_default_username(
|
let username = match execcontext::resolve_default_username(
|
||||||
params.username.as_deref(),
|
params.username.as_deref(),
|
||||||
&state.defaults.user,
|
&default_user,
|
||||||
) {
|
) {
|
||||||
Ok(u) => u.to_string(),
|
Ok(u) => u.to_string(),
|
||||||
Err(e) => return json_error(StatusCode::BAD_REQUEST, e),
|
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 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,
|
Ok(p) => p,
|
||||||
Err(e) => return json_error(StatusCode::BAD_REQUEST, &e),
|
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 path_str = params.path.as_deref().unwrap_or("");
|
||||||
let header_token = extract_header_token(&req);
|
let header_token = extract_header_token(&req);
|
||||||
|
|
||||||
|
let default_user = state.defaults.user();
|
||||||
let username = match execcontext::resolve_default_username(
|
let username = match execcontext::resolve_default_username(
|
||||||
params.username.as_deref(),
|
params.username.as_deref(),
|
||||||
&state.defaults.user,
|
&default_user,
|
||||||
) {
|
) {
|
||||||
Ok(u) => u.to_string(),
|
Ok(u) => u.to_string(),
|
||||||
Err(e) => return json_error(StatusCode::BAD_REQUEST, e),
|
Err(e) => return json_error(StatusCode::BAD_REQUEST, e),
|
||||||
@ -266,6 +269,7 @@ pub async fn post_files(
|
|||||||
};
|
};
|
||||||
|
|
||||||
let mut uploaded: Vec<EntryInfo> = Vec::new();
|
let mut uploaded: Vec<EntryInfo> = Vec::new();
|
||||||
|
let default_workdir = state.defaults.workdir();
|
||||||
|
|
||||||
while let Ok(Some(field)) = multipart.next_field().await {
|
while let Ok(Some(field)) = multipart.next_field().await {
|
||||||
let field_name = field.name().unwrap_or("").to_string();
|
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() {
|
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,
|
Ok(p) => p,
|
||||||
Err(e) => return json_error(StatusCode::BAD_REQUEST, &e),
|
Err(e) => return json_error(StatusCode::BAD_REQUEST, &e),
|
||||||
}
|
}
|
||||||
@ -283,7 +287,7 @@ pub async fn post_files(
|
|||||||
.file_name()
|
.file_name()
|
||||||
.unwrap_or("upload")
|
.unwrap_or("upload")
|
||||||
.to_string();
|
.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,
|
Ok(p) => p,
|
||||||
Err(e) => return json_error(StatusCode::BAD_REQUEST, &e),
|
Err(e) => return json_error(StatusCode::BAD_REQUEST, &e),
|
||||||
}
|
}
|
||||||
|
|||||||
@ -78,11 +78,15 @@ pub async fn post_init(
|
|||||||
if let Some(ref user) = init_req.default_user {
|
if let Some(ref user) = init_req.default_user {
|
||||||
if !user.is_empty() {
|
if !user.is_empty() {
|
||||||
tracing::debug!(user = %user, "setting default user");
|
tracing::debug!(user = %user, "setting default user");
|
||||||
let mut defaults = state.defaults.clone();
|
state.defaults.set_user(user.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.
|
// 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()));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -196,7 +196,8 @@ fn spawn_initial_command(cmd: &str, state: &AppState) {
|
|||||||
use crate::rpc::process_handler;
|
use crate::rpc::process_handler;
|
||||||
use std::collections::HashMap;
|
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,
|
Ok(u) => u,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
tracing::error!(error = %e, "cmd: failed to lookup user");
|
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 home = user.dir.to_string_lossy().to_string();
|
||||||
let cwd = state
|
let default_workdir = state.defaults.workdir();
|
||||||
.defaults
|
let cwd = default_workdir
|
||||||
.workdir
|
|
||||||
.as_deref()
|
.as_deref()
|
||||||
.unwrap_or(&home);
|
.unwrap_or(&home);
|
||||||
|
|
||||||
|
|||||||
@ -31,15 +31,15 @@ impl FilesystemServiceImpl {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn resolve_path(&self, path: &str, ctx: &Context) -> Result<String, ConnectError> {
|
fn resolve_path(&self, path: &str, ctx: &Context) -> Result<String, ConnectError> {
|
||||||
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| {
|
let user = lookup_user(&username).map_err(|e| {
|
||||||
ConnectError::new(ErrorCode::Unauthenticated, format!("invalid user: {e}"))
|
ConnectError::new(ErrorCode::Unauthenticated, format!("invalid user: {e}"))
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let home_dir = user.dir.to_string_lossy().to_string();
|
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))
|
.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 =
|
let user =
|
||||||
lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?;
|
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 source = self.resolve_path(request.source, &ctx)?;
|
||||||
let destination = self.resolve_path(request.destination, &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 =
|
let user =
|
||||||
lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?;
|
lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?;
|
||||||
|
|
||||||
|
|||||||
@ -71,7 +71,7 @@ impl ProcessServiceImpl {
|
|||||||
ConnectError::new(ErrorCode::InvalidArgument, "process config required")
|
ConnectError::new(ErrorCode::InvalidArgument, "process config required")
|
||||||
})?;
|
})?;
|
||||||
|
|
||||||
let username = self.state.defaults.user.clone();
|
let username = self.state.defaults.user();
|
||||||
let user =
|
let user =
|
||||||
lookup_user(&username).map_err(|e| ConnectError::new(ErrorCode::Internal, e))?;
|
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 home_dir = user.dir.to_string_lossy().to_string();
|
||||||
let cwd_str: &str = proc_config.cwd.unwrap_or("");
|
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))?;
|
.map_err(|e| ConnectError::new(ErrorCode::InvalidArgument, e))?;
|
||||||
|
|
||||||
let effective_cwd = if cwd.is_empty() { "/" } else { &cwd };
|
let effective_cwd = if cwd.is_empty() { "/" } else { &cwd };
|
||||||
|
|||||||
Reference in New Issue
Block a user