Merge pull request #152 from publify/pull-in-10-0-stable

Pull in latest changes from 10-0-stable

.github/workflows/ruby.yml

@@ -8,10 +8,12 @@ name: CI
   push:
     branches:
       - master
+      - 10-0-stable
       - 9-2-stable
   pull_request:
     branches:
       - master
+      - 10-0-stable
       - 9-2-stable
   schedule:
     - cron: '16 4 12 * *'

CHANGELOG.md

@@ -1,5 +1,37 @@
 # Changelog
 
+## 10.0.2 / 2024-06-28
+
+### Security updates
+
+* Safely link target URLs for Redirects in admin ([#148] by [mvz])
+* Upgrade jquery-ui-rails to version 7.0 ([#149] by [mvz])
+
+### Functional changes
+
+* Use native datetime inputs in the Admin ([#121] by [mvz])
+* Display Theme description nicely in the admin ([#151] by [mvz])
+
+### Internal changes
+
+* Stop using and depending on REXML ([#123] by [mvz])
+* Remove inline javascript ([#124] by [mvz])
+* Switch to no-trailing-comma style ([#127] by [mvz])
+* Remove inline styles assigned in ERB templates ([#128] by [mvz])
+* Make Content.searchstring scope code more transparent ([#150] by [mvz])
+* Add erb-lint and fix initial warnings ([#125] by [mvz])
+
+[#121]: https://github.com/publify/publify_core/pull/121
+[#123]: https://github.com/publify/publify_core/pull/123
+[#124]: https://github.com/publify/publify_core/pull/124
+[#125]: https://github.com/publify/publify_core/pull/125
+[#127]: https://github.com/publify/publify_core/pull/127
+[#128]: https://github.com/publify/publify_core/pull/128
+[#148]: https://github.com/publify/publify_core/pull/148
+[#149]: https://github.com/publify/publify_core/pull/149
+[#150]: https://github.com/publify/publify_core/pull/150
+[#151]: https://github.com/publify/publify_core/pull/151
+
 ## 10.0.1 / 2023-10-28
 
 * Update CarrierWave dependency to version 3.0 ([#102] by [mvz])
@@ -18,7 +50,6 @@
 [#118]: https://github.com/publify/publify_core/pull/118
 [#119]: https://github.com/publify/publify_core/pull/119
 [mvz]: https://github.com/mvz
-[dependabot]: https://github.com/apps/dependabot
 
 ## 10.0.0 / 2023-06-25
 

app/controllers/admin/themes_controller.rb

@@ -3,16 +3,12 @@
 class Admin::ThemesController < Admin::BaseController
   def index
     @themes = Theme.find_all
-    @themes.each do |theme|
-      # TODO: Move to Theme
-      theme.description_html = TextFilter.none.filter_text(theme.description)
-    end
     @active = this_blog.current_theme
   end
 
   def preview
     theme = Theme.find(params[:theme])
-    send_file File.join(theme.path, "preview.png"),
+    send_file theme.theme_file("preview.png"),
               type: "image/png", disposition: "inline", stream: false
   end
 

app/models/content.rb

@@ -25,10 +25,15 @@ class Content < ApplicationRecord
   scope :drafts, -> { where(state: "draft").order("created_at DESC") }
   scope :no_draft, -> { where.not(state: "draft").order("published_at DESC") }
   scope :searchstring, lambda { |search_string|
+    result = where(state: "published")
+
     tokens = search_string.split(" ").map { |c| "%#{c.downcase}%" }
-    matcher = "(LOWER(body) LIKE ? OR LOWER(extended) LIKE ? OR LOWER(title) LIKE ?)"
-    template = "state = ? AND #{([matcher] * tokens.size).join(" AND ")}"
-    where(template, "published", *tokens.map { |token| [token] * 3 }.flatten)
+    tokens.each do |token|
+      result = result
+        .where("(LOWER(body) LIKE ? OR LOWER(extended) LIKE ? OR LOWER(title) LIKE ?)",
+               token, token, token)
+    end
+    result
   }
 
   scope :published_at_like, lambda { |date_at|

app/models/redirect.rb

@@ -17,8 +17,8 @@ def full_to_path
     return path if %r{^(https?)://([^/]*)(.*)}.match?(path)
 
     url_root = blog.root_path
-    unless url_root.nil? || path[0, url_root.length] == url_root
-      path = File.join(url_root, path)
+    if url_root.length == 0 || path[0, url_root.length] != url_root
+      path = blog.url_for(path, only_path: true)
     end
     path
   end

app/views/admin/redirects/_index_and_form.html.erb

@@ -58,7 +58,7 @@
             <%= button_to_delete redirect %>
           </div>
         </td>
-        <td><%= link_to(redirect.to_path, redirect.to_path) %></td>
+        <td><%= link_to(redirect.to_path, redirect.full_to_path) %></td>
       </tr>
       <% end %>
       <%= display_pagination(@redirects, 2) %>

lib/publify_core/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module PublifyCore
-  VERSION = "10.0.1"
+  VERSION = "10.0.2"
 end

lib/theme.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class Theme
-  attr_accessor :name, :path, :description_html
+  attr_reader :name, :path
 
   def initialize(name, path)
     @name = name
@@ -16,18 +16,29 @@ def layout(action = :default)
   end
 
   def description
-    about_file = "#{path}/about.markdown"
-    if File.exist? about_file
-      File.read about_file
-    else
-      "### #{name}"
-    end
+    @description ||=
+      begin
+        about_file = theme_file("about.markdown")
+        if File.exist? about_file
+          File.read about_file
+        else
+          "### #{name}"
+        end
+      end
+  end
+
+  def description_html
+    TextFilter.markdown.filter_text(description)
   end
 
   def view_path
     "#{path}/views"
   end
 
+  def theme_file(filename)
+    File.join(path, filename)
+  end
+
   # Find a theme, given the theme name
   def self.find(name)
     registered_themes[name]

publify_core.gemspec

@@ -32,7 +32,7 @@ Gem::Specification.new do |s|
   s.add_dependency "html-pipeline", "~> 2.14"
   s.add_dependency "html-pipeline-hashtag", "~> 0.1.2"
   s.add_dependency "jquery-rails", ">= 4.5", "< 4.7"
-  s.add_dependency "jquery-ui-rails", ">= 6.0.1", "< 7.1.0"
+  s.add_dependency "jquery-ui-rails", "~> 7.0"
   s.add_dependency "kaminari", ["~> 1.2", ">= 1.2.1"]
   s.add_dependency "marcel", "~> 1.0.0"
   s.add_dependency "mini_magick", ["~> 4.9", ">= 4.9.4"]

spec/controllers/admin/redirects_controller_spec.rb

@@ -3,8 +3,9 @@
 require "rails_helper"
 
 RSpec.describe Admin::RedirectsController, type: :controller do
+  let!(:blog) { create(:blog) }
+
   before do
-    create(:blog)
     admin = create(:user, :as_admin)
     sign_in admin
   end
@@ -39,6 +40,16 @@
         create(:redirect)
         expect { get :index }.not_to raise_error
       end
+
+      it "links to the redirect target using the full target path" do
+        blog.update(base_url: "https://foo.bar/baz")
+        create(:redirect, to_path: "qux")
+        get :index
+        aggregate_failures do
+          expect(response.body).not_to have_link href: "qux"
+          expect(response.body).to have_link "qux", href: "/baz/qux"
+        end
+      end
     end
   end
 

spec/lib/theme_spec.rb

@@ -28,6 +28,13 @@
     end
   end
 
+  describe "#description_html" do
+    it "returns the contents of the about file processed as markdown" do
+      expect(default_theme.description_html)
+        .to start_with "<h4>Plain theme for Publify</h4>"
+    end
+  end
+
   describe ".find_all" do
     let(:theme_directories) do
       Dir.glob(PublifyCore::Engine.instance.root.join("themes/[a-zA-Z0-9]*"))

spec/models/redirect_spec.rb

@@ -3,24 +3,40 @@
 require "rails_helper"
 
 RSpec.describe Redirect, type: :model do
-  let(:blog) { create(:blog) }
-
-  it "redirects are unique" do
-    expect { blog.redirects.create!(from_path: "foo/bar", to_path: "/") }.not_to raise_error
-
-    redirect = blog.redirects.build(from_path: "foo/bar", to_path: "/")
-
-    expect(redirect).not_to be_valid
-    expect(redirect.errors[:from_path]).to eq(["has already been taken"])
-  end
-
   describe "#from_url" do
     it "is based on the blog's base_url" do
+      blog = Blog.new(base_url: "https://quuz.bar/foo")
       redirect = blog.redirects.build(from_path: "right/here", to_path: "over_there")
       expect(redirect.from_url).to eq "#{blog.base_url}/right/here"
     end
   end
 
+  describe "#full_to_path" do
+    it "returns to_path if it includes an http or https scheme" do
+      blog = Blog.new(base_url: "https://quuz.bar/")
+      redirect = described_class.new(to_path: "https://foo.baz/", blog: blog)
+      expect(redirect.full_to_path).to eq "https://foo.baz/"
+    end
+
+    it "includes the blog's root path" do
+      blog = Blog.new(base_url: "https://quuz.bar/foo")
+      redirect = described_class.new(to_path: "baz", blog: blog)
+      expect(redirect.full_to_path).to eq "/foo/baz"
+    end
+
+    it "makes malicious target paths safe" do
+      blog = Blog.new(base_url: "https://quuz.bar/")
+      redirect = described_class.new(to_path: "javascript:alert()", blog: blog)
+      expect(redirect.full_to_path).to eq "/javascript:alert()"
+    end
+
+    it "ignores the blog's root path if it is included in the redirect" do
+      blog = Blog.new(base_url: "https://quuz.bar/foo")
+      redirect = described_class.new(to_path: "/foo/baz", blog: blog)
+      expect(redirect.full_to_path).to eq "/foo/baz"
+    end
+  end
+
   describe "validations" do
     let(:redirect) { described_class.new }
 
@@ -31,5 +47,17 @@
     it "requires to_path to not be too long" do
       expect(redirect).to validate_length_of(:to_path).is_at_most(255)
     end
+
+    it "requires redirects to be unique" do
+      blog = create(:blog)
+      blog.redirects.create!(from_path: "foo/bar", to_path: "/")
+
+      redirect = blog.redirects.build(from_path: "foo/bar", to_path: "/")
+
+      aggregate_failures do
+        expect(redirect).not_to be_valid
+        expect(redirect.errors[:from_path]).to eq(["has already been taken"])
+      end
+    end
   end
 end