michal/tit

Browse tree · Show commit · Download archive

Diff

15c6c1fb75f8 → 35eaf31a6a0d

@@ -46,6 +46,18 @@
 checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923"
 
 [[package]]
+name = "ammonia"
+version = "4.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dc6d763210e2eb7670d1a5183a08bebefa3f97db2a738a684f2ce00bd49f681d"
+dependencies = [
+ "cssparser",
+ "html5ever",
+ "maplit",
+ "url",
+]
+
+[[package]]
 name = "android_system_properties"
 version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -521,6 +533,17 @@
 ]
 
 [[package]]
+name = "cssparser"
+version = "0.37.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8c9cdaae01d5ed7882b04d795e7f752f46ff52d2fa3b50a20d28c464510bba98"
+dependencies = [
+ "dtoa-short",
+ "itoa",
+ "smallvec",
+]
+
+[[package]]
 name = "ctr"
 version = "0.10.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -680,6 +703,21 @@
  "proc-macro2",
  "quote",
  "syn 2.0.119",
+]
+
+[[package]]
+name = "dtoa"
+version = "1.0.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "4c3cf4824e2d5f025c7b531afcb2325364084a16806f6d47fbc1f5fbd9960590"
+
+[[package]]
+name = "dtoa-short"
+version = "0.3.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cd1511a7b6a56299bd043a9c167a6d2bfb37bf84a6dfceaba651168adfb43c87"
+dependencies = [
+ "dtoa",
 ]
 
 [[package]]
@@ -1853,6 +1891,16 @@
 ]
 
 [[package]]
+name = "html5ever"
+version = "0.39.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "46a1761807faccc9a19e86944bbf40610014066306f96edcdedc2fb714bcb7b8"
+dependencies = [
+ "log",
+ "markup5ever",
+]
+
+[[package]]
 name = "http"
 version = "1.4.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2247,6 +2295,23 @@
 checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad"
 
 [[package]]
+name = "maplit"
+version = "1.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3e2e65a1a2e43cfcb47a895c4c8b10d1f4a61097f9f254f183aee60cad9c651d"
+
+[[package]]
+name = "markup5ever"
+version = "0.39.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7122d987ec5f704ee56f6e5b41a7d93722e9aae27ae07cafa4036c4d3f9757de"
+dependencies = [
+ "log",
+ "tendril",
+ "web_atoms",
+]
+
+[[package]]
 name = "matchit"
 version = "0.8.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2325,6 +2390,12 @@
  "hybrid-array",
  "num-traits",
 ]
+
+[[package]]
+name = "new_debug_unreachable"
+version = "1.0.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "650eef8c711430f1a879fdd01d4745a7deea475becfb90269c06775983bbf086"
 
 [[package]]
 name = "nix"
@@ -2507,6 +2578,45 @@
 ]
 
 [[package]]
+name = "phf"
+version = "0.13.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "c1562dc717473dbaa4c1f85a36410e03c047b2e7df7f45ee938fbef64ae7fadf"
+dependencies = [
+ "phf_shared",
+ "serde",
+]
+
+[[package]]
+name = "phf_codegen"
+version = "0.13.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "49aa7f9d80421bca176ca8dbfebe668cc7a2684708594ec9f3c0db0805d5d6e1"
+dependencies = [
+ "phf_generator",
+ "phf_shared",
+]
+
+[[package]]
+name = "phf_generator"
+version = "0.13.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "135ace3a761e564ec88c03a77317a7c6b80bb7f7135ef2544dbe054243b89737"
+dependencies = [
+ "fastrand",
+ "phf_shared",
+]
+
+[[package]]
+name = "phf_shared"
+version = "0.13.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e57fef6bc5981e38c2ce2d63bfa546861309f875b8a75f092d1d54ae2d64f266"
+dependencies = [
+ "siphasher",
+]
+
+[[package]]
 name = "pin-project-lite"
 version = "0.2.17"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2594,6 +2704,12 @@
 ]
 
 [[package]]
+name = "precomputed-hash"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c"
+
+[[package]]
 name = "primefield"
 version = "0.14.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -2637,6 +2753,24 @@
 dependencies = [
  "parking_lot",
 ]
