Page MenuHomeDevCentral
Files F42908304

D4151.diff
No OneTemporary
Actions

D4151.diff
View Options

diff --git a/Cargo.toml b/Cargo.toml
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -8,6 +8,7 @@
[dependencies]
axum = "0.8.9"
env_logger = "^0.11.11"
+idna = "1.1.0"
lazy_static = "^1.5.0"
limiting-factor-axum = "0.2.0"
log = "^0.4.33"
diff --git a/README.md b/README.md
--- a/README.md
+++ b/README.md
@@ -21,6 +21,9 @@
Both modes will run the same action code.
+Site names use hostname syntax. Internationalized domain names can be passed
+as Unicode (for example, `dæghrefn.nasqueron.org`) or in their punycode form.
+
## Usage
### Alkane update
diff --git a/src/command.rs b/src/command.rs
--- a/src/command.rs
+++ b/src/command.rs
@@ -7,6 +7,8 @@
use clap::{Args, Parser};
+use crate::services::validation::is_valid_site_name;
+
// -------------------------------------------------------------
// Main command
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -42,6 +44,7 @@
#[derive(Debug, Args)]
pub struct DeployArgs {
/// The name of the site to deploy, using sub.domain.tld format
+ #[arg(value_parser = parse_site_name)]
pub site_name: String,
/// The artifact to deploy. Allows CD to give metadata or a URL to download last artifact
@@ -54,9 +57,18 @@
pub quiet: bool,
/// The name of the site to deploy, using sub.domain.tld format
+ #[arg(value_parser = parse_site_name)]
pub site_name: String,
}
+fn parse_site_name(site_name: &str) -> Result<String, String> {
+ if is_valid_site_name(site_name) {
+ Ok(site_name.to_string())
+ } else {
+ Err("site name must be a valid hostname".to_string())
+ }
+}
+
// -------------------------------------------------------------
// Helper methods
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
diff --git a/src/config.rs b/src/config.rs
--- a/src/config.rs
+++ b/src/config.rs
@@ -16,6 +16,7 @@
use crate::runner::site::Site;
use crate::services::tld::extract_domain_parts;
+use crate::services::validation::is_valid_site_name;
// -------------------------------------------------------------
// Constants:
@@ -98,6 +99,10 @@
}
pub fn get_site_path(&self, site_name: &str) -> Option<String> {
+ if !is_valid_site_name(site_name) {
+ return None;
+ }
+
let root = self.get_root("sites")?;
let root = root.replace("/", MAIN_SEPARATOR_STR);
@@ -188,6 +193,13 @@
assert_eq!(expected, config.get_site_path("foo.example.org"));
}
+ #[test]
+ pub fn test_get_site_path_rejects_traversal() {
+ let config = AlkaneConfig::load().unwrap();
+
+ assert_eq!(None, config.get_site_path("../example.org"));
+ }
+
#[test]
pub fn test_contains_domain_parts_variables() {
assert_eq!(
diff --git a/src/db.rs b/src/db.rs
--- a/src/db.rs
+++ b/src/db.rs
@@ -13,6 +13,7 @@
use tokio::fs::{self, OpenOptions};
use crate::config::AlkaneConfig;
+use crate::services::validation::is_valid_site_name;
pub struct Database {
root: String,
@@ -33,11 +34,18 @@
}
pub fn is_initialized(&self, site_name: &str) -> bool {
- self.get_initialized_path(site_name).exists()
+ self.get_initialized_path(site_name)
+ .is_some_and(|path| path.exists())
}
pub async fn set_initialized(&self, site_name: &str) -> bool {
- let path = self.get_initialized_path(site_name);
+ let Some(path) = self.get_initialized_path(site_name) else {
+ warn!(
+ "Can't mark invalid site name {:?} as initialized",
+ site_name
+ );
+ return false;
+ };
match fs::try_exists(&path).await {
Ok(false) => match ensure_parent_directory_exists(&path).await {
@@ -67,8 +75,9 @@
}
}
- fn get_initialized_path(&self, site_name: &str) -> PathBuf {
- Path::new(&self.root).join("initialized").join(site_name)
+ fn get_initialized_path(&self, site_name: &str) -> Option<PathBuf> {
+ is_valid_site_name(site_name)
+ .then(|| Path::new(&self.root).join("initialized").join(site_name))
}
}
@@ -99,6 +108,13 @@
mod tests {
use super::*;
+ #[test]
+ pub fn test_get_initialized_path_rejects_traversal() {
+ let database = Database::new("tests/data/db");
+
+ assert_eq!(None, database.get_initialized_path("../example.org"));
+ }
+
#[tokio::test]
pub async fn test_touch() {
let path = Path::new("tmp-touch.empty");
diff --git a/src/runner/store.rs b/src/runner/store.rs
--- a/src/runner/store.rs
+++ b/src/runner/store.rs
@@ -8,10 +8,13 @@
use std::collections::HashMap;
use std::path::Path;
+use log::error;
+
use crate::config::AlkaneConfig;
use crate::runner::run;
use crate::runner::site::Site;
use crate::runner::RecipeStatus;
+use crate::services::validation::is_valid_site_name;
pub struct RecipesStore {
root: String,
@@ -31,17 +34,26 @@
config.get_root("recipes").map(Self::new)
}
- fn get_recipe_path(&self, site_name: &str, action: &str) -> String {
+ fn get_recipe_path(&self, site_name: &str, action: &str) -> Option<String> {
+ if !is_valid_site_name(site_name) {
+ return None;
+ }
+
Path::new(&self.root)
.join(site_name)
.join(action)
.to_str()
- .expect("Can't read recipe path as UTF-8")
- .to_string()
+ .map(str::to_string)
}
pub async fn run_recipe(&self, site: &Site, action: &str) -> RecipeStatus {
- let command = self.get_recipe_path(&site.name, action);
+ let Some(command) = self.get_recipe_path(&site.name, action) else {
+ error!(
+ "Refusing to run a recipe for invalid site name {:?}",
+ site.name
+ );
+ return RecipeStatus::Error;
+ };
let environment = self.get_environment(&site);
run(command, Vec::new(), environment).await
@@ -81,7 +93,29 @@
let test_store_path = "tests/data/recipes".replace("/", MAIN_SEPARATOR_STR);
let store = RecipesStore::new(&test_store_path);
- assert_eq!(expected, store.get_recipe_path("foo.acme.tld", "update"));
+ assert_eq!(
+ Some(expected),
+ store.get_recipe_path("foo.acme.tld", "update")
+ );
+ }
+
+ #[test]
+ pub fn test_get_recipe_path_rejects_traversal() {
+ let store = RecipesStore::new("tests/data/recipes");
+
+ assert_eq!(None, store.get_recipe_path("../foo.acme.tld", "update"));
+ }
+
+ #[test]
+ pub fn test_get_recipe_path_preserves_idn() {
+ let expected =
+ "tests/data/recipes/dæghrefn.nasqueron.org/update".replace("/", MAIN_SEPARATOR_STR);
+ let store = RecipesStore::new("tests/data/recipes".replace("/", MAIN_SEPARATOR_STR));
+
+ assert_eq!(
+ Some(expected),
+ store.get_recipe_path("dæghrefn.nasqueron.org", "update")
+ );
}
#[test]
diff --git a/src/server/requests.rs b/src/server/requests.rs
--- a/src/server/requests.rs
+++ b/src/server/requests.rs
@@ -7,15 +7,17 @@
use axum::extract::{Path, State};
use axum::http::StatusCode;
+use axum::Json;
use limiting_factor_axum::api::guards::AxumRequestBody as RequestBody;
use limiting_factor_axum::api::replies::{ApiJsonResponse, ApiResponse, FailureResponse};
-
use log::{debug, info, warn};
+
use crate::actions;
use crate::config::AlkaneConfig;
use crate::deploy::DeployError;
use crate::runner::RecipeStatus;
+use crate::services::validation::is_valid_site_name;
// -------------------------------------------------------------
// Monitoring
@@ -33,6 +35,10 @@
Path(site_name): Path<String>,
State(config): State<AlkaneConfig>,
) -> ApiJsonResponse<bool> {
+ if !is_valid_site_name(&site_name) {
+ return invalid_site_name_response(&site_name);
+ }
+
actions::is_present(&site_name, &config).into_json_response()
}
@@ -41,6 +47,10 @@
State(config): State<AlkaneConfig>,
context: RequestBody,
) -> ApiJsonResponse<RecipeStatus> {
+ if !is_valid_site_name(&site_name) {
+ return invalid_site_name_response(&site_name);
+ }
+
info!("Deploying {}", &site_name);
let context = context.into_optional_string();
@@ -56,6 +66,10 @@
State(config): State<AlkaneConfig>,
context: RequestBody,
) -> ApiJsonResponse<RecipeStatus> {
+ if !is_valid_site_name(&site_name) {
+ return invalid_site_name_response(&site_name);
+ }
+
info!("Deploying {}", &site_name);
let context = context.into_optional_string();
@@ -71,6 +85,10 @@
State(config): State<AlkaneConfig>,
context: RequestBody,
) -> ApiJsonResponse<RecipeStatus> {
+ if !is_valid_site_name(&site_name) {
+ return invalid_site_name_response(&site_name);
+ }
+
info!("Deploying {}", &site_name);
let context = context.into_optional_string();
@@ -81,6 +99,15 @@
.into_json_response()
}
+fn invalid_site_name_response<T>(site_name: &str) -> ApiJsonResponse<T> {
+ warn!("Invalid site name requested: {:?}", site_name);
+
+ Err((
+ StatusCode::BAD_REQUEST,
+ Json("Invalid site name".to_string()),
+ ))
+}
+
// -------------------------------------------------------------
// Custom error handling
//
@@ -93,7 +120,25 @@
}
fn response(&self) -> String {
- warn!("{}", self); // Server log
- format!("{}", self) // API response
+ warn!("{}", self); // Server log
+ format!("{}", self) // API response
+ }
+}
+
+// -------------------------------------------------------------
+// Tests
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[tokio::test]
+ async fn invalid_site_name_returns_bad_request() {
+ let config = AlkaneConfig::load().unwrap();
+
+ let response = is_present(Path("../example.org".to_string()), State(config)).await;
+
+ assert_eq!(StatusCode::BAD_REQUEST, response.unwrap_err().0);
}
}
diff --git a/src/services/mod.rs b/src/services/mod.rs
--- a/src/services/mod.rs
+++ b/src/services/mod.rs
@@ -6,3 +6,4 @@
// -------------------------------------------------------------
pub mod tld;
+pub mod validation;
diff --git a/src/services/validation.rs b/src/services/validation.rs
new file mode 100644
--- /dev/null
+++ b/src/services/validation.rs
@@ -0,0 +1,60 @@
+// -------------------------------------------------------------
+// Alkane :: Services :: Validation
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+// Project: Nasqueron
+// License: BSD-2-Clause
+// Description: Input validation for site names
+// -------------------------------------------------------------
+
+use idna::domain_to_ascii_strict;
+
+/// Determines whether a site name has valid hostname syntax.
+///
+/// Internationalized domain names can be supplied as Unicode or punycode.
+/// Validation uses their ASCII representation for DNS label and length limits,
+/// but the original spelling remains the site name used for filesystem paths.
+pub fn is_valid_site_name(site_name: &str) -> bool {
+ if site_name.is_empty()
+ || site_name.ends_with('.')
+ || site_name.contains('/')
+ || site_name.contains('\\')
+ {
+ return false;
+ }
+
+ domain_to_ascii_strict(site_name).is_ok_and(|ascii_site_name| ascii_site_name.len() <= 253)
+}
+
+// -------------------------------------------------------------
+// Tests
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn accepts_ascii_hostnames_and_idns() {
+ assert!(is_valid_site_name("foo.example.org"));
+ assert!(is_valid_site_name("EXAMPLE.org"));
+ assert!(is_valid_site_name("bücher.example"));
+ assert!(is_valid_site_name("xn--bcher-kva.example"));
+ }
+
+ #[test]
+ fn rejects_invalid_hostname_syntax() {
+ assert!(!is_valid_site_name(""));
+ assert!(!is_valid_site_name("example..org"));
+ assert!(!is_valid_site_name("-example.org"));
+ assert!(!is_valid_site_name("example-.org"));
+ assert!(!is_valid_site_name("example_org"));
+ assert!(!is_valid_site_name("example.org."));
+ assert!(!is_valid_site_name("hello world.example"));
+ assert!(!is_valid_site_name("../example.org"));
+ assert!(!is_valid_site_name("/example.org"));
+ assert!(!is_valid_site_name("example.org/../foo"));
+ assert!(!is_valid_site_name(r"example.org\foo"));
+ assert!(!is_valid_site_name(&format!("{}.org", "a".repeat(64))));
+ assert!(!is_valid_site_name(&"a".repeat(254)));
+ }
+}

File Metadata

Mime Type
text/plain
Expires
Mon, Aug 10, 22:32 (21 h, 42 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
3983416
Default Alt Text
D4151.diff (12 KB)

Event Timeline

Nasqueron DevCentral · If it had been much bigger the moon would have had a core of ice. · Powered by Phabricator