diff --git a/apps/desktop/src-tauri/src/extensions.rs b/apps/desktop/src-tauri/src/extensions.rs index 9d1714ad..e8236cdb 100644 --- a/apps/desktop/src-tauri/src/extensions.rs +++ b/apps/desktop/src-tauri/src/extensions.rs @@ -266,8 +266,131 @@ fn get_extensions_dir(app: &AppHandle) -> Result { Ok(ext_dir) } +/// Hosts we allow `git clone` to target from the extension marketplace. Every +/// legitimate entry today points at one of these, and the constraint blocks +/// the bulk of the attack surface: `file://`, Windows UNC, `ssh://`, +/// `git://`, and `ext::` transports can't reach this point because the host +/// won't match, and typo-squatted domains have nowhere to land. +const ALLOWED_GIT_HOSTS: &[&str] = &["github.com", "gitlab.com", "bitbucket.org"]; + +/// Enforces the URL shape that `git clone` will accept: strict `https://`, +/// allow-listed host, exactly `owner/repo[.git]`, ASCII-safe segments, no +/// credentials, no transport-helper syntax. Everything coming out of +/// `resolveGitRepoUrl` on the frontend already looks like this, so the +/// check is free for legitimate catalog entries and hard-stops a compromised +/// registry. +fn validate_git_clone_url(url: &str) -> Result<(), String> { + // Reject control characters and whitespace up front — these should never + // appear in a real clone URL and catching them here keeps downstream + // argv/process parsers honest. + if url.chars().any(|c| c.is_control() || c == ' ') { + return Err("git_url must not contain control characters or whitespace".to_string()); + } + + // Block the `ext::` transport-helper syntax that lets git run arbitrary + // helper binaries. The https:// strip below would catch this too, but + // the explicit message helps when debugging a compromised catalog. + if url.contains("::") { + return Err("git_url must not contain transport-helper syntax (`::`)".to_string()); + } + + let after_scheme = url + .strip_prefix("https://") + .ok_or_else(|| format!("git_url must begin with https:// (got `{}`)", url))?; + + // `user@host/...` shapes shift the host parser and let an attacker embed + // credentials or, worse, alternate hosts. No legitimate marketplace + // entry uses them. + if after_scheme.contains('@') { + return Err("git_url must not contain `@` (credentials/alternate host)".to_string()); + } + + let (host, path) = after_scheme + .split_once('/') + .ok_or_else(|| format!("git_url is missing a repository path: `{}`", url))?; + + if !ALLOWED_GIT_HOSTS.contains(&host) { + return Err(format!( + "git_url host `{}` is not in the allowlist ({})", + host, + ALLOWED_GIT_HOSTS.join(", ") + )); + } + + let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + if segments.len() != 2 { + return Err(format!( + "git_url path must be `/` or `/.git`; got `{}`", + path + )); + } + + for seg in &segments { + if seg.starts_with('-') { + return Err(format!( + "git_url path segment `{}` cannot start with `-`", + seg + )); + } + if !seg + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.') + { + return Err(format!( + "git_url path segment `{}` contains disallowed characters", + seg + )); + } + } + + Ok(()) +} + +/// Reject extension ids that would escape `ext_dir` on join, contain +/// path separators, parent-directory components, or anything other than +/// the safe marketplace-style slug characters. Required because a +/// compromised catalog could supply `id = "../../../Windows/foo"` and +/// `ext_dir.join(id)` on Windows happily resolves that outside the +/// extensions directory — `git clone` would then write to the escaped +/// location instead of the sandboxed app-data subfolder. +fn validate_extension_id(id: &str) -> Result<(), String> { + if id.is_empty() { + return Err("extension id is empty".to_string()); + } + if id == "." || id == ".." { + return Err(format!( + "extension id `{}` is not a valid directory name", + id + )); + } + if id.contains('/') || id.contains('\\') || id.contains("..") { + return Err(format!( + "extension id `{}` cannot contain path separators or `..`", + id + )); + } + if id.starts_with('-') { + return Err(format!("extension id `{}` cannot start with `-`", id)); + } + if !id + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_' || c == '.') + { + return Err(format!( + "extension id `{}` contains disallowed characters (allowed: ASCII alphanumeric, `-`, `_`, `.`)", + id + )); + } + Ok(()) +} + #[tauri::command] pub async fn install_extension(app: AppHandle, id: String, git_url: String) -> Result<(), String> { + // Validate id and git_url before we even build the target directory so a + // bad input never reaches `git clone` or the filesystem. + validate_extension_id(&id)?; + validate_git_clone_url(&git_url)?; + let ext_dir = get_extensions_dir(&app)?; let target_dir = ext_dir.join(&id); @@ -275,7 +398,18 @@ pub async fn install_extension(app: AppHandle, id: String, git_url: String) -> R return Err("Extension is already installed".into()); } + // Belt-and-suspenders: even though `validate_git_clone_url` already + // enforces an https:// allow-listed host, turn off the protocols that are + // most commonly abused for arbitrary command execution during clone. If + // anything ever gets past the URL check, git itself will still refuse + // `ext::`, `file://`, and raw `git://`. let output = Command::new("git") + .arg("-c") + .arg("protocol.ext.allow=never") + .arg("-c") + .arg("protocol.file.allow=never") + .arg("-c") + .arg("protocol.git.allow=never") .arg("clone") .arg("--depth") .arg("1") @@ -295,6 +429,7 @@ pub async fn install_extension(app: AppHandle, id: String, git_url: String) -> R #[tauri::command] pub async fn uninstall_extension(app: AppHandle, id: String) -> Result<(), String> { + validate_extension_id(&id)?; let ext_dir = get_extensions_dir(&app)?; let target_dir = ext_dir.join(&id); @@ -307,6 +442,7 @@ pub async fn uninstall_extension(app: AppHandle, id: String) -> Result<(), Strin #[tauri::command] pub async fn update_extension(app: AppHandle, id: String) -> Result<(), String> { + validate_extension_id(&id)?; let ext_dir = get_extensions_dir(&app)?; let target_dir = ext_dir.join(&id); @@ -353,6 +489,7 @@ pub async fn get_installed_extensions(app: AppHandle) -> Result, Str #[tauri::command] pub async fn is_extension_active(app: AppHandle, id: String) -> Result { + validate_extension_id(&id)?; let app_data = app.path().app_data_dir().map_err(|e| e.to_string())?; let disabled_file = app_data.join("disabled_extensions.json"); @@ -372,6 +509,7 @@ pub async fn toggle_extension_state( id: String, is_active: bool, ) -> Result<(), String> { + validate_extension_id(&id)?; let app_data = app.path().app_data_dir().map_err(|e| e.to_string())?; let disabled_file = app_data.join("disabled_extensions.json"); @@ -396,6 +534,7 @@ pub async fn toggle_extension_state( #[tauri::command] pub async fn read_extension_script(app: AppHandle, id: String) -> Result { + validate_extension_id(&id)?; let ext_dir = get_extensions_dir(&app)?; let target_dir = ext_dir.join(&id); let index_file = target_dir.join("index.js"); @@ -407,3 +546,112 @@ pub async fn read_extension_script(app: AppHandle, id: String) -> Result