+
+[[package]]
+name = "pulldown-cmark"
+version = "0.13.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e9f068eba8e7071c5f9511831b44f32c740d5adf574e990f946ddb53db2f314e"
+dependencies = [
+ "bitflags 2.13.1",
+ "memchr",
+ "pulldown-cmark-escape",
+ "unicase",
+]
+
+[[package]]
+name = "pulldown-cmark-escape"
+version = "0.11.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "007d8adb5ddab6f8e3f491ac63566a7d5002cc7ed73901f72057943fa71ae1ae"
 
 [[package]]
 name = "quote"
@@ -3084,6 +3218,12 @@
 ]
 
 [[package]]
+name = "siphasher"
+version = "1.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8ee5873ec9cce0195efcb7a4e9507a04cd49aec9c83d0389df45b1ef7ba2e649"
+
+[[package]]
 name = "slab"
 version = "0.4.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -3191,6 +3331,30 @@
 checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f"
 
 [[package]]
+name = "string_cache"
+version = "0.9.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a18596f8c785a729f2819c0f6a7eae6ebeebdfffbfe4214ae6b087f690e31901"
+dependencies = [
+ "new_debug_unreachable",
+ "parking_lot",
+ "phf_shared",
+ "precomputed-hash",
+]
+
+[[package]]
+name = "string_cache_codegen"
+version = "0.6.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "585635e46db231059f76c5849798146164652513eb9e8ab2685939dd90f29b69"
+dependencies = [
+ "phf_generator",
+ "phf_shared",
+ "proc-macro2",
+ "quote",
+]
+
+[[package]]
 name = "subtle"
 version = "2.6.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -3249,6 +3413,15 @@
 ]
 
 [[package]]
+name = "tendril"
+version = "0.5.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5fed54709c5b3a53d09bb1c113ea4f5ceafd1e772ddcb0030a82e1d56c087b08"
+dependencies = [
+ "new_debug_unreachable",
+]
+
+[[package]]
 name = "thiserror"
 version = "2.0.19"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -3297,11 +3470,13 @@
 name = "tit-cde"
 version = "0.1.0"
 dependencies = [
+ "ammonia",
  "askama",
  "axum",
  "clap",
  "gix",
  "gix-pack",
+ "pulldown-cmark",
  "rand",
  "rusqlite",
  "russh",
@@ -3424,6 +3599,12 @@
 version = "1.20.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20"
+
+[[package]]
+name = "unicase"
+version = "2.9.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "dbc4bc3a9f746d862c45cb89d705aa10f187bb96c76001afab07a0d35ce60142"
 
 [[package]]
 name = "unicode-bom"
@@ -3562,6 +3743,18 @@
 checksum = "dc1b4cb0cc549fcf58d7dfc081778139b3d283a081644e833e84682ad71cea24"
 dependencies = [
  "unicode-ident",
+]
+
+[[package]]
+name = "web_atoms"
+version = "0.2.5"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "075474b12bcb3d2e3d4546580e9de478eeeead668a1761e2a8860c836b7ef297"
+dependencies = [
+ "phf",
+ "phf_codegen",
+ "string_cache",
+ "string_cache_codegen",
 ]
 
 [[package]]

@@ -11,11 +11,13 @@
 path = "src/main.rs"
 
 [dependencies]
+ammonia = "4.1.4"
 askama = { version = "0.16", default-features = false, features = ["derive", "std"] }
 axum = { version = "0.8", default-features = false, features = ["http1", "original-uri", "query", "tokio"] }
 clap = { version = "4.6", default-features = false, features = ["derive", "error-context", "help", "std", "usage"] }
 gix = { version = "0.84", default-features = false, features = ["blame", "parallel", "sha1", "sha256"] }
 gix-pack = { version = "0.71", default-features = false, features = ["generate", "sha1", "sha256", "streaming-input"] }
+pulldown-cmark = { version = "0.13.4", default-features = false, features = ["html"] }
 rand = "0.10"
 rusqlite = { version = "0.40", default-features = false, features = ["backup", "bundled"] }
 russh = { version = "0.62", default-features = false, features = ["ring"] }

@@ -13,6 +13,10 @@
     "Unicode-3.0",
     "Zlib",
 ]
+exceptions = [
+    { allow = ["MPL-2.0"], crate = "cssparser@0.37.0" },
+    { allow = ["MPL-2.0"], crate = "dtoa-short@0.3.5" },
+]
 
 [bans]
 multiple-versions = "warn"

@@ -62,6 +62,5 @@
 filesystem name and repository ID do not change.
 
 HTML repository views use server-rendered templates and embedded CSS. Raw files
