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..eeea905 100644 --- a/docs/hardening.md +++ b/docs/hardening.md @@ -98,6 +98,21 @@ 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; 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 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 +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..ec96b20 100644 --- a/knowledge/foundations/fetchers.md +++ b/knowledge/foundations/fetchers.md @@ -141,10 +141,24 @@ 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 (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; + 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 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. - `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..f2377cb 100644 --- a/knowledge/foundations/tool-contract.md +++ b/knowledge/foundations/tool-contract.md @@ -343,7 +343,14 @@ 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 + 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..b9a7443 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 (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 +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,15 @@ 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 +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. +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 |