mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-08-18 16:02:10 +00:00
fix: validate websocket trigger urls and gate trigger test route (#9682)
* fix: validate websocket trigger urls and gate trigger test route Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: clarify validate_websocket_url_for_ssrf call sites 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:
@@ -97,7 +97,7 @@ published advisory history (73 GHSA advisories, several rated 9.9 critical).
|
||||
| id | threat | actor | surface | asset | impact | likelihood | status | controls | evidence |
|
||||
|---|---|---|---|---|---|---|---|---|---|
|
||||
| T1 | SQL injection in app/internal query builders and trigger clauses compromises the metadata DB and connected databases | remote_auth | EP8 | Database, downstream connected systems | critical | almost_certain | partially_mitigated | sqlx parameterized queries elsewhere; query-builder safety reviews | GHSA-225c-j3xq-g6x6, GHSA-78p7-jc72-gv66, GHSA-hvc7-f67h-jx3g, GHSA-wrrg-f89m-f84q, GHSA-79vf-3qwm-2w64, GHSA-55p6-fxj4-v983, GHSA-5g4v-49rj-r52r, GHSA-x6cq-7xr8-53x3, 2cf4bb180b |
|
||||
| T2 | Server-side request forgery via proxies/executors reaches cloud metadata, internal network, and downstream credentials | remote_auth | EP6, EP7 | Cloud metadata, internal network, downstream connected systems, resource creds | critical | almost_certain | partially_mitigated | SSRF URL validation + redirect-following disabled added piecemeal; MCP private URL access requires the instance-wide `ALLOW_PRIVATE_MCP_SERVER_URLS` opt-in; outbound network isolation (`clone_newnet`) is opt-in and off by default | GHSA-3ggp-h37f-5qfw, GHSA-98qq-g8rh-xhff, GHSA-hfw8-27mx-63jm, GHSA-3r59-qvvc-774j, GHSA-4pj9-w5jc-g8w7, GHSA-8hh3-jf25-78j5, GHSA-3pjm-4w7f-3r2w, GHSA-f44c-x9hq-h68r, GHSA-j4h4-f8fj-3m3c, 4b06881918, 96a8eb63d4, dbd3942ef3 |
|
||||
| T2 | Server-side request forgery via proxies/executors reaches cloud metadata, internal network, and downstream credentials | remote_auth | EP6, EP7 | Cloud metadata, internal network, downstream connected systems, resource creds | critical | almost_certain | partially_mitigated | SSRF URL validation + redirect-following disabled added piecemeal; MCP private URL access requires the instance-wide `ALLOW_PRIVATE_MCP_SERVER_URLS` opt-in; WebSocket trigger URLs (stored, test, and runnable-resolved) are SSRF-validated at connect time behind the `ALLOW_PRIVATE_WEBSOCKET_URLS` opt-in, and the trigger test route now requires `:write` scope; outbound network isolation (`clone_newnet`) is opt-in and off by default | GHSA-3ggp-h37f-5qfw, GHSA-98qq-g8rh-xhff, GHSA-hfw8-27mx-63jm, GHSA-3r59-qvvc-774j, GHSA-4pj9-w5jc-g8w7, GHSA-8hh3-jf25-78j5, GHSA-3pjm-4w7f-3r2w, GHSA-f44c-x9hq-h68r, GHSA-j4h4-f8fj-3m3c, 4b06881918, 96a8eb63d4, dbd3942ef3 |
|
||||
| T3 | Broken authorization / IDOR lets a scoped token or low-privilege member read scripts, job data, and secrets across folders and workspaces | remote_auth | EP5, EP2, EP1 | Scripts, job data, secrets, isolation | critical | almost_certain | partially_mitigated | RLS, token scopes, folder ACLs, view-token HMAC (added incrementally); on managed, sensitive tenants can opt into dedicated DB/worker/namespace, but the shared tier IS the software boundary | GHSA-qfg7-x243-5hg4, GHSA-8x8x-88qc-qp4r, GHSA-2ppx-66jv-wpw5, GHSA-x3x7-g97v-mp59, GHSA-j276-g4h8-g6h5, GHSA-8mv7-hmrg-96xv, GHSA-x2wf-f962-7frq, GHSA-qc7c-gcw6-h4xp, GHSA-vxc5-w28p-m9xw, GHSA-2g34-wfvr-5qqj, GHSA-w7p6-wpxm-pp66, 7edf3f0212, 89a7a37776, ab11c7747a, 664edcdfb7 |
|
||||
| T4 | Remote code execution by injecting attacker-controlled identifiers into generated worker wrappers | remote_auth | EP10 | Worker host, isolation, downstream | critical | likely | partially_mitigated | entrypoint/env-var-name validation added | GHSA-wxjq-w5pj-jqhx, GHSA-5f5q-2vg2-r2x4, GHSA-8q8j-mm3g-5c2q (CVE-2026-33881), bf93657fee, bd05bcadde, 22ec4da5f0 |
|
||||
| T5 | Worker compromise & cross-tenant access via weak-by-default isolation (nsjail off by default → user code runs with only PID-ns `unshare`); sandbox escape where nsjail/dind/podman is enabled | remote_auth | EP9, EP15 | Worker host, isolation, downstream | critical | likely | unmitigated | nsjail off by default everywhere (`DISABLE_NSJAIL=true`); shipped compose gives PID-ns `unshare` only (`FAVOR_UNSHARE_PID=true`), bare installs get no isolation. Where nsjail enabled: read-only remounts, jail-tmp refusal, podman socket gating | GHSA-6qr8-xhg4-453q, GHSA-3vpp-vf62-wqp6, f8467f38c8, df5aec0f5d, f1b6746e0e |
|
||||
|
||||
@@ -15,8 +15,8 @@ use windmill_git_sync::DeployedObject;
|
||||
use windmill_trigger::{Trigger, TriggerCrud, TriggerData};
|
||||
|
||||
use super::{
|
||||
get_url_from_runnable_value, proxy::connect_async_with_proxy, TestWebsocketConfig,
|
||||
WebsocketConfig, WebsocketConfigRequest, WebsocketTrigger,
|
||||
get_url_from_runnable_value, proxy::connect_async_with_proxy, validate_websocket_url_for_ssrf,
|
||||
TestWebsocketConfig, WebsocketConfig, WebsocketConfigRequest, WebsocketTrigger,
|
||||
};
|
||||
|
||||
#[async_trait]
|
||||
@@ -28,7 +28,8 @@ impl TriggerCrud for WebsocketTrigger {
|
||||
|
||||
const TABLE_NAME: &'static str = "websocket_trigger";
|
||||
const TRIGGER_TYPE: &'static str = "websocket";
|
||||
const DRAFT_KIND: windmill_common::user_drafts::UserDraftItemKind = windmill_common::user_drafts::UserDraftItemKind::TriggerWebsocket;
|
||||
const DRAFT_KIND: windmill_common::user_drafts::UserDraftItemKind =
|
||||
windmill_common::user_drafts::UserDraftItemKind::TriggerWebsocket;
|
||||
const SUPPORTS_SERVER_STATE: bool = true;
|
||||
const SUPPORTS_TEST_CONNECTION: bool = true;
|
||||
const ROUTE_PREFIX: &'static str = "/websocket_triggers";
|
||||
@@ -61,6 +62,13 @@ impl TriggerCrud for WebsocketTrigger {
|
||||
));
|
||||
}
|
||||
|
||||
// Reject SSRF targets at save time for static URLs. A `$flow:`/`$script:`
|
||||
// URL is only known at runtime, so it is validated at connect time
|
||||
// instead (in the listener and test handler).
|
||||
if !config.url.starts_with('$') {
|
||||
validate_websocket_url_for_ssrf(&config.url).await?;
|
||||
}
|
||||
|
||||
if let Some(args) = &config.url_runnable_args {
|
||||
if !args.is_object() {
|
||||
return Err(Error::BadRequest(
|
||||
@@ -277,6 +285,8 @@ impl TriggerCrud for WebsocketTrigger {
|
||||
Cow::Borrowed(&url)
|
||||
};
|
||||
|
||||
validate_websocket_url_for_ssrf(&connect_url).await?;
|
||||
|
||||
connect_async_with_proxy(&*connect_url)
|
||||
.await
|
||||
.map_err(|err| {
|
||||
|
||||
@@ -104,6 +104,58 @@ pub fn value_to_args_hashmap(
|
||||
Ok(args)
|
||||
}
|
||||
|
||||
/// Env var that opts a deployment out of SSRF validation for WebSocket trigger
|
||||
/// URLs, permitting connections to private/internal addresses. Off by default.
|
||||
pub const ALLOW_PRIVATE_WEBSOCKET_URLS_ENV: &str = "ALLOW_PRIVATE_WEBSOCKET_URLS";
|
||||
|
||||
/// Reject WebSocket URLs that target (or resolve to) a private/internal address,
|
||||
/// blocking SSRF probes of the host's internal network and cloud metadata
|
||||
/// endpoints.
|
||||
///
|
||||
/// `ws://`/`wss://` are mapped to `http`/`https` so the shared
|
||||
/// `validate_url_for_ssrf` host + DNS-resolution checks apply. The
|
||||
/// security-critical call sites are the outbound connects (the test handler and
|
||||
/// every listener (re)connect): validating the *resolved* URL there means a
|
||||
/// `$flow:`/`$script:` URL is checked on its returned value and re-checked on
|
||||
/// each reconnect (DNS rebinding). `validate_config` also calls this at save
|
||||
/// time to reject static URLs early.
|
||||
pub async fn validate_websocket_url_for_ssrf(url: &str) -> Result<()> {
|
||||
if std::env::var(ALLOW_PRIVATE_WEBSOCKET_URLS_ENV)
|
||||
.ok()
|
||||
.is_some_and(|v| v == "true" || v == "1")
|
||||
{
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// `ws`/`wss` aren't recognised by `validate_url_for_ssrf`'s scheme check, so
|
||||
// map them to the http(s) equivalent the same connection would tunnel over.
|
||||
// The prefixes are ASCII, so byte-slicing at their length stays on a char
|
||||
// boundary.
|
||||
let lower = url.to_ascii_lowercase();
|
||||
let http_url = if lower.starts_with("wss://") {
|
||||
format!("https://{}", &url["wss://".len()..])
|
||||
} else if lower.starts_with("ws://") {
|
||||
format!("http://{}", &url["ws://".len()..])
|
||||
} else {
|
||||
url.to_string()
|
||||
};
|
||||
|
||||
windmill_common::ssrf::validate_url_for_ssrf(&http_url)
|
||||
.await
|
||||
.map_err(|e| match e {
|
||||
// The env-var hint is only actionable for a well-formed URL blocked
|
||||
// for targeting a private address; a malformed URL or bad scheme
|
||||
// surfaces its real error so the user fixes the URL (see #9171).
|
||||
e @ windmill_common::ssrf::SsrfValidationError::Private { .. } => {
|
||||
Error::BadRequest(format!(
|
||||
"{e}. If you need to connect to private/internal WebSocket endpoints, \
|
||||
set the {ALLOW_PRIVATE_WEBSOCKET_URLS_ENV}=true environment variable"
|
||||
))
|
||||
}
|
||||
e => Error::from(e),
|
||||
})
|
||||
}
|
||||
|
||||
pub async fn get_url_from_runnable_value(
|
||||
path: &str,
|
||||
is_flow: bool,
|
||||
@@ -144,3 +196,40 @@ pub async fn get_url_from_runnable_value(
|
||||
))
|
||||
})
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[tokio::test]
|
||||
async fn ssrf_blocks_private_and_metadata_ws_urls() {
|
||||
// ws:// → http:// mapping must still reach the IP-literal block.
|
||||
let err = validate_websocket_url_for_ssrf("ws://127.0.0.1:6379/")
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(matches!(err, Error::BadRequest(_)));
|
||||
// Private errors carry the opt-out hint so operators can allow internal
|
||||
// targets deliberately.
|
||||
assert!(err.to_string().contains(ALLOW_PRIVATE_WEBSOCKET_URLS_ENV));
|
||||
|
||||
// wss:// → https:// mapping blocks the cloud metadata endpoint.
|
||||
assert!(
|
||||
validate_websocket_url_for_ssrf("wss://169.254.169.254/latest/meta-data")
|
||||
.await
|
||||
.is_err()
|
||||
);
|
||||
assert!(validate_websocket_url_for_ssrf("ws://10.0.0.5:6379/")
|
||||
.await
|
||||
.is_err());
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn ssrf_rejects_non_ws_scheme_without_private_hint() {
|
||||
// A non-ws scheme isn't mapped and fails the scheme check; it must not
|
||||
// get the "set ALLOW_PRIVATE_WEBSOCKET_URLS" hint (issue #9171).
|
||||
let err = validate_websocket_url_for_ssrf("file:///etc/passwd")
|
||||
.await
|
||||
.unwrap_err();
|
||||
assert!(!err.to_string().contains(ALLOW_PRIVATE_WEBSOCKET_URLS_ENV));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::{
|
||||
get_url_from_runnable_value, proxy::connect_async_with_proxy, WebsocketConfig, WebsocketTrigger,
|
||||
get_url_from_runnable_value, proxy::connect_async_with_proxy, validate_websocket_url_for_ssrf,
|
||||
WebsocketConfig, WebsocketTrigger,
|
||||
};
|
||||
use anyhow::Context;
|
||||
use async_trait::async_trait;
|
||||
@@ -173,6 +174,8 @@ impl Listener for WebsocketTrigger {
|
||||
Cow::Borrowed(&url)
|
||||
};
|
||||
|
||||
validate_websocket_url_for_ssrf(&connect_url).await?;
|
||||
|
||||
let connection = connect_async_with_proxy(&*connect_url)
|
||||
.await
|
||||
.map(|conn| Some(conn))
|
||||
|
||||
@@ -1057,6 +1057,10 @@ async fn test_connection<T: TriggerCrud>(
|
||||
Path(workspace_id): Path<String>,
|
||||
Json(config): Json<T::TestConnectionConfig>,
|
||||
) -> Result<()> {
|
||||
// Test connection opens an outbound connection to a caller-supplied target,
|
||||
// so gate it behind write access like the other mutating trigger routes.
|
||||
check_scopes(&authed, || format!("{}:write", T::scope_domain_name()))?;
|
||||
|
||||
let connect_f = async move {
|
||||
handler
|
||||
.test_connection(&db, &authed, &user_db, &workspace_id, config)
|
||||
|
||||
Reference in New Issue
Block a user