-use `application/octet-stream`. Archives use a streamed ustar body. The first
-public interface shows plain-text README content. Milestone 2.6 owns Markdown
-rendering and sanitization.
+use `application/octet-stream`. Archives use a streamed ustar body. ADR 0006
+specifies Markdown rendering and sanitization for README content.

@@ -1,0 +1,61 @@
+# Architectural decision record 0006: safe Markdown
+
+Status: Accepted
+
+Date: 2026-07-23
+
+## Context
+
+`tit` stores Markdown source from users and repositories. A public page must not
+let this source add active HTML, load a remote image, or use an unsafe link.
+The same rules must apply to README files and later CDE text fields.
+
+## Decision
+
+Support these CommonMark elements:
+
+- paragraphs and line breaks;
+- headings from level 1 through level 6;
+- strong text and emphasized text;
+- block quotes;
+- ordered and unordered lists;
+- inline code and fenced or indented code blocks;
+- thematic breaks; and
+- links.
+
+Do not support raw HTML, images, tables, task lists, footnotes, heading
+attributes, or other extensions. Show the alternative text of an image as
+plain text. Ignore a code-block language value.
+
+Permit relative links and absolute `http`, `https`, and `mailto` links. Reject
+scheme-relative links, backslashes, control characters, surrounding spaces,
+and all other URL schemes. Add `rel="nofollow noopener noreferrer"` to each
+link.
+
+Parse Markdown with `pulldown-cmark` version 0.13.4. Remove unsupported parser
+events before HTML generation. Clean the generated HTML with `ammonia` version
+4.1.4 and an exact allowlist. Permit only the tags that the supported subset
+can generate. Permit `href` and `title` on links, and permit `start` on ordered
+lists. Do not permit generic attributes.
+
+Ammonia uses `cssparser` version 0.37.0 and `dtoa-short` version 0.3.5. These
+two crates use the MPL-2.0 license. Add a license exception for each exact crate
+version. Do not add MPL-2.0 to the general license allowlist.
+
+Return rendered content in a separate Rust type. The Askama template can mark
+only this type as safe HTML. Continue to use normal Askama escaping for all
+other repository content.
+
+## Evidence
+
+Unit tests render each supported element. They also use raw scripts, raw
+images, Markdown images, active URL schemes, scheme-relative URLs, mixed-case
+schemes, controls, and backslashes. The public-route test reads hostile content
+from a real Git repository and checks the complete server-rendered page.
+
+## Consequences
+
+README files have a useful small format without an active-content path. Remote
+images cannot track a page view. A future milestone can use the same renderer
+for issue, pull-request, and comment bodies. A change to the subset or URL
+policy needs a change to this record and its hostile-content tests.

@@ -1,0 +1,6 @@
+#!/bin/sh
+set -eu
+
+./scripts/check
+cargo test --locked --release --bin tit markdown::tests
+cargo test --locked --release --test public_routes browses_and_clones_public_repositories_for_both_hash_formats

@@ -26,6 +26,7 @@
     RepositoryReadService, TreeEntryInfo,
 };
 use crate::git::upload_pack::{ProtocolVersion, UploadPack};
+use crate::markdown::{self, RenderedMarkdown};
 use crate::store::{DATABASE_FILE, RepositoryRecord, Store, StoreError};
 
 use super::{PublicWebConfig, RequestId, WebState, render_error};
@@ -830,7 +831,7 @@
     has_head: bool,
     has_readme: bool,
     readme_path: String,
-    readme_content: String,
+    readme_html: RenderedMarkdown,
     readme_binary: bool,
     history: Vec<CommitView>,
     entries: Vec<TreeView>,
@@ -861,7 +862,7 @@
             has_head: false,
             has_readme: false,
             readme_path: String::new(),
-            readme_content: String::new(),
+            readme_html: RenderedMarkdown::default(),
             readme_binary: false,
             history: Vec::new(),
             entries: Vec::new(),
@@ -898,7 +899,7 @@
             if let Ok(content) = std::str::from_utf8(&data)
                 && !data.contains(&0)
             {
-                page.readme_content = content.to_owned();
+                page.readme_html = markdown::render(content);
             } else {
                 page.readme_binary = true;
             }

@@ -16,6 +16,7 @@
 )]
 mod http;
 mod instance;
