fc-common: allow configuring url schemes to allow for testing
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I99912d7c45f1a4664d4823ddd793b5af6a6a6964
This commit is contained in:
parent
861eda231f
commit
a5768d46eb
4 changed files with 129 additions and 34 deletions
|
|
@ -38,15 +38,18 @@ pub struct DatabaseConfig {
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
#[serde(default)]
|
#[serde(default)]
|
||||||
pub struct ServerConfig {
|
pub struct ServerConfig {
|
||||||
pub host: String,
|
pub host: String,
|
||||||
pub port: u16,
|
pub port: u16,
|
||||||
pub request_timeout: u64,
|
pub request_timeout: u64,
|
||||||
pub max_body_size: usize,
|
pub max_body_size: usize,
|
||||||
pub api_key: Option<String>,
|
pub api_key: Option<String>,
|
||||||
pub allowed_origins: Vec<String>,
|
pub allowed_origins: Vec<String>,
|
||||||
pub cors_permissive: bool,
|
pub cors_permissive: bool,
|
||||||
pub rate_limit_rps: Option<u64>,
|
pub rate_limit_rps: Option<u64>,
|
||||||
pub rate_limit_burst: Option<u32>,
|
pub rate_limit_burst: Option<u32>,
|
||||||
|
/// Allowed URL schemes for repository URLs. Insecure schemes emit a warning
|
||||||
|
/// on startup
|
||||||
|
pub allowed_url_schemes: Vec<String>,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||||
|
|
@ -472,15 +475,21 @@ impl DatabaseConfig {
|
||||||
impl Default for ServerConfig {
|
impl Default for ServerConfig {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self {
|
Self {
|
||||||
host: "127.0.0.1".to_string(),
|
host: "127.0.0.1".to_string(),
|
||||||
port: 3000,
|
port: 3000,
|
||||||
request_timeout: 30,
|
request_timeout: 30,
|
||||||
max_body_size: 10 * 1024 * 1024, // 10MB
|
max_body_size: 10 * 1024 * 1024, // 10MB
|
||||||
api_key: None,
|
api_key: None,
|
||||||
allowed_origins: Vec::new(),
|
allowed_origins: Vec::new(),
|
||||||
cors_permissive: false,
|
cors_permissive: false,
|
||||||
rate_limit_rps: None,
|
rate_limit_rps: None,
|
||||||
rate_limit_burst: None,
|
rate_limit_burst: None,
|
||||||
|
allowed_url_schemes: vec![
|
||||||
|
"https".into(),
|
||||||
|
"http".into(),
|
||||||
|
"git".into(),
|
||||||
|
"ssh".into(),
|
||||||
|
],
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -30,8 +30,9 @@ static COMMIT_HASH_RE: LazyLock<Regex> =
|
||||||
static SYSTEM_RE: LazyLock<Regex> =
|
static SYSTEM_RE: LazyLock<Regex> =
|
||||||
LazyLock::new(|| Regex::new(r"^\w+-\w+$").unwrap());
|
LazyLock::new(|| Regex::new(r"^\w+-\w+$").unwrap());
|
||||||
|
|
||||||
const VALID_REPO_PREFIXES: &[&str] =
|
/// Schemes considered insecure for repository URLs.
|
||||||
&["https://", "http://", "git://", "ssh://", "file://"];
|
const INSECURE_SCHEMES: &[&str] = &["file", "http"];
|
||||||
|
|
||||||
const VALID_FORGE_TYPES: &[&str] = &["github", "gitea", "forgejo", "gitlab"];
|
const VALID_FORGE_TYPES: &[&str] = &["github", "gitea", "forgejo", "gitlab"];
|
||||||
|
|
||||||
/// Known internal/metadata IP ranges and hostnames to block for SSRF
|
/// Known internal/metadata IP ranges and hostnames to block for SSRF
|
||||||
|
|
@ -61,7 +62,7 @@ fn extract_host_from_url(url: &str) -> Option<String> {
|
||||||
|
|
||||||
/// Check if a hostname is internal/metadata (SSRF targets).
|
/// Check if a hostname is internal/metadata (SSRF targets).
|
||||||
fn is_internal_host(host: &str) -> bool {
|
fn is_internal_host(host: &str) -> bool {
|
||||||
if INTERNAL_HOSTS.iter().any(|&h| host == h) {
|
if INTERNAL_HOSTS.contains(&host) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
// Block localhost variants
|
// Block localhost variants
|
||||||
|
|
@ -121,17 +122,9 @@ fn validate_repository_url(url: &str) -> Result<(), String> {
|
||||||
if url.len() > 2048 {
|
if url.len() > 2048 {
|
||||||
return Err("repository_url must be at most 2048 characters".to_string());
|
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(
|
return Err(
|
||||||
"repository_url must start with https://, http://, git://, ssh://, or \
|
"repository_url must contain a valid URL scheme (e.g. https://)"
|
||||||
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"
|
|
||||||
.to_string(),
|
.to_string(),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
@ -147,6 +140,46 @@ fn validate_repository_url(url: &str) -> Result<(), String> {
|
||||||
Ok(())
|
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::<Vec<_>>()
|
||||||
|
.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> {
|
fn validate_description(desc: &str) -> Result<(), String> {
|
||||||
if desc.len() > 4096 {
|
if desc.len() > 4096 {
|
||||||
return Err("description must be at most 4096 characters".to_string());
|
return Err("description must be at most 4096 characters".to_string());
|
||||||
|
|
@ -544,10 +577,11 @@ mod tests {
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_create_project_invalid_url() {
|
fn test_create_project_invalid_url() {
|
||||||
|
// URL without scheme separator is rejected structurally
|
||||||
let p = CreateProject {
|
let p = CreateProject {
|
||||||
name: "valid-name".to_string(),
|
name: "valid-name".to_string(),
|
||||||
description: None,
|
description: None,
|
||||||
repository_url: "ftp://example.com".to_string(),
|
repository_url: "not-a-url".to_string(),
|
||||||
};
|
};
|
||||||
assert!(p.validate().is_err());
|
assert!(p.validate().is_err());
|
||||||
}
|
}
|
||||||
|
|
@ -741,8 +775,39 @@ mod tests {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_repository_url_rejects_file_scheme() {
|
fn test_validate_url_scheme_rejects_file_by_default() {
|
||||||
assert!(validate_repository_url("file:///etc/passwd").is_err());
|
let default_schemes: Vec<String> = 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<String> = 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<String> =
|
||||||
|
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]
|
#[test]
|
||||||
|
|
|
||||||
|
|
@ -51,6 +51,10 @@ async fn main() -> anyhow::Result<()> {
|
||||||
let host = cli.host.unwrap_or(config.server.host.clone());
|
let host = cli.host.unwrap_or(config.server.host.clone());
|
||||||
let port = cli.port.unwrap_or(config.server.port);
|
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?;
|
let db = Database::new(config.database.clone()).await?;
|
||||||
|
|
||||||
// Bootstrap declarative projects, jobsets, and API keys from config
|
// Bootstrap declarative projects, jobsets, and API keys from config
|
||||||
|
|
|
||||||
|
|
@ -62,6 +62,11 @@ async fn create_project(
|
||||||
input
|
input
|
||||||
.validate()
|
.validate()
|
||||||
.map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?;
|
.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)
|
let project = fc_common::repo::projects::create(&state.pool, input)
|
||||||
.await
|
.await
|
||||||
.map_err(ApiError)?;
|
.map_err(ApiError)?;
|
||||||
|
|
@ -87,6 +92,13 @@ async fn update_project(
|
||||||
input
|
input
|
||||||
.validate()
|
.validate()
|
||||||
.map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?;
|
.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)
|
let project = fc_common::repo::projects::update(&state.pool, id, input)
|
||||||
.await
|
.await
|
||||||
.map_err(ApiError)?;
|
.map_err(ApiError)?;
|
||||||
|
|
@ -231,6 +243,11 @@ async fn setup_project(
|
||||||
create_project
|
create_project
|
||||||
.validate()
|
.validate()
|
||||||
.map_err(|msg| ApiError(fc_common::CiError::Validation(msg)))?;
|
.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)
|
let project = fc_common::repo::projects::create(&state.pool, create_project)
|
||||||
.await
|
.await
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue