mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 08:02:18 +00:00
fix: re-enforce scoped API token boundaries across handlers (#9712)
* fix: re-enforce per-path token scope on store rename, delete and interpolation Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: enforce token scope on workspace export and resume-url minting Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: enforce per-item and runnable scope on trigger create paths Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: enforce app write scope before persistence and on rename Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: enforce scope containment on mcp oauth approval Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: scope mcp endpoint-proxy jwt to the proxied route Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: treat resource-linked variables and resources as covered by resource scope Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: only require variables:read for plaintext-secret workspace export Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: handle singlestepflow resume, reject empty mcp grant, scope var-skipped tarball Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
6e96f90065
commit
e19594df2a
+35
@@ -0,0 +1,35 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT kind::text as \"kind!\", parent_job, runnable_path\n FROM v2_job WHERE id = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "kind!",
|
||||
"type_info": "Text"
|
||||
},
|
||||
{
|
||||
"ordinal": 1,
|
||||
"name": "parent_job",
|
||||
"type_info": "Uuid"
|
||||
},
|
||||
{
|
||||
"ordinal": 2,
|
||||
"name": "runnable_path",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Uuid",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
null,
|
||||
true,
|
||||
true
|
||||
]
|
||||
},
|
||||
"hash": "19f1cb1c7a1974920549917a6392ff0d56f31ca5662062f4a71fed0ccf859cc4"
|
||||
}
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "SELECT runnable_path FROM v2_job WHERE id = $1 AND workspace_id = $2",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"ordinal": 0,
|
||||
"name": "runnable_path",
|
||||
"type_info": "Varchar"
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Uuid",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": [
|
||||
true
|
||||
]
|
||||
},
|
||||
"hash": "c167db39eeed526449dc064eaeb26e648aa9455cb81bab86fd106f76537701ba"
|
||||
}
|
||||
@@ -191,7 +191,11 @@ impl AuthCache {
|
||||
is_operator: claims.is_operator,
|
||||
groups: claims.groups,
|
||||
folders: claims.folders,
|
||||
scopes: None,
|
||||
// Honor the scopes embedded in the JWT (mirrors the EE
|
||||
// jwt_ext_ branch). The route middleware only enforces
|
||||
// scopes when Some, so a None-scoped JWT (e.g. the job
|
||||
// WM_TOKEN) keeps full user privileges as before.
|
||||
scopes: claims.scopes,
|
||||
username_override,
|
||||
token_prefix: claims.audit_span,
|
||||
read_only: false,
|
||||
|
||||
@@ -699,6 +699,23 @@ pub fn check_read_only_for_route(route_path: &str, http_method: &str) -> Result<
|
||||
}
|
||||
}
|
||||
|
||||
/// The minimal scope string that grants access to exactly `{method} {path}`, as
|
||||
/// `check_route_access` would require it. Used to mint a least-privilege JWT for
|
||||
/// a single proxied request (the MCP endpoint proxy), so the minted token can do
|
||||
/// only that one operation rather than acting as a blank check.
|
||||
///
|
||||
/// `path` is the request path (e.g. `/api/w/{workspace}/variables/get/...`).
|
||||
/// Returns `None` if the route's domain can't be determined — the caller should
|
||||
/// then fail closed.
|
||||
pub fn scope_for_route(method: &str, path: &str) -> Option<String> {
|
||||
let action = map_http_method_to_action(method, path);
|
||||
let (domain, kind, _suffix) = extract_domain_from_route(path).ok()?;
|
||||
Some(match (domain, action, kind) {
|
||||
(ScopeDomain::Jobs, ScopeAction::Run, Some(kind)) => format!("jobs:run:{}", kind),
|
||||
(domain, action, _) => format!("{}:{}", domain.as_str(), action.as_str()),
|
||||
})
|
||||
}
|
||||
|
||||
/// Helper function to check if scopes allow access to a route
|
||||
pub fn check_scopes_for_route(
|
||||
token_scopes: Option<&[String]>,
|
||||
@@ -1083,4 +1100,38 @@ mod tests {
|
||||
let scopes = vec!["jobs:read".to_string(), "mcp:all".to_string()];
|
||||
assert!(check_route_access(&scopes, "/api/w/test_workspace/mcp/something", "GET").is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_scope_for_route() {
|
||||
// The minted scope must be exactly what check_route_access requires for
|
||||
// the same route, so a JWT carrying it passes for that one route only.
|
||||
assert_eq!(
|
||||
scope_for_route("GET", "/api/w/ws/variables/get/u/x/y").as_deref(),
|
||||
Some("variables:read")
|
||||
);
|
||||
assert_eq!(
|
||||
scope_for_route("POST", "/api/w/ws/variables/create").as_deref(),
|
||||
Some("variables:write")
|
||||
);
|
||||
assert_eq!(
|
||||
scope_for_route("DELETE", "/api/w/ws/resources/delete/u/x/y").as_deref(),
|
||||
Some("resources:write")
|
||||
);
|
||||
// jobs run paths carry the runnable kind.
|
||||
assert_eq!(
|
||||
scope_for_route("POST", "/api/w/ws/jobs/run/p/u/x/y").as_deref(),
|
||||
Some("jobs:run:scripts")
|
||||
);
|
||||
assert_eq!(
|
||||
scope_for_route("POST", "/api/w/ws/jobs/run/f/u/x/y").as_deref(),
|
||||
Some("jobs:run:flows")
|
||||
);
|
||||
|
||||
// The minted scope actually satisfies the route check it targets.
|
||||
let s = scope_for_route("POST", "/api/w/ws/variables/create").unwrap();
|
||||
assert!(check_route_access(&[s], "/api/w/ws/variables/create", "POST").is_ok());
|
||||
|
||||
// Unknown route -> None so the caller fails closed.
|
||||
assert!(scope_for_route("GET", "/healthz").is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1245,8 +1245,6 @@ async fn create_app_raw<'a>(
|
||||
)
|
||||
.await?;
|
||||
|
||||
check_scopes(&authed, || format!("apps:write:{}", path))?;
|
||||
|
||||
webhook.send_message(
|
||||
w_id.clone(),
|
||||
WebhookMessage::CreateApp { workspace: w_id, path: path.clone() },
|
||||
@@ -1290,7 +1288,6 @@ async fn create_app(
|
||||
));
|
||||
}
|
||||
let path = app.path.clone();
|
||||
check_scopes(&authed, || format!("apps:write:{}", &path))?;
|
||||
|
||||
if let RuleCheckResult::Blocked(msg) = check_deploy_rules(
|
||||
&w_id,
|
||||
@@ -1304,6 +1301,7 @@ async fn create_app(
|
||||
return Err(Error::PermissionDenied(msg));
|
||||
}
|
||||
|
||||
// scope is enforced inside create_app_internal, before any persistence.
|
||||
let (new_tx, _path, _id) = create_app_internal(authed, db, user_db, &w_id, false, app).await?;
|
||||
|
||||
new_tx.commit().await?;
|
||||
@@ -1350,6 +1348,10 @@ async fn create_app_internal<'a>(
|
||||
raw_app: bool,
|
||||
mut app: CreateApp,
|
||||
) -> Result<(sqlx::Transaction<'a, sqlx::Postgres>, String, i64)> {
|
||||
// Enforce scope before any persistence: the raw-app create path commits
|
||||
// inside process_app_multipart!, so checking after this call would leave a
|
||||
// denied app committed in the DB.
|
||||
check_scopes(&authed, || format!("apps:write:{}", &app.path))?;
|
||||
if *CLOUD_HOSTED {
|
||||
let nb_apps =
|
||||
sqlx::query_scalar!("SELECT COUNT(*) FROM app WHERE workspace_id = $1", &w_id)
|
||||
@@ -1891,6 +1893,13 @@ async fn update_app_internal<'a>(
|
||||
ns: EditApp,
|
||||
) -> Result<(sqlx::Transaction<'a, sqlx::Postgres>, String, i64)> {
|
||||
use sql_builder::prelude::*;
|
||||
|
||||
// A rename moves the app to ns.path, so the destination must also be within
|
||||
// the token's write scope, not just the source path.
|
||||
if let Some(npath) = ns.path.as_deref() {
|
||||
check_scopes(&authed, || format!("apps:write:{}", npath))?;
|
||||
}
|
||||
|
||||
let mut tx = user_db.clone().begin(&authed).await?;
|
||||
|
||||
let mut preserved_on_behalf_of: Option<String> = None;
|
||||
|
||||
@@ -4022,11 +4022,17 @@ fn conditionally_require_authed_user(
|
||||
}
|
||||
|
||||
pub async fn create_job_signature(
|
||||
_authed: ApiAuthed,
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
Path((w_id, job_id, resume_id)): Path<(String, Uuid, u32)>,
|
||||
Query(approver): Query<QueryApprover>,
|
||||
) -> error::Result<String> {
|
||||
// The HMAC is treated as full authority by the resume endpoints, so minting
|
||||
// it requires run scope on the suspended job's flow — not merely any
|
||||
// jobs:run scope. No-op for unscoped tokens (incl. the in-flow substep token
|
||||
// used by wmill.get_resume_urls()).
|
||||
let flow_path = resume_target_flow_path(&db, &w_id, job_id).await?;
|
||||
check_scopes(&authed, || format!("jobs:run:flows:{}", flow_path))?;
|
||||
let key = get_workspace_key(&w_id, &db).await?;
|
||||
create_signature(key, job_id, resume_id, approver.approver)
|
||||
}
|
||||
@@ -4109,11 +4115,17 @@ fn build_resume_url(
|
||||
}
|
||||
|
||||
pub async fn get_resume_urls(
|
||||
_authed: ApiAuthed,
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
Path((w_id, job_id, resume_id)): Path<(String, Uuid, u32)>,
|
||||
Query(approver): Query<QueryApprover>,
|
||||
) -> error::JsonResult<ResumeUrls> {
|
||||
// These URLs embed a resume signature (full resume capability), so a scoped
|
||||
// token must hold run scope on the suspended job's flow. No-op for unscoped
|
||||
// tokens (incl. the in-flow substep token). Trusted internal callers use
|
||||
// get_resume_urls_internal directly and are unaffected.
|
||||
let flow_path = resume_target_flow_path(&db, &w_id, job_id).await?;
|
||||
check_scopes(&authed, || format!("jobs:run:flows:{}", flow_path))?;
|
||||
get_resume_urls_internal(
|
||||
Extension(db),
|
||||
Path((w_id, job_id, resume_id)),
|
||||
@@ -4180,6 +4192,46 @@ pub async fn get_resume_urls_internal(
|
||||
Ok(Json(res))
|
||||
}
|
||||
|
||||
/// Resolve the runnable path of the flow a (possibly step) job belongs to, used
|
||||
/// to scope-check resume-signature minting against `jobs:run:flows:<path>`.
|
||||
/// Returns an empty string when the path can't be resolved (e.g. previews or an
|
||||
/// unknown job); an empty path only matters for path-restricted tokens, which
|
||||
/// would not be running such a flow. Never hard-fails, so it can't break resume
|
||||
/// for unscoped tokens (the in-flow `get_resume_urls()` path).
|
||||
async fn resume_target_flow_path(db: &DB, w_id: &str, job_id: Uuid) -> error::Result<String> {
|
||||
let job = sqlx::query!(
|
||||
r#"SELECT kind::text as "kind!", parent_job, runnable_path
|
||||
FROM v2_job WHERE id = $1 AND workspace_id = $2"#,
|
||||
job_id,
|
||||
w_id
|
||||
)
|
||||
.fetch_optional(db)
|
||||
.await?;
|
||||
let Some(job) = job else {
|
||||
return Ok(String::new());
|
||||
};
|
||||
// All flow kinds: the job itself is the flow whose path scopes the resume.
|
||||
if matches!(
|
||||
job.kind.as_str(),
|
||||
"flow" | "flowpreview" | "flownode" | "singlestepflow"
|
||||
) {
|
||||
return Ok(job.runnable_path.unwrap_or_default());
|
||||
}
|
||||
// Otherwise it's a step; its parent is the flow.
|
||||
if let Some(parent) = job.parent_job {
|
||||
return Ok(sqlx::query_scalar!(
|
||||
"SELECT runnable_path FROM v2_job WHERE id = $1 AND workspace_id = $2",
|
||||
parent,
|
||||
w_id
|
||||
)
|
||||
.fetch_optional(db)
|
||||
.await?
|
||||
.flatten()
|
||||
.unwrap_or_default());
|
||||
}
|
||||
Ok(job.runnable_path.unwrap_or_default())
|
||||
}
|
||||
|
||||
/// Get the flow ID for a job. If the job is a flow, returns the job_id.
|
||||
/// If the job is a step in a flow, returns the parent flow ID.
|
||||
async fn get_flow_id_for_job(db: &DB, job_id: Uuid) -> error::Result<Uuid> {
|
||||
@@ -9488,16 +9540,31 @@ mod approval_view_gate_tests {
|
||||
fn anonymous_cannot_view_when_auth_required() {
|
||||
// The regression: an unauthenticated holder of the approval token must see nothing.
|
||||
let c = Some(conds(true, vec![]));
|
||||
assert!(!can_view(&None, &c, Some("f/team/flow"), "trigger@example.com"));
|
||||
assert!(!can_view(
|
||||
&None,
|
||||
&c,
|
||||
Some("f/team/flow"),
|
||||
"trigger@example.com"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn anonymous_can_view_when_no_auth_required() {
|
||||
// Unchanged behaviour: token alone is sufficient when auth isn't required.
|
||||
let c = Some(conds(false, vec![]));
|
||||
assert!(can_view(&None, &c, Some("f/team/flow"), "trigger@example.com"));
|
||||
assert!(can_view(
|
||||
&None,
|
||||
&c,
|
||||
Some("f/team/flow"),
|
||||
"trigger@example.com"
|
||||
));
|
||||
// No approval conditions at all also allows token-only view.
|
||||
assert!(can_view(&None, &None, Some("f/team/flow"), "trigger@example.com"));
|
||||
assert!(can_view(
|
||||
&None,
|
||||
&None,
|
||||
Some("f/team/flow"),
|
||||
"trigger@example.com"
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -9522,7 +9589,17 @@ mod approval_view_gate_tests {
|
||||
let member = Some(authed("carol", false, vec!["approvers".to_string()]));
|
||||
let outsider = Some(authed("dave", false, vec!["other".to_string()]));
|
||||
// Use a non-owned folder path so ownership doesn't short-circuit the check.
|
||||
assert!(can_view(&member, &c, Some("f/team/flow"), "trigger@example.com"));
|
||||
assert!(!can_view(&outsider, &c, Some("f/team/flow"), "trigger@example.com"));
|
||||
assert!(can_view(
|
||||
&member,
|
||||
&c,
|
||||
Some("f/team/flow"),
|
||||
"trigger@example.com"
|
||||
));
|
||||
assert!(!can_view(
|
||||
&outsider,
|
||||
&c,
|
||||
Some("f/team/flow"),
|
||||
"trigger@example.com"
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ use windmill_common::{
|
||||
};
|
||||
|
||||
use crate::db::ApiAuthed;
|
||||
use windmill_mcp::parse_mcp_scopes;
|
||||
|
||||
/// Token expiration for MCP OAuth tokens (1 week in seconds)
|
||||
const MCP_OAUTH_TOKEN_EXPIRATION_SECS: u64 = 7 * 24 * 60 * 60;
|
||||
@@ -585,6 +586,8 @@ async fn handle_refresh_token_grant(
|
||||
Some(&new_access_token)
|
||||
};
|
||||
let new_refresh_token = rd_string(32);
|
||||
// Re-issues the already-approved (hence already-contained) scopes verbatim;
|
||||
// containment is enforced once at approval time, so no re-check here.
|
||||
let scopes = token_row.scopes;
|
||||
|
||||
// Create new access token (rejects archived workspaces inline)
|
||||
@@ -820,6 +823,40 @@ async fn oauth_approve_inner(
|
||||
.map(|s| s.to_string())
|
||||
.collect();
|
||||
|
||||
// The approver's own token bounds what it may grant: a scope-restricted MCP
|
||||
// token must not approve a broader one (e.g. mcp:scripts:f/x -> mcp:all). An
|
||||
// unrestricted approver (interactive session, scopes None) grants freely,
|
||||
// which is the normal consent flow. This is the legitimate MCP-narrowing
|
||||
// path, so it uses MCP-pattern containment rather than the byte-identical
|
||||
// rule ensure_scopes_within_caller applies on the generic token endpoints.
|
||||
let caller_restricted = authed
|
||||
.scopes
|
||||
.as_deref()
|
||||
.is_some_and(|s| s.iter().any(|x| !x.starts_with("if_jobs:filter_tags:")));
|
||||
if caller_restricted {
|
||||
// An empty grant would mint a token the auth layer treats as unscoped
|
||||
// (full privileges), so a restricted approver must not produce one.
|
||||
if scopes.is_empty() {
|
||||
return Err(Error::NotAuthorized(
|
||||
"A scope-restricted token cannot approve an empty scope grant".to_string(),
|
||||
));
|
||||
}
|
||||
if scopes.iter().any(|s| !s.starts_with("mcp:")) {
|
||||
return Err(Error::NotAuthorized(
|
||||
"A scope-restricted token can only approve MCP (mcp:*) scopes".to_string(),
|
||||
));
|
||||
}
|
||||
let caller_config = parse_mcp_scopes(authed.scopes.as_deref().unwrap_or(&[]))
|
||||
.map_err(|e| Error::InternalErr(format!("Failed to parse caller MCP scopes: {e}")))?;
|
||||
let requested_config = parse_mcp_scopes(&scopes)
|
||||
.map_err(|e| Error::BadRequest(format!("Failed to parse requested MCP scopes: {e}")))?;
|
||||
if !caller_config.contains(&requested_config) {
|
||||
return Err(Error::NotAuthorized(
|
||||
"Requested scopes exceed the approving token's own MCP scopes".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
sqlx::query!(
|
||||
"INSERT INTO mcp_oauth_server_code
|
||||
(code, client_id, user_email, workspace_id, scopes, redirect_uri, code_challenge, code_challenge_method)
|
||||
|
||||
@@ -455,9 +455,35 @@ pub async fn create_http_request(
|
||||
}
|
||||
};
|
||||
|
||||
// Bound the minted JWT to exactly this proxied route so a scope-restricted
|
||||
// MCP token can't be widened into a full-privilege blank check. The
|
||||
// endpoint-name gate (in the MCP runner) already authorized *which* endpoint
|
||||
// may be called; this constrains what the resulting request can do. Unscoped
|
||||
// callers (cookie / full-privilege tokens) keep an unscoped JWT to preserve
|
||||
// existing behavior. A scope-restricted caller whose route can't be resolved
|
||||
// fails closed.
|
||||
let caller_restricted = api_authed
|
||||
.scopes
|
||||
.as_deref()
|
||||
.is_some_and(|s| s.iter().any(|x| !x.starts_with("if_jobs:filter_tags:")));
|
||||
let scopes = if caller_restricted {
|
||||
let parsed = reqwest::Url::parse(url)
|
||||
.map_err(|e| ErrorData::internal_error(format!("Invalid proxied URL: {}", e), None))?;
|
||||
let scope =
|
||||
windmill_api_auth::scopes::scope_for_route(method, parsed.path()).ok_or_else(|| {
|
||||
ErrorData::internal_error(
|
||||
"Could not derive route scope for proxied MCP endpoint".to_string(),
|
||||
None,
|
||||
)
|
||||
})?;
|
||||
Some(vec![scope])
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Add authorization header
|
||||
let authed = Authed::from(api_authed.clone());
|
||||
let token = create_jwt_token(authed, workspace_id, 3600, None, None, None, None)
|
||||
let token = create_jwt_token(authed, workspace_id, 3600, None, None, None, scopes)
|
||||
.await
|
||||
.map_err(|e| ErrorData::internal_error(e.to_string(), None))?;
|
||||
request_builder = request_builder.header("Authorization", format!("Bearer {}", token));
|
||||
|
||||
@@ -12,6 +12,8 @@ use crate::db::ApiAuthed;
|
||||
|
||||
use crate::{apps::AppWithLastVersion, db::DB, folders::Folder};
|
||||
|
||||
use windmill_api_auth::check_scopes;
|
||||
|
||||
#[cfg(any(
|
||||
feature = "http_trigger",
|
||||
feature = "websocket",
|
||||
@@ -582,6 +584,18 @@ pub(crate) async fn tarball_workspace(
|
||||
skip_resources
|
||||
);
|
||||
|
||||
// The route is gated by workspaces:read, but exporting DECRYPTED secrets is a
|
||||
// variable-read capability beyond workspace metadata. Require variables:read
|
||||
// only on the plaintext-secret path: ordinary tarball pulls (structure and
|
||||
// encrypted-only values) keep working with workspaces:read, and the workspace
|
||||
// key itself stays admin-only (include_key). No-op for unscoped tokens.
|
||||
if plain_secret.or(plain_secrets).unwrap_or(false)
|
||||
&& !skip_secrets.unwrap_or(false)
|
||||
&& !skip_variables.unwrap_or(false)
|
||||
{
|
||||
check_scopes(&authed, || "variables:read".to_string())?;
|
||||
}
|
||||
|
||||
// Opt-in behavior for surfacing per-resource ACLs on flow/app rows.
|
||||
// Folder and group rows have always carried `extra_perms` in source and
|
||||
// continue to do so unconditionally (`KeepEvenEmpty`) so existing
|
||||
@@ -594,6 +608,24 @@ pub(crate) async fn tarball_workspace(
|
||||
|
||||
let mut tx = user_db.begin(&authed).await?;
|
||||
|
||||
// Exporting decrypted secrets in bulk is the same capability as a per-item
|
||||
// secret read, so record it for parity with variables.decrypt_secret.
|
||||
if plain_secret.or(plain_secrets).unwrap_or(false)
|
||||
&& !skip_variables.unwrap_or(false)
|
||||
&& !skip_secrets.unwrap_or(false)
|
||||
{
|
||||
windmill_audit::audit_oss::audit_log(
|
||||
&mut *tx,
|
||||
&authed,
|
||||
"variables.decrypt_secret",
|
||||
windmill_audit::ActionKind::Execute,
|
||||
&w_id,
|
||||
Some("workspace_tarball_export"),
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
// Source-of-truth for fork-ness: the workspace's parent_workspace_id column.
|
||||
// The wm-fork-* prefix is a creation-time naming convention that could in
|
||||
// principle drift (rename, manual SQL); the column is the contract that
|
||||
|
||||
@@ -38,6 +38,71 @@ impl McpScopeConfig {
|
||||
|
||||
is_resource_allowed(path, patterns)
|
||||
}
|
||||
|
||||
/// Directional subset check: does this config grant at least everything
|
||||
/// `requested` grants? Used to enforce monotonic containment when an MCP
|
||||
/// OAuth approval mints a token (the granted scopes must be within the
|
||||
/// approving token's own scopes).
|
||||
///
|
||||
/// Unlike `is_allowed` (which tests a single concrete path with OR
|
||||
/// semantics), this requires every requested pattern to be covered by some
|
||||
/// caller pattern — so `mcp:scripts:f/x` cannot widen into `mcp:scripts:*`.
|
||||
pub fn contains(&self, requested: &McpScopeConfig) -> bool {
|
||||
if self.all {
|
||||
return true;
|
||||
}
|
||||
if requested.all {
|
||||
return false;
|
||||
}
|
||||
if requested.favorites && !self.favorites {
|
||||
return false;
|
||||
}
|
||||
if let Some(req_hub) = requested.hub_apps.as_ref() {
|
||||
match self.hub_apps.as_ref() {
|
||||
Some(caller_hub) => {
|
||||
let caller_apps: std::collections::HashSet<&str> =
|
||||
caller_hub.split(',').map(|s| s.trim()).collect();
|
||||
if !req_hub
|
||||
.split(',')
|
||||
.map(|s| s.trim())
|
||||
.all(|a| caller_apps.contains(a))
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
None => return false,
|
||||
}
|
||||
}
|
||||
resource_list_covers(&self.scripts, &requested.scripts)
|
||||
&& resource_list_covers(&self.flows, &requested.flows)
|
||||
&& resource_list_covers(&self.endpoints, &requested.endpoints)
|
||||
}
|
||||
}
|
||||
|
||||
/// Every requested pattern must be covered by some caller pattern.
|
||||
fn resource_list_covers(caller: &[String], requested: &[String]) -> bool {
|
||||
requested
|
||||
.iter()
|
||||
.all(|req| caller.iter().any(|c| pattern_covers(c, req)))
|
||||
}
|
||||
|
||||
/// Directional: does the single caller pattern cover `requested`? `caller` may
|
||||
/// be `*`, an exact path/name, or a `<prefix>/*` subtree; `requested` may itself
|
||||
/// be a subtree wildcard, in which case the whole requested subtree must fall
|
||||
/// within the caller's. Mirrors the route-scope containment in windmill-api-auth.
|
||||
fn pattern_covers(caller: &str, requested: &str) -> bool {
|
||||
if caller == "*" || caller == requested {
|
||||
return true;
|
||||
}
|
||||
// An exact caller pattern only covers itself (handled above); a wildcard
|
||||
// requested can never be covered by a non-`*` exact caller.
|
||||
let Some(prefix) = caller.strip_suffix("/*") else {
|
||||
return false;
|
||||
};
|
||||
let requested_base = requested.strip_suffix("/*").unwrap_or(requested);
|
||||
requested_base == prefix
|
||||
|| (requested_base.starts_with(prefix)
|
||||
&& requested_base.as_bytes().get(prefix.len()) == Some(&b'/'))
|
||||
}
|
||||
|
||||
/// Parse MCP scopes from token scope strings
|
||||
@@ -254,4 +319,51 @@ mod tests {
|
||||
assert!(config.is_allowed("flow", "f/automation/test"));
|
||||
assert!(!config.is_allowed("flow", "f/other/test"));
|
||||
}
|
||||
|
||||
fn cfg(scopes: &[&str]) -> McpScopeConfig {
|
||||
parse_mcp_scopes(&scopes.iter().map(|s| s.to_string()).collect::<Vec<_>>()).unwrap()
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_contains_subset_and_widening() {
|
||||
// mcp:all contains anything.
|
||||
assert!(cfg(&["mcp:all"]).contains(&cfg(&["mcp:scripts:f/x"])));
|
||||
assert!(cfg(&["mcp:all"]).contains(&cfg(&["mcp:all"])));
|
||||
|
||||
// A wildcard caller covers narrower requests, but not other domains/all.
|
||||
let star = cfg(&["mcp:scripts:*"]);
|
||||
assert!(star.contains(&cfg(&["mcp:scripts:f/x"])));
|
||||
assert!(star.contains(&cfg(&["mcp:scripts:*"])));
|
||||
assert!(!star.contains(&cfg(&["mcp:all"])));
|
||||
assert!(!star.contains(&cfg(&["mcp:flows:f/x"])));
|
||||
|
||||
// The core regression: a single-path caller must NOT widen into `*` or
|
||||
// into another path.
|
||||
let narrow = cfg(&["mcp:scripts:f/x"]);
|
||||
assert!(narrow.contains(&cfg(&["mcp:scripts:f/x"])));
|
||||
assert!(!narrow.contains(&cfg(&["mcp:scripts:*"])));
|
||||
assert!(!narrow.contains(&cfg(&["mcp:scripts:f/y"])));
|
||||
assert!(!narrow.contains(&cfg(&["mcp:all"])));
|
||||
|
||||
// Subtree wildcard covers paths within it but not a sibling subtree.
|
||||
let subtree = cfg(&["mcp:scripts:f/team/*"]);
|
||||
assert!(subtree.contains(&cfg(&["mcp:scripts:f/team/sub"])));
|
||||
assert!(subtree.contains(&cfg(&["mcp:scripts:f/team/sub/*"])));
|
||||
assert!(!subtree.contains(&cfg(&["mcp:scripts:f/other/x"])));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_contains_favorites_and_endpoints() {
|
||||
assert!(cfg(&["mcp:favorites"]).contains(&cfg(&["mcp:favorites"])));
|
||||
// A caller without favorites cannot grant favorites.
|
||||
assert!(!cfg(&["mcp:scripts:*"]).contains(&cfg(&["mcp:favorites"])));
|
||||
|
||||
// Endpoint names match exactly (or via `*`).
|
||||
let ep = cfg(&["mcp:endpoints:getVariable"]);
|
||||
assert!(ep.contains(&cfg(&["mcp:endpoints:getVariable"])));
|
||||
assert!(!ep.contains(&cfg(&["mcp:endpoints:getResource"])));
|
||||
assert!(!ep.contains(&cfg(&["mcp:all"])));
|
||||
// mcp:all grants all endpoints.
|
||||
assert!(cfg(&["mcp:all"]).contains(&cfg(&["mcp:endpoints:getResource"])));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1634,6 +1634,12 @@ async fn update_resource(
|
||||
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("resources:write:{}", path))?;
|
||||
// A rename moves the resource (and its linked variable) to ns.path, so the
|
||||
// destination must also be within the token's write scope, not just the
|
||||
// source path.
|
||||
if let Some(npath) = ns.path.as_deref() {
|
||||
check_scopes(&authed, || format!("resources:write:{}", npath))?;
|
||||
}
|
||||
if let RuleCheckResult::Blocked(msg) = check_deploy_rules(
|
||||
&w_id,
|
||||
AuditAuthorable::username(&authed),
|
||||
|
||||
@@ -1037,6 +1037,12 @@ async fn update_variable(
|
||||
|
||||
let path = path.to_path();
|
||||
check_scopes(&authed, || format!("variables:write:{}", path))?;
|
||||
// A rename moves the (possibly secret) variable to ns.path, so the
|
||||
// destination must also be within the token's write scope, not just the
|
||||
// source path.
|
||||
if let Some(npath) = ns.path.as_deref() {
|
||||
check_scopes(&authed, || format!("variables:write:{}", npath))?;
|
||||
}
|
||||
let authed = maybe_refresh_folders(&path, &w_id, authed, &db).await;
|
||||
|
||||
let mut sqlb = SqlBuilder::update_table("variable");
|
||||
|
||||
@@ -7,7 +7,7 @@ use axum::{extract::Path, routing::post, Extension, Json, Router};
|
||||
use http::StatusCode;
|
||||
use sqlx::PgConnection;
|
||||
use std::collections::HashSet;
|
||||
use windmill_api_auth::ApiAuthed;
|
||||
use windmill_api_auth::{check_scopes, ApiAuthed};
|
||||
use windmill_audit::{audit_oss::audit_log, ActionKind};
|
||||
use windmill_common::global_settings::HTTP_ROUTE_WORKSPACED_ROUTE;
|
||||
use windmill_common::{
|
||||
@@ -262,6 +262,12 @@ pub async fn create_many_http_triggers(
|
||||
let mut route_path_keys = Vec::with_capacity(new_http_triggers.len());
|
||||
|
||||
for new_http_trigger in new_http_triggers.iter() {
|
||||
// Per-item write scope, matching the single-create handler. The bulk
|
||||
// endpoint must not let a path-scoped token create triggers outside it.
|
||||
check_scopes(&authed, || {
|
||||
format!("http_triggers:write:{}", &new_http_trigger.base.path)
|
||||
})?;
|
||||
|
||||
handler
|
||||
.validate_new(&db, &w_id, &new_http_trigger.config)
|
||||
.await
|
||||
@@ -373,7 +379,8 @@ impl TriggerCrud for HttpTrigger {
|
||||
|
||||
const TABLE_NAME: &'static str = "http_trigger";
|
||||
const TRIGGER_TYPE: &'static str = "http";
|
||||
const DRAFT_KIND: windmill_common::user_drafts::UserDraftItemKind = windmill_common::user_drafts::UserDraftItemKind::TriggerHttp;
|
||||
const DRAFT_KIND: windmill_common::user_drafts::UserDraftItemKind =
|
||||
windmill_common::user_drafts::UserDraftItemKind::TriggerHttp;
|
||||
const SUPPORTS_SERVER_STATE: bool = false;
|
||||
const SUPPORTS_TEST_CONNECTION: bool = false;
|
||||
const ROUTE_PREFIX: &'static str = "/http_triggers";
|
||||
|
||||
@@ -4,7 +4,7 @@ use async_trait::async_trait;
|
||||
use itertools::Itertools;
|
||||
use serde_json::value::RawValue;
|
||||
use sqlx::{types::Json as SqlxJson, PgConnection};
|
||||
use windmill_api_auth::ApiAuthed;
|
||||
use windmill_api_auth::{check_scopes, ApiAuthed};
|
||||
use windmill_common::DB;
|
||||
use windmill_common::{
|
||||
db::UserDB,
|
||||
@@ -15,10 +15,39 @@ use windmill_git_sync::DeployedObject;
|
||||
use windmill_trigger::{Trigger, TriggerCrud, TriggerData};
|
||||
|
||||
use super::{
|
||||
get_url_from_runnable_value, proxy::connect_async_with_proxy, validate_websocket_url_for_ssrf,
|
||||
TestWebsocketConfig, WebsocketConfig, WebsocketConfigRequest, WebsocketTrigger,
|
||||
get_url_from_runnable_value, listener::InitialMessage, proxy::connect_async_with_proxy,
|
||||
validate_websocket_url_for_ssrf, TestWebsocketConfig, WebsocketConfig, WebsocketConfigRequest,
|
||||
WebsocketTrigger,
|
||||
};
|
||||
|
||||
/// A websocket_triggers:write token can configure secondary runnables that the
|
||||
/// listener later executes under the trigger owner's identity: a `$flow:`/
|
||||
/// `$script:` URL resolver and `initial_messages` of kind `runnable_result`.
|
||||
/// That execution happens in a background task where the reconstructed authed is
|
||||
/// scopeless (so its check_scopes is a no-op), so enforce run scope here, at
|
||||
/// create/update time, against the API caller's token.
|
||||
fn check_secondary_runnable_scopes(
|
||||
authed: &ApiAuthed,
|
||||
config: &WebsocketConfigRequest,
|
||||
) -> Result<()> {
|
||||
if let Some(rest) = config.url.strip_prefix("$flow:") {
|
||||
check_scopes(authed, || format!("jobs:run:flows:{}", rest))?;
|
||||
} else if let Some(rest) = config.url.strip_prefix("$script:") {
|
||||
check_scopes(authed, || format!("jobs:run:scripts:{}", rest))?;
|
||||
}
|
||||
if let Some(messages) = config.initial_messages.as_ref() {
|
||||
for msg in messages {
|
||||
if let Ok(InitialMessage::RunnableResult { path, is_flow, .. }) =
|
||||
serde_json::from_value::<InitialMessage>(msg.clone())
|
||||
{
|
||||
let kind = if is_flow { "flows" } else { "scripts" };
|
||||
check_scopes(authed, || format!("jobs:run:{}:{}", kind, path))?;
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl TriggerCrud for WebsocketTrigger {
|
||||
type TriggerConfig = WebsocketConfig;
|
||||
@@ -101,6 +130,7 @@ impl TriggerCrud for WebsocketTrigger {
|
||||
w_id: &str,
|
||||
trigger: TriggerData<Self::TriggerConfigRequest>,
|
||||
) -> Result<()> {
|
||||
check_secondary_runnable_scopes(authed, &trigger.config)?;
|
||||
let resolved_edited_by = trigger.base.resolve_edited_by(authed);
|
||||
let resolved_permissioned_as = trigger.base.resolve_permissioned_as(authed);
|
||||
let filters = trigger
|
||||
@@ -178,6 +208,7 @@ impl TriggerCrud for WebsocketTrigger {
|
||||
path: &str,
|
||||
trigger: TriggerData<Self::TriggerConfigRequest>,
|
||||
) -> Result<()> {
|
||||
check_secondary_runnable_scopes(authed, &trigger.config)?;
|
||||
let resolved_edited_by = trigger.base.resolve_edited_by(authed);
|
||||
let resolved_permissioned_as = trigger.base.resolve_permissioned_as(authed);
|
||||
let filters = trigger
|
||||
|
||||
@@ -509,7 +509,7 @@ impl Clone for ReturnMessageChannels {
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize)]
|
||||
enum InitialMessage {
|
||||
pub(crate) enum InitialMessage {
|
||||
#[serde(rename = "raw_message")]
|
||||
RawMessage(String),
|
||||
#[serde(rename = "runnable_result")]
|
||||
|
||||
Reference in New Issue
Block a user