+mod markdown;
 #[allow(dead_code, reason = "M1B proves the SSH server before M2 calls it")]
 mod ssh;
 mod store;

@@ -1,0 +1,139 @@
+use std::collections::{HashMap, HashSet};
+use std::fmt;
+
+use ammonia::{Builder, UrlRelative};
+use pulldown_cmark::{Event, Parser, Tag, TagEnd, html};
+use url::Url;
+
+#[derive(Default)]
+pub struct RenderedMarkdown(String);
+
+impl fmt::Display for RenderedMarkdown {
+    fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
+        formatter.write_str(&self.0)
+    }
+}
+
+pub fn render(source: &str) -> RenderedMarkdown {
+    let mut skipped_link = false;
+    let events = Parser::new(source).filter_map(|event| match event {
+        Event::Start(Tag::Link { ref dest_url, .. }) if !safe_link(dest_url) => {
+            skipped_link = true;
+            None
+        }
+        Event::End(TagEnd::Link) if skipped_link => {
+            skipped_link = false;
+            None
+        }
+        Event::Start(Tag::Image { .. }) | Event::End(TagEnd::Image) => None,
+        Event::Html(_) | Event::InlineHtml(_) => None,
+        event => Some(event),
+    });
+
+    let mut rendered = String::new();
+    html::push_html(&mut rendered, events);
+
+    let tags = HashSet::from([
+        "a",
+        "blockquote",
+        "br",
+        "code",
+        "em",
+        "h1",
+        "h2",
+        "h3",
+        "h4",
+        "h5",
+        "h6",
+        "hr",
+        "li",
+        "ol",
+        "p",
+        "pre",
+        "strong",
+        "ul",
+    ]);
+    let tag_attributes = HashMap::from([
+        ("a", HashSet::from(["href", "title"])),
+        ("ol", HashSet::from(["start"])),
+    ]);
+    let url_schemes = HashSet::from(["http", "https", "mailto"]);
+    let mut sanitizer = Builder::new();
+    sanitizer
+        .tags(tags)
+        .tag_attributes(tag_attributes)
+        .generic_attributes(HashSet::new())
+        .url_schemes(url_schemes)
+        .url_relative(UrlRelative::PassThrough)
+        .link_rel(Some("nofollow noopener noreferrer"));
+
+    RenderedMarkdown(sanitizer.clean(&rendered).to_string())
+}
+
+fn safe_link(destination: &str) -> bool {
+    if destination.trim() != destination
+        || destination.chars().any(char::is_control)
+        || destination.contains('\\')
+        || destination.starts_with("//")
+    {
+        return false;
+    }
+
+    match Url::parse(destination) {
+        Ok(url) => matches!(url.scheme(), "http" | "https" | "mailto"),
+        Err(url::ParseError::RelativeUrlWithoutBase) => true,
+        Err(_) => false,
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::render;
+
+    #[test]
+    fn renders_the_documented_subset() {
+        let output = render(
+            "# Title\n\nA **strong** and *short* paragraph.\n\n- one\n- two\n\n> quote\n\n```rust\nlet safe = true;\n```\n\n[local](docs/guide.md) [web](https://example.com).",
+        )
+        .to_string();
+
+        assert!(output.contains("<h1>Title</h1>"));
+        assert!(output.contains("<strong>strong</strong>"));
+        assert!(output.contains("<em>short</em>"));
+        assert!(output.contains("<ul>"));
+        assert!(output.contains("<blockquote>"));
+        assert!(output.contains("<pre><code>let safe = true;"));
+        assert!(output.contains("href=\"docs/guide.md\""));
+        assert!(output.contains("href=\"https://example.com\""));
+        assert!(!output.contains("class="));
+    }
+
+    #[test]
+    fn removes_html_images_and_active_links() {
+        let output = render(
+            "<script>alert(1)</script>\n\n<img src=x onerror=alert(2)>\n\n![alt](https://tracker.example/pixel)\n\n[bad](javascript:alert(3)) [data](data:text/html,x) [network](//example.com/x) [mail](mailto:user@example.com)",
+        )
+        .to_string();
+
+        assert!(!output.contains("script"));
+        assert!(!output.contains("img"));
+        assert!(!output.contains("tracker.example"));
+        assert!(!output.contains("javascript"));
+        assert!(!output.contains("data:text"));
+        assert!(!output.contains("//example.com"));
+        assert!(output.contains("alt"));
+        assert!(output.contains("href=\"mailto:user@example.com\""));
+        assert!(output.contains("rel=\"nofollow noopener noreferrer\""));
+    }
+
+    #[test]
+    fn escapes_text_and_rejects_obscure_schemes() {
+        let output = render(
+            "Text & <b>markup</b>. [case](JaVaScRiPt:alert(1)) [space]( javascript:alert(2)) [slash](\\\\example.com/x)",
+        )
+        .to_string();
+
+        assert!(output.contains("Text &amp; markup."));
+        assert!(!output.contains("href="));
+    }
+}

@@ -39,7 +39,7 @@
 {% if readme_binary %}
     <p>Binary README content cannot be shown.</p>
 {% else %}
-    <pre>{{ readme_content }}</pre>
+    <div class="markdown">{{ readme_html|safe }}</div>
 {% endif %}
   </section>
 {% endif %}

@@ -24,6 +24,8 @@
 )]
 #[path = "../src/instance.rs"]
 mod instance;
