mirror of
https://github.com/postalserver/postal.git
synced 2026-10-18 18:26:06 +00:00
chore(deps): upgrade to Rails 7.2.3.2, Ruby 3.4.10 and latest gems
Upgrades Rails from 7.1.6 to 7.2.3.2, Ruby from 3.4.6 to 3.4.10, and brings the rest of the bundle up to date. Two things were broken by Rails 7.2 and are fixed here: * MigrationWaiter called `ActiveRecord::Base.connection.migration_context`, which moved to the connection pool in 7.2. This raised NoMethodError on boot for any deployment with `migration_waiter.enabled` set, and had no test covering it. * `config.action_dispatch.show_exceptions = false` was deprecated in 7.1 and removed in 7.2. Now `:none`. `json` is pinned to < 3. Active Support 7.2 passes `quirks_mode:` to both `JSON.generate` and `JSON.parse`, and json 3.0 removed that option, so an unpinned json resolves to 3.x and makes every `to_json` raise - webhook payloads included. Nothing else in the bundle constrains it. The pin can go when we reach Rails 8.1, which no longer passes the option. `dynamic_form` is dropped. It is unmaintained and from 1.3.1 requires Ruby < 3.4, so it could never be updated again. Ten views used it, so `error_messages_for` now lives in ErrorMessagesHelper and `f.error_messages` in a form builder extension. The rendered markup is unchanged apart from a heading and description the stylesheet had always hidden; the rules that hid them are removed too. `execjs` is unpinned. The `< 2.8` constraint dated from the Rails 6.0 upgrade and asset output is byte-identical without it. Notable gem majors: puma 8, haml 7, secure_headers 7, sentry 7, jwt 3, prometheus-client 5, highline 3, omniauth-rails_csrf_protection 2, rspec-rails 8, shoulda-matchers 8, rubocop 1.91. secure_headers 7 changes X-XSS-Protection from "1; mode=block" to "0", which is the correct modern default and matches Rails' own. `config.load_defaults` deliberately stays at 7.0. The pending 7.1 and 7.2 options are listed, commented out, in new_framework_defaults_7_1.rb and _7_2.rb so they can be adopted in stages - several change the signed cookie and cache formats and need a staged deploy. The vestigial new_framework_defaults_7_0.rb is removed, as `load_defaults 7.0` already applies all of it. Adds tests for the areas this touched, which were mostly uncovered: MigrationWaiter, the serialized columns that gate a defaults bump, JSON encoding, HAML template compilation for all 103 templates, page rendering, form error rendering and security headers. The assets Gemfile group now includes :test so the view layer can be exercised at all.
This commit is contained in:
@@ -0,0 +1,60 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
|
||||
RSpec.describe ErrorMessagesHelper, type: :helper do
|
||||
describe "#error_messages_for" do
|
||||
let(:user) { User.new }
|
||||
|
||||
context "when the record has no errors" do
|
||||
it "renders nothing" do
|
||||
expect(helper.error_messages_for(user)).to eq ""
|
||||
end
|
||||
end
|
||||
|
||||
context "when the record is nil" do
|
||||
it "renders nothing" do
|
||||
expect(helper.error_messages_for(nil)).to eq ""
|
||||
end
|
||||
end
|
||||
|
||||
context "when the record has errors" do
|
||||
before do
|
||||
user.valid?
|
||||
end
|
||||
|
||||
it "renders the errorExplanation container the stylesheet targets" do
|
||||
html = helper.error_messages_for(user)
|
||||
expect(html).to include %(<div id="errorExplanation" class="errorExplanation">)
|
||||
end
|
||||
|
||||
it "renders each full message as a list item" do
|
||||
html = helper.error_messages_for(user)
|
||||
user.errors.full_messages.each do |message|
|
||||
expect(html).to include "<li>#{ERB::Util.html_escape(message)}</li>"
|
||||
end
|
||||
end
|
||||
|
||||
it "renders one list item per error" do
|
||||
html = helper.error_messages_for(user)
|
||||
expect(html.scan("<li>").size).to eq user.errors.count
|
||||
end
|
||||
|
||||
it "returns an html_safe string" do
|
||||
expect(helper.error_messages_for(user)).to be_html_safe
|
||||
end
|
||||
end
|
||||
|
||||
context "when an error message contains HTML" do
|
||||
before do
|
||||
user.errors.add(:base, %q(x'"><script>alert(1)</script>))
|
||||
end
|
||||
|
||||
it "escapes it rather than emitting it verbatim" do
|
||||
html = helper.error_messages_for(user)
|
||||
expect(html).to_not include "<script>alert(1)</script>"
|
||||
expect(html).to include "<script>alert(1)</script>"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,34 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
|
||||
# Active Support 7.2 calls `JSON.generate(..., quirks_mode: true)` and
|
||||
# `JSON.parse(json, quirks_mode: true)`. The json gem removed `quirks_mode` in
|
||||
# 3.0, so an unpinned json resolves to 3.x and every `to_json` in the app raises
|
||||
# ArgumentError - webhook payloads and JSON API responses included. The Gemfile
|
||||
# pins json to < 3 because of this; these examples fail loudly if that pin is
|
||||
# removed before we are on a Rails version that no longer passes the option.
|
||||
RSpec.describe "JSON encoding" do
|
||||
it "encodes a hash" do
|
||||
expect({ "a" => 1 }.to_json).to eq '{"a":1}'
|
||||
end
|
||||
|
||||
it "encodes nested structures with mixed types" do
|
||||
payload = { "event" => "MessageSent", "timestamp" => 1.5, "payload" => { "list" => [1, "two", nil, true] } }
|
||||
expect(JSON.parse(payload.to_json)).to eq payload
|
||||
end
|
||||
|
||||
it "round trips through Active Support's encoder" do
|
||||
payload = { "uuid" => "abc-123", "nested" => { "x" => [1, 2] } }
|
||||
expect(ActiveSupport::JSON.decode(ActiveSupport::JSON.encode(payload))).to eq payload
|
||||
end
|
||||
|
||||
it "encodes a bare value, which is what quirks_mode allowed" do
|
||||
expect(ActiveSupport::JSON.encode("hello")).to eq '"hello"'
|
||||
expect(ActiveSupport::JSON.encode(42)).to eq "42"
|
||||
end
|
||||
|
||||
it "encodes times the way the webhook payload relies on" do
|
||||
expect(Time.at(1_700_000_000).utc.to_f.to_json).to eq "1700000000.0"
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,89 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
require "migration_waiter"
|
||||
|
||||
RSpec.describe MigrationWaiter do
|
||||
describe ".pending_migration_count" do
|
||||
# This talks to Active Record's migration APIs directly and is the part of
|
||||
# this class most likely to break on a Rails upgrade (the call moved from
|
||||
# the connection to the connection pool in Rails 7.2). The rest of this
|
||||
# class is stubbed, so this example is what actually exercises it.
|
||||
it "returns the number of migrations which have not been applied" do
|
||||
expect(described_class.pending_migration_count).to be_a Integer
|
||||
end
|
||||
|
||||
it "returns zero when the test database is up to date" do
|
||||
expect(described_class.pending_migration_count).to eq 0
|
||||
end
|
||||
end
|
||||
|
||||
describe ".wait" do
|
||||
before do
|
||||
allow(Postal.logger).to receive(:info)
|
||||
allow(described_class).to receive(:sleep)
|
||||
end
|
||||
|
||||
it "returns immediately when there are no pending migrations" do
|
||||
allow(described_class).to receive(:pending_migration_count).and_return(0)
|
||||
described_class.wait
|
||||
expect(described_class).to have_received(:pending_migration_count).once
|
||||
expect(described_class).to_not have_received(:sleep)
|
||||
end
|
||||
|
||||
it "waits until there are no pending migrations remaining" do
|
||||
allow(described_class).to receive(:pending_migration_count).and_return(2, 1, 0)
|
||||
described_class.wait
|
||||
expect(described_class).to have_received(:sleep).with(described_class::SLEEP_TIME).twice
|
||||
end
|
||||
|
||||
it "exits when migrations are still pending after the maximum number of attempts" do
|
||||
stub_const("#{described_class}::ATTEMPTS", 3)
|
||||
allow(described_class).to receive(:pending_migration_count).and_return(1)
|
||||
# Process.exit raises SystemExit for real. It has to do so here too,
|
||||
# otherwise the wait loop never terminates.
|
||||
allow(Process).to receive(:exit).and_raise(SystemExit)
|
||||
|
||||
expect { described_class.wait }.to raise_error(SystemExit)
|
||||
|
||||
expect(Process).to have_received(:exit).with(1)
|
||||
expect(described_class).to have_received(:sleep).twice
|
||||
end
|
||||
end
|
||||
|
||||
describe ".wait_if_appropriate" do
|
||||
before do
|
||||
allow(described_class).to receive(:wait)
|
||||
allow(described_class).to receive(:console?).and_return(false)
|
||||
allow(described_class).to receive(:rake_task?).and_return(false)
|
||||
allow(described_class).to receive(:rails_command?).and_return(false)
|
||||
end
|
||||
|
||||
context "when the migration waiter is not enabled" do
|
||||
it "does not wait" do
|
||||
allow(Postal::Config.migration_waiter).to receive(:enabled?).and_return(false)
|
||||
described_class.wait_if_appropriate
|
||||
expect(described_class).to_not have_received(:wait)
|
||||
end
|
||||
end
|
||||
|
||||
context "when the migration waiter is enabled" do
|
||||
before do
|
||||
allow(Postal::Config.migration_waiter).to receive(:enabled?).and_return(true)
|
||||
end
|
||||
|
||||
it "waits" do
|
||||
described_class.wait_if_appropriate
|
||||
expect(described_class).to have_received(:wait)
|
||||
end
|
||||
|
||||
%w[console? rake_task? rails_command?].each do |predicate|
|
||||
it "does not wait when #{predicate.delete_suffix('?')} is true" do
|
||||
allow(described_class).to receive(predicate).and_return(true)
|
||||
described_class.wait_if_appropriate
|
||||
expect(described_class).to_not have_received(:wait)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,68 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
|
||||
# Active Record's serialization defaults have changed across recent Rails
|
||||
# releases (`config.active_record.default_column_serializer` becomes `nil` in
|
||||
# the 7.1 defaults, which makes an implicit coder an error rather than YAML).
|
||||
# These columns hold data written by older versions of Postal, so the on-disk
|
||||
# format has to stay YAML. These examples pin both the format and the round trip
|
||||
# so a future framework defaults bump cannot change them silently.
|
||||
RSpec.describe "serialized columns" do
|
||||
shared_examples "a YAML serialized hash column" do |attribute|
|
||||
let(:value) { { "string" => "value", "number" => 123, "nested" => { "a" => [1, 2] } } }
|
||||
|
||||
def write_raw(record, attribute, raw)
|
||||
sql = described_class.sanitize_sql_array(
|
||||
["UPDATE #{described_class.table_name} SET #{attribute} = ? WHERE id = ?", raw, record.id]
|
||||
)
|
||||
described_class.connection.update(sql)
|
||||
end
|
||||
|
||||
def read_raw(record, attribute)
|
||||
described_class.connection.select_value(
|
||||
described_class.sanitize_sql_array(
|
||||
["SELECT #{attribute} FROM #{described_class.table_name} WHERE id = ?", record.id]
|
||||
)
|
||||
)
|
||||
end
|
||||
|
||||
it "defaults to an empty hash" do
|
||||
expect(described_class.new.public_send(attribute)).to eq({})
|
||||
end
|
||||
|
||||
it "round trips a hash through the database" do
|
||||
record.update!(attribute => value)
|
||||
expect(record.reload.public_send(attribute)).to eq value
|
||||
end
|
||||
|
||||
it "stores the value as YAML in the database" do
|
||||
record.update!(attribute => value)
|
||||
raw = read_raw(record, attribute)
|
||||
|
||||
expect(raw).to be_a String
|
||||
expect(YAML.safe_load(raw)).to eq value
|
||||
end
|
||||
|
||||
it "reads YAML which was written by a previous version of Postal" do
|
||||
write_raw(record, attribute, value.to_yaml)
|
||||
expect(record.reload.public_send(attribute)).to eq value
|
||||
end
|
||||
|
||||
it "rejects a value which is not a hash" do
|
||||
expect { record.update!(attribute => "not a hash") }.to raise_error ActiveRecord::SerializationTypeMismatch
|
||||
end
|
||||
end
|
||||
|
||||
describe Credential do
|
||||
let(:record) { create(:credential) }
|
||||
|
||||
include_examples "a YAML serialized hash column", :options
|
||||
end
|
||||
|
||||
describe WebhookRequest do
|
||||
let(:record) { create(:webhook_request) }
|
||||
|
||||
include_examples "a YAML serialized hash column", :payload
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,90 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
|
||||
# These examples exist to exercise the view layer end to end - HAML, the asset
|
||||
# pipeline, the layout, form helpers and the session middleware. The rest of the
|
||||
# request specs only assert on redirects, so without these nothing in the suite
|
||||
# would notice if a Rails upgrade broke rendering.
|
||||
RSpec.describe "rendering", type: :request do
|
||||
before do
|
||||
host! Postal::Config.postal.web_hostname
|
||||
end
|
||||
|
||||
describe "the login page" do
|
||||
it "renders" do
|
||||
get "/login"
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(response.media_type).to eq "text/html"
|
||||
expect(response.body).to include "<form"
|
||||
expect(response.body).to include "password"
|
||||
end
|
||||
|
||||
it "renders the layout including compiled assets" do
|
||||
get "/login"
|
||||
|
||||
expect(response.body).to include "<html"
|
||||
expect(response.body).to match(/<link[^>]+stylesheets?/i).or match(/<link[^>]+\.css/i)
|
||||
expect(response.body).to match(/<script[^>]+\.js/i)
|
||||
end
|
||||
end
|
||||
|
||||
describe "when not authenticated" do
|
||||
it "redirects to the login page" do
|
||||
get "/"
|
||||
|
||||
expect(response).to redirect_to("/login?return_to=%2F")
|
||||
end
|
||||
end
|
||||
|
||||
describe "when authenticated" do
|
||||
let(:user) { create(:user) }
|
||||
|
||||
before do
|
||||
post "/login", params: { email_address: user.email_address, password: "passw0rd" }
|
||||
end
|
||||
|
||||
it "renders the organizations index" do
|
||||
get "/"
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(response.body).to include "<html"
|
||||
end
|
||||
|
||||
it "renders the user settings page" do
|
||||
get "/settings"
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(response.body).to include user.email_address
|
||||
end
|
||||
|
||||
# `f.error_messages` and `error_messages_for` used to come from the
|
||||
# dynamic_form gem. The app provides them itself now (see
|
||||
# ErrorMessagesHelper and config/initializers/form_builder_extensions.rb),
|
||||
# so the rendered output is our responsibility to keep working.
|
||||
describe "a form which fails validation" do
|
||||
before do
|
||||
patch "/settings", params: {
|
||||
password: "passw0rd",
|
||||
user: { first_name: "", last_name: "Smith", email_address: user.email_address }
|
||||
}
|
||||
end
|
||||
|
||||
it "re-renders the form" do
|
||||
expect(response).to have_http_status(:ok)
|
||||
end
|
||||
|
||||
it "renders the validation errors in the errorExplanation container" do
|
||||
expect(response.body).to include %(<div id="errorExplanation" class="errorExplanation">)
|
||||
expect(response.body).to include "<li>First name can't be blank</li>"
|
||||
end
|
||||
end
|
||||
|
||||
it "does not render an error container on a form which has no errors" do
|
||||
get "/settings"
|
||||
|
||||
expect(response.body).to_not include "errorExplanation"
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,86 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
|
||||
# The security headers come from the secure_headers gem plus Rails' own
|
||||
# defaults, and both have changed them across major versions. Nothing else in
|
||||
# the suite looks at response headers, so pin them here.
|
||||
RSpec.describe "security headers", type: :request do
|
||||
before do
|
||||
host! Postal::Config.postal.web_hostname
|
||||
get "/login"
|
||||
end
|
||||
|
||||
it "responds successfully" do
|
||||
expect(response).to have_http_status(:ok)
|
||||
end
|
||||
|
||||
it "sets X-Frame-Options to sameorigin" do
|
||||
expect(response.headers["x-frame-options"]).to eq "sameorigin"
|
||||
end
|
||||
|
||||
it "sets X-Content-Type-Options to nosniff" do
|
||||
expect(response.headers["x-content-type-options"]).to eq "nosniff"
|
||||
end
|
||||
|
||||
it "disables the legacy XSS auditor rather than enabling it" do
|
||||
# secure_headers 7 changed this from "1; mode=block". `0` is correct: the
|
||||
# legacy auditor is deprecated and blocking mode was itself exploitable.
|
||||
expect(response.headers["x-xss-protection"]).to eq "0"
|
||||
end
|
||||
|
||||
it "sets X-Permitted-Cross-Domain-Policies to none" do
|
||||
expect(response.headers["x-permitted-cross-domain-policies"]).to eq "none"
|
||||
end
|
||||
|
||||
describe "the content security policy" do
|
||||
let(:csp) { response.headers["content-security-policy"] }
|
||||
|
||||
it "is present" do
|
||||
expect(csp).to be_present
|
||||
end
|
||||
|
||||
it "restricts script-src to self" do
|
||||
expect(csp).to include "script-src 'self'"
|
||||
end
|
||||
|
||||
it "restricts connect-src to self" do
|
||||
expect(csp).to include "connect-src 'self'"
|
||||
end
|
||||
|
||||
it "forbids object-src" do
|
||||
expect(csp).to include "object-src 'none'"
|
||||
end
|
||||
end
|
||||
|
||||
describe "cookies" do
|
||||
# Rack 3 returns set-cookie as a String for one cookie and an Array for
|
||||
# several, so normalise before looking for a particular cookie.
|
||||
def cookie_named(name)
|
||||
Array(response.headers["set-cookie"]).flat_map { |v| v.split("\n") }.find { |c| c.start_with?("#{name}=") }
|
||||
end
|
||||
|
||||
it "marks the browser id cookie httponly and samesite lax" do
|
||||
expect(cookie_named("browser_id")).to include "httponly"
|
||||
expect(cookie_named("browser_id")).to include "samesite=lax"
|
||||
end
|
||||
|
||||
context "once logged in" do
|
||||
let(:user) { create(:user) }
|
||||
|
||||
before do
|
||||
post "/login", params: { email_address: user.email_address, password: "passw0rd" }
|
||||
end
|
||||
|
||||
it "marks the authie session cookie httponly and samesite lax" do
|
||||
expect(cookie_named("user_session")).to include "httponly"
|
||||
expect(cookie_named("user_session")).to include "samesite=lax"
|
||||
end
|
||||
|
||||
it "marks the rails session cookie httponly and samesite lax" do
|
||||
expect(cookie_named("_postal_session")).to include "httponly"
|
||||
expect(cookie_named("_postal_session")).to include "samesite=lax"
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,46 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "rails_helper"
|
||||
require "haml"
|
||||
|
||||
# Compiles every HAML template and checks the Ruby it produces actually parses.
|
||||
# A HAML major version bump changes the compiler, and the request specs only
|
||||
# render a handful of pages, so without this most of app/views is unverified
|
||||
# until someone visits the page in production.
|
||||
RSpec.describe "HAML templates" do
|
||||
def compile(source)
|
||||
# Wrapped in a method body because layouts use `yield`, which is not legal
|
||||
# at the top level.
|
||||
RubyVM::AbstractSyntaxTree.parse("def _template\n#{Haml::Engine.new.call(source)}\nend")
|
||||
end
|
||||
|
||||
templates = Dir[Rails.root.join("app/views/**/*.haml")]
|
||||
|
||||
it "finds the templates to check" do
|
||||
expect(templates.size).to be > 50
|
||||
end
|
||||
|
||||
# Without this, a change in how HAML reports errors could quietly turn every
|
||||
# example below into a no-op that passes on anything.
|
||||
describe "the check itself" do
|
||||
it "rejects a template whose Ruby does not parse" do
|
||||
expect { compile("= foo(\n") }.to raise_error(SyntaxError)
|
||||
end
|
||||
|
||||
it "rejects a template with a malformed attribute hash" do
|
||||
expect { compile("%div{:a =>}\n") }.to raise_error(SyntaxError)
|
||||
end
|
||||
|
||||
it "accepts a valid template" do
|
||||
expect { compile("%div\n %p hello\n") }.to_not raise_error
|
||||
end
|
||||
end
|
||||
|
||||
templates.each do |path|
|
||||
relative = path.sub("#{Rails.root}/", "")
|
||||
|
||||
it "compiles #{relative}" do
|
||||
expect { compile(File.read(path)) }.to_not raise_error
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user