Validate URL params before joining filesystem paths
Both handlers fed URL-derived repo and run_id directly into Path::join.
Validate at handler entry — repo via Quire::repo's name check, run_id
via uuid::Uuid::parse_str — and 404 on either failure. Also gate the
DB-sourced job_id at the log-read site so a malicious or corrupt row
can't reach the filesystem.
Assisted-by: Claude Opus 4.7 via Claude Code
diff --git a/src/quire/web/db.rs b/src/quire/web/db.rs
index ff7c362..6a4c052 100644
--- a/src/quire/web/db.rs
+++ b/src/quire/web/db.rs
@@ -157,3 +157,59 @@ pub fn resolve_repo_name(slug: &str) -> String {
format!("{slug}.git")
}
}
+
+/// True if the given run_id parses as a UUID.
+///
+/// CI runs are assigned UUIDv7 ids at creation time. Anything else
+/// reaching the web layer is either a typo or a probe.
+pub fn is_valid_run_id(s: &str) -> bool {
+ uuid::Uuid::parse_str(s).is_ok()
+}
+
+/// True if the given string is safe to use as a single filesystem path
+/// component.
+///
+/// Rejects empty strings, path separators, NUL, and the `.`/`..` traversal
+/// segments. Used to gate DB-sourced job ids before they touch `Path::join`.
+pub fn is_safe_path_segment(s: &str) -> bool {
+ !s.is_empty()
+ && !s.contains('/')
+ && !s.contains('\\')
+ && !s.contains('\0')
+ && s != "."
+ && s != ".."
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn run_id_accepts_uuid() {
+ assert!(is_valid_run_id("0194f3a5-2b3c-7000-8000-000000000000"));
+ }
+
+ #[test]
+ fn run_id_rejects_traversal() {
+ assert!(!is_valid_run_id("../etc/passwd"));
+ assert!(!is_valid_run_id(""));
+ assert!(!is_valid_run_id("foo"));
+ }
+
+ #[test]
+ fn safe_path_segment_accepts_normal_names() {
+ assert!(is_safe_path_segment("build"));
+ assert!(is_safe_path_segment("test-job"));
+ assert!(is_safe_path_segment("job_1"));
+ }
+
+ #[test]
+ fn safe_path_segment_rejects_traversal_and_separators() {
+ assert!(!is_safe_path_segment(""));
+ assert!(!is_safe_path_segment("."));
+ assert!(!is_safe_path_segment(".."));
+ assert!(!is_safe_path_segment("a/b"));
+ assert!(!is_safe_path_segment("a\\b"));
+ assert!(!is_safe_path_segment("a\0b"));
+ }
+}
diff --git a/src/quire/web/handlers.rs b/src/quire/web/handlers.rs
index a2b6893..38db969 100644
--- a/src/quire/web/handlers.rs
+++ b/src/quire/web/handlers.rs
@@ -47,6 +47,9 @@ pub async fn run_list(
let _user = user;
let repo_display = repo.trim_end_matches(".git").to_string();
let repo_name = db::resolve_repo_name(&repo);
+ if quire.repo(&repo_name).is_err() {
+ return StatusCode::NOT_FOUND.into_response();
+ }
let runs = match db::load_runs(&quire, &repo_name) {
Ok(r) => r,
@@ -90,6 +93,9 @@ pub async fn run_detail(
let _user = user;
let repo_display = repo.trim_end_matches(".git").to_string();
let repo_name = db::resolve_repo_name(&repo);
+ if quire.repo(&repo_name).is_err() || !db::is_valid_run_id(&run_id) {
+ return StatusCode::NOT_FOUND.into_response();
+ }
let result = db::load_run_detail(&quire, &repo_name, &run_id);
let (run, jobs, sh_events) = match result {
@@ -119,6 +125,10 @@ pub async fn run_detail(
let mut log_contents: std::collections::HashMap<(String, usize), String> =
std::collections::HashMap::new();
for (idx, ev) in sh_events.iter().enumerate() {
+ if !db::is_safe_path_segment(&ev.job_id) {
+ tracing::warn!(job_id = %ev.job_id, "skipping CRI log read for unsafe job_id");
+ continue;
+ }
let sh_n = db::sh_index_for_event(&sh_events, &ev.job_id, idx);
let key = (ev.job_id.clone(), sh_n);
if log_contents.contains_key(&key) {