Skip to main content

max / makenotwork

docengine: detect slug collisions, document the sanitize ordering, drop regex Four doc_loader items that all land in one file. Slug collisions: slugs are file_stem() keyed into one flat map with no regard for section, so guide/faq.md and support/faq.md both want /docs/faq and one silently became unreachable. DocLoader::collisions() now reports them and each is logged at warn. Last writer still wins: surfacing the clash is the fix, and choosing a different winner is a routing decision, not a bug fix. Sanitization ordering: post_process_directives and resolve_ui_examples run after the only ammonia pass and are never re-cleaned, and resolve_ui_examples inlines files from examples_path verbatim. The ordering is deliberate, since both exist to emit markup the default policy would strip, so the safety rests entirely on those directories being operator-controlled. Written down in the module docs, because a writable examples_path is a stored-XSS primitive. regex -> regex-lite: doc_loader held the last heavy-regex use, and both its patterns are well within regex-lite, which five other features already depend on. Drops regex, regex-automata, regex-syntax, and aho-corasick from the tree. Heading ids on for docs pages, now that the mechanism exists. Docs are operator-authored, and without it extract_toc's anchors point at nothing and no one can deep-link a section. Also clears the two pre-existing clippy -D warnings failures in this file (type_complexity on pre_process, a collapsible if in rewrite_links). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-20 18:43 UTC
Commit: d5629ddd662cfb7f30fed5f80edebd749a314d5e
Parent: a9d6d18
4 files changed, +143 insertions, -53 deletions
@@ -3,15 +3,6 @@
3 3 version = 4
4 4
5 5 [[package]]
6 - name = "aho-corasick"
7 - version = "1.1.4"
8 - source = "registry+https://github.com/rust-lang/crates.io-index"
9 - checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301"
10 - dependencies = [
11 - "memchr",
12 - ]
13 -
14 - [[package]]
15 6 name = "ammonia"
16 7 version = "4.1.2"
17 8 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -88,7 +79,6 @@ version = "0.3.5"
88 79 dependencies = [
89 80 "ammonia",
90 81 "pulldown-cmark",
91 - "regex",
92 82 "regex-lite",
93 83 "serde",
94 84 "tempfile",
@@ -620,41 +610,12 @@ dependencies = [
620 610 ]
621 611
622 612 [[package]]
623 - name = "regex"
624 - version = "1.12.3"
625 - source = "registry+https://github.com/rust-lang/crates.io-index"
626 - checksum = "e10754a14b9137dd7b1e3e5b0493cc9171fdd105e0ab477f51b72e7f3ac0e276"
627 - dependencies = [
628 - "aho-corasick",
629 - "memchr",
630 - "regex-automata",
631 - "regex-syntax",
632 - ]
633 -
634 - [[package]]
635 - name = "regex-automata"
636 - version = "0.4.14"
637 - source = "registry+https://github.com/rust-lang/crates.io-index"
638 - checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f"
639 - dependencies = [
640 - "aho-corasick",
641 - "memchr",
642 - "regex-syntax",
643 - ]
644 -
645 - [[package]]
646 613 name = "regex-lite"
647 614 version = "0.1.9"
648 615 source = "registry+https://github.com/rust-lang/crates.io-index"
649 616 checksum = "cab834c73d247e67f4fae452806d17d3c7501756d98c8808d7c9c7aa7d18f973"
650 617
651 618 [[package]]
652 - name = "regex-syntax"
653 - version = "0.8.10"
654 - source = "registry+https://github.com/rust-lang/crates.io-index"
655 - checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a"
656 -
657 - [[package]]
658 619 name = "rustix"
659 620 version = "1.1.4"
660 621 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -5,7 +5,7 @@ edition = "2024"
5 5
6 6 [features]
7 7 default = []
8 - doc-loader = ["dep:regex", "dep:tracing"]
8 + doc-loader = ["dep:regex-lite", "dep:tracing"]
9 9 directives = ["dep:regex-lite"]
10 10 mentions = ["dep:regex-lite"]
11 11 quotes = ["dep:regex-lite", "dep:uuid"]
@@ -19,7 +19,6 @@ pulldown-cmark = "0.12"
19 19 ammonia = "4"
20 20 serde = { version = "1", features = ["derive"] }
21 21
22 - regex = { version = "1", optional = true }
23 22 regex-lite = { version = "0.1", optional = true }
24 23 uuid = { version = "1", features = ["serde", "v4"], optional = true }
25 24 toml = { version = "0.8", optional = true }
@@ -1,13 +1,49 @@
1 + //! Loading a directory of markdown into rendered, in-memory documentation
2 + //! pages.
3 + //!
4 + //! # Sanitization ordering contract
5 + //!
6 + //! The page pipeline is:
7 + //!
8 + //! 1. `pre_process` (caller-supplied, e.g. assumption substitution)
9 + //! 2. `rewrite_links`
10 + //! 3. **`render_permissive` — the only ammonia pass**
11 + //! 4. `post_process_directives` (feature `directives`)
12 + //! 5. `resolve_ui_examples`
13 + //!
14 + //! Steps 4 and 5 run *after* sanitization and their output is never re-cleaned.
15 + //! Both therefore inject trusted-by-construction HTML into already-sanitized
16 + //! markup, and `resolve_ui_examples` in particular reads files from
17 + //! `examples_path` and inlines their contents **verbatim**.
18 + //!
19 + //! What that requires of callers:
20 + //!
21 + //! - `base_path` and `examples_path` must be operator-controlled directories
22 + //! shipped with the deployment. They are not a place to put user uploads; a
23 + //! writable `examples_path` is a stored-XSS primitive, since anything in an
24 + //! example file lands in the page unfiltered.
25 + //! - Anything added to steps 4-5 must emit only HTML it constructs itself, or
26 + //! sanitize its own input. Do not widen them to interpolate page content.
27 + //!
28 + //! The ordering is deliberate, not incidental: directives and UI examples exist
29 + //! precisely to emit markup ammonia's default policy would strip, so moving
30 + //! them before step 3 would defeat them. The safety comes from the inputs being
31 + //! trusted, which is why that constraint is written down here.
32 +
1 33 use std::collections::HashMap;
2 34 use std::path::Path;
3 35 use std::sync::LazyLock;
4 36
5 - use regex::Regex;
37 + use regex_lite::Regex;
6 38
7 39 static LINK_RE: LazyLock<Regex> = LazyLock::new(|| {
8 40 Regex::new(r"\[([^\]]+)\]\(([^)]+)\)").expect("valid regex")
9 41 });
10 42
43 + /// Transform applied to raw markdown before link rewriting. `Err` skips the
44 + /// page with a warning.
45 + pub type PreProcessor = Box<dyn Fn(&str) -> Result<String, String> + Send + Sync>;
46 +
11 47 /// Configuration for the doc loader.
12 48 pub struct DocLoaderConfig {
13 49 /// Sections as `(directory_name, display_name)` pairs in display order.
@@ -22,7 +58,7 @@ pub struct DocLoaderConfig {
22 58 /// Optional pre-processor applied to raw markdown before link rewriting.
23 59 /// On `Err`, the page is skipped with a warning. Use to wire
24 60 /// [`crate::Assumptions::substitute`] or a similar transform.
25 - pub pre_process: Option<Box<dyn Fn(&str) -> Result<String, String> + Send + Sync>>,
61 + pub pre_process: Option<PreProcessor>,
26 62 }
27 63
28 64 /// A rendered documentation page.
@@ -51,11 +87,27 @@ pub struct DocSearchEntry {
51 87 pub body_text: String,
52 88 }
53 89
90 + /// Two documentation files that slugify to the same URL.
91 + ///
92 + /// Slugs are `file_stem()` keyed into one flat map with no regard for section,
93 + /// so `guide/faq.md` and `reference/faq.md` both want `/docs/faq`. One of them
94 + /// wins and the other is unreachable.
95 + #[derive(Clone, Debug, PartialEq, Eq)]
96 + pub struct SlugCollision {
97 + /// The contested slug.
98 + pub slug: String,
99 + /// Section display name of the page that was displaced.
100 + pub displaced_section: String,
101 + /// Section display name of the page now serving this slug.
102 + pub winning_section: String,
103 + }
104 +
54 105 /// In-memory store of rendered documentation pages, built once at startup.
55 106 #[derive(Clone, Debug)]
56 107 pub struct DocLoader {
57 108 pages: HashMap<String, DocPage>,
58 109 index: Vec<DocIndexEntry>,
110 + collisions: Vec<SlugCollision>,
59 111 }
60 112
61 113 impl DocLoader {
@@ -65,6 +117,7 @@ impl DocLoader {
65 117 pub fn load(base_path: &Path, config: &DocLoaderConfig) -> Self {
66 118 let mut pages = HashMap::new();
67 119 let mut index = Vec::new();
120 + let mut collisions = Vec::new();
68 121
69 122 for (dir_name, section_display) in &config.sections {
70 123 let section_path = base_path.join(dir_name);
@@ -128,7 +181,12 @@ impl DocLoader {
128 181 config.unpublished_pattern.as_deref(),
129 182 );
130 183 let md_without_title = crate::text::strip_first_heading(&rewritten_md);
131 - let html_content = crate::render_permissive(&md_without_title);
184 + // Heading ids on: docs are operator-authored (trusted), and
185 + // without them `extract_toc`'s anchors point at nothing and no
186 + // one can deep-link a section.
187 + let html_content = crate::Renderer::permissive()
188 + .with_heading_ids(true)
189 + .render(&md_without_title);
132 190 #[cfg(feature = "directives")]
133 191 let html_content = crate::directives::post_process_directives(&html_content);
134 192 let html_content = resolve_ui_examples(&html_content, config.examples_path.as_deref());
@@ -147,11 +205,29 @@ impl DocLoader {
147 205 });
148 206
149 207 let slug_key = page.slug.clone();
150 - pages.insert(slug_key, page);
208 + if let Some(displaced) = pages.insert(slug_key, page) {
209 + // Last writer wins, as it always has. Surfacing the clash
210 + // is the fix; picking a different winner would be a routing
211 + // decision, and silently serving one of two pages under a
212 + // URL is what made this invisible in the first place.
213 + let winning_section = pages[&displaced.slug].section.clone();
214 + tracing::warn!(
215 + slug = %displaced.slug,
216 + displaced_section = %displaced.section,
217 + winning_section = %winning_section,
218 + "docs slug collision: two pages resolve to the same URL, \
219 + only the last is reachable"
220 + );
221 + collisions.push(SlugCollision {
222 + slug: displaced.slug.clone(),
223 + displaced_section: displaced.section.clone(),
224 + winning_section,
225 + });
226 + }
151 227 }
152 228 }
153 229
154 - DocLoader { pages, index }
230 + DocLoader { pages, index, collisions }
155 231 }
156 232
157 233 /// Look up a rendered page by slug.
@@ -164,6 +240,15 @@ impl DocLoader {
164 240 &self.index
165 241 }
166 242
243 + /// Slug collisions found during load, in the order they were hit.
244 + ///
245 + /// Empty for a healthy corpus. A non-empty result means some page is
246 + /// indexed but unreachable, so a caller that can fail loudly (a build step,
247 + /// a startup check) should. Each collision is also logged at `warn`.
248 + pub fn collisions(&self) -> &[SlugCollision] {
249 + &self.collisions
250 + }
251 +
167 252 /// Build a search index with HTML stripped to plain text.
168 253 pub fn search_index(&self) -> Vec<DocSearchEntry> {
169 254 self.index
@@ -186,6 +271,14 @@ impl DocLoader {
186 271 ///
187 272 /// If no examples path is configured or a file is missing, the placeholder is
188 273 /// replaced with a fallback message.
274 + ///
275 + /// # Trust
276 + ///
277 + /// File contents are inlined **verbatim** into already-sanitized HTML and are
278 + /// never cleaned — see the sanitization ordering contract in the module docs.
279 + /// `examples_path` must be an operator-controlled directory. The `data-ui`
280 + /// capture is restricted to `[a-z0-9_-]+` by the placeholder pattern, so a
281 + /// name cannot traverse out of that directory.
189 282 fn resolve_ui_examples(html: &str, examples_path: Option<&Path>) -> String {
190 283 static UI_PLACEHOLDER: LazyLock<Regex> = LazyLock::new(|| {
191 284 Regex::new(r#"<div class="doc-ui-frame" data-ui="([a-z0-9_-]+)"></div>"#)
@@ -196,7 +289,7 @@ fn resolve_ui_examples(html: &str, examples_path: Option<&Path>) -> String {
196 289 return html.to_string();
197 290 }
198 291
199 - UI_PLACEHOLDER.replace_all(html, |caps: &regex::Captures| {
292 + UI_PLACEHOLDER.replace_all(html, |caps: &regex_lite::Captures| {
200 293 let name = &caps[1];
201 294 match examples_path {
202 295 Some(dir) => {
@@ -254,7 +347,7 @@ fn strip_html_tags(html: &str) -> String {
254 347 /// Rewrite relative `.md` links to the configured prefix.
255 348 fn rewrite_links(markdown: &str, link_prefix: &str, unpublished_pattern: Option<&str>) -> String {
256 349 LINK_RE
257 - .replace_all(markdown, |caps: &regex::Captures| {
350 + .replace_all(markdown, |caps: &regex_lite::Captures| {
258 351 let text = &caps[1];
259 352 let url = &caps[2];
260 353
@@ -268,10 +361,10 @@ fn rewrite_links(markdown: &str, link_prefix: &str, unpublished_pattern: Option<
268 361 }
269 362
270 363 // Unpublished docs: strip link, keep text.
271 - if let Some(pattern) = unpublished_pattern {
272 - if url.contains(pattern) {
273 - return text.to_string();
274 - }
364 + if let Some(pattern) = unpublished_pattern
365 + && url.contains(pattern)
366 + {
367 + return text.to_string();
275 368 }
276 369
277 370 // Only rewrite links containing .md
@@ -515,6 +608,41 @@ mod tests {
515 608 }
516 609
517 610 #[test]
611 + fn same_stem_in_two_sections_is_reported_as_a_collision() {
612 + let tmp = tempfile::tempdir().unwrap();
613 + let base = tmp.path();
614 + write(base, "guide/faq.md", "# Guide FAQ\n\nguide body");
615 + write(base, "support/faq.md", "# Support FAQ\n\nsupport body");
616 +
617 + let loader = DocLoader::load(base, &config_for(base));
618 +
619 + assert_eq!(
620 + loader.collisions(),
621 + [SlugCollision {
622 + slug: "faq".to_string(),
623 + displaced_section: "Guide".to_string(),
624 + winning_section: "Support".to_string(),
625 + }]
626 + );
627 +
628 + // Both are still indexed, but only one is reachable — which is exactly
629 + // the damage the warning exists to surface.
630 + assert_eq!(loader.index().len(), 2);
631 + assert_eq!(loader.get("faq").unwrap().section, "Support");
632 + }
633 +
634 + #[test]
635 + fn distinct_slugs_produce_no_collisions() {
636 + let tmp = tempfile::tempdir().unwrap();
637 + let base = tmp.path();
638 + write(base, "guide/setup.md", "# Setup");
639 + write(base, "support/faq.md", "# FAQ");
640 +
641 + let loader = DocLoader::load(base, &config_for(base));
642 + assert!(loader.collisions().is_empty());
643 + }
644 +
645 + #[test]
518 646 fn load_indexes_pages_across_sections_in_display_order() {
519 647 let tmp = tempfile::tempdir().unwrap();
520 648 let base = tmp.path();
@@ -51,7 +51,9 @@ pub use filters::{Filter, FilterArg, FilterError};
51 51 #[cfg(feature = "directives")]
52 52 pub use directives::post_process_directives;
53 53 #[cfg(feature = "doc-loader")]
54 - pub use doc_loader::{DocIndexEntry, DocLoader, DocLoaderConfig, DocPage, DocSearchEntry};
54 + pub use doc_loader::{
55 + DocIndexEntry, DocLoader, DocLoaderConfig, DocPage, DocSearchEntry, SlugCollision,
56 + };
55 57 #[cfg(feature = "frontmatter")]
56 58 pub use frontmatter::{Frontmatter, parse_frontmatter};
57 59 #[cfg(feature = "mentions")]