diff --git a/crates/ogar-render-askama/src/artifact_kinds/html_detail_view.rs b/crates/ogar-render-askama/src/artifact_kinds/html_detail_view.rs new file mode 100644 index 0000000..e13147a --- /dev/null +++ b/crates/ogar-render-askama/src/artifact_kinds/html_detail_view.rs @@ -0,0 +1,210 @@ +//! `HtmlDetailView` emitter — T3 per Northstar plan §3, the detail-page +//! sibling of T2's [`html_list_view`](super::html_list_view). +//! +//! Reuses every piece T2 built (`RenderColumn`, `ColumnKind`, the cell +//! sub-templates from [`super::cells`], the two-stage cells-pre-rendered- +//! in-Rust pattern). The difference is the spine: a `
` of inline +//! fields + `
` blocks for prose / family-edge collections, +//! instead of a `` of rows. +//! +//! Mirror of Redmine's `app/views/issues/show.html.erb` shape — same +//! field-by-field laydown, with the family-edge sections (subtasks, +//! relations, attachments, watchers, journals) as sibling `
`s. + +use askama::Template; + +use super::cells::{ + AttachmentEntry, AttachmentListCell, HoursCell, IdLinkCell, PlainCell, PrimaryLinkCell, + ProgressBarCell, RecordRefCell, RelationEntry, RelationListCell, RichTextCell, UserEntry, + UserListCell, +}; +use super::html_list_view::{CellData, CellSource}; +use super::ArtifactEmitter; +use crate::list_view::{ColumnKind, RenderColumn}; +use crate::spec::ArtifactSpec; +use ogar_vocab::canonical_concept_id; + +// ── Spine binding struct ───────────────────────────────────────────── + +#[derive(Template)] +#[template(path = "dispatch/html_detail_view.askama", escape = "html")] +struct HtmlDetailViewCtx { + class_id_hex: String, + canonical_concept: String, + record_id: u64, + /// Pre-rendered headline (typically the primary-link or plain-text + /// for the record's primary attribute). Empty means no headline. + headline_html: String, + /// Optional subtitle line (e.g. status + priority badges); empty + /// for none. + subtitle: String, + inline_cells: Vec, + block_sections: Vec, +} + +struct DetailField { + name: String, + label: String, + css_classes: String, + body_html: String, +} + +struct DetailSection { + name: String, + label: String, + css_classes: String, + body_html: String, +} + +// ── Public entry point ────────────────────────────────────────────── + +/// Render one canonical record as a detail page (definition-list inline +/// fields + section blocks for prose + family-edge collections). +/// +/// `columns` carries both `inline=true` (laid out as `
/
`) and +/// `inline=false` (rendered as full-width `
`s). The classifier +/// is done at render time from `column.inline`. +/// +/// `cells` is one entry per column in the same order — paired by index. +/// `headline_html` is the (already-rendered) primary-link / plain text +/// the page header shows; empty string for none. +#[allow(clippy::too_many_arguments)] +pub fn render_detail( + class_id: u16, + canonical_concept: &str, + record_id: u64, + headline_html: &str, + subtitle: &str, + columns: &[RenderColumn], + cells: &[CellSource<'_>], +) -> Result { + if columns.len() != cells.len() { + // Hard caller-side bug; surface it loudly. Tests pin this contract. + return Err(askama::Error::from(std::fmt::Error)); + } + + let mut inline_cells = Vec::new(); + let mut block_sections = Vec::new(); + for (col, src) in columns.iter().zip(cells.iter()) { + let body = render_cell_body(src)?; + if col.inline { + inline_cells.push(DetailField { + name: col.name.clone(), + label: col.caption.clone(), + css_classes: src.css_classes.to_string(), + body_html: body, + }); + } else { + block_sections.push(DetailSection { + name: col.name.clone(), + label: col.caption.clone(), + css_classes: src.css_classes.to_string(), + body_html: body, + }); + } + } + + HtmlDetailViewCtx { + class_id_hex: format!("0x{class_id:04X}"), + canonical_concept: canonical_concept.to_string(), + record_id, + headline_html: headline_html.to_string(), + subtitle: subtitle.to_string(), + inline_cells, + block_sections, + } + .render() +} + +/// Same per-kind dispatch as the list view (kept duplicated here rather +/// than re-exported because the list-view side has crate-internal naming; +/// once T3 + T4 stabilise we factor the dispatch into one helper). +fn render_cell_body(src: &CellSource<'_>) -> Result { + Ok(match &src.data { + CellData::Plain { value } => PlainCell { value }.render()?, + CellData::IdLink { id, href } => IdLinkCell { id: *id, href }.render()?, + CellData::PrimaryLink { label, href } => PrimaryLinkCell { label, href }.render()?, + CellData::RecordRef { label, href, target_concept } => RecordRefCell { + label, + href, + target_concept, + } + .render()?, + CellData::RichText { body } => RichTextCell { body }.render()?, + CellData::ProgressBar { pct } => ProgressBarCell { pct: *pct }.render()?, + CellData::RelationList { relations } => { + let mapped: Vec> = relations + .iter() + .map(|r| RelationEntry { id: r.id, kind: r.kind.as_str(), href: r.href.as_str() }) + .collect(); + RelationListCell { relations: mapped }.render()? + } + CellData::Hours { hours, href } => HoursCell { hours, href }.render()?, + CellData::AttachmentList { attachments } => { + let mapped: Vec> = attachments + .iter() + .map(|a| AttachmentEntry { filename: a.filename.as_str(), href: a.href.as_str() }) + .collect(); + AttachmentListCell { attachments: mapped }.render()? + } + CellData::UserList { users } => { + let mapped: Vec> = users + .iter() + .map(|u| UserEntry { name: u.name.as_str(), href: u.href.as_str() }) + .collect(); + UserListCell { users: mapped }.render()? + } + }) +} + +/// The codebook-only dispatch entry point used by [`for_kind`](super::for_kind). +/// Synthesises a no-data detail page from the class's attributes (proof +/// of shape); real callers use [`render_detail`] with real cell data. +pub struct HtmlDetailViewEmitter; + +impl ArtifactEmitter for HtmlDetailViewEmitter { + fn emit(&self, spec: &ArtifactSpec<'_>) -> Result { + use crate::list_view::default_kind_for; + let class = spec.class; + let concept = class.canonical_concept.as_deref().unwrap_or(""); + let class_id = canonical_concept_id(concept).unwrap_or(0); + + // Synthesise columns from the class's attributes. + let columns: Vec = class + .attributes + .iter() + .map(|attr| { + let kind = default_kind_for(&attr.name, attr.type_name.as_deref()); + let col = RenderColumn::new(&attr.name, &attr.name, kind); + if matches!(kind, ColumnKind::RichText) { + col.block() + } else { + col + } + }) + .collect(); + + // Placeholder values, kept alive across the render. One per column. + let placeholders: Vec = columns.iter().map(|_| "—".to_string()).collect(); + + // Pair columns with placeholder cells. + let cells: Vec> = columns + .iter() + .zip(placeholders.iter()) + .map(|(col, p)| { + let data = if matches!(col.kind, ColumnKind::RichText) { + CellData::RichText { body: p.as_str() } + } else { + CellData::Plain { value: p.as_str() } + }; + CellSource { + column: col, + css_classes: "muted", + data, + } + }) + .collect(); + + render_detail(class_id, concept, 0, "", "", &columns, &cells) + } +} diff --git a/crates/ogar-render-askama/src/artifact_kinds/html_list_view.rs b/crates/ogar-render-askama/src/artifact_kinds/html_list_view.rs index e4b6d9f..7342600 100644 --- a/crates/ogar-render-askama/src/artifact_kinds/html_list_view.rs +++ b/crates/ogar-render-askama/src/artifact_kinds/html_list_view.rs @@ -30,7 +30,7 @@ use ogar_vocab::canonical_concept_id; // ── Spine binding struct ───────────────────────────────────────────── #[derive(Template)] -#[template(path = "dispatch/html_list_view.askama", escape = "none")] +#[template(path = "dispatch/html_list_view.askama", escape = "html")] struct HtmlListViewCtx { title: String, class_id_hex: String, diff --git a/crates/ogar-render-askama/src/artifact_kinds/mod.rs b/crates/ogar-render-askama/src/artifact_kinds/mod.rs index f9739cb..41a5db8 100644 --- a/crates/ogar-render-askama/src/artifact_kinds/mod.rs +++ b/crates/ogar-render-askama/src/artifact_kinds/mod.rs @@ -24,10 +24,12 @@ pub(crate) mod cells; use crate::spec::{ArtifactKind, ArtifactSpec}; +pub mod html_detail_view; pub mod html_list_view; pub mod rust_struct; pub mod stub; +pub use html_detail_view::{render_detail, HtmlDetailViewEmitter}; pub use html_list_view::{ render_list, AttachmentEntryOwned, CellData, CellSource, GroupHeader, HtmlListViewEmitter, RelationEntryOwned, RowSource, UserEntryOwned, @@ -47,6 +49,7 @@ pub fn for_kind(kind: ArtifactKind) -> Box { match kind { ArtifactKind::RustStruct => Box::new(rust_struct::RustStructEmitter), ArtifactKind::HtmlListView => Box::new(html_list_view::HtmlListViewEmitter), + ArtifactKind::HtmlDetailView => Box::new(html_detail_view::HtmlDetailViewEmitter), other => Box::new(stub::Stub { kind: other }), } } diff --git a/crates/ogar-render-askama/src/lib.rs b/crates/ogar-render-askama/src/lib.rs index c1a6fb7..6696ba7 100644 --- a/crates/ogar-render-askama/src/lib.rs +++ b/crates/ogar-render-askama/src/lib.rs @@ -73,8 +73,8 @@ pub mod list_view; pub mod spec; pub use artifact_kinds::{ - for_kind, render_list, ArtifactEmitter, AttachmentEntryOwned, CellData, CellSource, - GroupHeader, RelationEntryOwned, RowSource, UserEntryOwned, + for_kind, render_detail, render_list, ArtifactEmitter, AttachmentEntryOwned, CellData, + CellSource, GroupHeader, RelationEntryOwned, RowSource, UserEntryOwned, }; pub use list_view::{default_kind_for, ColumnKind, RenderColumn, SortOrder}; pub use spec::{ArtifactKind, ArtifactSpec}; @@ -126,12 +126,13 @@ mod tests { assert!( all.contains(&ArtifactKind::RustStruct) && all.contains(&ArtifactKind::HtmlListView) + && all.contains(&ArtifactKind::HtmlDetailView) && all.contains(&ArtifactKind::SurrealqlTable) && all.contains(&ArtifactKind::OpenapiSchema) && all.contains(&ArtifactKind::NodeGuidRoutingArm), "ArtifactKind::ALL missing a variant" ); - assert_eq!(all.len(), 5); + assert_eq!(all.len(), 6); } #[test] @@ -332,6 +333,267 @@ mod tests { assert!(src.contains("5"), "{src}"); } + // ── XSS regression — codex P1 on #83 + #84 ────────────────────── + + #[test] + fn html_list_view_escapes_data_derived_strings_xss_regression() { + // Codex P1 on PR #83: the spine template was compiled with + // `escape = "none"`, so every interpolation was raw — a malicious + // group-header label, title, caption, or css_class would inject + // raw HTML into the page. The fix is `escape = "html"` on the + // spine binding + `|safe` only on the intentionally-pre-rendered + // `cell.body_html`. This test pins the contract: untrusted + // strings get escaped; only cell bodies are raw. + let col = RenderColumn::new( + "subject", + "", // poisoned caption + ColumnKind::PrimaryLink, + ); + let row = RowSource { + record_id: 1, + css_classes: "", + group: Some(GroupHeader { + label: "", + count: 0, + }), + inline: vec![CellSource { + column: &col, + css_classes: "", + data: CellData::PrimaryLink { + label: "label", + href: "/i/1", + }, + }], + block: vec![], + }; + let src = render_list( + "x", + 0x0102, + "project_work_item", + std::slice::from_ref(&col), + &[], + std::slice::from_ref(&row), + ) + .unwrap(); + + // Untrusted strings MUST be escaped — no raw `", + ColumnKind::PrimaryLink, + ); + let block = RenderColumn::new( + "description", + "", + ColumnKind::RichText, + ) + .block(); + let columns = vec![col, block]; + let cells = vec![ + CellSource { + column: &columns[0], + css_classes: "", + data: CellData::PrimaryLink { + label: "x", + href: "/s/1", + }, + }, + CellSource { + column: &columns[1], + css_classes: "", + data: CellData::RichText { + body: "

Trusted prose.

", // intentional HTML + }, + }, + ]; + + let src = render_detail( + 0x0102, + "project_work_item", + 42, + // headline_html — intentional HTML, gets through + "Headline", + // subtitle — data-derived, MUST be escaped + "", + &columns, + &cells, + ) + .unwrap(); + + // Subtitle XSS attempt must not survive. + assert!( + !src.contains("alert('block-label')"), + "block section label was rendered raw — XSS hazard:\n{src}" + ); + // Inline field labels must be escaped. + assert!( + !src.contains("