From 6887ae40878fd4a8880379154270d0b68031af87 Mon Sep 17 00:00:00 2001
From: Willem van den Ende <willem+gitea@livingsoftware.co.uk>
Date: Tue, 17 Mar 2026 12:17:29 +0000
Subject: [PATCH] Security: Validate blog controller inputs (page param, blog
 ID)

Ran a claude /security-review, fixed two vulnerabilities

  Use a plug to resolve blog_id, returning a clean 404 for unknown blogs
  instead of raising with inspect(). Parse page param with Integer.parse
  so invalid values (non-numeric, negative, zero) fall back to page 1
  instead of crashing. Add 5 tests covering these cases.a
---
 .../controllers/blog_controller.ex            | 39 +++++++++++++------
 .../firehose_web/controllers/blog_test.exs    | 28 +++++++++++++
 2 files changed, 56 insertions(+), 11 deletions(-)

diff --git a/app/lib/firehose_web/controllers/blog_controller.ex b/app/lib/firehose_web/controllers/blog_controller.ex
index c010fa5..6890a09 100644
--- a/app/lib/firehose_web/controllers/blog_controller.ex
+++ b/app/lib/firehose_web/controllers/blog_controller.ex
@@ -1,9 +1,11 @@
 defmodule FirehoseWeb.BlogController do
   use FirehoseWeb, :controller
 
-  def index(conn, %{"blog_id" => blog_id} = params) do
-    blog = resolve_blog!(blog_id)
-    page = String.to_integer(params["page"] || "1")
+  plug :resolve_blog
+
+  def index(conn, params) do
+    blog = conn.assigns.blog
+    page = parse_page(params["page"])
     result = blog.paginate(page)
 
     render(conn, :index,
@@ -17,8 +19,8 @@ defmodule FirehoseWeb.BlogController do
     )
   end
 
-  def show(conn, %{"blog_id" => blog_id, "slug" => slug}) do
-    blog = resolve_blog!(blog_id)
+  def show(conn, %{"slug" => slug}) do
+    blog = conn.assigns.blog
     post = blog.get_post!(slug)
 
     render(conn, :show,
@@ -28,8 +30,8 @@ defmodule FirehoseWeb.BlogController do
     )
   end
 
-  def tag(conn, %{"blog_id" => blog_id, "tag" => tag}) do
-    blog = resolve_blog!(blog_id)
+  def tag(conn, %{"tag" => tag}) do
+    blog = conn.assigns.blog
     posts = blog.posts_by_tag(tag)
 
     render(conn, :tag,
@@ -41,10 +43,25 @@ defmodule FirehoseWeb.BlogController do
     )
   end
 
-  defp resolve_blog!("engineering"), do: Firehose.EngineeringBlog
-  defp resolve_blog!("releases"), do: Firehose.ReleaseNotes
+  defp resolve_blog(%{params: %{"blog_id" => "engineering"}} = conn, _opts),
+    do: assign(conn, :blog, Firehose.EngineeringBlog)
 
-  defp resolve_blog!(id) do
-    raise Blogex.NotFoundError, "unknown blog: #{inspect(id)}"
+  defp resolve_blog(%{params: %{"blog_id" => "releases"}} = conn, _opts),
+    do: assign(conn, :blog, Firehose.ReleaseNotes)
+
+  defp resolve_blog(conn, _opts) do
+    conn
+    |> put_status(:not_found)
+    |> put_view(FirehoseWeb.ErrorHTML)
+    |> render(:"404")
+    |> halt()
+  end
+
+  defp parse_page(nil), do: 1
+  defp parse_page(str) do
+    case Integer.parse(str) do
+      {page, ""} when page > 0 -> page
+      _ -> 1
+    end
   end
 end
diff --git a/app/test/firehose_web/controllers/blog_test.exs b/app/test/firehose_web/controllers/blog_test.exs
index 378bc9c..f1cf813 100644
--- a/app/test/firehose_web/controllers/blog_test.exs
+++ b/app/test/firehose_web/controllers/blog_test.exs
@@ -26,6 +26,34 @@ defmodule FirehoseWeb.BlogTest do
     end
   end
 
+  describe "input validation" do
+    test "GET /blog/nonexistent returns 404", %{conn: conn} do
+      conn = get(conn, "/blog/nonexistent")
+      assert html_response(conn, 404)
+    end
+
+    test "GET /blog/engineering?page=abc falls back to page 1", %{conn: conn} do
+      conn = get(conn, "/blog/engineering?page=abc")
+      assert html_response(conn, 200) =~ "Engineering Blog"
+    end
+
+    test "GET /blog/engineering?page=-1 falls back to page 1", %{conn: conn} do
+      conn = get(conn, "/blog/engineering?page=-1")
+      assert html_response(conn, 200) =~ "Engineering Blog"
+    end
+
+    test "GET /blog/engineering?page=0 falls back to page 1", %{conn: conn} do
+      conn = get(conn, "/blog/engineering?page=0")
+      assert html_response(conn, 200) =~ "Engineering Blog"
+    end
+
+    test "GET /blog/engineering/nonexistent-post returns 404", %{conn: conn} do
+      assert_raise Blogex.NotFoundError, fn ->
+        get(conn, "/blog/engineering/nonexistent-post")
+      end
+    end
+  end
+
   describe "release notes blog (HTML)" do
     test "GET /blog/releases returns HTML index", %{conn: conn} do
       conn = get(conn, "/blog/releases")