From 1bb3c649a931795bc8c3ac82caef49e6d354c2a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 20 Jun 2026 00:44:29 +0000 Subject: [PATCH 1/2] =?UTF-8?q?feat(ogar-render-askama):=20T3=20=E2=80=94?= =?UTF-8?q?=20HtmlDetailView=20(Redmine=20show.html.erb=20on=20ClassView)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third real emitter per Northstar plan §3, the detail-page sibling of T2's HtmlListView. Mirrors Redmine's `app/views/issues/show.html.erb` shape: definition-list (dt/dd) for inline typed fields + full-width
blocks for prose / family-edge collections. Reuses every piece T2 built — RenderColumn, ColumnKind, the 10 cell sub- templates, the two-stage cells-pre-rendered-in-Rust pattern. T3 is just a different spine template; the substrate-agnostic property holds. Surface: - ArtifactKind::HtmlDetailView variant (slot 2, append-only). - templates/dispatch/html_detail_view.askama — spine (
+ header +
+ per-block
s). - artifact_kinds::html_detail_view::render_detail(class_id, concept, record_id, headline_html, subtitle, columns, cells) — the real entry point. - HtmlDetailViewEmitter — codebook-only proof-of-shape for the for_kind dispatch path. Arity contract: `columns.len() == cells.len()`; the emitter returns an askama::Error on mismatch (caller-side bug, pinned by test). Tests (+3 T3 specific, 23/23 total): - html_detail_view_proof_of_shape_renders_dl_with_class_meta — codebook emit on project_work_item; surfaces data-class-id + data-concept + detail-field- per attribute. - html_detail_view_renders_inline_dl_and_block_sections — substantive render with PrimaryLink + ProgressBar inline + RichText block; pins record-id, headline, subtitle, inline cells via per-kind sub-templates, block
for description, and a NEGATIVE assert that the block didn't leak into the inline
. - html_detail_view_column_cell_arity_mismatch_returns_error — caller- side contract. Workspace check + test green. Cell-renderer dispatch duplicated between list_view and detail_view emitters today; once T4 (HtmlForm) lands we factor it into one helper. Both pipelines feed the same per-kind sub-templates from artifact_kinds::cells. --- .../src/artifact_kinds/html_detail_view.rs | 210 ++++++++++++++++++ .../src/artifact_kinds/mod.rs | 3 + crates/ogar-render-askama/src/lib.rs | 105 ++++++++- crates/ogar-render-askama/src/spec.rs | 7 + .../dispatch/html_detail_view.askama | 34 +++ 5 files changed, 356 insertions(+), 3 deletions(-) create mode 100644 crates/ogar-render-askama/src/artifact_kinds/html_detail_view.rs create mode 100644 crates/ogar-render-askama/templates/dispatch/html_detail_view.askama 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..65979ea --- /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 = "none")] +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/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..e31a894 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,104 @@ mod tests { assert!(src.contains("5"), "{src}"); } + // ── T3 (HtmlDetailView) tests ─────────────────────────────────── + + #[test] + fn html_detail_view_proof_of_shape_renders_dl_with_class_meta() { + // Codebook-only emit: synthesised dl from class attributes, "—" + // placeholders, no headline. Pins data-class-id + data-concept. + let class = project_work_item(); + let src = render(&class, ArtifactKind::HtmlDetailView).unwrap(); + assert!( + src.contains("data-class-id=\"0x0102\""), + "expected data-class-id in:\n{src}" + ); + assert!(src.contains("data-concept=\"project_work_item\"")); + assert!(src.contains("
"), "{src}"); + // Every typed attribute should land as a detail-field-. + for attr in &class.attributes { + assert!( + src.contains(&format!("detail-field-{}", attr.name)), + "missing detail-field-{} in:\n{src}", + attr.name + ); + } + } + + #[test] + fn html_detail_view_renders_inline_dl_and_block_sections() { + let inline = RenderColumn::new("status", "Status", ColumnKind::PrimaryLink); + let pct = RenderColumn::new("done_ratio", "% Done", ColumnKind::ProgressBar); + let block_desc = + RenderColumn::new("description", "Description", ColumnKind::RichText).block(); + + let columns = vec![inline.clone(), pct.clone(), block_desc.clone()]; + let cells = vec![ + CellSource { + column: &columns[0], + css_classes: "", + data: CellData::PrimaryLink { + label: "Open", + href: "/statuses/1", + }, + }, + CellSource { + column: &columns[1], + css_classes: "num", + data: CellData::ProgressBar { pct: 60 }, + }, + CellSource { + column: &columns[2], + css_classes: "", + data: CellData::RichText { + body: "

Detailed body here.

