From 9ffdd4347e7e2d52d3f6c5b3904b7035d693e1e2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 05:36:18 +0000 Subject: [PATCH 1/2] Fix PagesController slug derivation for root and custom paths Deriving the page slug from request.path broke two cases: - bunko_page :home, path: "/" resolved to a nil slug and 404ed - bunko_page :about, path: "about-us" looked up "about-us" instead of the post's actual slug "about" Read params[:page] instead, which bunko_page already provides via defaults: {page: ...}. Route defaults are path parameters and take precedence over query string params in Rails, so the query string still cannot override the slug. A format guard 404s if the action is ever wired up without a :page default. Updates the template and the committed namespaced dummy controller in sync, and adds regression tests for root-path pages, custom-path pages, and query-param override attempts. Fixes #57 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019SzbpeC8uQM8DmEPpi86vT --- CHANGELOG.md | 3 + .../controllers/pages_controller.rb.tt | 14 ++- test/controllers/pages_controller_test.rb | 88 +++++++++++++++++++ .../app/controllers/legal/pages_controller.rb | 15 +++- 4 files changed, 112 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 025c278..b85d562 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Building toward 1.0.0 release. Using 0.x versions during active development. +**Fixed:** +- `PagesController` now reads the page slug from the route's `defaults: {page: ...}` (via `params[:page]`) instead of parsing `request.path`, fixing 404s for root-path pages (`bunko_page :home, path: "/"`) and pages with custom paths (`bunko_page :about, path: "about-us"`) (#57) + ## [0.2.0] - 2025-11-14 First functional release of Bunko - a lightweight Rails CMS based on the "one model, infinite collections" philosophy. diff --git a/lib/tasks/templates/controllers/pages_controller.rb.tt b/lib/tasks/templates/controllers/pages_controller.rb.tt index 3772490..f6b280a 100644 --- a/lib/tasks/templates/controllers/pages_controller.rb.tt +++ b/lib/tasks/templates/controllers/pages_controller.rb.tt @@ -2,10 +2,16 @@ class PagesController < ApplicationController def show - # Extract page slug from request path (not user-controllable params) - # This prevents path traversal attacks via query string manipulation - # e.g., GET /about?page=../../admin/users - page_slug = request.path.split("/").reject(&:empty?).last + # The page slug comes from the route's defaults (bunko_page sets + # defaults: {page: ...}). Route defaults are path parameters, which + # take precedence over query string params in Rails, so + # e.g. GET /about?page=../../admin/users cannot override the slug. + page_slug = params[:page].to_s + + # Format guard in case this action is wired up without a :page default + unless page_slug.match?(/\A[a-z0-9-]+\z/) + raise ActiveRecord::RecordNotFound, "Page not found" + end @post = Post.published.find_by( post_type: PostType.find_by(name: "pages"), diff --git a/test/controllers/pages_controller_test.rb b/test/controllers/pages_controller_test.rb index db6f8e4..b329e48 100644 --- a/test/controllers/pages_controller_test.rb +++ b/test/controllers/pages_controller_test.rb @@ -149,4 +149,92 @@ class PagesControllerTest < ActionDispatch::IntegrationTest assert_match "Our privacy policy", response.body assert_no_match "Our terms", response.body end + + # Regression tests for slug derivation (GitHub issue #57) + # The slug must come from the route's defaults, not from request.path + + test "bunko_page with root path renders the home page" do + Post.create!( + title: "Home", + slug: "home", + content: "Welcome to the home page", + post_type: @pages_type, + status: "published", + published_at: 1.day.ago + ) + + draw_dummy_routes_with do + bunko_page :home, path: "/" + end + + get "/" + assert_response :success + assert_match "Welcome to the home page", response.body + ensure + Rails.application.reload_routes! + end + + test "bunko_page with custom path finds post by page name, not path" do + Post.create!( + title: "About", + slug: "about", + content: "All about the company", + post_type: @pages_type, + status: "published", + published_at: 1.day.ago + ) + + draw_dummy_routes_with do + bunko_page :about, path: "company-info" + end + + get "/company-info" + assert_response :success + assert_match "All about the company", response.body + ensure + Rails.application.reload_routes! + end + + test "query param cannot override route default on custom-path page" do + Post.create!( + title: "About", + slug: "about", + content: "All about the company", + post_type: @pages_type, + status: "published", + published_at: 1.day.ago + ) + + draw_dummy_routes_with do + bunko_page :about, path: "about-us" + end + + get "/about-us?page=privacy-policy" + assert_response :success + # Route default (page: "about") wins over the query string param + assert_match "All about the company", response.body + assert_no_match "Our privacy policy", response.body + ensure + Rails.application.reload_routes! + end + + private + + # Redraws the dummy app's routes with extra routes prepended, so tests can + # exercise bunko_page options (root path, custom paths) the committed routes + # file doesn't use. The shared nav partial links to every collection, so the + # full set from test/dummy/config/routes.rb must stay present. + def draw_dummy_routes_with(&extra) + Rails.application.routes.draw do + instance_exec(&extra) + + root "blog#index" + bunko_collection :blog + bunko_collection :docs + bunko_collection :articles + bunko_collection :videos + bunko_collection :long_reads + bunko_collection :all_content + end + end end diff --git a/test/dummy/app/controllers/legal/pages_controller.rb b/test/dummy/app/controllers/legal/pages_controller.rb index 33584a3..57570c9 100644 --- a/test/dummy/app/controllers/legal/pages_controller.rb +++ b/test/dummy/app/controllers/legal/pages_controller.rb @@ -3,10 +3,17 @@ module Legal class PagesController < ApplicationController def show - # Extract page slug from request path (not user-controllable params) - # This prevents path traversal attacks via query string manipulation - # e.g., GET /legal/privacy-policy?page=../../admin/users - page_slug = request.path.split("/").reject(&:empty?).last + # The page slug comes from the route's defaults (bunko_page sets + # defaults: {page: ...}). Route defaults are path parameters, which + # take precedence over query string params in Rails, so + # e.g. GET /legal/privacy-policy?page=../../admin/users cannot + # override the slug. + page_slug = params[:page].to_s + + # Format guard in case this action is wired up without a :page default + unless page_slug.match?(/\A[a-z0-9-]+\z/) + raise ActiveRecord::RecordNotFound, "Page not found" + end @post = Post.published.find_by( post_type: PostType.find_by(name: "pages"), From c89d8c80a519fd816ff10af175fe66daaf8ad88d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Jul 2026 05:48:44 +0000 Subject: [PATCH 2/2] Document issue-closing keyword convention in CLAUDE.md Always mark PRs and commits with "Closes #xx" / "Fixes #xx" when working on a ticket so the issue auto-closes on merge. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019SzbpeC8uQM8DmEPpi86vT --- CLAUDE.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 54c6e7f..9eea729 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -144,6 +144,14 @@ Integration tests run against a minimal Rails app in `test/dummy/`: - Tests use **Minitest** framework - CI runs on GitHub Actions (`.github/workflows/main.yml`) and executes `bundle exec rake` +### Linking Work to GitHub Issues + +When working on a GitHub issue/ticket, always include a closing keyword so the issue closes automatically on merge: + +- Add `Closes #xx` (or `Fixes #xx`) to the **pull request description** — this is what GitHub uses to auto-close the issue when the PR merges +- Also include it in the commit message body when the commit resolves the issue +- Use one line per issue if a change resolves multiple issues (e.g., `Closes #12`, `Closes #34`) + ## Current Features (Implemented) **Milestone 1 - Post Model Behavior:**