mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-03 08:02:19 +00:00
fix: hold interpolated references and captures to the token path scopes (#11391)
* fix: hold interpolated references and captures to the token path scopes Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: let a resource read cover its own linked secret variable Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: resolve policy-granted app upload resources on the viewer's rls Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix: cover multi-secret linked variables and keep capture paths out of refusals Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
163a4ffa4e
commit
cedd6dc901
+23
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT path FROM capture WHERE id = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "path",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Int8",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "2da4b7cf1549494945c9761885dfd18ae25e0ee4810f1c9638e4400ab0c914a7"
|
||||
}
|
||||
@@ -566,6 +566,113 @@ async fn test_resource_value_cache_is_identity_scoped(db: Pool<Postgres>) -> any
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A token scoped to one resource must not read, through the `$var:`/`$res:` references it
|
||||
/// writes into that resource, what its scopes would refuse to read directly.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base"))]
|
||||
async fn test_interpolated_references_need_the_token_scopes(
|
||||
db: Pool<Postgres>,
|
||||
) -> anyhow::Result<()> {
|
||||
initialize_tracing().await;
|
||||
let server = ApiServer::start(db.clone()).await?;
|
||||
let port = server.addr.port();
|
||||
let base = format!("http://localhost:{port}/api/w/test-workspace");
|
||||
|
||||
let resp = authed(client().post(format!("{base}/variables/create")))
|
||||
.json(&json!({"path": "u/test-user/secret", "value": "CANARY", "is_secret": true, "description": ""}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201);
|
||||
let resp = authed(client().post(format!("{base}/resources/create")))
|
||||
.json(&json!({"path": "u/test-user/other", "value": {"pw": "OTHER"}, "resource_type": "object", "description": ""}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201);
|
||||
|
||||
let mint = |scopes: serde_json::Value| async move {
|
||||
let resp =
|
||||
authed(client().post(format!("http://localhost:{port}/api/users/tokens/create")))
|
||||
.json(
|
||||
&json!({"label": "scoped", "scopes": scopes, "workspace_id": "test-workspace"}),
|
||||
)
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 201);
|
||||
resp.text().await.unwrap()
|
||||
};
|
||||
let probe_only = mint(json!(["resources:write:u/test-user/probe"])).await;
|
||||
let with_var = mint(json!([
|
||||
"resources:read:u/test-user/probe",
|
||||
"variables:read:u/test-user/secret"
|
||||
]))
|
||||
.await;
|
||||
|
||||
let read_probe = |value: serde_json::Value, token: String| {
|
||||
let base = base.clone();
|
||||
let probe_only = probe_only.clone();
|
||||
async move {
|
||||
let resp = client()
|
||||
.post(format!("{base}/resources/create?update_if_exists=true"))
|
||||
.bearer_auth(&probe_only)
|
||||
.json(&json!({"path": "u/test-user/probe", "value": value, "resource_type": "object", "description": ""}))
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
assert_eq!(resp.status(), 201);
|
||||
let resp = client()
|
||||
.get(format!(
|
||||
"{base}/resources/get_value_interpolated/u/test-user/probe"
|
||||
))
|
||||
.bearer_auth(&token)
|
||||
.send()
|
||||
.await
|
||||
.unwrap();
|
||||
(resp.status().as_u16(), resp.text().await.unwrap())
|
||||
}
|
||||
};
|
||||
|
||||
for value in [
|
||||
json!({"v": "$var:u/test-user/secret"}),
|
||||
json!({"v": "$jsonvar:u/test-user/secret"}),
|
||||
json!({"v": ["$res:u/test-user/other"]}),
|
||||
] {
|
||||
let (status, body) = read_probe(value.clone(), probe_only.clone()).await;
|
||||
assert_eq!(status, 403, "{value}: {body}");
|
||||
assert!(
|
||||
!body.contains("CANARY") && !body.contains("OTHER"),
|
||||
"{body}"
|
||||
);
|
||||
}
|
||||
|
||||
let (status, body) =
|
||||
read_probe(json!({"v": "$var:u/test-user/secret"}), with_var.clone()).await;
|
||||
assert_eq!((status, body.as_str()), (200, r#"{"v":"CANARY"}"#));
|
||||
|
||||
// The resource's own linked secrets (at its path, or `<path>_<field>`) are part of it.
|
||||
for (path, value) in [
|
||||
("u/test-user/probe", "LINKED"),
|
||||
("u/test-user/probe_key", "KEY"),
|
||||
] {
|
||||
let resp = authed(client().post(format!("{base}/variables/create")))
|
||||
.json(&json!({"path": path, "value": value, "is_secret": true, "description": ""}))
|
||||
.send()
|
||||
.await?;
|
||||
assert_eq!(resp.status(), 201);
|
||||
}
|
||||
let (status, body) = read_probe(
|
||||
json!({"pw": "$var:u/test-user/probe", "key": "$var:u/test-user/probe_key"}),
|
||||
probe_only.clone(),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(status, 200, "{body}");
|
||||
assert_eq!(
|
||||
serde_json::from_str::<serde_json::Value>(&body)?,
|
||||
json!({"pw": "LINKED", "key": "KEY"})
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// A resource whose value contains a `$WM_*` contextual variable (e.g. `$WM_TOKEN`) is
|
||||
/// job-dependent and must NEVER be cached — even when first read WITHOUT a `job_id`, where the
|
||||
/// placeholder is left unresolved (caching that would serve a stale placeholder to a later job
|
||||
|
||||
@@ -4812,8 +4812,9 @@ async fn upload_s3_file_from_app(
|
||||
if let Some(ref s3_resource_path) = query.s3_resource_path {
|
||||
if matched_input.allow_user_resources {
|
||||
if let Some(authed) = opt_authed {
|
||||
let viewer = policy_granted_viewer(&authed);
|
||||
let db_with_opt_authed = DbWithOptAuthed::from_authed(
|
||||
&authed,
|
||||
&viewer,
|
||||
db.clone(),
|
||||
Some(user_db.clone()),
|
||||
);
|
||||
@@ -5780,6 +5781,14 @@ async fn exists_app(
|
||||
Ok(Json(exists))
|
||||
}
|
||||
|
||||
/// The viewer as whom a resource an app policy lets the viewer pick (`allow_user_resources`)
|
||||
/// is resolved. The policy, not the token, grants that resource, and app tokens are minted
|
||||
/// with a fixed scope set that never names variables, so the references inside it resolve on
|
||||
/// the viewer's RLS alone.
|
||||
fn policy_granted_viewer(authed: &ApiAuthed) -> ApiAuthed {
|
||||
ApiAuthed { scopes: None, ..authed.clone() }
|
||||
}
|
||||
|
||||
async fn build_args(
|
||||
policy: &Policy,
|
||||
PolicyTriggerableInputs {
|
||||
@@ -5806,8 +5815,9 @@ async fn build_args(
|
||||
key.and_then(|x| x.clone().strip_prefix("$res:").map(|x| x.to_string()))
|
||||
{
|
||||
if let Some(authed) = authed {
|
||||
let viewer = policy_granted_viewer(authed);
|
||||
let db_with_opt_authed =
|
||||
DbWithOptAuthed::from_authed(authed, db.clone(), Some(user_db.clone()));
|
||||
DbWithOptAuthed::from_authed(&viewer, db.clone(), Some(user_db.clone()));
|
||||
let res = get_resource_value_interpolated_internal(
|
||||
&db_with_opt_authed,
|
||||
w_id,
|
||||
|
||||
@@ -66,6 +66,7 @@ use crate::{
|
||||
args::RawWebhookArgs,
|
||||
db::{ApiAuthed, DB},
|
||||
users::fetch_api_authed,
|
||||
utils::check_scopes,
|
||||
};
|
||||
|
||||
use axum::{
|
||||
@@ -319,6 +320,7 @@ async fn get_configs(
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Path((w_id, runnable_kind, path)): Path<(String, RunnableKind, StripPath)>,
|
||||
) -> JsonResult<Vec<CaptureConfig>> {
|
||||
check_scopes(&authed, || format!("capture:read:{}", path.to_path()))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
let configs = sqlx::query_as!(
|
||||
@@ -557,6 +559,7 @@ async fn set_config(
|
||||
Path(w_id): Path<String>,
|
||||
Json(nc): Json<NewCaptureConfig>,
|
||||
) -> JsonResult<Option<TriggerConfig>> {
|
||||
check_scopes(&authed, || format!("capture:write:{}", nc.path))?;
|
||||
let nc = match nc.trigger_kind {
|
||||
TriggerKind::Postgres => {
|
||||
set_postgres_trigger_config(&w_id, authed.clone(), &db, user_db.clone(), nc).await?
|
||||
@@ -616,6 +619,7 @@ async fn ping_config(
|
||||
StripPath,
|
||||
)>,
|
||||
) -> Result<()> {
|
||||
check_scopes(&authed, || format!("capture:write:{}", path.to_path()))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
if matches!(trigger_kind, TriggerKind::Postgres) {
|
||||
windmill_common::datatable_roles::lock_datatable_streams(&mut *tx, false).await?;
|
||||
@@ -667,6 +671,7 @@ async fn list_captures(
|
||||
Path((w_id, runnable_kind, path)): Path<(String, RunnableKind, StripPath)>,
|
||||
Query(query): Query<ListCapturesQuery>,
|
||||
) -> JsonResult<Vec<Capture>> {
|
||||
check_scopes(&authed, || format!("capture:read:{}", path.to_path()))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
let (per_page, offset) = paginate(Pagination { page: query.page, per_page: query.per_page });
|
||||
@@ -713,12 +718,44 @@ async fn list_captures(
|
||||
Ok(Json(captures))
|
||||
}
|
||||
|
||||
/// A capture addressed by id carries no path in the route, so its path scope can only be
|
||||
/// checked against the row. Looked up under the caller's RLS so the check never reveals
|
||||
/// whether an id the caller cannot see exists, and the refusal omits the path, which the
|
||||
/// caller named only by id.
|
||||
async fn check_capture_id_scope(
|
||||
tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
|
||||
authed: &ApiAuthed,
|
||||
w_id: &str,
|
||||
id: i64,
|
||||
action: &str,
|
||||
) -> Result<()> {
|
||||
if authed.scopes.is_none() {
|
||||
return Ok(());
|
||||
}
|
||||
let path = sqlx::query_scalar!(
|
||||
"SELECT path FROM capture WHERE id = $1 AND workspace_id = $2",
|
||||
id,
|
||||
w_id,
|
||||
)
|
||||
.fetch_optional(&mut **tx)
|
||||
.await?;
|
||||
if let Some(path) = path {
|
||||
check_scopes(authed, || format!("capture:{action}:{path}")).map_err(|_| {
|
||||
windmill_common::error::Error::PermissionDenied(format!(
|
||||
"This token's capture:{action} scope does not cover capture {id}"
|
||||
))
|
||||
})?;
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn get_capture(
|
||||
authed: ApiAuthed,
|
||||
Extension(user_db): Extension<UserDB>,
|
||||
Path((w_id, id)): Path<(String, i64)>,
|
||||
) -> JsonResult<Capture> {
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
check_capture_id_scope(&mut tx, &authed, &w_id, id, "read").await?;
|
||||
|
||||
let capture = sqlx::query_as!(
|
||||
Capture,
|
||||
@@ -751,6 +788,7 @@ async fn delete_capture(
|
||||
Path((w_id, id)): Path<(String, i64)>,
|
||||
) -> Result<()> {
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
check_capture_id_scope(&mut tx, &authed, &w_id, id, "write").await?;
|
||||
// capture RLS only keys on the path segment, so without workspace_id an id from
|
||||
// another workspace whose path collides with the caller's grants would be deleted.
|
||||
sqlx::query!(
|
||||
@@ -781,8 +819,10 @@ async fn move_captures_and_configs(
|
||||
Path((w_id, runnable_kind, old_path)): Path<(String, RunnableKind, StripPath)>,
|
||||
Json(body): Json<MoveCapturesAndConfigsBody>,
|
||||
) -> Result<()> {
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
let old_path = old_path.to_path();
|
||||
check_scopes(&authed, || format!("capture:write:{}", old_path))?;
|
||||
check_scopes(&authed, || format!("capture:write:{}", body.new_path))?;
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
sqlx::query!(
|
||||
r#"
|
||||
|
||||
@@ -812,6 +812,7 @@ pub async fn get_resource_value_interpolated_internal<'a>(
|
||||
token_for_context,
|
||||
0,
|
||||
&used_job_context,
|
||||
Some(path),
|
||||
)
|
||||
.await?;
|
||||
if let Some(identity) = cache_identity.as_deref() {
|
||||
@@ -851,10 +852,29 @@ pub async fn transform_json_value(
|
||||
token,
|
||||
depth,
|
||||
&used_job_context,
|
||||
None,
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
/// RLS resolves a reference as the token's user, not as the token: a token scoped to one
|
||||
/// resource would otherwise read, through that resource, every variable or resource its
|
||||
/// user can. Job tokens carry no scopes, so what a runnable resolves is unaffected.
|
||||
/// `resource_path` is the resource being expanded: its own linked secrets
|
||||
/// ([`is_owned_linked_var`]) are covered by the read of it.
|
||||
fn check_interpolation_scope(
|
||||
db_with_opt_authed: &DbWithOptAuthed<'_, ApiAuthed>,
|
||||
domain: &str,
|
||||
path: &str,
|
||||
resource_path: Option<&str>,
|
||||
) -> Result<()> {
|
||||
match db_with_opt_authed.authed() {
|
||||
Some(_) if resource_path.is_some_and(|r| is_owned_linked_var(r, path)) => Ok(()),
|
||||
Some(authed) => check_scopes(authed, || format!("{domain}:read:{path}")),
|
||||
None => Ok(()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Like [`transform_json_value`], but records into `used_job_context` whether the value
|
||||
/// contains a `$WM_*` contextual variable (resolved from `job_id`/`token`). A value that did
|
||||
/// not is job-independent and safe to cache; one that did must not be cached or shared across
|
||||
@@ -868,6 +888,7 @@ pub async fn transform_json_value_tracked(
|
||||
token: Option<&str>,
|
||||
depth: u8,
|
||||
used_job_context: &std::sync::atomic::AtomicBool,
|
||||
resource_path: Option<&str>,
|
||||
) -> Result<Value> {
|
||||
if depth >= MAX_RESOURCE_INTERPOLATION_DEPTH {
|
||||
return Err(Error::internal_err(format!(
|
||||
@@ -877,6 +898,7 @@ pub async fn transform_json_value_tracked(
|
||||
match v {
|
||||
Value::String(y) if y.starts_with("$var:") => {
|
||||
let path = y.strip_prefix("$var:").unwrap();
|
||||
check_interpolation_scope(db_with_opt_authed, "variables", path, resource_path)?;
|
||||
|
||||
let v =
|
||||
crate::variables::get_value_internal(&db_with_opt_authed, workspace, path, false)
|
||||
@@ -885,6 +907,7 @@ pub async fn transform_json_value_tracked(
|
||||
}
|
||||
Value::String(y) if y.starts_with("$jsonvar:") => {
|
||||
let path = y.strip_prefix("$jsonvar:").unwrap();
|
||||
check_interpolation_scope(db_with_opt_authed, "variables", path, resource_path)?;
|
||||
|
||||
let v =
|
||||
crate::variables::get_value_internal(&db_with_opt_authed, workspace, path, false)
|
||||
@@ -900,6 +923,7 @@ pub async fn transform_json_value_tracked(
|
||||
"Invalid resource path: {path}"
|
||||
)));
|
||||
}
|
||||
check_interpolation_scope(db_with_opt_authed, "resources", path, None)?;
|
||||
let mut tx: Transaction<'_, Postgres> = db_with_opt_authed.begin().await?;
|
||||
let v = sqlx::query_scalar!(
|
||||
"SELECT value from resource WHERE path = $1 AND workspace_id = $2",
|
||||
@@ -919,6 +943,7 @@ pub async fn transform_json_value_tracked(
|
||||
token,
|
||||
depth + 1,
|
||||
used_job_context,
|
||||
Some(path),
|
||||
)
|
||||
.await
|
||||
} else {
|
||||
@@ -1018,6 +1043,7 @@ pub async fn transform_json_value_tracked(
|
||||
token,
|
||||
depth + 1,
|
||||
used_job_context,
|
||||
resource_path,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
@@ -1042,6 +1068,7 @@ pub async fn transform_json_value_tracked(
|
||||
token,
|
||||
depth + 1,
|
||||
used_job_context,
|
||||
resource_path,
|
||||
)
|
||||
.await?;
|
||||
m.insert(a.clone(), v);
|
||||
|
||||
Reference in New Issue
Block a user