", + }, + }, + ]; + + let src = render_detail( + 0x0102, + "project_work_item", + 42, + "Fix the foo", + "Open · High", + &columns, + &cells, + ) + .unwrap(); + + // Header + assert!(src.contains("data-record-id=\"42\""), "{src}"); + assert!(src.contains("class=\"detail-id\">#42"), "{src}"); + assert!(src.contains("Fix the foo"), "headline missing in:\n{src}"); + assert!(src.contains("Open · High"), "subtitle missing in:\n{src}"); + // Inline dl entries + assert!(src.contains("detail-field-status"), "{src}"); + assert!(src.contains("detail-field-done_ratio"), "{src}"); + // Inline cells render through the per-kind sub-templates. + assert!(src.contains("href=\"/statuses/1\""), "{src}"); + assert!(src.contains("aria-valuenow=\"60\""), "{src}"); + // Block section for description + assert!(src.contains("detail-section-description"), "{src}"); + assert!(src.contains("
`. + // (Negative pin: detail-field-description should not exist.) + assert!( + !src.contains("detail-field-description"), + "block field leaked into inline dl in:\n{src}" + ); + } + + #[test] + fn html_detail_view_column_cell_arity_mismatch_returns_error() { + let col = RenderColumn::new("subject", "Subject", ColumnKind::PrimaryLink); + let columns = vec![col]; + let cells: Vec> = vec![]; // intentional mismatch + let r = render_detail(0x0102, "project_work_item", 1, "", "", &columns, &cells); + assert!(r.is_err(), "expected mismatch to error, got Ok:\n{r:?}"); + } + #[test] fn default_kind_resolver_is_wired_through_render_kit() { // Smoke: the resolver lib.rs re-exports is the one consumers diff --git a/crates/ogar-render-askama/src/spec.rs b/crates/ogar-render-askama/src/spec.rs index 4e93cf9..6750895 100644 --- a/crates/ogar-render-askama/src/spec.rs +++ b/crates/ogar-render-askama/src/spec.rs @@ -23,6 +23,11 @@ pub enum ArtifactKind { /// `ClassView` substrate. Spec: `docs/integration/REDMINE-QUERY- /// HARVEST.md` §3. HtmlListView, + /// Single-record HTML detail view: definition-list inline fields + + /// `
` blocks for prose / family-edge collections. **Render + /// flavour**; mirror of Redmine's `show.html.erb`. Reuses every + /// `RenderColumn` / `ColumnKind` / cell sub-template from T2. + HtmlDetailView, /// SurrealQL `DEFINE TABLE` + per-field `DEFINE FIELD` statements. /// **Codegen flavour** — DB engine is the final consumer. SurrealqlTable, @@ -44,6 +49,7 @@ impl ArtifactKind { pub const ALL: &'static [Self] = &[ Self::RustStruct, Self::HtmlListView, + Self::HtmlDetailView, Self::SurrealqlTable, Self::OpenapiSchema, Self::NodeGuidRoutingArm, @@ -55,6 +61,7 @@ impl ArtifactKind { match self { Self::RustStruct => "rust_struct", Self::HtmlListView => "html_list_view", + Self::HtmlDetailView => "html_detail_view", Self::SurrealqlTable => "surrealql_table", Self::OpenapiSchema => "openapi_schema", Self::NodeGuidRoutingArm => "node_guid_routing_arm", diff --git a/crates/ogar-render-askama/templates/dispatch/html_detail_view.askama b/crates/ogar-render-askama/templates/dispatch/html_detail_view.askama new file mode 100644 index 0000000..b0807ff --- /dev/null +++ b/crates/ogar-render-askama/templates/dispatch/html_detail_view.askama @@ -0,0 +1,34 @@ +{# Detail view for a single canonical record. Same `RenderColumn` shape #} +{# as the list view, just laid out as a definition list (dt/dd) for #} +{# inline fields + full-width sections for block fields and family-edge #} +{# collections. Substrate-agnostic — driven by the binding-struct, no #} +{# class name hardcoded. #} +
+
+

+ #{{ record_id }} + {{ headline_html|safe }} +

+ {%- if !subtitle.is_empty() %} +

{{ subtitle }}

+ {%- endif %} +
+ +
+ {%- for cell in inline_cells %} +
+
{{ cell.label }}
+
{{ cell.body_html|safe }}
+
+ {%- endfor %} +
+ + {%- for section in block_sections %} +
+

{{ section.label }}

+
+ {{ section.body_html|safe }} +
+
+ {%- endfor %} +
From 2b3fb88b4e23613fc3dbbe8b801e46c41db3f1a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 20 Jun 2026 01:09:34 +0000 Subject: [PATCH 2/2] =?UTF-8?q?fix(ogar-render-askama):=20codex=20P1=20?= =?UTF-8?q?=E2=80=94=20re-enable=20HTML=20escaping=20on=20spine=20template?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex P1 on PR #83 (list spine) + PR #84 (detail spine), same root issue: both spine templates were compiled with `escape = "none"`, so EVERY interpolation was raw — a malicious group-header label, title, caption, subtitle, or css_class would inject raw HTML into the page (XSS hazard). Fix: `escape = "html"` on the spine binding-struct attribute. The templates already use `|safe` correctly on the intentionally-pre-rendered HTML fields (`cell.body_html`, `headline_html`, `section.body_html`), so only those bypass escaping. Every other interpolation (title, captions, labels, css_classes, group header labels, subtitle) now gets HTML-escaped by askama. Two-layer escaping for cell content is consistent: - The cell sub-template (e.g. PrimaryLinkCell) escapes its variables (label, href, …) under its own `escape = "html"`. - The spine receives the already-escaped HTML as `body_html` and passes it through with `|safe` (no double-escape). `escape = "none"` retained only on: - `rust_struct.askama` — emits Rust source where `<` etc. must NOT be HTML-escaped. - `cell/rich_text.askama` — wraps trusted pre-rendered prose (Markdown/Textile already expanded upstream). Tests (+2 XSS regressions, 25/25 total): - html_list_view_escapes_data_derived_strings_xss_regression — pokes every untrusted slot (title, caption, group label, css_classes) with XSS payloads (`", // 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("