From c34fe832bd8918587991a939a2627f371ce3bd1c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 20:27:32 -0500 Subject: [PATCH 1/2] fix(documents): bound DOCX table rendering --- CHANGELOG.md | 6 + crates/fetchkit/Cargo.toml | 2 +- crates/fetchkit/src/documents/markkit.rs | 45 ++- crates/fetchkit/src/documents/mod.rs | 2 + crates/fetchkit/src/documents/office.rs | 382 +++++++++++++++++++---- crates/fetchkit/src/documents/tables.rs | 298 ++++++++++++++++++ docs/hardening.md | 14 + knowledge/foundations/fetchers.md | 14 +- knowledge/foundations/tool-contract.md | 6 + knowledge/log.md | 2 + knowledge/security/threat-model.md | 18 +- 11 files changed, 717 insertions(+), 72 deletions(-) create mode 100644 crates/fetchkit/src/documents/tables.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 7eff1b8..2dfa8ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 content focus. Custom processors constructing this public input must populate the new fields. +### Fixed + +- Bound DOCX table dimensions and padded cell work before local conversion, + including the default markkit backend. The deprecated Office renderer admits + Markdown bytes before escaping or padding into one capped buffer. + ## [0.5.0] - 2026-07-14 ### Highlights diff --git a/crates/fetchkit/Cargo.toml b/crates/fetchkit/Cargo.toml index 371673b..d5f900c 100644 --- a/crates/fetchkit/Cargo.toml +++ b/crates/fetchkit/Cargo.toml @@ -15,7 +15,7 @@ default = ["docs-pdf", "docs-markkit"] # Document conversion backends (see knowledge/foundations/fetchers.md) docs-pdf = ["dep:pdf-inspector"] # Office documents (DOCX, XLSX/XLSM, XLS, ODS) via markkit. -docs-markkit = ["dep:markkit"] +docs-markkit = ["dep:markkit", "dep:zip", "dep:quick-xml"] # Deprecated in-crate Office converter (DOCX, PPTX, spreadsheets); use docs-markkit. docs-office = ["dep:calamine", "dep:zip", "dep:quick-xml"] docs-cloudflare = [] diff --git a/crates/fetchkit/src/documents/markkit.rs b/crates/fetchkit/src/documents/markkit.rs index 45b57bb..9e827e1 100644 --- a/crates/fetchkit/src/documents/markkit.rs +++ b/crates/fetchkit/src/documents/markkit.rs @@ -9,6 +9,8 @@ //! fetchkit adds the blocking-pool two-document concurrency cap and caps the final //! Markdown at `max_body_size`. Callers tune markkit limits through //! [`MarkkitConverter::with_markkit`]. +//! - DOCX tables pass Fetchkit's archive preflight before markkit parses them: +//! sheet limits in markkit do not apply to DOCX padding (TM-DOS-008). //! - The confirmed fetchkit format's canonical media type is passed as the hint, so //! markkit parses the same format fetchkit detected and magic-checked. //! - markkit warnings are `code: detail`; only the stable code is surfaced as a @@ -107,20 +109,25 @@ impl DocumentConverter for MarkkitConverter { } let markkit = Arc::clone(&self.markkit); let media_type = input.format.media_type(); + let is_docx = input.format == DocumentFormat::Docx; let DocumentInput { body, file_name, .. } = input; let extraction = run_document_task(move || { - markkit.extract_document_sync( - &body, - DocumentHint { - content_type: Some(media_type), - file_name: Some(&file_name), - }, - ) + if is_docx { + super::tables::check_archive(&body, markkit.config())?; + } + markkit + .extract_document_sync( + &body, + DocumentHint { + content_type: Some(media_type), + file_name: Some(&file_name), + }, + ) + .map_err(map_error) }) - .await? - .map_err(map_error)?; + .await??; let mut warnings: Vec = Vec::new(); for warning in &extraction.warnings { @@ -279,6 +286,26 @@ mod tests { assert!(converted.warnings.is_empty()); } + #[tokio::test] + async fn converts_docx_with_relationship_selected_main_part() { + let body = zip(&[ + ( + "_rels/.rels", + r#""#, + ), + ( + "parts/custom.data", + r#"OwnerAnn"#, + ), + ("word/media/image.png", "ordinary binary fixture"), + ]); + let converted = MarkkitConverter::new() + .convert(input(DocumentFormat::Docx, body)) + .await + .unwrap(); + assert!(converted.markdown.contains("| Owner |\n| --- |\n| Ann |")); + } + #[tokio::test] async fn converts_xlsx_and_reports_warning_codes() { let converted = MarkkitConverter::new() diff --git a/crates/fetchkit/src/documents/mod.rs b/crates/fetchkit/src/documents/mod.rs index ee336c8..14509cb 100644 --- a/crates/fetchkit/src/documents/mod.rs +++ b/crates/fetchkit/src/documents/mod.rs @@ -32,6 +32,8 @@ mod pdf_inspector; #[cfg(any(feature = "docs-cloudflare", feature = "docs-http"))] mod remote; mod settings; +#[cfg(any(feature = "docs-markkit", feature = "docs-office"))] +mod tables; use std::collections::BTreeMap; use std::fmt; diff --git a/crates/fetchkit/src/documents/office.rs b/crates/fetchkit/src/documents/office.rs index 3431f92..53bc50c 100644 --- a/crates/fetchkit/src/documents/office.rs +++ b/crates/fetchkit/src/documents/office.rs @@ -13,8 +13,9 @@ //! inflated, the central directory is checked against an entry-count and a total //! uncompressed-size budget, and every part fetchkit reads itself is read through a //! size-capped reader, so a small zip bomb within `max_body_size` cannot expand -//! without limit. Spreadsheet output is capped per sheet (rows and columns) and the -//! caller caps the final Markdown at `max_body_size`. Parsing runs on a blocking +//! without limit. DOCX tables reuse the sheet dimensions and a checked padded-cell +//! budget. DOCX Markdown is admitted into one 10 MiB buffer before escaping or +//! padding; the caller may apply a smaller `max_body_size`. Parsing runs on a blocking //! task under the same two-document concurrency cap as PDFs. //! - Deprecated in favour of `MarkkitConverter` (`docs-markkit`), the default Office //! backend. Kept behind the non-default `docs-office` feature for one release; it @@ -29,6 +30,7 @@ use calamine::{open_workbook_auto_from_rs, Data, Reader}; use quick_xml::events::{BytesStart, Event}; use quick_xml::Reader as XmlReader; +use super::tables::{TableLimits as DocxLimits, TABLE_LIMITS as DOCX_LIMITS}; use super::{ run_document_task, ConvertError, ConvertedDocument, DocumentConverter, DocumentFormat, DocumentInput, Locality, @@ -44,9 +46,9 @@ const MAX_PART_BYTES: u64 = 64 * 1024 * 1024; /// Maximum slides read from one presentation. const MAX_SLIDES: usize = 1_000; /// Maximum rows rendered per spreadsheet sheet. -const MAX_SHEET_ROWS: usize = 1_000; +const MAX_SHEET_ROWS: usize = DOCX_LIMITS.rows; /// Maximum columns rendered per spreadsheet sheet. -const MAX_SHEET_COLUMNS: usize = 50; +const MAX_SHEET_COLUMNS: usize = DOCX_LIMITS.columns; /// Office document kinds with distinct extraction paths. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -287,16 +289,19 @@ struct DocxParagraph { } impl DocxParagraph { - fn render(self) -> Option { + fn render(self, limit: usize) -> Result, ContentProcessorError> { let text = self.text.trim(); if text.is_empty() { - return None; + return Ok(None); } - Some(match (self.heading, self.list_level) { - (Some(level), _) => format!("{} {text}", "#".repeat(level)), - (None, Some(level)) => format!("{}- {text}", " ".repeat(level)), - (None, None) => text.to_string(), - }) + let prefix = match (self.heading, self.list_level) { + (Some(level), _) => format!("{} ", "#".repeat(level)), + (None, Some(level)) => format!("{}- ", " ".repeat(level)), + (None, None) => String::new(), + }; + let mut rendered = prefix; + append_bounded(&mut rendered, text, limit)?; + Ok(Some(rendered)) } } @@ -316,15 +321,25 @@ fn heading_level(style: &str) -> Option { } fn docx_to_markdown(xml: &str) -> Result { + docx_to_markdown_with_limits(xml, DOCX_LIMITS) +} + +fn docx_to_markdown_with_limits( + xml: &str, + limits: DocxLimits, +) -> Result { let mut reader = XmlReader::from_str(xml); - let mut blocks: Vec = Vec::new(); + let mut output = String::new(); let mut paragraph: Option = None; let mut in_text = false; // Tables: rows of cells, each cell a list of paragraph texts. Nested tables are // flattened into their parent cell. let mut table_depth = 0usize; let mut rows: Vec> = Vec::new(); - let mut cell: Option> = None; + let mut cell: Option = None; + let mut width = 0; + let mut source_cells = 0; + let mut table_text_bytes = 0; loop { match reader.read_event().map_err(xml_error)? { @@ -335,45 +350,80 @@ fn docx_to_markdown(xml: &str) -> Result { table_depth += 1; if table_depth == 1 { rows.clear(); + width = 0; + source_cells = 0; + table_text_bytes = 0; } } - b"tr" if table_depth == 1 => rows.push(Vec::new()), - b"tc" if table_depth == 1 => cell = Some(Vec::new()), - _ => docx_property(&start, &mut paragraph), + b"tr" if table_depth == 1 => { + check_table_shape(rows.len() + 1, width, limits)?; + rows.push(Vec::new()); + } + b"tc" if table_depth > 0 => { + source_cells = checked_size(source_cells, 1, limits.cells)?; + if table_depth == 1 { + width = width.max(rows.last().map_or(1, |row| row.len() + 1)); + check_table_shape(rows.len(), width, limits)?; + cell = Some(String::new()); + } + } + _ => docx_property(&start, &mut paragraph, limits.output_bytes)?, }, // Self-closing elements never open a container: an empty `` or // `` has no matching End event. - Event::Empty(start) => docx_property(&start, &mut paragraph), + Event::Empty(start) => docx_property(&start, &mut paragraph, limits.output_bytes)?, Event::Text(text) if in_text => { let text = text.xml10_content().map_err(|e| xml_error(e.into()))?; - push_paragraph_text(&mut paragraph, &text); + if let Some(p) = &mut paragraph { + append_bounded(&mut p.text, &text, limits.output_bytes)?; + } } Event::GeneralRef(reference) if in_text => { if let Some(p) = &mut paragraph { - push_reference(&mut p.text, &reference); + let mut text = String::new(); + push_reference(&mut text, &reference); + append_bounded(&mut p.text, &text, limits.output_bytes)?; } } Event::End(end) => match end.local_name().as_ref() { b"t" => in_text = false, b"p" => { - if let Some(rendered) = paragraph.take().and_then(DocxParagraph::render) { + if let Some(rendered) = paragraph + .take() + .map(|p| p.render(limits.output_bytes)) + .transpose()? + .flatten() + { match &mut cell { - Some(cell) => cell.push(rendered), - None => blocks.push(rendered), + Some(cell) => { + let separator = if cell.is_empty() { "" } else { "
" }; + table_text_bytes = checked_size( + table_text_bytes, + separator.len(), + limits.output_bytes, + )?; + table_text_bytes = checked_size( + table_text_bytes, + rendered.len(), + limits.output_bytes, + )?; + append_bounded(cell, separator, limits.output_bytes)?; + append_bounded(cell, &rendered, limits.output_bytes)?; + } + None => append_block(&mut output, &rendered, limits.output_bytes)?, } } } b"tc" if table_depth == 1 => { if let (Some(done), Some(row)) = (cell.take(), rows.last_mut()) { - row.push(done.join("
")); + row.push(done); } } b"tbl" => { table_depth = table_depth.saturating_sub(1); if table_depth == 0 { - if let Some(table) = markdown_table(&std::mem::take(&mut rows)) { - blocks.push(table); - } + append_table(&rows, &mut output, limits)?; + rows.clear(); } } _ => {} @@ -382,13 +432,17 @@ fn docx_to_markdown(xml: &str) -> Result { _ => {} } } - Ok(blocks.join("\n\n")) + Ok(output) } /// Paragraph properties and inline breaks, which may be self-closing. -fn docx_property(start: &BytesStart<'_>, paragraph: &mut Option) { +fn docx_property( + start: &BytesStart<'_>, + paragraph: &mut Option, + limit: usize, +) -> Result<(), ContentProcessorError> { let Some(p) = paragraph else { - return; + return Ok(()); }; match start.local_name().as_ref() { b"pStyle" => { @@ -405,16 +459,11 @@ fn docx_property(start: &BytesStart<'_>, paragraph: &mut Option) .unwrap_or(0usize); p.list_level = Some(level.min(8)); } - b"tab" => p.text.push('\t'), - b"br" | b"cr" => p.text.push('\n'), + b"tab" => append_bounded(&mut p.text, "\t", limit)?, + b"br" | b"cr" => append_bounded(&mut p.text, "\n", limit)?, _ => {} } -} - -fn push_paragraph_text(paragraph: &mut Option, text: &str) { - if let Some(p) = paragraph { - p.text.push_str(text); - } + Ok(()) } /// Slide part names in presentation order, read from `ppt/presentation.xml` @@ -601,7 +650,7 @@ fn spreadsheet_to_markdown(body: &[u8]) -> Result String { /// Render rows as a Markdown table, the first row as the header. Rows are padded /// to the widest row; fully empty rows and trailing empty columns are dropped. -fn markdown_table(rows: &[Vec]) -> Option { - let rows: Vec<&Vec> = rows - .iter() - .filter(|row| row.iter().any(|cell| !cell.trim().is_empty())) - .collect(); +fn markdown_table(rows: &[Vec]) -> Result, ContentProcessorError> { + let mut output = String::new(); + append_table(rows, &mut output, DOCX_LIMITS)?; + Ok((!output.is_empty()).then_some(output)) +} + +fn limit_error() -> ContentProcessorError { + ContentProcessorError("Office table or DOCX output exceeds the conversion limit".into()) +} + +fn checked_size( + current: usize, + extra: usize, + limit: usize, +) -> Result { + current + .checked_add(extra) + .filter(|size| *size <= limit) + .ok_or_else(limit_error) +} + +fn check_table_shape( + rows: usize, + width: usize, + limits: DocxLimits, +) -> Result<(), ContentProcessorError> { + if rows > limits.rows + || width > limits.columns + || rows + .checked_mul(width) + .is_none_or(|cells| cells > limits.cells) + { + return Err(limit_error()); + } + Ok(()) +} + +fn reserve_bounded( + out: &mut String, + extra: usize, + limit: usize, +) -> Result<(), ContentProcessorError> { + let size = checked_size(out.len(), extra, limit)?; + if size > out.capacity() { + let capacity = out.capacity().saturating_mul(2).max(size).min(limit); + out.try_reserve_exact(capacity - out.len()) + .map_err(|_| limit_error())?; + } + Ok(()) +} + +fn append_bounded(out: &mut String, text: &str, limit: usize) -> Result<(), ContentProcessorError> { + reserve_bounded(out, text.len(), limit)?; + out.push_str(text); + Ok(()) +} + +fn append_block(out: &mut String, block: &str, limit: usize) -> Result<(), ContentProcessorError> { + let separator = if out.is_empty() { "" } else { "\n\n" }; + reserve_bounded( + out, + checked_size(block.len(), separator.len(), limit)?, + limit, + )?; + out.push_str(separator); + out.push_str(block); + Ok(()) +} + +fn nonempty_row(row: &&Vec) -> bool { + row.iter().any(|cell| !cell.trim().is_empty()) +} + +fn append_table( + rows: &[Vec], + out: &mut String, + limits: DocxLimits, +) -> Result<(), ContentProcessorError> { + let count = rows.iter().filter(nonempty_row).count(); let width = rows .iter() .map(|row| { @@ -660,25 +783,90 @@ fn markdown_table(rows: &[Vec]) -> Option { .map_or(0, |last| last + 1) }) .max() - .filter(|width| *width > 0)?; - let render_row = |row: &Vec| { - let cells: Vec = (0..width) - .map(|index| escape_cell(row.get(index).map(String::as_str).unwrap_or(""))) - .collect(); - format!("| {} |", cells.join(" | ")) - }; - let mut lines = vec![render_row(rows[0])]; - lines.push(format!("|{}", " --- |".repeat(width))); - lines.extend(rows[1..].iter().map(|row| render_row(row))); - Some(lines.join("\n")) + .unwrap_or(0); + check_table_shape(rows.len(), width, limits)?; + if width == 0 { + return Ok(()); + } + let mut bytes = if out.is_empty() { 0 } else { 2 }; + // Admission precedes all escaping, padding and allocation. Include every + // delimiter, newline and UTF-8 byte in the single output buffer. + for row in rows.iter().filter(nonempty_row) { + bytes = checked_size( + bytes, + width + .checked_mul(3) + .and_then(|n| n.checked_add(1)) + .ok_or_else(limit_error)?, + limits.output_bytes, + )?; + for cell in row.iter().take(width) { + visit_escaped_cell(cell, |piece| { + bytes = checked_size(bytes, piece.len(), limits.output_bytes)?; + Ok(()) + })?; + } + } + bytes = checked_size( + bytes, + width + .checked_mul(6) + .and_then(|n| n.checked_add(1)) + .ok_or_else(limit_error)?, + limits.output_bytes, + )?; + bytes = checked_size(bytes, count, limits.output_bytes)?; + reserve_bounded(out, bytes, limits.output_bytes)?; + if !out.is_empty() { + out.push_str("\n\n"); + } + for (index, row) in rows.iter().filter(nonempty_row).enumerate() { + if index > 0 { + out.push('\n'); + } + out.push_str("| "); + for column in 0..width { + if column > 0 { + out.push_str(" | "); + } + visit_escaped_cell(row.get(column).map(String::as_str).unwrap_or(""), |piece| { + out.push_str(piece); + Ok(()) + })?; + } + out.push_str(" |"); + if index == 0 { + out.push_str("\n|"); + for _ in 0..width { + out.push_str(" --- |"); + } + } + } + Ok(()) } -fn escape_cell(cell: &str) -> String { - cell.trim() - .replace('\\', "\\\\") - .replace('|', "\\|") - .replace("\r\n", "
") - .replace(['\n', '\r'], "
") +fn visit_escaped_cell( + cell: &str, + mut visit: impl FnMut(&str) -> Result<(), ContentProcessorError>, +) -> Result<(), ContentProcessorError> { + let mut chars = cell.trim().chars().peekable(); + while let Some(ch) = chars.next() { + let mut utf8 = [0; 4]; + let piece = match ch { + '\\' => "\\\\", + '|' => "\\|", + '\r' => { + if chars.peek() == Some(&'\n') { + chars.next(); + } + "
" + } + '\n' => "
", + other => other.encode_utf8(&mut utf8), + }; + visit(piece)?; + } + Ok(()) } #[cfg(test)] @@ -748,6 +936,80 @@ mod tests { const CORE_XML: &str = r#" Q3 & outlook"#; + fn small_table() -> &'static str { + r#" +ABC +D +"# + } + + #[test] + fn docx_table_limits_admit_before_rendering() { + for limits in [ + DocxLimits { + rows: 1, + ..DOCX_LIMITS + }, + DocxLimits { + columns: 2, + ..DOCX_LIMITS + }, + DocxLimits { + cells: 5, + ..DOCX_LIMITS + }, + DocxLimits { + output_bytes: 32, + ..DOCX_LIMITS + }, + ] { + let error = docx_to_markdown_with_limits(small_table(), limits).unwrap_err(); + assert!(error.0.contains("limit"), "{error}"); + } + } + + #[test] + fn docx_table_limits_preserve_ordinary_padding() { + let expected = "| A | B | C |\n| --- | --- | --- |\n| D | | |"; + let limits = DocxLimits { + rows: 2, + columns: 3, + cells: 6, + output_bytes: expected.len(), + }; + assert_eq!( + docx_to_markdown_with_limits(small_table(), limits).unwrap(), + expected + ); + } + + #[test] + fn docx_table_bytes_include_escaping_and_document_blocks() { + let rows = vec![vec![" é|\\\r\nZ\nY\rQ ".into(), "".into()], vec!["".into()]]; + let expected = "intro\n\n| é\\|\\\\
Z
Y
Q |\n| --- |"; + let limits = DocxLimits { + output_bytes: expected.len(), + ..DOCX_LIMITS + }; + let mut output = "intro".to_string(); + append_table(&rows, &mut output, limits).unwrap(); + assert_eq!(output, expected); + let before = output.clone(); + assert!(append_table(&rows, &mut output, limits).is_err()); + assert_eq!(output, before); + let mut short = "intro".to_string(); + assert!(append_table( + &rows, + &mut short, + DocxLimits { + output_bytes: expected.len() - 1, + ..limits + } + ) + .is_err()); + assert_eq!(short, "intro"); + } + #[tokio::test] async fn docx_becomes_markdown_with_headings_lists_and_tables() { let body = zip_of(&[ diff --git a/crates/fetchkit/src/documents/tables.rs b/crates/fetchkit/src/documents/tables.rs new file mode 100644 index 0000000..93a6eb7 --- /dev/null +++ b/crates/fetchkit/src/documents/tables.rs @@ -0,0 +1,298 @@ +// Decisions: +// - THREAT[TM-DOS-008]: DOCX dimensions and padded cell work need admission +// before either local backend renders. markkit's sheet limits exclude DOCX. +// - Use the legacy sheet dimensions for DOCX. This preflight bounds markkit's +// table expansion; the legacy renderer additionally admits exact output bytes. +#[cfg(feature = "docs-markkit")] +use crate::content::ContentProcessorError; + +#[derive(Clone, Copy)] +pub(super) struct TableLimits { + pub rows: usize, + pub columns: usize, + pub cells: usize, + pub output_bytes: usize, +} + +pub(super) const TABLE_LIMITS: TableLimits = TableLimits { + rows: 1_000, + columns: 50, + cells: 50_000, + output_bytes: 10 * 1024 * 1024, +}; + +#[cfg(feature = "docs-markkit")] +const MAX_TABLE_DEPTH: usize = 50; + +#[cfg(feature = "docs-markkit")] +pub(super) fn check_xml(xml: &[u8], limits: TableLimits) -> Result<(), ContentProcessorError> { + use quick_xml::events::Event; + let xml = xml.strip_prefix(&[0xef, 0xbb, 0xbf]).unwrap_or(xml); + let fail = || ContentProcessorError("DOCX table exceeds the conversion limit".into()); + let mut reader = quick_xml::Reader::from_reader(xml); + let mut tables: Vec<(usize, usize, usize)> = Vec::new(); + let mut source_cells = 0usize; + let mut padded_cells = 0usize; + let mut padding_bytes = 0usize; + loop { + let event = reader.read_event().map_err(|_| fail())?; + let empty = matches!(&event, Event::Empty(_)); + match event { + Event::Start(start) | Event::Empty(start) => { + let name = start.local_name(); + if name.as_ref() == b"tbl" { + if tables.len() >= MAX_TABLE_DEPTH { + return Err(fail()); + } + tables.push((0, 0, 0)); + } + if let Some((rows, width, columns)) = tables.last_mut() { + match name.as_ref() { + b"tr" => { + *rows = rows.checked_add(1).ok_or_else(fail)?; + *columns = 0; + } + b"tc" => { + source_cells = source_cells + .checked_add(1) + .filter(|n| *n <= limits.cells) + .ok_or_else(fail)?; + *columns = columns.checked_add(1).ok_or_else(fail)?; + } + b"gridSpan" => { + let span = start + .attributes() + .flatten() + .find(|a| a.key.local_name().as_ref() == b"val") + .and_then(|a| { + a.normalized_value(quick_xml::XmlVersion::Implicit1_0) + .ok()? + .parse::() + .ok() + }) + .unwrap_or(1) + .min(64); + *columns = columns + .checked_add(span.saturating_sub(1)) + .ok_or_else(fail)?; + } + _ => {} + } + *width = (*width).max(*columns); + if *rows > limits.rows + || *width > limits.columns + || rows.checked_mul(*width).is_none_or(|n| n > limits.cells) + { + return Err(fail()); + } + } + if empty && name.as_ref() == b"tbl" { + tables.pop(); + } + } + Event::End(end) if end.local_name().as_ref() == b"tbl" => { + if let Some((rows, width, _)) = tables.pop() { + let cells = rows.checked_mul(width).ok_or_else(fail)?; + padded_cells = padded_cells + .checked_add(cells) + .filter(|n| *n <= limits.cells) + .ok_or_else(fail)?; + let bytes = cells + .checked_mul(3) + .and_then(|n| width.checked_mul(6)?.checked_add(n)) + .and_then(|n| rows.checked_mul(2)?.checked_add(n)) + .and_then(|n| n.checked_add(4)) + .ok_or_else(fail)?; + padding_bytes = padding_bytes + .checked_add(bytes) + .filter(|n| *n <= limits.output_bytes) + .ok_or_else(fail)?; + } + } + Event::Eof => break, + _ => {} + } + } + Ok(()) +} + +#[cfg(feature = "docs-markkit")] +pub(super) fn check_archive( + body: &[u8], + config: &markkit::MarkkitBuilder, +) -> Result<(), super::ConvertError> { + use std::io::{Cursor, Read}; + let fail = || { + super::ConvertError::Failed( + "DOCX table admission failed: archive or conversion limits".into(), + ) + }; + let mut archive = zip::ZipArchive::new(Cursor::new(body)).map_err(|_| fail())?; + if archive.len() > config.max_archive_entries.min(10_000) { + return Err(fail()); + } + let mut total = 0u64; + let part_limit = config.max_archive_entry_size.min(64 * 1024 * 1024); + let total_limit = config.max_archive_total_size.min(256 * 1024 * 1024); + for index in 0..archive.len() { + let entry = archive.by_index_raw(index).map_err(|_| fail())?; + total = total + .checked_add(entry.size()) + .filter(|n| *n <= total_limit) + .ok_or_else(fail)?; + if entry.size() > part_limit { + return Err(fail()); + } + } + let mut inflated = 0u64; + let mut read = |name: &str| -> Result, super::ConvertError> { + let entry = match archive.by_name(name) { + Ok(entry) => entry, + Err(zip::result::ZipError::FileNotFound) => return Ok(Vec::new()), + Err(_) => return Err(fail()), + }; + let mut bytes = Vec::new(); + entry + .take(part_limit + 1) + .read_to_end(&mut bytes) + .map_err(|_| fail())?; + if bytes.len() as u64 > part_limit { + return Err(fail()); + } + inflated = inflated + .checked_add(bytes.len() as u64) + .filter(|n| *n <= total_limit) + .ok_or_else(fail)?; + Ok(bytes) + }; + // markkit may choose any internal officeDocument relationship, not just the + // conventional filename. Admit every candidate before it makes that choice. + let rels = read("_rels/.rels")?; + let mut reader = quick_xml::Reader::from_reader(rels.as_slice()); + let mut paths = std::collections::BTreeSet::new(); + loop { + use quick_xml::events::Event; + match reader.read_event().map_err(|_| fail())? { + Event::Start(start) | Event::Empty(start) + if start.local_name().as_ref() == b"Relationship" => + { + let attr = |name: &[u8]| { + start + .attributes() + .flatten() + .find(|a| a.key.local_name().as_ref() == name) + .and_then(|a| { + a.normalized_value(quick_xml::XmlVersion::Implicit1_0) + .ok() + .map(|v| v.into_owned()) + }) + }; + if attr(b"Type").is_some_and(|t| t.ends_with("/officeDocument")) + && !attr(b"TargetMode").is_some_and(|m| m.eq_ignore_ascii_case("External")) + { + if let Some(target) = attr(b"Target") { + let path = if let Some(path) = target.strip_prefix('/') { + path.to_string() + } else { + let mut parts = Vec::new(); + for part in target.split('/') { + match part { + "" | "." => {} + ".." => { + parts.pop(); + } + part => parts.push(part), + } + } + parts.join("/") + }; + paths.insert(path); + if paths.len() > config.max_archive_entries.min(10_000) { + return Err(fail()); + } + } + } + } + Event::Eof => break, + _ => {} + } + } + paths.insert("word/document.xml".to_string()); + for path in paths { + check_xml(&read(&path)?, TABLE_LIMITS).map_err(|e| super::ConvertError::Failed(e.0))?; + } + Ok(()) +} + +#[cfg(all(test, feature = "docs-markkit"))] +mod tests { + use super::*; + + const TABLE: &[u8] = b""; + + #[test] + fn docx_preflight_admits_small_table_dimensions() { + for limits in [ + TableLimits { + rows: 1, + ..TABLE_LIMITS + }, + TableLimits { + columns: 1, + ..TABLE_LIMITS + }, + TableLimits { + cells: 3, + ..TABLE_LIMITS + }, + TableLimits { + output_bytes: 8, + ..TABLE_LIMITS + }, + ] { + assert!(check_xml(TABLE, limits).is_err()); + } + assert!(check_xml(TABLE, TABLE_LIMITS).is_ok()); + assert!(check_xml( + b"prefix", + TableLimits { + rows: 1, + ..TABLE_LIMITS + } + ) + .is_err()); + } + + #[test] + fn docx_preflight_counts_spans_nested_and_multiple_tables() { + let limits = TableLimits { + rows: 2, + columns: 3, + cells: 4, + ..TABLE_LIMITS + }; + assert!(check_xml( + b"", + limits + ) + .is_err()); + assert!(check_xml( + b"", + limits + ) + .is_ok()); + assert!(check_xml( + b"", + TableLimits { + columns: 1, + ..limits + } + ) + .is_ok()); + assert!(check_xml( + b"", + limits + ) + .is_err()); + } +} diff --git a/docs/hardening.md b/docs/hardening.md index ba346db..5475bbf 100644 --- a/docs/hardening.md +++ b/docs/hardening.md @@ -98,6 +98,20 @@ If you must fetch nonstandard ports or internal-looking public domains, start fr ## Document conversion +Both built-in DOCX backends reject tables exceeding 1,000 rows, 50 columns or +50,000 padded cells. The default markkit backend preflights archive parts before +conversion, including merged-cell spans and nested tables; its cell-work budget +is cumulative within each XML part. These DOCX limits cannot be raised through +markkit's spreadsheet settings. Archive preflight honors tighter markkit archive +settings and caps entry count and inflated sizes at the defaults. + +The deprecated `office` backend also rejects DOCX output over 10 MiB before +allocating escaped or padded Markdown. Smaller `max_body_size` settings still +truncate the returned output. Limits produce a converter failure; another +configured backend may handle the document. Normal table padding and escaping +are preserved. These bounds address table expansion; they are not a process +memory limit for all backend parsing or text rendering. + PDF and Office documents are converted by a chain of document converters. The default converters (`pdf-inspector`, `markkit`) run in-process and make no network requests; markkit is built without its HTTP client. Remote converters diff --git a/knowledge/foundations/fetchers.md b/knowledge/foundations/fetchers.md index 286c94e..6a758d9 100644 --- a/knowledge/foundations/fetchers.md +++ b/knowledge/foundations/fetchers.md @@ -141,10 +141,22 @@ Built-in converters (Cargo features): sheet, cell grid, hidden sheets) are markkit's defaults, tunable with `MarkkitConverter::with_markkit` (`fetchkit::documents::{Markkit, MarkkitBuilder}`). Spreadsheet titles are not reported (markkit returns none). + DOCX gets a Fetchkit archive/XML preflight because markkit's sheet limits do + not constrain Word tables: 1,000 rows, 50 columns and a cumulative 50,000 + source/padded cells per XML part, including nested tables and merged-cell spans. + The standard main part and all internal `officeDocument` relationship targets + are checked, including nonstandard paths. The preflight honors tighter configured archive budgets and + caps them at markkit's defaults. It admits padding bytes before backend rendering; + other markkit output remains bounded by its input/archive controls and the final + response cap, rather than an incremental output writer. - `office` (deprecated, "not necessarily correct"; kept one release): DOCX and PPTX XML parts via `quick-xml`, spreadsheets via `calamine`. Zip containers are checked against an entry-count and total uncompressed-size budget before any part is inflated, and each part is read through a size cap (TM-DOS-008). - Output caps: 1,000 slides, 1,000 rows and 50 columns per sheet. + Output caps: 1,000 slides, 1,000 rows and 50 columns per sheet. DOCX uses the same + table dimensions plus 50,000 source/padded cells per table. Paragraph/cell text + and cumulative Markdown are capped at 10 MiB. A single output buffer admits + exact UTF-8/escaped bytes, padding and block separators before reservation; + limit failures are converter errors, not silent table truncation. - `cloudflare`: Workers AI `toMarkdown` REST endpoint, one multipart file per request, bearer token held in `SecretString`. Images are not routed (billed by Cloudflare). - `HttpConverter`: multipart upload to an operator-configured URL; Markdown from the raw diff --git a/knowledge/foundations/tool-contract.md b/knowledge/foundations/tool-contract.md index fa141bc..ad9ab3a 100644 --- a/knowledge/foundations/tool-contract.md +++ b/knowledge/foundations/tool-contract.md @@ -344,6 +344,12 @@ By default, Fetchkit blocks connections to private/reserved IP ranges: `markkit` when enabled): DOCX/PPTX/XLSX/XLSM/XLSB/XLS/ODS (`office_docx`, `office_pptx`, `office_spreadsheet`; one `## Slide N: title` per slide; sheets capped at 1,000 rows and 50 columns with `sheet_truncated`). + - Both local DOCX backends reject tables above 1,000 rows, 50 columns or + 50,000 padded cells before rendering. markkit's admission also counts source + cells, merged-cell spans and nested tables across an XML part. The deprecated + Office backend additionally admits all DOCX Markdown into a 10 MiB buffer + before allocation; a smaller response cap is still applied afterwards. + Exceeding these limits yields a converter failure and normal chain fallthrough. - Remote converters (`cloudflare` -> `cloudflare_tomarkdown`, `HttpConverter`) run only when remote conversion is allowed and never under `hardened()`. - Converter fallthrough adds `fallback_converter`; quality score is 1.0 clean, diff --git a/knowledge/log.md b/knowledge/log.md index 5c5cb57..3245335 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -2,6 +2,8 @@ ## 2026-10-12 +* **DOCX table budgets**: Both local backends now admit table dimensions and padded cell work before conversion; the deprecated renderer additionally admits exact Markdown bytes before allocation. Tiny offline fixtures cover limits, escaping and normal output under TM-DOS-008. + * **Office backend**: `markkit` (`MarkkitConverter`, `docs-markkit`, default) is now the Office converter for DOCX/XLSX/XLS/ODS; the in-crate `office` converter is deprecated, off by default, excluded from `docs-all`, and remains the only PPTX/XLSB backend when enabled. TM-DOS-008 now describes markkit's limits. * **Document conversion**: Replaced the PDF and Office processors with one `DocumentProcessor` running a configurable, per-format `DocumentConverter` chain; added feature-gated converters (`docs-pdf`, `docs-office`, `docs-cloudflare`, `docs-http`), a separate 25 MB document cap, remote-converter opt-in, and threats TM-LEAK-006/007. * **Office documents**: Added the local Office processor (DOCX, PPTX, XLSX, XLS, ODS) to the fetchers and tool contract, its zip-bomb threat TM-DOS-008, and the Markdown-first `Accept` header. diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 78c77e6..71146d0 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -298,7 +298,7 @@ to protect against changes between validation and write. | TM-DOS-005 | DNS resolution delay | Low | DNS resolution uses system resolver; no explicit timeout on DNS lookup | **ACCEPTED** | | TM-DOS-006 | Memory exhaustion from large HTML conversion | Medium | Conversion input bounded by `max_body_size` (10 MB default) | MITIGATED | | TM-DOS-007 | CPU/memory exhaustion from adversarial PDF parsing | Medium | Bounded input/output, blocking-task isolation, and a cancellation-safe two-document concurrency cap; parser-internal expansion remains possible | **ACCEPTED** | -| TM-DOS-008 | Zip bomb or adversarial Office document | Medium | markkit archive entry/size budgets and cell-grid pre-scan before inflating (deprecated `office`: own budget and capped part reads), per-sheet output caps, shared two-document blocking cap | MITIGATED | +| TM-DOS-008 | Zip bomb or adversarial Office document | Medium | Archive/part budgets, DOCX dimension and padded-cell admission before rendering, legacy DOCX incremental byte admission, per-sheet output caps, shared two-document blocking cap | MITIGATED | ### Mitigation Details @@ -343,6 +343,14 @@ final Markdown is capped at `max_body_size`. A markkit parser panic is contained blocking task (`JoinError` becomes a converter failure) unless the embedding binary uses `panic = "abort"`. +markkit's sheet caps do not constrain DOCX. Fetchkit therefore preflights all +internal main-part candidates from DOCX relationships before calling markkit, with checked row/column +counts (1,000/50), merged-cell spans, nesting, and cumulative source/padded cell +work (50,000 per XML part). Padding bytes are admitted before backend rendering. +The preflight honors tighter markkit archive settings and uses its default +archive ceilings. This bounds table padding amplification; it does not claim an +incremental byte limit for markkit's other text/markup allocations. + The deprecated `office` converter (`docs-office`) rejects archives with more than 10,000 entries or more than 200 MB of declared uncompressed content, and every XML part it reads itself is read through a 64 MB cap, since declared sizes can lie. Spreadsheet output is capped at 1,000 rows and 50 columns @@ -350,6 +358,14 @@ per sheet before the `max_body_size` output cap. Parsing shares the local conver blocking path and two-document concurrency cap. `calamine` inflates sheet parts itself; the declared-size budget bounds those as well. +Legacy DOCX tables reuse the sheet dimensions and reject more than 50,000 +source/padded cells per table before collecting rows/cells. Retained table text, +paragraphs and cumulative Markdown have a 10 MiB conversion ceiling. Rendering +admits exact escaped UTF-8 bytes, separators and padding before bounded buffer +reservation; it no longer retains all rendered rows and joins a second copy. +Tiny ordinary fixtures exercise lower injected limits, exact byte boundaries, +escaping and unchanged normal tables; no resource-exhaustion payload is needed. + ## 5. Information Leakage (TM-LEAK) | ID | Threat | Severity | Mitigation | Status | From 862c35ab9bda2ebeb535d95c774fafc3bad2b27f Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Mon, 12 Oct 2026 20:30:46 -0500 Subject: [PATCH 2/2] docs(documents): clarify table conversion ceilings --- docs/hardening.md | 7 ++++--- knowledge/foundations/fetchers.md | 6 ++++-- knowledge/foundations/tool-contract.md | 3 ++- knowledge/security/threat-model.md | 5 +++-- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/hardening.md b/docs/hardening.md index 5475bbf..eeea905 100644 --- a/docs/hardening.md +++ b/docs/hardening.md @@ -101,12 +101,13 @@ If you must fetch nonstandard ports or internal-looking public domains, start fr Both built-in DOCX backends reject tables exceeding 1,000 rows, 50 columns or 50,000 padded cells. The default markkit backend preflights archive parts before conversion, including merged-cell spans and nested tables; its cell-work budget -is cumulative within each XML part. These DOCX limits cannot be raised through +is cumulative within each XML part; table nesting is capped at 50 levels. These DOCX limits cannot be raised through markkit's spreadsheet settings. Archive preflight honors tighter markkit archive settings and caps entry count and inflated sizes at the defaults. -The deprecated `office` backend also rejects DOCX output over 10 MiB before -allocating escaped or padded Markdown. Smaller `max_body_size` settings still +The deprecated `office` backend caps each rendered table at 10 MiB and also +rejects cumulative DOCX output over 10 MiB before allocating escaped or padded +Markdown. Smaller `max_body_size` settings still truncate the returned output. Limits produce a converter failure; another configured backend may handle the document. Normal table padding and escaping are preserved. These bounds address table expansion; they are not a process diff --git a/knowledge/foundations/fetchers.md b/knowledge/foundations/fetchers.md index 6a758d9..ec96b20 100644 --- a/knowledge/foundations/fetchers.md +++ b/knowledge/foundations/fetchers.md @@ -143,7 +143,8 @@ Built-in converters (Cargo features): Spreadsheet titles are not reported (markkit returns none). DOCX gets a Fetchkit archive/XML preflight because markkit's sheet limits do not constrain Word tables: 1,000 rows, 50 columns and a cumulative 50,000 - source/padded cells per XML part, including nested tables and merged-cell spans. + source/padded cells per XML part, including nested tables (at most 50 levels) + and merged-cell spans. The standard main part and all internal `officeDocument` relationship targets are checked, including nonstandard paths. The preflight honors tighter configured archive budgets and caps them at markkit's defaults. It admits padding bytes before backend rendering; @@ -153,7 +154,8 @@ Built-in converters (Cargo features): containers are checked against an entry-count and total uncompressed-size budget before any part is inflated, and each part is read through a size cap (TM-DOS-008). Output caps: 1,000 slides, 1,000 rows and 50 columns per sheet. DOCX uses the same - table dimensions plus 50,000 source/padded cells per table. Paragraph/cell text + table dimensions plus 50,000 collected/padded cells per table. Every rendered + table (including sheets) has a 10 MiB byte ceiling. Paragraph/cell text and cumulative Markdown are capped at 10 MiB. A single output buffer admits exact UTF-8/escaped bytes, padding and block separators before reservation; limit failures are converter errors, not silent table truncation. diff --git a/knowledge/foundations/tool-contract.md b/knowledge/foundations/tool-contract.md index ad9ab3a..f2377cb 100644 --- a/knowledge/foundations/tool-contract.md +++ b/knowledge/foundations/tool-contract.md @@ -343,7 +343,8 @@ By default, Fetchkit blocks connections to private/reserved IP ranges: - Deprecated `office` converter (`docs-office`, off by default; fallback after `markkit` when enabled): DOCX/PPTX/XLSX/XLSM/XLSB/XLS/ODS (`office_docx`, `office_pptx`, `office_spreadsheet`; one `## Slide N: title` per slide; sheets - capped at 1,000 rows and 50 columns with `sheet_truncated`). + capped at 1,000 rows and 50 columns with `sheet_truncated`; each rendered + table has a 10 MiB conversion ceiling). - Both local DOCX backends reject tables above 1,000 rows, 50 columns or 50,000 padded cells before rendering. markkit's admission also counts source cells, merged-cell spans and nested tables across an XML part. The deprecated diff --git a/knowledge/security/threat-model.md b/knowledge/security/threat-model.md index 71146d0..b9a7443 100644 --- a/knowledge/security/threat-model.md +++ b/knowledge/security/threat-model.md @@ -345,7 +345,7 @@ blocking task (`JoinError` becomes a converter failure) unless the embedding bin markkit's sheet caps do not constrain DOCX. Fetchkit therefore preflights all internal main-part candidates from DOCX relationships before calling markkit, with checked row/column -counts (1,000/50), merged-cell spans, nesting, and cumulative source/padded cell +counts (1,000/50), merged-cell spans, nesting (at most 50 levels), and cumulative source/padded cell work (50,000 per XML part). Padding bytes are admitted before backend rendering. The preflight honors tighter markkit archive settings and uses its default archive ceilings. This bounds table padding amplification; it does not claim an @@ -359,7 +359,8 @@ blocking path and two-document concurrency cap. `calamine` inflates sheet parts the declared-size budget bounds those as well. Legacy DOCX tables reuse the sheet dimensions and reject more than 50,000 -source/padded cells per table before collecting rows/cells. Retained table text, +collected/padded cells per table before collecting rows/cells. Each rendered +table, including spreadsheet tables, has a 10 MiB conversion ceiling. Retained table text, paragraphs and cumulative Markdown have a 10 MiB conversion ceiling. Rendering admits exact escaped UTF-8 bytes, separators and padding before bounded buffer reservation; it no longer retains all rendered rows and joins a second copy.