+#[path = "../src/markdown.rs"]
+mod markdown;
 #[allow(
     dead_code,
     reason = "the public-route test does not use each store API"
@@ -66,8 +68,15 @@
         assert!(summary_text.contains("ssh://tit.example:2222/alice/example"));
         assert!(summary_text.contains(&fixture.head));
         assert!(summary_text.contains("README.md"));
-        assert!(summary_text.contains("tit fixture <safe>"));
+        assert!(summary_text.contains("<h1>tit fixture</h1>"));
+        assert!(summary_text.contains("<strong>safe</strong>"));
+        assert!(summary_text.contains("<code>&lt;safe&gt;</code>"));
+        assert!(summary_text.contains("href=\"docs/guide.md\""));
         assert!(!summary_text.contains("<script"));
+        assert!(!summary_text.contains("javascript:"));
+        assert!(!summary_text.contains("<img"));
+        assert!(!summary_text.contains("tracker.example"));
+        assert!(summary_text.contains("&#60;script&#62;alert(3)&#60;/script&#62;"));
 
         let empty = request(server.address(), "GET", "/alice/empty", &[], &[]);
         assert_eq!(empty.status, 200);
@@ -113,6 +122,8 @@
         assert!(tree_text.contains("binary.dat"));
         assert!(tree_text.contains("non-å.txt"));
         assert!(tree_text.contains("non-%C3%A5.txt"));
+        assert!(tree_text.contains("&#60;img src=x onerror=alert(4)&#62;.txt"));
+        assert!(!tree_text.contains("<img"));
 
         let blob = request(
             server.address(),
@@ -378,10 +389,19 @@
         run(Command::new("git")
             .args(["init", "-q", "-b", "main", "--object-format", format])
             .arg(&worktree));
-        fs::write(worktree.join("README.md"), b"# tit fixture <safe>\n").expect("write the README");
+        fs::write(
+            worktree.join("README.md"),
+            b"# tit fixture\n\n**safe** and `<safe>`\n\n[guide](docs/guide.md) [bad](javascript:alert(1))\n\n![tracker](https://tracker.example/pixel)\n\n<script>alert(2)</script>\n",
+        )
+        .expect("write the README");
         fs::create_dir(worktree.join("nested")).expect("create a nested directory");
         fs::write(worktree.join("nested/file.txt"), b"first line\n").expect("write the text file");
         fs::write(worktree.join("binary.dat"), b"binary\0content").expect("write the binary file");
+        fs::write(
+            worktree.join("<img src=x onerror=alert(4)>.txt"),
+            b"escaped path\n",
+        )
+        .expect("write the hostile path");
         fs::write(worktree.join("non-å.txt"), b"non-UTF-8 path\n")
             .expect("write the percent-encoded path");
         commit_all(&worktree, "first commit");
@@ -391,7 +411,7 @@
             b"first line\nsecond line\n",
         )
         .expect("update the text file");
-        commit_all(&worktree, "second commit");
+        commit_all(&worktree, "<script>alert(3)</script>");
         let head = rev_parse(&worktree, "HEAD");
 
         run(Command::new("git")

@@ -22,6 +22,8 @@
 )]
 #[path = "../src/instance.rs"]
 mod instance;
+#[path = "../src/markdown.rs"]
+mod markdown;
 #[allow(dead_code, reason = "the shell test does not use repository storage")]
 #[path = "../src/store/mod.rs"]
 mod store;