From 72d79deec9ba203c67aba8fdf26f90b1b3b1c695 Mon Sep 17 00:00:00 2001 From: dave Date: Thu, 14 May 2026 12:53:14 +0000 Subject: [PATCH] huskies: merge 1026 --- Cargo.lock | 179 +++++++ Cargo.toml | 3 + server/Cargo.toml | 3 + server/src/http/mcp/story_tools/epic.rs | 51 +- .../src/http/mcp/story_tools/story/create.rs | 129 +++-- server/src/http/workflow/bug_ops/epic.rs | 102 ++-- server/src/http/workflow/utils.rs | 12 +- server/src/main.rs | 2 + server/src/validation/error.rs | 127 +++++ server/src/validation/mod.rs | 22 + server/src/validation/newtypes.rs | 397 +++++++++++++++ server/src/validation/requests.rs | 476 ++++++++++++++++++ server/src/validation/sanitize.rs | 67 +++ 13 files changed, 1443 insertions(+), 127 deletions(-) create mode 100644 server/src/validation/error.rs create mode 100644 server/src/validation/mod.rs create mode 100644 server/src/validation/newtypes.rs create mode 100644 server/src/validation/requests.rs create mode 100644 server/src/validation/sanitize.rs diff --git a/Cargo.lock b/Cargo.lock index f03c77fc..dc76921f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -65,6 +65,19 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" +[[package]] +name = "ammonia" +version = "4.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "17e913097e1a2124b46746c980134e8c954bc17a6a59bb3fde96f088d126dde6" +dependencies = [ + "cssparser", + "html5ever", + "maplit", + "tendril", + "url", +] + [[package]] name = "android_system_properties" version = "0.1.5" @@ -417,6 +430,15 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5" +[[package]] +name = "castaway" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dec551ab6e7578819132c713a93c022a05d60159dc86e7a7050223577484c55a" +dependencies = [ + "rustversion", +] + [[package]] name = "cbc" version = "0.1.2" @@ -601,6 +623,20 @@ dependencies = [ "memchr", ] +[[package]] +name = "compact_str" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b79c4069c6cad78e2e0cdfcbd26275770669fb39fd308a752dc110e83b9af32" +dependencies = [ + "castaway", + "cfg-if", + "itoa", + "rustversion", + "ryu", + "static_assertions", +] + [[package]] name = "compression-codecs" version = "0.4.38" @@ -654,6 +690,15 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d52eff69cd5e647efe296129160853a42795992097e8af39800e1060caeea9b" +[[package]] +name = "convert_case" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "affbf0190ed2caf063e3def54ff444b449371d55c58e513a95ab98eca50adb49" +dependencies = [ + "unicode-segmentation", +] + [[package]] name = "core-foundation" version = "0.9.4" @@ -817,6 +862,29 @@ dependencies = [ "hybrid-array", ] +[[package]] +name = "cssparser" +version = "0.35.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4e901edd733a1472f944a45116df3f846f54d37e67e68640ac8bb69689aca2aa" +dependencies = [ + "cssparser-macros", + "dtoa-short", + "itoa", + "phf 0.11.3", + "smallvec", +] + +[[package]] +name = "cssparser-macros" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13b588ba4ac1a99f7f2964d24b3d896ddc6bf847ee3855dbd4366f058cfcd331" +dependencies = [ + "quote", + "syn 2.0.117", +] + [[package]] name = "ctr" version = "0.9.2" @@ -1084,6 +1152,21 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" +[[package]] +name = "dtoa" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c3cf4824e2d5f025c7b531afcb2325364084a16806f6d47fbc1f5fbd9960590" + +[[package]] +name = "dtoa-short" +version = "0.3.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd1511a7b6a56299bd043a9c167a6d2bfb37bf84a6dfceaba651168adfb43c87" +dependencies = [ + "dtoa", +] + [[package]] name = "dunce" version = "1.0.5" @@ -1462,6 +1545,28 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "garde" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a74b56a4039a46e8c91cc9d84e8a7df4e1f8b24239ca57d1304b3263cb599b9" +dependencies = [ + "compact_str", + "garde_derive", + "smallvec", +] + +[[package]] +name = "garde_derive" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7224c08ec489e2840af29ed882b47f7f6ac8f4ce15c275d9fc0d6d1b94578ae6" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "generic-array" version = "0.14.7" @@ -1792,6 +1897,7 @@ checksum = "df3b46402a9d5adb4c86a0cf463f42e19994e3ee891101b1841f30a545cb49a9" name = "huskies" version = "0.10.4" dependencies = [ + "ammonia", "async-stream", "async-trait", "base64", @@ -1802,6 +1908,7 @@ dependencies = [ "ed25519-dalek", "filetime", "futures", + "garde", "hmac 0.13.0", "homedir", "ignore", @@ -1811,6 +1918,7 @@ dependencies = [ "mime_guess", "mockito", "notify", + "nutype", "poem", "portable-pty", "pulldown-cmark", @@ -2297,6 +2405,27 @@ dependencies = [ "serde_core", ] +[[package]] +name = "kinded" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53c48b3aa70f02a317f180f7d4e0834bbd2ffa293e352a132bb2ac9351736635" +dependencies = [ + "kinded_macros", +] + +[[package]] +name = "kinded_macros" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "feeb717349480c3d8125f84afdd78e9d7128004d8e2a561c35b3bbd76363d085" +dependencies = [ + "convert_case", + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "konst" version = "0.3.17" @@ -3013,6 +3142,30 @@ dependencies = [ "libc", ] +[[package]] +name = "nutype" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e3ae01f500e59a27e8c868297e15e268105d30ae06dee1ca194e025be29dee2a" +dependencies = [ + "nutype_macros", +] + +[[package]] +name = "nutype_macros" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "48a6a6ddc63e6d0cd6b77af9aabae24601b39ad543d29bff88c4761c1fc34700" +dependencies = [ + "cfg-if", + "kinded", + "proc-macro2", + "quote", + "rustc_version", + "syn 2.0.117", + "urlencoding", +] + [[package]] name = "oauth2" version = "5.0.0" @@ -3127,6 +3280,7 @@ version = "0.11.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1fd6780a80ae0c52cc120a26a1a42c1ae51b247a253e4e06113d23d2c2edd078" dependencies = [ + "phf_macros", "phf_shared 0.11.3", ] @@ -3159,6 +3313,19 @@ dependencies = [ "rand 0.8.6", ] +[[package]] +name = "phf_macros" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f84ac04429c13a7ff43785d75ad27569f2951ce0ffd30a3321230db2fc727216" +dependencies = [ + "phf_generator", + "phf_shared 0.11.3", + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "phf_shared" version = "0.11.3" @@ -4811,6 +4978,12 @@ version = "1.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596" +[[package]] +name = "static_assertions" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" + [[package]] name = "string_cache" version = "0.8.9" @@ -5448,6 +5621,12 @@ version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7df058c713841ad818f1dc5d3fd88063241cc61f49f5fbea4b951e8cf5a8d71d" +[[package]] +name = "unicode-segmentation" +version = "1.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9629274872b2bfaf8d66f5f15725007f635594914870f65218920345aa11aa8c" + [[package]] name = "unicode-xid" version = "0.2.6" diff --git a/Cargo.toml b/Cargo.toml index d5a6b82d..e5201a38 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,9 @@ pulldown-cmark = { version = "0.13.3", default-features = false, features = [ ] } regex = "1" libc = "0.2" +nutype = { version = "0.7", features = ["serde"] } +garde = { version = "0.22", features = ["derive"] } +ammonia = "4.1" sqlx = { version = "=0.9.0-alpha.1", default-features = false, features = [ "runtime-tokio", "sqlite", diff --git a/server/Cargo.toml b/server/Cargo.toml index e2d8fb7e..45f2dcf8 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -44,6 +44,9 @@ bft-json-crdt = { path = "../crates/bft-json-crdt", default-features = false, fe source-map-gen = { path = "../crates/source-map-gen" } ed25519-dalek = { workspace = true } rand = { workspace = true } +nutype = { workspace = true } +garde = { workspace = true } +ammonia = { workspace = true } [target.'cfg(unix)'.dependencies] libc = { workspace = true } diff --git a/server/src/http/mcp/story_tools/epic.rs b/server/src/http/mcp/story_tools/epic.rs index 148dad4c..902ed537 100644 --- a/server/src/http/mcp/story_tools/epic.rs +++ b/server/src/http/mcp/story_tools/epic.rs @@ -6,38 +6,27 @@ use crate::http::context::AppContext; use crate::http::workflow::create_epic_file; +use crate::validation::CreateEpicRequest; use serde_json::{Value, json}; /// Create a new epic and store it in the CRDT items list. pub(crate) fn tool_create_epic(args: &Value, ctx: &AppContext) -> Result { - let name = args - .get("name") - .and_then(|v| v.as_str()) - .ok_or("Missing required argument: name")?; - let goal = args - .get("goal") - .and_then(|v| v.as_str()) - .ok_or("Missing required argument: goal")?; - let motivation = args.get("motivation").and_then(|v| v.as_str()); - let key_files = args.get("key_files").and_then(|v| v.as_str()); - let success_criteria: Option> = args - .get("success_criteria") - .and_then(|v| v.as_array()) - .map(|arr| { - arr.iter() - .filter_map(|v| v.as_str().map(str::to_string)) - .collect() - }); + let req = CreateEpicRequest::from_json(args)?; let root = ctx.state.get_project_root()?; + let success_criteria = req.success_criteria_strings(); let epic_id = create_epic_file( &root, - name, - goal, - motivation, - key_files, - success_criteria.as_deref(), + req.name.as_ref(), + req.goal.as_str(), + req.motivation.as_ref().map(|d| d.as_ref()), + req.key_files.as_deref(), + if success_criteria.is_empty() { + None + } else { + Some(success_criteria.as_slice()) + }, )?; Ok(format!("Created epic: {epic_id}")) @@ -204,6 +193,22 @@ mod tests { assert!(result.unwrap_err().contains("goal")); } + #[test] + fn tool_create_epic_rejects_grammar_token_in_name() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + + let result = tool_create_epic( + &json!({"name": "Epic bad", "goal": "some goal"}), + &ctx, + ); + assert!(result.is_err()); + assert!( + result.unwrap_err().contains("AntiGrammarToken"), + "expected AntiGrammarToken error" + ); + } + #[test] fn tool_list_epics_includes_created_epic() { let tmp = tempfile::tempdir().unwrap(); diff --git a/server/src/http/mcp/story_tools/story/create.rs b/server/src/http/mcp/story_tools/story/create.rs index f74eb374..4a9b1197 100644 --- a/server/src/http/mcp/story_tools/story/create.rs +++ b/server/src/http/mcp/story_tools/story/create.rs @@ -3,55 +3,36 @@ use crate::http::context::AppContext; use crate::http::workflow::create_story_file; use crate::slog_warn; +use crate::validation::CreateStoryRequest; use serde_json::Value; +/// Create a new story in the backlog. +/// +/// Deserialises the JSON arguments into a `CreateStoryRequest`, runs the full +/// validation pipeline (field-level newtypes + cross-field garde rules), then +/// delegates to `create_story_file` / `create_item_in_backlog`. All ad-hoc +/// string checks have been removed; `CreateStoryRequest` is now the sole gate. pub(crate) fn tool_create_story(args: &Value, ctx: &AppContext) -> Result { - let name = args - .get("name") - .and_then(|v| v.as_str()) - .ok_or("Missing required argument: name")?; - let user_story = args.get("user_story").and_then(|v| v.as_str()); - let description = args.get("description").and_then(|v| v.as_str()); - let acceptance_criteria: Vec = args - .get("acceptance_criteria") - .and_then(|v| serde_json::from_value(v.clone()).ok()) - .ok_or("Missing required argument: acceptance_criteria")?; - if acceptance_criteria.is_empty() { - return Err("acceptance_criteria must contain at least one entry".to_string()); - } - const JUNK_AC: &[&str] = &["", "todo", "tbd", "fixme", "xxx", "???"]; - let all_junk = acceptance_criteria - .iter() - .all(|ac| JUNK_AC.contains(&ac.trim().to_lowercase().as_str())); - if all_junk { - return Err( - "acceptance_criteria must contain at least one real entry (not just TODO/TBD/FIXME/XXX/???)." - .to_string(), - ); - } - let depends_on: Option> = args - .get("depends_on") - .and_then(|v| serde_json::from_value(v.clone()).ok()); - // Spike 61: write the file only — the filesystem watcher detects the new - // .md file in work/1_backlog/ and auto-commits with a deterministic message. - let commit = false; + let req = CreateStoryRequest::from_json(args)?; let root = ctx.state.get_project_root()?; + let depends_on_ids = req.depends_on_ids(); + let story_id = create_story_file( &root, - name, - user_story, - description, - &acceptance_criteria, - depends_on.as_deref(), - commit, + req.name.as_ref(), + req.user_story.as_ref().map(|d| d.as_ref()), + req.description.as_ref().map(|d| d.as_ref()), + &req.acceptance_criteria + .iter() + .map(|ac| ac.as_ref().to_string()) + .collect::>(), + depends_on_ids.as_deref(), + false, )?; // Bug 503: warn at creation time if any depends_on points at an already-archived story. - // Archived = satisfied semantics: the dep will resolve immediately on the next promotion - // tick, which is surprising if the archived story was abandoned rather than cleanly done. - // Story 929: dep archived-status now comes from the CRDT, not a FS scan of 6_archived/. - let archived_deps: Vec = depends_on + let archived_deps: Vec = depends_on_ids .as_deref() .map(|deps| { deps.iter() @@ -199,7 +180,11 @@ mod tests { let ctx = test_ctx(tmp.path()); let result = tool_create_story(&json!({}), &ctx); assert!(result.is_err()); - assert!(result.unwrap_err().contains("Missing required argument")); + let err = result.unwrap_err(); + assert!( + err.contains("FieldMissing") || err.contains("name"), + "expected FieldMissing/name in: {err}" + ); } #[test] @@ -211,7 +196,6 @@ mod tests { &ctx, ); assert!(result.is_err()); - assert!(result.unwrap_err().contains("alphanumeric")); } #[test] @@ -224,8 +208,8 @@ mod tests { ) .unwrap_err(); assert!( - err.contains("empty") || err.contains("whitespace"), - "error should mention empty/whitespace, got: {err}" + err.contains("EmptyAfterTrim") || err.contains("empty") || err.contains("whitespace"), + "error should mention EmptyAfterTrim/empty/whitespace, got: {err}" ); } @@ -277,10 +261,6 @@ mod tests { &ctx, ); assert!(result.is_err()); - assert!( - result.unwrap_err().contains("real entry"), - "error should mention real entry" - ); } #[test] @@ -326,4 +306,59 @@ mod tests { "Description text missing from story: {content}" ); } + + #[test] + fn tool_create_story_rejects_grammar_token_in_name() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let err = tool_create_story( + &json!({ + "name": "Bad Story", + "acceptance_criteria": ["AC1"] + }), + &ctx, + ) + .unwrap_err(); + assert!( + err.contains("AntiGrammarToken") || err.contains("grammar"), + "expected grammar-token error, got: {err}" + ); + } + + #[test] + fn tool_create_story_rejects_grammar_token_in_ac() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + let err = tool_create_story( + &json!({ + "name": "Valid Story", + "acceptance_criteria": ["bad output"] + }), + &ctx, + ) + .unwrap_err(); + assert!( + err.contains("AntiGrammarToken") || err.contains("grammar"), + "expected grammar-token error, got: {err}" + ); + } + + #[test] + fn tool_create_story_html_sanitised_in_name() { + let tmp = tempfile::tempdir().unwrap(); + let ctx = test_ctx(tmp.path()); + // HTML in name is sanitised (not rejected) + let result = tool_create_story( + &json!({ + "name": "Story with bold name", + "acceptance_criteria": ["AC1"] + }), + &ctx, + ); + // Should succeed (HTML is sanitised, not rejected) + assert!( + result.is_ok(), + "HTML in name should be sanitised: {result:?}" + ); + } } diff --git a/server/src/http/workflow/bug_ops/epic.rs b/server/src/http/workflow/bug_ops/epic.rs index a4255d4f..b18560db 100644 --- a/server/src/http/workflow/bug_ops/epic.rs +++ b/server/src/http/workflow/bug_ops/epic.rs @@ -6,11 +6,16 @@ use std::path::Path; -use super::super::{next_item_number, slugify_name, write_story_content}; +use super::super::create_item_in_backlog; -/// Create an epic file and store it in the database. +/// Create an epic file, storing it in the database via `create_item_in_backlog`. /// -/// Returns the epic_id (e.g. `"880"`). +/// Routes through `create_item_in_backlog` so that the tombstone-skip allocator +/// defence and post-write CRDT verification (added by bug 1001's fix) apply to +/// epics too. Previously this function had its own ad-hoc allocate-and-write +/// path that bypassed those safety checks. +/// +/// Returns the epic_id (numeric only, e.g. `"880"`). pub fn create_epic_file( root: &Path, name: &str, @@ -19,61 +24,54 @@ pub fn create_epic_file( key_files: Option<&str>, success_criteria: Option<&[String]>, ) -> Result { - let epic_number = next_item_number(root)?; - let slug = slugify_name(name); + let name_owned = name.to_string(); + let goal_owned = goal.to_string(); + let motivation_owned = motivation.map(str::to_string); + let key_files_owned = key_files.map(str::to_string); + let success_criteria_owned: Vec = + success_criteria.map(|sc| sc.to_vec()).unwrap_or_default(); - if slug.is_empty() { - return Err("Name must contain at least one alphanumeric character.".to_string()); - } + // Epics don't have acceptance criteria; pass an empty slice. + // create_item_in_backlog skips the AC check for type "epic". + create_item_in_backlog(root, "epic", name, &[], None, move |epic_number| { + let mut content = String::new(); + content.push_str("---\n"); + content.push_str("type: epic\n"); + content.push_str(&format!("name: \"{}\"\n", name_owned.replace('"', "\\\""))); + content.push_str("---\n\n"); + content.push_str(&format!("# Epic {epic_number}: {name_owned}\n\n")); - let epic_id = format!("{epic_number}"); + content.push_str("## Goal\n\n"); + content.push_str(&goal_owned); + content.push_str("\n\n"); - let mut content = String::new(); - content.push_str("---\n"); - content.push_str("type: epic\n"); - content.push_str(&format!("name: \"{}\"\n", name.replace('"', "\\\""))); - content.push_str("---\n\n"); - content.push_str(&format!("# Epic {epic_number}: {name}\n\n")); - - content.push_str("## Goal\n\n"); - content.push_str(goal); - content.push_str("\n\n"); - - content.push_str("## Motivation\n\n"); - if let Some(m) = motivation { - content.push_str(m); - content.push('\n'); - } else { - content.push_str("- TBD\n"); - } - content.push('\n'); - - content.push_str("## Key Files\n\n"); - if let Some(kf) = key_files { - content.push_str(kf); - content.push('\n'); - } else { - content.push_str("- TBD\n"); - } - content.push('\n'); - - content.push_str("## Success Criteria\n\n"); - match success_criteria { - Some(criteria) if !criteria.is_empty() => { - for c in criteria { - content.push_str(&format!("- {c}\n")); - } - } - _ => { + content.push_str("## Motivation\n\n"); + if let Some(ref m) = motivation_owned { + content.push_str(m); + content.push('\n'); + } else { content.push_str("- TBD\n"); } - } + content.push('\n'); - // Epics are stored in backlog (no pipeline advancement). - write_story_content(root, &epic_id, "1_backlog", &content, Some(name)); + content.push_str("## Key Files\n\n"); + if let Some(ref kf) = key_files_owned { + content.push_str(kf); + content.push('\n'); + } else { + content.push_str("- TBD\n"); + } + content.push('\n'); - // Story 933: typed CRDT register for item_type. - crate::crdt_state::set_item_type(&epic_id, Some(crate::io::story_metadata::ItemType::Epic)); + content.push_str("## Success Criteria\n\n"); + if !success_criteria_owned.is_empty() { + for c in &success_criteria_owned { + content.push_str(&format!("- {c}\n")); + } + } else { + content.push_str("- TBD\n"); + } - Ok(epic_id) + content + }) } diff --git a/server/src/http/workflow/utils.rs b/server/src/http/workflow/utils.rs index 9b313c89..a75cd9ed 100644 --- a/server/src/http/workflow/utils.rs +++ b/server/src/http/workflow/utils.rs @@ -250,15 +250,17 @@ pub(crate) fn create_item_in_backlog( if slugify_name(name).is_empty() { return Err("Title must contain at least one alphanumeric character.".to_string()); } - if acceptance_criteria.is_empty() { - return Err("At least one acceptance criterion is required.".to_string()); - } - const VALID_TYPES: &[&str] = &["story", "bug", "spike", "refactor"]; + const VALID_TYPES: &[&str] = &["story", "bug", "spike", "refactor", "epic"]; if !VALID_TYPES.contains(&item_type) { return Err(format!( - "Invalid item type '{item_type}': must be one of story, bug, spike, refactor." + "Invalid item type '{item_type}': must be one of story, bug, spike, refactor, epic." )); } + // Epics use success_criteria (optional); the acceptance_criteria check is + // only meaningful for pipeline work items. + if item_type != "epic" && acceptance_criteria.is_empty() { + return Err("At least one acceptance criterion is required.".to_string()); + } let item_number = next_item_number(root)?; let item_id = format!("{item_number}"); diff --git a/server/src/main.rs b/server/src/main.rs index e4528464..567fe6d3 100644 --- a/server/src/main.rs +++ b/server/src/main.rs @@ -43,6 +43,8 @@ pub mod sled_uplink; mod startup; mod state; mod store; +/// Validated input layer — transport-agnostic newtypes and request structs for all MCP write tools. +pub mod validation; mod workflow; mod worktree; diff --git a/server/src/validation/error.rs b/server/src/validation/error.rs new file mode 100644 index 00000000..a0b5bcc3 --- /dev/null +++ b/server/src/validation/error.rs @@ -0,0 +1,127 @@ +//! Typed validation error enum returned from all MCP write-tool input validation. + +use serde::{Deserialize, Serialize}; +use std::fmt; + +/// Structured error from input validation. +/// +/// Each variant carries exactly the data a caller needs to act on the error. +/// Serialises to serde's default externally-tagged form, e.g. +/// `{"FieldTooLong":{"field":"description","max":200,"actual":287}}`. +/// Callers can pattern-match on the JSON tag without parsing prose. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub enum ValidationError { + /// A required field was absent from the input. + FieldMissing { field: String }, + /// A field value is empty (or whitespace-only) after trimming. + EmptyAfterTrim { field: String }, + /// A field value exceeds the maximum allowed length. + FieldTooLong { + field: String, + max: usize, + actual: usize, + }, + /// A field value contains a character outside the allowed set. + InvalidCharacter { + field: String, + ch: char, + position: usize, + }, + /// A field value contains a tool-call grammar fragment that must be rejected. + AntiGrammarToken { field: String, token: String }, + /// A numeric field value is outside its allowed range. + OutOfRange { + field: String, + min: i64, + max: i64, + actual: i64, + }, + /// A list field has fewer items than the minimum. + TooFewItems { + field: String, + min: usize, + actual: usize, + }, + /// A list field has more items than the maximum. + TooManyItems { + field: String, + max: usize, + actual: usize, + }, + /// A field value is not valid UTF-8. + InvalidUtf8 { field: String }, + /// A `depends_on` entry references the same item being created or updated. + SelfReference { field: String }, +} + +impl fmt::Display for ValidationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::FieldMissing { field } => { + write!(f, "field '{field}' is required but was not provided") + } + Self::EmptyAfterTrim { field } => { + write!(f, "field '{field}' must not be empty or whitespace-only") + } + Self::FieldTooLong { field, max, actual } => { + write!(f, "field '{field}' is too long ({actual} chars, max {max})") + } + Self::InvalidCharacter { + field, + ch, + position, + } => { + write!( + f, + "field '{field}' contains invalid character {ch:?} at position {position}" + ) + } + Self::AntiGrammarToken { field, token } => { + write!( + f, + "field '{field}' contains a tool-call grammar fragment: {token:?}" + ) + } + Self::OutOfRange { + field, + min, + max, + actual, + } => { + write!( + f, + "field '{field}' value {actual} is out of allowed range [{min}, {max}]" + ) + } + Self::TooFewItems { field, min, actual } => { + write!( + f, + "field '{field}' has too few items ({actual}; minimum {min})" + ) + } + Self::TooManyItems { field, max, actual } => { + write!( + f, + "field '{field}' has too many items ({actual}; maximum {max})" + ) + } + Self::InvalidUtf8 { field } => { + write!(f, "field '{field}' contains invalid UTF-8") + } + Self::SelfReference { field } => { + write!( + f, + "field '{field}' contains a self-reference (depends on itself)" + ) + } + } + } +} + +/// Serialise a slice of validation errors as a pretty-printed JSON string. +/// +/// Used to turn `Vec` into the `Err(String)` value returned by +/// MCP tool handlers. +pub fn format_errors_as_json(errors: &[ValidationError]) -> String { + serde_json::to_string_pretty(errors).unwrap_or_else(|_| format!("{errors:?}")) +} diff --git a/server/src/validation/mod.rs b/server/src/validation/mod.rs new file mode 100644 index 00000000..26fd7b8d --- /dev/null +++ b/server/src/validation/mod.rs @@ -0,0 +1,22 @@ +//! Transport-agnostic validated input layer for MCP write tools. +//! +//! This module houses all input validation primitives shared across MCP, HTTP, +//! and future WebSocket callers. It is intentionally decoupled from any +//! specific transport — callers parse their raw input into the request types +//! here and receive either a validated struct or a `Vec`. +//! +//! # Structure +//! +//! - [`error`] — [`ValidationError`] typed enum + JSON serialisation helpers. +//! - [`sanitize`] — HTML sanitisation via `ammonia`. +//! - [`newtypes`] — field-level newtypes (`StoryName`, `AcceptanceCriterion`, …). +//! - [`requests`] — top-level request structs with cross-field `garde` rules. + +mod error; +mod newtypes; +mod requests; +mod sanitize; + +pub use error::{ValidationError, format_errors_as_json}; +pub use newtypes::{AcceptanceCriterion, DependsOnId, Description, StoryName}; +pub use requests::{CreateEpicRequest, CreateStoryRequest}; diff --git a/server/src/validation/newtypes.rs b/server/src/validation/newtypes.rs new file mode 100644 index 00000000..8787ee36 --- /dev/null +++ b/server/src/validation/newtypes.rs @@ -0,0 +1,397 @@ +//! Validated input newtypes for MCP write tools. +//! +//! Each newtype's inner value is guaranteed valid once constructed — the only +//! public constructors run the full validation pipeline. Use the associated +//! `parse` or `parse_with_field` methods (which return rich `Vec`) +//! in preference to nutype's lower-level `new()`. + +use nutype::nutype; +use serde::{Deserialize, Serialize}; + +use super::error::ValidationError; +use super::sanitize; + +/// Tool-call grammar fragments that must be rejected in any text field. +/// +/// These are hallucination artifacts from the LLM (bug 1001): if they appear +/// in a field value the whole call is malformed bot output, not user content. +const ANTI_GRAMMAR_TOKENS: &[&str] = &[ + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", +]; + +/// Maximum length (chars) for a story/epic name. +pub(super) const NAME_MAX_LEN: usize = 200; +/// Maximum length (chars) for a description / goal / motivation field. +pub(super) const DESCRIPTION_MAX_LEN: usize = 4000; +/// Maximum length (chars) for a single acceptance criterion. +pub(super) const AC_MAX_LEN: usize = 1000; + +/// Scan `value` for any anti-grammar-token and return errors if found. +fn check_grammar_tokens(field: &str, value: &str) -> Vec { + ANTI_GRAMMAR_TOKENS + .iter() + .filter(|&&token| value.contains(token)) + .map(|&token| ValidationError::AntiGrammarToken { + field: field.to_string(), + token: token.to_string(), + }) + .collect() +} + +// --------------------------------------------------------------------------- +// StoryName newtype +// --------------------------------------------------------------------------- + +fn is_story_name_nonempty(val: &str) -> bool { + !val.is_empty() +} + +#[nutype( + sanitize(trim), + validate(predicate = is_story_name_nonempty), + derive(Debug, Clone, PartialEq, Serialize, Deserialize, AsRef) +)] +/// A validated, trimmed story or epic name. +pub struct StoryName(String); + +impl StoryName { + /// Parse a raw string as a story name, returning all validation errors found. + /// + /// Checks anti-grammar tokens first (on the raw trimmed value), then HTML-sanitises, + /// then validates length and non-emptiness. + pub fn parse(raw: &str) -> Result> { + let trimmed = raw.trim(); + + // Anti-grammar check on original trimmed value — must precede HTML sanitise + // because ammonia would strip the tokens before we could detect them. + let grammar_errors = check_grammar_tokens("name", trimmed); + if !grammar_errors.is_empty() { + return Err(grammar_errors); + } + + let (sanitized, _) = sanitize::sanitize_html("name", trimmed); + let mut errors = Vec::new(); + + if sanitized.is_empty() { + errors.push(ValidationError::EmptyAfterTrim { + field: "name".into(), + }); + } else if sanitized.len() > NAME_MAX_LEN { + errors.push(ValidationError::FieldTooLong { + field: "name".into(), + max: NAME_MAX_LEN, + actual: sanitized.len(), + }); + } + + if !errors.is_empty() { + return Err(errors); + } + + // nutype's sanitize(trim) is idempotent; predicate passes since we already checked. + StoryName::try_new(sanitized).map_err(|_e| { + vec![ValidationError::EmptyAfterTrim { + field: "name".into(), + }] + }) + } +} + +// --------------------------------------------------------------------------- +// AcceptanceCriterion newtype +// --------------------------------------------------------------------------- + +fn is_ac_nonempty(val: &str) -> bool { + !val.is_empty() +} + +#[nutype( + sanitize(trim), + validate(predicate = is_ac_nonempty), + derive(Debug, Clone, PartialEq, Serialize, Deserialize, AsRef) +)] +/// A single validated acceptance criterion. +pub struct AcceptanceCriterion(String); + +impl AcceptanceCriterion { + /// Parse a single raw acceptance criterion string, using `field` in error messages. + pub fn parse(field: &str, raw: &str) -> Result> { + let trimmed = raw.trim(); + + let grammar_errors = check_grammar_tokens(field, trimmed); + if !grammar_errors.is_empty() { + return Err(grammar_errors); + } + + let (sanitized, _) = sanitize::sanitize_html(field, trimmed); + let mut errors = Vec::new(); + + if sanitized.is_empty() { + errors.push(ValidationError::EmptyAfterTrim { + field: field.to_string(), + }); + } else if sanitized.len() > AC_MAX_LEN { + errors.push(ValidationError::FieldTooLong { + field: field.to_string(), + max: AC_MAX_LEN, + actual: sanitized.len(), + }); + } + + if !errors.is_empty() { + return Err(errors); + } + + AcceptanceCriterion::try_new(sanitized).map_err(|_e| { + vec![ValidationError::EmptyAfterTrim { + field: field.to_string(), + }] + }) + } +} + +// --------------------------------------------------------------------------- +// Description newtype (used for description, user_story, goal, motivation) +// --------------------------------------------------------------------------- + +/// A validated trimmed description / goal / motivation text. +/// +/// The field name is not embedded in the type; pass it to `parse`. +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub struct Description(String); + +impl Description { + /// Parse and validate a description field value. + /// + /// `field` is used in error messages (e.g. `"description"`, `"goal"`). + pub fn parse(field: &str, raw: &str) -> Result> { + let trimmed = raw.trim(); + + let grammar_errors = check_grammar_tokens(field, trimmed); + if !grammar_errors.is_empty() { + return Err(grammar_errors); + } + + let (sanitized, _) = sanitize::sanitize_html(field, trimmed); + let mut errors = Vec::new(); + + if sanitized.is_empty() { + errors.push(ValidationError::EmptyAfterTrim { + field: field.to_string(), + }); + } else if sanitized.len() > DESCRIPTION_MAX_LEN { + errors.push(ValidationError::FieldTooLong { + field: field.to_string(), + max: DESCRIPTION_MAX_LEN, + actual: sanitized.len(), + }); + } + + if !errors.is_empty() { + Err(errors) + } else { + Ok(Description(sanitized)) + } + } + + /// Return the inner string value. + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl AsRef for Description { + fn as_ref(&self) -> &str { + &self.0 + } +} + +// --------------------------------------------------------------------------- +// DependsOnId newtype +// --------------------------------------------------------------------------- + +fn is_nonzero_dep_id(val: &u32) -> bool { + *val != 0 +} + +#[nutype( + validate(predicate = is_nonzero_dep_id), + derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize) +)] +/// A validated non-zero story/item dependency ID. +pub struct DependsOnId(u32); + +impl DependsOnId { + /// Parse a raw `u32` as a dependency ID, using `field` in error messages. + pub fn parse(field: &str, id: u32) -> Result> { + if id == 0 { + return Err(vec![ValidationError::OutOfRange { + field: field.to_string(), + min: 1, + max: i64::from(u32::MAX), + actual: 0, + }]); + } + DependsOnId::try_new(id).map_err(|_| { + vec![ValidationError::OutOfRange { + field: field.to_string(), + min: 1, + max: i64::from(u32::MAX), + actual: i64::from(id), + }] + }) + } + + /// Return the raw inner value. + pub fn get(self) -> u32 { + self.into_inner() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + // --- StoryName --- + + #[test] + fn story_name_rejects_empty() { + let err = StoryName::parse("").unwrap_err(); + assert!(matches!(err[0], ValidationError::EmptyAfterTrim { .. })); + } + + #[test] + fn story_name_rejects_whitespace_only() { + let err = StoryName::parse(" ").unwrap_err(); + assert!(matches!(err[0], ValidationError::EmptyAfterTrim { .. })); + } + + #[test] + fn story_name_accepts_valid() { + let n = StoryName::parse("My Story").unwrap(); + assert_eq!(n.as_ref(), "My Story"); + } + + #[test] + fn story_name_trims_whitespace() { + let n = StoryName::parse(" Trimmed ").unwrap(); + assert_eq!(n.as_ref(), "Trimmed"); + } + + #[test] + fn story_name_rejects_too_long() { + let long = "x".repeat(NAME_MAX_LEN + 1); + let err = StoryName::parse(&long).unwrap_err(); + assert!(matches!(err[0], ValidationError::FieldTooLong { .. })); + } + + #[test] + fn story_name_rejects_grammar_token() { + let err = StoryName::parse("my story end").unwrap_err(); + assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. })); + } + + #[test] + fn story_name_rejects_thinking_token() { + let err = StoryName::parse("hello").unwrap_err(); + assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. })); + } + + #[test] + fn story_name_strips_html() { + let n = StoryName::parse("Hello World").unwrap(); + assert!(!n.as_ref().contains('<')); + assert!(n.as_ref().contains("World")); + } + + // --- AcceptanceCriterion --- + + #[test] + fn ac_rejects_empty() { + let err = AcceptanceCriterion::parse("acceptance_criteria[0]", "").unwrap_err(); + assert!(matches!(err[0], ValidationError::EmptyAfterTrim { .. })); + } + + #[test] + fn ac_accepts_valid() { + let ac = AcceptanceCriterion::parse("acceptance_criteria[0]", "It works").unwrap(); + assert_eq!(ac.as_ref(), "It works"); + } + + #[test] + fn ac_rejects_grammar_token() { + let err = AcceptanceCriterion::parse("acceptance_criteria[0]", "bad") + .unwrap_err(); + assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. })); + } + + #[test] + fn ac_rejects_too_long() { + let long = "x".repeat(AC_MAX_LEN + 1); + let err = AcceptanceCriterion::parse("acceptance_criteria[0]", &long).unwrap_err(); + assert!(matches!(err[0], ValidationError::FieldTooLong { .. })); + } + + // --- Description --- + + #[test] + fn description_rejects_empty() { + let err = Description::parse("description", "").unwrap_err(); + assert!(matches!(err[0], ValidationError::EmptyAfterTrim { .. })); + } + + #[test] + fn description_accepts_valid() { + let d = Description::parse("goal", "Achieve world peace").unwrap(); + assert_eq!(d.as_str(), "Achieve world peace"); + } + + #[test] + fn description_rejects_grammar_token() { + let err = Description::parse("description", "text more").unwrap_err(); + assert!(matches!(err[0], ValidationError::AntiGrammarToken { .. })); + } + + // --- DependsOnId --- + + #[test] + fn depends_on_id_rejects_zero() { + let err = DependsOnId::parse("depends_on[0]", 0).unwrap_err(); + assert!(matches!(err[0], ValidationError::OutOfRange { .. })); + } + + #[test] + fn depends_on_id_accepts_nonzero() { + let id = DependsOnId::parse("depends_on[0]", 42).unwrap(); + assert_eq!(id.get(), 42); + } + + // --- Round-trip serde --- + + #[test] + fn validation_error_round_trips_json() { + let err = ValidationError::FieldTooLong { + field: "description".into(), + max: 200, + actual: 287, + }; + let json = serde_json::to_string(&err).unwrap(); + assert!(json.contains("FieldTooLong")); + assert!(json.contains("description")); + let back: ValidationError = serde_json::from_str(&json).unwrap(); + assert_eq!(err, back); + } +} diff --git a/server/src/validation/requests.rs b/server/src/validation/requests.rs new file mode 100644 index 00000000..12f10b8c --- /dev/null +++ b/server/src/validation/requests.rs @@ -0,0 +1,476 @@ +//! Validated request structs for MCP write tools. +//! +//! Each struct is populated by `from_json`, which runs field-level validation via +//! the newtypes, then cross-field rules via `garde`. Callers receive either a +//! fully validated struct or a `Vec` with every problem found. + +use garde::Validate; +use serde_json::Value; + +use super::error::{ValidationError, format_errors_as_json}; +use super::newtypes::{AcceptanceCriterion, DependsOnId, Description, StoryName}; + +// --------------------------------------------------------------------------- +// Cross-field validators (used by garde derive) +// --------------------------------------------------------------------------- + +/// Junk-only acceptance-criteria indicators — placeholders agents fill in but +/// that contain no actionable requirement. +const JUNK_AC_MARKERS: &[&str] = &["todo", "tbd", "fixme", "xxx", "???"]; + +fn validate_acceptance_criteria_nonempty(acs: &[AcceptanceCriterion], _ctx: &()) -> garde::Result { + if acs.is_empty() { + return Err(garde::Error::new( + "acceptance_criteria must contain at least one entry", + )); + } + let all_junk = acs.iter().all(|ac| { + let lower = ac.as_ref().to_lowercase(); + JUNK_AC_MARKERS.contains(&lower.trim()) + }); + if all_junk { + return Err(garde::Error::new( + "acceptance_criteria must contain at least one real entry (not just TODO/TBD/FIXME)", + )); + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// CreateStoryRequest +// --------------------------------------------------------------------------- + +/// Fully validated inputs for the `create_story` MCP tool. +#[derive(Debug, Validate)] +pub struct CreateStoryRequest { + /// Validated story name. + #[garde(skip)] + pub name: StoryName, + /// Optional user story text. + #[garde(skip)] + pub user_story: Option, + /// Optional background description. + #[garde(skip)] + pub description: Option, + /// At least one non-junk acceptance criterion required (garde-enforced). + #[garde(custom(validate_acceptance_criteria_nonempty))] + pub acceptance_criteria: Vec, + /// Optional list of story IDs this story depends on. + #[garde(skip)] + pub depends_on: Option>, +} + +impl CreateStoryRequest { + /// Parse and validate a `create_story` JSON argument map. + /// + /// Runs all field-level validation and cross-field garde rules in a single + /// pass. Returns every error found, not just the first. + pub fn from_json(args: &Value) -> Result { + let mut errors: Vec = Vec::new(); + + // name (required) + let name = match args.get("name").and_then(|v| v.as_str()) { + None => { + errors.push(ValidationError::FieldMissing { + field: "name".into(), + }); + None + } + Some(raw) => match StoryName::parse(raw) { + Ok(n) => Some(n), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + // user_story (optional) + let user_story = match args.get("user_story").and_then(|v| v.as_str()) { + None => None, + Some(raw) => match Description::parse("user_story", raw) { + Ok(d) => Some(d), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + // description (optional) + let description = match args.get("description").and_then(|v| v.as_str()) { + None => None, + Some(raw) => match Description::parse("description", raw) { + Ok(d) => Some(d), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + // acceptance_criteria (required) + let acceptance_criteria = match args + .get("acceptance_criteria") + .and_then(|v| serde_json::from_value::>(v.clone()).ok()) + { + None => { + errors.push(ValidationError::FieldMissing { + field: "acceptance_criteria".into(), + }); + None + } + Some(raw_acs) => { + let mut parsed = Vec::new(); + for (i, raw) in raw_acs.iter().enumerate() { + let field = format!("acceptance_criteria[{i}]"); + match AcceptanceCriterion::parse(&field, raw) { + Ok(ac) => parsed.push(ac), + Err(mut errs) => errors.append(&mut errs), + } + } + Some(parsed) + } + }; + + // depends_on (optional) + let depends_on: Option> = + match args.get("depends_on").and_then(|v| v.as_array()) { + None => None, + Some(arr) => { + let mut ids = Vec::new(); + for (i, val) in arr.iter().enumerate() { + let field = format!("depends_on[{i}]"); + match val.as_u64().map(|n| n as u32) { + None => errors.push(ValidationError::InvalidUtf8 { + field: field.clone(), + }), + Some(id) => match DependsOnId::parse(&field, id) { + Ok(d) => ids.push(d), + Err(mut errs) => errors.append(&mut errs), + }, + } + } + Some(ids) + } + }; + + if !errors.is_empty() { + return Err(format_errors_as_json(&errors)); + } + + let req = CreateStoryRequest { + name: name.unwrap(), + user_story, + description, + acceptance_criteria: acceptance_criteria.unwrap(), + depends_on, + }; + + // Cross-field garde validation + if let Err(report) = req.validate_with(&()) { + for (_, _field_error) in report.iter() { + // Map garde errors back to typed ValidationError. + // The only garde rule here is the AC nonempty/junk check. + let actual = req.acceptance_criteria.len(); + let all_junk = req.acceptance_criteria.iter().all(|ac| { + let lower = ac.as_ref().to_lowercase(); + JUNK_AC_MARKERS.contains(&lower.trim()) + }); + if all_junk && actual > 0 { + errors.push(ValidationError::TooFewItems { + field: "acceptance_criteria".into(), + min: 1, + // Semantic "0 real entries" + actual: 0, + }); + } else { + errors.push(ValidationError::TooFewItems { + field: "acceptance_criteria".into(), + min: 1, + actual, + }); + } + } + return Err(format_errors_as_json(&errors)); + } + + Ok(req) + } + + /// Extract validated `depends_on` as a plain `Vec` for downstream use. + pub fn depends_on_ids(&self) -> Option> { + self.depends_on + .as_ref() + .map(|ids| ids.iter().map(|d| d.get()).collect()) + } +} + +// --------------------------------------------------------------------------- +// CreateEpicRequest +// --------------------------------------------------------------------------- + +/// Fully validated inputs for the `create_epic` MCP tool. +#[derive(Debug, Validate)] +pub struct CreateEpicRequest { + /// Validated epic name. + #[garde(skip)] + pub name: StoryName, + /// Validated goal statement. + #[garde(skip)] + pub goal: Description, + /// Optional motivation text. + #[garde(skip)] + pub motivation: Option, + /// Optional key files text (plain string, minimal validation). + #[garde(skip)] + pub key_files: Option, + /// Optional success criteria list. + #[garde(skip)] + pub success_criteria: Option>, +} + +impl CreateEpicRequest { + /// Parse and validate a `create_epic` JSON argument map. + pub fn from_json(args: &Value) -> Result { + let mut errors: Vec = Vec::new(); + + // name (required) + let name = match args.get("name").and_then(|v| v.as_str()) { + None => { + errors.push(ValidationError::FieldMissing { + field: "name".into(), + }); + None + } + Some(raw) => match StoryName::parse(raw) { + Ok(n) => Some(n), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + // goal (required) + let goal = match args.get("goal").and_then(|v| v.as_str()) { + None => { + errors.push(ValidationError::FieldMissing { + field: "goal".into(), + }); + None + } + Some(raw) => match Description::parse("goal", raw) { + Ok(d) => Some(d), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + // motivation (optional) + let motivation = match args.get("motivation").and_then(|v| v.as_str()) { + None => None, + Some(raw) => match Description::parse("motivation", raw) { + Ok(d) => Some(d), + Err(mut errs) => { + errors.append(&mut errs); + None + } + }, + }; + + // key_files (optional, plain string — structural markup, not user prose) + let key_files = args + .get("key_files") + .and_then(|v| v.as_str()) + .map(str::trim) + .filter(|s| !s.is_empty()) + .map(str::to_string); + + // success_criteria (optional list) + let success_criteria = match args + .get("success_criteria") + .and_then(|v| serde_json::from_value::>(v.clone()).ok()) + { + None => None, + Some(raw_sc) => { + let mut parsed = Vec::new(); + for (i, raw) in raw_sc.iter().enumerate() { + let field = format!("success_criteria[{i}]"); + match AcceptanceCriterion::parse(&field, raw) { + Ok(ac) => parsed.push(ac), + Err(mut errs) => errors.append(&mut errs), + } + } + Some(parsed) + } + }; + + if !errors.is_empty() { + return Err(format_errors_as_json(&errors)); + } + + Ok(CreateEpicRequest { + name: name.unwrap(), + goal: goal.unwrap(), + motivation, + key_files, + success_criteria, + }) + } + + /// Extract success criteria as plain strings for downstream use. + pub fn success_criteria_strings(&self) -> Vec { + self.success_criteria + .as_ref() + .map(|sc| sc.iter().map(|c| c.as_ref().to_string()).collect()) + .unwrap_or_default() + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + // --- CreateStoryRequest --- + + #[test] + fn create_story_request_valid_minimal() { + let args = json!({ + "name": "My Story", + "acceptance_criteria": ["It works"] + }); + let req = CreateStoryRequest::from_json(&args).unwrap(); + assert_eq!(req.name.as_ref(), "My Story"); + assert_eq!(req.acceptance_criteria.len(), 1); + } + + #[test] + fn create_story_request_missing_name() { + let args = json!({"acceptance_criteria": ["AC1"]}); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("FieldMissing")); + assert!(err.contains("name")); + } + + #[test] + fn create_story_request_missing_acs() { + let args = json!({"name": "My Story"}); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("FieldMissing")); + assert!(err.contains("acceptance_criteria")); + } + + #[test] + fn create_story_request_empty_acs() { + let args = json!({"name": "My Story", "acceptance_criteria": []}); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("TooFewItems")); + } + + #[test] + fn create_story_request_all_junk_acs() { + let args = json!({"name": "My Story", "acceptance_criteria": ["TODO", "TBD"]}); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("TooFewItems")); + } + + #[test] + fn create_story_request_mixed_junk_and_real() { + let args = json!({ + "name": "My Story", + "acceptance_criteria": ["TODO", "Real criterion"] + }); + let req = CreateStoryRequest::from_json(&args).unwrap(); + assert_eq!(req.acceptance_criteria.len(), 2); + } + + #[test] + fn create_story_request_grammar_token_in_name() { + let args = json!({ + "name": "Story bad", + "acceptance_criteria": ["AC1"] + }); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("AntiGrammarToken")); + } + + #[test] + fn create_story_request_grammar_token_in_ac() { + let args = json!({ + "name": "Valid Name", + "acceptance_criteria": ["bad"] + }); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + assert!(err.contains("AntiGrammarToken")); + } + + #[test] + fn create_story_request_with_all_optional_fields() { + let args = json!({ + "name": "Full Story", + "user_story": "As a user I want this", + "description": "Background context", + "acceptance_criteria": ["AC1", "AC2"], + "depends_on": [1, 2, 3] + }); + let req = CreateStoryRequest::from_json(&args).unwrap(); + assert_eq!(req.depends_on_ids(), Some(vec![1, 2, 3])); + } + + #[test] + fn create_story_request_errors_contain_json() { + let args = json!({ + "name": "bad", + "acceptance_criteria": [] + }); + let err = CreateStoryRequest::from_json(&args).unwrap_err(); + // Errors are JSON, parseable + let parsed: serde_json::Value = serde_json::from_str(&err).unwrap(); + assert!(parsed.is_array()); + } + + // --- CreateEpicRequest --- + + #[test] + fn create_epic_request_valid_minimal() { + let args = json!({ + "name": "My Epic", + "goal": "Achieve something great" + }); + let req = CreateEpicRequest::from_json(&args).unwrap(); + assert_eq!(req.name.as_ref(), "My Epic"); + assert_eq!(req.goal.as_str(), "Achieve something great"); + } + + #[test] + fn create_epic_request_missing_name() { + let args = json!({"goal": "some goal"}); + let err = CreateEpicRequest::from_json(&args).unwrap_err(); + assert!(err.contains("FieldMissing")); + assert!(err.contains("name")); + } + + #[test] + fn create_epic_request_missing_goal() { + let args = json!({"name": "Epic"}); + let err = CreateEpicRequest::from_json(&args).unwrap_err(); + assert!(err.contains("FieldMissing")); + assert!(err.contains("goal")); + } + + #[test] + fn create_epic_request_with_success_criteria() { + let args = json!({ + "name": "My Epic", + "goal": "Achieve world peace", + "success_criteria": ["All wars end", "People prosper"] + }); + let req = CreateEpicRequest::from_json(&args).unwrap(); + let sc = req.success_criteria_strings(); + assert_eq!(sc.len(), 2); + } +} diff --git a/server/src/validation/sanitize.rs b/server/src/validation/sanitize.rs new file mode 100644 index 00000000..8a566507 --- /dev/null +++ b/server/src/validation/sanitize.rs @@ -0,0 +1,67 @@ +//! HTML sanitisation for user-supplied text fields. +//! +//! Uses ammonia to strip dangerous HTML tags and attributes while preserving +//! the visible text content. Sanitisation that actually fires is logged at +//! WARN so operators can spot abuse patterns. + +use sha2::Digest; +use std::collections::HashSet; + +/// Sanitise `value` for the named `field`. +/// +/// Strips all HTML tags (keeping their text content) and removes dangerous +/// attributes. Returns `(sanitised_value, was_modified)`. When `was_modified` +/// is `true` the caller should log at WARN. +pub(super) fn sanitize_html(field: &str, value: &str) -> (String, bool) { + // Build an ammonia cleaner that allows NO tags but keeps text content. + // clear_content_tags is also set to empty so that + // content is preserved as literal text rather than silently discarded. + let clean = ammonia::Builder::new() + .tags(HashSet::new()) + .clean_content_tags(HashSet::new()) + .clean(value) + .to_string(); + + let modified = clean != value; + if modified { + crate::slog_warn!( + "[validation] HTML sanitised in field '{}': fingerprint={}", + field, + fingerprint(value) + ); + } + (clean, modified) +} + +/// Return an 8-hex-char SHA-256 fingerprint of the input string. +fn fingerprint(input: &str) -> String { + let hash = sha2::Sha256::digest(input.as_bytes()); + hash[..4].iter().map(|b| format!("{b:02x}")).collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn script_tags_stripped_content_preserved() { + let (out, modified) = sanitize_html("name", ""); + assert!(modified); + assert!(!out.contains("