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:
Ruben Fiszel
2026-05-20 14:02:50 +00:00
committed by GitHub
parent bb78b1c06d
commit 22ec4da5f0
4 changed files with 67 additions and 5 deletions
@@ -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"
}
+13 -4
View File
@@ -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?;
+29
View File
@@ -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
+9 -1
View File
@@ -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()),