mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-20 08:01:35 +00:00
tighten security from vuln report (#9264)
* fix: harden app preview S3, WM_ env reservation, set_progress scoping * fixup: minimize #1 fix to single SQL-level filter * fixup: apply WM_* filter to HTTP agent-worker branch + normalize app S3 scope path
This commit is contained in:
+16
@@ -0,0 +1,16 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE v2_job_status s\n SET flow_status = JSONB_SET(s.flow_status, ARRAY['modules', s.flow_status->>'step', 'progress'], $1)\n FROM v2_job j\n WHERE s.id = $2 AND j.id = s.id AND j.workspace_id = $3",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Jsonb",
|
||||
"Uuid",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "d4af472614e1b6af8defa40a79d5f584c8b114ef24fdb67a5eefb40ce17acdef"
|
||||
}
|
||||
@@ -180,13 +180,22 @@ async fn set_job_progress(
|
||||
// If flow_job_id exists, than we should modify flow_status of corresponding module
|
||||
// Individual jobs and flows are handled differently
|
||||
if let Some(flow_job_id) = flow_job_id {
|
||||
// `v2_job_status` has no workspace_id column and the root db handle
|
||||
// bypasses RLS (the per-row policy on the table is also inert today —
|
||||
// `ENABLE ROW LEVEL SECURITY` was never set). Scope the update by
|
||||
// joining `v2_job` so the URL's workspace_id confines tampering to the
|
||||
// caller's workspace; without this, an authed member of any workspace
|
||||
// could overwrite the flow `progress` UI field of a flow in another
|
||||
// workspace given just the flow UUID.
|
||||
// TODO: Return error if trying to set completed job?
|
||||
sqlx::query!(
|
||||
"UPDATE v2_job_status
|
||||
SET flow_status = JSONB_SET(flow_status, ARRAY['modules', flow_status->>'step', 'progress'], $1)
|
||||
WHERE id = $2",
|
||||
"UPDATE v2_job_status s
|
||||
SET flow_status = JSONB_SET(s.flow_status, ARRAY['modules', s.flow_status->>'step', 'progress'], $1)
|
||||
FROM v2_job j
|
||||
WHERE s.id = $2 AND j.id = s.id AND j.workspace_id = $3",
|
||||
serde_json::json!(percent.clamp(0, 99)),
|
||||
flow_job_id
|
||||
flow_job_id,
|
||||
w_id,
|
||||
)
|
||||
.execute(&db)
|
||||
.await?;
|
||||
|
||||
@@ -2589,6 +2589,21 @@ async fn upload_s3_file_from_app(
|
||||
request: axum::extract::Request,
|
||||
) -> JsonResult<AppUploadFileResponse> {
|
||||
let policy = if let Some(file_key_regex) = query.force_viewer_file_key_regex {
|
||||
// `force_viewer_*` lets the caller supply a synthetic upload policy that
|
||||
// bypasses the deployed app's file_key_regex / resource restrictions.
|
||||
// It is intended for the app editor's preview path, so it must enforce
|
||||
// the same guards as `execute_component`'s preview mode (PR #9235):
|
||||
// authed caller, not an operator, and `apps:write` scope to make sure
|
||||
// an `apps:run`-scoped token cannot pick its own policy.
|
||||
let authed = opt_authed.as_ref().ok_or_else(|| {
|
||||
Error::NotAuthorized("App S3 preview upload requires authentication".to_string())
|
||||
})?;
|
||||
if authed.is_operator {
|
||||
return Err(Error::NotAuthorized(
|
||||
"Operators cannot run app S3 previews for security reasons".to_string(),
|
||||
));
|
||||
}
|
||||
check_scopes(authed, || format!("apps:write:{}", path.to_path()))?;
|
||||
Some(Policy {
|
||||
execution_mode: ExecutionMode::Viewer,
|
||||
triggerables: None,
|
||||
@@ -3100,6 +3115,20 @@ async fn download_s3_file_from_app(
|
||||
let force_viewer_allowed_s3_keys = if let Some(force_viewer_allowed_s3_keys) =
|
||||
query.force_viewer_allowed_s3_keys.clone()
|
||||
{
|
||||
// `force_viewer_allowed_s3_keys` lets the caller supply a synthetic
|
||||
// allowlist that bypasses the deployed app policy. Apply the same
|
||||
// preview-mode guard as `execute_component` (PR #9235): authed, not an
|
||||
// operator, `apps:write` scope so an `apps:run`-scoped token cannot
|
||||
// pick its own allowlist.
|
||||
let authed = opt_authed.as_ref().ok_or_else(|| {
|
||||
Error::NotAuthorized("App S3 preview download requires authentication".to_string())
|
||||
})?;
|
||||
if authed.is_operator {
|
||||
return Err(Error::NotAuthorized(
|
||||
"Operators cannot run app S3 previews for security reasons".to_string(),
|
||||
));
|
||||
}
|
||||
check_scopes(authed, || format!("apps:write:{}", path))?;
|
||||
Some(serde_json::from_str::<Vec<S3Key>>(&force_viewer_allowed_s3_keys).unwrap_or_default())
|
||||
} else {
|
||||
None
|
||||
|
||||
@@ -19,6 +19,7 @@ use serde::{Deserialize, Serialize};
|
||||
|
||||
lazy_static::lazy_static! {
|
||||
pub static ref SECRET_SALT: Option<String> = std::env::var("SECRET_SALT").ok();
|
||||
static ref RESERVED_WM_VAR_NAME: regex::Regex = regex::Regex::new(r"^WM_[A-Z_]+$").unwrap();
|
||||
}
|
||||
|
||||
#[derive(Serialize, Clone)]
|
||||
@@ -452,7 +453,7 @@ async fn get_cached_workspace_envs(conn: &Connection, w_id: &str) -> Vec<(String
|
||||
let custom_envs = if let Some(cached_envs) = cached_envs_o {
|
||||
cached_envs
|
||||
} else {
|
||||
let custom_envs = match conn {
|
||||
let raw_envs = match conn {
|
||||
Connection::Sql(db) => sqlx::query_as::<_, (String, String)>(
|
||||
"SELECT name, value FROM workspace_env WHERE workspace_id = $1",
|
||||
)
|
||||
@@ -465,6 +466,13 @@ async fn get_cached_workspace_envs(conn: &Connection, w_id: &str) -> Vec<(String
|
||||
.await
|
||||
.unwrap_or_default(),
|
||||
};
|
||||
// Applied here (not in the SQL branch alone) so agent workers going
|
||||
// through `Connection::Http` are covered too — drop any name that
|
||||
// would shadow a built-in `%%WM_*%%` contextual var.
|
||||
let custom_envs: Vec<(String, String)> = raw_envs
|
||||
.into_iter()
|
||||
.filter(|(name, _)| !RESERVED_WM_VAR_NAME.is_match(name))
|
||||
.collect();
|
||||
CUSTOM_ENVS_CACHE.insert(
|
||||
w_id.to_string(),
|
||||
(chrono::Utc::now().timestamp(), custom_envs.clone()),
|
||||
|
||||
Reference in New Issue
Block a user