fix(security): validate ansible git repository URLs before invoking git (#10759)

The Ansible executor passed the user-controlled git repository `url` (from
playbook YAML or a `git_repository` resource) straight into `git clone`,
`git ls-remote` and `git remote add` on the worker host. A URL that git parses
as an option — e.g. `--upload-pack=<cmd>` — turns `git ls-remote <url> HEAD`
into arbitrary command execution on the host, outside any job sandbox. Non-http
transports (`ext::`, `file://`, local paths) similarly run programs or read
host files.

Add `validate_git_repo_url` in windmill-common: reject a leading `-`, reject
remote-helper `::` syntax, and allow only the `http(s)`, `ssh`, `git` and
scp-like `[user@]host:path` transports. Also reject a `branch`/`commit` that
starts with `-`. Validation runs at every ansible entry point that spawns git,
covering both the inline-YAML and resource-provided URL paths.

CWE-88 (argument injection) / CWE-78. Reported by Nitin Gavhane.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

backend/windmill-common/src/git_sync_oss.rs

@@ -38,6 +38,73 @@ pub fn sanitize_git_url(url: &str) -> String {
     GIT_URL_USERINFO_RE.replace(url, "://***@").into_owned()
 }
 
+/// Validate a user-supplied git remote URL before it is handed to `git` (`clone`,
+/// `ls-remote`, `remote add`, `fetch`, ...). Two classes of abuse are rejected:
+///  - Argument injection: a URL that git parses as a command-line option (e.g.
+///    `--upload-pack=<cmd>`) turns `git ls-remote <url> HEAD` into arbitrary command
+///    execution on the worker host, outside any job sandbox.
+///  - Dangerous transports: git's remote-helper syntax (`ext::sh -c ...`, `fd::...`) runs
+///    arbitrary programs, and `file://` / local paths read host files — both escape the
+///    intended network-only fetch.
+///
+/// Only the standard network transports are allowed: `http(s)`, `ssh`, `git`, and the
+/// scp-like `[user@]host:path` shorthand. Validation is transport-syntax based (not git
+/// version dependent) so it holds regardless of git's own option/protocol handling.
+pub fn validate_git_repo_url(url: &str) -> crate::error::Result<()> {
+    let reject =
+        |msg: &str| crate::error::Error::BadRequest(format!("Invalid git repository URL: {msg}"));
+
+    let trimmed = url.trim();
+    if trimmed.is_empty() {
+        return Err(reject("the URL is empty"));
+    }
+    // Leading '-' makes git parse the URL as an option (argument injection).
+    if trimmed.starts_with('-') {
+        return Err(reject("the URL must not start with '-'"));
+    }
+    // `<helper>::<address>` remote-helper transports execute arbitrary programs.
+    if trimmed.contains("::") {
+        return Err(reject("remote-helper transports (`::`) are not allowed"));
+    }
+
+    if let Some((scheme, _rest)) = trimmed.split_once("://") {
+        // A real scheme is ASCII-alnum plus `+ - .` and holds no slash (a slash means the
+        // `://` came from the path, so there is no scheme and this is not a valid URL).
+        let is_scheme = !scheme.is_empty()
+            && !scheme.contains('/')
+            && scheme
+                .chars()
+                .all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.'));
+        if !is_scheme {
+            return Err(reject("malformed URL scheme"));
+        }
+        match scheme.to_ascii_lowercase().as_str() {
+            "http" | "https" | "ssh" | "git" => Ok(()),
+            other => Err(reject(&format!(
+                "scheme `{other}` is not allowed (use http(s), ssh, or git)"
+            ))),
+        }
+    } else {
+        // No scheme: accept only the scp-like `[user@]host:path` shorthand. The host (the
+        // part before the first `:`) must be non-empty and slash-free; a slash there means a
+        // local path (`./repo`, `/abs/repo`), and a single-letter host is a Windows drive.
+        let Some((host, _path)) = trimmed.split_once(':') else {
+            return Err(reject(
+                "local paths are not allowed; use an http(s), ssh, or git URL",
+            ));
+        };
+        let bad_host = host.is_empty()
+            || host.contains('/')
+            || (host.len() == 1 && host.chars().all(|c| c.is_ascii_alphabetic()));
+        if bad_host {
+            return Err(reject(
+                "local paths are not allowed; use an http(s), ssh, or git URL",
+            ));
+        }
+        Ok(())
+    }
+}
+
 pub fn prepend_token_to_github_url(
     github_url: &str,
     installation_token: &str,
@@ -92,4 +159,51 @@ mod tests {
             "not a url://***@host/repo"
         );
     }
+
+    use super::validate_git_repo_url;
+
+    #[test]
+    fn accepts_standard_transports() {
+        for url in [
+            "https://github.com/org/repo.git",
+            "http://internal.example/org/repo.git",
+            "https://user:token@github.com/org/repo.git",
+            "ssh://git@github.com/org/repo.git",
+            "ssh://git@github.com:2222/org/repo.git",
+            "git://github.com/org/repo.git",
+            "git@github.com:org/repo.git",
+            "user@host.example:path/to/repo",
+        ] {
+            assert!(validate_git_repo_url(url).is_ok(), "should accept {url}");
+        }
+    }
+
+    #[test]
+    fn rejects_argument_injection() {
+        for url in [
+            "--upload-pack=touch /tmp/pwned",
+            "-oProxyCommand=touch /tmp/pwned",
+            "--config=core.fsmonitor=touch /tmp/pwned",
+        ] {
+            assert!(validate_git_repo_url(url).is_err(), "should reject {url}");
+        }
+    }
+
+    #[test]
+    fn rejects_remote_helpers_and_local_transports() {
+        for url in [
+            "ext::sh -c 'id > /tmp/pwned'",
+            "fd::17/foo",
+            "file:///etc/passwd",
+            "/etc/passwd",
+            "./local/repo",
+            "../local/repo",
+            "ftp://host/repo",
+            "C:\\path\\to\\repo",
+            "",
+            "   ",
+        ] {
+            assert!(validate_git_repo_url(url).is_err(), "should reject {url:?}");
+        }
+    }
 }

