From c04d8c856cf38fee896255eb226adcee616679c4 Mon Sep 17 00:00:00 2001 From: Willem van den Ende Date: Wed, 28 Oct 2026 22:08:45 +0000 Subject: [PATCH] =?UTF-8?q?yak(link-validator-never-sees-compiled-html-int?= =?UTF-8?q?ernal-link-validation-is-vacuous-ddd1):=20Link=20validator=20ne?= =?UTF-8?q?ver=20sees=20compiled=20HTML=20=E2=80=94=20internal-link=20vali?= =?UTF-8?q?dation=20is=20vacuous?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/mix.lock | 1 + .../firehose_web/controllers/blog_test.exs | 43 ++++++++++++++ blogex/lib/blogex/link_validator.ex | 51 ++++++++++------- blogex/mix.exs | 1 + blogex/mix.lock | 1 + .../test/2026/01-03-internal-link-post.md | 7 +++ blogex/test/blogex/blog_integration_test.exs | 34 +++++++++++ blogex/test/blogex/link_validator_test.exs | 56 +++++++++++++++++++ 8 files changed, 175 insertions(+), 19 deletions(-) create mode 100644 blogex/priv/blog/test/2026/01-03-internal-link-post.md diff --git a/app/mix.lock b/app/mix.lock index 068a819..fa58e95 100644 --- a/app/mix.lock +++ b/app/mix.lock @@ -19,6 +19,7 @@ "file_system": {:hex, :file_system, "1.1.1", "31864f4685b0148f25bd3fbef2b1228457c0c89024ad67f7a81a3ffbc0bbad3a", [:mix], [], "hexpm", "7a15ff97dfe526aeefb090a7a9d3d03aa907e100e262a0f8f7746b78f8f87a5d"}, "finch": {:hex, :finch, "0.24.0", "4022b6194e907b6d9b597c168001cac38e488a35fe36cef9d8297e3159782510", [:mix], [{:mime, "~> 1.0 or ~> 2.0", [hex: :mime, repo: "hexpm", optional: false]}, {:mint, "~> 1.8", [hex: :mint, repo: "hexpm", optional: false]}, {:nimble_options, "~> 0.4 or ~> 1.0", [hex: :nimble_options, repo: "hexpm", optional: false]}, {:nimble_pool, "~> 1.1", [hex: :nimble_pool, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "33ba40069c3587c2f99f9125b766e19dad87d6d54be3c6961db2304df04cef00"}, "fine": {:hex, :fine, "0.1.6", "4bf7151493443c454aac9f2fa2f34f5fefd0346a83fb5586a016c4a135c63247", [:mix], [], "hexpm", "5638eb4495488e885ebec167fa57973e5c35e1a50c344eb7666c90ec1c4e3b12"}, + "floki": {:hex, :floki, "0.38.4", "10f98971e892aed2c2f1b3a0f928e488e3797e1c6dd3dfd98db40b14e9a78bcf", [:mix], [], "hexpm", "bdb34645eee8e79845c7edaca2d4099a52804ee4d4a3ecc683a69451f0244973"}, "gen_smtp": {:hex, :gen_smtp, "1.3.0", "62c3d91f0dcf6ce9db71bcb6881d7ad0d1d834c7f38c13fa8e952f4104a8442e", [:rebar3], [{:ranch, ">= 1.8.0", [hex: :ranch, repo: "hexpm", optional: false]}], "hexpm", "0b73fbf069864ecbce02fe653b16d3f35fd889d0fdd4e14527675565c39d84e6"}, "gettext": {:hex, :gettext, "0.26.2", "5978aa7b21fada6deabf1f6341ddba50bc69c999e812211903b169799208f2a8", [:mix], [{:expo, "~> 0.5.1 or ~> 1.0", [hex: :expo, repo: "hexpm", optional: false]}], "hexpm", "aa978504bcf76511efdc22d580ba08e2279caab1066b76bb9aa81c4a1e0a32a5"}, "heroicons": {:git, "https://github.com/tailwindlabs/heroicons.git", "0435d4ca364a608cc75e2f8683d374e55abbae26", [tag: "v2.2.0", sparse: "optimized", depth: 1]}, diff --git a/app/test/firehose_web/controllers/blog_test.exs b/app/test/firehose_web/controllers/blog_test.exs index 55f6d42..1284e83 100644 --- a/app/test/firehose_web/controllers/blog_test.exs +++ b/app/test/firehose_web/controllers/blog_test.exs @@ -88,6 +88,49 @@ defmodule FirehoseWeb.BlogTest do end end + describe "internal links in compiled posts" do + # `Blogex.Blog._validate_links/2` validates `post.body` after + # NimblePublisher rendered it to HTML. If extraction cannot read that HTML, + # every post looks link-free and compile-time validation is vacuous - which + # is the failure the real posts below pin against. + defp posts_with_internal_links do + Enum.filter(Firehose.EngineeringBlog.all_posts(), fn post -> + Blogex.LinkValidator.extract_links(post.body) != [] + end) + end + + test "the validator sees internal links in real compiled posts" do + refute posts_with_internal_links() == [] + end + + test "the post that links to swim-the-test exposes that link" do + post = + Enum.find(posts_with_internal_links(), &(&1.id == "delayed-gratification-in-ai-summer")) + + assert post, "expected the compiled post to be visible with internal links extracted" + + assert "/blog/engineering/swim-the-test" in Blogex.LinkValidator.extract_links(post.body) + end + + test "no internal link in a real compiled post is invisible to extraction" do + # Deliberately permissive scan (any whose href appears in any + # position, quoted or not): every /blog/ link it finds must also come out + # of extract_links/1, or a bad one could never be reported. + links_in_body = fn body -> + ~r/]*?href=["']?(\/blog\/[^"'\s>]+)/ + |> Regex.scan(body) + |> Enum.map(fn [_, link] -> link end) + |> Enum.uniq() + end + + for post <- Firehose.EngineeringBlog.all_posts(), + link <- links_in_body.(post.body) do + assert link in Blogex.LinkValidator.extract_links(post.body), + "#{post.id}: #{link} is in the compiled body but not extracted" + end + end + end + describe "input validation" do test "GET /blog/nonexistent returns 404", %{conn: conn} do assert conn |> get("/blog/nonexistent") |> html_response(404) diff --git a/blogex/lib/blogex/link_validator.ex b/blogex/lib/blogex/link_validator.ex index 6969347..e363415 100644 --- a/blogex/lib/blogex/link_validator.ex +++ b/blogex/lib/blogex/link_validator.ex @@ -1,6 +1,12 @@ defmodule Blogex.LinkValidator do @moduledoc """ - Validates internal blog link semantics in markdown post bodies. + Validates internal blog link semantics in compiled post bodies. + + Post bodies reach this module as HTML: `Blogex.Blog` validates them in + `@before_compile`, after NimblePublisher has rendered the markdown. Markdown + link syntax is still understood so bodies and snippets can be validated + before rendering, but the HTML `` anchors are the ones that + matter in the real pipeline. Checks that internal links follow the pattern `/blog/{blog_id}/{slug}` where `blog_id` is a valid blog identifier and `slug` matches the @@ -50,21 +56,25 @@ defmodule Blogex.LinkValidator do @slug_regex ~r/^[a-z0-9]+(-[a-z0-9]+)*$/ @doc """ - Extracts internal blog links from a markdown body. + Extracts internal blog links from a post body. - Returns a list of link paths (strings) that match the pattern - `/blog/{engineering|releases}/{slug}`. External links and non-blog - internal links are ignored. + Returns a list of link paths (strings) that look like `/blog/…` links. + External links and non-blog internal links are ignored. Blog IDs and slugs + are *not* judged here — `validate_link/1` classifies those. - Handles both markdown link syntax `[text](url)` and HTML ``. - (Post bodies are HTML, rendered by NimblePublisher at compile time.) + HTML anchors are read with Floki, so attribute order, quoting and extra + attributes do not matter. Markdown link syntax `[text](url)` is extracted + too (raw markdown bodies and snippets), and the two scans are merged. ## Examples iex> extract_links("[link](/blog/engineering/post)") ["/blog/engineering/post"] - iex> extract_links("

link

") + iex> extract_links("

link

") + ["/blog/engineering/post"] + + iex> extract_links("

link

") ["/blog/engineering/post"] iex> extract_links("See [GitHub](https://github.com)") @@ -72,21 +82,24 @@ defmodule Blogex.LinkValidator do """ @spec extract_links(String.t()) :: [String.t()] def extract_links(body) when is_binary(body) do - markdown_links = - ~r/\[([^\]]+)\]\(([^)]+)\)/ - |> Regex.scan(body) - |> Enum.map(fn [_, _, path] -> path end) - - html_links = - ~r/ Regex.scan(body) - |> Enum.map(fn [_, path] -> path end) - - (markdown_links ++ html_links) + (markdown_links(body) ++ html_links(body)) |> Enum.uniq() |> Enum.filter(&internal_blog_link?/1) end + defp markdown_links(body) do + ~r/\[([^\]]+)\]\(([^)]+)\)/ + |> Regex.scan(body) + |> Enum.map(fn [_, _, path] -> path end) + end + + defp html_links(body) do + body + |> Floki.parse_document!() + |> Floki.find("a[href]") + |> Enum.flat_map(&Floki.attribute(&1, "href")) + end + defp internal_blog_link?(path) do case parse_blog_link(path) do {_, _} -> true diff --git a/blogex/mix.exs b/blogex/mix.exs index 16c56b7..d867a65 100644 --- a/blogex/mix.exs +++ b/blogex/mix.exs @@ -38,6 +38,7 @@ defmodule Blogex.MixProject do defp deps do [ {:nimble_publisher, "~> 2.0"}, + {:floki, "~> 0.38"}, {:makeup_elixir, ">= 0.0.0"}, {:makeup_erlang, ">= 0.0.0"}, {:phoenix, "~> 1.7"}, diff --git a/blogex/mix.lock b/blogex/mix.lock index 2f300b8..02a418f 100644 --- a/blogex/mix.lock +++ b/blogex/mix.lock @@ -4,6 +4,7 @@ "earmark_parser": {:hex, :earmark_parser, "1.4.44", "f20830dd6b5c77afe2b063777ddbbff09f9759396500cdbe7523efd58d7a339c", [:mix], [], "hexpm", "4778ac752b4701a5599215f7030989c989ffdc4f6df457c5f36938cc2d2a2750"}, "ex_crap": {:hex, :ex_crap, "0.1.1", "c1770e7ce1843f304a27f5d32bfa867a340140e337684ee1929c770b2499f884", [:mix], [{:boundary, "~> 0.10", [hex: :boundary, repo: "hexpm", optional: false]}], "hexpm", "ada5d59bce442c1a8a987cb5328cbf4469fb56eb8d04641886f2199569a75b12"}, "ex_doc": {:hex, :ex_doc, "0.40.1", "67542e4b6dde74811cfd580e2c0149b78010fd13001fda7cfeb2b2c2ffb1344d", [:mix], [{:earmark_parser, "~> 1.4.44", [hex: :earmark_parser, repo: "hexpm", optional: false]}, {:makeup_c, ">= 0.1.0", [hex: :makeup_c, repo: "hexpm", optional: true]}, {:makeup_elixir, "~> 0.14 or ~> 1.0", [hex: :makeup_elixir, repo: "hexpm", optional: false]}, {:makeup_erlang, "~> 0.1 or ~> 1.0", [hex: :makeup_erlang, repo: "hexpm", optional: false]}, {:makeup_html, ">= 0.1.0", [hex: :makeup_html, repo: "hexpm", optional: true]}], "hexpm", "bcef0e2d360d93ac19f01a85d58f91752d930c0a30e2681145feea6bd3516e00"}, + "floki": {:hex, :floki, "0.38.4", "10f98971e892aed2c2f1b3a0f928e488e3797e1c6dd3dfd98db40b14e9a78bcf", [:mix], [], "hexpm", "bdb34645eee8e79845c7edaca2d4099a52804ee4d4a3ecc683a69451f0244973"}, "jason": {:hex, :jason, "1.4.5", "2e3a008590b0b8d7388c20293e9dcc9cf3e5d642fd2a114e4cbbb52e595d940a", [:mix], [{:decimal, "~> 1.0 or ~> 2.0 or ~> 3.0", [hex: :decimal, repo: "hexpm", optional: true]}], "hexpm", "b0c823996102bcd0239b3c2444eb00409b72f6a140c1950bc8b457d836b30684"}, "makeup": {:hex, :makeup, "1.2.2", "882d46dc0905e9ff7abf2aab61a7e6b3dcc555533977d8a23b06019e6c89ac94", [:mix], [{:nimble_parsec, "~> 1.4", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "9a1a24e5b343b8ae16abea0822c10a6f75da27af7fa802ada5251f7579bfccfa"}, "makeup_elixir": {:hex, :makeup_elixir, "1.0.1", "e928a4f984e795e41e3abd27bfc09f51db16ab8ba1aebdba2b3a575437efafc2", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "7284900d412a3e5cfd97fdaed4f5ed389b8f2b4cb49efc0eb3bd10e2febf9507"}, diff --git a/blogex/priv/blog/test/2026/01-03-internal-link-post.md b/blogex/priv/blog/test/2026/01-03-internal-link-post.md new file mode 100644 index 0000000..70263b6 --- /dev/null +++ b/blogex/priv/blog/test/2026/01-03-internal-link-post.md @@ -0,0 +1,7 @@ +%{ + title: "Internal Link Post", + author: "Test Author", + description: "A post that links to another post" +} +--- +This post links to [hello world](/blog/engineering/hello-world) in its body. diff --git a/blogex/test/blogex/blog_integration_test.exs b/blogex/test/blogex/blog_integration_test.exs index 351bf5c..70214a5 100644 --- a/blogex/test/blogex/blog_integration_test.exs +++ b/blogex/test/blogex/blog_integration_test.exs @@ -93,4 +93,38 @@ defmodule Blogex.BlogIntegrationTest do File.rm!(tmp_file) end end + + describe "validation sees compiled post bodies" do + # `_validate_links/2` runs on `post.body` *after* NimblePublisher rendered + # the markdown, so validation only works if extraction reads the compiled + # HTML. This pins that contract against a real compiled body: if + # extraction ever goes back to scanning markdown syntax only, it fails. + test "extract_links finds internal links in a compiled post body" do + tmp_file = Path.join(System.tmp_dir!(), "test_blog_compiled_body.ex") + + File.write!(tmp_file, """ + defmodule TestBlogCompiledBody do + use Blogex.Blog, + blog_id: :test, + app: :blogex, + from: "priv/blog/test/2026/01-03-internal-link-post.md", + title: "Test Blog", + base_path: "/blog/test" + end + """) + + [{TestBlogCompiledBody, _bytecode}] = Code.compile_file(tmp_file, __ENV__.file) + + # `apply/3`: the module is compiled at runtime, so a static call would + # warn ("module is not available") on every test compile. + [post] = apply(TestBlogCompiledBody, :all_posts, []) + + # The markdown source is `[hello world](/blog/engineering/hello-world)`. + assert post.body =~ "/blog/engineering/hello-world" + + assert "/blog/engineering/hello-world" in Blogex.LinkValidator.extract_links(post.body) + + File.rm!(tmp_file) + end + end end diff --git a/blogex/test/blogex/link_validator_test.exs b/blogex/test/blogex/link_validator_test.exs index 3c42010..3681c81 100644 --- a/blogex/test/blogex/link_validator_test.exs +++ b/blogex/test/blogex/link_validator_test.exs @@ -29,6 +29,21 @@ defmodule Blogex.LinkValidatorTest do assert LinkValidator.extract_links(body) == ["/blog/engineering/tag/pi.dev"] end + # HTML anchors are not "the href right after first) <> + ~s( second) <> + ~s( third

) + + assert LinkValidator.extract_links(body) == [ + "/blog/engineering/first", + "/blog/releases/second", + "/blog/engineering/third" + ] + end + test "ignores external links" do body = "See [GitHub](https://github.com) and [internal](/blog/engineering/post)." @@ -157,6 +172,12 @@ defmodule Blogex.LinkValidatorTest do assert LinkValidator.validate_link("/blog/engineering/tag/pi.dev#top") == :ok end + # Tags are user-defined: the documented rule restricts *slugs*, not tags. + test "accepts tag links with dots and capitals" do + assert LinkValidator.validate_link("/blog/engineering/tag/pi.dev") == :ok + assert LinkValidator.validate_link("/blog/engineering/tag/Pi.dev") == :ok + end + test "rejects tag page link with empty tag" do assert LinkValidator.validate_link("/blog/engineering/tag/") == {:error, "empty tag in tag link"} @@ -267,5 +288,40 @@ defmodule Blogex.LinkValidatorTest do ] } end + + # `_validate_links/2` hands this module `post.body`, which NimblePublisher + # already rendered to HTML, so failure reporting has to work on HTML bodies. + # These are the two reports the compile-time integration tests depend on. + test "reports an unknown blog ID in an HTML body" do + body = ~s(

bad

) + + assert LinkValidator.validate_body(body, :engineering, post_id: "html-post") == { + :error, + [ + { + 1, + "/blog/unknown/broken", + "unknown blog ID: unknown", + post_id: "html-post" + } + ] + } + end + + test "reports an invalid slug in an HTML body" do + body = ~s(

bad

) + + assert LinkValidator.validate_body(body, :engineering, post_id: "html-post") == { + :error, + [ + { + 1, + "/blog/engineering/My-Post", + "slug must be lowercase alphanumeric with hyphens: My-Post", + post_id: "html-post" + } + ] + } + end end end