From dc4dc416701de65c33225d9cd72fc9cf693e1b24 Mon Sep 17 00:00:00 2001 From: NotAShelf Date: Wed, 11 Mar 2026 17:12:07 +0300 Subject: [PATCH] pinakes-plugin-api: consolidate reserved-route check; reject widget data-source refs Signed-off-by: NotAShelf Change-Id: I042ee31e95822f46520a618de8dcaf786a6a6964 --- crates/pinakes-plugin-api/src/ui_schema.rs | 483 +++++++++++++++++++- crates/pinakes-plugin-api/src/validation.rs | 173 ++++++- 2 files changed, 627 insertions(+), 29 deletions(-) diff --git a/crates/pinakes-plugin-api/src/ui_schema.rs b/crates/pinakes-plugin-api/src/ui_schema.rs index 783237e..02ec93d 100644 --- a/crates/pinakes-plugin-api/src/ui_schema.rs +++ b/crates/pinakes-plugin-api/src/ui_schema.rs @@ -25,7 +25,7 @@ //! "sidebar": { //! "type": "list", //! "data": "playlists", -//! "item_template": { "type": "text", "content": "{{title}}" } +//! "item_template": { "type": "text", "content": "title" } //! }, //! "main": { //! "type": "data_table", @@ -40,6 +40,11 @@ //! "playlists": { "type": "endpoint", "path": "/api/v1/collections" } //! } //! } +//! +//! Note: expression values are `Expression::Path` strings, not mustache +//! templates. A bare string like `"title"` resolves the `title` field in the +//! current item context. Nested fields use dotted segments: `"artist.name"`. +//! Array indices use the same notation: `"items.0.title"`. //! ``` use std::collections::HashMap; @@ -102,6 +107,7 @@ pub type SchemaResult = Result; /// padding: None, /// }, /// data_sources: Default::default(), +/// actions: Default::default(), /// }; /// ``` #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -127,6 +133,10 @@ pub struct UiPage { /// Named data sources available to this page #[serde(default, skip_serializing_if = "HashMap::is_empty")] pub data_sources: HashMap, + + /// Named actions available to this page (referenced by `ActionRef::Name`) + #[serde(default, skip_serializing_if = "HashMap::is_empty")] + pub actions: HashMap, } impl UiPage { @@ -151,6 +161,13 @@ impl UiPage { )); } + if crate::validation::SchemaValidator::is_reserved_route(&self.route) { + return Err(SchemaError::ValidationError(format!( + "Route '{}' conflicts with a built-in app route", + self.route + ))); + } + let depth = self.root_element.depth(); if depth > MAX_ELEMENT_DEPTH { return Err(SchemaError::DepthLimitExceeded); @@ -158,6 +175,11 @@ impl UiPage { self.root_element.validate(self)?; + for (name, action) in &self.actions { + validate_id(name)?; + action.validate()?; + } + for (name, source) in &self.data_sources { validate_id(name)?; source.validate()?; @@ -246,6 +268,28 @@ pub struct UiWidget { pub content: UiElement, } +impl UiWidget { + /// Validates this widget definition + /// + /// # Errors + /// + /// Returns `SchemaError::ValidationError` if validation fails + pub fn validate(&self) -> SchemaResult<()> { + if self.id.is_empty() { + return Err(SchemaError::ValidationError( + "Widget id cannot be empty".to_string(), + )); + } + if self.target.is_empty() { + return Err(SchemaError::ValidationError( + "Widget target cannot be empty".to_string(), + )); + } + validate_id(&self.id)?; + Ok(()) + } +} + /// String constants for widget injection locations. /// /// Use these with `UiWidget::target` in plugin manifests: @@ -259,6 +303,7 @@ pub mod widget_location { pub const LIBRARY_SIDEBAR: &str = "library_sidebar"; pub const DETAIL_PANEL: &str = "detail_panel"; pub const SEARCH_FILTERS: &str = "search_filters"; + pub const SETTINGS_SECTION: &str = "settings_section"; } /// Core UI element enum - the building block of all plugin UIs @@ -817,6 +862,11 @@ impl UiElement { Self::Button { action, .. } => { action.validate()?; }, + Self::Link { href, .. } if !is_safe_href(href) => { + return Err(SchemaError::ValidationError(format!( + "Link href has a disallowed scheme (must be '/', 'http://', or 'https://'): {href}" + ))); + }, Self::Form { fields, submit_action, @@ -1046,7 +1096,7 @@ pub struct ColumnDef { } /// Row action for `DataTable` -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] pub struct RowAction { /// Action identifier (unique within this table) pub id: String, @@ -1290,15 +1340,60 @@ pub enum ChartType { Scatter, } +/// Client-side action types that do not require an HTTP call. +/// +/// Used as `{"action": "", ...}` in JSON. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(tag = "action", rename_all = "snake_case")] +pub enum SpecialAction { + /// Trigger a data refresh (re-runs all data sources for the current page). + Refresh, + /// Navigate to a different route. + Navigate { + /// Target route path (must start with `/`) + to: String, + }, + /// Emit a named event to the server-side plugin event bus. + Emit { + /// Event name + event: String, + /// Optional payload (any JSON value) + #[serde(default)] + payload: serde_json::Value, + }, + /// Update a local state key (resolved against the current data context). + UpdateState { + /// State key name + key: String, + /// Expression whose value is stored at `key` + value: Expression, + }, + /// Open a modal overlay containing the given element. + OpenModal { + /// Element to render inside the modal + content: Box, + }, + /// Close the currently open modal overlay. + CloseModal, +} + /// Action reference - identifies an action to execute -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +/// +/// Deserialization order for `#[serde(untagged)]`: +/// 1. `Special` - JSON objects with an `"action"` string key +/// 2. `Inline` - JSON objects with a `"path"` key +/// 3. `Name` - bare JSON strings +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(untagged)] pub enum ActionRef { + /// Client-side special action (no HTTP call required) + Special(SpecialAction), + + /// Inline action definition (HTTP call) + Inline(ActionDefinition), + /// Simple action name (references page.actions) Name(String), - - /// Inline action definition - Inline(ActionDefinition), } impl ActionRef { @@ -1312,6 +1407,26 @@ impl ActionRef { /// Returns `SchemaError::ValidationError` if validation fails. pub fn validate(&self) -> SchemaResult<()> { match self { + Self::Special(s) => { + match s { + SpecialAction::Navigate { to } if to.is_empty() => { + return Err(SchemaError::ValidationError( + "Navigate.to cannot be empty".to_string(), + )); + }, + SpecialAction::UpdateState { key, .. } if key.is_empty() => { + return Err(SchemaError::ValidationError( + "UpdateState.key cannot be empty".to_string(), + )); + }, + SpecialAction::Emit { event, .. } if event.is_empty() => { + return Err(SchemaError::ValidationError( + "Emit.event cannot be empty".to_string(), + )); + }, + _ => {}, + } + }, Self::Name(name) => { if name.is_empty() { return Err(SchemaError::ValidationError( @@ -1376,6 +1491,18 @@ impl ActionDefinition { self.path ))); } + if !self.path.starts_with("/api/") { + return Err(SchemaError::ValidationError(format!( + "Action path must start with '/api/': {}", + self.path + ))); + } + if self.path.contains("..") { + return Err(SchemaError::ValidationError(format!( + "Action path contains invalid traversal sequence: {}", + self.path + ))); + } Ok(()) } } @@ -1462,6 +1589,16 @@ impl DataSource { "Endpoint path must start with '/': {path}" ))); } + if !path.starts_with("/api/") { + return Err(SchemaError::InvalidDataSource(format!( + "Endpoint path must start with '/api/': {path}" + ))); + } + if path.contains("..") { + return Err(SchemaError::InvalidDataSource(format!( + "Endpoint path contains invalid traversal sequence: {path}" + ))); + } }, Self::Transform { source_name, .. } => { validate_id(source_name)?; @@ -1475,16 +1612,31 @@ impl DataSource { /// Expression for dynamic value evaluation /// /// Expressions use JSONPath-like syntax for data access. +/// +/// ## JSON representation (serde untagged; order matters) +/// +/// Variants are tried in declaration order during deserialization: +/// +/// | JSON shape | Deserializes as | +/// |---------------------------------------------------|-----------------| +/// | `"users.0.name"` (string) | `Path` | +/// | `{"left":…,"op":"eq","right":…}` (object) | `Operation` | +/// | `{"function":"len","args":[…]}` (object) | `Call` | +/// | `42`, `true`, `null`, `[…]`, `{other fields}` … | `Literal` | +/// +/// `Literal` is intentionally last so that the more specific variants take +/// priority. A bare JSON string is always a **path reference**; to embed a +/// literal string value use `DataSource::Static` or a `Call` expression. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(untagged)] pub enum Expression { - /// Literal JSON value - Literal(serde_json::Value), - - /// Data path reference (e.g., "$.users[0].name") + /// Data path reference: a dotted key sequence walked against the context. + /// + /// e.g. `"user.name"` resolves to `ctx["user"]["name"]`; `"items.0"` resolves + /// to the first element. Path(String), - /// Binary operation + /// Binary operation applied to two sub-expressions. Operation { /// Left operand left: Box, @@ -1494,13 +1646,22 @@ pub enum Expression { right: Box, }, - /// Function call + /// Built-in function call. + /// + /// e.g. `{"function": "len", "args": ["tags"]}` returns the count of items + /// in the `tags` data source. Call { - /// Function name + /// Function name (see built-in function table in docs) function: String, - /// Function arguments + /// Positional arguments, each an `Expression` args: Vec, }, + + /// Literal JSON value: a constant that is returned unchanged. + /// + /// Matches numbers, booleans, null, arrays, and objects that do not match + /// the `Operation` or `Call` shapes above. + Literal(serde_json::Value), } impl Default for Expression { @@ -1579,6 +1740,18 @@ const fn default_http_method() -> HttpMethod { HttpMethod::Get } +/// Returns `true` if `href` uses a scheme safe to render in an anchor element. +/// +/// Allows relative paths (`/`), plain `http://`, and `https://`. Rejects +/// `javascript:`, `data:`, `vbscript:`, and any other scheme that could be +/// used for script injection or data exfiltration. +#[must_use] +pub fn is_safe_href(href: &str) -> bool { + href.starts_with('/') + || href.starts_with("https://") + || href.starts_with("http://") +} + /// Validates an identifier string /// /// IDs must: @@ -1729,6 +1902,7 @@ mod tests { row_actions: vec![], }, data_sources: HashMap::new(), + actions: HashMap::new(), }; let refs = page.referenced_data_sources(); @@ -1748,6 +1922,7 @@ mod tests { gap: 16, }, data_sources: HashMap::new(), + actions: HashMap::new(), }; assert!(page.validate().is_err()); @@ -1766,8 +1941,288 @@ mod tests { id: None, }, data_sources: HashMap::new(), + actions: HashMap::new(), }; assert!(page.validate().is_err()); } + + // Expression JSON round-trip tests + + /// A JSON string must deserialise as Path, not Literal. + #[test] + fn test_expression_string_deserialises_as_path() { + let expr: Expression = serde_json::from_str(r#""user.name""#).unwrap(); + assert_eq!(expr, Expression::Path("user.name".to_string())); + } + + /// A JSON number must deserialise as Literal, not Path. + #[test] + fn test_expression_number_deserialises_as_literal() { + let expr: Expression = serde_json::from_str("42").unwrap(); + assert_eq!(expr, Expression::Literal(serde_json::json!(42))); + } + + /// An Operation object is correctly deserialised. + #[test] + fn test_expression_operation_deserialises() { + let json = r#"{"left": "count", "op": "gt", "right": 0}"#; + let expr: Expression = serde_json::from_str(json).unwrap(); + match expr { + Expression::Operation { left, op, right } => { + assert_eq!(*left, Expression::Path("count".to_string())); + assert_eq!(op, Operator::Gt); + assert_eq!(*right, Expression::Literal(serde_json::json!(0))); + }, + other => panic!("expected Operation, got {other:?}"), + } + } + + /// A Call object is correctly deserialised. + #[test] + fn test_expression_call_deserialises() { + let json = r#"{"function": "len", "args": ["items"]}"#; + let expr: Expression = serde_json::from_str(json).unwrap(); + match expr { + Expression::Call { function, args } => { + assert_eq!(function, "len"); + assert_eq!(args, vec![Expression::Path("items".to_string())]); + }, + other => panic!("expected Call, got {other:?}"), + } + } + + /// Path expressions survive a full JSON round-trip. + #[test] + fn test_expression_path_round_trip() { + let original = Expression::Path("a.b.c".to_string()); + let json = serde_json::to_string(&original).unwrap(); + let recovered: Expression = serde_json::from_str(&json).unwrap(); + assert_eq!(original, recovered); + } + + // DataSource/ActionDefinition security validation tests + + #[test] + fn test_endpoint_path_must_start_with_api() { + let bad = DataSource::Endpoint { + method: HttpMethod::Get, + path: "/not-api/something".to_string(), + params: HashMap::new(), + poll_interval: 0, + transform: None, + }; + assert!(bad.validate().is_err()); + } + + #[test] + fn test_endpoint_path_rejects_traversal() { + let bad = DataSource::Endpoint { + method: HttpMethod::Get, + path: "/api/v1/../admin".to_string(), + params: HashMap::new(), + poll_interval: 0, + transform: None, + }; + assert!(bad.validate().is_err()); + } + + #[test] + fn test_action_path_must_start_with_api() { + let bad = ActionDefinition { + method: HttpMethod::Post, + path: "/admin/reset".to_string(), + ..ActionDefinition::default() + }; + assert!(bad.validate().is_err()); + } + + #[test] + fn test_action_path_rejects_traversal() { + let bad = ActionDefinition { + method: HttpMethod::Post, + path: "/api/v1/tags/../../auth/login".to_string(), + ..ActionDefinition::default() + }; + assert!(bad.validate().is_err()); + } + + // Link href safety tests + + #[test] + fn test_is_safe_href_allows_relative() { + assert!(is_safe_href("/some/path")); + } + + #[test] + fn test_is_safe_href_allows_https() { + assert!(is_safe_href("https://example.com/page")); + } + + #[test] + fn test_is_safe_href_allows_http() { + assert!(is_safe_href("http://example.com/page")); + } + + #[test] + fn test_is_safe_href_rejects_javascript() { + assert!(!is_safe_href("javascript:alert(1)")); + } + + #[test] + fn test_is_safe_href_rejects_data_uri() { + assert!(!is_safe_href("data:text/html,")); + } + + #[test] + fn test_is_safe_href_rejects_vbscript() { + assert!(!is_safe_href("vbscript:msgbox(1)")); + } + + #[test] + fn test_link_validation_rejects_unsafe_href() { + use std::collections::HashMap as HM; + let page = UiPage { + id: "p".to_string(), + title: "P".to_string(), + route: "/api/plugins/p/p".to_string(), + icon: None, + root_element: UiElement::Link { + text: "click".to_string(), + href: "javascript:alert(1)".to_string(), + external: false, + }, + data_sources: HM::new(), + actions: HM::new(), + }; + assert!(page.validate().is_err()); + } + + #[test] + fn test_reserved_route_rejected() { + use std::collections::HashMap as HM; + let page = UiPage { + id: "search-page".to_string(), + title: "Search".to_string(), + route: "/search".to_string(), + icon: None, + root_element: UiElement::Container { + children: vec![], + gap: 0, + padding: None, + }, + data_sources: HM::new(), + actions: HM::new(), + }; + let err = page.validate().unwrap_err(); + assert!( + matches!(err, SchemaError::ValidationError(_)), + "expected ValidationError, got {err:?}" + ); + assert!( + format!("{err}").contains("/search"), + "error should mention the conflicting route" + ); + } + + // --- SpecialAction JSON round-trips --- + + #[test] + fn test_special_action_refresh_roundtrip() { + let action = SpecialAction::Refresh; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!(json["action"], "refresh"); + let back: SpecialAction = serde_json::from_value(json).unwrap(); + assert_eq!(back, SpecialAction::Refresh); + } + + #[test] + fn test_special_action_navigate_roundtrip() { + let action = SpecialAction::Navigate { + to: "/foo".to_string(), + }; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!(json["action"], "navigate"); + assert_eq!(json["to"], "/foo"); + let back: SpecialAction = serde_json::from_value(json).unwrap(); + assert_eq!(back, SpecialAction::Navigate { + to: "/foo".to_string(), + }); + } + + #[test] + fn test_special_action_emit_roundtrip() { + let action = SpecialAction::Emit { + event: "my-event".to_string(), + payload: serde_json::json!({"key": "val"}), + }; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!(json["action"], "emit"); + assert_eq!(json["event"], "my-event"); + let back: SpecialAction = serde_json::from_value(json).unwrap(); + assert_eq!(back, action); + } + + #[test] + fn test_special_action_update_state_roundtrip() { + let action = SpecialAction::UpdateState { + key: "my-key".to_string(), + value: Expression::Literal(serde_json::json!(42)), + }; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!(json["action"], "update_state"); + assert_eq!(json["key"], "my-key"); + let back: SpecialAction = serde_json::from_value(json).unwrap(); + assert_eq!(back, action); + } + + #[test] + fn test_special_action_close_modal_roundtrip() { + let action = SpecialAction::CloseModal; + let json = serde_json::to_value(&action).unwrap(); + assert_eq!(json["action"], "close_modal"); + let back: SpecialAction = serde_json::from_value(json).unwrap(); + assert_eq!(back, SpecialAction::CloseModal); + } + + // --- ActionRef deserialization ordering --- + + #[test] + fn test_action_ref_special_refresh_deserializes() { + let json = serde_json::json!({"action": "refresh"}); + let action_ref: ActionRef = serde_json::from_value(json).unwrap(); + assert!(matches!( + action_ref, + ActionRef::Special(SpecialAction::Refresh) + )); + } + + #[test] + fn test_action_ref_special_navigate_deserializes() { + let json = serde_json::json!({"action": "navigate", "to": "/foo"}); + let action_ref: ActionRef = serde_json::from_value(json).unwrap(); + assert!(matches!( + action_ref, + ActionRef::Special(SpecialAction::Navigate { to }) if to == "/foo" + )); + } + + #[test] + fn test_action_ref_name_still_works() { + let json = serde_json::json!("my-action"); + let action_ref: ActionRef = serde_json::from_value(json).unwrap(); + assert!(matches!(action_ref, ActionRef::Name(n) if n == "my-action")); + } + + #[test] + fn test_action_ref_special_takes_priority_over_inline() { + // An object with "action":"refresh" must be SpecialAction, not + // misinterpreted as ActionDefinition. + let json = serde_json::json!({"action": "refresh"}); + let action_ref: ActionRef = serde_json::from_value(json).unwrap(); + assert!( + matches!(action_ref, ActionRef::Special(_)), + "SpecialAction must be matched before ActionDefinition" + ); + } } diff --git a/crates/pinakes-plugin-api/src/validation.rs b/crates/pinakes-plugin-api/src/validation.rs index d232f29..fc060f2 100644 --- a/crates/pinakes-plugin-api/src/validation.rs +++ b/crates/pinakes-plugin-api/src/validation.rs @@ -122,6 +122,10 @@ impl SchemaValidator { Self::validate_element(&widget.content, &mut errors); + if Self::element_references_data_source(&widget.content) { + errors.push("widgets cannot reference data sources".to_string()); + } + if errors.is_empty() { Ok(()) } else { @@ -132,19 +136,9 @@ impl SchemaValidator { /// Recursively validate a [`UiElement`] subtree. pub fn validate_element(element: &UiElement, errors: &mut Vec) { match element { - UiElement::Container { children, .. } => { - for child in children { - Self::validate_element(child, errors); - } - }, - - UiElement::Grid { children, .. } => { - for child in children { - Self::validate_element(child, errors); - } - }, - - UiElement::Flex { children, .. } => { + UiElement::Container { children, .. } + | UiElement::Grid { children, .. } + | UiElement::Flex { children, .. } => { for child in children { Self::validate_element(child, errors); } @@ -206,10 +200,15 @@ impl SchemaValidator { } }, - UiElement::List { data, .. } => { + UiElement::List { + data, + item_template, + .. + } => { if data.is_empty() { errors.push("List 'data' source key must not be empty".to_string()); } + Self::validate_element(item_template, errors); }, // Leaf elements with no children to recurse into @@ -226,6 +225,66 @@ impl SchemaValidator { } } + /// Returns true if any element in the tree references a named data source. + /// + /// Widgets have no data-fetching mechanism, so any data source reference + /// in a widget content tree is invalid and must be rejected at load time. + fn element_references_data_source(element: &UiElement) -> bool { + match element { + // Variants that reference a data source by name + UiElement::DataTable { .. } + | UiElement::MediaGrid { .. } + | UiElement::DescriptionList { .. } + | UiElement::Chart { .. } + | UiElement::Loop { .. } + | UiElement::List { .. } => true, + + // Container variants - recurse into children + UiElement::Container { children, .. } + | UiElement::Grid { children, .. } + | UiElement::Flex { children, .. } => { + children.iter().any(Self::element_references_data_source) + }, + + UiElement::Split { sidebar, main, .. } => { + Self::element_references_data_source(sidebar) + || Self::element_references_data_source(main) + }, + + UiElement::Tabs { tabs, .. } => { + tabs + .iter() + .any(|tab| Self::element_references_data_source(&tab.content)) + }, + + UiElement::Card { + content, footer, .. + } => { + content.iter().any(Self::element_references_data_source) + || footer.iter().any(Self::element_references_data_source) + }, + + UiElement::Conditional { + then, else_element, .. + } => { + Self::element_references_data_source(then) + || else_element + .as_ref() + .is_some_and(|e| Self::element_references_data_source(e)) + }, + + // Leaf elements with no data source references + UiElement::Heading { .. } + | UiElement::Text { .. } + | UiElement::Code { .. } + | UiElement::Button { .. } + | UiElement::Form { .. } + | UiElement::Link { .. } + | UiElement::Progress { .. } + | UiElement::Badge { .. } => false, + } + } + fn validate_data_source( name: &str, source: &DataSource, @@ -243,6 +302,12 @@ impl SchemaValidator { "Data source '{name}': endpoint path must start with '/': {path}" )); } + if !path.starts_with("/api/") { + errors.push(format!( + "DataSource '{name}': endpoint path must start with /api/ (got \ + '{path}')" + )); + } }, DataSource::Transform { source_name, .. } => { if source_name.is_empty() { @@ -264,7 +329,7 @@ impl SchemaValidator { && chars.all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') } - fn is_reserved_route(route: &str) -> bool { + pub(crate) fn is_reserved_route(route: &str) -> bool { RESERVED_ROUTES.iter().any(|reserved| { route == *reserved || route.starts_with(&format!("{reserved}/")) }) @@ -290,6 +355,7 @@ mod tests { padding: None, }, data_sources: HashMap::new(), + actions: HashMap::new(), } } @@ -580,4 +646,81 @@ mod tests { }; assert!(SchemaValidator::validate_page(&page).is_err()); } + + #[test] + fn test_widget_badge_content_passes_validation() { + let widget = crate::UiWidget { + id: "status-badge".to_string(), + target: "library_header".to_string(), + content: UiElement::Badge { + text: "active".to_string(), + variant: Default::default(), + }, + }; + assert!( + SchemaValidator::validate_widget(&widget).is_ok(), + "a widget with Badge content should pass validation" + ); + } + + #[test] + fn test_widget_datatable_fails_validation() { + let col: crate::ColumnDef = + serde_json::from_value(serde_json::json!({"key": "id", "header": "ID"})) + .unwrap(); + let widget = crate::UiWidget { + id: "my-widget".to_string(), + target: "library_header".to_string(), + content: UiElement::DataTable { + data: "items".to_string(), + columns: vec![col], + sortable: false, + filterable: false, + page_size: 0, + row_actions: vec![], + }, + }; + let result = SchemaValidator::validate_widget(&widget); + assert!( + result.is_err(), + "DataTable in widget should fail validation" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("cannot reference data sources"), + "error message should mention data sources: {err}" + ); + } + + #[test] + fn test_widget_container_with_loop_fails_validation() { + // Container whose child is a Loop - recursive check must catch it + let widget = crate::UiWidget { + id: "loop-widget".to_string(), + target: "library_header".to_string(), + content: UiElement::Container { + children: vec![UiElement::Loop { + data: "items".to_string(), + template: Box::new(UiElement::Text { + content: Default::default(), + variant: Default::default(), + allow_html: false, + }), + empty: None, + }], + gap: 0, + padding: None, + }, + }; + let result = SchemaValidator::validate_widget(&widget); + assert!( + result.is_err(), + "Container wrapping a Loop should fail widget validation" + ); + let err = result.unwrap_err().to_string(); + assert!( + err.contains("cannot reference data sources"), + "error message should mention data sources: {err}" + ); + } }