backend/windmill-worker/src/ansible_executor.rs

@@ -13,7 +13,7 @@ use tokio::process::Command;
 use uuid::Uuid;
 use windmill_common::{
     error,
-    git_sync_oss::{prepend_token_to_github_url, sanitize_git_url},
+    git_sync_oss::{prepend_token_to_github_url, sanitize_git_url, validate_git_repo_url},
     worker::{
         is_allowed_file_location, split_python_requirements, to_raw_value, write_file,
         write_file_at_user_defined_location, Connection, PyVAlias, WORKER_CONFIG,
@@ -247,6 +247,24 @@ async fn prepare_socket_root(root: &str, stale_after: std::time::Duration) {
     }
 }
 
+/// Validate every user-controlled field of a `GitRepo` before it reaches `git`. The `url` goes
+/// through the transport allowlist (`validate_git_repo_url`); `branch` and `commit` are passed as
+/// positional/option arguments, so a leading `-` would let git parse them as options (argument
+/// injection). Called at each entry point that spawns `git` with these fields.
+fn validate_git_repo(repo: &GitRepo) -> error::Result<()> {
+    validate_git_repo_url(&repo.url)?;
+    for (field, value) in [("branch", &repo.branch), ("commit", &repo.commit)] {
+        if let Some(value) = value {
+            if value.trim_start().starts_with('-') {
+                return Err(error::Error::BadRequest(format!(
+                    "Invalid git repository `{field}`: must not start with '-'"
+                )));
+            }
+        }
+    }
+    Ok(())
+}
+
 async fn clone_repo(
     repo: &GitRepo,
     job_dir: &str,
@@ -259,6 +277,7 @@ async fn clone_repo(
     occupancy_metrics: &mut OccupancyMetrics,
     git_ssh_cmd: &str,
 ) -> error::Result<String> {
+    validate_git_repo(repo)?;
     let target_path = is_allowed_file_location(job_dir, &repo.target_path)?;
 
     let mut clone_cmd = Command::new(GIT_PATH.as_str());
@@ -400,6 +419,7 @@ async fn clone_repo_without_history(
     occupancy_metrics: &mut OccupancyMetrics,
     git_ssh_cmd: &str,
 ) -> error::Result<()> {
+    validate_git_repo(repo)?;
     let target_path = is_allowed_file_location(job_dir, &repo.target_path)?;
 
     create_empty_dir(&target_path)?;
@@ -926,6 +946,7 @@ pub async fn get_git_repo_full_head_commit_hash(
     repo: &GitRepo,
     git_ssh_cmd: &str,
 ) -> anyhow::Result<String> {
+    validate_git_repo(repo)?;
     let mut git_cmd = Command::new(GIT_PATH.as_str());
 
     git_cmd