diff --git a/crates/common/src/config.rs b/crates/common/src/config.rs index af11af8..721130d 100644 --- a/crates/common/src/config.rs +++ b/crates/common/src/config.rs @@ -38,15 +38,18 @@ pub struct DatabaseConfig { #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(default)] pub struct ServerConfig { - pub host: String, - pub port: u16, - pub request_timeout: u64, - pub max_body_size: usize, - pub api_key: Option, - pub allowed_origins: Vec, - pub cors_permissive: bool, - pub rate_limit_rps: Option, - pub rate_limit_burst: Option, + pub host: String, + pub port: u16, + pub request_timeout: u64, + pub max_body_size: usize, + pub api_key: Option, + pub allowed_origins: Vec, + pub cors_permissive: bool, + pub rate_limit_rps: Option, + pub rate_limit_burst: Option, + /// Allowed URL schemes for repository URLs. Insecure schemes emit a warning + /// on startup + pub allowed_url_schemes: Vec, } #[derive(Debug, Clone, Serialize, Deserialize)] @@ -472,15 +475,21 @@ impl DatabaseConfig { impl Default for ServerConfig { fn default() -> Self { Self { - host: "127.0.0.1".to_string(), - port: 3000, - request_timeout: 30, - max_body_size: 10 * 1024 * 1024, // 10MB - api_key: None, - allowed_origins: Vec::new(), - cors_permissive: false, - rate_limit_rps: None, - rate_limit_burst: None, + host: "127.0.0.1".to_string(), + port: 3000, + request_timeout: 30, + max_body_size: 10 * 1024 * 1024, // 10MB + api_key: None, + allowed_origins: Vec::new(), + cors_permissive: false, + rate_limit_rps: None, + rate_limit_burst: None, + allowed_url_schemes: vec![ + "https".into(), + "http".into(), + "git".into(), + "ssh".into(), + ], } } } diff --git a/crates/common/src/validate.rs b/crates/common/src/validate.rs index 7c38a6e..cacc5fa 100644 --- a/crates/common/src/validate.rs +++ b/crates/common/src/validate.rs @@ -30,8 +30,9 @@ static COMMIT_HASH_RE: LazyLock = static SYSTEM_RE: LazyLock = LazyLock::new(|| Regex::new(r"^\w+-\w+$").unwrap()); -const VALID_REPO_PREFIXES: &[&str] = - &["https://", "http://", "git://", "ssh://", "file://"]; +/// Schemes considered insecure for repository URLs. +const INSECURE_SCHEMES: &[&str] = &["file", "http"]; + const VALID_FORGE_TYPES: &[&str] = &["github", "gitea", "forgejo", "gitlab"]; /// Known internal/metadata IP ranges and hostnames to block for SSRF @@ -61,7 +62,7 @@ fn extract_host_from_url(url: &str) -> Option { /// Check if a hostname is internal/metadata (SSRF targets). fn is_internal_host(host: &str) -> bool { - if INTERNAL_HOSTS.iter().any(|&h| host == h) { + if INTERNAL_HOSTS.contains(&host) { return true; } // Block localhost variants @@ -121,17 +122,9 @@ fn validate_repository_url(url: &str) -> Result<(), String> { if url.len() > 2048 { return Err("repository_url must be at most 2048 characters".to_string()); } - if !VALID_REPO_PREFIXES.iter().any(|p| url.starts_with(p)) { + if !url.contains("://") { return Err( - "repository_url must start with https://, http://, git://, ssh://, or \ - file://" - .to_string(), - ); - } - // Reject file:// URLs for SSRF protection (local filesystem access) - if url.starts_with("file://") { - return Err( - "repository_url must not use file:// scheme for security reasons" + "repository_url must contain a valid URL scheme (e.g. https://)" .to_string(), ); } @@ -147,6 +140,46 @@ fn validate_repository_url(url: &str) -> Result<(), String> { Ok(()) } +/// Validate that a URL uses one of the allowed schemes. +/// Logs a warning when insecure schemes (`file`, `http`) are used. +pub fn validate_url_scheme( + url: &str, + allowed_schemes: &[String], +) -> Result<(), String> { + let scheme = url.split("://").next().unwrap_or(""); + if !allowed_schemes.iter().any(|s| s == scheme) { + return Err(format!( + "repository_url scheme '{scheme}://' is not allowed. Allowed schemes: {}", + allowed_schemes + .iter() + .map(|s| format!("{s}://")) + .collect::>() + .join(", ") + )); + } + if INSECURE_SCHEMES.contains(&scheme) { + tracing::warn!( + url = url, + scheme = scheme, + "Repository URL uses insecure scheme" + ); + } + Ok(()) +} + +/// Log warnings at startup for any insecure schemes in the allowed list. +pub fn warn_insecure_schemes(allowed_schemes: &[String]) { + for scheme in allowed_schemes { + if INSECURE_SCHEMES.contains(&scheme.as_str()) { + tracing::warn!( + scheme = scheme.as_str(), + "Insecure URL scheme '{scheme}://' is enabled in \ + server.allowed_url_schemes" + ); + } + } +} + fn validate_description(desc: &str) -> Result<(), String> { if desc.len() > 4096 { return Err("description must be at most 4096 characters".to_string()); @@ -544,10 +577,11 @@ mod tests { #[test] fn test_create_project_invalid_url() { + // URL without scheme separator is rejected structurally let p = CreateProject { name: "valid-name".to_string(), description: None, - repository_url: "ftp://example.com".to_string(), + repository_url: "not-a-url".to_string(), }; assert!(p.validate().is_err()); } @@ -741,8 +775,39 @@ mod tests { } #[test] - fn test_repository_url_rejects_file_scheme() { - assert!(validate_repository_url("file:///etc/passwd").is_err()); + fn test_validate_url_scheme_rejects_file_by_default() { + let default_schemes: Vec = vec!["https", "http", "git", "ssh"] + .into_iter() + .map(Into::into) + .collect(); + assert!( + validate_url_scheme("file:///etc/passwd", &default_schemes).is_err() + ); + } + + #[test] + fn test_validate_url_scheme_allows_file_when_configured() { + let schemes: Vec = vec!["https", "http", "git", "ssh", "file"] + .into_iter() + .map(Into::into) + .collect(); + assert!(validate_url_scheme("file:///var/lib/repo.git", &schemes).is_ok()); + } + + #[test] + fn test_validate_url_scheme_rejects_unknown() { + let schemes: Vec = + vec!["https", "ssh"].into_iter().map(Into::into).collect(); + assert!( + validate_url_scheme("ftp://example.com/repo.git", &schemes).is_err() + ); + } + + #[test] + fn test_repository_url_accepts_file_structurally() { + // validate_repository_url no longer checks schemes (that's + // validate_url_scheme's job) + assert!(validate_repository_url("file:///etc/passwd").is_ok()); } #[test] diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 294921c..ac0ac70 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -51,6 +51,10 @@ async fn main() -> anyhow::Result<()> { let host = cli.host.unwrap_or(config.server.host.clone()); let port = cli.port.unwrap_or(config.server.port); + fc_common::validate::warn_insecure_schemes( + &config.server.allowed_url_schemes, + ); + let db = Database::new(config.database.clone()).await?; // Bootstrap declarative projects, jobsets, and API keys from config diff --git a/crates/server/src/routes/projects.rs b/crates/server/src/routes/projects.rs index 965821f..04b27e7 100644 --- a/crates/server/src/routes/projects.rs +++ b/crates/server/src/routes/projects.rs @@ -62,6 +62,11 @@ async fn create_project( input .validate() .map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?; + fc_common::validate::validate_url_scheme( + &input.repository_url, + &state.config.server.allowed_url_schemes, + ) + .map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?; let project = fc_common::repo::projects::create(&state.pool, input) .await .map_err(ApiError)?; @@ -87,6 +92,13 @@ async fn update_project( input .validate() .map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?; + if let Some(ref url) = input.repository_url { + fc_common::validate::validate_url_scheme( + url, + &state.config.server.allowed_url_schemes, + ) + .map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?; + } let project = fc_common::repo::projects::update(&state.pool, id, input) .await .map_err(ApiError)?; @@ -231,6 +243,11 @@ async fn setup_project( create_project .validate() .map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?; + fc_common::validate::validate_url_scheme( + &create_project.repository_url, + &state.config.server.allowed_url_schemes, + ) + .map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?; let project = fc_common::repo::projects::create(&state.pool, create_project) .await