Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Procfile.dev
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
web: env RUBY_DEBUG_OPEN=true bin/rails server
js: yarn build --watch
css: yarn build:css --watch
css: yarn build:css --watch=always

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I verified this is functionally correct: with the pinned mise toolchain, yarn build:css --watch=always remained alive after stdin was closed. It is unrelated to YAML front matter, though, so consider moving it to a separate PR to keep review and rollback scope focused. Non-blocking.

40 changes: 35 additions & 5 deletions app/models/markdown_document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ class MarkdownDocument
autolink: true, tasklist: true, footnotes: true
}.freeze

# A leading `---` block of YAML, terminated by another `---` line. Common to
# every static-site generator's Markdown dialect (Jekyll, Hugo, Astro, ...).
FRONT_MATTER_PATTERN = /\A---[ \t]*\r?\n(.*?)\r?\n---[ \t]*\r?\n?(.*)\z/m

# Pinned so a stored paste renders identically forever. The ESM build boots
# itself via startOnLoad and upgrades every <pre class="mermaid">.
MERMAID_MODULE = "https://cdn.jsdelivr.net/npm/mermaid@11.4.1/dist/mermaid.esm.min.mjs".freeze
Expand Down Expand Up @@ -67,7 +71,7 @@ class MarkdownDocument
CSS

def initialize(markdown, filename: nil)
@markdown = markdown.to_s
@front_matter, @markdown = self.class.split_front_matter(markdown.to_s)
@filename = filename.to_s
end

Expand All @@ -81,7 +85,7 @@ def to_html
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>#{CGI.escapeHTML(document_title)}</title>
<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
#{description_meta_tag}<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed. The resulting HTML remains valid, so I consider this cosmetic/non-blocking, but constructing the <head> as consistently indented lines would remove the formatting glitch.

<link rel="stylesheet" href="#{FONTS_HREF}">
<style>
#{BRAND_CSS}#{HIGHLIGHT_CSS}
Expand Down Expand Up @@ -141,9 +145,35 @@ def mermaid_script
%(</script>)
end

# The document's own H1 names it; fall back to the filename (sans extension)
# so the paste still gets a meaningful <title> -- and title.
# Front matter's `title` wins if present; then the document's own H1; then
# the filename (sans extension), so the paste still gets a meaningful
# <title>.
def document_title
@heading.presence || File.basename(@filename, ".*").presence || "Untitled"
@front_matter["title"].presence || @heading.presence ||

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: @front_matter["title"] is not guaranteed to be a String. YAML.safe_load can return an Integer, TrueClass, Date, Array, or Hash here, and CGI.escapeHTML(document_title) then raises TypeError. I reproduced this with title: 2026-07-13 (a Date, because dates are explicitly permitted), title: 1984, title: true, a list, and a mapping. This makes an otherwise valid Markdown upload fail before model validation. Please validate the metadata schema or deliberately normalize supported scalar values before returning the title, and add regression tests for implicit YAML types.

File.basename(@filename, ".*").presence || "Untitled"
end

def description_meta_tag
description = @front_matter["description"].presence
return "" unless description

%(<meta name="description" content="#{CGI.escapeHTML(description.to_s)}">\n)
end

class << self
# Splits a leading YAML front-matter block off the Markdown body. Returns
# [{}, raw] unchanged if there's no front matter, the YAML doesn't parse,
# or it doesn't parse to a Hash -- front matter is metadata, never load-bearing
# for rendering, so any trouble here just means the whole file is treated
# as plain Markdown body.
def split_front_matter(raw)
match = FRONT_MATTER_PATTERN.match(raw)
return [ {}, raw ] unless match

data = YAML.safe_load(match[1], permitted_classes: [ Date, Time ])

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-severity resilience issue: deeply nested valid YAML can raise SystemStackError, which is not covered by the Psych::Exception rescue below. I reproduced SystemStackError: stack level too deep with a 4,021-byte front-matter document containing a 2,000-level nested flow sequence. That escapes the promised plain-Markdown fallback on public creation paths. Please enforce a safe nesting limit/parser strategy, or narrowly handle SystemStackError around this parse operation and fall back.

data.is_a?(Hash) ? [ data, match[2] ] : [ {}, raw ]
Comment on lines +170 to +174

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed locally, with one additional regex edge case: ---\n---\nbody does not match FRONT_MATTER_PATTERN at all because the pattern requires a newline between the captured YAML and the closing delimiter. ---\n\n---\nbody does match, but safe_load returns nil and the code falls back to the raw body. Both forms therefore render the delimiters (including an <hr>), so the fix needs to make the pattern accept an adjacent closing delimiter as well as treating nil as an empty mapping.

rescue Psych::Exception
[ {}, raw ]
end
end
end
58 changes: 58 additions & 0 deletions test/models/markdown_document_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,4 +56,62 @@ class MarkdownDocumentTest < ActiveSupport::TestCase

assert_not_includes html, "alert('xss')"
end

test "strips yaml front matter and uses it for title and description" do
html = MarkdownDocument.new(<<~MD, filename: "x.md").to_html
---
title: "My Document Title"
author: "Jane Doe"
date: 2026-07-13
tags: [markdown, tutorial, yaml]
draft: false
description: "A test document"
---

# Document Heading Starts Here
This is the regular Markdown body text.
MD

assert_includes html, "<title>My Document Title</title>"
assert_includes html, '<meta name="description" content="A test document">'
assert_not_includes html, "title: \"My Document Title\""
assert_includes html, ">Document Heading Starts Here</h1>"
end

test "front matter title overrides an h1" do
html = MarkdownDocument.new(<<~MD, filename: "x.md").to_html
---
title: From Front Matter
---
# From Heading
MD

assert_includes html, "<title>From Front Matter</title>"
end

test "falls back to h1, then filename, when front matter has no title" do
assert_includes MarkdownDocument.new("---\nauthor: Jane\n---\n# Heading", filename: "x.md").to_html,
"<title>Heading</title>"
assert_includes MarkdownDocument.new("---\nauthor: Jane\n---\nbody", filename: "my-notes.md").to_html,
"<title>my-notes</title>"
end

test "omits the description tag when front matter has none" do
html = MarkdownDocument.new("# Plain\n\ntext", filename: "x.md").to_html
assert_not_includes html, 'name="description"'
end

test "treats malformed or non-mapping front matter as plain body" do
malformed = MarkdownDocument.new("---\n[1, 2\n---\nbody", filename: "x.md").to_html
assert_includes malformed, "[1, 2"

sequence = MarkdownDocument.new("---\n- a\n- b\n---\nbody", filename: "x.md").to_html
assert_includes sequence, "<li>a</li>"
assert_includes sequence, "<li>b</li>"
end

test "does not treat a body horizontal rule as front matter" do
html = MarkdownDocument.new("no leading marker\n\n---\n\nafter the rule", filename: "x.md").to_html
assert_includes html, "<hr"
end

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please extend the boundary coverage before merging: non-string/implicitly typed title values, empty ---\n--- front matter, deeply nested YAML, aliases, and CRLF input. In particular, the alias case motivated broadening the rescue to Psych::Exception, but its regression test was removed in the final commit; keeping that test would protect the behavior from a later narrowing of the rescue.

end